diff mbox series

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

Message ID 20241120120336.1063979-3-niklas.soderlund+renesas@ragnatech.se
State New
Headers show
Series thermal: rcar_gen3: Improve reading calibration fuses | expand

Commit Message

Niklas Söderlund Nov. 20, 2024, 12:03 p.m. UTC 
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(-)

Comments

Geert Uytterhoeven Nov. 27, 2024, 12:37 p.m. UTC | #1 
Hi Niklas,

On Wed, Nov 20, 2024 at 1:04 PM Niklas Söderlund
<niklas.soderlund+renesas@ragnatech.se> wrote:
> 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>

Thanks for your patch!

Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

Still, some suggestions for improvement below...

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

Perhaps ptat[1-3], to match REG_GEN3_PTAT[1-3]?

> +                                         u32 thcode0, u32 thcode1, u32 thcode2,
> +                                         u32 mask)
> +{

>  static void rcar_gen3_thermal_read_fuses_gen3(struct rcar_gen3_thermal_priv *priv)
>  {

[...]

> +       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)
>  {

[...]

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

As both rcar_gen3_thermal_read_fuses_gen[34] became wrappers around
rcar_gen3_thermal_fetch_fuses(), what about parameterizing by data
instead of by code? I.e. replace the rcar_thermal_info.read_fuse()
function pointer by a pointer to a structure?

    struct rcar_gen3_thermal_fuse_info {
            u32 ptat[3];
            u32 thcode[3];
            u32 mask;
    };

Gr{oetje,eeting}s,

                        Geert
diff mbox series

Patch

diff --git a/drivers/thermal/renesas/rcar_gen3_thermal.c b/drivers/thermal/renesas/rcar_gen3_thermal.c
index 95b636f79e07..5f54e235da2f 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)