Re: [PATCH v8 1/3] dt-bindings: thermal-zones: Document critical-action

[Daniel Lezcano <daniel.lezcano@linaro.org>]

On 16/09/2023 03:49, Fabio Estevam wrote:
> From: Fabio Estevam <festevam@denx.de>
> 
> Document the critical-action property to describe the thermal action
> the OS should perform after the critical temperature is reached.
> 
> The possible values are "shutdown" and "reboot".
> 
> The motivation for introducing the critical-action property is that
> different systems may need different thermal actions when the critical
> temperature is reached.
> 
> For example, a desktop PC may want the OS to trigger a shutdown
> when the critical temperature is reached.
> 
> However, in some embedded cases, such behavior does not suit well,
> as the board may be unattended in the field and rebooting may be a
> better approach.
> 
> The bootloader may also benefit from this new property as it can check
> the SoC temperature and in case the temperature is above the critical
> point, it can trigger a shutdown or reboot accordingly.
> 
> Signed-off-by: Fabio Estevam <festevam@denx.de>
> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> ---
> Changes since v7:
> - Made critical-action a property of the top-level thermal-zone node. (Rafael)
> 
>   .../devicetree/bindings/thermal/thermal-zones.yaml        | 8 ++++++++
>   1 file changed, 8 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/thermal/thermal-zones.yaml b/Documentation/devicetree/bindings/thermal/thermal-zones.yaml
> index 4f3acdc4dec0..d28f3fe1045d 100644
> --- a/Documentation/devicetree/bindings/thermal/thermal-zones.yaml
> +++ b/Documentation/devicetree/bindings/thermal/thermal-zones.yaml
> @@ -48,6 +48,14 @@ properties:
>         platform-data regarding temperature thresholds and the mitigation actions
>         to take when the temperature crosses those thresholds.
>   
> +  critical-action:
> +    $ref: /schemas/types.yaml#/definitions/string
> +    description:
> +      The action the OS should perform after the critical temperature is reached.
> +    enum:
> +      - shutdown
> +      - reboot

Given I'm catching up the versions, I'll directly comment this series 
instead of the previous versions.

 From my POV, the critical action to be done is not per system, but per 
critical trip point, which is per thermal zone.

Putting the critical action at the thermal zone device level is 
inconsistent with the fact the thermal zone may not have a critical trip 
point described.

So, the property should be either:

  - In the critical trip point description as optional

Or

  - At the thermal zone level if there is a critical trip point 
described (I don't know how this dependency can be described)

>   patternProperties:
>     "^[a-zA-Z][a-zA-Z0-9\\-]{1,12}-thermal$":
>       type: object

-- 
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog