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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID:
 <OSCPR01MB1464771054F720542F817E84FFF22A@OSCPR01MB14647.jpnprd01.prod.outlook.com>
Date: Tue, 5 Aug 2025 07:49:33 +0000
From: John Madieu <john.madieu.xa@...renesas.com>
To: Daniel Lezcano <daniel.lezcano@...aro.org>
CC: "conor+dt@...nel.org" <conor+dt@...nel.org>, "geert+renesas@...der.be"
	<geert+renesas@...der.be>, "krzk+dt@...nel.org" <krzk+dt@...nel.org>,
	"rafael@...nel.org" <rafael@...nel.org>, Biju Das
	<biju.das.jz@...renesas.com>, "devicetree@...r.kernel.org"
	<devicetree@...r.kernel.org>, "john.madieu@...il.com"
	<john.madieu@...il.com>, "linux-kernel@...r.kernel.org"
	<linux-kernel@...r.kernel.org>, "linux-pm@...r.kernel.org"
	<linux-pm@...r.kernel.org>, "linux-renesas-soc@...r.kernel.org"
	<linux-renesas-soc@...r.kernel.org>, "lukasz.luba@....com"
	<lukasz.luba@....com>, magnus.damm <magnus.damm@...il.com>, "robh@...nel.org"
	<robh@...nel.org>, "rui.zhang@...el.com" <rui.zhang@...el.com>,
	"sboyd@...nel.org" <sboyd@...nel.org>,
	"niklas.soderlund+renesas@...natech.se"
	<niklas.soderlund+renesas@...natech.se>
Subject: RE: [PATCH v6 3/5] thermal: renesas: rzg3e: Add thermal driver for
 the Renesas RZ/G3E SoC

Hi Danoel,

> -----Original Message-----
> From: Daniel Lezcano <daniel.lezcano@...aro.org>
> Sent: Monday, August 4, 2025 6:08 PM
> To: John Madieu <john.madieu.xa@...renesas.com>
> Subject: Re: [PATCH v6 3/5] thermal: renesas: rzg3e: Add thermal driver for
> the Renesas RZ/G3E SoC
> 
> On 31/07/2025 19:19, John Madieu wrote:
> > Hi Daniel,
> >
> > Thanks for your review.
> >
> >> -----Original Message-----
> >> From: Daniel Lezcano <daniel.lezcano@...aro.org>
> >> Sent: Wednesday, July 16, 2025 11:11 PM
> >> To: John Madieu <john.madieu.xa@...renesas.com>
> >> Subject: Re: [PATCH v6 3/5] thermal: renesas: rzg3e: Add thermal
> >> driver for the Renesas RZ/G3E SoC
> >>
> >> On Thu, May 22, 2025 at 08:22:46PM +0200, John Madieu wrote:
> >>> The RZ/G3E SoC integrates a Temperature Sensor Unit (TSU) block
> >>> designed to monitor the chip's junction temperature. This sensor is
> >>> connected to channel 1 of the APB port clock/reset and provides
> >> temperature measurements.
> >>>
> >>> It also requires calibration values stored in the system controller
> >>> registers for accurate temperature measurement. Add a driver for the
> >> Renesas RZ/G3E TSU.
> >>>
> >>> Signed-off-by: John Madieu <john.madieu.xa@...renesas.com>
> >>> ---
> 
> [ ... ]
> 
> >>> +static int rzg3e_thermal_get_temp(struct thermal_zone_device *zone,
> >>> +int *temp) {
> >>> +	struct rzg3e_thermal_priv *priv = thermal_zone_device_priv(zone);
> >>> +	u32 val;
> >>> +	int ret;
> >>> +
> >>> +	if (priv->mode == THERMAL_DEVICE_DISABLED)
> >>> +		return -EBUSY;
> 
> [ ... ]
> 
> >>> +	reinit_completion(&priv->conv_complete);
> >>> +
> >>> +	/* Enable ADC interrupt */
> >>> +	writel(TSU_SIER_ADIE, priv->base + TSU_SIER);
> >>
> >> Why enable irq here ?
> >>
> >
> > I did it this way because, in 'set_trips' callback, the driver does
> > trigger conversion to check whether the current temperature is part of
> > the window or not, and triggers the comparison interrupt accordingly.
> > Because of that, I did not want the conversion-complete interrupt to
> > also be triggered.
> >
> > That's the reason why I enable conversion-complete interrupt in
> > 'get_temp', to make sure its interrupt is being triggered only when
> > the thermal core calls it.
> >
> > Should I do it another way ?
> 
> I don't ATM, the approach is very unusual so I'm still trying to figure out
> what is this completion approach and readl_poll_timeout_atomic. At the
> first glance I would say it is wrong.
> 
> 
> >>> +	/* Verify no ongoing conversion */
> >>> +	ret = readl_poll_timeout_atomic(priv->base + TSU_SSR, val,
> >>> +					!(val & TSU_SSR_CONV_RUNNING),
> >>> +					TSU_POLL_DELAY_US, TSU_TIMEOUT_US);
> >>> +	if (ret) {
> >>> +		dev_err(priv->dev, "ADC conversion timed out\n");
> >>> +		return ret;
> >>> +	}
> >>> +
> >>> +	/* Start conversion */
> >>> +	writel(TSU_STRGR_ADST, priv->base + TSU_STRGR);
> >>> +
> >>> +	if (!wait_for_completion_timeout(&priv->conv_complete,
> >>> +					 msecs_to_jiffies(100))) {
> >>> +		dev_err(priv->dev, "ADC conversion completion timeout\n");
> >>> +		return -ETIMEDOUT;
> >>> +	}
> >>
> >> Can you explain what is happening here ?
> >>
> >
> > 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-hardware?r=25574493

>   [ ... ]
Regards,
John