Matt Papageorge has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43303 )
Change subject: soc/amd/common: Use SPI settings from EFS ......................................................................
Patch Set 4:
(4 comments)
https://review.coreboot.org/c/coreboot/+/43303/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/43303/1//COMMIT_MSG@9 PS1, Line 9: Picasso Kconfig
Yes, but you are editing common code.. not just picasso code.
Ack
https://review.coreboot.org/c/coreboot/+/43303/1/src/soc/amd/common/block/in... File src/soc/amd/common/block/include/amdblocks/chip.h:
https://review.coreboot.org/c/coreboot/+/43303/1/src/soc/amd/common/block/in... PS1, Line 7: #include <amdblocks/spi.h>
Remove this include?
Done
https://review.coreboot.org/c/coreboot/+/43303/1/src/soc/amd/common/block/sp... File src/soc/amd/common/block/spi/fch_spi.c:
https://review.coreboot.org/c/coreboot/+/43303/1/src/soc/amd/common/block/sp... PS1, Line 52: uint8_t spi_speed = 0xff;
I was actually thinking that coreboot can read out these values from EFS. […]
Good idea. new patch implements this method
https://review.coreboot.org/c/coreboot/+/43303/1/src/soc/amd/common/block/sp... PS1, Line 57: if (CONFIG(SPI_SPEED_33M))
Should all these be else if? compiler will do dead code optimization regardless and just leave the a […]
No longer applicable with the new patch but will keep it in mind for the future