Message-ID: <20260109095937.3317991-1-geert+renesas@glider.be>
Date: Fri,  9 Jan 2026 10:59:37 +0100
From: Geert Uytterhoeven <geert+renesas@...der.be>
To: biju.das.jz@...renesas.com,
	cosmin-gabriel.tanislav.xa@...esas.com,
	john.madieu.xa@...renesas.com
Cc: conor+dt@...nel.org,
	daniel.lezcano@...aro.org,
	devicetree@...r.kernel.org,
	geert+renesas@...der.be,
	krzk+dt@...nel.org,
	linux-kernel@...r.kernel.org,
	linux-pm@...r.kernel.org,
	linux-renesas-soc@...r.kernel.org,
	lukasz.luba@....com,
	magnus.damm@...il.com,
	p.zabel@...gutronix.de,
	rafael@...nel.org,
	robh@...nel.org,
	rui.zhang@...el.com
Subject: Re: [PATCH v5 5/5] thermal: renesas: rzg3e: add support for RZ/T2H and RZ/N2H

Hi Biju, Cosmin, John,

On Fri, 9 Jan 2026 at 10:06, Biju Das <biju.das.jz@...renesas.com> wrote:
> > From: Cosmin-Gabriel Tanislav <cosmin-gabriel.tanislav.xa@...esas.com>
> > > From: Biju Das <biju.das.jz@...renesas.com>
> > > > From: Cosmin Tanislav <cosmin-gabriel.tanislav.xa@...esas.com>
> > > > The Renesas RZ/T2H (R9A09G077) and RZ/N2H (R9A09G087) SoCs expose
> > > > the temperature calibration via SMC
> > > > SIP and do not have a reset for the TSU peripheral, and use
> > > > different minimum and maximum temperature
> > > > values compared to the already supported RZ/G3E.
> > > >
> > > > Although the calibration data is stored in an OTP memory, the OTP
> > > > itself is not memory-mapped, access
> > > > to it is done through an OTP controller.
> > > >
> > > > The OTP controller is only accessible from the secure world, but the
> > > > temperature calibration data stored in the OTP is exposed via SMC.
> > > >
> > > > Add support for retrieving the calibration data using arm_smcc_smc().
> > > >
> > > > Add a compatible for RZ/T2H, RZ/N2H can use it as a fallback.
> > > >
> > > > Reviewed-by: John Madieu <john.madieu.xa@...renesas.com>
> > > > Tested-by: John Madieu <john.madieu.xa@...renesas.com>
> > > > Signed-off-by: Cosmin Tanislav <cosmin-gabriel.tanislav.xa@...esas.com>

> > > > --- a/drivers/thermal/renesas/rzg3e_thermal.c
> > > > +++ b/drivers/thermal/renesas/rzg3e_thermal.c
> > > > @@ -4,6 +4,7 @@
> > > >   *
> > > >   * Copyright (C) 2025 Renesas Electronics Corporation
> > > >   */
> > > > +#include <linux/arm-smccc.h>
> > > >  #include <linux/clk.h>
> > > >  #include <linux/cleanup.h>
> > > >  #include <linux/delay.h>
> > > > @@ -70,6 +71,10 @@
> > > >  #define TSU_POLL_DELAY_US        10      /* Polling interval */
> > > >  #define TSU_MIN_CLOCK_RATE       24000000  /* TSU_PCLK minimum 24MHz */
> > > >
> > > > +#define RZ_SIP_SVC_GET_SYSTSU    0x82000022
> > >
> > > Maybe add a comment mentioning firmware should support this index and
> > > the otp value is stored in
> > > arm_smccc_res.a0
> >
> > The fact that the calibration value is stored in .a0 is clear from the retrieval code, let's not add
> > comments where the code is straightforward.

Agreed.

> If you have just a0, then driver expect a0 from firmware
> is either error and OTP value.
>
> If you have a0 and a1
>
> Success case a0=0
> Error case a0=SMC_UNK
>
> a1 will have the value from OTP.
>
> > Regarding the firmware support, it's obvious that the firmware needs to support this and that the
> > values don't just magically appear, no?
>
> How do you share this info to customers that they have their own firmware?
>
> Eg: Customer firmware is using different service ID and driver uses different one.

Specify the service ID in DT?

> > > > +#define OTP_TSU_REG_ADR_TEMPHI   0x01DC
> > > > +#define OTP_TSU_REG_ADR_TEMPLO   0x01DD
> > > > +
> > > >  struct rzg3e_thermal_priv;
> > > >
> > > >  struct rzg3e_thermal_info {
> > > > @@ -362,6 +367,21 @@ static int rzg3e_thermal_get_syscon_trim(struct rzg3e_thermal_priv *priv)
> > > >   return 0;
> > > >  }
> > > >
> > > > +static int rzg3e_thermal_get_smc_trim(struct rzg3e_thermal_priv
> > > > +*priv) {
> > > > + struct arm_smccc_res local_res;
> > > > +
> > > > + arm_smccc_smc(RZ_SIP_SVC_GET_SYSTSU, OTP_TSU_REG_ADR_TEMPLO,
> > > > +               0, 0, 0, 0, 0, 0, &local_res);
> > > > + priv->trmval0 = local_res.a0 & TSU_CODE_MAX;
> > >
> > > Do you think it is worth to ask firmware team to return error values
> > > in a0 and actual OTP value in a1.
> > >
> > > So that driver can check the error code and propagate to the caller.
> >
> > If we do that, we will have one more variant to handle here, as we cannot make sure that the TF-A
> > running on the board is always the latest.
>
> Mainline will use new variant, that can have both a0 and a1, if we take that route.
>
> > Right now things are simple as it's either supported or not supported.

Agreed.

> > If a0 is some error value, how would you distinguish between an error in the new variant and a proper
> > value in the old variant? Both cases would only populate a0.
> >
> > Also, I'm not sure how much use we can get out of a TF-A error value.
> >
> > The error that TF-A already returns in SMC_UNK = -1, or 0xFFFFFFFF in u32, it is pretty standard for
> > SMC calls and the probe() function already checks against it.
>
> The OTP value can be 0xFFFFFFFF, if it is not programmed, if that is case
> How do you distinguish error with respect actual otp value.

What is the expected behavior when the firmware interface is not
available, or the OTP is not programmed?
Currently the thermal driver fails to probe, which is IMHO suboptimal.
An alternative would be to fallback to default calibration values,
as done on R-Car Gen3.  To handle that, rzg3e_thermal_get_smc_trim()
should store the unmasked values in trmval[01], so the probe code can
detect this case.

BTW, what are the default values of the trim registers on RZ/G3E, when
the device is not trimmed? Or is this always done, unlike programming
the OTP on RZ/T2H?

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@...ux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Powered by blists - more mailing lists