[coreboot-gerrit] Change in coreboot[master]: soc/intel/common/block: Enable PCH Thermal Sensor for threshold confi...

[Tim Wawrzynczak (Code Review)]

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

Change subject: soc/intel/common/block: Enable PCH Thermal Sensor for threshold configuration
......................................................................

Patch Set 17:

(5 comments)

https://review.coreboot.org/c/coreboot/+/33129/17/src/soc/intel/common/block... 
File src/soc/intel/common/block/thermal/thermal.c:

https://review.coreboot.org/c/coreboot/+/33129/17/src/soc/intel/common/block... 
PS17, Line 29: get_thermal_config
Maybe rename this to get_thermal_trip_temp ?

https://review.coreboot.org/c/coreboot/+/33129/17/src/soc/intel/common/block... 
PS17, Line 41: thermal_config;
             : 
             : 	thermal_config = get_thermal_config();
see above, maybe rename variable to thermal_trip_temp ?

https://review.coreboot.org/c/coreboot/+/33129/17/src/soc/intel/common/block... 
PS17, Line 56: pch_thermal_get_bar
Since this function is local to this file, and its only purpose is to get the BAR of the thermal device, does it really need to be passed the device struct?  It could take no parameters, and just get 'dev = pcidev_path_on_root(PCH_DEVFN_THERMAL)' itself.

https://review.coreboot.org/c/coreboot/+/33129/17/src/soc/intel/common/block... 
PS17, Line 95: void
nit: consider uint16_t*

https://review.coreboot.org/c/coreboot/+/33129/17/src/soc/intel/common/block... 
PS17, Line 99: void
nit: consider uint16_t*