Attention is currently required from: Marshall Dawson. Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/52772 )
Change subject: mb/google/guybrush: Add Thermal Zone support ......................................................................
Patch Set 1:
(4 comments)
Commit Message:
https://review.coreboot.org/c/coreboot/+/52772/comment/4c5421bd_ddcd0368 PS1, Line 10: like line
https://review.coreboot.org/c/coreboot/+/52772/comment/fb72927b_a95b7378 PS1, Line 9: The file thermal.asl is a : direct copy with the exception of removing an extra blank like and : declaring PPKG as External. Please fix Zork’s file first, and then copy it.
File src/mainboard/google/guybrush/variants/baseboard/include/baseboard/thermal.h:
https://review.coreboot.org/c/coreboot/+/52772/comment/c12e942a_96c4dcb6 PS1, Line 6: /* : * Picasso Thermal Requirements : * TDP (W) 15 : * T die,max (°C) 105 : * T ctl,max 105 : * T die,lmt (default) 100 : * T ctl,lmt (default) 100 : */ If this is SoC specific, why not put it to the SoC directory?
https://review.coreboot.org/c/coreboot/+/52772/comment/90ebd0ab_47f34365 PS1, Line 15: /* Control TDP Settings */ : #define CTL_TDP_SENSOR_ID 2 /* EC TIN2 */ : : /* Temperature which OS will shutdown at */ : #define CRITICAL_TEMPERATURE 104 : : /* Temperature which OS will throttle CPU */ : #define PASSIVE_TEMPERATURE 95 What is the source of these values?
-
Paul Menzel (Code Review)