Attention is currently required from: Felix Singer, Furquan Shaikh, Ravishankar Sarawadi, Tim Wawrzynczak, Srinidhi N Kaushik, Raj Astekar. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/49055 )
Change subject: soc/intel/tigerlake: Add Kconfig options for each platform ......................................................................
Patch Set 4:
(1 comment)
Patchset:
PS4:
On the other hand, this way it is more modular. […]
I too agree with Tim. Although, my interpretation may differ. :)
We should have something selectable to decide the FSP version. Or better to set the default in the choice (in the follow-up change). Instead of going the indirect route via SKUs, it should just reflect what it effectively does, e.g.
config SOC_INTEL_TIGERLAKE_CLIENT_FSP_DEFAULT bool
config SOC_INTEL_TIGERLAKE_IOT_FSP_DEFAULT bool
Or even just one and an implicit default for the other.
These would be additional configs, on top of the SOC_INTEL_TIGERLAKE selection. Thus, should be part of the follow-up change, and IMHO we can abandon this one.
(Overthinking it: To fully model reality, we would need both, the SKU selection and the FSP version. The SKU selection would decide which options are visible in the choice and the FSP option which one would be the default if multiple options are visible. Adding an SKU selection may be error-prone, though. Something more to main- tain that doesn't have a direct effect, and thus could result in subtle, hard to debug issues. It would be a different story if we did it globally in coreboot. I.e. always select exactly the chip SKUs that are expected to be soldered on a board. This would also have other advantages like better default choices of added microcode updates. But without training us first (hence better do it globally) it could be too error-prone.)