All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 0/4] thermal: imx: cleanup and new formula
@ 2017-11-30  9:17 Uwe Kleine-König
  2017-11-30  9:17 ` [PATCH v2 1/4] thermal: imx: Use better parameter names than "val" Uwe Kleine-König
                   ` (4 more replies)
  0 siblings, 5 replies; 6+ messages in thread
From: Uwe Kleine-König @ 2017-11-30  9:17 UTC (permalink / raw)
  To: Leonard Crestez, Shawn Guo, Zhang Rui, Eduardo Valentin; +Cc: kernel, linux-pm

Hello,

this is v2 of the series to (mainly) update the driver to use an updated
algorithm. It also contains a few cleanups that I noticed on the way.

The changes are documented in the individual patches and are the result
of feedback by Leonard Crestez.

Uwe Kleine-König (4):
  thermal: imx: Use better parameter names than "val"
  thermal: imx: improve comments describing algorithm for temp
    calculation
  thermal: imx: use consistent style to write temperatures
  thermal: imx: update to new formula according to NXP AN5215

 drivers/thermal/imx_thermal.c | 74 +++++++++++++++++++------------------------
 1 file changed, 33 insertions(+), 41 deletions(-)

-- 
2.15.0

^ permalink raw reply	[flat|nested] 6+ messages in thread
* [PATCH v2 1/4] thermal: imx: Use better parameter names than "val"
  2017-11-30  9:17 [PATCH v2 0/4] thermal: imx: cleanup and new formula Uwe Kleine-König
@ 2017-11-30  9:17 ` Uwe Kleine-König
  2017-11-30  9:17 ` [PATCH v2 2/4] thermal: imx: improve comments describing algorithm for temp calculation Uwe Kleine-König
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 6+ messages in thread
From: Uwe Kleine-König @ 2017-11-30  9:17 UTC (permalink / raw)
  To: Leonard Crestez, Shawn Guo, Zhang Rui, Eduardo Valentin; +Cc: kernel, linux-pm

The values passed to imx_init_calib() and imx_init_temp_grade() are
read from specific OCOTP values. Use their names (in lower case) as
parameter name instead of "val" to make the code easier to understand.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
No changes since (implicit) v1, sent with
Message-ID: 20171121200225.23316-2-u.kleine-koenig@pengutronix.de

Leonard Crestez had doubts because on i.MX7 the OTP registers are called
differently, but I ignored these as the driver up to now only supports
i.MX6.
---
 drivers/thermal/imx_thermal.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/thermal/imx_thermal.c b/drivers/thermal/imx_thermal.c
index e7d4ffc3de7f..21b8c4c4da3c 100644
--- a/drivers/thermal/imx_thermal.c
+++ b/drivers/thermal/imx_thermal.c
@@ -347,13 +347,13 @@ static struct thermal_zone_device_ops imx_tz_ops = {
 	.set_trip_temp = imx_set_trip_temp,
 };
 
-static int imx_init_calib(struct platform_device *pdev, u32 val)
+static int imx_init_calib(struct platform_device *pdev, u32 ocotp_ana1)
 {
 	struct imx_thermal_data *data = platform_get_drvdata(pdev);
 	int t1, n1;
 	u64 temp64;
 
-	if (val == 0 || val == ~0) {
+	if (ocotp_ana1 == 0 || ocotp_ana1 == ~0) {
 		dev_err(&pdev->dev, "invalid sensor calibration data\n");
 		return -EINVAL;
 	}
@@ -364,7 +364,7 @@ static int imx_init_calib(struct platform_device *pdev, u32 val)
 	 * Use universal formula now and only need sensor value @ 25C
 	 * slope = 0.4297157 - (0.0015976 * 25C fuse)
 	 */
-	n1 = val >> 20;
+	n1 = ocotp_ana1 >> 20;
 	t1 = 25; /* t1 always 25C */
 
 	/*
@@ -392,12 +392,12 @@ static int imx_init_calib(struct platform_device *pdev, u32 val)
 	return 0;
 }
 
-static void imx_init_temp_grade(struct platform_device *pdev, u32 val)
+static void imx_init_temp_grade(struct platform_device *pdev, u32 ocotp_mem0)
 {
 	struct imx_thermal_data *data = platform_get_drvdata(pdev);
 
 	/* The maximum die temp is specified by the Temperature Grade */
-	switch ((val >> 6) & 0x3) {
+	switch ((ocotp_mem0 >> 6) & 0x3) {
 	case 0: /* Commercial (0 to 95C) */
 		data->temp_grade = "Commercial";
 		data->temp_max = 95000;
-- 
2.15.0

^ permalink raw reply related	[flat|nested] 6+ messages in thread
* [PATCH v2 2/4] thermal: imx: improve comments describing algorithm for temp calculation
  2017-11-30  9:17 [PATCH v2 0/4] thermal: imx: cleanup and new formula Uwe Kleine-König
  2017-11-30  9:17 ` [PATCH v2 1/4] thermal: imx: Use better parameter names than "val" Uwe Kleine-König
@ 2017-11-30  9:17 ` Uwe Kleine-König
  2017-11-30  9:17 ` [PATCH v2 3/4] thermal: imx: use consistent style to write temperatures Uwe Kleine-König
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 6+ messages in thread
From: Uwe Kleine-König @ 2017-11-30  9:17 UTC (permalink / raw)
  To: Leonard Crestez, Shawn Guo, Zhang Rui, Eduardo Valentin; +Cc: kernel, linux-pm

The description of the implemented algorithm is hardly understandable
without having the right application note side-by-side to the code.

Fix this by using shorter and more intuitive variable names, describe
their meaning and transform a single formula instead of first talking about
slope and then about "milli_Tmeas".

There are no code changes.

Reviewed-by: Leonard Crestez <leonard.crestez@nxp.com>
Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
Changes since (implicit) v1, sent with
Message-ID: 20171121200225.23316-3-u.kleine-koenig@pengutronix.de

 - Add Reviewed-by: Leonard Crestez
---
 drivers/thermal/imx_thermal.c | 42 +++++++++++++++++++-----------------------
 1 file changed, 19 insertions(+), 23 deletions(-)

diff --git a/drivers/thermal/imx_thermal.c b/drivers/thermal/imx_thermal.c
index 21b8c4c4da3c..c08883dff2cb 100644
--- a/drivers/thermal/imx_thermal.c
+++ b/drivers/thermal/imx_thermal.c
@@ -359,32 +359,28 @@ static int imx_init_calib(struct platform_device *pdev, u32 ocotp_ana1)
 	}
 
 	/*
-	 * Sensor data layout:
-	 *   [31:20] - sensor value @ 25C
-	 * Use universal formula now and only need sensor value @ 25C
-	 * slope = 0.4297157 - (0.0015976 * 25C fuse)
+	 * The sensor is calibrated at 25 °C (aka T1) and the value measured
+	 * (aka N1) at this temperature is provided in bits [31:20] in the
+	 * i.MX's OCOTP value ANA1.
+	 * To find the actual temperature T, the following formula has to be used
+	 * when reading value n from the sensor:
+	 *
+	 * T = T1 + (N - N1) / (0.4297157 - 0.0015976 * N1) °C
+	 *   = [T1 - N1 / (0.4297157 - 0.0015976 * N1) °C] + N / (0.4297157 - 0.0015976 * N1) °C
+	 *   = [T1 + N1 / (0.0015976 * N1 - 0.4297157) °C] - N / (0.0015976 * N1 - 0.4297157) °C
+	 *   = c2 - c1 * N
+	 *
+	 * with
+	 *
+	 *   c1 = 1 / (0.0015976 * N1 - 0.4297157) °C
+	 *   c2 = T1 + N1 / (0.0015976 * N1 - 0.4297157) °C
+	 *      = T1 + N1 * C1
 	 */
 	n1 = ocotp_ana1 >> 20;
-	t1 = 25; /* t1 always 25C */
+	t1 = 25; /* °C */
 
-	/*
-	 * Derived from linear interpolation:
-	 * slope = 0.4297157 - (0.0015976 * 25C fuse)
-	 * slope = (FACTOR2 - FACTOR1 * n1) / FACTOR0
-	 * (Nmeas - n1) / (Tmeas - t1) = slope
-	 * We want to reduce this down to the minimum computation necessary
-	 * for each temperature read.  Also, we want Tmeas in millicelsius
-	 * and we don't want to lose precision from integer division. So...
-	 * Tmeas = (Nmeas - n1) / slope + t1
-	 * milli_Tmeas = 1000 * (Nmeas - n1) / slope + 1000 * t1
-	 * milli_Tmeas = -1000 * (n1 - Nmeas) / slope + 1000 * t1
-	 * Let constant c1 = (-1000 / slope)
-	 * milli_Tmeas = (n1 - Nmeas) * c1 + 1000 * t1
-	 * Let constant c2 = n1 *c1 + 1000 * t1
-	 * milli_Tmeas = c2 - Nmeas * c1
-	 */
-	temp64 = FACTOR0;
-	temp64 *= 1000;
+	temp64 = FACTOR0; /* 10^7 for FACTOR1 and FACTOR2 */
+	temp64 *= 1000; /* to get result in °mC */
 	do_div(temp64, FACTOR1 * n1 - FACTOR2);
 	data->c1 = temp64;
 	data->c2 = n1 * data->c1 + 1000 * t1;
-- 
2.15.0

^ permalink raw reply related	[flat|nested] 6+ messages in thread
* [PATCH v2 3/4] thermal: imx: use consistent style to write temperatures
  2017-11-30  9:17 [PATCH v2 0/4] thermal: imx: cleanup and new formula Uwe Kleine-König
  2017-11-30  9:17 ` [PATCH v2 1/4] thermal: imx: Use better parameter names than "val" Uwe Kleine-König
  2017-11-30  9:17 ` [PATCH v2 2/4] thermal: imx: improve comments describing algorithm for temp calculation Uwe Kleine-König
@ 2017-11-30  9:17 ` Uwe Kleine-König
  2017-11-30  9:17 ` [PATCH v2 4/4] thermal: imx: update to new formula according to NXP AN5215 Uwe Kleine-König
  2017-12-11 16:25 ` [PATCH v2 0/4] thermal: imx: cleanup and new formula Uwe Kleine-König
  4 siblings, 0 replies; 6+ messages in thread
From: Uwe Kleine-König @ 2017-11-30  9:17 UTC (permalink / raw)
  To: Leonard Crestez, Shawn Guo, Zhang Rui, Eduardo Valentin; +Cc: kernel, linux-pm

The previous commit already took care to use the right notation for
temperatures. Add correct units to all values representing temperatures in
the right notation for the rest of the file.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
Changes since (implicit) v1, sent with
Message-ID: 20171121200225.23316-4-u.kleine-koenig@pengutronix.de

 - s/changable/changeable/ noticed by Leonard Crestez
---
 drivers/thermal/imx_thermal.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/thermal/imx_thermal.c b/drivers/thermal/imx_thermal.c
index c08883dff2cb..41e15cf5ffd7 100644
--- a/drivers/thermal/imx_thermal.c
+++ b/drivers/thermal/imx_thermal.c
@@ -394,27 +394,27 @@ static void imx_init_temp_grade(struct platform_device *pdev, u32 ocotp_mem0)
 
 	/* The maximum die temp is specified by the Temperature Grade */
 	switch ((ocotp_mem0 >> 6) & 0x3) {
-	case 0: /* Commercial (0 to 95C) */
+	case 0: /* Commercial (0 to 95 °C) */
 		data->temp_grade = "Commercial";
 		data->temp_max = 95000;
 		break;
-	case 1: /* Extended Commercial (-20 to 105C) */
+	case 1: /* Extended Commercial (-20 °C to 105 °C) */
 		data->temp_grade = "Extended Commercial";
 		data->temp_max = 105000;
 		break;
-	case 2: /* Industrial (-40 to 105C) */
+	case 2: /* Industrial (-40 °C to 105 °C) */
 		data->temp_grade = "Industrial";
 		data->temp_max = 105000;
 		break;
-	case 3: /* Automotive (-40 to 125C) */
+	case 3: /* Automotive (-40 °C to 125 °C) */
 		data->temp_grade = "Automotive";
 		data->temp_max = 125000;
 		break;
 	}
 
 	/*
-	 * Set the critical trip point at 5C under max
-	 * Set the passive trip point at 10C under max (can change via sysfs)
+	 * Set the critical trip point at 5 °C under max
+	 * Set the passive trip point at 10 °C under max (changeable via sysfs)
 	 */
 	data->temp_critical = data->temp_max - (1000 * 5);
 	data->temp_passive = data->temp_max - (1000 * 10);
-- 
2.15.0

^ permalink raw reply related	[flat|nested] 6+ messages in thread
* [PATCH v2 4/4] thermal: imx: update to new formula according to NXP AN5215
  2017-11-30  9:17 [PATCH v2 0/4] thermal: imx: cleanup and new formula Uwe Kleine-König
                   ` (2 preceding siblings ...)
  2017-11-30  9:17 ` [PATCH v2 3/4] thermal: imx: use consistent style to write temperatures Uwe Kleine-König
@ 2017-11-30  9:17 ` Uwe Kleine-König
  2017-12-11 16:25 ` [PATCH v2 0/4] thermal: imx: cleanup and new formula Uwe Kleine-König
  4 siblings, 0 replies; 6+ messages in thread
From: Uwe Kleine-König @ 2017-11-30  9:17 UTC (permalink / raw)
  To: Leonard Crestez, Shawn Guo, Zhang Rui, Eduardo Valentin; +Cc: kernel, linux-pm

According to an application note from 03/2017 there is an updated formula to
calculate the temperature that better matches reality. This is implemented here.

While updating move the magic constants from cpp defines which are far above the
explaining formula to constants in the code just under the explaining comment.

Reviewed-by: Leonard Crestez <leonard.crestez@nxp.com>
Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
Changes since (implicit) v1, sent with
Message-ID: 20171121200225.23316-5-u.kleine-koenig@pengutronix.de

 - Add Reviewed-by: Leonard Crestez
---
 drivers/thermal/imx_thermal.c | 26 +++++++++++---------------
 1 file changed, 11 insertions(+), 15 deletions(-)

diff --git a/drivers/thermal/imx_thermal.c b/drivers/thermal/imx_thermal.c
index 41e15cf5ffd7..a67781b7a0b2 100644
--- a/drivers/thermal/imx_thermal.c
+++ b/drivers/thermal/imx_thermal.c
@@ -70,10 +70,6 @@ enum imx_thermal_trip {
 #define IMX_POLLING_DELAY		2000 /* millisecond */
 #define IMX_PASSIVE_DELAY		1000
 
-#define FACTOR0				10000000
-#define FACTOR1				15976
-#define FACTOR2				4297157
-
 #define TEMPMON_IMX6Q			1
 #define TEMPMON_IMX6SX			2
 
@@ -350,7 +346,7 @@ static struct thermal_zone_device_ops imx_tz_ops = {
 static int imx_init_calib(struct platform_device *pdev, u32 ocotp_ana1)
 {
 	struct imx_thermal_data *data = platform_get_drvdata(pdev);
-	int t1, n1;
+	int n1;
 	u64 temp64;
 
 	if (ocotp_ana1 == 0 || ocotp_ana1 == ~0) {
@@ -365,25 +361,25 @@ static int imx_init_calib(struct platform_device *pdev, u32 ocotp_ana1)
 	 * To find the actual temperature T, the following formula has to be used
 	 * when reading value n from the sensor:
 	 *
-	 * T = T1 + (N - N1) / (0.4297157 - 0.0015976 * N1) °C
-	 *   = [T1 - N1 / (0.4297157 - 0.0015976 * N1) °C] + N / (0.4297157 - 0.0015976 * N1) °C
-	 *   = [T1 + N1 / (0.0015976 * N1 - 0.4297157) °C] - N / (0.0015976 * N1 - 0.4297157) °C
+	 * T = T1 + (N - N1) / (0.4148468 - 0.0015423 * N1) °C + 3.580661 °C
+	 *   = [T1' - N1 / (0.4148468 - 0.0015423 * N1) °C] + N / (0.4148468 - 0.0015423 * N1) °C
+	 *   = [T1' + N1 / (0.0015423 * N1 - 0.4148468) °C] - N / (0.0015423 * N1 - 0.4148468) °C
 	 *   = c2 - c1 * N
 	 *
 	 * with
 	 *
-	 *   c1 = 1 / (0.0015976 * N1 - 0.4297157) °C
-	 *   c2 = T1 + N1 / (0.0015976 * N1 - 0.4297157) °C
-	 *      = T1 + N1 * C1
+	 *  T1' = 28.580661 °C
+	 *   c1 = 1 / (0.0015423 * N1 - 0.4297157) °C
+	 *   c2 = T1' + N1 / (0.0015423 * N1 - 0.4148468) °C
+	 *      = T1' + N1 * c1
 	 */
 	n1 = ocotp_ana1 >> 20;
-	t1 = 25; /* °C */
 
-	temp64 = FACTOR0; /* 10^7 for FACTOR1 and FACTOR2 */
+	temp64 = 10000000; /* use 10^7 as fixed point constant for values in formula */
 	temp64 *= 1000; /* to get result in °mC */
-	do_div(temp64, FACTOR1 * n1 - FACTOR2);
+	do_div(temp64, 15423 * n1 - 4148468);
 	data->c1 = temp64;
-	data->c2 = n1 * data->c1 + 1000 * t1;
+	data->c2 = n1 * data->c1 + 28581;
 
 	return 0;
 }
-- 
2.15.0

^ permalink raw reply related	[flat|nested] 6+ messages in thread
* Re: [PATCH v2 0/4] thermal: imx: cleanup and new formula
  2017-11-30  9:17 [PATCH v2 0/4] thermal: imx: cleanup and new formula Uwe Kleine-König
                   ` (3 preceding siblings ...)
  2017-11-30  9:17 ` [PATCH v2 4/4] thermal: imx: update to new formula according to NXP AN5215 Uwe Kleine-König
@ 2017-12-11 16:25 ` Uwe Kleine-König
  4 siblings, 0 replies; 6+ messages in thread
From: Uwe Kleine-König @ 2017-12-11 16:25 UTC (permalink / raw)
  To: Leonard Crestez, Shawn Guo, Zhang Rui, Eduardo Valentin; +Cc: kernel, linux-pm

Hello,

On Thu, Nov 30, 2017 at 10:17:34AM +0100, Uwe Kleine-König wrote:
> this is v2 of the series to (mainly) update the driver to use an updated
> algorithm. It also contains a few cleanups that I noticed on the way.
> 
> The changes are documented in the individual patches and are the result
> of feedback by Leonard Crestez.

I wonder how it goes forward with this series. Are there chances to get
this into 4.16-rc1?

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

^ permalink raw reply	[flat|nested] 6+ messages in thread
end of thread, other threads:[~2017-12-11 16:25 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-11-30  9:17 [PATCH v2 0/4] thermal: imx: cleanup and new formula Uwe Kleine-König
2017-11-30  9:17 ` [PATCH v2 1/4] thermal: imx: Use better parameter names than "val" Uwe Kleine-König
2017-11-30  9:17 ` [PATCH v2 2/4] thermal: imx: improve comments describing algorithm for temp calculation Uwe Kleine-König
2017-11-30  9:17 ` [PATCH v2 3/4] thermal: imx: use consistent style to write temperatures Uwe Kleine-König
2017-11-30  9:17 ` [PATCH v2 4/4] thermal: imx: update to new formula according to NXP AN5215 Uwe Kleine-König
2017-12-11 16:25 ` [PATCH v2 0/4] thermal: imx: cleanup and new formula Uwe Kleine-König

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.