Angel Pons 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 7:
(2 comments)
https://review.coreboot.org/c/coreboot/+/37466/7//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/37466/7//COMMIT_MSG@11 PS7, Line 11: replace replace*able*
https://review.coreboot.org/c/coreboot/+/37466/7/src/soc/intel/cannonlake/vr... File src/soc/intel/cannonlake/vr_config.c:
https://review.coreboot.org/c/coreboot/+/37466/7/src/soc/intel/cannonlake/vr... PS7, Line 311: /* If board provided non-zero value, use it. */ Could use the ternary operator:
vr_params->VrVoltageLimit[domain] = cfg->voltage_limit ? cfg->voltage_limit : get_sku_voltagelimit(domain);
If too long, it could be split like this:
vr_params->VrVoltageLimit[domain] = cfg->voltage_limit ? cfg->voltage_limit : get_sku_voltagelimit(domain);
Or like this:
vr_params->VrVoltageLimit[domain] = cfg->voltage_limit ? cfg->voltage_limit : get_sku_voltagelimit(domain);
In any case, it is more succint than an if/else. This "use X if non-zero, else use Y" pattern seems to be rather common in coreboot, maybe creating some macro for it would be useful.