- flashrom-gerrit - mail.coreboot.org

[S] Change in flashrom[main]: flashchips.c: Add reg_bits for W25Q256JW
by Hsuan-ting Chen (Code Review) 27 Nov '24

27 Nov '24

Attention is currently required from: Hsuan Ting Chen, Kapil Porwal. Hsuan-ting Chen has posted comments on this change by Kapil Porwal. ( https://review.coreboot.org/c/flashrom/+/85323?usp=email ) Change subject: flashchips.c: Add reg_bits for W25Q256JW ...................................................................... Patch Set 1: Code-Review+2 (1 comment) Commit Message: https://review.coreboot.org/c/flashrom/+/85323/comment/418a0007_f87a2ca6?us… : PS1, Line 11: Please add the link-to datasheet, e.g. https://www.winbond.com/hq/support/documentation/levelOne.jsp?__locale=en&D… -- To view, visit https://review.coreboot.org/c/flashrom/+/85323?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: I050754b28a90911a50f891869297524ce9a6720e Gerrit-Change-Number: 85323 Gerrit-PatchSet: 1 Gerrit-Owner: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Hsuan Ting Chen <roccochen(a)chromium.org> Gerrit-Reviewer: Hsuan-ting Chen <roccochen(a)google.com> Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Hsuan Ting Chen <roccochen(a)chromium.org> Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Comment-Date: Wed, 27 Nov 2024 06:40:33 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes

1 0

[S] Change in flashrom[main]: flashchips.c: Add reg_bits for W25Q256JW
by Subrata Banik (Code Review) 27 Nov '24

27 Nov '24

Attention is currently required from: Kapil Porwal. Subrata Banik has posted comments on this change by Kapil Porwal. ( https://review.coreboot.org/c/flashrom/+/85323?usp=email ) Change subject: flashchips.c: Add reg_bits for W25Q256JW ...................................................................... Patch Set 1: Code-Review+1 -- To view, visit https://review.coreboot.org/c/flashrom/+/85323?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: I050754b28a90911a50f891869297524ce9a6720e Gerrit-Change-Number: 85323 Gerrit-PatchSet: 1 Gerrit-Owner: Kapil Porwal <kapilporwal(a)google.com> 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: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Comment-Date: Wed, 27 Nov 2024 04:57:47 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes

1 0

[S] Change in flashrom[main]: flashchips.c: Add reg_bits for W25Q256JW
by Kapil Porwal (Code Review) 26 Nov '24

26 Nov '24

Kapil Porwal has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/85323?usp=email ) Change subject: flashchips.c: Add reg_bits for W25Q256JW ...................................................................... flashchips.c: Add reg_bits for W25Q256JW Add reg_bits for W25Q256JW as per the datasheet. The register definitions are same as W25Q256JW_DTR. BUG=none Change-Id: I050754b28a90911a50f891869297524ce9a6720e Signed-off-by: Kapil Porwal <kapilporwal(a)google.com> --- M flashchips.c 1 file changed, 13 insertions(+), 2 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/23/85323/1 diff --git a/flashchips.c b/flashchips.c index 0e83200..ff2d834 100644 --- a/flashchips.c +++ b/flashchips.c @@ -20336,8 +20336,9 @@ .page_size = 256, /* supports SFDP */ /* OTP: 1024B total, 256B reserved; read 0x48; write 0x42, erase 0x44, read ID 0x4B */ - .feature_bits = FEATURE_WRSR_WREN | FEATURE_OTP | FEATURE_4BA, - .tested = TEST_OK_PREW, + .feature_bits = FEATURE_WRSR_WREN | FEATURE_OTP | FEATURE_4BA | FEATURE_WRSR2 + | FEATURE_WRSR3, + .tested = TEST_OK_PREWB, .probe = PROBE_SPI_RDID, .probe_timing = TIMING_ZERO, .block_erasers = @@ -20370,6 +20371,16 @@ .write = SPI_CHIP_WRITE256, .read = SPI_CHIP_READ, .voltage = {1650, 1950}, + .reg_bits = + { + .srp = {STATUS1, 7, RW}, + .srl = {STATUS2, 0, RW}, + .bp = {{STATUS1, 2, RW}, {STATUS1, 3, RW}, {STATUS1, 4, RW}, {STATUS1, 5, RW}}, + .tb = {STATUS1, 6, RW}, + .cmp = {STATUS2, 6, RW}, + .wps = {STATUS3, 2, RW}, + }, + .decode_range = DECODE_RANGE_SPI25, }, { -- To view, visit https://review.coreboot.org/c/flashrom/+/85323?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: I050754b28a90911a50f891869297524ce9a6720e Gerrit-Change-Number: 85323 Gerrit-PatchSet: 1 Gerrit-Owner: Kapil Porwal <kapilporwal(a)google.com>

1 0

[S] Change in flashrom[main]: Extract cli_output declarations to a separate header.
by Anastasia Klimchuk (Code Review) 26 Nov '24

26 Nov '24

Attention is currently required from: Antonio Vázquez Blanco. Anastasia Klimchuk has posted comments on this change by Antonio Vázquez Blanco. ( https://review.coreboot.org/c/flashrom/+/85134?usp=email ) Change subject: Extract cli_output declarations to a separate header. ...................................................................... Patch Set 2: Code-Review+2 (1 comment) Patchset: PS2: I leave this unresolved comment as a note this patch will be submitted after v1.5.0 tag is done. ETA 6th December. Perhaps you already know, but just in case, we are now preparing a release and only merging bugfixes. See the thread for details: https://mail.coreboot.org/hyperkitty/list/flashrom@flashrom.org/thread/WM4C… So this is a special time now. At normal time, merging would be in 3 days, process documented here: https://flashrom.org/dev_guide/development_guide.html#merging-patches -- To view, visit https://review.coreboot.org/c/flashrom/+/85134?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: I4209d5ed205ca14c39e83aa923e103b7282a7059 Gerrit-Change-Number: 85134 Gerrit-PatchSet: 2 Gerrit-Owner: Antonio Vázquez Blanco <antoniovazquezblanco(a)gmail.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Antonio Vázquez Blanco <antoniovazquezblanco(a)gmail.com> Gerrit-Comment-Date: Tue, 26 Nov 2024 02:34:09 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes

1 0

[M] Change in flashrom[main]: cli_classic.c: Make -r/-w/-v argument optional when using -i
by Anastasia Klimchuk (Code Review) 26 Nov '24

26 Nov '24

Attention is currently required from: Matt DeVillier. Anastasia Klimchuk has posted comments on this change by Matt DeVillier. ( https://review.coreboot.org/c/flashrom/+/85159?usp=email ) Change subject: cli_classic.c: Make -r/-w/-v argument optional when using -i ...................................................................... Patch Set 8: Code-Review+2 (1 comment) Patchset: PS8: I leave this unresolved comment as a note this patch will be submitted after v1.5.0 tag is done. ETA 6th December. Perhaps you already know, but just in case, we are now preparing a release and only merging bugfixes. See the thread for details: https://mail.coreboot.org/hyperkitty/list/flashrom@flashrom.org/thread/WM4C… So this is a special time now. At normal time, merging would be in 3 days, process documented here: https://flashrom.org/dev_guide/development_guide.html#merging-patches -- To view, visit https://review.coreboot.org/c/flashrom/+/85159?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: I6eba095d478f1a7bdbc3854627a656f93dd9e452 Gerrit-Change-Number: 85159 Gerrit-PatchSet: 8 Gerrit-Owner: Matt DeVillier <matt.devillier(a)gmail.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Matt DeVillier <matt.devillier(a)gmail.com> Gerrit-Comment-Date: Tue, 26 Nov 2024 02:29:27 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes

1 0

[S] Change in flashrom[main]: libflashrom: Fix comparison of layout romentry regions
by Anastasia Klimchuk (Code Review) 26 Nov '24

26 Nov '24

Anastasia Klimchuk has posted comments on this change by Anastasia Klimchuk. ( https://review.coreboot.org/c/flashrom/+/85292?usp=email ) Change subject: libflashrom: Fix comparison of layout romentry regions ...................................................................... Patch Set 1: (1 comment) File libflashrom.c: https://review.coreboot.org/c/flashrom/+/85292/comment/c1a1ccc7_5db26e7f?us… : PS1, Line 309: static int compare_region_with_dump(const struct romentry *const a, const struct romentry *const b) > It should always be zeroed, but that's true because `flashrom_layout_new` uses `calloc` and that doe […] Alright, then I leave it as is. Thank you. -- To view, visit https://review.coreboot.org/c/flashrom/+/85292?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: I715969036c8e516aac8d90b46830f1f92ae6a160 Gerrit-Change-Number: 85292 Gerrit-PatchSet: 1 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Tue, 26 Nov 2024 01:48:52 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org> Comment-In-Reply-To: Peter Marheine <pmarheine(a)chromium.org>

1 0

[S] Change in flashrom[main]: libflashrom: Fix comparison of layout romentry regions
by Peter Marheine (Code Review) 25 Nov '24

25 Nov '24

Attention is currently required from: Anastasia Klimchuk. Peter Marheine has posted comments on this change by Anastasia Klimchuk. ( https://review.coreboot.org/c/flashrom/+/85292?usp=email ) Change subject: libflashrom: Fix comparison of layout romentry regions ...................................................................... Patch Set 1: Code-Review+2 (1 comment) File libflashrom.c: https://review.coreboot.org/c/flashrom/+/85292/comment/ec85ff10_0d9cbde8?us… : PS1, Line 309: static int compare_region_with_dump(const struct romentry *const a, const struct romentry *const b) > I am wondering whether `read_prot` and `write_prot` should be here. […] It should always be zeroed, but that's true because `flashrom_layout_new` uses `calloc` and that doesn't seem like a guarantee the API actually makes- it would be easy to change things to not always zero a layout on creation and introduce a bug if this were to be reading fields that aren't meaningful, so I'm leaning toward not checking those fields. -- To view, visit https://review.coreboot.org/c/flashrom/+/85292?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: I715969036c8e516aac8d90b46830f1f92ae6a160 Gerrit-Change-Number: 85292 Gerrit-PatchSet: 1 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Mon, 25 Nov 2024 22:48:05 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org>

1 0

[M] Change in flashrom[main]: cli_classic.c: Make -r/-w/-v argument optional when using -i
by Matt DeVillier (Code Review) 25 Nov '24

25 Nov '24

Attention is currently required from: Anastasia Klimchuk. Matt DeVillier has posted comments on this change by Matt DeVillier. ( https://review.coreboot.org/c/flashrom/+/85159?usp=email ) Change subject: cli_classic.c: Make -r/-w/-v argument optional when using -i ...................................................................... Patch Set 8: (1 comment) File doc/classic_cli_manpage.rst: https://review.coreboot.org/c/flashrom/+/85159/comment/a30238cf_43736a37?us… : PS5, Line 227: If filenames are specified for -r and -i args, then: > That is only repro when generating manpage from rst, html is fine (I couldn't find in the tool the o […] Done -- To view, visit https://review.coreboot.org/c/flashrom/+/85159?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: I6eba095d478f1a7bdbc3854627a656f93dd9e452 Gerrit-Change-Number: 85159 Gerrit-PatchSet: 8 Gerrit-Owner: Matt DeVillier <matt.devillier(a)gmail.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Mon, 25 Nov 2024 14:23:47 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Matt DeVillier <matt.devillier(a)gmail.com> Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org>

1 0

[S] Change in flashrom[main]: libflashrom: Fix comparison of layout romentry regions
by Anastasia Klimchuk (Code Review) 25 Nov '24

25 Nov '24

Attention is currently required from: Peter Marheine. Anastasia Klimchuk has posted comments on this change by Anastasia Klimchuk. ( https://review.coreboot.org/c/flashrom/+/85292?usp=email ) Change subject: libflashrom: Fix comparison of layout romentry regions ...................................................................... Patch Set 1: (1 comment) File libflashrom.c: https://review.coreboot.org/c/flashrom/+/85292/comment/6f73d737_d7317e88?us… : PS1, Line 309: static int compare_region_with_dump(const struct romentry *const a, const struct romentry *const b) I am wondering whether `read_prot` and `write_prot` should be here. There are not filled in from dump, but that means they will have default values, and always the same? (so, won't make an effect on comparison). -- To view, visit https://review.coreboot.org/c/flashrom/+/85292?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: I715969036c8e516aac8d90b46830f1f92ae6a160 Gerrit-Change-Number: 85292 Gerrit-PatchSet: 1 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Attention: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Comment-Date: Mon, 25 Nov 2024 10:33:26 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No

1 0

[S] Change in flashrom[main]: libflashrom: Fix comparison of layout romentry regions
by Anastasia Klimchuk (Code Review) 25 Nov '24

25 Nov '24

Anastasia Klimchuk has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/85292?usp=email ) Change subject: libflashrom: Fix comparison of layout romentry regions ...................................................................... libflashrom: Fix comparison of layout romentry regions Comparing structs (romentries in this case) with memcmp won't work if the struct includes pointers. Also in this case romentry region is compared to the one loaded from dump, and from dump only start, end and name are filled in. https://ticket.coreboot.org/issues/570 Prior effort: https://review.coreboot.org/c/flashrom/+/72433 Change-Id: I715969036c8e516aac8d90b46830f1f92ae6a160 Signed-off-by: Anastasia Klimchuk <aklm(a)flashrom.org> --- M libflashrom.c 1 file changed, 10 insertions(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/92/85292/1 diff --git a/libflashrom.c b/libflashrom.c index dfc86e6..78e5c9b 100644 --- a/libflashrom.c +++ b/libflashrom.c @@ -306,6 +306,15 @@ } } +static int compare_region_with_dump(const struct romentry *const a, const struct romentry *const b) +{ + if (a->region.start != b->region.end + || a->region.end != b->region.end + || strcmp(a->region.name, b->region.name)) + return 1; + return 0; +} + int flashrom_layout_read_from_ifd(struct flashrom_layout **const layout, struct flashctx *const flashctx, const void *const dump, const size_t len) { @@ -343,7 +352,7 @@ const struct romentry *chip_entry = layout_next(chip_layout, NULL); const struct romentry *dump_entry = layout_next(dump_layout, NULL); - while (chip_entry && dump_entry && !memcmp(chip_entry, dump_entry, sizeof(*chip_entry))) { + while (chip_entry && dump_entry && !compare_region_with_dump(chip_entry, dump_entry)) { chip_entry = layout_next(chip_layout, chip_entry); dump_entry = layout_next(dump_layout, dump_entry); } -- To view, visit https://review.coreboot.org/c/flashrom/+/85292?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: I715969036c8e516aac8d90b46830f1f92ae6a160 Gerrit-Change-Number: 85292 Gerrit-PatchSet: 1 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org>

1 0

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit