- flashrom-gerrit - mail.coreboot.org

[S] Change in flashrom[master]: jedec.c: Drop branching non-zero programmer_delay() operands
by Edward O'Callaghan (Code Review) 25 Feb '23

25 Feb '23

Attention is currently required from: Stefan Reinauer. Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/72692 ) Change subject: jedec.c: Drop branching non-zero programmer_delay() operands ...................................................................... Patch Set 2: (1 comment) File jedec.c: https://review.coreboot.org/c/flashrom/+/72692/comment/46c61518_f7e9f280 PS1, Line 219: programmer_delay(flash, 10); > this makes a conditional 10 unit delay into an unconditional one. Was too aggressive with regex. Done. -- To view, visit https://review.coreboot.org/c/flashrom/+/72692 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ic547669bb16e6ace4fe283e07345fc2d7075d63e Gerrit-Change-Number: 72692 Gerrit-PatchSet: 2 Gerrit-Owner: 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-Attention: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Comment-Date: Sun, 26 Feb 2023 06:07:50 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-MessageType: comment

1 0

[S] Change in flashrom[master]: jedec.c: Provide better lexical scope to itermediates
by Edward O'Callaghan (Code Review) 25 Feb '23

25 Feb '23

Edward O'Callaghan has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/73285 ) Change subject: jedec.c: Provide better lexical scope to itermediates ...................................................................... jedec.c: Provide better lexical scope to itermediates Change-Id: I8e01d471bb33a933b80760df2c69a4bf3589ba76 Signed-off-by: Edward O'Callaghan <quasisec(a)google.com> --- M jedec.c 1 file changed, 16 insertions(+), 10 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/85/73285/1 diff --git a/jedec.c b/jedec.c index 01c401f..db2e3e8 100644 --- a/jedec.c +++ b/jedec.c @@ -38,13 +38,11 @@ static void toggle_ready_jedec_common(const struct flashctx *flash, chipaddr dst, unsigned int delay) { unsigned int i = 0; - uint8_t tmp1, tmp2; - - tmp1 = chip_readb(flash, dst) & 0x40; + uint8_t tmp1 = chip_readb(flash, dst) & 0x40; while (i++ < 0xFFFFFFF) { programmer_delay(flash, delay); - tmp2 = chip_readb(flash, dst) & 0x40; + uint8_t tmp2 = chip_readb(flash, dst) & 0x40; if (tmp1 == tmp2) { break; } @@ -76,12 +74,11 @@ uint8_t data) { unsigned int i = 0; - uint8_t tmp; data &= 0x80; while (i++ < 0xFFFFFFF) { - tmp = chip_readb(flash, dst) & 0x80; + uint8_t tmp = chip_readb(flash, dst) & 0x80; if (tmp == data) { break; } @@ -377,12 +374,11 @@ int write_jedec_1(struct flashctx *flash, const uint8_t *src, unsigned int start, unsigned int len) { - unsigned int i; int failed = 0; chipaddr dst = flash->virtual_memory + start; const chipaddr olddst = dst; - for (i = 0; i < len; i++) { + for (unsigned int i = 0; i < len; i++) { if (write_byte_program_jedec_common(flash, src, dst)) failed = 1; dst++, src++; @@ -444,7 +440,7 @@ int write_jedec(struct flashctx *flash, const uint8_t *buf, unsigned int start, int unsigned len) { - unsigned int i, starthere, lenhere; + unsigned int starthere, lenhere; /* FIXME: page_size is the wrong variable. We need max_writechunk_size * in struct flashctx to do this properly. All chips using * write_jedec have page_size set to max_writechunk_size, so @@ -462,7 +458,7 @@ * (start + len - 1) / page_size. Since we want to include that last * page as well, the loop condition uses <=. */ - for (i = start / page_size; i <= nwrites; i++) { + for (unsigned int i = start / page_size; i <= nwrites; i++) { /* Byte position of the first byte in the range in this page. */ /* starthere is an offset to the base address of the chip. */ starthere = max(start, i * page_size); -- To view, visit https://review.coreboot.org/c/flashrom/+/73285 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I8e01d471bb33a933b80760df2c69a4bf3589ba76 Gerrit-Change-Number: 73285 Gerrit-PatchSet: 1 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-MessageType: newchange

1 0

[S] Change in flashrom[master]: jedec.c: Tidy up whitespace and line wraps
by Edward O'Callaghan (Code Review) 25 Feb '23

25 Feb '23

Edward O'Callaghan has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/73284 ) Change subject: jedec.c: Tidy up whitespace and line wraps ...................................................................... jedec.c: Tidy up whitespace and line wraps Change-Id: I3f18609709e59cdfc5a853c6e1ae5f53aecacc1a Signed-off-by: Edward O'Callaghan <quasisec(a)google.com> --- M jedec.c 1 file changed, 16 insertions(+), 7 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/84/73284/1 diff --git a/jedec.c b/jedec.c index bb617e0..01c401f 100644 --- a/jedec.c +++ b/jedec.c @@ -305,11 +305,11 @@ int erase_sector_jedec(struct flashctx *flash, unsigned int page, unsigned int size) { - /* Issue the Sector Erase command */ + /* Issue the Sector Erase command */ issuecmd(flash, 0x80, 0); issuecmd(flash, 0x30, page); - /* wait for Toggle bit ready */ + /* Wait for Toggle bit ready */ toggle_ready_jedec_slow(flash); /* FIXME: Check the status register for errors. */ @@ -318,11 +318,11 @@ int erase_block_jedec(struct flashctx *flash, unsigned int block, unsigned int size) { - /* Issue the Sector Erase command */ + /* Issue the Block Erase command */ issuecmd(flash, 0x80, 0); issuecmd(flash, 0x50, block); - /* wait for Toggle bit ready */ + /* Wait for Toggle bit ready */ toggle_ready_jedec_slow(flash); /* FIXME: Check the status register for errors. */ @@ -333,12 +333,11 @@ int erase_chip_block_jedec(struct flashctx *flash, unsigned int addr, unsigned int blocksize) { if ((addr != 0) || (blocksize != flash->chip->total_size * 1024)) { - msg_cerr("%s called with incorrect arguments\n", - __func__); + msg_cerr("%s called with incorrect arguments\n", __func__); return -1; } - /* Issue the JEDEC Chip Erase command */ + /* Issue the JEDEC Chip Erase command */ issuecmd(flash, 0x80, 0); issuecmd(flash, 0x10, 0); -- To view, visit https://review.coreboot.org/c/flashrom/+/73284 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I3f18609709e59cdfc5a853c6e1ae5f53aecacc1a Gerrit-Change-Number: 73284 Gerrit-PatchSet: 1 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-MessageType: newchange

1 0

[S] Change in flashrom[master]: jedec.c: Add a little more const correctness
by Edward O'Callaghan (Code Review) 25 Feb '23

25 Feb '23

Edward O'Callaghan has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/73283 ) Change subject: jedec.c: Add a little more const correctness ...................................................................... jedec.c: Add a little more const correctness Change-Id: Ic9a76ce3734bd83399c95478a7c0bfc081211124 Signed-off-by: Edward O'Callaghan <quasisec(a)google.com> --- M jedec.c 1 file changed, 14 insertions(+), 5 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/83/73283/1 diff --git a/jedec.c b/jedec.c index 2cdde61..bb617e0 100644 --- a/jedec.c +++ b/jedec.c @@ -123,7 +123,7 @@ int probe_jedec_29gl(struct flashctx *flash) { const unsigned int mask = getaddrmask(flash->chip); - chipaddr bios = flash->virtual_memory; + const chipaddr bios = flash->virtual_memory; const struct flashchip *chip = flash->chip; /* Reset chip to a clean slate */ @@ -381,9 +381,8 @@ unsigned int i; int failed = 0; chipaddr dst = flash->virtual_memory + start; - chipaddr olddst; + const chipaddr olddst = dst; - olddst = dst; for (i = 0; i < len; i++) { if (write_byte_program_jedec_common(flash, src, dst)) failed = 1; @@ -452,8 +451,8 @@ * write_jedec have page_size set to max_writechunk_size, so * we're OK for now. */ - unsigned int page_size = flash->chip->page_size; - unsigned int nwrites = (start + len - 1) / page_size; + const unsigned int page_size = flash->chip->page_size; + const unsigned int nwrites = (start + len - 1) / page_size; /* Warning: This loop has a very unusual condition and body. * The loop needs to go through each page with at least one affected -- To view, visit https://review.coreboot.org/c/flashrom/+/73283 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ic9a76ce3734bd83399c95478a7c0bfc081211124 Gerrit-Change-Number: 73283 Gerrit-PatchSet: 1 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-MessageType: newchange

1 0

[S] Change in flashrom[master]: jedec.c: Rename func to jedec_write_page()
by Edward O'Callaghan (Code Review) 25 Feb '23

25 Feb '23

Edward O'Callaghan has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/73282 ) Change subject: jedec.c: Rename func to jedec_write_page() ...................................................................... jedec.c: Rename func to jedec_write_page() Change-Id: I1be83d5974e305bddceaa34b64e982b774ade0d2 Signed-off-by: Edward O'Callaghan <quasisec(a)google.com> --- M jedec.c 1 file changed, 12 insertions(+), 2 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/82/73282/1 diff --git a/jedec.c b/jedec.c index f9442ec..2cdde61 100644 --- a/jedec.c +++ b/jedec.c @@ -396,7 +396,7 @@ return failed; } -static int write_page_write_jedec_common(struct flashctx *flash, const uint8_t *src, +static int jedec_write_page(struct flashctx *flash, const uint8_t *src, unsigned int start, unsigned int page_size) { int tries = 0, failed; @@ -471,7 +471,7 @@ /* Length of bytes in the range in this page. */ lenhere = min(start + len, (i + 1) * page_size) - starthere; - if (write_page_write_jedec_common(flash, buf + starthere - start, starthere, lenhere)) + if (jedec_write_page(flash, buf + starthere - start, starthere, lenhere)) return 1; update_progress(flash, FLASHROM_PROGRESS_WRITE, i + 1, nwrites + 1); } -- To view, visit https://review.coreboot.org/c/flashrom/+/73282 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I1be83d5974e305bddceaa34b64e982b774ade0d2 Gerrit-Change-Number: 73282 Gerrit-PatchSet: 1 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-MessageType: newchange

1 0

[S] Change in flashrom[master]: jedec.c: Drop branching non-zero programmer_delay() operands
by Edward O'Callaghan (Code Review) 25 Feb '23

25 Feb '23

Attention is currently required from: Edward O'Callaghan. Hello build bot (Jenkins), Stefan Reinauer, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/72692 to look at the new patch set (#2). Change subject: jedec.c: Drop branching non-zero programmer_delay() operands ...................................................................... jedec.c: Drop branching non-zero programmer_delay() operands The programmer_delay() function is already tolerant upon zero delay values and will simply just return with a NOP. Therefore there is no need to branch. Change-Id: Ic547669bb16e6ace4fe283e07345fc2d7075d63e Signed-off-by: Edward O'Callaghan <quasisec(a)google.com> --- M jedec.c 1 file changed, 19 insertions(+), 10 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/92/72692/2 -- To view, visit https://review.coreboot.org/c/flashrom/+/72692 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ic547669bb16e6ace4fe283e07345fc2d7075d63e Gerrit-Change-Number: 72692 Gerrit-PatchSet: 2 Gerrit-Owner: 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-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-MessageType: newpatchset

1 0

[S] Change in flashrom[master]: flashrom.8.tmpl: add missing chip for ft2232_spi programmer
by Alexander Goncharov (Code Review) 25 Feb '23

25 Feb '23

Alexander Goncharov has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/73260 ) Change subject: flashrom.8.tmpl: add missing chip for ft2232_spi programmer ...................................................................... flashrom.8.tmpl: add missing chip for ft2232_spi programmer FTDI FT4233H chip was added in commit bc2e3b6b79dd1383cdf3d965df2fc5c0d6596b02 Change-Id: Id25f6444bfe871f7b01ae2caff23ce973b44567a Signed-off-by: Alexander Goncharov <chat(a)joursoir.net> --- M flashrom.8.tmpl 1 file changed, 14 insertions(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/60/73260/1 diff --git a/flashrom.8.tmpl b/flashrom.8.tmpl index 0eabee5..7a9784e 100644 --- a/flashrom.8.tmpl +++ b/flashrom.8.tmpl @@ -899,7 +899,7 @@ .SS .BR "ft2232_spi " programmer .IP -This module supports various programmers based on FTDI FT2232/FT4232H/FT232H chips including the DLP Design +This module supports various programmers based on FTDI FT2232/FT4232H/FT4233H/FT232H chips including the DLP Design DLP-USB1232H, openbiosprog-spi, Amontec JTAGkey/JTAGkey-tiny/JTAGkey-2, Dangerous Prototypes Bus Blaster, Olimex ARM-USB-TINY/-H, Olimex ARM-USB-OCD/-H, OpenMoko Neo1973 Debug board (V2+), TIAO/DIYGADGET USB Multi-Protocol Adapter (TUMPA), TUMPA Lite, GOEPEL PicoTAP, Google Servo v1/v2, Tin Can Tools -- To view, visit https://review.coreboot.org/c/flashrom/+/73260 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Id25f6444bfe871f7b01ae2caff23ce973b44567a Gerrit-Change-Number: 73260 Gerrit-PatchSet: 1 Gerrit-Owner: Alexander Goncharov <chat(a)joursoir.net> Gerrit-MessageType: newchange

1 0

[S] Change in flashrom[master]: MAINTAINERS: Update unit tests from Supported to Maintained
by Thomas Heijligen (Code Review) 25 Feb '23

25 Feb '23

Attention is currently required from: Felix Singer, Angel Pons, Anastasia Klimchuk. Thomas Heijligen has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/73277 ) Change subject: MAINTAINERS: Update unit tests from Supported to Maintained ...................................................................... Patch Set 1: Code-Review+2 -- To view, visit https://review.coreboot.org/c/flashrom/+/73277 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I13853c6c48eb4054d2ed7d79bcfc768684914797 Gerrit-Change-Number: 73277 Gerrit-PatchSet: 1 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Sat, 25 Feb 2023 13:07:47 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[M] Change in flashrom[master]: flashrom: rewrite flashbuses_to_text()
by Alexander Goncharov (Code Review) 25 Feb '23

25 Feb '23

Hello build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/73039 to look at the new patch set (#3). Change subject: flashrom: rewrite flashbuses_to_text() ...................................................................... flashrom: rewrite flashbuses_to_text() The previous implementation had no error handling, as a result the flashrom could crash if the computer ran out of memory. The new version returns NULL in such cases. Also rewrite lots of `if` conditions to one cycle, store a name of buses in an array. The caller always expected a non-null value, so change its behavior to handle possible null value. As far as `printf()` and `free()` can handle null pointers, do nothing with such callers. Change-Id: I59b9044c99b4ba6c00d8c97f1e91af09d70dce2c Signed-off-by: Alexander Goncharov <chat(a)joursoir.net> Ticket: https://ticket.coreboot.org/issues/408 --- M flashrom.c M print.c 2 files changed, 69 insertions(+), 20 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/39/73039/3 -- To view, visit https://review.coreboot.org/c/flashrom/+/73039 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I59b9044c99b4ba6c00d8c97f1e91af09d70dce2c Gerrit-Change-Number: 73039 Gerrit-PatchSet: 3 Gerrit-Owner: Alexander Goncharov <chat(a)joursoir.net> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: newpatchset

1 0

[L] Change in flashrom[master]: ch347_spi: Add initial support for the WCH CH347
by Alexander Goncharov (Code Review) 25 Feb '23

25 Feb '23

Attention is currently required from: Felix Singer, Angel Pons, qianfan, Nicholas Chin. Alexander Goncharov has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/70573 ) Change subject: ch347_spi: Add initial support for the WCH CH347 ...................................................................... Patch Set 8: Code-Review+1 -- To view, visit https://review.coreboot.org/c/flashrom/+/70573 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I31b86c41076cc45d4a416a73fa1131350fb745ba Gerrit-Change-Number: 70573 Gerrit-PatchSet: 8 Gerrit-Owner: Nicholas Chin <nic.c3.14(a)gmail.com> Gerrit-Reviewer: Alexander Goncharov <chat(a)joursoir.net> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Nicholas Chin <nic.c3.14(a)gmail.com> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: qianfan <qianfanguijin(a)163.com> Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: qianfan <qianfanguijin(a)163.com> Gerrit-Attention: Nicholas Chin <nic.c3.14(a)gmail.com> Gerrit-Comment-Date: Sat, 25 Feb 2023 09:29:44 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

2025

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit