[PATCH 0/5] thermal: renesas: rzg3e: TSU driver fixes and cleanups

[PATCH 0/5] thermal: renesas: rzg3e: TSU driver fixes and cleanups

[Ovidiu Panait]

Hi,

This series contains a few fixes for the Renesas RZ/G3E TSU thermal
driver, found while reviewing the driver against the hardware manual,
along with some small cleanups.

Thanks,
Ovidiu

Ovidiu Panait (5):
  thermal: renesas: rzg3e: Fix signed integer overflow in temp_to_code()
  thermal: renesas: rzg3e: Fix runtime PM handling of compare irq
  thermal: renesas: rzg3e: Fix race between set_trips() and get_temp()
  thermal: renesas: rzg3e: Remove unneeded low/high check in set_trips()
  thermal: renesas: rzg3e: Remove redundant pm_runtime_mark_last_busy()
    calls

 drivers/thermal/renesas/rzg3e_thermal.c | 35 +++++++++++++++++--------
 1 file changed, 24 insertions(+), 11 deletions(-)

-- 
2.34.1

[PATCH 1/5] thermal: renesas: rzg3e: Fix signed integer overflow in temp_to_code()

[Ovidiu Panait]

The thermal core passes -INT_MAX/INT_MAX as low/high trip points, which
the driver then converts to code values for programming the registers:

numerator = (temp_mc - info->temp_d_mc) * (priv->trmval1 - priv->trmval0);

Although 'numerator' is defined as s64, the arithmetic is performed as int.
Because of this, the multiplication overflows for -INT_MAX and, because
temp_d_mc is negative, the subtraction overflows for INT_MAX.

To fix this, cast the temperature to s64, so that the arithmetic is
performed as s64.

Fixes: dc67521c20b7 ("thermal/drivers/renesas/rzg3e: Fix add thermal driver for the Renesas RZ/G3E SoC")
Signed-off-by: Ovidiu Panait <ovidiu.panait.rb@renesas.com>
---
 drivers/thermal/renesas/rzg3e_thermal.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/thermal/renesas/rzg3e_thermal.c b/drivers/thermal/renesas/rzg3e_thermal.c
index c44f5b8858d0..4acee4eaff05 100644
--- a/drivers/thermal/renesas/rzg3e_thermal.c
+++ b/drivers/thermal/renesas/rzg3e_thermal.c
@@ -194,7 +194,12 @@ static u16 rzg3e_thermal_temp_to_code(struct rzg3e_thermal_priv *priv, int temp_
 	s64 numerator, denominator;
 	s64 code;
 
-	numerator = (temp_mc - info->temp_d_mc) * (priv->trmval1 - priv->trmval0);
+	/*
+	 * Perform the arithmetic in 64 bits so that it cannot overflow for
+	 * -INT_MAX/INT_MAX values passed from the thermal core or when
+	 * userspace writes arbitrary trip point temperatures.
+	 */
+	numerator = ((s64)temp_mc - info->temp_d_mc) * (priv->trmval1 - priv->trmval0);
 	denominator = info->temp_e_mc - info->temp_d_mc;
 
 	code = div64_s64(numerator, denominator) + priv->trmval0;
-- 
2.34.1

[PATCH 2/5] thermal: renesas: rzg3e: Fix runtime PM handling of compare irq

[Ovidiu Panait]

The TSU hw is limited in the sense that lower and upper trip points can be
set, but the hw cannot raise interrupts on its own. It can only signal that
the thresholds were passed when the temperature is read, either during
polling or on a manual read. Therefore, the interrupt enable state
configured by set_trips() must remain active for the next read to trigger
an event.

Currently, runtime PM autosuspend puts the chip to sleep and clears the
compare interrupt that was enabled during set_trips(). On power up, the
compare interrupt enable is not restored, which means that it won't ever
fire.

Fix this by caching the contents of the SIER register during runtime power
off and restore it during runtime power on.

Fixes: dc67521c20b7 ("thermal/drivers/renesas/rzg3e: Fix add thermal driver for the Renesas RZ/G3E SoC")
Signed-off-by: Ovidiu Panait <ovidiu.panait.rb@renesas.com>
---
 drivers/thermal/renesas/rzg3e_thermal.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/thermal/renesas/rzg3e_thermal.c b/drivers/thermal/renesas/rzg3e_thermal.c
index 4acee4eaff05..45f67b2e1131 100644
--- a/drivers/thermal/renesas/rzg3e_thermal.c
+++ b/drivers/thermal/renesas/rzg3e_thermal.c
@@ -93,6 +93,7 @@ struct rzg3e_thermal_info {
  * @info: chip type specific information
  * @trmval0: calibration value 0 (b)
  * @trmval1: calibration value 1 (c)
+ * @sier: cached interrupt enable register
  * @lock: protects hardware access during conversions
  */
 struct rzg3e_thermal_priv {
@@ -103,6 +104,7 @@ struct rzg3e_thermal_priv {
 	const struct rzg3e_thermal_info *info;
 	u16 trmval0;
 	u16 trmval1;
+	u32 sier;
 	struct mutex lock;
 };
 
@@ -148,12 +150,16 @@ static int rzg3e_thermal_power_on(struct rzg3e_thermal_priv *priv)
 		return ret;
 	}
 
+	/* Restore interrupt enable state */
+	writel(priv->sier, priv->base + TSU_SIER);
+
 	return 0;
 }
 
 static void rzg3e_thermal_power_off(struct rzg3e_thermal_priv *priv)
 {
-	/* Disable all interrupts */
+	/* Save and disable all interrupts */
+	priv->sier = readl(priv->base + TSU_SIER);
 	writel(0, priv->base + TSU_SIER);
 
 	/* Clear pending interrupts */
-- 
2.34.1

[PATCH 3/5] thermal: renesas: rzg3e: Fix race between set_trips() and get_temp()

[Ovidiu Panait]

Datasheet section 7.11.9.2 "Conversion Start Trigger" states that a
conversion must only be started while no other conversion is in
progress:

  "Before making the setting to start conversion by software or an ELC
   trigger, check that the CONV bit of the sensor status register is 0b."

set_trips() starts a conversion at the end of the function and returns
without waiting for it to finish. A get_temp() call arriving right after
that starts a conversion while the CONV bit may still be set, violating
the requirement above.

Fix this by:
 - polling for CONV == 0 in get_temp() before starting a conversion
 - not starting a conversion at the end of set_trips() anymore

The conversion started at the end of set_trips() was supposed to check
whether the thresholds have been passed, since the hw cannot raise
interrupts on its own, but only when the temperature is read. However,
the thermal core reads the temperature and computes the trip window from
it just before calling into set_trips(), so reading the temperature again
straight away is useless.

Fixes: dc67521c20b7 ("thermal/drivers/renesas/rzg3e: Fix add thermal driver for the Renesas RZ/G3E SoC")
Signed-off-by: Ovidiu Panait <ovidiu.panait.rb@renesas.com>
---
 drivers/thermal/renesas/rzg3e_thermal.c | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/drivers/thermal/renesas/rzg3e_thermal.c b/drivers/thermal/renesas/rzg3e_thermal.c
index 45f67b2e1131..58fdcc66456a 100644
--- a/drivers/thermal/renesas/rzg3e_thermal.c
+++ b/drivers/thermal/renesas/rzg3e_thermal.c
@@ -225,6 +225,16 @@ static int rzg3e_thermal_get_temp(struct thermal_zone_device *tz, int *temp)
 
 	guard(mutex)(&priv->lock);
 
+	/* Make sure a previous conversion is not in progress */
+	ret = readl_poll_timeout(priv->base + TSU_SSR, status,
+				 !(status & TSU_SSR_CONV),
+				 TSU_POLL_DELAY_US,
+				 USEC_PER_MSEC);
+	if (ret) {
+		dev_err(priv->dev, "Timeout waiting for conversion\n");
+		goto out;
+	}
+
 	/* Clear any previous conversion status */
 	writel(TSU_SICR_ADCLR, priv->base + TSU_SICR);
 
@@ -304,9 +314,8 @@ static int rzg3e_thermal_set_trips(struct thermal_zone_device *tz,
 	/* Enable comparison with "out of range" mode (CMPCOND=0) */
 	writel(TSU_CMSR_CMPEN, priv->base + TSU_CMSR);
 
-	/* Unmask compare IRQ and start a conversion to evaluate window */
+	/* Unmask compare IRQ */
 	writel(TSU_SIER_CMPIE, priv->base + TSU_SIER);
-	writel(TSU_STRGR_ADST, priv->base + TSU_STRGR);
 
 	pm_runtime_mark_last_busy(priv->dev);
 	pm_runtime_put_autosuspend(priv->dev);
-- 
2.34.1

[PATCH 4/5] thermal: renesas: rzg3e: Remove unneeded low/high check in set_trips()

[Ovidiu Panait]

The hardware manual doesn't specify that the trip settings must be
strictly low < high, just that when the CMPCOND bit is 0, an interrupt
will be raised when the acquired temperature data is > ULIM or < LLIM.

Also, the thermal core already guarantees low < high.

Remove the redundant check.

Signed-off-by: Ovidiu Panait <ovidiu.panait.rb@renesas.com>
---
 drivers/thermal/renesas/rzg3e_thermal.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/drivers/thermal/renesas/rzg3e_thermal.c b/drivers/thermal/renesas/rzg3e_thermal.c
index 58fdcc66456a..ffe2b8725031 100644
--- a/drivers/thermal/renesas/rzg3e_thermal.c
+++ b/drivers/thermal/renesas/rzg3e_thermal.c
@@ -275,10 +275,6 @@ static int rzg3e_thermal_set_trips(struct thermal_zone_device *tz,
 	u32 val;
 	int ret;
 
-	/* Hardware requires low < high */
-	if (low >= high)
-		return -EINVAL;
-
 	ret = pm_runtime_resume_and_get(priv->dev);
 	if (ret < 0)
 		return ret;
-- 
2.34.1

[PATCH 5/5] thermal: renesas: rzg3e: Remove redundant pm_runtime_mark_last_busy() calls

[Ovidiu Panait]

pm_runtime_put_autosuspend() already calls pm_runtime_mark_last_busy()
internally, so the explicit calls are not needed. Remove them.

Signed-off-by: Ovidiu Panait <ovidiu.panait.rb@renesas.com>
---
 drivers/thermal/renesas/rzg3e_thermal.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/thermal/renesas/rzg3e_thermal.c b/drivers/thermal/renesas/rzg3e_thermal.c
index ffe2b8725031..a1ceff79e83e 100644
--- a/drivers/thermal/renesas/rzg3e_thermal.c
+++ b/drivers/thermal/renesas/rzg3e_thermal.c
@@ -262,7 +262,6 @@ static int rzg3e_thermal_get_temp(struct thermal_zone_device *tz, int *temp)
 		*temp, *temp / 1000, abs(*temp) % 1000, code);
 
 out:
-	pm_runtime_mark_last_busy(priv->dev);
 	pm_runtime_put_autosuspend(priv->dev);
 	return ret;
 }
@@ -313,7 +312,6 @@ static int rzg3e_thermal_set_trips(struct thermal_zone_device *tz,
 	/* Unmask compare IRQ */
 	writel(TSU_SIER_CMPIE, priv->base + TSU_SIER);
 
-	pm_runtime_mark_last_busy(priv->dev);
 	pm_runtime_put_autosuspend(priv->dev);
 
 	return 0;
@@ -485,7 +483,6 @@ static int rzg3e_thermal_probe(struct platform_device *pdev)
 	if (ret)
 		dev_warn(dev, "Failed to add hwmon sysfs attributes\n");
 
-	pm_runtime_mark_last_busy(dev);
 	pm_runtime_put_autosuspend(dev);
 
 	dev_info(dev, "RZ/G3E thermal sensor registered\n");
-- 
2.34.1

Re: [PATCH 0/5] thermal: renesas: rzg3e: TSU driver fixes and cleanups

[Daniel Lezcano]

On 9/16/26 13:59, Ovidiu Panait wrote:
> Hi,
> 
> This series contains a few fixes for the Renesas RZ/G3E TSU thermal
> driver, found while reviewing the driver against the hardware manual,
> along with some small cleanups.
> 
> Thanks,
> Ovidiu
> 
> Ovidiu Panait (5):
>    thermal: renesas: rzg3e: Fix signed integer overflow in temp_to_code()
>    thermal: renesas: rzg3e: Fix runtime PM handling of compare irq
>    thermal: renesas: rzg3e: Fix race between set_trips() and get_temp()
>    thermal: renesas: rzg3e: Remove unneeded low/high check in set_trips()
>    thermal: renesas: rzg3e: Remove redundant pm_runtime_mark_last_busy()
>      calls
> 
>   drivers/thermal/renesas/rzg3e_thermal.c | 35 +++++++++++++++++--------
>   1 file changed, 24 insertions(+), 11 deletions(-)
> 

Applied, thanks