Maxim Polyakov has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35167 )
Change subject: soc/skylake/vr_config: Add VR config for Skylake S ......................................................................
Patch Set 3:
(1 comment)
https://review.coreboot.org/c/coreboot/+/35167/2/src/soc/intel/skylake/vr_co... File src/soc/intel/skylake/vr_config.c:
https://review.coreboot.org/c/coreboot/+/35167/2/src/soc/intel/skylake/vr_co... PS2, Line 176: else if (tdp >= 45) : icc_max[VR_IA_CORE] = VR_CFG_AMP(70);
GT_UNSLICED and GT_SLICED don't get the IccMax of 35A configured, which they should according to the […]
if I'm not mistaken, then this isn't in the documentation. The Iccmax_GT for 45 watts is missed. I think it would be better to set the smallest limit 35A, the same as for 35 watts. And also, add a comment about this. Do you agree?