Attention is currently required from: Martin Roth, Subrata Banik, Meera Ravindranath, Aamir Bohra, Tim Wawrzynczak, Andrey Petrov, Patrick Rudolph. Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/49474 )
Change subject: drivers/intel/fsp2_0: Add support for MP PPI services2 APIs ......................................................................
Patch Set 4:
(5 comments)
File src/drivers/intel/fsp2_0/Kconfig:
https://review.coreboot.org/c/coreboot/+/49474/comment/5a249642_2cfe0d22 PS4, Line 163: SOC_INTEL_ALDERLAKE I don't think we should be adding per-SoC conditional here. It will keep growing over time. Instead the selection of FSP_USES_MP_SERVICES* should be moved to soc/.../Kconfig
File src/drivers/intel/fsp2_0/ppi/Kconfig:
https://review.coreboot.org/c/coreboot/+/49474/comment/2f0789a7_bb3e4c7c PS4, Line 5: default n Why was `default n` dropped?
https://review.coreboot.org/c/coreboot/+/49474/comment/9476a882_8dc2b060 PS4, Line 2: Shouldn't these be choice since you cannot have both v1 and v2 selected at the same time.
https://review.coreboot.org/c/coreboot/+/49474/comment/03b2f851_a8cd60dd PS4, Line 17: EFI_PEI_MP_SERVICES_PPI Isn't this EFI_PEI_MP_SERVICES2_PPI?
File src/soc/intel/common/block/cpu/Kconfig:
https://review.coreboot.org/c/coreboot/+/49474/comment/22f40e06_8706c150 PS4, Line 78: default y if FSP_PEIM_TO_PEIM_INTERFACE && (FSP_USES_MP_SERVICES_PPI || FSP_USES_MP_SERVICES2_PPI) Shouldn't this be: depends on FSP_PEIM_TO_PEIM_INTERFACE
and select USE_INTEL_FSP_TO_CALL_COREBOOT_PUBLISH_MP_PPI be done as part of each of FSP_USES_MP_SERVICES_PPI and FSP_USES_MP_SERVICES2_PPI?
-
Furquan Shaikh (Code Review)