Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41728 )
Change subject: drivers/intel/fsp2_0: Add FSP 2.2 specific support ......................................................................
Patch Set 11:
(4 comments)
https://review.coreboot.org/c/coreboot/+/41728/11/src/drivers/intel/fsp2_0/K... File src/drivers/intel/fsp2_0/Kconfig:
https://review.coreboot.org/c/coreboot/+/41728/11/src/drivers/intel/fsp2_0/K... PS11, Line 27: impacts
impact
Ack
https://review.coreboot.org/c/coreboot/+/41728/11/src/drivers/intel/fsp2_0/i... File src/drivers/intel/fsp2_0/include/fsp/info_header.h:
https://review.coreboot.org/c/coreboot/+/41728/11/src/drivers/intel/fsp2_0/i... PS11, Line 33: size_t multi_phase_si_init_entry_offset;
Doesn't this make the struct layout incompatible with FSP 2. […]
not actually as its not a packed structure that mean we don't use this structure directly anywhere rather we use dedicated field hence it won't break anything, tested on FSP2.0 and FSP2.1 platform already
https://review.coreboot.org/c/coreboot/+/41728/11/src/drivers/intel/fsp2_0/i... File src/drivers/intel/fsp2_0/include/fsp/util.h:
https://review.coreboot.org/c/coreboot/+/41728/11/src/drivers/intel/fsp2_0/i... PS11, Line 36: uint32_t phase_index
Are these phase indices enumerated? If so, I'd use an enum for this
we really don't know how many phase FSP will populate hence we can't use enum here. what is possible been mentioned at https://review.coreboot.org/c/coreboot/+/41728/11/src/drivers/intel/fsp2_0/i...
Here is the flow
1. Check if FSP header has implemented multiphasesiinit API 2. If yes, then first call multiphasesiinit API with GET_NUMBER_OF_PHASES (0) to know the number of phase supported 3.Based on return value from #2, we call each phase index (from 1 till struct fsp_multi_phase_params.phase_index) multiphasesiinit API with EXECUTE_PHASE (1)
https://review.coreboot.org/c/coreboot/+/41728/11/src/drivers/intel/fsp2_0/s... File src/drivers/intel/fsp2_0/silicon_init.c:
https://review.coreboot.org/c/coreboot/+/41728/11/src/drivers/intel/fsp2_0/s... PS11, Line 57: BIOS_SPEW
BIOS_EMERG […]
Thanks 😊