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?usp... : PS1, Line 230: fo
for (typo)
Done
https://review.coreboot.org/c/flashrom/+/86946/comment/4d0e5185_a6d43927?usp... : 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?usp... : 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?usp... : 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
-
Dmitry Zhadinets (Code Review)