Attention is currently required from: Jason Glenesk, Raul Rangel, Martin Roth, Furquan Shaikh, Marshall Dawson, Karthik Ramasubramanian. Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/56959 )
Change subject: soc/amd/common: Update SPI based on Kconfig & EFS instead of devtree ......................................................................
Patch Set 1:
(4 comments)
File src/soc/amd/common/block/psp/psp_efs.c:
https://review.coreboot.org/c/coreboot/+/56959/comment/5ca7c1a6_dd64bfd5 PS1, Line 11: read32((uint32_t *)EFS_ADDRESS) use efs->signature here? at least for me that would clarify what we're doing here
File src/soc/amd/common/block/spi/fch_spi.c:
https://review.coreboot.org/c/coreboot/+/56959/comment/f0be98c7_3f368207 PS1, Line 42: !
When would this fail?
when this one would fail, we probably already failed much earlier, but i don't see a strong reason not to check it again here and apply the Kconfig defaults in that failure case
https://review.coreboot.org/c/coreboot/+/56959/comment/eccd584a_94012383 PS1, Line 48: fch_spi_set_spi100(CONFIG_NORMAL_READ_SPI_SPEED, fast_speed, CONFIG_ALT_SPI_SPEED, should the normal read speed and the altio read speed the minimum speed of EFS fast read setting and Kconfig settings for those two other frequencies? the plan was to be able to change part of the spi configuration in efs by some external tool, to be able to use an automatically built image with an EM100 that often can't run at the speed a real spi flash chip would run at
https://review.coreboot.org/c/coreboot/+/56959/comment/a6914123_9cf63977 PS1, Line 52: static void fch_spi_config_modes(void) i'd rename fch_spi_config_mb_modes to fch_spi_config_modes and drop this function that only does one function call