[coreboot-gerrit] Change in coreboot[master]: soc/intel/cannonlake: Add VR config for CFL, CNL and WHL

[Tim Wawrzynczak (Code Review)]

Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37466 )

Change subject: soc/intel/cannonlake: Add VR config for CFL, CNL and WHL
......................................................................

Patch Set 5:

(3 comments)

https://review.coreboot.org/c/coreboot/+/37466/5/src/soc/intel/cannonlake/vr... 
File src/soc/intel/cannonlake/vr_config.c:

https://review.coreboot.org/c/coreboot/+/37466/5/src/soc/intel/cannonlake/vr... 
PS5, Line 151: 8.5
Are you using the floating point for documentation purposes?  This will be just 8 when it gets converted to a uint16_t

https://review.coreboot.org/c/coreboot/+/37466/5/src/soc/intel/cannonlake/vr... 
PS5, Line 190: if (tdp > 35)
Above you're using "if (tdp >= 54)" but here you're using >, could you use either >= or > everywhere?

https://review.coreboot.org/c/coreboot/+/37466/5/src/soc/intel/cannonlake/vr... 
PS5, Line 250: uint16_t loadline[NUM_VR_DOMAINS] =
             : 				VR_CFG_ALL_DOMAINS_LOADLINE(10.3, 2.4, 2.0, 2.0);
Your const usage is spotty, these should all be const.  Then hopefully the compiler can turn the floating numbers into ints at compile time and not runtime.