Edward O'Callaghan submitted this change.

View Change



1 is the latest approved patch-set.
No files were changed between the latest approved patch-set and the submitted one.

Approvals: build bot (Jenkins): Verified Stefan Reinauer: Looks good to me, approved
parallel: Drop explicit fallback_chip_X boilerplate

A NULL func pointer is necessary and sufficient for the
condition `NULL func pointer => fallback_chip_X' as to not
need this explicit specification.

Therefore drop the explicit need to specify these fallback
callback function pointer in the par_master struct.
This is a reasonable default for every driver in the tree.

Furthermore, move the 'fallback_chip_X()' func from the
generic programmer.c register logic into its relevant
home of parallel.c and make static local to clean up
link-time symbol space.

This simplifies the code and driver development.

Change-Id: If25c0048a07057aa72be6ffa8d8ad7f0a568dcf7
Signed-off-by: Edward O'Callaghan <quasisec@google.com>
Reviewed-on: https://review.coreboot.org/c/flashrom/+/71745
Reviewed-by: Stefan Reinauer <stefan.reinauer@coreboot.org>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
---
M atahpt.c
M atapromise.c
M atavia.c
M drkaiser.c
M gfxnvidia.c
M include/programmer.h
M internal.c
M it8212.c
M nic3com.c
M nicintel.c
M nicnatsemi.c
M nicrealtek.c
M parallel.c
M programmer.c
M satamv.c
M satasii.c
M serprog.c
17 files changed, 98 insertions(+), 144 deletions(-)

diff --git a/atahpt.c b/atahpt.c
index 89b44c2..71fd4d5 100644
--- a/atahpt.c
+++ b/atahpt.c
@@ -74,13 +74,7 @@

static const struct par_master par_master_atahpt = {
.chip_readb = atahpt_chip_readb,
- .chip_readw = fallback_chip_readw,
- .chip_readl = fallback_chip_readl,
- .chip_readn = fallback_chip_readn,
.chip_writeb = atahpt_chip_writeb,
- .chip_writew = fallback_chip_writew,
- .chip_writel = fallback_chip_writel,
- .chip_writen = fallback_chip_writen,
.shutdown = atahpt_shutdown,
};

diff --git a/atapromise.c b/atapromise.c
index e1b8638..9beebf1 100644
--- a/atapromise.c
+++ b/atapromise.c
@@ -113,13 +113,7 @@

static const struct par_master par_master_atapromise = {
.chip_readb = atapromise_chip_readb,
- .chip_readw = fallback_chip_readw,
- .chip_readl = fallback_chip_readl,
- .chip_readn = fallback_chip_readn,
.chip_writeb = atapromise_chip_writeb,
- .chip_writew = fallback_chip_writew,
- .chip_writel = fallback_chip_writel,
- .chip_writen = fallback_chip_writen,
.shutdown = atapromise_shutdown,
};

diff --git a/atavia.c b/atavia.c
index 8af52d6..f35771a 100644
--- a/atavia.c
+++ b/atavia.c
@@ -135,13 +135,7 @@
static const struct par_master lpc_master_atavia = {
.map_flash_region = atavia_map,
.chip_readb = atavia_chip_readb,
- .chip_readw = fallback_chip_readw,
- .chip_readl = fallback_chip_readl,
- .chip_readn = fallback_chip_readn,
.chip_writeb = atavia_chip_writeb,
- .chip_writew = fallback_chip_writew,
- .chip_writel = fallback_chip_writel,
- .chip_writen = fallback_chip_writen,
};

static int atavia_init(const struct programmer_cfg *cfg)
diff --git a/drkaiser.c b/drkaiser.c
index 96f2c99..ebf5119 100644
--- a/drkaiser.c
+++ b/drkaiser.c
@@ -71,13 +71,7 @@

static const struct par_master par_master_drkaiser = {
.chip_readb = drkaiser_chip_readb,
- .chip_readw = fallback_chip_readw,
- .chip_readl = fallback_chip_readl,
- .chip_readn = fallback_chip_readn,
.chip_writeb = drkaiser_chip_writeb,
- .chip_writew = fallback_chip_writew,
- .chip_writel = fallback_chip_writel,
- .chip_writen = fallback_chip_writen,
.shutdown = drkaiser_shutdown,
};

diff --git a/gfxnvidia.c b/gfxnvidia.c
index 1d484ea..b420975 100644
--- a/gfxnvidia.c
+++ b/gfxnvidia.c
@@ -96,13 +96,7 @@

static const struct par_master par_master_gfxnvidia = {
.chip_readb = gfxnvidia_chip_readb,
- .chip_readw = fallback_chip_readw,
- .chip_readl = fallback_chip_readl,
- .chip_readn = fallback_chip_readn,
.chip_writeb = gfxnvidia_chip_writeb,
- .chip_writew = fallback_chip_writew,
- .chip_writel = fallback_chip_writel,
- .chip_writen = fallback_chip_writen,
.shutdown = gfxnvidia_shutdown,
};

diff --git a/include/programmer.h b/include/programmer.h
index 67e7b4a..281a4f6 100644
--- a/include/programmer.h
+++ b/include/programmer.h
@@ -442,12 +442,6 @@
int register_par_master(const struct par_master *mst, const enum chipbustype buses, void *data);

/* programmer.c */
-void fallback_chip_writew(const struct flashctx *flash, uint16_t val, chipaddr addr);
-void fallback_chip_writel(const struct flashctx *flash, uint32_t val, chipaddr addr);
-void fallback_chip_writen(const struct flashctx *flash, const uint8_t *buf, chipaddr addr, size_t len);
-uint16_t fallback_chip_readw(const struct flashctx *flash, const chipaddr addr);
-uint32_t fallback_chip_readl(const struct flashctx *flash, const chipaddr addr);
-void fallback_chip_readn(const struct flashctx *flash, uint8_t *buf, const chipaddr addr, size_t len);
struct registered_master {
enum chipbustype buses_supported;
struct {
diff --git a/internal.c b/internal.c
index 9b80f37..f21c4fc 100644
--- a/internal.c
+++ b/internal.c
@@ -116,7 +116,6 @@
.chip_writeb = internal_chip_writeb,
.chip_writew = internal_chip_writew,
.chip_writel = internal_chip_writel,
- .chip_writen = fallback_chip_writen,
};

static int get_params(const struct programmer_cfg *cfg,
diff --git a/it8212.c b/it8212.c
index 8fe2b59..3c1161d 100644
--- a/it8212.c
+++ b/it8212.c
@@ -64,13 +64,7 @@

static const struct par_master par_master_it8212 = {
.chip_readb = it8212_chip_readb,
- .chip_readw = fallback_chip_readw,
- .chip_readl = fallback_chip_readl,
- .chip_readn = fallback_chip_readn,
.chip_writeb = it8212_chip_writeb,
- .chip_writew = fallback_chip_writew,
- .chip_writel = fallback_chip_writel,
- .chip_writen = fallback_chip_writen,
.shutdown = it8212_shutdown,
};

diff --git a/nic3com.c b/nic3com.c
index efba979..a578d48 100644
--- a/nic3com.c
+++ b/nic3com.c
@@ -91,13 +91,7 @@

static const struct par_master par_master_nic3com = {
.chip_readb = nic3com_chip_readb,
- .chip_readw = fallback_chip_readw,
- .chip_readl = fallback_chip_readl,
- .chip_readn = fallback_chip_readn,
.chip_writeb = nic3com_chip_writeb,
- .chip_writew = fallback_chip_writew,
- .chip_writel = fallback_chip_writel,
- .chip_writen = fallback_chip_writen,
.shutdown = nic3com_shutdown,
};

diff --git a/nicintel.c b/nicintel.c
index 47edbb7..feb07b6 100644
--- a/nicintel.c
+++ b/nicintel.c
@@ -67,13 +67,7 @@

static const struct par_master par_master_nicintel = {
.chip_readb = nicintel_chip_readb,
- .chip_readw = fallback_chip_readw,
- .chip_readl = fallback_chip_readl,
- .chip_readn = fallback_chip_readn,
.chip_writeb = nicintel_chip_writeb,
- .chip_writew = fallback_chip_writew,
- .chip_writel = fallback_chip_writel,
- .chip_writen = fallback_chip_writen,
.shutdown = nicintel_shutdown,
};

diff --git a/nicnatsemi.c b/nicnatsemi.c
index efa879a..65377dc 100644
--- a/nicnatsemi.c
+++ b/nicnatsemi.c
@@ -78,13 +78,7 @@

static const struct par_master par_master_nicnatsemi = {
.chip_readb = nicnatsemi_chip_readb,
- .chip_readw = fallback_chip_readw,
- .chip_readl = fallback_chip_readl,
- .chip_readn = fallback_chip_readn,
.chip_writeb = nicnatsemi_chip_writeb,
- .chip_writew = fallback_chip_writew,
- .chip_writel = fallback_chip_writel,
- .chip_writen = fallback_chip_writen,
.shutdown = nicnatsemi_shutdown,
};

diff --git a/nicrealtek.c b/nicrealtek.c
index 103ea9e..5937e35 100644
--- a/nicrealtek.c
+++ b/nicrealtek.c
@@ -87,13 +87,7 @@

static const struct par_master par_master_nicrealtek = {
.chip_readb = nicrealtek_chip_readb,
- .chip_readw = fallback_chip_readw,
- .chip_readl = fallback_chip_readl,
- .chip_readn = fallback_chip_readn,
.chip_writeb = nicrealtek_chip_writeb,
- .chip_writew = fallback_chip_writew,
- .chip_writel = fallback_chip_writel,
- .chip_writen = fallback_chip_writen,
.shutdown = nicrealtek_shutdown,
};

diff --git a/parallel.c b/parallel.c
index cea9e7a..b193723 100644
--- a/parallel.c
+++ b/parallel.c
@@ -27,19 +27,49 @@
flash->mst->par.chip_writeb(flash, val, addr);
}

+/* Little-endian fallback for drivers not supporting 16 bit accesses */
+static void fallback_chip_writew(const struct flashctx *flash, uint16_t val,
+ chipaddr addr)
+{
+ chip_writeb(flash, val & 0xff, addr);
+ chip_writeb(flash, (val >> 8) & 0xff, addr + 1);
+}
+
void chip_writew(const struct flashctx *flash, uint16_t val, chipaddr addr)
{
- flash->mst->par.chip_writew(flash, val, addr);
+ if (flash->mst->par.chip_writew)
+ flash->mst->par.chip_writew(flash, val, addr);
+ fallback_chip_writew(flash, val, addr);
+}
+
+/* Little-endian fallback for drivers not supporting 32 bit accesses */
+static void fallback_chip_writel(const struct flashctx *flash, uint32_t val,
+ chipaddr addr)
+{
+ chip_writew(flash, val & 0xffff, addr);
+ chip_writew(flash, (val >> 16) & 0xffff, addr + 2);
}

void chip_writel(const struct flashctx *flash, uint32_t val, chipaddr addr)
{
- flash->mst->par.chip_writel(flash, val, addr);
+ if (flash->mst->par.chip_writel)
+ flash->mst->par.chip_writel(flash, val, addr);
+ fallback_chip_writel(flash, val, addr);
+}
+
+static void fallback_chip_writen(const struct flashctx *flash, const uint8_t *buf, chipaddr addr, size_t len)
+{
+ size_t i;
+ for (i = 0; i < len; i++)
+ chip_writeb(flash, buf[i], addr + i);
+ return;
}

void chip_writen(const struct flashctx *flash, const uint8_t *buf, chipaddr addr, size_t len)
{
- flash->mst->par.chip_writen(flash, buf, addr, len);
+ if (flash->mst->par.chip_writen)
+ flash->mst->par.chip_writen(flash, buf, addr, len);
+ fallback_chip_writen(flash, buf, addr, len);
}

uint8_t chip_readb(const struct flashctx *flash, const chipaddr addr)
@@ -47,20 +77,53 @@
return flash->mst->par.chip_readb(flash, addr);
}

+/* Little-endian fallback for drivers not supporting 16 bit accesses */
+static uint16_t fallback_chip_readw(const struct flashctx *flash, const chipaddr addr)
+{
+ uint16_t val;
+ val = chip_readb(flash, addr);
+ val |= chip_readb(flash, addr + 1) << 8;
+ return val;
+}
+
uint16_t chip_readw(const struct flashctx *flash, const chipaddr addr)
{
- return flash->mst->par.chip_readw(flash, addr);
+ if (flash->mst->par.chip_readw)
+ return flash->mst->par.chip_readw(flash, addr);
+ return fallback_chip_readw(flash, addr);
+}
+
+/* Little-endian fallback for drivers not supporting 32 bit accesses */
+static uint32_t fallback_chip_readl(const struct flashctx *flash, const chipaddr addr)
+{
+ uint32_t val;
+ val = chip_readw(flash, addr);
+ val |= chip_readw(flash, addr + 2) << 16;
+ return val;
}

uint32_t chip_readl(const struct flashctx *flash, const chipaddr addr)
{
- return flash->mst->par.chip_readl(flash, addr);
+ if (flash->mst->par.chip_readl)
+ return flash->mst->par.chip_readl(flash, addr);
+ return fallback_chip_readl(flash, addr);
+}
+
+static void fallback_chip_readn(const struct flashctx *flash, uint8_t *buf,
+ chipaddr addr, size_t len)
+{
+ size_t i;
+ for (i = 0; i < len; i++)
+ buf[i] = chip_readb(flash, addr + i);
+ return;
}

void chip_readn(const struct flashctx *flash, uint8_t *buf, chipaddr addr,
size_t len)
{
- flash->mst->par.chip_readn(flash, buf, addr, len);
+ if (flash->mst->par.chip_readn)
+ flash->mst->par.chip_readn(flash, buf, addr, len);
+ fallback_chip_readn(flash, buf, addr, len);
}

int register_par_master(const struct par_master *mst,
@@ -87,9 +150,7 @@
return ERROR_FLASHROM_BUG;
}

- if (!mst->chip_writeb || !mst->chip_writew || !mst->chip_writel ||
- !mst->chip_writen || !mst->chip_readb || !mst->chip_readw ||
- !mst->chip_readl || !mst->chip_readn) {
+ if (!mst->chip_writeb || !mst->chip_readb) {
msg_perr("%s called with incomplete master definition. "
"Please report a bug at flashrom@flashrom.org\n",
__func__);
diff --git a/programmer.c b/programmer.c
index d4f15a1..15fd1b8 100644
--- a/programmer.c
+++ b/programmer.c
@@ -17,57 +17,6 @@
#include "flash.h"
#include "programmer.h"

-/* Little-endian fallback for drivers not supporting 16 bit accesses */
-void fallback_chip_writew(const struct flashctx *flash, uint16_t val,
- chipaddr addr)
-{
- chip_writeb(flash, val & 0xff, addr);
- chip_writeb(flash, (val >> 8) & 0xff, addr + 1);
-}
-
-/* Little-endian fallback for drivers not supporting 16 bit accesses */
-uint16_t fallback_chip_readw(const struct flashctx *flash, const chipaddr addr)
-{
- uint16_t val;
- val = chip_readb(flash, addr);
- val |= chip_readb(flash, addr + 1) << 8;
- return val;
-}
-
-/* Little-endian fallback for drivers not supporting 32 bit accesses */
-void fallback_chip_writel(const struct flashctx *flash, uint32_t val,
- chipaddr addr)
-{
- chip_writew(flash, val & 0xffff, addr);
- chip_writew(flash, (val >> 16) & 0xffff, addr + 2);
-}
-
-/* Little-endian fallback for drivers not supporting 32 bit accesses */
-uint32_t fallback_chip_readl(const struct flashctx *flash, const chipaddr addr)
-{
- uint32_t val;
- val = chip_readw(flash, addr);
- val |= chip_readw(flash, addr + 2) << 16;
- return val;
-}
-
-void fallback_chip_writen(const struct flashctx *flash, const uint8_t *buf, chipaddr addr, size_t len)
-{
- size_t i;
- for (i = 0; i < len; i++)
- chip_writeb(flash, buf[i], addr + i);
- return;
-}
-
-void fallback_chip_readn(const struct flashctx *flash, uint8_t *buf,
- chipaddr addr, size_t len)
-{
- size_t i;
- for (i = 0; i < len; i++)
- buf[i] = chip_readb(flash, addr + i);
- return;
-}
-
/* The limit of 4 is totally arbitrary. */
#define MASTERS_MAX 4
struct registered_master registered_masters[MASTERS_MAX];
diff --git a/satamv.c b/satamv.c
index dcd53c7..fea4c2b 100644
--- a/satamv.c
+++ b/satamv.c
@@ -88,13 +88,7 @@

static const struct par_master par_master_satamv = {
.chip_readb = satamv_chip_readb,
- .chip_readw = fallback_chip_readw,
- .chip_readl = fallback_chip_readl,
- .chip_readn = fallback_chip_readn,
.chip_writeb = satamv_chip_writeb,
- .chip_writew = fallback_chip_writew,
- .chip_writel = fallback_chip_writel,
- .chip_writen = fallback_chip_writen,
.shutdown = satamv_shutdown,
};

diff --git a/satasii.c b/satasii.c
index 011562e..e881f96 100644
--- a/satasii.c
+++ b/satasii.c
@@ -95,13 +95,7 @@

static const struct par_master par_master_satasii = {
.chip_readb = satasii_chip_readb,
- .chip_readw = fallback_chip_readw,
- .chip_readl = fallback_chip_readl,
- .chip_readn = fallback_chip_readn,
.chip_writeb = satasii_chip_writeb,
- .chip_writew = fallback_chip_writew,
- .chip_writel = fallback_chip_writel,
- .chip_writen = fallback_chip_writen,
.shutdown = satasii_shutdown,
};

diff --git a/serprog.c b/serprog.c
index 4e131e5..5ddbe3d 100644
--- a/serprog.c
+++ b/serprog.c
@@ -594,13 +594,8 @@
static const struct par_master par_master_serprog = {
.map_flash_region = serprog_map,
.chip_readb = serprog_chip_readb,
- .chip_readw = fallback_chip_readw,
- .chip_readl = fallback_chip_readl,
.chip_readn = serprog_chip_readn,
.chip_writeb = serprog_chip_writeb,
- .chip_writew = fallback_chip_writew,
- .chip_writel = fallback_chip_writel,
- .chip_writen = fallback_chip_writen,
.delay = serprog_delay,
};


To view, visit change 71745. To unsubscribe, or for help writing mail filters, visit settings.

Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: If25c0048a07057aa72be6ffa8d8ad7f0a568dcf7
Gerrit-Change-Number: 71745
Gerrit-PatchSet: 3
Gerrit-Owner: Edward O'Callaghan <quasisec@chromium.org>
Gerrit-Reviewer: Edward O'Callaghan <quasisec@chromium.org>
Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer@coreboot.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org>
Gerrit-MessageType: merged