Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33129 )
Change subject: soc/intel/common/pch: Enable PCH Thermal Sensor for threshold configuration ......................................................................
Patch Set 12: Code-Review-1
(6 comments)
https://review.coreboot.org/c/coreboot/+/33129/12/src/soc/intel/common/pch/t... File src/soc/intel/common/pch/thermal/thermal.c:
https://review.coreboot.org/c/coreboot/+/33129/12/src/soc/intel/common/pch/t... PS12, Line 33: int pch_thermal_trip is defined as uint8_t. Is there a good reason to promote it to an int?
https://review.coreboot.org/c/coreboot/+/33129/12/src/soc/intel/common/pch/t... PS12, Line 45: uint16_t thermal_config = get_thermal_config(); And here you're casting the uint8_t to an int to a uint16_t
https://review.coreboot.org/c/coreboot/+/33129/12/src/soc/intel/common/pch/t... PS12, Line 78: struct device *dev = pcidev_path_on_root(PCH_DEVFN_THERMAL); Prefer separating declaration from assignment, please.
https://review.coreboot.org/c/coreboot/+/33129/12/src/soc/intel/common/pch/t... PS12, Line 84: uintptr_t Prefer variable definitions at the top of the scope, please.
https://review.coreboot.org/c/coreboot/+/33129/12/src/soc/intel/common/pch/t... PS12, Line 91: void *)thermalbar + THERMAL_SENSOR_POWER_MANAGEMENT); Pointer arithmetic on void* pointers is illegal in C.
https://review.coreboot.org/c/coreboot/+/33129/12/src/soc/intel/common/pch/t... PS12, Line 95: (void *)thermalbar + THERMAL_SENSOR_POWER_MANAGEMENT Pointer arithmetic on void* pointers is illegal in C.