Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33142 )
Change subject: Kconfig: Create coreboot separate stage kconfigs ......................................................................
Patch Set 2: Code-Review-1
(1 comment)
Sorry, I still think this patch is a bad idea and just adding to confusion and redundancy in the configs. The architecture stage Kconfigs are just meant to select the right toolchain, they don't actually mean that that stage is included and I don't think they need to. ARCH_VERSTAGE_XXX is often set even when we don't actually have a verstage (and I also don't see how you're disabling ARCH_RAMSTAGE_XXX in your case?). Trying to solve this all at this level would pull a lot of stuff into the architecture configs where it needs to be unnecessarily duplicated six times.
If you want to know whether you have a verstage, check CONFIG_SEPARATE_VERSTAGE. If you want to know whether you have a ramstage, check CONFIG_RAMPAYLOAD. Those options already correspond 1-to-1 to what you want to know, I don't see the need for adding any further ones. If at some future point we have more than one case where there's no ramstage, we can add a helper option and select that based on RAMPAYLOAD. But for now I don't see why we need to add anything, and we definitely shouldn't need to pull all of this into the toolchain selectors.
https://review.coreboot.org/#/c/33142/2/src/arch/x86/Kconfig File src/arch/x86/Kconfig:
https://review.coreboot.org/#/c/33142/2/src/arch/x86/Kconfig@34 PS2, Line 34: select ENABLE_STAGE_VERSTAGE Is this actually doing what you want? This option is usually selected unconditionally by the SoC, regardless of whether SEPARATE_VERSTAGE is actually enabled or not. So this option will be on most of the time even if there is no verstage.