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

[Sumeet R Pawnikar (Code Review)]

Sumeet R Pawnikar 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 14:

(6 comments)
> Patch Set 12:
>
> (1 comment)

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
> Make it to uint8_t as pch_thermal_trip […]

Ack

https://review.coreboot.org/c/coreboot/+/33129/12/src/soc/intel/common/pch/t... 
PS12, Line 45: uint16_t thermal_config = get_thermal_config();
> uint8_t thermal_config = get_thermal_config();

Done

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);
> struct device *dev; […]

Ack

https://review.coreboot.org/c/coreboot/+/33129/12/src/soc/intel/common/pch/t... 
PS12, Line 84: uintptr_t
> look at above

Done

https://review.coreboot.org/c/coreboot/+/33129/12/src/soc/intel/common/pch/t... 
PS12, Line 91: void *)thermalbar + THERMAL_SENSOR_POWER_MANAGEMENT);
> you can create a local variable for thermal_pm […]

Done

https://review.coreboot.org/c/coreboot/+/33129/12/src/soc/intel/common/pch/t... 
PS12, Line 95: (void *)thermalbar + THERMAL_SENSOR_POWER_MANAGEMENT
> write16((void *)thermalbar_pm, reg16);

Done