Sumeet R Pawnikar has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33129 )
Change subject: soc/intel/cannonlake: set temperature threshold for PCH Thermal Sensor ......................................................................
Patch Set 8:
(5 comments)
https://review.coreboot.org/c/coreboot/+/33129/7/src/soc/intel/cannonlake/in... File src/soc/intel/cannonlake/include/soc/iomap.h:
https://review.coreboot.org/c/coreboot/+/33129/7/src/soc/intel/cannonlake/in... PS7, Line 71:
nit: Please use tabs like the other macros.
Ack
https://review.coreboot.org/c/coreboot/+/33129/7/src/soc/intel/cannonlake/th... File src/soc/intel/cannonlake/thermal.c:
https://review.coreboot.org/c/coreboot/+/33129/7/src/soc/intel/cannonlake/th... PS7, Line 32: (
nit: use tab like the other macros?
Ack
https://review.coreboot.org/c/coreboot/+/33129/7/src/soc/intel/cannonlake/th... PS7, Line 54: = dev->chip_info; : : if (!config) { : printk(BIOS_ERR, "PCH thermal config not found!\n"); : return 0; : } :
It would be good to use config_of() which will be going in here: https://review.coreboot. […]
Sure. ACK.
https://review.coreboot.org/c/coreboot/+/33129/7/src/soc/intel/cannonlake/th... PS7, Line 62: printk(BIOS_ERR, "PCH trip temp does not contain valid value!\n");
Also add --> "Using default"?
I will add this.
https://review.coreboot.org/c/coreboot/+/33129/7/src/soc/intel/cannonlake/th... PS7, Line 84: PCH_DEV_THERMAL
pcidev_path_on_root(PCH_DEVFN_THERMAL);
I will update with this function call. Thanks.