All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/2] thermal: rcar_gen3_thermal: Read calibration from fuses
@ 2021-10-11 22:58 Niklas Söderlund
  2021-10-11 22:58 ` [PATCH 1/2] thermal: rcar_gen3_thermal: Store thcode and ptat in priv data Niklas Söderlund
                   ` (2 more replies)
  0 siblings, 3 replies; 7+ messages in thread
From: Niklas Söderlund @ 2021-10-11 22:58 UTC (permalink / raw)
  To: Kuninori Morimoto, Daniel Lezcano, Geert Uytterhoeven, linux-pm
  Cc: linux-renesas-soc, Niklas Söderlund

Hello Morimoto-san,

This series allows the Gen3 thermal driver to read its calibration
values (THCODE and PTAT) from the fuses, if set. If the values are not
set the driver fall-back to the previously used pseudo values from the
datasheet.

I don't have access to a system where the fuses are set but I have been 
told that you Morimoto-san do have access to one. Would it be possible 
for you to test this series for me?

The test case is a rather straight forward boot test. The patches are 
based on recent changes to the driver but everything is included in 
v5.15-rc4. If you have time to build and boot these patches on top of 
that and just check two things I would be very happy.

1. Check that the driver uses the fused values, this is showed by the 
   log message, dev_info(dev, "Using fused calibration values\n"); is 
   trigged when the driver probe.

2. Check the temperature readings in sysfs are reasonable, the values are 
   in millidegree Celsius. On my system that uses the coefficients from 
   the driver I have 41-42 degrees Celsius at the moment.

   # grep . /sys/class/thermal/thermal_zone*/temp
   /sys/class/thermal/thermal_zone0/temp:41000
   /sys/class/thermal/thermal_zone1/temp:42000
   /sys/class/thermal/thermal_zone2/temp:41000

If these two checks are OK for you I think this patch is ready to be 
consumed. Thanks for your help!

Patch 1/2 prepare for reading the values from fuses by moving the
storage of the values used during calculation from global const to
members of the private data structures. While patch 2/2 populates the
private members with data from the fuses if available.

Niklas Söderlund (2):
  thermal: rcar_gen3_thermal: Store thcode and ptat in priv data
  thermal: rcar_gen3_thermal: Read calibration from hardware

 drivers/thermal/rcar_gen3_thermal.c | 113 +++++++++++++++++++++-------
 1 file changed, 86 insertions(+), 27 deletions(-)

-- 
2.33.0


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH 1/2] thermal: rcar_gen3_thermal: Store thcode and ptat in priv data
  2021-10-11 22:58 [PATCH 0/2] thermal: rcar_gen3_thermal: Read calibration from fuses Niklas Söderlund
@ 2021-10-11 22:58 ` Niklas Söderlund
  2021-10-13 13:12   ` Geert Uytterhoeven
  2021-10-11 22:58 ` [PATCH 2/2] thermal: rcar_gen3_thermal: Read calibration from hardware Niklas Söderlund
  2021-10-12  3:11 ` [PATCH 0/2] thermal: rcar_gen3_thermal: Read calibration from fuses Kuninori Morimoto
  2 siblings, 1 reply; 7+ messages in thread
From: Niklas Söderlund @ 2021-10-11 22:58 UTC (permalink / raw)
  To: Kuninori Morimoto, Daniel Lezcano, Geert Uytterhoeven, linux-pm
  Cc: linux-renesas-soc, Niklas Söderlund

Prepare for reading the THCODE and PTAT values from hardware fuses by
storing the values used during calculations in the drivers private
data structures.

As the values are now stored directly in the private data structures
there is no need to keep track of the TSC channel id as its only usage
was to lookup the THCODE row, drop it.

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

diff --git a/drivers/thermal/rcar_gen3_thermal.c b/drivers/thermal/rcar_gen3_thermal.c
index 85228d308dd35b19..7d7e6ebe837a83af 100644
--- a/drivers/thermal/rcar_gen3_thermal.c
+++ b/drivers/thermal/rcar_gen3_thermal.c
@@ -62,15 +62,6 @@
 
 #define TSC_MAX_NUM	5
 
-/* default THCODE values if FUSEs are missing */
-static const int thcodes[TSC_MAX_NUM][3] = {
-	{ 3397, 2800, 2221 },
-	{ 3393, 2795, 2216 },
-	{ 3389, 2805, 2237 },
-	{ 3415, 2694, 2195 },
-	{ 3356, 2724, 2244 },
-};
-
 /* Structure for thermal temperature calculation */
 struct equation_coefs {
 	int a1;
@@ -84,13 +75,14 @@ struct rcar_gen3_thermal_tsc {
 	struct thermal_zone_device *zone;
 	struct equation_coefs coef;
 	int tj_t;
-	unsigned int id; /* thermal channel id */
+	int thcode[3];
 };
 
 struct rcar_gen3_thermal_priv {
 	struct rcar_gen3_thermal_tsc *tscs[TSC_MAX_NUM];
 	unsigned int num_tscs;
 	void (*thermal_init)(struct rcar_gen3_thermal_tsc *tsc);
+	int ptat[3];
 };
 
 static inline u32 rcar_gen3_thermal_read(struct rcar_gen3_thermal_tsc *tsc,
@@ -133,8 +125,8 @@ static inline void rcar_gen3_thermal_write(struct rcar_gen3_thermal_tsc *tsc,
 /* no idea where these constants come from */
 #define TJ_3 -41
 
-static void rcar_gen3_thermal_calc_coefs(struct rcar_gen3_thermal_tsc *tsc,
-					 int *ptat, const int *thcode,
+static void rcar_gen3_thermal_calc_coefs(struct rcar_gen3_thermal_priv *priv,
+					 struct rcar_gen3_thermal_tsc *tsc,
 					 int ths_tj_1)
 {
 	/* TODO: Find documentation and document constant calculation formula */
@@ -143,16 +135,16 @@ static void rcar_gen3_thermal_calc_coefs(struct rcar_gen3_thermal_tsc *tsc,
 	 * Division is not scaled in BSP and if scaled it might overflow
 	 * the dividend (4095 * 4095 << 14 > INT_MAX) so keep it unscaled
 	 */
-	tsc->tj_t = (FIXPT_INT((ptat[1] - ptat[2]) * (ths_tj_1 - TJ_3))
-		     / (ptat[0] - ptat[2])) + FIXPT_INT(TJ_3);
+	tsc->tj_t = (FIXPT_INT((priv->ptat[1] - priv->ptat[2]) * (ths_tj_1 - TJ_3))
+		     / (priv->ptat[0] - priv->ptat[2])) + FIXPT_INT(TJ_3);
 
-	tsc->coef.a1 = FIXPT_DIV(FIXPT_INT(thcode[1] - thcode[2]),
+	tsc->coef.a1 = FIXPT_DIV(FIXPT_INT(tsc->thcode[1] - tsc->thcode[2]),
 				 tsc->tj_t - FIXPT_INT(TJ_3));
-	tsc->coef.b1 = FIXPT_INT(thcode[2]) - tsc->coef.a1 * TJ_3;
+	tsc->coef.b1 = FIXPT_INT(tsc->thcode[2]) - tsc->coef.a1 * TJ_3;
 
-	tsc->coef.a2 = FIXPT_DIV(FIXPT_INT(thcode[1] - thcode[0]),
+	tsc->coef.a2 = FIXPT_DIV(FIXPT_INT(tsc->thcode[1] - tsc->thcode[0]),
 				 tsc->tj_t - FIXPT_INT(ths_tj_1));
-	tsc->coef.b2 = FIXPT_INT(thcode[0]) - tsc->coef.a2 * ths_tj_1;
+	tsc->coef.b2 = FIXPT_INT(tsc->thcode[0]) - tsc->coef.a2 * ths_tj_1;
 }
 
 static int rcar_gen3_thermal_round(int temp)
@@ -174,7 +166,7 @@ static int rcar_gen3_thermal_get_temp(void *devdata, int *temp)
 	/* Read register and convert to mili Celsius */
 	reg = rcar_gen3_thermal_read(tsc, REG_GEN3_TEMP) & CTEMP_MASK;
 
-	if (reg <= thcodes[tsc->id][1])
+	if (reg <= tsc->thcode[1])
 		val = FIXPT_DIV(FIXPT_INT(reg) - tsc->coef.b1,
 				tsc->coef.a1);
 	else
@@ -401,9 +393,15 @@ static int rcar_gen3_thermal_probe(struct platform_device *pdev)
 	unsigned int i;
 	int ret;
 
-	/* default values if FUSEs are missing */
+	/* Default THCODE values in case FUSEs are not set. */
 	/* TODO: Read values from hardware on supported platforms */
-	int ptat[3] = { 2631, 1509, 435 };
+	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)
@@ -413,6 +411,10 @@ 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))
@@ -439,7 +441,10 @@ static int rcar_gen3_thermal_probe(struct platform_device *pdev)
 			ret = PTR_ERR(tsc->base);
 			goto error_unregister;
 		}
-		tsc->id = i;
+
+		tsc->thcode[0] = thcodes[i][0];
+		tsc->thcode[1] = thcodes[i][1];
+		tsc->thcode[2] = thcodes[i][2];
 
 		priv->tscs[i] = tsc;
 
@@ -453,7 +458,7 @@ static int rcar_gen3_thermal_probe(struct platform_device *pdev)
 		tsc->zone = zone;
 
 		priv->thermal_init(tsc);
-		rcar_gen3_thermal_calc_coefs(tsc, ptat, thcodes[i], *ths_tj_1);
+		rcar_gen3_thermal_calc_coefs(priv, tsc, *ths_tj_1);
 
 		tsc->zone->tzp->no_hwmon = false;
 		ret = thermal_add_hwmon_sysfs(tsc->zone);
-- 
2.33.0


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH 2/2] thermal: rcar_gen3_thermal: Read calibration from hardware
  2021-10-11 22:58 [PATCH 0/2] thermal: rcar_gen3_thermal: Read calibration from fuses Niklas Söderlund
  2021-10-11 22:58 ` [PATCH 1/2] thermal: rcar_gen3_thermal: Store thcode and ptat in priv data Niklas Söderlund
@ 2021-10-11 22:58 ` Niklas Söderlund
  2021-10-13 13:27   ` Geert Uytterhoeven
  2021-10-12  3:11 ` [PATCH 0/2] thermal: rcar_gen3_thermal: Read calibration from fuses Kuninori Morimoto
  2 siblings, 1 reply; 7+ messages in thread
From: Niklas Söderlund @ 2021-10-11 22:58 UTC (permalink / raw)
  To: Kuninori Morimoto, Daniel Lezcano, Geert Uytterhoeven, linux-pm
  Cc: linux-renesas-soc, Niklas Söderlund

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

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;
-- 
2.33.0


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH 0/2] thermal: rcar_gen3_thermal: Read calibration from fuses
  2021-10-11 22:58 [PATCH 0/2] thermal: rcar_gen3_thermal: Read calibration from fuses Niklas Söderlund
  2021-10-11 22:58 ` [PATCH 1/2] thermal: rcar_gen3_thermal: Store thcode and ptat in priv data Niklas Söderlund
  2021-10-11 22:58 ` [PATCH 2/2] thermal: rcar_gen3_thermal: Read calibration from hardware Niklas Söderlund
@ 2021-10-12  3:11 ` Kuninori Morimoto
  2021-10-12  8:24   ` Niklas Söderlund
  2 siblings, 1 reply; 7+ messages in thread
From: Kuninori Morimoto @ 2021-10-12  3:11 UTC (permalink / raw)
  To: Niklas Söderlund
  Cc: Daniel Lezcano, Geert Uytterhoeven, linux-pm, linux-renesas-soc


Hi Niklas

Thank you for the patches.

> 1. Check that the driver uses the fused values, this is showed by the
>    log message, dev_info(dev, "Using fused calibration values\n"); is
>    trigged when the driver probe.
>
> 2. Check the temperature readings in sysfs are reasonable, the values are
>    in millidegree Celsius. On my system that uses the coefficients from
>    the driver I have 41-42 degrees Celsius at the moment.
>
>    # grep . /sys/class/thermal/thermal_zone*/temp
>    /sys/class/thermal/thermal_zone0/temp:41000
>    /sys/class/thermal/thermal_zone1/temp:42000
>    /sys/class/thermal/thermal_zone2/temp:41000

I checkout:ed v5.15-rc4, and apply your patches.
This is the result

--- log ----
	...
	[    1.516781] i2c-rcar e6510000.i2c: probed
	[    1.532108] i2c-rcar e66d8000.i2c: probed
=>	[    1.539314] rcar_gen3_thermal e6198000.thermal: Using fused calibration values
	[    1.551116] rcar_gen3_thermal e6198000.thermal: TSC0: Loaded 1 trip points
	[    1.562274] rcar_gen3_thermal e6198000.thermal: TSC1: Loaded 1 trip points
	[    1.573413] rcar_gen3_thermal e6198000.thermal: TSC2: Loaded 2 trip points
	[    1.595676] random: fast init done
	[    1.612993] NET: Registered PF_PACKET protocol family
	...
	# login
=>	# grep . /sys/class/thermal/thermal_zone*/temp 
	/sys/class/thermal/thermal_zone0/temp:37000
	/sys/class/thermal/thermal_zone1/temp:38500
	/sys/class/thermal/thermal_zone2/temp:38500

I'm happy if these are the expected.

Tested-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>

Thank you for your help !!

Best regards
---
Kuninori Morimoto

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 0/2] thermal: rcar_gen3_thermal: Read calibration from fuses
  2021-10-12  3:11 ` [PATCH 0/2] thermal: rcar_gen3_thermal: Read calibration from fuses Kuninori Morimoto
@ 2021-10-12  8:24   ` Niklas Söderlund
  0 siblings, 0 replies; 7+ messages in thread
From: Niklas Söderlund @ 2021-10-12  8:24 UTC (permalink / raw)
  To: Kuninori Morimoto
  Cc: Daniel Lezcano, Geert Uytterhoeven, linux-pm, linux-renesas-soc

Hello Morimoro-san,

Thanks for your test.

On 2021-10-12 12:11:01 +0900, Kuninori Morimoto wrote:
> 
> Hi Niklas
> 
> Thank you for the patches.
> 
> > 1. Check that the driver uses the fused values, this is showed by the
> >    log message, dev_info(dev, "Using fused calibration values\n"); is
> >    trigged when the driver probe.
> >
> > 2. Check the temperature readings in sysfs are reasonable, the values are
> >    in millidegree Celsius. On my system that uses the coefficients from
> >    the driver I have 41-42 degrees Celsius at the moment.
> >
> >    # grep . /sys/class/thermal/thermal_zone*/temp
> >    /sys/class/thermal/thermal_zone0/temp:41000
> >    /sys/class/thermal/thermal_zone1/temp:42000
> >    /sys/class/thermal/thermal_zone2/temp:41000
> 
> I checkout:ed v5.15-rc4, and apply your patches.
> This is the result
> 
> --- log ----
> 	...
> 	[    1.516781] i2c-rcar e6510000.i2c: probed
> 	[    1.532108] i2c-rcar e66d8000.i2c: probed
> =>	[    1.539314] rcar_gen3_thermal e6198000.thermal: Using fused calibration values
> 	[    1.551116] rcar_gen3_thermal e6198000.thermal: TSC0: Loaded 1 trip points
> 	[    1.562274] rcar_gen3_thermal e6198000.thermal: TSC1: Loaded 1 trip points
> 	[    1.573413] rcar_gen3_thermal e6198000.thermal: TSC2: Loaded 2 trip points
> 	[    1.595676] random: fast init done
> 	[    1.612993] NET: Registered PF_PACKET protocol family
> 	...
> 	# login
> =>	# grep . /sys/class/thermal/thermal_zone*/temp 
> 	/sys/class/thermal/thermal_zone0/temp:37000
> 	/sys/class/thermal/thermal_zone1/temp:38500
> 	/sys/class/thermal/thermal_zone2/temp:38500
> 
> I'm happy if these are the expected.
> 
> Tested-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>

These are reasonable temperature readings, unless you moved to an igloo 
to live with the penguins :-)

Thanks again for locating a board and taking the time to test this 
series much appreciated!

> 
> Thank you for your help !!
> 
> Best regards
> ---
> Kuninori Morimoto

-- 
Regards,
Niklas Söderlund

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 1/2] thermal: rcar_gen3_thermal: Store thcode and ptat in priv data
  2021-10-11 22:58 ` [PATCH 1/2] thermal: rcar_gen3_thermal: Store thcode and ptat in priv data Niklas Söderlund
@ 2021-10-13 13:12   ` Geert Uytterhoeven
  0 siblings, 0 replies; 7+ messages in thread
From: Geert Uytterhoeven @ 2021-10-13 13:12 UTC (permalink / raw)
  To: Niklas Söderlund
  Cc: Kuninori Morimoto, Daniel Lezcano, Linux PM list, Linux-Renesas

Hi Niklas,

On Tue, Oct 12, 2021 at 12:58 AM Niklas Söderlund
<niklas.soderlund+renesas@ragnatech.se> wrote:
> Prepare for reading the THCODE and PTAT values from hardware fuses by
> storing the values used during calculations in the drivers private
> data structures.
>
> As the values are now stored directly in the private data structures
> there is no need to keep track of the TSC channel id as its only usage
> was to lookup the THCODE row, drop it.
>
> 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>

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

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 2/2] thermal: rcar_gen3_thermal: Read calibration from hardware
  2021-10-11 22:58 ` [PATCH 2/2] thermal: rcar_gen3_thermal: Read calibration from hardware Niklas Söderlund
@ 2021-10-13 13:27   ` Geert Uytterhoeven
  0 siblings, 0 replies; 7+ messages in thread
From: Geert Uytterhoeven @ 2021-10-13 13:27 UTC (permalink / raw)
  To: Niklas Söderlund
  Cc: Kuninori Morimoto, Daniel Lezcano, Linux PM list, Linux-Renesas

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

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

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2021-10-13 13:27 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-10-11 22:58 [PATCH 0/2] thermal: rcar_gen3_thermal: Read calibration from fuses Niklas Söderlund
2021-10-11 22:58 ` [PATCH 1/2] thermal: rcar_gen3_thermal: Store thcode and ptat in priv data Niklas Söderlund
2021-10-13 13:12   ` Geert Uytterhoeven
2021-10-11 22:58 ` [PATCH 2/2] thermal: rcar_gen3_thermal: Read calibration from hardware Niklas Söderlund
2021-10-13 13:27   ` Geert Uytterhoeven
2021-10-12  3:11 ` [PATCH 0/2] thermal: rcar_gen3_thermal: Read calibration from fuses Kuninori Morimoto
2021-10-12  8:24   ` Niklas Söderlund

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.