Attention is currently required from: Jason Glenesk, Raul Rangel, Marshall Dawson, Felix Held. Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/51711 )
Change subject: soc/amd/cezanne: select HAVE_EM100_SUPPORT ......................................................................
Patch Set 1:
(1 comment)
File src/soc/amd/cezanne/Kconfig:
https://review.coreboot.org/c/coreboot/+/51711/comment/397ad123_4147a12a PS1, Line 25: HAVE_EM100_SUPPORT
Can we remove fch_spi_config_em100_modes? The PSP already sets up the SPI registers via EFS. This way if a board can support 33MHz with the EM100 it can be changed.
i'd prefer to write the registers in coreboot and not rely on psp/abl doing the expected thing, since it's not always completely clear if it really did the thing that it was supposed to do.
I see one advantage of relying on PSP to do this configuration. Switching between different frequencies can be made possible without having to rebuild a firmware image. This is especially useful when you want to test pre-built firmware images (e.g. CPFE). Obviously, there is some more work involved to ensure that amdfwtool allows changing the SPI speeds as requested. But, that can be a useful feature to have.
With respect to coreboot configuring speeds, would it make sense to use the same EFS structure to extract the configuration settings and use that for writing the registers?
-
Furquan Shaikh (Code Review)