Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32791 )
Change subject: soc/intel/cannonlake: Pass more SPI options to FSP. ......................................................................
Patch Set 1:
Patch Set 1:
Patch Set 1:
(1 comment)
These settings are just tweaking configuration int he designware ip block, no? Why are we putting in code to feed this into FSP when we already have code which configures everything?
Unfortunately, FSP ends up configuring these based on the default values in the UPDs. Ideally, FSP should not configure any of this. But currently there is no option to say skip all this programming.
So, looking at this change, I don't think we need to set the options for all the SPI buses. We probably just care about the buses that BIOS is actually using e.g. TPM bus on hatch. All other buses are not used by BIOS and so even if FSP ends up configuring them differently, OS will re-configure them anyways.
Based on this, we should be able to just use gpsi structure within common_soc_config to determine if a bus is being used in coreboot (speed_mhz is a good way to check that) and then call gspi_get_soc_spi_cfg to get bus params and setup the required FSP UPDs. Thoughts?