- flashrom-gerrit - mail.coreboot.org

[M] Change in flashrom[master]: parallel: Drop explicit fallback_chip_X boilerplate
by Edward O'Callaghan (Code Review) 12 Jan '23

12 Jan '23

Edward O'Callaghan has submitted this change. ( https://review.coreboot.org/c/flashrom/+/71745 ) ( 1 is the latest approved patch-set. No files were changed between the latest approved patch-set and the submitted one. )Change subject: parallel: Drop explicit fallback_chip_X boilerplate ...................................................................... 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(a)google.com> Reviewed-on: https://review.coreboot.org/c/flashrom/+/71745 Reviewed-by: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Tested-by: build bot (Jenkins) <no-reply(a)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(-) Approvals: build bot (Jenkins): Verified Stefan Reinauer: Looks good to me, approved 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(a)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 https://review.coreboot.org/c/flashrom/+/71745 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: If25c0048a07057aa72be6ffa8d8ad7f0a568dcf7 Gerrit-Change-Number: 71745 Gerrit-PatchSet: 3 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: merged

1 0

[M] Change in flashrom[master]: flashrom_tester: Remove --output log redirect option
by Evan Benn (Code Review) 12 Jan '23

12 Jan '23

Attention is currently required from: Edward O'Callaghan, Peter Marheine. Evan Benn has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/71773 ) Change subject: flashrom_tester: Remove --output log redirect option ...................................................................... Patch Set 2: (1 comment) Patchset: PS2: > This has been a pretty useful facility during AVL is `flashrom_tester > log_file` sufficiently useful? -- To view, visit https://review.coreboot.org/c/flashrom/+/71773 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I5eab8169644a16ba31b203e8607853c459f92978 Gerrit-Change-Number: 71773 Gerrit-PatchSet: 2 Gerrit-Owner: Evan Benn <evanbenn(a)google.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Comment-Date: Wed, 11 Jan 2023 22:46:53 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-MessageType: comment

1 0

[M] Change in flashrom[master]: sb600spi.c: Move promontory code into a mmap_read=yes mode
by Arthur Heymans (Code Review) 11 Jan '23

11 Jan '23

Attention is currently required from: Thomas Heijligen, Edward O'Callaghan. Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/71565 ) Change subject: sb600spi.c: Move promontory code into a mmap_read=yes mode ...................................................................... Patch Set 2: (1 comment) Commit Message: https://review.coreboot.org/c/flashrom/+/71565/comment/56fb5635_e0a7e86f PS2, Line 10: This is not : hardware specific > I believe it is hardware specific to controllers after a certain generation. i.e., supported on SPI100 controllers (Zen etc) however not supported on pre-SPI100 controllers was my understanding. I think the parts controlling the speed and mode might not apply, but it is memory mapped for sure. -- To view, visit https://review.coreboot.org/c/flashrom/+/71565 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I13eb5a646d3569170b3911ae7b3127cd3e6022aa Gerrit-Change-Number: 71565 Gerrit-PatchSet: 2 Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Comment-Date: Wed, 11 Jan 2023 14:58:24 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-MessageType: comment

1 0

[S] Change in flashrom[master]: layout: Rename find_romentry() -> romentry_exists()
by Edward O'Callaghan (Code Review) 11 Jan '23

11 Jan '23

Edward O'Callaghan has submitted this change. ( https://review.coreboot.org/c/flashrom/+/70893 ) ( 1 is the latest approved patch-set. No files were changed between the latest approved patch-set and the submitted one. )Change subject: layout: Rename find_romentry() -> romentry_exists() ...................................................................... layout: Rename find_romentry() -> romentry_exists() The functions purpose is to test for existence not to actually return the entry, therefore rename accordingly. Change-Id: Ibf14357c00717d1a7b6bc9c83e797fac125559c4 Signed-off-by: Edward O'Callaghan <quasisec(a)google.com> Reviewed-on: https://review.coreboot.org/c/flashrom/+/70893 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Felix Singer <felixsinger(a)posteo.net> --- M layout.c 1 file changed, 18 insertions(+), 2 deletions(-) Approvals: build bot (Jenkins): Verified Felix Singer: Looks good to me, approved diff --git a/layout.c b/layout.c index 6af0e41..4f02be6 100644 --- a/layout.c +++ b/layout.c @@ -209,7 +209,7 @@ } /* returns -1 if an entry is not found, 0 if found. */ -static int find_romentry(struct flashrom_layout *const l, char *name, char *file) +static int romentry_exists(struct flashrom_layout *const l, char *name, char *file) { if (!l->head) return -1; @@ -244,7 +244,7 @@ tmp = args; while (tmp) { - if (find_romentry(l, tmp->name, tmp->file) < 0) { + if (romentry_exists(l, tmp->name, tmp->file) < 0) { msg_gerr("Invalid region specified: \"%s\".\n", tmp->name); return 1; -- To view, visit https://review.coreboot.org/c/flashrom/+/70893 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ibf14357c00717d1a7b6bc9c83e797fac125559c4 Gerrit-Change-Number: 70893 Gerrit-PatchSet: 3 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: merged

1 0

[M] Change in flashrom[master]: flashrom_tester: Remove --output log redirect option
by Edward O'Callaghan (Code Review) 11 Jan '23

11 Jan '23

Attention is currently required from: Evan Benn, Peter Marheine. Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/71773 ) Change subject: flashrom_tester: Remove --output log redirect option ...................................................................... Patch Set 2: Code-Review-2 (1 comment) Patchset: PS2: This has been a pretty useful facility during AVL -- To view, visit https://review.coreboot.org/c/flashrom/+/71773 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I5eab8169644a16ba31b203e8607853c459f92978 Gerrit-Change-Number: 71773 Gerrit-PatchSet: 2 Gerrit-Owner: Evan Benn <evanbenn(a)google.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Evan Benn <evanbenn(a)google.com> Gerrit-Attention: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Comment-Date: Wed, 11 Jan 2023 12:28:05 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[S] Change in flashrom[master]: rust: Add license and other metadata to Cargo.toml
by Edward O'Callaghan (Code Review) 11 Jan '23

11 Jan '23

Attention is currently required from: Evan Benn. Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/71618 ) Change subject: rust: Add license and other metadata to Cargo.toml ...................................................................... Patch Set 1: Code-Review+2 -- To view, visit https://review.coreboot.org/c/flashrom/+/71618 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ibdab16713395509be511e45c5eae946496020429 Gerrit-Change-Number: 71618 Gerrit-PatchSet: 1 Gerrit-Owner: Evan Benn <evanbenn(a)google.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Evan Benn <evanbenn(a)google.com> Gerrit-Comment-Date: Wed, 11 Jan 2023 12:27:14 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[M] Change in flashrom[master]: flashrom.c:Add function to get a flattened view of the chip erase blocks
by Edward O'Callaghan (Code Review) 11 Jan '23

11 Jan '23

Attention is currently required from: Simon Buhrow, Nico Huber, Thomas Heijligen, Paul Menzel, Aarya. Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/65879 ) Change subject: flashrom.c:Add function to get a flattened view of the chip erase blocks ...................................................................... Patch Set 56: Code-Review+2 -- To view, visit https://review.coreboot.org/c/flashrom/+/65879 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Iafe78de00daa55f7114bd4ce09465dd88074ece4 Gerrit-Change-Number: 65879 Gerrit-PatchSet: 56 Gerrit-Owner: Aarya <aarya.chaumal(a)gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Simon Buhrow Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-CC: Nico Huber <nico.h(a)gmx.de> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Simon Buhrow Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Aarya <aarya.chaumal(a)gmail.com> Gerrit-Comment-Date: Wed, 11 Jan 2023 11:57:50 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[M] Change in flashrom[master]: flashrom_tester: Remove --output log redirect option
by Evan Benn (Code Review) 11 Jan '23

11 Jan '23

Attention is currently required from: Peter Marheine. Evan Benn has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/71773 ) Change subject: flashrom_tester: Remove --output log redirect option ...................................................................... Patch Set 2: This change is ready for review. -- To view, visit https://review.coreboot.org/c/flashrom/+/71773 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I5eab8169644a16ba31b203e8607853c459f92978 Gerrit-Change-Number: 71773 Gerrit-PatchSet: 2 Gerrit-Owner: Evan Benn <evanbenn(a)google.com> Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Comment-Date: Wed, 11 Jan 2023 03:26:09 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[M] Change in flashrom[master]: programmer: Add bitbanging programmer driver for Linux libgpiod
by Steve Markgraf (Code Review) 10 Jan '23

10 Jan '23

Steve Markgraf has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/71801 ) Change subject: programmer: Add bitbanging programmer driver for Linux libgpiod ...................................................................... programmer: Add bitbanging programmer driver for Linux libgpiod With this driver, any single board computer, old smartphone, etc. with a few spare GPIOs can be used for flashrom. An example invocation looks like this: flashrom -p linux_gpiod:gpiochip=0,cs=18,sck=19,mosi=13,miso=56 This uses /dev/gpiochip0 with the specified GPIO numbers for the SPI lines. All arguments are required. Reading of a 2048 kB flash chip on a Qualcomm MSM8916 SoC @800 MHz: Found GigaDevice flash chip "GD25LQ16" (2048 kB, SPI) on linux_gpiod. real 1m 33.96s Change-Id: Icad3eb7764f28feaea51bda3a7893da724c86d06 Signed-off-by: Steve Markgraf <steve(a)steve-m.de> --- M Makefile M include/programmer.h A linux_gpiod.c M programmer_table.c 4 files changed, 225 insertions(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/01/71801/1 diff --git a/Makefile b/Makefile index 425b58c..33de024 100644 --- a/Makefile +++ b/Makefile @@ -179,6 +179,9 @@ CONFIG_REALTEK_MST_I2C_SPI \ CONFIG_MEDIATEK_I2C_SPI \ +DEPENDS_ON_LIBGPIOD := \ + CONFIG_LINUX_GPIOD \ + ifeq ($(CONFIG_ENABLE_LIBUSB1_PROGRAMMERS), no) $(call disable_all,$(DEPENDS_ON_LIBUSB1)) endif @@ -233,6 +236,10 @@ CONFIG_LIBPCI_CFLAGS := $(call dependency_cflags, libpci) CONFIG_LIBPCI_LDFLAGS := $(call dependency_ldflags, libpci) +CONFIG_LIBGPIOD_VERSION := $(call dependency_version, libgpiod) +CONFIG_LIBGPIOD_CFLAGS := $(call dependency_cflags, libgpiod) +CONFIG_LIBGPIOD_LDFLAGS := $(call dependency_ldflags, libgpiod) + # Determine the destination OS, architecture and endian # IMPORTANT: The following lines must be placed before TARGET_OS, ARCH or ENDIAN # is ever used (of course), but should come after any lines setting CC because @@ -247,6 +254,7 @@ HAS_LIBJAYLINK := $(call find_dependency, libjaylink) HAS_LIBUSB1 := $(call find_dependency, libusb-1.0) HAS_LIBPCI := $(call find_dependency, libpci) +HAS_LIBGPIOD := $(call find_dependency, libgpiod) HAS_PCI_OLD_GET_DEV := $(call c_compile_test, Makefile.d/pci_old_get_dev_test.c, $(CONFIG_LIBPCI_CFLAGS)) HAS_FT232H := $(call c_compile_test, Makefile.d/ft232h_test.c, $(CONFIG_LIBFTDI1_CFLAGS)) @@ -350,6 +358,10 @@ $(call mark_unsupported,$(DEPENDS_ON_LIBUSB1)) endif +ifeq ($(HAS_LIBGPIOD), no) +$(call mark_unsupported,$(DEPENDS_ON_LIBGPIOD)) +endif + ifeq ($(HAS_SERIAL), no) $(call mark_unsupported, $(DEPENDS_ON_SERIAL)) endif @@ -506,9 +518,10 @@ # Always enable Marvell SATA controllers for now. CONFIG_SATAMV ?= yes -# Enable Linux spidev and MTD interfaces by default. We disable them on non-Linux targets. +# Enable Linux spidev, MTD and gpiod interfaces by default. We disable them on non-Linux targets. CONFIG_LINUX_MTD ?= yes CONFIG_LINUX_SPI ?= yes +CONFIG_LINUX_GPIOD ?= yes # Always enable ITE IT8212F PATA controllers for now. CONFIG_IT8212 ?= yes @@ -757,6 +770,11 @@ PROGRAMMER_OBJS += linux_spi.o endif +ifeq ($(CONFIG_LINUX_GPIOD), yes) +FEATURE_FLAGS += -D'CONFIG_LINUX_GPIOD=1' +PROGRAMMER_OBJS += linux_gpiod.o +endif + ifeq ($(CONFIG_MSTARDDC_SPI), yes) FEATURE_FLAGS += -D'CONFIG_MSTARDDC_SPI=1' PROGRAMMER_OBJS += mstarddc_spi.o @@ -865,6 +883,12 @@ endif endif +USE_LIBGPIOD := $(if $(call filter_deps,$(DEPENDS_ON_LIBGPIOD)),yes,no) +ifeq ($(USE_LIBGPIOD), yes) +override CFLAGS += $(CONFIG_LIBGPIOD_CFLAGS) +override LDFLAGS += $(CONFIG_LIBGPIOD_LDFLAGS) +endif + USE_LIB_NI845X := $(if $(call filter_deps,$(DEPENDS_ON_LIB_NI845X)),yes,no) ifeq ($(USE_LIB_NI845X), yes) override CFLAGS += $(CONFIG_LIB_NI845X_CFLAGS) @@ -943,6 +967,11 @@ echo " CFLAGS: $(CONFIG_LIBFTDI1_CFLAGS)"; \ echo " LDFLAGS: $(CONFIG_LIBFTDI1_LDFLAGS)"; \ fi + @echo Dependency libgpiod found: $(HAS_LIBGPIOD) $(CONFIG_LIBGPIOD_VERSION) + @if [ $(HAS_LIBGPIOD) = yes ]; then \ + echo " CFLAGS: $(CONFIG_LIBGPIOD_CFLAGS)"; \ + echo " LDFLAGS: $(CONFIG_LIBGPIOD_LDFLAGS)"; \ + fi @echo "Checking for header \"mtd/mtd-user.h\": $(HAS_LINUX_MTD)" @echo "Checking for header \"linux/spi/spidev.h\": $(HAS_LINUX_SPI)" @echo "Checking for header \"linux/i2c-dev.h\": $(HAS_LINUX_I2C)" diff --git a/include/programmer.h b/include/programmer.h index 67e7b4a..e364c63 100644 --- a/include/programmer.h +++ b/include/programmer.h @@ -72,6 +72,7 @@ extern const struct programmer_entry programmer_jlink_spi; extern const struct programmer_entry programmer_linux_mtd; extern const struct programmer_entry programmer_linux_spi; +extern const struct programmer_entry programmer_linux_gpiod; extern const struct programmer_entry programmer_parade_lspcon; extern const struct programmer_entry programmer_mediatek_i2c_spi; extern const struct programmer_entry programmer_mstarddc_spi; diff --git a/linux_gpiod.c b/linux_gpiod.c new file mode 100644 index 0000000..1e0fa67 --- /dev/null +++ b/linux_gpiod.c @@ -0,0 +1,165 @@ +/* + * This file is part of the flashrom project. + * + * Copyright (C) 2023 Steve Markgraf <steve(a)steve-m.de> + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + */ + +#include <stdlib.h> +#include <string.h> +#include <errno.h> +#include <gpiod.h> +#include "programmer.h" +#include "spi.h" +#include "flash.h" + +#define CONSUMER "flashrom" + +struct gpiod_spi_data { + struct gpiod_chip *chip; + struct gpiod_line_bulk bulk; + struct gpiod_line *cs_line, *sck_line, *mosi_line, *miso_line; +}; + +static void linux_gpiod_bitbang_set_cs(int val, void *spi_data) +{ + struct gpiod_spi_data *data = spi_data; + if (gpiod_line_set_value(data->cs_line, val) < 0) + msg_perr("Setting cs line failed\n"); +} + +static void linux_gpiod_bitbang_set_sck(int val, void *spi_data) +{ + struct gpiod_spi_data *data = spi_data; + if (gpiod_line_set_value(data->sck_line, val) < 0) + msg_perr("Setting sck line failed\n"); +} + +static void linux_gpiod_bitbang_set_mosi(int val, void *spi_data) +{ + struct gpiod_spi_data *data = spi_data; + if (gpiod_line_set_value(data->mosi_line, val) < 0) + msg_perr("Setting sck line failed\n"); +} + +static int linux_gpiod_bitbang_get_miso(void *spi_data) +{ + struct gpiod_spi_data *data = spi_data; + int r = gpiod_line_get_value(data->miso_line); + if (r < 0) + msg_perr("Getting miso line failed\n"); + return r; +} + +static const struct bitbang_spi_master bitbang_spi_master_gpiod = { + .set_cs = linux_gpiod_bitbang_set_cs, + .set_sck = linux_gpiod_bitbang_set_sck, + .set_mosi = linux_gpiod_bitbang_set_mosi, + .get_miso = linux_gpiod_bitbang_get_miso, +}; + +static int linux_gpiod_spi_shutdown(void *spi_data) +{ + struct gpiod_spi_data *data = spi_data; + + if (gpiod_line_bulk_num_lines(&data->bulk) > 0) + gpiod_line_release_bulk(&data->bulk); + + if (data->chip) + gpiod_chip_close(data->chip); + + free(data); + + return 0; +} + +static int linux_gpiod_spi_init(const struct programmer_cfg *cfg) +{ + struct gpiod_spi_data *data = NULL; + struct gpiod_chip *chip = NULL; + const char *param_str[] = { "cs", "sck", "mosi", "miso", "gpiochip" }; + unsigned int param_int[ARRAY_SIZE(param_str)]; + unsigned int i; + int r; + + for (i = 0; i < ARRAY_SIZE(param_str); i++) { + char *param = extract_programmer_param_str(cfg, param_str[i]); + if (param) { + param_int[i] = atoi(param); + free(param); + } else { + msg_perr("Missing required programmer parameter %s=<n>\n", param_str[i]); + goto err_exit; + } + } + + data = calloc(1, sizeof(*data)); + if (!data) { + msg_perr("Unable to allocate space for SPI master data\n"); + goto err_exit; + } + + chip = gpiod_chip_open_by_number(param_int[4]); + if (!chip) { + msg_perr("Failed to open gpiochip: %s\n", strerror(errno)); + goto err_exit; + } + + data->chip = chip; + + if (gpiod_chip_get_lines(chip, param_int, 4, &data->bulk)) { + msg_perr("Error getting GPIO lines\n"); + goto err_exit; + } + + data->cs_line = gpiod_line_bulk_get_line(&data->bulk, 0); + data->sck_line = gpiod_line_bulk_get_line(&data->bulk, 1); + data->mosi_line = gpiod_line_bulk_get_line(&data->bulk, 2); + data->miso_line = gpiod_line_bulk_get_line(&data->bulk, 3); + + r = gpiod_line_request_output(data->cs_line, CONSUMER, 1); + r |= gpiod_line_request_output(data->sck_line, CONSUMER, 1); + r |= gpiod_line_request_output(data->mosi_line, CONSUMER, 1); + r |= gpiod_line_request_input(data->miso_line, CONSUMER); + + if (r < 0) { + msg_perr("Requesting GPIO lines failed\n"); + goto err_exit; + } + + if (register_shutdown(linux_gpiod_spi_shutdown, data)) + goto err_exit; + + if (register_spi_bitbang_master(&bitbang_spi_master_gpiod, data)) + return 1; /* shutdown function does the cleanup */ + + return 0; + +err_exit: + if (data) { + if (gpiod_line_bulk_num_lines(&data->bulk) > 0) + gpiod_line_release_bulk(&data->bulk); + + free(data); + } + + if (chip) + gpiod_chip_close(chip); + return 1; +} + +const struct programmer_entry programmer_linux_gpiod = { + .name = "linux_gpiod", + .type = OTHER, + .devs.note = "Device file /dev/gpiochip<n>\n", + .init = linux_gpiod_spi_init, +}; diff --git a/programmer_table.c b/programmer_table.c index d58a155..848cd4e 100644 --- a/programmer_table.c +++ b/programmer_table.c @@ -124,6 +124,10 @@ &programmer_linux_spi, #endif +#if CONFIG_LINUX_GPIOD == 1 + &programmer_linux_gpiod, +#endif + #if CONFIG_PARADE_LSPCON == 1 &programmer_parade_lspcon, #endif -- To view, visit https://review.coreboot.org/c/flashrom/+/71801 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Icad3eb7764f28feaea51bda3a7893da724c86d06 Gerrit-Change-Number: 71801 Gerrit-PatchSet: 1 Gerrit-Owner: Steve Markgraf <steve(a)steve-m.de> Gerrit-MessageType: newchange

1 0

[S] Change in flashrom[master]: sfdp.c : Initializing hbuf to avoid warnings generated by scan-build
by Name of user not set (Code Review) 10 Jan '23

10 Jan '23

Attention is currently required from: eshankelkar(a)galorithm.com. Hello build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/71791 to look at the new patch set (#3). Change subject: sfdp.c : Initializing hbuf to avoid warnings generated by scan-build ...................................................................... sfdp.c : Initializing hbuf to avoid warnings generated by scan-build scan-build uses the clang analyzer which is giving warnings about garbage/undefined values in hbuf assigned to member of hdrs[0] if hbuf is not initialized. Though the path of the control flow shown by it in its html output cannot occur in reality(since it assumes that (nph+1)*8 is <= 0 i.e nph<=-1 but later assumes 0 <= nph ) hence its a false positive. Still initializing all bytes of hbuf to 0 leads to the two warnings for sfdp.c (one for hbuf, one for tbuf) to go away. Signed-off-by : Eshan Kelkar <eshankelkar(a)galorithm.com> Change-Id: I6815e246b4fd225d1837cae6e7d2aa0236b48b1b --- M sfdp.c 1 file changed, 28 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/91/71791/3 -- To view, visit https://review.coreboot.org/c/flashrom/+/71791 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I6815e246b4fd225d1837cae6e7d2aa0236b48b1b Gerrit-Change-Number: 71791 Gerrit-PatchSet: 3 Gerrit-Owner: eshankelkar(a)galorithm.com Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: eshankelkar(a)galorithm.com Gerrit-MessageType: newpatchset

1 0

2025

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit