Attention is currently required from: Xiang Wang, Stefan Reinauer, Angel Pons. Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/49255 )
Change subject: bitbang-spi.c: support clock polarity and phase ......................................................................
Patch Set 25:
(3 comments)
File bitbang_spi.c:
https://review.coreboot.org/c/flashrom/+/49255/comment/2ae60be5_32b86965 PS12, Line 208: : if (parse_spi_mode(&cpol, &cpha)) : return ERROR_FLASHROM_BUG;
Global static variables are initialized to 0 by default
No you should initialise to 0 explicitly, not rely on the specific toolchain that happens to be installed.
File bitbang_spi.c:
https://review.coreboot.org/c/flashrom/+/49255/comment/38c17da0_85c08208 PS14, Line 41: : static int cpol; /* Clock Polarity */ : static int cpha; /* Clock Phase */
Done
not done.
https://review.coreboot.org/c/flashrom/+/49255/comment/1a52d274_cbe7f28c PS14, Line 210: ERROR_FLASHROM_BUG
Done
not done, please don't resolve if not actually resolved.
You want `SPI_PROGRAMMER_ERROR` here, ERROR_FLASHROM_BUG has a very specific meaning for when things miscompile etc. It's not for here.
Flashrom has rather poor to non-existent error code sanity, lets not make it any worse here.
-
Edward O'Callaghan (Code Review)