diff mbox series

[2/2] thermal: rcar_gen3_thermal: Read calibration from hardware

Message ID 20211011225802.11497-3-niklas.soderlund+renesas@ragnatech.se (mailing list archive)
State Superseded
Delegated to: Geert Uytterhoeven
Headers show
Series thermal: rcar_gen3_thermal: Read calibration from fuses | expand

Commit Message

Niklas Söderlund Oct. 11, 2021, 10:58 p.m. UTC
In production hardware the calibration values used to convert register
values to temperatures can be read from hardware. While pre-production
hardware still depends on pseudo values hard-coded in the driver.

Add support for reading out calibration values from hardware if it's
fused. The presence of fused calibration is indicated in the THSCP
register.

Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
---
* Changes since RFT
- Keep thcodes array static.
---
 drivers/thermal/rcar_gen3_thermal.c | 94 +++++++++++++++++++++++------
 1 file changed, 74 insertions(+), 20 deletions(-)

Comments

Geert Uytterhoeven Oct. 13, 2021, 1:27 p.m. UTC | #1
Hi Niklas,

On Tue, Oct 12, 2021 at 12:58 AM Niklas Söderlund
<niklas.soderlund+renesas@ragnatech.se> wrote:
> In production hardware the calibration values used to convert register
> values to temperatures can be read from hardware. While pre-production
> hardware still depends on pseudo values hard-coded in the driver.
>
> Add support for reading out calibration values from hardware if it's
> fused. The presence of fused calibration is indicated in the THSCP
> register.
>
> Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
> ---
> * Changes since RFT
> - Keep thcodes array static.

Thanks for the update!

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

A few minor bike sheddings^W^Wnits below...

> --- a/drivers/thermal/rcar_gen3_thermal.c
> +++ b/drivers/thermal/rcar_gen3_thermal.c

> @@ -245,6 +252,64 @@ static const struct soc_device_attribute r8a7795es1[] = {
>         { /* sentinel */ }
>  };
>
> +static bool rcar_gen3_thermal_update_fuses(struct rcar_gen3_thermal_priv *priv)

This doesn't sound like a good name to me, as the function does not
update the fuses, but reads their values.

> +{
> +       unsigned int i;
> +       u32 thscp;
> +
> +       /* Default THCODE values in case FUSEs are not set. */
> +       static const int thcodes[TSC_MAX_NUM][3] = {
> +               { 3397, 2800, 2221 },
> +               { 3393, 2795, 2216 },
> +               { 3389, 2805, 2237 },
> +               { 3415, 2694, 2195 },
> +               { 3356, 2724, 2244 },
> +       };

Given this is used only inside the if statement below, perhaps it
should be moved there?

> +
> +       /* If fuses are not set, fallback to pseudo values. */
> +       thscp = rcar_gen3_thermal_read(priv->tscs[0], REG_GEN3_THSCP);
> +       if ((thscp & THSCP_COR_PARA_VLD) != THSCP_COR_PARA_VLD) {
> +               priv->ptat[0] = 2631;
> +               priv->ptat[1] = 1509;
> +               priv->ptat[2] = 435;
> +
> +               for (i = 0; i < priv->num_tscs; i++) {
> +                       struct rcar_gen3_thermal_tsc *tsc = priv->tscs[i];
> +
> +                       tsc->thcode[0] = thcodes[i][0];
> +                       tsc->thcode[1] = thcodes[i][1];
> +                       tsc->thcode[2] = thcodes[i][2];
> +               }
> +
> +               return false;
> +       }
> +
> +       /*
> +        * 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;
> +       }
> +
> +       return true;
> +}
> +
>  static void rcar_gen3_thermal_init_r8a7795es1(struct rcar_gen3_thermal_tsc *tsc)
>  {
>         rcar_gen3_thermal_write(tsc, REG_GEN3_CTSR,  CTSR_THBGR);

> @@ -442,11 +493,16 @@ static int rcar_gen3_thermal_probe(struct platform_device *pdev)
>                         goto error_unregister;
>                 }
>
> -               tsc->thcode[0] = thcodes[i][0];
> -               tsc->thcode[1] = thcodes[i][1];
> -               tsc->thcode[2] = thcodes[i][2];
> -
>                 priv->tscs[i] = tsc;
> +       }
> +
> +       priv->num_tscs = i;
> +
> +       if (rcar_gen3_thermal_update_fuses(priv))
> +               dev_info(dev, "Using fused calibration values\n");

Despite our lack of test hardware having programmed fuses, using the
values from the fuses should be the normal situation, right?
So perhaps print a message when falling back to the default values
instead?

> +
> +       for (i = 0; i < priv->num_tscs; i++) {
> +               struct rcar_gen3_thermal_tsc *tsc = priv->tscs[i];
>
>                 zone = devm_thermal_zone_of_sensor_register(dev, i, tsc,
>                                                             &rcar_gen3_tz_of_ops);

Gr{oetje,eeting}s,

                        Geert
diff mbox series

Patch

diff --git a/drivers/thermal/rcar_gen3_thermal.c b/drivers/thermal/rcar_gen3_thermal.c
index 7d7e6ebe837a83af..897b625e1b498f05 100644
--- a/drivers/thermal/rcar_gen3_thermal.c
+++ b/drivers/thermal/rcar_gen3_thermal.c
@@ -34,6 +34,10 @@ 
 #define REG_GEN3_THCODE1	0x50
 #define REG_GEN3_THCODE2	0x54
 #define REG_GEN3_THCODE3	0x58
+#define REG_GEN3_PTAT1		0x5c
+#define REG_GEN3_PTAT2		0x60
+#define REG_GEN3_PTAT3		0x64
+#define REG_GEN3_THSCP		0x68
 
 /* IRQ{STR,MSK,EN} bits */
 #define IRQ_TEMP1		BIT(0)
@@ -55,6 +59,9 @@ 
 #define THCTR_PONM	BIT(6)
 #define THCTR_THSST	BIT(0)
 
+/* THSCP bits */
+#define THSCP_COR_PARA_VLD	(BIT(15) | BIT(14))
+
 #define CTEMP_MASK	0xFFF
 
 #define MCELSIUS(temp)	((temp) * 1000)
@@ -245,6 +252,64 @@  static const struct soc_device_attribute r8a7795es1[] = {
 	{ /* sentinel */ }
 };
 
+static bool rcar_gen3_thermal_update_fuses(struct rcar_gen3_thermal_priv *priv)
+{
+	unsigned int i;
+	u32 thscp;
+
+	/* Default THCODE values in case FUSEs are not set. */
+	static const int thcodes[TSC_MAX_NUM][3] = {
+		{ 3397, 2800, 2221 },
+		{ 3393, 2795, 2216 },
+		{ 3389, 2805, 2237 },
+		{ 3415, 2694, 2195 },
+		{ 3356, 2724, 2244 },
+	};
+
+	/* If fuses are not set, fallback to pseudo values. */
+	thscp = rcar_gen3_thermal_read(priv->tscs[0], REG_GEN3_THSCP);
+	if ((thscp & THSCP_COR_PARA_VLD) != THSCP_COR_PARA_VLD) {
+		priv->ptat[0] = 2631;
+		priv->ptat[1] = 1509;
+		priv->ptat[2] = 435;
+
+		for (i = 0; i < priv->num_tscs; i++) {
+			struct rcar_gen3_thermal_tsc *tsc = priv->tscs[i];
+
+			tsc->thcode[0] = thcodes[i][0];
+			tsc->thcode[1] = thcodes[i][1];
+			tsc->thcode[2] = thcodes[i][2];
+		}
+
+		return false;
+	}
+
+	/*
+	 * 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;
+	}
+
+	return true;
+}
+
 static void rcar_gen3_thermal_init_r8a7795es1(struct rcar_gen3_thermal_tsc *tsc)
 {
 	rcar_gen3_thermal_write(tsc, REG_GEN3_CTSR,  CTSR_THBGR);
@@ -393,16 +458,6 @@  static int rcar_gen3_thermal_probe(struct platform_device *pdev)
 	unsigned int i;
 	int ret;
 
-	/* Default THCODE values in case FUSEs are not set. */
-	/* TODO: Read values from hardware on supported platforms */
-	static const int thcodes[TSC_MAX_NUM][3] = {
-		{ 3397, 2800, 2221 },
-		{ 3393, 2795, 2216 },
-		{ 3389, 2805, 2237 },
-		{ 3415, 2694, 2195 },
-		{ 3356, 2724, 2244 },
-	};
-
 	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
 	if (!priv)
 		return -ENOMEM;
@@ -411,10 +466,6 @@  static int rcar_gen3_thermal_probe(struct platform_device *pdev)
 	if (soc_device_match(r8a7795es1))
 		priv->thermal_init = rcar_gen3_thermal_init_r8a7795es1;
 
-	priv->ptat[0] = 2631;
-	priv->ptat[1] = 1509;
-	priv->ptat[2] = 435;
-
 	platform_set_drvdata(pdev, priv);
 
 	if (rcar_gen3_thermal_request_irqs(priv, pdev))
@@ -442,11 +493,16 @@  static int rcar_gen3_thermal_probe(struct platform_device *pdev)
 			goto error_unregister;
 		}
 
-		tsc->thcode[0] = thcodes[i][0];
-		tsc->thcode[1] = thcodes[i][1];
-		tsc->thcode[2] = thcodes[i][2];
-
 		priv->tscs[i] = tsc;
+	}
+
+	priv->num_tscs = i;
+
+	if (rcar_gen3_thermal_update_fuses(priv))
+		dev_info(dev, "Using fused calibration values\n");
+
+	for (i = 0; i < priv->num_tscs; i++) {
+		struct rcar_gen3_thermal_tsc *tsc = priv->tscs[i];
 
 		zone = devm_thermal_zone_of_sensor_register(dev, i, tsc,
 							    &rcar_gen3_tz_of_ops);
@@ -476,8 +532,6 @@  static int rcar_gen3_thermal_probe(struct platform_device *pdev)
 		dev_info(dev, "TSC%u: Loaded %d trip points\n", i, ret);
 	}
 
-	priv->num_tscs = i;
-
 	if (!priv->num_tscs) {
 		ret = -ENODEV;
 		goto error_unregister;