Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/38833 )
Change subject: sb600spi: Add spireadmode ......................................................................
Patch Set 2:
(2 comments)
https://review.coreboot.org/c/flashrom/+/38833/1/sb600spi.c File sb600spi.c:
https://review.coreboot.org/c/flashrom/+/38833/1/sb600spi.c@451 PS1, Line 451: (amd_gen < CHIPSET_BOLTON))
I'm not sure. […]
Looking at the code below again, it seems that spireadmode does not have an effect for chipsets < Bolton. Instead of returning, I would just ignore the parameter (and maybe print a warning that it's not being used)
https://review.coreboot.org/c/flashrom/+/38833/1/sb600spi.c@509 PS1, Line 509: if (spispeed_idx >= 0 && set_speed(dev, spispeed_idx) != 0) { : return 1; : } : return 0;
Agree. Except it would return 0 (success) if spispeed_idx < 0.
Well, if spispeed_idx is negative, the previous code would have barfed. Is it a problem if set_speed is not called?