Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36839 )
Change subject: soc/intel/icelake: Make CpuMpPpi implementation default for ICL ......................................................................
Patch Set 1:
Patch Set 1:
My concern is not at all with this code. Kconfig still presents the option to not select USE_INTEL_FSP_TO_CALL_COREBOOT_PUBLISH_MP_PPI and even use FSP MP init, which the code does not respect. So you please make sure those confusing bogus options don't present to the user.
Sorry for bring late in response, Got your point. But looks like its part of common code, where we have all sorts of option listed and we can't have SOC specific guard here. But have you see below Kconfig has "PLATFORM_USES_FSP2_1" to default enable USE_INTEL_FSP_TO_CALL_COREBOOT_PUBLISH_MP_PPI and not select USE_INTEL_FSP_MP_INIT. ICL onwards all platform are PLATFORM_USES_FSP2_1 so isn't that says USE_INTEL_FSP_TO_CALL_COREBOOT_PUBLISH_MP_PPI is default option ? i guess thats what you want to make sure what ever this CL is doing and Kconfig shoudl honor the same ? isn't both the same in that way ?
config USE_INTEL_FSP_MP_INIT bool "Perform MP Initialization by FSP" default n depends on !USE_INTEL_FSP_TO_CALL_COREBOOT_PUBLISH_MP_PPI help This option allows FSP to perform multiprocessor initialization.
config USE_INTEL_FSP_TO_CALL_COREBOOT_PUBLISH_MP_PPI bool "Perform MP Initialization by FSP using coreboot MP PPI service" depends on FSP_USES_MP_SERVICES_PPI default y if PLATFORM_USES_FSP2_1 default n help This option allows FSP to make use of MP services PPI published by coreboot to perform multiprocessor initialization.
It's not about default options, it's about not presenting invalid options regardless of the defaults.