Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35595 )
Change subject: soc/intel/cannonlake: Fix FSP UPDs settings with disabled GBE ......................................................................
Patch Set 2:
(2 comments)
I have reviewed this CL again and posted couple of comments. Can you please take a look?
https://review.coreboot.org/c/coreboot/+/35595/2/src/soc/intel/cannonlake/fs... File src/soc/intel/cannonlake/fsp_params.c:
https://review.coreboot.org/c/coreboot/+/35595/2/src/soc/intel/cannonlake/fs... PS2, Line 196: SlpS0WithGbeSupport
Isn't the default for this 0?
This line should not be required, right?
https://review.coreboot.org/c/coreboot/+/35595/2/src/soc/intel/cannonlake/fs... PS2, Line 207: && params->PchLanEnable
For more details, we can go back to issue tracker to discussed […]
So, as per discussion it looks like: 1. VmControl needs to be set as per board design to allow voltage margining in S0ix to lower power consumption. 2. But, if GbE is enabled, voltage magining cannot be enabled and so the Vm control UPDs need to be set to 0.
Can you please add the comment here indicating this?