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

[M] Change in flashrom[main]: libflashrom: Added API for capabilitible chips
by Dmitry Zhadinets (Code Review) 04 Apr '25

04 Apr '25

Attention is currently required from: Anastasia Klimchuk. Dmitry Zhadinets has posted comments on this change by Dmitry Zhadinets. ( https://review.coreboot.org/c/flashrom/+/86946?usp=email ) Change subject: libflashrom: Added API for capabilitible chips ...................................................................... Patch Set 3: (5 comments) Patchset: PS1: > I started adding comments, but now I am thinking about the patch overall: is this the idea which was […] After implementing flashrom_programmer_capabilities I think that it is better to leave the probing logic as is. Because as I think now libflashrom and cli should work differently. The issue was that previously the only way to get a list of compatible flashchips was through the logger. and if libflashrom does not return all chips there is no other options except to call cli version to get the name and set it directly in UI Cli version is the thing itself. The user is calling probe and in the console he will see all possible variants. Now I can get the list in advance. and even if I have an error from regular libflashrom probe "Multiple chips were found" then this is it what I need. To get status "more that one" is faster then to get whole list only for logger File include/libflashrom.h: https://review.coreboot.org/c/flashrom/+/86946/comment/0065fe29_dd662b99?us… : PS1, Line 230: fo > for (typo) Done https://review.coreboot.org/c/flashrom/+/86946/comment/4d0e5185_a6d43927?us… : PS1, Line 231: * @param[in] flashprog The flash programmer used to access the chip. > We usually add a line between brief and the rest, also a line between param and return Done File libflashrom.map: https://review.coreboot.org/c/flashrom/+/86946/comment/d134e185_d54fd2cd?us… : PS1, Line 34: flashrom_supported_programmers > This needs to be in the previous commit CB:86921 Done File tests/tests.c: https://review.coreboot.org/c/flashrom/+/86946/comment/86354d59_d1420274?us… : PS1, Line 456: const struct CMUnitTest libflashrom_tests[] = { : cmocka_unit_test(enumerators_test_success), : }; : ret |= cmocka_run_group_tests_name("libflashrom.c tests", libflashrom_tests, NULL, NULL); > What is the reason you are moving these lines? […] Unfortunately no As far as I can see there is an issue with spi tests and how it works with flashctx it leads to ``` [ ERROR ] --- %s() has remaining non-returned values. : __wrap_spi_send_command../tests/spi25.c:167: note: remaining item was declared here ../tests/spi25.c:168: note: remaining item was declared here ../tests/spi25.c:169: note: remaining item was declared here __wrap_spi_send_command: '%s' parameter still has values that haven't been checked. : flash../tests/spi25.c:164: note: remaining item was declared here [ ERROR ] --- %s() has remaining non-returned values. : __wrap_spi_send_command../tests/spi25.c:182: note: remaining item was declared here ../tests/spi25.c:183: note: remaining item was declared here ../tests/spi25.c:184: note: remaining item was declared here __wrap_spi_send_command: '%s' parameter still has values that haven't been checked. : flash../tests/spi25.c:179: note: remaining item was declared here [ ERROR ] --- 0 != 0x1 [ LINE ] --- ../tests/spi25.c:200: error: Failure! [ ERROR ] --- %s() has remaining non-returned values. : __wrap_spi_send_command../tests/spi25.c:212: note: remaining item was declared here ../tests/spi25.c:213: note: remaining item was declared here ../tests/spi25.c:214: note: remaining item was declared here __wrap_spi_send_command: '%s' parameter still has values that haven't been checked. : flash../tests/spi25.c:209: note: remaining item was declared here ``` I faced with this issue because I'm calling ``` flashrom_programmer_init ... flashrom_programmer_shutdown ``` in my test before spi Either ```flashrom_programmer_shutdown``` does not clean everything correctly or some other global scope variables. I will dive dipper here . Maybe I will add one more commit before this one or Fix it within another patchset -- To view, visit https://review.coreboot.org/c/flashrom/+/86946?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: I13c951f9404ae8b042cdb572e6117a09ac231747 Gerrit-Change-Number: 86946 Gerrit-PatchSet: 3 Gerrit-Owner: Dmitry Zhadinets <dzhadinets(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: Fri, 04 Apr 2025 17:04:41 +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: Added API to enumerate supported programmers
by Dmitry Zhadinets (Code Review) 04 Apr '25

04 Apr '25

Attention is currently required from: Anastasia Klimchuk. Dmitry Zhadinets has posted comments on this change by Dmitry Zhadinets. ( https://review.coreboot.org/c/flashrom/+/86921?usp=email ) Change subject: libflashrom: Added API to enumerate supported programmers ...................................................................... Patch Set 6: (5 comments) Patchset: PS3: > I realised that by now I already added comments to all three patches in the chain, hope it's fine. […] Looks like your link does not work. Could you check? File libflashrom.c: https://review.coreboot.org/c/flashrom/+/86921/comment/216b2b88_d00576ec?us… : PS3, Line 147: > no space here Done https://review.coreboot.org/c/flashrom/+/86921/comment/cee4d53a_79a7f176?us… : PS3, Line 159: const char *flashrom_get_programmer_name(int n) : { : if (n < 0 || (size_t)n >= programmer_table_size) : return NULL; : return programmer_table[n]->name; : } > Why this is needed? Client can get the n-th name using the function just above `flashrom_supported_p […] It is because of my squashed commits. I re-based it not is it here in another commit. Lets discuss it there. https://review.coreboot.org/c/flashrom/+/86947/3 It was requested by Peter here https://review.coreboot.org/c/flashrom/+/86921/comment/305f6656_3d9d7212/ File tests/libflashrom.c: https://review.coreboot.org/c/flashrom/+/86921/comment/0d7fe06f_70fd3595?us… : PS3, Line 40: #if CONFIG_DUMMY == 1 > For tests, dummy is always included, you can remove this check Again, my swashed commits . it is here now https://review.coreboot.org/c/flashrom/+/86946/3 Removed there https://review.coreboot.org/c/flashrom/+/86921/comment/c5e7a483_9ae840e3?us… : PS3, Line 62: > This needs to be tabs (instead of spaces). Fixed here https://review.coreboot.org/c/flashrom/+/86947/3 -- To view, visit https://review.coreboot.org/c/flashrom/+/86921?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: Ib5275b742b849183b1fe701900040fee369a1d78 Gerrit-Change-Number: 86921 Gerrit-PatchSet: 6 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: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Fri, 04 Apr 2025 16:56:37 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Dmitry Zhadinets <dzhadinets(a)gmail.com> Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org>

1 0

[M] Change in flashrom[main]: libflashrom: Added API to enumerate supported programmers
by Dmitry Zhadinets (Code Review) 04 Apr '25

04 Apr '25

Attention is currently required from: Dmitry Zhadinets. Hello Anastasia Klimchuk, Peter Marheine, build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/86921?usp=email to look at the new patch set (#6). The following approvals got outdated and were removed: Verified-1 by build bot (Jenkins) Change subject: libflashrom: Added API to enumerate supported programmers ...................................................................... libflashrom: Added API to enumerate supported programmers Change-Id: Ib5275b742b849183b1fe701900040fee369a1d78 Signed-off-by: Dmitry Zhadinets <dzhadinets(a)gmail.com> --- M include/libflashrom.h M libflashrom.c M libflashrom.map M tests/libflashrom.c M tests/tests.c M tests/tests.h 6 files changed, 102 insertions(+), 10 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/21/86921/6 -- To view, visit https://review.coreboot.org/c/flashrom/+/86921?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: Ib5275b742b849183b1fe701900040fee369a1d78 Gerrit-Change-Number: 86921 Gerrit-PatchSet: 6 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>

1 0

[S] Change in flashrom[main]: libflashrom: Added API to enumerate supported programmers
by Dmitry Zhadinets (Code Review) 04 Apr '25

04 Apr '25

Attention is currently required from: Dmitry Zhadinets. Hello Anastasia Klimchuk, Peter Marheine, build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/86921?usp=email to look at the new patch set (#5). The following approvals got outdated and were removed: Verified+1 by build bot (Jenkins) Change subject: libflashrom: Added API to enumerate supported programmers ...................................................................... libflashrom: Added API to enumerate supported programmers Change-Id: Ib5275b742b849183b1fe701900040fee369a1d78 Signed-off-by: Dmitry Zhadinets <dzhadinets(a)gmail.com> --- M include/libflashrom.h M libflashrom.c M libflashrom.map M tests/libflashrom.c M tests/tests.c M tests/tests.h 6 files changed, 42 insertions(+), 2 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/21/86921/5 -- To view, visit https://review.coreboot.org/c/flashrom/+/86921?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: Ib5275b742b849183b1fe701900040fee369a1d78 Gerrit-Change-Number: 86921 Gerrit-PatchSet: 5 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>

1 0

[S] Change in flashrom[main]: libflashrom: Iterator like API to get list of programmers
by Dmitry Zhadinets (Code Review) 04 Apr '25

04 Apr '25

Attention is currently required from: Dmitry Zhadinets. Hello Anastasia Klimchuk, build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/86947?usp=email to look at the new patch set (#2). The following approvals got outdated and were removed: Verified+1 by build bot (Jenkins) Change subject: libflashrom: Iterator like API to get list of programmers ...................................................................... libflashrom: Iterator like API to get list of programmers Change-Id: I89cce7625e8731ee26331934f36cfd1c8c570225 Signed-off-by: Dmitry Zhadinets <dzhadinets(a)gmail.com> --- M include/libflashrom.h M libflashrom.c M libflashrom.map M tests/libflashrom.c M tests/tests.c M tests/tests.h 6 files changed, 28 insertions(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/47/86947/2 -- To view, visit https://review.coreboot.org/c/flashrom/+/86947?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: I89cce7625e8731ee26331934f36cfd1c8c570225 Gerrit-Change-Number: 86947 Gerrit-PatchSet: 2 Gerrit-Owner: Dmitry Zhadinets <dzhadinets(a)gmail.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Dmitry Zhadinets <dzhadinets(a)gmail.com>

1 0

[M] Change in flashrom[main]: libflashrom: Added API for capabilitible chips
by Dmitry Zhadinets (Code Review) 04 Apr '25

04 Apr '25

Attention is currently required from: Dmitry Zhadinets. Hello Anastasia Klimchuk, build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/86946?usp=email to look at the new patch set (#2). The following approvals got outdated and were removed: Verified+1 by build bot (Jenkins) Change subject: libflashrom: Added API for capabilitible chips ...................................................................... libflashrom: Added API for capabilitible chips There were no options available to obtain the list of programmers, compatible chips for the programmer, and layout region names. The implementation is based on flashrom_supported_flash_chips. Arrays of constant strings are returned, and the array must be freed using flashrom_data_free. BTW it would be good to add a notice about freeing memory to the other flashrom_supported* functions. Testing: Both unit tests and CLI tools serve as libflashrom clients. All unit tests run successfully. Change-Id: I13c951f9404ae8b042cdb572e6117a09ac231747 Signed-off-by: Dmitry Zhadinets <dzhadinets(a)gmail.com> --- M include/libflashrom.h M libflashrom.c M libflashrom.map M tests/libflashrom.c M tests/tests.c M tests/tests.h 6 files changed, 57 insertions(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/46/86946/2 -- To view, visit https://review.coreboot.org/c/flashrom/+/86946?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: I13c951f9404ae8b042cdb572e6117a09ac231747 Gerrit-Change-Number: 86946 Gerrit-PatchSet: 2 Gerrit-Owner: Dmitry Zhadinets <dzhadinets(a)gmail.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Dmitry Zhadinets <dzhadinets(a)gmail.com>

1 0

[M] Change in flashrom[main]: libflashrom: Added API to enumerate supported programmers
by Dmitry Zhadinets (Code Review) 04 Apr '25

04 Apr '25

Attention is currently required from: Dmitry Zhadinets. Hello Anastasia Klimchuk, Peter Marheine, build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/86921?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: Added API to enumerate supported programmers ...................................................................... libflashrom: Added API to enumerate supported programmers Change-Id: Ib5275b742b849183b1fe701900040fee369a1d78 Signed-off-by: Dmitry Zhadinets <dzhadinets(a)gmail.com> --- M include/libflashrom.h M libflashrom.c M libflashrom.map A tests/libflashrom.c M tests/meson.build M tests/tests.c M tests/tests.h 7 files changed, 63 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/21/86921/4 -- To view, visit https://review.coreboot.org/c/flashrom/+/86921?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: Ib5275b742b849183b1fe701900040fee369a1d78 Gerrit-Change-Number: 86921 Gerrit-PatchSet: 4 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>

1 0

[XS] Change in flashrom[main]: cli: Set maximum log level for log callback
by Dmitry Zhadinets (Code Review) 04 Apr '25

04 Apr '25

Attention is currently required from: Anastasia Klimchuk, Peter Marheine. Dmitry Zhadinets has posted comments on this change by Anastasia Klimchuk. ( https://review.coreboot.org/c/flashrom/+/87174?usp=email ) Change subject: cli: Set maximum log level for log callback ...................................................................... Patch Set 2: Code-Review+1 (1 comment) Patchset: PS1: > I realised this is missing only after merging log level callback! […] Yeah, It should be merged as soon as possible RUST binding is misaligned with new API as well. It has own set_log_level. It will work as far as I can see. But maybe someone else can take a look. -- To view, visit https://review.coreboot.org/c/flashrom/+/87174?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: I70a02ea1a1d692267fd6d92cdb5273786a913777 Gerrit-Change-Number: 87174 Gerrit-PatchSet: 2 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: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Attention: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Comment-Date: Fri, 04 Apr 2025 14:01:04 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org>

1 0

[XS] Change in flashrom[main]: cli: Set maximum log level for log callback
by Anastasia Klimchuk (Code Review) 04 Apr '25

04 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/+/87174?usp=email to look at the new patch set (#2). Change subject: cli: Set maximum log level for log callback ...................................................................... cli: Set maximum log level for log callback Follow up (or fix) on commit 6571f263b52710579f27b3c53ade52d46acbc8e3 which adds ability to set maximum log level to log callback API. And INFO as the default. Without this patch cli options -V, -VV, -VVV not working anymore. cli at the moment processes all the messages in the callback, so log level should be maximum possible to get all the messages. Alternative to this could be setting the default max log level for callback as SPEW. Tested by running with -V, -VV, -VVV flashrom -p dummy:emulate=W25Q128FV,freq=64mhz -r dump.rom --progress Change-Id: I70a02ea1a1d692267fd6d92cdb5273786a913777 Signed-off-by: Anastasia Klimchuk <aklm(a)flashrom.org> --- M cli_classic.c 1 file changed, 3 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/74/87174/2 -- To view, visit https://review.coreboot.org/c/flashrom/+/87174?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: I70a02ea1a1d692267fd6d92cdb5273786a913777 Gerrit-Change-Number: 87174 Gerrit-PatchSet: 2 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

[XS] Change in flashrom[main]: cli: Set maximum log level for log callback
by Anastasia Klimchuk (Code Review) 04 Apr '25

04 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/+/87174?usp=email ) Change subject: cli: Set maximum log level for log callback ...................................................................... Patch Set 1: (1 comment) Patchset: PS1: I realised this is missing only after merging log level callback! cli handles all the messages in the callback, and the decision is made in the callback. -- To view, visit https://review.coreboot.org/c/flashrom/+/87174?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: I70a02ea1a1d692267fd6d92cdb5273786a913777 Gerrit-Change-Number: 87174 Gerrit-PatchSet: 1 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-Attention: Dmitry Zhadinets <dzhadinets(a)gmail.com> Gerrit-Attention: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Comment-Date: Fri, 04 Apr 2025 12:19:30 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No

1 0

2025

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit April 2025