[PATCH 0/2] thermal: rcar_gen3: Improve reading calibration fuses

[PATCH 0/2] thermal: rcar_gen3: Improve reading calibration fuses

[Niklas Söderlund]

Hello,

This small series aims to remove the code duplication that was added 
when adding support for Gen4 devices. The logic to read the calibration 
fuses are identical for Gen3 and Gen4, but the registers are at 
different offsets.

Patch 1/2 is a small drive-by patch which fixes a style issue of the 
constants found when working on the fuses code. While patch 2/2 is the 
real work removing the code duplication.

It is tested on both Gen3 and Gen4 boards, but as not all boards have 
the fused calibration values and the driver fallback to hardcoded values 
only Gen4 have really been tested as I don't have access to a Gen3 board 
with fused calibration values.

Niklas Söderlund (2):
  thermal: rcar_gen3: Use lowercase hex constants
  thermal: rcar_gen3: Reuse logic to read fuses on Gen3 and Gen4

 drivers/thermal/renesas/rcar_gen3_thermal.c | 85 +++++++++------------
 1 file changed, 34 insertions(+), 51 deletions(-)

-- 
2.47.0

[PATCH 1/2] thermal: rcar_gen3: Use lowercase hex constants

[Niklas Söderlund]

The style of the driver is to use lowercase hex constants, correct the
few outlines.

Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
---
 drivers/thermal/renesas/rcar_gen3_thermal.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/thermal/renesas/rcar_gen3_thermal.c b/drivers/thermal/renesas/rcar_gen3_thermal.c
index 810f86677461..35b8ecb8877e 100644
--- a/drivers/thermal/renesas/rcar_gen3_thermal.c
+++ b/drivers/thermal/renesas/rcar_gen3_thermal.c
@@ -57,11 +57,11 @@
 /* THSCP bits */
 #define THSCP_COR_PARA_VLD	(BIT(15) | BIT(14))
 
-#define CTEMP_MASK	0xFFF
+#define CTEMP_MASK	0xfff
 
 #define MCELSIUS(temp)	((temp) * 1000)
-#define GEN3_FUSE_MASK	0xFFF
-#define GEN4_FUSE_MASK	0xFFF
+#define GEN3_FUSE_MASK	0xfff
+#define GEN4_FUSE_MASK	0xfff
 
 #define TSC_MAX_NUM	5
 
-- 
2.47.0

[PATCH 2/2] thermal: rcar_gen3: Reuse logic to read fuses on Gen3 and Gen4

[Niklas Söderlund]

The hardware calibration is fused on some, but not all, Gen3 and Gen4
boards. The calibrations values are the same on both generations but
located at different register offsets.

Instead of having duplicated logic to read the and store the values
create a helper function to do the reading and just feed it with the
correct register addresses for each generation,

Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
---
 drivers/thermal/renesas/rcar_gen3_thermal.c | 79 ++++++++-------------
 1 file changed, 31 insertions(+), 48 deletions(-)

diff --git a/drivers/thermal/renesas/rcar_gen3_thermal.c b/drivers/thermal/renesas/rcar_gen3_thermal.c
index 35b8ecb8877e..821b4405e346 100644
--- a/drivers/thermal/renesas/rcar_gen3_thermal.c
+++ b/drivers/thermal/renesas/rcar_gen3_thermal.c
@@ -253,60 +253,43 @@ static irqreturn_t rcar_gen3_thermal_irq(int irq, void *data)
 	return IRQ_HANDLED;
 }
 
+static void rcar_gen3_thermal_fetch_fuses(struct rcar_gen3_thermal_priv *priv,
+					  u32 ptat0, u32 ptat1, u32 ptat2,
+					  u32 thcode0, u32 thcode1, u32 thcode2,
+					  u32 mask)
+{
+	/*
+	 * Set the pseudo calibration points with fused values.
+	 * PTAT is shared between all TSCs but only fused for the first
+	 * TSC while THCODEs are fused for each TSC.
+	 */
+	priv->ptat[0] = rcar_gen3_thermal_read(priv->tscs[0], ptat0) & mask;
+	priv->ptat[1] = rcar_gen3_thermal_read(priv->tscs[0], ptat1) & mask;
+	priv->ptat[2] = rcar_gen3_thermal_read(priv->tscs[0], ptat2) & mask;
+
+	for (unsigned int i = 0; i < priv->num_tscs; i++) {
+		struct rcar_gen3_thermal_tsc *tsc = priv->tscs[i];
+
+		tsc->thcode[0] = rcar_gen3_thermal_read(tsc, thcode0) & mask;
+		tsc->thcode[1] = rcar_gen3_thermal_read(tsc, thcode1) & mask;
+		tsc->thcode[2] = rcar_gen3_thermal_read(tsc, thcode2) & mask;
+	}
+}
+
 static void rcar_gen3_thermal_read_fuses_gen3(struct rcar_gen3_thermal_priv *priv)
 {
-	unsigned int i;
-
-	/*
-	 * Set the pseudo calibration points with fused values.
-	 * PTAT is shared between all TSCs but only fused for the first
-	 * TSC while THCODEs are fused for each TSC.
-	 */
-	priv->ptat[0] = rcar_gen3_thermal_read(priv->tscs[0], REG_GEN3_PTAT1) &
-		GEN3_FUSE_MASK;
-	priv->ptat[1] = rcar_gen3_thermal_read(priv->tscs[0], REG_GEN3_PTAT2) &
-		GEN3_FUSE_MASK;
-	priv->ptat[2] = rcar_gen3_thermal_read(priv->tscs[0], REG_GEN3_PTAT3) &
-		GEN3_FUSE_MASK;
-
-	for (i = 0; i < priv->num_tscs; i++) {
-		struct rcar_gen3_thermal_tsc *tsc = priv->tscs[i];
-
-		tsc->thcode[0] = rcar_gen3_thermal_read(tsc, REG_GEN3_THCODE1) &
-			GEN3_FUSE_MASK;
-		tsc->thcode[1] = rcar_gen3_thermal_read(tsc, REG_GEN3_THCODE2) &
-			GEN3_FUSE_MASK;
-		tsc->thcode[2] = rcar_gen3_thermal_read(tsc, REG_GEN3_THCODE3) &
-			GEN3_FUSE_MASK;
-	}
+	rcar_gen3_thermal_fetch_fuses(priv,
+				      REG_GEN3_PTAT1, REG_GEN3_PTAT2, REG_GEN3_PTAT3,
+				      REG_GEN3_THCODE1, REG_GEN3_THCODE2, REG_GEN3_THCODE3,
+				      GEN3_FUSE_MASK);
 }
 
 static void rcar_gen3_thermal_read_fuses_gen4(struct rcar_gen3_thermal_priv *priv)
 {
-	unsigned int i;
-
-	/*
-	 * Set the pseudo calibration points with fused values.
-	 * PTAT is shared between all TSCs but only fused for the first
-	 * TSC while THCODEs are fused for each TSC.
-	 */
-	priv->ptat[0] = rcar_gen3_thermal_read(priv->tscs[0], REG_GEN4_THSFMON16) &
-		GEN4_FUSE_MASK;
-	priv->ptat[1] = rcar_gen3_thermal_read(priv->tscs[0], REG_GEN4_THSFMON17) &
-		GEN4_FUSE_MASK;
-	priv->ptat[2] = rcar_gen3_thermal_read(priv->tscs[0], REG_GEN4_THSFMON15) &
-		GEN4_FUSE_MASK;
-
-	for (i = 0; i < priv->num_tscs; i++) {
-		struct rcar_gen3_thermal_tsc *tsc = priv->tscs[i];
-
-		tsc->thcode[0] = rcar_gen3_thermal_read(tsc, REG_GEN4_THSFMON01) &
-			GEN4_FUSE_MASK;
-		tsc->thcode[1] = rcar_gen3_thermal_read(tsc, REG_GEN4_THSFMON02) &
-			GEN4_FUSE_MASK;
-		tsc->thcode[2] = rcar_gen3_thermal_read(tsc, REG_GEN4_THSFMON00) &
-			GEN4_FUSE_MASK;
-	}
+	rcar_gen3_thermal_fetch_fuses(priv,
+				      REG_GEN4_THSFMON16, REG_GEN4_THSFMON17, REG_GEN4_THSFMON15,
+				      REG_GEN4_THSFMON01, REG_GEN4_THSFMON02, REG_GEN4_THSFMON00,
+				      GEN4_FUSE_MASK);
 }
 
 static bool rcar_gen3_thermal_read_fuses(struct rcar_gen3_thermal_priv *priv)
-- 
2.47.0

Re: [PATCH 1/2] thermal: rcar_gen3: Use lowercase hex constants

[Geert Uytterhoeven]

Hi Niklas,

On Sat, Nov 16, 2024 at 6:30 PM Niklas Söderlund
<niklas.soderlund+renesas@ragnatech.se> wrote:
> The style of the driver is to use lowercase hex constants, correct the
> few outlines.
>
> Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>

Thanks for your patch!

> --- a/drivers/thermal/renesas/rcar_gen3_thermal.c
> +++ b/drivers/thermal/renesas/rcar_gen3_thermal.c
> @@ -57,11 +57,11 @@
>  /* THSCP bits */
>  #define THSCP_COR_PARA_VLD     (BIT(15) | BIT(14))
>
> -#define CTEMP_MASK     0xFFF
> +#define CTEMP_MASK     0xfff
>
>  #define MCELSIUS(temp) ((temp) * 1000)
> -#define GEN3_FUSE_MASK 0xFFF
> -#define GEN4_FUSE_MASK 0xFFF
> +#define GEN3_FUSE_MASK 0xfff
> +#define GEN4_FUSE_MASK 0xfff
>
>  #define TSC_MAX_NUM    5

LGTM, but you missed a few:

$ git grep "\<0x.*[A-Z]" -- drivers/thermal/renesas/rcar_gen3_thermal.c
drivers/thermal/renesas/rcar_gen3_thermal.c:#define REG_GEN3_IRQCTL
         0x0C
drivers/thermal/renesas/rcar_gen3_thermal.c:#define REG_GEN3_IRQTEMP3   0x1C
drivers/thermal/renesas/rcar_gen3_thermal.c:#define REG_GEN4_THSFMON15  0x1BC
drivers/thermal/renesas/rcar_gen3_thermal.c:#define REG_GEN4_THSFMON16  0x1C0
drivers/thermal/renesas/rcar_gen3_thermal.c:#define REG_GEN4_THSFMON17  0x1C4

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-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

Re: [PATCH 1/2] thermal: rcar_gen3: Use lowercase hex constants

[Niklas Söderlund]

Hi Geert,

On 2024-11-18 11:30:17 +0100, Geert Uytterhoeven wrote:
> Hi Niklas,
> 
> On Sat, Nov 16, 2024 at 6:30 PM Niklas Söderlund
> <niklas.soderlund+renesas@ragnatech.se> wrote:
> > The style of the driver is to use lowercase hex constants, correct the
> > few outlines.
> >
> > Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
> 
> Thanks for your patch!
> 
> > --- a/drivers/thermal/renesas/rcar_gen3_thermal.c
> > +++ b/drivers/thermal/renesas/rcar_gen3_thermal.c
> > @@ -57,11 +57,11 @@
> >  /* THSCP bits */
> >  #define THSCP_COR_PARA_VLD     (BIT(15) | BIT(14))
> >
> > -#define CTEMP_MASK     0xFFF
> > +#define CTEMP_MASK     0xfff
> >
> >  #define MCELSIUS(temp) ((temp) * 1000)
> > -#define GEN3_FUSE_MASK 0xFFF
> > -#define GEN4_FUSE_MASK 0xFFF
> > +#define GEN3_FUSE_MASK 0xfff
> > +#define GEN4_FUSE_MASK 0xfff
> >
> >  #define TSC_MAX_NUM    5
> 
> LGTM, but you missed a few:
> 
> $ git grep "\<0x.*[A-Z]" -- drivers/thermal/renesas/rcar_gen3_thermal.c
> drivers/thermal/renesas/rcar_gen3_thermal.c:#define REG_GEN3_IRQCTL
>          0x0C
> drivers/thermal/renesas/rcar_gen3_thermal.c:#define REG_GEN3_IRQTEMP3   0x1C
> drivers/thermal/renesas/rcar_gen3_thermal.c:#define REG_GEN4_THSFMON15  0x1BC
> drivers/thermal/renesas/rcar_gen3_thermal.c:#define REG_GEN4_THSFMON16  0x1C0
> drivers/thermal/renesas/rcar_gen3_thermal.c:#define REG_GEN4_THSFMON17  0x1C4

d00h, I just checked manually for things around the fuses, will fix.

> 
> Gr{oetje,eeting}s,
> 
>                         Geert
> 
> -- 
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-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

-- 
Kind Regards,
Niklas Söderlund