April 2025 - flashrom-gerrit - mail.coreboot.org

[XS] Change in flashrom[main]: flashchips: Mark W25Q256JV_Q as tested for read/write/erase
by Anastasia Klimchuk (Code Review) 29 Apr '25

29 Apr '25

Anastasia Klimchuk has submitted this change. ( https://review.coreboot.org/c/flashrom/+/87459?usp=email ) Change subject: flashchips: Mark W25Q256JV_Q as tested for read/write/erase ...................................................................... flashchips: Mark W25Q256JV_Q as tested for read/write/erase As reported on the mailing list: https://mail.coreboot.org/hyperkitty/list/flashrom@flashrom.org/thread/Y27H… Change-Id: I267ee7e86c682626ead2310b920a0e5026982312 Tested-by: Attila Veghelyi <aveghelyi(a)dension.com> Signed-off-by: Anastasia Klimchuk <aklm(a)flashrom.org> Reviewed-on: https://review.coreboot.org/c/flashrom/+/87459 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Reviewed-by: attila-v --- M flashchips/winbond.c 1 file changed, 1 insertion(+), 1 deletion(-) Approvals: build bot (Jenkins): Verified Stefan Reinauer: Looks good to me, approved attila-v: Looks good to me, but someone else must approve diff --git a/flashchips/winbond.c b/flashchips/winbond.c index ae80b13..957c4b0 100644 --- a/flashchips/winbond.c +++ b/flashchips/winbond.c @@ -546,7 +546,7 @@ /* 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 | FEATURE_WRSR2, - .tested = TEST_UNTESTED, + .tested = TEST_OK_PREW, .probe = PROBE_SPI_RDID, .probe_timing = TIMING_ZERO, .block_erasers = -- To view, visit https://review.coreboot.org/c/flashrom/+/87459?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: merged Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: I267ee7e86c682626ead2310b920a0e5026982312 Gerrit-Change-Number: 87459 Gerrit-PatchSet: 2 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> 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: attila-v Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>

1 0

[XS] Change in flashrom[main]: Add more delay time for jlink power on feature to stabilize the LDO a...
by attila-v (Code Review) 29 Apr '25

29 Apr '25

Attention is currently required from: Anastasia Klimchuk. attila-v has posted comments on this change by attila-v. ( https://review.coreboot.org/c/flashrom/+/86085?usp=email ) Change subject: Add more delay time for jlink power on feature to stabilize the LDO and the decoupling capacitors ...................................................................... Patch Set 2: (1 comment) File jlink_spi.c: https://review.coreboot.org/c/flashrom/+/86085/comment/e77c15d1_246da726?us… : PS2, Line 469: internal_sleep(100000); > I have a question: is this longer delay needed unconditionally (always, for any environment), or onl […] I think it is a safety time for any other LDO-s and decoupling capacitors too. I used only one type (HT7333) and it was definitely needed. -- To view, visit https://review.coreboot.org/c/flashrom/+/86085?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: Ic2dd94e99ac6ffa17a009b8488ce027698ae2c28 Gerrit-Change-Number: 86085 Gerrit-PatchSet: 2 Gerrit-Owner: attila-v Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: attila-v Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Tue, 29 Apr 2025 16:57:33 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org>

1 0

[M] Change in flashrom[main]: libflashrom: Add probing v2 which can find all mathching chips
by Anastasia Klimchuk (Code Review) 28 Apr '25

28 Apr '25

Attention is currently required from: Dmitry Zhadinets, Peter Marheine. Anastasia Klimchuk has posted comments on this change by Anastasia Klimchuk. ( https://review.coreboot.org/c/flashrom/+/87341?usp=email ) Change subject: libflashrom: Add probing v2 which can find all mathching chips ...................................................................... Patch Set 4: (3 comments) File cli_classic.c: https://review.coreboot.org/c/flashrom/+/87341/comment/1f89c545_a671a4e3?us… : PS3, Line 1216: if (ret == 1) { > A comment here is that: probing never returns 1, and v1 probing never returned 1 either. […] I mark this unresolved, because otherwise it is not shown in the latest diffs :( but mostly this is to ask what people think about it. Even if there will be changes to probe_flash, that's not for this commit. File include/libflashrom.h: https://review.coreboot.org/c/flashrom/+/87341/comment/4646ffed_6b038800?us… : PS4, Line 312: const char **all_matched_names, > I see from the implementation that the caller is expected to allocate this buffer, but that's not cl […] I initially tried p.2 too! :) but from my memory I got stuck because chip name is const and it did not allow copying into non-const array. And I couldn't pass const array into function if I were to allocate memory for it... it was something like that. The other APIs return the allocated array rather than taking it as param. I can try again, in case I missed something. But possibly that p.2 needs to return array of names (I like more the idea to return the number of matches. as you suggested in other comment) https://review.coreboot.org/c/flashrom/+/87341/comment/cae28c49_4a4384f1?us… : PS4, Line 313: unsigned int *all_matched_count, > I'd say it's more conventional to use negative values for error and positive values for success when […] That's an interesting idea! I also like to have one less parameter. The only thing to double-check: my initial version was keeping the same convention for return codes as it was in probing v1. This idea will change it (especially that 0 no longer indicates success). So you think it's fine? -- To view, visit https://review.coreboot.org/c/flashrom/+/87341?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: Idfcf377a8071e22028ba98515f08495ed2a6e9f0 Gerrit-Change-Number: 87341 Gerrit-PatchSet: 4 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Dmitry Zhadinets <dzhadinets(a)gmail.com> Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Dmitry Zhadinets <dzhadinets(a)gmail.com> Gerrit-Attention: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Comment-Date: Mon, 28 Apr 2025 09:48:32 +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

[XS] Change in flashrom[main]: flashchips: Mark W25Q256JV_Q as tested for read/write/erase
by attila-v (Code Review) 28 Apr '25

28 Apr '25

Attention is currently required from: Anastasia Klimchuk, Nikolai Artemiev. attila-v has posted comments on this change by Anastasia Klimchuk. ( https://review.coreboot.org/c/flashrom/+/87459?usp=email ) Change subject: flashchips: Mark W25Q256JV_Q as tested for read/write/erase ...................................................................... Patch Set 1: Code-Review+1 -- To view, visit https://review.coreboot.org/c/flashrom/+/87459?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: I267ee7e86c682626ead2310b920a0e5026982312 Gerrit-Change-Number: 87459 Gerrit-PatchSet: 1 Gerrit-Owner: 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: attila-v Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Comment-Date: Mon, 28 Apr 2025 07:50:36 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes

1 0

[M] Change in flashrom[main]: libflashrom: Add probing v2 which can find all mathching chips
by Peter Marheine (Code Review) 27 Apr '25

27 Apr '25

Attention is currently required from: Anastasia Klimchuk, Dmitry Zhadinets. Peter Marheine has posted comments on this change by Anastasia Klimchuk. ( https://review.coreboot.org/c/flashrom/+/87341?usp=email ) Change subject: libflashrom: Add probing v2 which can find all mathching chips ...................................................................... Patch Set 4: (2 comments) File include/libflashrom.h: https://review.coreboot.org/c/flashrom/+/87341/comment/7c246aab_9dd4cb2c?us… : PS4, Line 312: const char **all_matched_names, I see from the implementation that the caller is expected to allocate this buffer, but that's not clear from the documentation here and there's currently no guarantee that the buffer is large enough for all the detected chips. Either: 1. add a `size_t` parameter so the caller can indicate how large the buffer is and make the implementation avoid writing more than that many names (while still counting matches if more are found) 2. dynamically allocate the buffer so the caller doesn't need to allocate it and document that it needs to be freed with `flashrom_data_free`. 3. Instead take a callback which the name of matched chips get passed to, allowing the caller to buffer names (or not) as desired (kind of an iterator-based solution). I'm more inclined toward the second option, since a few allocations shouldn't matter and then a caller never needs to probe multiple times if they discover their buffer was too small. https://review.coreboot.org/c/flashrom/+/87341/comment/959a938c_66a60e55?us… : PS4, Line 313: unsigned int *all_matched_count, I'd say it's more conventional to use negative values for error and positive values for success when the logical return value is an int, so you could eliminate this parameter and redefine the return value to be the number of matched chips (if zero of more) or a negative error code. -- To view, visit https://review.coreboot.org/c/flashrom/+/87341?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: Idfcf377a8071e22028ba98515f08495ed2a6e9f0 Gerrit-Change-Number: 87341 Gerrit-PatchSet: 4 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Dmitry Zhadinets <dzhadinets(a)gmail.com> Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Dmitry Zhadinets <dzhadinets(a)gmail.com> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Mon, 28 Apr 2025 00:10:55 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No

1 0

[S] Change in flashrom[main]: cli: Use flashrom_supported_programmers API in cli
by Peter Marheine (Code Review) 27 Apr '25

27 Apr '25

Attention is currently required from: Dmitry Zhadinets. Peter Marheine has posted comments on this change by Dmitry Zhadinets. ( https://review.coreboot.org/c/flashrom/+/87195?usp=email ) Change subject: cli: Use flashrom_supported_programmers API in cli ...................................................................... Patch Set 2: (1 comment) File flashrom.c: https://review.coreboot.org/c/flashrom/+/87195/comment/257270a7_79893248?us… : PS2, Line 1562: return; This error path should probably print a message, since silently doing nothing could be confusing. -- To view, visit https://review.coreboot.org/c/flashrom/+/87195?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: I86ac83ee0ef4e850e69aa4e7b607e28ebab9d3d5 Gerrit-Change-Number: 87195 Gerrit-PatchSet: 2 Gerrit-Owner: Dmitry Zhadinets <dzhadinets(a)gmail.com> Gerrit-Reviewer: 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: Dmitry Zhadinets <dzhadinets(a)gmail.com> Gerrit-Comment-Date: Sun, 27 Apr 2025 23:46:34 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No

1 0

[XS] Change in flashrom[main]: flashchips: Mark W25Q256JV_Q as tested for read/write/erase
by Stefan Reinauer (Code Review) 26 Apr '25

26 Apr '25

Attention is currently required from: Anastasia Klimchuk, Nikolai Artemiev, attila-v. Stefan Reinauer has posted comments on this change by Anastasia Klimchuk. ( https://review.coreboot.org/c/flashrom/+/87459?usp=email ) Change subject: flashchips: Mark W25Q256JV_Q as tested for read/write/erase ...................................................................... Patch Set 1: Code-Review+2 -- To view, visit https://review.coreboot.org/c/flashrom/+/87459?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: I267ee7e86c682626ead2310b920a0e5026982312 Gerrit-Change-Number: 87459 Gerrit-PatchSet: 1 Gerrit-Owner: 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: attila-v Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Attention: attila-v Gerrit-Comment-Date: Sat, 26 Apr 2025 18:25:11 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes

1 0

[XS] Change in flashrom[main]: flashchips: Mark W25Q256JV_Q as tested for read/write/erase
by Anastasia Klimchuk (Code Review) 26 Apr '25

26 Apr '25

Anastasia Klimchuk has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/87459?usp=email ) Change subject: flashchips: Mark W25Q256JV_Q as tested for read/write/erase ...................................................................... flashchips: Mark W25Q256JV_Q as tested for read/write/erase As reported on the mailing list: https://mail.coreboot.org/hyperkitty/list/flashrom@flashrom.org/thread/Y27H… Change-Id: I267ee7e86c682626ead2310b920a0e5026982312 Tested-by: Attila Veghelyi <aveghelyi(a)dension.com> Signed-off-by: Anastasia Klimchuk <aklm(a)flashrom.org> --- M flashchips/winbond.c 1 file changed, 1 insertion(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/59/87459/1 diff --git a/flashchips/winbond.c b/flashchips/winbond.c index ae80b13..957c4b0 100644 --- a/flashchips/winbond.c +++ b/flashchips/winbond.c @@ -546,7 +546,7 @@ /* 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 | FEATURE_WRSR2, - .tested = TEST_UNTESTED, + .tested = TEST_OK_PREW, .probe = PROBE_SPI_RDID, .probe_timing = TIMING_ZERO, .block_erasers = -- To view, visit https://review.coreboot.org/c/flashrom/+/87459?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: I267ee7e86c682626ead2310b920a0e5026982312 Gerrit-Change-Number: 87459 Gerrit-PatchSet: 1 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org>

1 0

[M] Change in flashrom[main]: libflashrom: Add probing v2 which can find all mathching chips
by Anastasia Klimchuk (Code Review) 25 Apr '25

25 Apr '25

Attention is currently required from: Dmitry Zhadinets, Peter Marheine. Anastasia Klimchuk has posted comments on this change by Anastasia Klimchuk. ( https://review.coreboot.org/c/flashrom/+/87341?usp=email ) Change subject: libflashrom: Add probing v2 which can find all mathching chips ...................................................................... Patch Set 4: (1 comment) File tests/lifecycle.c: https://review.coreboot.org/c/flashrom/+/87341/comment/350ea199_f1a0b446?us… : PS1, Line 27: assert_int_equal > For v2 tests, I want, in addition to return value, also assert which chip is in flashctx, how many m […] Done -- To view, visit https://review.coreboot.org/c/flashrom/+/87341?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: Idfcf377a8071e22028ba98515f08495ed2a6e9f0 Gerrit-Change-Number: 87341 Gerrit-PatchSet: 4 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Dmitry Zhadinets <dzhadinets(a)gmail.com> Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Dmitry Zhadinets <dzhadinets(a)gmail.com> Gerrit-Attention: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Comment-Date: Fri, 25 Apr 2025 10:25:08 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org>

1 0

[M] Change in flashrom[main]: libflashrom: Add probing v2 which can find all mathching chips
by Anastasia Klimchuk (Code Review) 25 Apr '25

25 Apr '25

Attention is currently required from: Dmitry Zhadinets, Peter Marheine. Hello Dmitry Zhadinets, Peter Marheine, build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/87341?usp=email to look at the new patch set (#4). The following approvals got outdated and were removed: Verified+1 by build bot (Jenkins) Change subject: libflashrom: Add probing v2 which can find all mathching chips ...................................................................... libflashrom: Add probing v2 which can find all mathching chips Probing v2 can (if requested) go through all flashchips and find all the matching chip definitions. This is the way cli behaves, so cli becomes a client of probing v2. Previously cli and libflashrom had different probing logic, and different code in different source files. This patch also adds tests for probing v2. Testing from the cli: ./flashrom -p dummy:emulate=W25Q128FV -r dump.rom ./flashrom -p dummy:emulate=MX25L6436 -r dump.rom ./flashrom -p dummy:emulate=MX25L6436 -c "MX25L6405" -r dump.rom ./flashrom -p dummy:emulate=SST25VF032B -E ./flashrom -p dummy:emulate=S25FL128L -r dump.rom ./flashrom -p dummy:emulate=INVALID -r dump.rom ./flashrom -p dummy:emulate=MX25L6436 -c "NONEXISTENT" -r dump.rom Change-Id: Idfcf377a8071e22028ba98515f08495ed2a6e9f0 Signed-off-by: Anastasia Klimchuk <aklm(a)flashrom.org> --- M cli_classic.c M include/libflashrom.h M libflashrom.c M libflashrom.map M tests/dummyflasher.c M tests/lifecycle.c M tests/lifecycle.h M tests/tests.c M tests/tests.h 9 files changed, 204 insertions(+), 24 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/41/87341/4 -- To view, visit https://review.coreboot.org/c/flashrom/+/87341?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: Idfcf377a8071e22028ba98515f08495ed2a6e9f0 Gerrit-Change-Number: 87341 Gerrit-PatchSet: 4 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Dmitry Zhadinets <dzhadinets(a)gmail.com> Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Dmitry Zhadinets <dzhadinets(a)gmail.com> Gerrit-Attention: Peter Marheine <pmarheine(a)chromium.org>

1 0

2025

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit April 2025