Attention is currently required from: Alok Agarwal, Anil Kumar K, Intel coreboot Reviewers, Jayvik Desai, Jérémy Compostella, Kapil Porwal, Paul Menzel, Pranava Y N, Vikrant L Jadeja.
Subrata Banik has posted comments on this change by Jérémy Compostella. ( https://review.coreboot.org/c/coreboot/+/85454?usp=email )
Change subject: soc/intel/pantherlake: Display Sign-of-Life during memory training ......................................................................
Patch Set 21:
(1 comment)
File src/soc/intel/pantherlake/romstage/fsp_params.c:
https://review.coreboot.org/c/coreboot/+/85454/comment/e0e9b370_8a949317?usp... : PS21, Line 387: m_cfg->VgaMessage = (efi_uintn_t)ux_locales_get_text(UX_LOCALE_MSG_MEMORY_TRAINING);
Why not try Jérémy's solution for now and use it for as many generations as it works in? "common" doesn't need to mean "it has to work in every SoC from now till the end of forever". As long as it's common between at least two SoCs and it's a large enough piece of code worth deduplicating, you can factor it out into common code.
Just call the file `common/fsp_esol.c` for now, and if they eventually change the UPDs you can rename it to `common/fsp_esol_2025.c` and put a `common/fsp_esol_2028.c` or whatever with the new implementation next to it (or whatever other version distinguisher seems suitable for that). Then each SoC just needs to select the version that's applicable to them, but we still duplicate less code than if we had kept a separate copy for each SoC. (Compare, for example, how there's a `dpm_v1.c` and `dpm_v2.c` in `src/soc/mediatek/common`. `mt8188`, `mt8192` and `mt8195` refer to `dpm_v1.c` in their Makefiles, and `mt8196` and later will use `dpm_v2.c`. On the Arm boards we just tend to pull in common `.c` files from the SoC-specific Makefile so this pick-and-choose approach becomes particularly easy, but you can achieve the same with SoC-selected Kconfigs if you prefer too.)
Since the inception of the IA common code, our approach has been to make things common based on their applicability. For example, SoC IP is used across multiple generations of SoCs, and features can be common across different SoC/platform generations.
If we were to follow your suggestion, there would be many opportunities to make things common, but this could lead to a lot of unmanageable fragmentation. For example, we could have two different versions of the `fsp_esol` folder, one per year. This would be churn and wouldn't make sense because the logic of common code is based on either generic HW or generic FW features. We're not trying to make things common just because they use the same variable names.
In the past, Intel CPU folks have tried to make common code in the same way you suggested, and it ended up creating multiple versions of the common folder. This happened because it's difficult to manage common code based on software commonality rather than a valid HW reason.
In summary, I don't think we need to make common code just because it uses eSOL UPD name same between MTL and PTL. We might even introduce new UPDs, even for PTL, to might add new UPD to handle battery level and avoid string comparison for rendering differnt eSOL msg. Therefore, the suggestion for `fsp_esol_2025` still doesn't apply to anything beyond MTL at this point.