Attention is currently required from: Ashish Kumar Mishra, Dinesh Gehlot, Krishna P Bhat D, Ronak Kanabar, Saurabh Mishra, Subrata Banik.
Appukuttan V K has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/80339?usp=email )
Change subject: [TEST][W/A for MTL Build]: Changes for MTL x64 Build ......................................................................
Patch Set 18:
(16 comments)
File src/commonlib/fsp_relocate.c:
https://review.coreboot.org/c/coreboot/+/80339/comment/50b64638_de4859cd : PS12, Line 16: #include <vendorcode/intel/fsp/fsp1_1/IntelFspPkg/Include/FspInfoHeader.h>
need to use a config like FSP 1_1 to choose the correct header file between FSP 1_1 and going forwar […]
Done
File src/drivers/mrc_cache/Kconfig:
https://review.coreboot.org/c/coreboot/+/80339/comment/295843c2_8ea3f8de : PS1, Line 57: default n if UDK_VERSION >= 202302
This line is redundant when set to 'n', as next line does the same.
Done
File src/mainboard/google/rex/Kconfig:
https://review.coreboot.org/c/coreboot/+/80339/comment/ce658583_531f987c : PS12, Line 34: select FSP_FULL_FD : select ADD_FSP_BINARIES
don't need
It not including the correct FSP binaries if this is not used.
https://review.coreboot.org/c/coreboot/+/80339/comment/9221c3b0_aa7327cd : PS12, Line 247: config FSP_FD_PATH : string : depends on FSP_USE_REPO : default "3rdparty/fsp/MeteorLakeFspBinPkg/Fsp.fd"
unable to follow the motivation of migrating this config from SoC to mainboard ?
Moved to SOC config.
File src/soc/intel/meteorlake/Kconfig:
https://review.coreboot.org/c/coreboot/+/80339/comment/7783e775_41521e57 : PS12, Line 23: #select FSP_USES_CB_DEBUG_EVENT_HANDLER
u can un-comment this config after making below changes […]
Done
https://review.coreboot.org/c/coreboot/+/80339/comment/a5226610_b0e7135a : PS12, Line 41: # select MRC_CACHE_USING_MRC_VERSION
do you know the reason for commenting this Kconfig. […]
FspProducerDataHeader.h header is missing in the used fsp collaterals. Commented to bypass build error.
https://review.coreboot.org/c/coreboot/+/80339/comment/14819157_81c37cac : PS12, Line 109: select ARCH_ALL_STAGES_X86_64
u don't need this, auto selected by USE_EXP_X86_64_SUPPORT config
Acknowledged
https://review.coreboot.org/c/coreboot/+/80339/comment/8a62c894_1b1ef60c : PS12, Line 112: select HAVE_INTEL_FSP_REPO
don't need
Done
https://review.coreboot.org/c/coreboot/+/80339/comment/4404f008_0cea21f1 : PS12, Line 113: select FSP_USES_CB_STACK
any idea why is this even needed ? […]
Acknowledged
File src/soc/intel/meteorlake/fsp_params.c:
https://review.coreboot.org/c/coreboot/+/80339/comment/4f2a411a_0a870e40 : PS1, Line 398: s_cfg->CpuMpPpi = (uintptr_t)NULL;//mp_fill_ppi_services_data();
Please remove the extra comment at end of line.
Done
https://review.coreboot.org/c/coreboot/+/80339/comment/92e5a6c8_cabfb6b6 : PS1, Line 711: //s_arch_cfg->EnableMultiPhaseSiliconInit = 1;
If we're not using MP stage 1 better remove this line or put in a conditional if/else.
Done
File src/soc/intel/meteorlake/fsp_params.c:
https://review.coreboot.org/c/coreboot/+/80339/comment/da2db028_8dd41cc9 : PS12, Line 716: s_arch_cfg->FspEventHandler = (uintptr_t)(FSP_EVENT_HANDLER) : fsp_debug_event_handler;
Done
https://review.coreboot.org/c/coreboot/+/80339/comment/5d3dfd1e_13ca66f4 : PS12, Line 876: #if 0
don't need https://b.corp.google. […]
Done
File src/soc/intel/meteorlake/meminit.c:
https://review.coreboot.org/c/coreboot/+/80339/comment/fc8e7d35_ec2defe6 : PS12, Line 80: uint64_t
uintptr_t
Getting build error if changed to uintptr_t
https://review.coreboot.org/c/coreboot/+/80339/comment/4e263861_35fbf941 : PS12, Line 109: uint64_t
uintptr_t
Getting build error if changed to uintptr_t
File src/soc/intel/meteorlake/romstage/fsp_params.c:
https://review.coreboot.org/c/coreboot/+/80339/comment/f763137c_2dc0881f : PS12, Line 506: arch_upd->FspEventHandler = (uintptr_t)((FSP_EVENT_HANDLER *) : fsp_debug_event_handler);
Done
-
Appukuttan V K (Code Review)