Re: [PATCH] arm64: dts: renesas: r8a779g3: Update thermal trip points on V4H Sparrow Hawk

[Marek Vasut <marek.vasut@mailbox.org>]

On 6/26/25 11:41 PM, Niklas Söderlund wrote:

Hello Niklas,

>> +&a76_3 {
>> +	a76_3_thermal_idle: thermal-idle {
>> +		#cooling-cells = <2>;
>> +		duration-us = <10000>;
>> +		exit-latency-us = <500>;
>> +	};
>> +};
> 
> I did not know you could do this and use it as a cooling device, thanks
> for teaching me something new!

You could, although the cooling effect may vary. Some cores enter e.g. 
clock stop during idle and then they really cool down, some do not.

>> +/* THS sensors in SoC, critical temperature trip point is 100C */
>> +&sensor1_crit {
>> +	temperature = <100000>;
>> +};
>> +
>> +&sensor2_crit {
>> +	temperature = <100000>;
>> +};
>> +
>> +&sensor3_crit {
>> +	temperature = <100000>;
>> +};
>> +
>> +&sensor4_crit {
>> +	temperature = <100000>;
>> +};
>> +
>> +&sensor_thermal_cr52 {
>> +	critical-action = "shutdown";
>> +};
>> +
>> +&sensor_thermal_cnn {
>> +	critical-action = "shutdown";
>> +};
> 
> Is this not the default action for critical trip points? In my testing
> in the past R-Car systems have always shutdown when the critical trip is
> reached.

It isn't quite that clear cut.

drivers/thermal/thermal_of.c thermal_of_zone_register() contains this 
piece of code:

"
407         ret = of_property_read_string(np, "critical-action", &action);
408         if (!ret && !of_ops.critical) {
409                 if (!strcasecmp(action, "reboot"))
410                         of_ops.critical = 
thermal_zone_device_critical_reboot;
411                 else if (!strcasecmp(action, "shutdown"))
412                         of_ops.critical = 
thermal_zone_device_critical_shutdown;
413         }
"

If "critical-action" DT property is not set, then of_ops.critical are 
not modified.

drivers/thermal/thermal_core.c thermal_zone_device_register_with_trips() 
contains this piece of code:

1571         if (!tz->ops.critical)
1572                 tz->ops.critical = thermal_zone_device_critical;

If (in case of OF) of_ops.critical is not set, use 
thermal_zone_device_critical() handler.

There is a slight difference:
- If critical-action = "shutdown" is set in DT, then handler
   thermal_zone_device_critical_shutdown() is called, which is a wrapper
   around thermal_zone_device_halt(tz, HWPROT_ACT_SHUTDOWN);
- If critical-action = "shutdown" is NOT set in DT, then handler
   thermal_zone_device_critical() is called, which is a wrapper
   around thermal_zone_device_halt(tz, HWPROT_ACT_DEFAULT);

thermal_zone_device_halt() itself is a wrapper around 
__hw_protection_trigger(msg, poweroff_delay_ms, action); , where action 
is either HWPROT_ACT_SHUTDOWN or HWPROT_ACT_DEFAULT , which is handled 
in kernel/reboot.c __hw_protection_trigger() implementation :

1028 void __hw_protection_trigger(const char *reason, int ms_until_forced,
1029                              enum hw_protection_action action)
1030 {
1031         static atomic_t allow_proceed = ATOMIC_INIT(1);
1032
1033         if (action == HWPROT_ACT_DEFAULT)
1034                 action = hw_protection_action;

In case of HWPROT_ACT_DEFAULT , the 'hw_protection_action' which is 
assigned into 'action' can be overridden, either via sysfs write, or 
hw_protection_ kernel command line parameter . In case of 
HWPROT_ACT_SHUTDOWN , the action cannot be overridden .

In case this hardware starts to melt, we surely want HWPROT_ACT_SHUTDOWN 
with no override options ...

> If it's not I think we should move these to r8a779g0.dtsi. And
> likely add them for all other SoCs too?

... the other hardware has non-optional heatsink, where override-able 
HWPROT_ACT_DEFAULT is the right option I think .