Attention is currently required from: Maulik V Vaghela, Tim Wawrzynczak, Subrata Banik, Patrick Rudolph. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/59509 )
Change subject: soc/intel/common/thermal: Refactor thermal block to improve reusability ......................................................................
Patch Set 4: Code-Review+1
(2 comments)
File src/soc/intel/common/block/include/intelblocks/thermal.h:
https://review.coreboot.org/c/coreboot/+/59509/comment/85d5d635_83ac9863 PS4, Line 20: #define GET_LTT_VALUE(x) ((x + 10) << 20 | (x + 5) << 10 | x) Please wrap `x` in parentheses to avoid operation order problems:
#define GET_LTT_VALUE(x) (((x) + 10) << 20 | ((x) + 5) << 10 | (x))
File src/soc/intel/common/block/thermal/Kconfig:
https://review.coreboot.org/c/coreboot/+/59509/comment/76b4da14_bfd2a61c PS4, Line 7: config SOC_INTEL_COMMON_BLOCK_THERMAL_PCI_DEV : bool : default n : select SOC_INTEL_COMMON_BLOCK_THERMAL : help : This option allows to configure PCH thermal registers using Thermal PCI device : for chipsets till Ice Lake PCH. : : config SOC_INTEL_COMMON_BLOCK_THERMAL_BEHIND_PMC : bool : default n : select SOC_INTEL_COMMON_BLOCK_THERMAL : help : This option allows to configure PCH thermal registers using PMC PWRMBASE : for chipsets since Tiger Lake PCH. Would be good to make these two Kconfig options mutually exclusive. I haven't tested this, but here's an idea:
config SOC_INTEL_COMMON_BLOCK_THERMAL_PCI_DEV bool default n depends on !SOC_INTEL_COMMON_BLOCK_THERMAL_BEHIND_PMC select SOC_INTEL_COMMON_BLOCK_THERMAL help This option allows to configure PCH thermal registers using Thermal PCI device for chipsets till Ice Lake PCH.
config SOC_INTEL_COMMON_BLOCK_THERMAL_BEHIND_PMC bool default n depends on !SOC_INTEL_COMMON_BLOCK_THERMAL_PCI_DEV select SOC_INTEL_COMMON_BLOCK_THERMAL help This option allows to configure PCH thermal registers using PMC PWRMBASE for chipsets since Tiger Lake PCH.