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 5:
(6 comments)
https://review.coreboot.org/c/coreboot/+/33129/5/src/soc/intel/cannonlake/th... File src/soc/intel/cannonlake/thermal.c:
https://review.coreboot.org/c/coreboot/+/33129/5/src/soc/intel/cannonlake/th... PS5, Line 31: trip_temp
This macro should not make assumptions about variable name. […]
Sure, we will use this way. Thanks for suggestion.
https://review.coreboot.org/c/coreboot/+/33129/5/src/soc/intel/cannonlake/th... PS5, Line 37: bar = pci_read_config32(dev, PCI_BASE_ADDRESS_0);
I don't see how this file affects libpayload. […]
To use find_resource(), we are planning to use below code struct resource *res; res = find_resource(dev, PCI_BASE_ADDRESS_0); /* Assign the base address of the resource */ bar = res->base;
https://review.coreboot.org/c/coreboot/+/33129/5/src/soc/intel/cannonlake/th... PS5, Line 74: if (config->pch_trip_temp)
you seem not to know what a NULL pointer dereference is.
We will check this condition and print an error message.
https://review.coreboot.org/c/coreboot/+/33129/5/src/soc/intel/cannonlake/th... PS5, Line 78: die("Input PCH temp trip is higher than allowed range!");
why is 205 deg C fatal, but 204 deg C is not?
worst case (MAX) value writing 1 to all [8:0] bits ie 0x1ff. MAX_TRIP_VALUE=(0x1ff / 2 - 50 deg C) = 205 deg C. Even for 204 deg C is not valid case. But, we have considered here the worst case of all 1s.
https://review.coreboot.org/c/coreboot/+/33129/5/src/soc/intel/cannonlake/th... PS5, Line 81: ltt_value = GET_LTT_VALUE;
the correct way of using defines for formular is […]
Ack
https://review.coreboot.org/c/coreboot/+/33129/5/src/soc/intel/cannonlake/th... PS5, Line 98: if (!thermalbar) {
You will need something like this for hatch: […]
Sure, we will implement this way. I have tested and it works.