Furquan Shaikh 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 7:
(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.
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?
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.org/c/coreboot/+/34328
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"?
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);