RE: [PATCH 2/6] thermal: of: Export non-devres helper to register/unregister thermal zone

Biju Das
2025-01-30T17:47:26Z

Hi Daniel Lezcano,

> -----Original Message-----
> From: Daniel Lezcano <daniel.lezcano@linaro.org>
> Sent: 30 January 2025 17:32
> Subject: Re: [PATCH 2/6] thermal: of: Export non-devres helper to register/unregister thermal zone
>
> On 30/01/2025 11:33, Biju Das wrote:
> > Hi Daniel Lezcano,
> >
> >> -----Original Message-----
>
> [ ... ]
>
> >>>> I've been through the driver before responding to this change. What
> >>>> is the benefit of powering down / up (or clock off / on) the
> >>>> thermal sensor when reading the temperature ?
> >>>>
> >>>> I can understand for disable / enable but I don't get for the
> >>>> classic usage where a governor will be reading the temperature regularly.
> >>>
> >>> I tried to be as power saving as possible both at runtime and after
> >>> the IP is not used anymore as the HW manual doesn't mentioned
> >>> anything about accuracy or implications of disabling the IP clock at runtime.
> >>> We use similar approach (of disabling clocks at runtime) for other
> >>> IPs in the RZ/G3S SoC as well.
> >>>
> >>>>
> >>>> Would the IP need some cycles to capture the temperature accurately
> >>>> after the clock is enabled ?
> >>>
> >>> There is nothing about this mentioned about this in the HW manual of
> >>> the RZ/G3S SoC. The only points mentioned are as described in the driver code:
> >>> - wait at least 3us after each IIO channel read
> >>> - wait at least 30us after enabling the sensor
> >>> - wait at least 50us after setting OE bit in TSU_SM
> >>>
> >>> For this I chose to have it implemented as proposed.
> >>
> >> IMO, disabling/enabling the clock between two reads through the pm
> >> runtime may not be a good thing, especially if the system enters a thermal situation where it has
> to mitigate.
> >
> > Just a question, You mean to avoid device destruction due to high
> > temperature?? Assuming disabling the clk happens when the temp reaches
> > the boundary and re-enabling of the clk after a time(which involves monitoring the CLK ON bit after
> enabling it, or a run time enable failure happens), where it exceeds the threshold??
>
>
> Well, I have some comments with the device tree thermal configuration which may answer your question
> but I'll wait for Claudiu to check the temperature read comparison without rounding to 0.5°C
>
> What I meant is if the temperature read is inaccurate, the mitigation will be inaccurate too. It may
> not reach the critical temperature but it is possible the performance could be impacted negatively
> under thermal stress.

Thanks for the explanation.

I thought you meant " disabling/enabling the clock between two reads through the pm
runtime may not be a good thing" under stress/hot condition, temperature raises, and
in those corner cases if runtime PM fails, then we cannot read temperature and
we cannot take any corrective action.

Cheers,
Biju