- flashrom-gerrit - mail.coreboot.org

[S] Change in flashrom[main]: flashchips.c: Add support for XM25RU256C
by Georg Gottleuber (Code Review) 28 May '24

28 May '24

Attention is currently required from: Anastasia Klimchuk, Nikolai Artemiev, Stefan Reinauer. Georg Gottleuber has posted comments on this change by Georg Gottleuber. ( https://review.coreboot.org/c/flashrom/+/82101?usp=email ) Change subject: flashchips.c: Add support for XM25RU256C ...................................................................... Patch Set 3: (2 comments) Patchset: PS3: I double-checked with the datasheet: looks good. Only one additional ".block_erase" mode with code 0x21 is mentioned, but I am unsure how to add it (SPI_BLOCK_ERASE_21). File flashchips.c: https://review.coreboot.org/c/flashrom/+/82101/comment/992bc5be_306fd6c6?us… : PS2, Line 21729: | FEATURE_4BA_EAR_C5C8 | FEATURE_4BA_READ | FEATURE_4BA_FAST_READ : | FEATURE_4BA_WRITE, > Sorry I didn't notice last time: please intend these two lines so that they align with the first fea […] Done -- To view, visit https://review.coreboot.org/c/flashrom/+/82101?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: I431474a662304d09438e274706d3fc9cfbbe0bd6 Gerrit-Change-Number: 82101 Gerrit-PatchSet: 3 Gerrit-Owner: Georg Gottleuber <ggo(a)tuxedo.de> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com> 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-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Comment-Date: Tue, 28 May 2024 10:57:41 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org>

1 0

[S] Change in flashrom[main]: flashchips.c: Add support for XM25RU256C
by Georg Gottleuber (Code Review) 28 May '24

28 May '24

Attention is currently required from: Georg Gottleuber, Nikolai Artemiev, Stefan Reinauer. Hello Anastasia Klimchuk, Nikolai Artemiev, Stefan Reinauer, build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/82101?usp=email to look at the new patch set (#3). The following approvals got outdated and were removed: Verified+1 by build bot (Jenkins) Change subject: flashchips.c: Add support for XM25RU256C ...................................................................... flashchips.c: Add support for XM25RU256C Tested read, write and erase with ch341a_spi programmer (and 1.8V adapter). Test log: https://paste.flashrom.org/view.php?id=3729 Change-Id: I431474a662304d09438e274706d3fc9cfbbe0bd6 Signed-off-by: Georg Gottleuber <ggo(a)tuxedocomputers.com> --- M flashchips.c M include/flashchips.h 2 files changed, 43 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/01/82101/3 -- To view, visit https://review.coreboot.org/c/flashrom/+/82101?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: newpatchset Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: I431474a662304d09438e274706d3fc9cfbbe0bd6 Gerrit-Change-Number: 82101 Gerrit-PatchSet: 3 Gerrit-Owner: Georg Gottleuber <ggo(a)tuxedo.de> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com> 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-Attention: Georg Gottleuber <ggo(a)tuxedo.de> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com>

1 0

[S] Change in flashrom[main]: Correct get_flash_region() to use inclusive upper bounds
by Hsuan-ting Chen (Code Review) 28 May '24

28 May '24

Attention is currently required from: Aarya, Nikolai Artemiev, Peter Marheine. Hsuan-ting Chen has posted comments on this change by Peter Marheine. ( https://review.coreboot.org/c/flashrom/+/82496?usp=email ) Change subject: Correct get_flash_region() to use inclusive upper bounds ...................................................................... Patch Set 7: Code-Review+1 -- To view, visit https://review.coreboot.org/c/flashrom/+/82496?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: Ia0ca867aef33b598c2697fc264807fa5e29683ec Gerrit-Change-Number: 82496 Gerrit-PatchSet: 7 Gerrit-Owner: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: Aarya <aarya.chaumal(a)gmail.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Hsuan-ting Chen <roccochen(a)google.com> Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Aarya <aarya.chaumal(a)gmail.com> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Attention: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Comment-Date: Tue, 28 May 2024 06:00:08 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes

1 0

[S] Change in flashrom[main]: Correct get_flash_region() to use inclusive upper bounds
by Anastasia Klimchuk (Code Review) 28 May '24

28 May '24

Attention is currently required from: Aarya, Hsuan-ting Chen, Nikolai Artemiev, Peter Marheine. Anastasia Klimchuk has posted comments on this change by Peter Marheine. ( https://review.coreboot.org/c/flashrom/+/82496?usp=email ) Change subject: Correct get_flash_region() to use inclusive upper bounds ...................................................................... Patch Set 7: Code-Review+2 -- To view, visit https://review.coreboot.org/c/flashrom/+/82496?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: Ia0ca867aef33b598c2697fc264807fa5e29683ec Gerrit-Change-Number: 82496 Gerrit-PatchSet: 7 Gerrit-Owner: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: Aarya <aarya.chaumal(a)gmail.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Hsuan-ting Chen <roccochen(a)google.com> Gerrit-Attention: Hsuan-ting Chen <roccochen(a)google.com> Gerrit-Attention: Aarya <aarya.chaumal(a)gmail.com> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Attention: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Comment-Date: Tue, 28 May 2024 05:50:53 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes

1 0

[XS] Change in flashrom[main]: Add clarification to struct flash_region on chipoff_t
by Hsuan-ting Chen (Code Review) 28 May '24

28 May '24

Attention is currently required from: Peter Marheine. Hsuan-ting Chen has posted comments on this change by Peter Marheine. ( https://review.coreboot.org/c/flashrom/+/82677?usp=email ) Change subject: Add clarification to struct flash_region on chipoff_t ...................................................................... Patch Set 1: Code-Review+1 -- To view, visit https://review.coreboot.org/c/flashrom/+/82677?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: I80b61a87ca31bd5a116224aadb4e211ee6841e1f Gerrit-Change-Number: 82677 Gerrit-PatchSet: 1 Gerrit-Owner: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Hsuan-ting Chen <roccochen(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Comment-Date: Tue, 28 May 2024 05:34:54 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes

1 0

[S] Change in flashrom[main]: Correct get_flash_region() to use inclusive upper bounds
by Peter Marheine (Code Review) 28 May '24

28 May '24

Attention is currently required from: Aarya, Anastasia Klimchuk, Hsuan-ting Chen, Nikolai Artemiev. Peter Marheine has posted comments on this change by Peter Marheine. ( https://review.coreboot.org/c/flashrom/+/82496?usp=email ) Change subject: Correct get_flash_region() to use inclusive upper bounds ...................................................................... Patch Set 7: (1 comment) Patchset: PS6: > I am wondering, this path in ichspi (ich_get_region) is the same in Chrome OS, is it possible to do […] I tested reading flash on a yaviks board both with and without this patch, and it looks okay. In both cases the debug messages correctly indicated the extent of the IFD regions that were being read. Added a TEST line to the commit message to reflect this. -- To view, visit https://review.coreboot.org/c/flashrom/+/82496?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: Ia0ca867aef33b598c2697fc264807fa5e29683ec Gerrit-Change-Number: 82496 Gerrit-PatchSet: 7 Gerrit-Owner: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: Aarya <aarya.chaumal(a)gmail.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Hsuan-ting Chen <roccochen(a)google.com> Gerrit-Attention: Hsuan-ting Chen <roccochen(a)google.com> Gerrit-Attention: Aarya <aarya.chaumal(a)gmail.com> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Comment-Date: Tue, 28 May 2024 05:27:03 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org>

1 0

[S] Change in flashrom[main]: Correct get_flash_region() to use inclusive upper bounds
by Peter Marheine (Code Review) 28 May '24

28 May '24

Attention is currently required from: Aarya, Anastasia Klimchuk, Hsuan-ting Chen, Nikolai Artemiev. Hello Aarya, Anastasia Klimchuk, Nikolai Artemiev, build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/82496?usp=email to look at the new patch set (#7). Change subject: Correct get_flash_region() to use inclusive upper bounds ...................................................................... Correct get_flash_region() to use inclusive upper bounds get_flash_region() emits a struct flash_region, which uses chipoff_t for the start and end addresses of a region. chipoff_t is defined as a valid flash address, so it was wrong to be setting the end address to start + len; this is clearly wrong in the case where there is a single region because setting end to the flash size generates an address that is beyond the end of the chip (due to zero-indexing). This changes the one actual implementation of .get_region in ichspi.c to use inclusive upper bounds, and corrects all callers of get_flash_region() to treat the upper bounds as inclusive. Overall this reduces complexity slightly by removing more downward adjustments by 1 than it needs to add upward adjustments. TEST=on yaviks, `flashrom -V -x` prints equivalent messages about "x region (0xZZZZ..0xZZZZ) is readable" before and after this patch. Change-Id: Ia0ca867aef33b598c2697fc264807fa5e29683ec Signed-off-by: Peter Marheine <pmarheine(a)chromium.org> --- M erasure_layout.c M flashrom.c M ichspi.c 3 files changed, 22 insertions(+), 22 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/96/82496/7 -- To view, visit https://review.coreboot.org/c/flashrom/+/82496?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: newpatchset Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: Ia0ca867aef33b598c2697fc264807fa5e29683ec Gerrit-Change-Number: 82496 Gerrit-PatchSet: 7 Gerrit-Owner: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: Aarya <aarya.chaumal(a)gmail.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Hsuan-ting Chen <roccochen(a)google.com> Gerrit-Attention: Hsuan-ting Chen <roccochen(a)google.com> Gerrit-Attention: Aarya <aarya.chaumal(a)gmail.com> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com>

1 0

[XS] Change in flashrom[main]: Add clarification to struct flash_region on chipoff_t
by Anastasia Klimchuk (Code Review) 28 May '24

28 May '24

Attention is currently required from: Peter Marheine. Anastasia Klimchuk has posted comments on this change by Peter Marheine. ( https://review.coreboot.org/c/flashrom/+/82677?usp=email ) Change subject: Add clarification to struct flash_region on chipoff_t ...................................................................... Patch Set 1: Code-Review+2 -- To view, visit https://review.coreboot.org/c/flashrom/+/82677?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: I80b61a87ca31bd5a116224aadb4e211ee6841e1f Gerrit-Change-Number: 82677 Gerrit-PatchSet: 1 Gerrit-Owner: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Comment-Date: Tue, 28 May 2024 04:29:27 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes

1 0

[S] Change in flashrom[main]: Correct get_flash_region() to use inclusive upper bounds
by Peter Marheine (Code Review) 28 May '24

28 May '24

Attention is currently required from: Aarya, Anastasia Klimchuk, Hsuan-ting Chen, Nikolai Artemiev. Peter Marheine has posted comments on this change by Peter Marheine. ( https://review.coreboot.org/c/flashrom/+/82496?usp=email ) Change subject: Correct get_flash_region() to use inclusive upper bounds ...................................................................... Patch Set 6: (2 comments) File flashrom.c: https://review.coreboot.org/c/flashrom/+/82496/comment/28a45641_d25f224b?us… : PS6, Line 2240: start = region->end + 1; > Just want to double-check: This and all other layout-related occurrences in `layout. […] I think that's right. I only identified `get_flash_region` as a problem, and that was added along with all the code that uses it at about the same time. File include/layout.h: -- To view, visit https://review.coreboot.org/c/flashrom/+/82496?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: Ia0ca867aef33b598c2697fc264807fa5e29683ec Gerrit-Change-Number: 82496 Gerrit-PatchSet: 6 Gerrit-Owner: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: Aarya <aarya.chaumal(a)gmail.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Hsuan-ting Chen <roccochen(a)google.com> Gerrit-Attention: Hsuan-ting Chen <roccochen(a)google.com> Gerrit-Attention: Aarya <aarya.chaumal(a)gmail.com> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Comment-Date: Tue, 28 May 2024 02:58:17 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Hsuan-ting Chen <roccochen(a)google.com>

1 0

[XS] Change in flashrom[main]: Add clarification to struct flash_region on chipoff_t
by Peter Marheine (Code Review) 28 May '24

28 May '24

Peter Marheine has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/82677?usp=email ) Change subject: Add clarification to struct flash_region on chipoff_t ...................................................................... Add clarification to struct flash_region on chipoff_t Although chipoff_t is fairly clearly documented on its own, it seems fairly frequent that developers will treat the end address of a flash region as an exclusive upper bound rather than the inclusive one it should be; for example CB:82496 fixes an incorrect use that affected multiple sites, and CB:73571 stemmed from a similar cause. Add a clarifying comment to call attention to this, to help programmers avoid making similar mistakes in the future. Change-Id: I80b61a87ca31bd5a116224aadb4e211ee6841e1f Signed-off-by: Peter Marheine <pmarheine(a)chromium.org> --- M include/layout.h 1 file changed, 5 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/77/82677/1 diff --git a/include/layout.h b/include/layout.h index 5614bea..d03a15b 100644 --- a/include/layout.h +++ b/include/layout.h @@ -37,6 +37,11 @@ struct flash_region { char *name; + /* + * Note that because start and end are chipoff_t, end is an inclusive upper + * bound: the length of a region is (end - start + 1) bytes and it is + * impossible to represent a region with zero length. + */ chipoff_t start; chipoff_t end; bool read_prot; -- To view, visit https://review.coreboot.org/c/flashrom/+/82677?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: newchange Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: I80b61a87ca31bd5a116224aadb4e211ee6841e1f Gerrit-Change-Number: 82677 Gerrit-PatchSet: 1 Gerrit-Owner: Peter Marheine <pmarheine(a)chromium.org>

1 0

2025

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit