RE: [PATCH v6 3/5] thermal: renesas: rzg3e: Add thermal driver for the Renesas RZ/G3E SoC

[John Madieu <john.madieu.xa@bp.renesas.com>]

Hi Daniel,

Thanks again for the feedback.

> -----Original Message-----
> From: Daniel Lezcano <daniel.lezcano@linaro.org>
> Sent: Tuesday, August 5, 2025 12:06 PM
> To: John Madieu <john.madieu.xa@bp.renesas.com>
> Subject: Re: [PATCH v6 3/5] thermal: renesas: rzg3e: Add thermal driver for
> the Renesas RZ/G3E SoC
> 
> 
> Hi John,
> 
> 
> On 05/08/2025 09:49, John Madieu wrote:
> 
> [ ... ]
> 
> >>> I might not get what you are asking, but since I compute the
> >>> temperature in the hard IRQ handler, I just wait for it to complete
> >>> and notify the completion so I can grab the processed value to
> >>> notify the thermal core.
> >>>
> >>> Please let me know if this does not answer your question.
> >>
> >> Can you describe how the sensor works ? And perhaps if you have a
> >> pointer to some documentation ?
> >
> > Here is the documentation [1]. The thermal device is referred to as TSU.
> >
> > [1]
> > https://www.renesas.com/en/document/mah/rzg3e-group-users-manual-hardw
> > are?r=25574493
> >
> >>    [ ... ]
> 
> Thanks for the pointer. I got it now ;)
> 
> I'm a bit worried about the latency introduced by this mechanism when the
> system is entering in a thermal pressure episode.
> 
> The get_temp function wait for a completion up to 100ms, it is a lot.
> Especially if the userspace can be reading the temperature and blocking the
> read.
> 
> There is also the spin_lock taken introducing another lock while the
> get_temp function is holding a mutex on the thermal zone.
> 
> Did you it that under stress ?
> 

After spending some time stressing the driver, I've noticed a
couple of issues:

 * Spinlock/mutex mux caused some issues, and I had cases where
   it timed-out while not even under stress.
 
 * Mixing both compare (cmp) and conversion complete (conv) IRQs
 introduced some latencies and inconsistencies.

After spending some time in the datasheet, I could notice that one
conversion takes around 50µs. In average mode (8 samples per conversion),
it would take 400µs, which I doubled (for margin) and used as timeout in
v7 series that is already ready. 

Moreover, as per datasheet recommendations, I kept comparison IRQ 
(for trip point violation) only, while using polling for get_temp()
(with the 800µs timeout), which gives better results.


If you don't mind, I'll send a v7 which addresses all the
requests from Geert as well as the above-mentioned changes.

It includes:

 * 800µs timeout for get_temp() in polling read
 * No spinlock/mutex mix and no completion anymore
 * Comparison-only IRQ for trip point violations
 * + Geert's change requests

What do you think ?

Regards,
John.

> --
> <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