Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39367 )
Change subject: src/soc/tigerlake: add S0ix support fsp_params ......................................................................
Patch Set 2:
(3 comments)
https://review.coreboot.org/c/coreboot/+/39367/2/src/soc/intel/tigerlake/fsp... File src/soc/intel/tigerlake/fsp_params_tgl.c:
https://review.coreboot.org/c/coreboot/+/39367/2/src/soc/intel/tigerlake/fsp... PS2, Line 162: 0x09 Why 0x09? Shouldn't this be board specific?
https://review.coreboot.org/c/coreboot/+/39367/2/src/soc/intel/tigerlake/fsp... PS2, Line 163: params->PchFivrVccinAuxLowToHighCurModeVolTranTime = 0; : params->PchFivrVccinAuxRetToHighCurModeVolTranTime = 0; : params->PchFivrVccinAuxOffToHighCurModeVolTranTime = 0; Why are these set to 0? I believe these have to be design specific.
https://review.coreboot.org/c/coreboot/+/39367/2/src/vendorcode/intel/fsp/fs... File src/vendorcode/intel/fsp/fsp2_0/tigerlake/FspsUpd.h:
PS2: Please update Fsp headers as a separate CL. Also, Intel is using a script to generate these. Please ensure that this is done following the same procedure.