All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] imx: thermal: use CPU temperature grade info for thresholds
@ 2015-04-30 16:02 Tim Harvey
  2015-05-11 12:22 ` Shawn Guo
                   ` (4 more replies)
  0 siblings, 5 replies; 19+ messages in thread
From: Tim Harvey @ 2015-04-30 16:02 UTC (permalink / raw)
  To: linux-arm-kernel

The IMX6Q/IMX6DL SoC's have a 2-bit temperature grade stored in OTP. Instead
of assuming 85C for passive cooling threshold and 105C for critical use
the thermal grade for these configurations. For IMX6SX which does not have
a temperature grade in OTP (according to the ref manual) assume 105C critical.
We will default the user-settable passive threshold to crit - 20C.

Cc: Anson Huang <b20788@freescale.com>
Cc: Fabio Estevam <fabio.estevam@freescale.com>
Cc: Shawn Guo <shawn.guo@linaro.org>
Signed-off-by: Tim Harvey <tharvey@gateworks.com>
---
 drivers/thermal/imx_thermal.c | 64 +++++++++++++++++++++++++++++++++----------
 1 file changed, 49 insertions(+), 15 deletions(-)

diff --git a/drivers/thermal/imx_thermal.c b/drivers/thermal/imx_thermal.c
index 2ccbc07..c6a4eed 100644
--- a/drivers/thermal/imx_thermal.c
+++ b/drivers/thermal/imx_thermal.c
@@ -55,6 +55,7 @@
 #define TEMPSENSE2_PANIC_VALUE_SHIFT	16
 #define TEMPSENSE2_PANIC_VALUE_MASK	0xfff0000
 
+#define OCOTP_MEM0			0x0480
 #define OCOTP_ANA1			0x04e0
 
 /* The driver supports 1 passive trip point and 1 critical trip point */
@@ -64,12 +65,6 @@ enum imx_thermal_trip {
 	IMX_TRIP_NUM,
 };
 
-/*
- * It defines the temperature in millicelsius for passive trip point
- * that will trigger cooling action when crossed.
- */
-#define IMX_TEMP_PASSIVE		85000
-
 #define IMX_POLLING_DELAY		2000 /* millisecond */
 #define IMX_PASSIVE_DELAY		1000
 
@@ -106,6 +101,7 @@ struct imx_thermal_data {
 	int irq;
 	struct clk *thermal_clk;
 	const struct thermal_soc_data *socdata;
+	const char *temp_grade;
 };
 
 static void imx_set_panic_temp(struct imx_thermal_data *data,
@@ -289,7 +285,8 @@ static int imx_set_trip_temp(struct thermal_zone_device *tz, int trip,
 	if (trip == IMX_TRIP_CRITICAL)
 		return -EPERM;
 
-	if (temp > IMX_TEMP_PASSIVE)
+	/* do not allow passive to be set higher than critical - 20C */
+	if (temp > (data->temp_critical - (1000 * 20)))
 		return -EINVAL;
 
 	data->temp_passive = temp;
@@ -404,17 +401,50 @@ static int imx_get_sensor_data(struct platform_device *pdev)
 	data->c1 = temp64;
 	data->c2 = n1 * data->c1 + 1000 * t1;
 
-	/*
-	 * Set the default passive cooling trip point,
-	 * can be changed from userspace.
-	 */
-	data->temp_passive = IMX_TEMP_PASSIVE;
+	/* Assume Extended Commercial temperature grade limits (105C max) */
+	data->temp_grade = "unknown";
+	data->temp_critical = 105000;
+
+	/* For IMX6Q use OTP for thermal grade */
+	if (data->socdata->version == TEMPMON_IMX6Q) {
+		ret = regmap_read(map, OCOTP_MEM0, &val);
+		if (ret) {
+			dev_err(&pdev->dev, "failed to read temp grade: %d\n",
+				ret);
+			return ret;
+		}
+
+		if (val == 0 || val == ~0) {
+			dev_err(&pdev->dev, "invalid temp grade data\n");
+			return -EINVAL;
+		}
+
+		/* The maximum die temp is specified by the Temperature Grade */
+		switch ((val >> 6) & 0x3) {
+		case 0: /* Commercial (0 to 95C) */
+			data->temp_grade = "Commercial";
+			data->temp_critical = 95000;
+			break;
+		case 1: /* Extended Commercial (-20 to 105C) */
+			data->temp_grade = "Extended Commercial";
+			data->temp_critical = 105000;
+			break;
+		case 2: /* Industrial (-40 to 105C) */
+			data->temp_grade = "Industrial";
+			data->temp_critical = 105000;
+			break;
+		case 3: /* Automotive (-40 to 125C) */
+			data->temp_grade = "Automotive";
+			data->temp_critical = 125000;
+			break;
+		}
+	}
 
 	/*
-	 * The maximum die temperature set to 20 C higher than
-	 * IMX_TEMP_PASSIVE.
+	 * Set the default passive cooling trip point to critical - 20C
+	 * (can be changed from userspace)
 	 */
-	data->temp_critical = 1000 * 20 + data->temp_passive;
+	data->temp_passive = data->temp_critical - (1000 * 20);
 
 	return 0;
 }
@@ -559,6 +589,10 @@ static int imx_thermal_probe(struct platform_device *pdev)
 		return ret;
 	}
 
+	dev_info(&pdev->dev, "%s CPU temperature grade - "
+		 "thresholds: crit:%ldC passive:%ldC\n", data->temp_grade,
+		 data->temp_critical / 1000, data->temp_passive / 1000);
+
 	/* Enable measurements at ~ 10 Hz */
 	regmap_write(map, TEMPSENSE1 + REG_CLR, TEMPSENSE1_MEASURE_FREQ);
 	measure_freq = DIV_ROUND_UP(32768, 10); /* 10 Hz */
-- 
1.9.1

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

* [PATCH] imx: thermal: use CPU temperature grade info for thresholds
  2015-04-30 16:02 [PATCH] imx: thermal: use CPU temperature grade info for thresholds Tim Harvey
@ 2015-05-11 12:22 ` Shawn Guo
  2015-05-11 12:26 ` Fabio Estevam
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 19+ messages in thread
From: Shawn Guo @ 2015-05-11 12:22 UTC (permalink / raw)
  To: linux-arm-kernel

On Thu, Apr 30, 2015 at 09:02:37AM -0700, Tim Harvey wrote:
> The IMX6Q/IMX6DL SoC's have a 2-bit temperature grade stored in OTP. Instead
> of assuming 85C for passive cooling threshold and 105C for critical use
> the thermal grade for these configurations. For IMX6SX which does not have
> a temperature grade in OTP (according to the ref manual) assume 105C critical.
> We will default the user-settable passive threshold to crit - 20C.
> 
> Cc: Anson Huang <b20788@freescale.com>
> Cc: Fabio Estevam <fabio.estevam@freescale.com>
> Cc: Shawn Guo <shawn.guo@linaro.org>
> Signed-off-by: Tim Harvey <tharvey@gateworks.com>

I'm fine with the patch.  But it'd be good to have Anson look at it as
well.

Acked-by: Shawn Guo <shawn.guo@linaro.org>

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

* [PATCH] imx: thermal: use CPU temperature grade info for thresholds
  2015-04-30 16:02 [PATCH] imx: thermal: use CPU temperature grade info for thresholds Tim Harvey
  2015-05-11 12:22 ` Shawn Guo
@ 2015-05-11 12:26 ` Fabio Estevam
  2015-05-11 13:31   ` Tim Harvey
  2015-05-20 22:08 ` Tim Harvey
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 19+ messages in thread
From: Fabio Estevam @ 2015-05-11 12:26 UTC (permalink / raw)
  To: linux-arm-kernel

On Thu, Apr 30, 2015 at 1:02 PM, Tim Harvey <tharvey@gateworks.com> wrote:

> +               /* The maximum die temp is specified by the Temperature Grade */
> +               switch ((val >> 6) & 0x3) {
> +               case 0: /* Commercial (0 to 95C) */
> +                       data->temp_grade = "Commercial";
> +                       data->temp_critical = 95000;
> +                       break;
> +               case 1: /* Extended Commercial (-20 to 105C) */
> +                       data->temp_grade = "Extended Commercial";
> +                       data->temp_critical = 105000;
> +                       break;

Looks good, only a minor suggestion for consistency: could you please
change "Commercial" to "Consumer"?

This is the term that is referenced in the datasheet, so better keep
it consistent.

Thanks

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

* [PATCH] imx: thermal: use CPU temperature grade info for thresholds
  2015-05-11 12:26 ` Fabio Estevam
@ 2015-05-11 13:31   ` Tim Harvey
  2015-05-12  1:32     ` Eduardo Valentin
  0 siblings, 1 reply; 19+ messages in thread
From: Tim Harvey @ 2015-05-11 13:31 UTC (permalink / raw)
  To: linux-arm-kernel

On Mon, May 11, 2015 at 5:26 AM, Fabio Estevam <festevam@gmail.com> wrote:
> On Thu, Apr 30, 2015 at 1:02 PM, Tim Harvey <tharvey@gateworks.com> wrote:
>
>> +               /* The maximum die temp is specified by the Temperature Grade */
>> +               switch ((val >> 6) & 0x3) {
>> +               case 0: /* Commercial (0 to 95C) */
>> +                       data->temp_grade = "Commercial";
>> +                       data->temp_critical = 95000;
>> +                       break;
>> +               case 1: /* Extended Commercial (-20 to 105C) */
>> +                       data->temp_grade = "Extended Commercial";
>> +                       data->temp_critical = 105000;
>> +                       break;
>
> Looks good, only a minor suggestion for consistency: could you please
> change "Commercial" to "Consumer"?
>
> This is the term that is referenced in the datasheet, so better keep
> it consistent.
>
> Thanks

Fabio,

Actually this seems to be very inconsistent from Freescale and the
register uses the word 'commercial' - I think you are referring to the
part identifier which is part of the ordering information. The Part
Number Nomenclature in the 'datasheets' use the word 'Commercial' in
the temperature grade field and the word 'Consumer' in some (but not
all) of the Part differentiator field. The register description uses
the word Commercial. I believe this is more of a marketing term than
anything else so I don't really care what word is used. I've heard the
word commercial used in terms of temperature grades and not consumer
personally.

I should note that the actual OTP fuse I'm looking at is only
described in the IMX6DQRM (Reference manual for the DUAL and QUAD) and
is shown as 'reserved' in the IMX6SDLRM (SOLO and DUALLITE Reference
Manual) and the 'IMX6SXRM' for the SX. I have been able to show
expirimentaly that the speed grade and temperature grade OTP's exist
in the IMX6SDL (even though they are marked as 'reserved' in the RM)
but I do not have an IMX6SX to see if the same is true. I have a query
out to our FAE for clarification on the OTP registers.

Tim

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

* [PATCH] imx: thermal: use CPU temperature grade info for thresholds
  2015-05-12  1:32     ` Eduardo Valentin
@ 2015-05-11 18:55       ` Fabio Estevam
  0 siblings, 0 replies; 19+ messages in thread
From: Fabio Estevam @ 2015-05-11 18:55 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Eduardo,

On Mon, May 11, 2015 at 10:32 PM, Eduardo Valentin <edubezval@gmail.com> wrote:

> Fabio, any particular preference ? Is the patch good to with the current
> nomenclature?

After Tim's explanation I agree with the current naming.

Regards,

Fabio Estevam

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

* [PATCH] imx: thermal: use CPU temperature grade info for thresholds
  2015-05-11 13:31   ` Tim Harvey
@ 2015-05-12  1:32     ` Eduardo Valentin
  2015-05-11 18:55       ` Fabio Estevam
  0 siblings, 1 reply; 19+ messages in thread
From: Eduardo Valentin @ 2015-05-12  1:32 UTC (permalink / raw)
  To: linux-arm-kernel

On Mon, May 11, 2015 at 06:31:55AM -0700, Tim Harvey wrote:
> On Mon, May 11, 2015 at 5:26 AM, Fabio Estevam <festevam@gmail.com> wrote:
> > On Thu, Apr 30, 2015 at 1:02 PM, Tim Harvey <tharvey@gateworks.com> wrote:
> >
> >> +               /* The maximum die temp is specified by the Temperature Grade */
> >> +               switch ((val >> 6) & 0x3) {
> >> +               case 0: /* Commercial (0 to 95C) */
> >> +                       data->temp_grade = "Commercial";
> >> +                       data->temp_critical = 95000;
> >> +                       break;
> >> +               case 1: /* Extended Commercial (-20 to 105C) */
> >> +                       data->temp_grade = "Extended Commercial";
> >> +                       data->temp_critical = 105000;
> >> +                       break;
> >
> > Looks good, only a minor suggestion for consistency: could you please
> > change "Commercial" to "Consumer"?
> >
> > This is the term that is referenced in the datasheet, so better keep
> > it consistent.
> >
> > Thanks
> 
> Fabio,
> 
> Actually this seems to be very inconsistent from Freescale and the
> register uses the word 'commercial' - I think you are referring to the
> part identifier which is part of the ordering information. The Part
> Number Nomenclature in the 'datasheets' use the word 'Commercial' in
> the temperature grade field and the word 'Consumer' in some (but not
> all) of the Part differentiator field. The register description uses
> the word Commercial. I believe this is more of a marketing term than
> anything else so I don't really care what word is used. I've heard the
> word commercial used in terms of temperature grades and not consumer
> personally.
> 
> I should note that the actual OTP fuse I'm looking at is only
> described in the IMX6DQRM (Reference manual for the DUAL and QUAD) and
> is shown as 'reserved' in the IMX6SDLRM (SOLO and DUALLITE Reference
> Manual) and the 'IMX6SXRM' for the SX. I have been able to show
> expirimentaly that the speed grade and temperature grade OTP's exist
> in the IMX6SDL (even though they are marked as 'reserved' in the RM)
> but I do not have an IMX6SX to see if the same is true. I have a query
> out to our FAE for clarification on the OTP registers.


Fabio, any particular preference ? Is the patch good to with the current
nomenclature?

> 
> Tim
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 490 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20150511/fba9f6d7/attachment.sig>

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

* [PATCH] imx: thermal: use CPU temperature grade info for thresholds
  2015-04-30 16:02 [PATCH] imx: thermal: use CPU temperature grade info for thresholds Tim Harvey
  2015-05-11 12:22 ` Shawn Guo
  2015-05-11 12:26 ` Fabio Estevam
@ 2015-05-20 22:08 ` Tim Harvey
  2015-05-21 23:45 ` [PATCH v2] " Tim Harvey
  2015-07-30  7:19 ` [PATCH] " Jon Nettleton
  4 siblings, 0 replies; 19+ messages in thread
From: Tim Harvey @ 2015-05-20 22:08 UTC (permalink / raw)
  To: linux-arm-kernel

On Thu, Apr 30, 2015 at 9:02 AM, Tim Harvey <tharvey@gateworks.com> wrote:
> The IMX6Q/IMX6DL SoC's have a 2-bit temperature grade stored in OTP. Instead
> of assuming 85C for passive cooling threshold and 105C for critical use
> the thermal grade for these configurations. For IMX6SX which does not have
> a temperature grade in OTP (according to the ref manual) assume 105C critical.
> We will default the user-settable passive threshold to crit - 20C.
>
> Cc: Anson Huang <b20788@freescale.com>
> Cc: Fabio Estevam <fabio.estevam@freescale.com>
> Cc: Shawn Guo <shawn.guo@linaro.org>
> Signed-off-by: Tim Harvey <tharvey@gateworks.com>
> ---
>  drivers/thermal/imx_thermal.c | 64 +++++++++++++++++++++++++++++++++----------
>  1 file changed, 49 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/thermal/imx_thermal.c b/drivers/thermal/imx_thermal.c
> index 2ccbc07..c6a4eed 100644
> --- a/drivers/thermal/imx_thermal.c
> +++ b/drivers/thermal/imx_thermal.c
> @@ -55,6 +55,7 @@
>  #define TEMPSENSE2_PANIC_VALUE_SHIFT   16
>  #define TEMPSENSE2_PANIC_VALUE_MASK    0xfff0000
>
> +#define OCOTP_MEM0                     0x0480
>  #define OCOTP_ANA1                     0x04e0
>
>  /* The driver supports 1 passive trip point and 1 critical trip point */
> @@ -64,12 +65,6 @@ enum imx_thermal_trip {
>         IMX_TRIP_NUM,
>  };
>
> -/*
> - * It defines the temperature in millicelsius for passive trip point
> - * that will trigger cooling action when crossed.
> - */
> -#define IMX_TEMP_PASSIVE               85000
> -
>  #define IMX_POLLING_DELAY              2000 /* millisecond */
>  #define IMX_PASSIVE_DELAY              1000
>
> @@ -106,6 +101,7 @@ struct imx_thermal_data {
>         int irq;
>         struct clk *thermal_clk;
>         const struct thermal_soc_data *socdata;
> +       const char *temp_grade;
>  };
>
>  static void imx_set_panic_temp(struct imx_thermal_data *data,
> @@ -289,7 +285,8 @@ static int imx_set_trip_temp(struct thermal_zone_device *tz, int trip,
>         if (trip == IMX_TRIP_CRITICAL)
>                 return -EPERM;
>
> -       if (temp > IMX_TEMP_PASSIVE)
> +       /* do not allow passive to be set higher than critical - 20C */
> +       if (temp > (data->temp_critical - (1000 * 20)))
>                 return -EINVAL;
>
>         data->temp_passive = temp;
> @@ -404,17 +401,50 @@ static int imx_get_sensor_data(struct platform_device *pdev)
>         data->c1 = temp64;
>         data->c2 = n1 * data->c1 + 1000 * t1;
>
> -       /*
> -        * Set the default passive cooling trip point,
> -        * can be changed from userspace.
> -        */
> -       data->temp_passive = IMX_TEMP_PASSIVE;
> +       /* Assume Extended Commercial temperature grade limits (105C max) */
> +       data->temp_grade = "unknown";
> +       data->temp_critical = 105000;
> +
> +       /* For IMX6Q use OTP for thermal grade */
> +       if (data->socdata->version == TEMPMON_IMX6Q) {
> +               ret = regmap_read(map, OCOTP_MEM0, &val);
> +               if (ret) {
> +                       dev_err(&pdev->dev, "failed to read temp grade: %d\n",
> +                               ret);
> +                       return ret;
> +               }
> +
> +               if (val == 0 || val == ~0) {
> +                       dev_err(&pdev->dev, "invalid temp grade data\n");
> +                       return -EINVAL;
> +               }
> +
> +               /* The maximum die temp is specified by the Temperature Grade */
> +               switch ((val >> 6) & 0x3) {
> +               case 0: /* Commercial (0 to 95C) */
> +                       data->temp_grade = "Commercial";
> +                       data->temp_critical = 95000;
> +                       break;
> +               case 1: /* Extended Commercial (-20 to 105C) */
> +                       data->temp_grade = "Extended Commercial";
> +                       data->temp_critical = 105000;
> +                       break;
> +               case 2: /* Industrial (-40 to 105C) */
> +                       data->temp_grade = "Industrial";
> +                       data->temp_critical = 105000;
> +                       break;
> +               case 3: /* Automotive (-40 to 125C) */
> +                       data->temp_grade = "Automotive";
> +                       data->temp_critical = 125000;
> +                       break;
> +               }
> +       }
>
>         /*
> -        * The maximum die temperature set to 20 C higher than
> -        * IMX_TEMP_PASSIVE.
> +        * Set the default passive cooling trip point to critical - 20C
> +        * (can be changed from userspace)
>          */
> -       data->temp_critical = 1000 * 20 + data->temp_passive;
> +       data->temp_passive = data->temp_critical - (1000 * 20);
>
>         return 0;
>  }
> @@ -559,6 +589,10 @@ static int imx_thermal_probe(struct platform_device *pdev)
>                 return ret;
>         }
>
> +       dev_info(&pdev->dev, "%s CPU temperature grade - "
> +                "thresholds: crit:%ldC passive:%ldC\n", data->temp_grade,
> +                data->temp_critical / 1000, data->temp_passive / 1000);
> +
>         /* Enable measurements at ~ 10 Hz */
>         regmap_write(map, TEMPSENSE1 + REG_CLR, TEMPSENSE1_MEASURE_FREQ);
>         measure_freq = DIV_ROUND_UP(32768, 10); /* 10 Hz */
> --
> 1.9.1
>

Any feedback on this?

After using it a bit, I personally feel like the passive temp should
be changed to max-5C instead of max-20C.

Tim

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

* [PATCH v2] imx: thermal: use CPU temperature grade info for thresholds
  2015-04-30 16:02 [PATCH] imx: thermal: use CPU temperature grade info for thresholds Tim Harvey
                   ` (2 preceding siblings ...)
  2015-05-20 22:08 ` Tim Harvey
@ 2015-05-21 23:45 ` Tim Harvey
  2015-05-24  2:48   ` Shawn Guo
  2015-07-30  7:19 ` [PATCH] " Jon Nettleton
  4 siblings, 1 reply; 19+ messages in thread
From: Tim Harvey @ 2015-05-21 23:45 UTC (permalink / raw)
  To: linux-arm-kernel

The IMX6Q/IMX6DL SoC's have a 2-bit temperature grade stored in OTP which
is valid for all IMX6 SoC's (despite the fact that the IMXSDLRM and
IMXSXRM do not document this - this has been proven via tests as well as
verified by Freescale FAE).

Instead of assuming a fixed 85C for passive cooling threshold and 105C for
critical use the thermal grade for these configurations.

We will set the critical to maxT - 5C and passive to maxT - 10C.

Cc: Anson Huang <b20788@freescale.com>
Cc: Fabio Estevam <fabio.estevam@freescale.com>
Cc: Eduardo Valentin <edubezval@gmail.com>
Cc: Zhang Rui <rui.zhang@intel.com>
Signed-off-by: Tim Harvey <tharvey@gateworks.com>
----
v2:
 - remove check for IMX6Q and update comments: The OTP values have been tested
   on IMX6SOLO, IMX6DUALLITE, and IMX6SX and Freescale FAE has shared data with
   me that the OTP settings are the same and that the reference manuals will
   reflect this in their next updates.
 - set critical to max - 5C
 - set passive to max - 10C
 - display max temp in info
 - do not allow passive to be set above critical

Signed-off-by: Tim Harvey <tharvey@gateworks.com>
---
 drivers/thermal/imx_thermal.c | 56 +++++++++++++++++++++++++++++++------------
 1 file changed, 41 insertions(+), 15 deletions(-)

diff --git a/drivers/thermal/imx_thermal.c b/drivers/thermal/imx_thermal.c
index 2ccbc07..d349d02 100644
--- a/drivers/thermal/imx_thermal.c
+++ b/drivers/thermal/imx_thermal.c
@@ -55,6 +55,7 @@
 #define TEMPSENSE2_PANIC_VALUE_SHIFT	16
 #define TEMPSENSE2_PANIC_VALUE_MASK	0xfff0000
 
+#define OCOTP_MEM0			0x0480
 #define OCOTP_ANA1			0x04e0
 
 /* The driver supports 1 passive trip point and 1 critical trip point */
@@ -64,12 +65,6 @@ enum imx_thermal_trip {
 	IMX_TRIP_NUM,
 };
 
-/*
- * It defines the temperature in millicelsius for passive trip point
- * that will trigger cooling action when crossed.
- */
-#define IMX_TEMP_PASSIVE		85000
-
 #define IMX_POLLING_DELAY		2000 /* millisecond */
 #define IMX_PASSIVE_DELAY		1000
 
@@ -100,12 +95,14 @@ struct imx_thermal_data {
 	u32 c1, c2; /* See formula in imx_get_sensor_data() */
 	unsigned long temp_passive;
 	unsigned long temp_critical;
+	unsigned long temp_max;
 	unsigned long alarm_temp;
 	unsigned long last_temp;
 	bool irq_enabled;
 	int irq;
 	struct clk *thermal_clk;
 	const struct thermal_soc_data *socdata;
+	const char *temp_grade;
 };
 
 static void imx_set_panic_temp(struct imx_thermal_data *data,
@@ -286,10 +283,12 @@ static int imx_set_trip_temp(struct thermal_zone_device *tz, int trip,
 {
 	struct imx_thermal_data *data = tz->devdata;
 
+	/* do not allow changing critical threshold */
 	if (trip == IMX_TRIP_CRITICAL)
 		return -EPERM;
 
-	if (temp > IMX_TEMP_PASSIVE)
+	/* do not allow passive to be set higher than critical */
+	if (temp > data->temp_critical)
 		return -EINVAL;
 
 	data->temp_passive = temp;
@@ -404,17 +403,39 @@ static int imx_get_sensor_data(struct platform_device *pdev)
 	data->c1 = temp64;
 	data->c2 = n1 * data->c1 + 1000 * t1;
 
-	/*
-	 * Set the default passive cooling trip point,
-	 * can be changed from userspace.
-	 */
-	data->temp_passive = IMX_TEMP_PASSIVE;
+	/* use OTP for thermal grade */
+	ret = regmap_read(map, OCOTP_MEM0, &val);
+	if (ret) {
+		dev_err(&pdev->dev, "failed to read temp grade: %d\n", ret);
+		return ret;
+	}
+
+	/* The maximum die temp is specified by the Temperature Grade */
+	switch ((val >> 6) & 0x3) {
+	case 0: /* Commercial (0 to 95C) */
+		data->temp_grade = "Commercial";
+		data->temp_max = 95000;
+		break;
+	case 1: /* Extended Commercial (-20 to 105C) */
+		data->temp_grade = "Extended Commercial";
+		data->temp_max = 105000;
+		break;
+	case 2: /* Industrial (-40 to 105C) */
+		data->temp_grade = "Industrial";
+		data->temp_max = 105000;
+		break;
+	case 3: /* Automotive (-40 to 125C) */
+		data->temp_grade = "Automotive";
+		data->temp_max = 125000;
+		break;
+	}
 
 	/*
-	 * The maximum die temperature set to 20 C higher than
-	 * IMX_TEMP_PASSIVE.
+	 * Set the critical trip point at 5C under max
+	 * Set the passive trip point at 10C under max (can change via sysfs)
 	 */
-	data->temp_critical = 1000 * 20 + data->temp_passive;
+	data->temp_critical = data->temp_max - (1000 * 5);
+	data->temp_passive = data->temp_max - (1000 * 10);
 
 	return 0;
 }
@@ -559,6 +580,11 @@ static int imx_thermal_probe(struct platform_device *pdev)
 		return ret;
 	}
 
+	dev_info(&pdev->dev, "%s CPU temperature grade - max:%ldC"
+		 " critical:%ldC passive:%ldC\n", data->temp_grade,
+		 data->temp_max / 1000, data->temp_critical / 1000,
+		 data->temp_passive / 1000);
+
 	/* Enable measurements at ~ 10 Hz */
 	regmap_write(map, TEMPSENSE1 + REG_CLR, TEMPSENSE1_MEASURE_FREQ);
 	measure_freq = DIV_ROUND_UP(32768, 10); /* 10 Hz */
-- 
1.9.1

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

* [PATCH v2] imx: thermal: use CPU temperature grade info for thresholds
  2015-05-21 23:45 ` [PATCH v2] " Tim Harvey
@ 2015-05-24  2:48   ` Shawn Guo
  2015-05-24  5:19     ` Jon Nettleton
  0 siblings, 1 reply; 19+ messages in thread
From: Shawn Guo @ 2015-05-24  2:48 UTC (permalink / raw)
  To: linux-arm-kernel

On Thu, May 21, 2015 at 04:45:47PM -0700, Tim Harvey wrote:
> The IMX6Q/IMX6DL SoC's have a 2-bit temperature grade stored in OTP which
> is valid for all IMX6 SoC's (despite the fact that the IMXSDLRM and
> IMXSXRM do not document this - this has been proven via tests as well as
> verified by Freescale FAE).
> 
> Instead of assuming a fixed 85C for passive cooling threshold and 105C for
> critical use the thermal grade for these configurations.
> 
> We will set the critical to maxT - 5C and passive to maxT - 10C.
> 
> Cc: Anson Huang <b20788@freescale.com>
> Cc: Fabio Estevam <fabio.estevam@freescale.com>
> Cc: Eduardo Valentin <edubezval@gmail.com>
> Cc: Zhang Rui <rui.zhang@intel.com>
> Signed-off-by: Tim Harvey <tharvey@gateworks.com>

Acked-by: Shawn Guo <shawn.guo@linaro.org>

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

* [PATCH v2] imx: thermal: use CPU temperature grade info for thresholds
  2015-05-24  2:48   ` Shawn Guo
@ 2015-05-24  5:19     ` Jon Nettleton
  2015-05-26 21:24       ` Tim Harvey
  0 siblings, 1 reply; 19+ messages in thread
From: Jon Nettleton @ 2015-05-24  5:19 UTC (permalink / raw)
  To: linux-arm-kernel

On Sun, May 24, 2015 at 4:48 AM, Shawn Guo <shawn.guo@linaro.org> wrote:
> On Thu, May 21, 2015 at 04:45:47PM -0700, Tim Harvey wrote:
>> The IMX6Q/IMX6DL SoC's have a 2-bit temperature grade stored in OTP which
>> is valid for all IMX6 SoC's (despite the fact that the IMXSDLRM and
>> IMXSXRM do not document this - this has been proven via tests as well as
>> verified by Freescale FAE).
>>
>> Instead of assuming a fixed 85C for passive cooling threshold and 105C for
>> critical use the thermal grade for these configurations.
>>
>> We will set the critical to maxT - 5C and passive to maxT - 10C.

I would like to chime in here if you don't mind.  I have been carrying
a patch similar to this in the SolidRun repo to fix cooling issues
that we have had.  I would recommend keeping the passive temp at maxT
- 20C due to the thermal properties of the chip.  I have found that
around 85-90C we can maintain a relatively steady thermal state with
only passive cooling.  Generally with a hard non-NEON based cpu
workload the iMX6 will level off at about 87C with all the cores
clocked to 1Ghz, and sometimes dipping down to 800Mhz periodically.
With a NEON based workload on all the cores it will push beyond this
and generally end up finding steady state at about 800Mhz right around
90C.

If you raise the initial passive threshold by 10C it will allow enough
heat to build up in the chip that the only way to avoid reaching
critical temps is by dropping the CPU down to its lowest frequency.
This is not the best experience as then you have a much warmer chip
and if the workload doesn't change you will just be switching between
running at the highest cpu frequency or lowest which makes for a
choppy experience.  A longer passive cooling zone allows the
temperature of the chip to be regulated using only passive methods but
without drastic performance drops.

I am doing things a bit differently in my implementation as I setup a
passive cooling zone for each cpu frequency, but that is just so you
can have more control from userspace by changing the different passive
trip points.

-Jon

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

* [PATCH v2] imx: thermal: use CPU temperature grade info for thresholds
  2015-05-24  5:19     ` Jon Nettleton
@ 2015-05-26 21:24       ` Tim Harvey
  2015-05-27  6:08         ` Jon Nettleton
  0 siblings, 1 reply; 19+ messages in thread
From: Tim Harvey @ 2015-05-26 21:24 UTC (permalink / raw)
  To: linux-arm-kernel

On Sat, May 23, 2015 at 10:19 PM, Jon Nettleton <jon.nettleton@gmail.com> wrote:
> On Sun, May 24, 2015 at 4:48 AM, Shawn Guo <shawn.guo@linaro.org> wrote:
>> On Thu, May 21, 2015 at 04:45:47PM -0700, Tim Harvey wrote:
>>> The IMX6Q/IMX6DL SoC's have a 2-bit temperature grade stored in OTP which
>>> is valid for all IMX6 SoC's (despite the fact that the IMXSDLRM and
>>> IMXSXRM do not document this - this has been proven via tests as well as
>>> verified by Freescale FAE).
>>>
>>> Instead of assuming a fixed 85C for passive cooling threshold and 105C for
>>> critical use the thermal grade for these configurations.
>>>
>>> We will set the critical to maxT - 5C and passive to maxT - 10C.
>
> I would like to chime in here if you don't mind.  I have been carrying
> a patch similar to this in the SolidRun repo to fix cooling issues
> that we have had.  I would recommend keeping the passive temp at maxT
> - 20C due to the thermal properties of the chip.  I have found that
> around 85-90C we can maintain a relatively steady thermal state with
> only passive cooling.  Generally with a hard non-NEON based cpu
> workload the iMX6 will level off at about 87C with all the cores
> clocked to 1Ghz, and sometimes dipping down to 800Mhz periodically.
> With a NEON based workload on all the cores it will push beyond this
> and generally end up finding steady state at about 800Mhz right around
> 90C.
>
> If you raise the initial passive threshold by 10C it will allow enough
> heat to build up in the chip that the only way to avoid reaching
> critical temps is by dropping the CPU down to its lowest frequency.
> This is not the best experience as then you have a much warmer chip
> and if the workload doesn't change you will just be switching between
> running at the highest cpu frequency or lowest which makes for a
> choppy experience.  A longer passive cooling zone allows the
> temperature of the chip to be regulated using only passive methods but
> without drastic performance drops.
>
> I am doing things a bit differently in my implementation as I setup a
> passive cooling zone for each cpu frequency, but that is just so you
> can have more control from userspace by changing the different passive
> trip points.
>
> -Jon

Jon,

I can agree with leaving a Max-20C passive delta. What do you think
about the critical threshold of Max-5C and rule of not allowing it to
be changed?

Tim

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

* [PATCH v2] imx: thermal: use CPU temperature grade info for thresholds
  2015-05-26 21:24       ` Tim Harvey
@ 2015-05-27  6:08         ` Jon Nettleton
  2015-07-28 14:50           ` Tim Harvey
  0 siblings, 1 reply; 19+ messages in thread
From: Jon Nettleton @ 2015-05-27  6:08 UTC (permalink / raw)
  To: linux-arm-kernel

On Tue, May 26, 2015 at 11:24 PM, Tim Harvey <tharvey@gateworks.com> wrote:
> On Sat, May 23, 2015 at 10:19 PM, Jon Nettleton <jon.nettleton@gmail.com> wrote:
>> On Sun, May 24, 2015 at 4:48 AM, Shawn Guo <shawn.guo@linaro.org> wrote:
>>> On Thu, May 21, 2015 at 04:45:47PM -0700, Tim Harvey wrote:
>>>> The IMX6Q/IMX6DL SoC's have a 2-bit temperature grade stored in OTP which
>>>> is valid for all IMX6 SoC's (despite the fact that the IMXSDLRM and
>>>> IMXSXRM do not document this - this has been proven via tests as well as
>>>> verified by Freescale FAE).
>>>>
>>>> Instead of assuming a fixed 85C for passive cooling threshold and 105C for
>>>> critical use the thermal grade for these configurations.
>>>>
>>>> We will set the critical to maxT - 5C and passive to maxT - 10C.
>>
>> I would like to chime in here if you don't mind.  I have been carrying
>> a patch similar to this in the SolidRun repo to fix cooling issues
>> that we have had.  I would recommend keeping the passive temp at maxT
>> - 20C due to the thermal properties of the chip.  I have found that
>> around 85-90C we can maintain a relatively steady thermal state with
>> only passive cooling.  Generally with a hard non-NEON based cpu
>> workload the iMX6 will level off at about 87C with all the cores
>> clocked to 1Ghz, and sometimes dipping down to 800Mhz periodically.
>> With a NEON based workload on all the cores it will push beyond this
>> and generally end up finding steady state at about 800Mhz right around
>> 90C.
>>
>> If you raise the initial passive threshold by 10C it will allow enough
>> heat to build up in the chip that the only way to avoid reaching
>> critical temps is by dropping the CPU down to its lowest frequency.
>> This is not the best experience as then you have a much warmer chip
>> and if the workload doesn't change you will just be switching between
>> running at the highest cpu frequency or lowest which makes for a
>> choppy experience.  A longer passive cooling zone allows the
>> temperature of the chip to be regulated using only passive methods but
>> without drastic performance drops.
>>
>> I am doing things a bit differently in my implementation as I setup a
>> passive cooling zone for each cpu frequency, but that is just so you
>> can have more control from userspace by changing the different passive
>> trip points.
>>
>> -Jon
>
> Jon,
>
> I can agree with leaving a Max-20C passive delta. What do you think
> about the critical threshold of Max-5C and rule of not allowing it to
> be changed?
>

Tim,

I definitely agree that the Critical temp should be a fixed point.  Is
the purpose of lowering the critical threshold from the hardware
default, to allow Linux to shutdown more cleanly rather than just have
the hardware shutting down?  If that is the case then I think that is
fine.  If it is to protect the SOC then that is unnecessary.  We have
heated the SOCs to well beyond the critical threshold and they have
survived just fine.

This is a bit out of context but here is the formula I am using to
figure out my trip points.  By default I use a linear set of trip
points for passive cooling.
https://github.com/linux4kix/linux-linaro-stable-mx6/commit/212c17d543739a5fe0bd75b66c10f05177e8bcb0

The short of it is I set a trip delta of 6C and then figure out the
lowest passive trip point as Critical - (#passive trip points * trip
delta), where each cpu frequency stage is a passive trip point.  This
will allow an 800Mhz SOC with 2 trip points to run at full speed
longer than a 1.2Ghz with 4 trip points.  The idea being that the
higher the clock rate means we will generate more heat and have more
passive cooling levels so it is better to drop the top speed of the
CPU earlier in order to let the passive cooling be effective and find
a steady state.

This may be a bit over the top but has fixed problems where long
running processes would build up heat and eventually cause a thermal
shutdown, but doesn't completely cripple the faster SOCs.

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

* [PATCH v2] imx: thermal: use CPU temperature grade info for thresholds
  2015-05-27  6:08         ` Jon Nettleton
@ 2015-07-28 14:50           ` Tim Harvey
  2015-07-28 15:01             ` Jon Nettleton
  2015-07-28 16:12             ` Jon Nettleton
  0 siblings, 2 replies; 19+ messages in thread
From: Tim Harvey @ 2015-07-28 14:50 UTC (permalink / raw)
  To: linux-arm-kernel

On Tue, May 26, 2015 at 11:08 PM, Jon Nettleton <jon.nettleton@gmail.com> wrote:
> On Tue, May 26, 2015 at 11:24 PM, Tim Harvey <tharvey@gateworks.com> wrote:
>> On Sat, May 23, 2015 at 10:19 PM, Jon Nettleton <jon.nettleton@gmail.com> wrote:
>>> On Sun, May 24, 2015 at 4:48 AM, Shawn Guo <shawn.guo@linaro.org> wrote:
>>>> On Thu, May 21, 2015 at 04:45:47PM -0700, Tim Harvey wrote:
>>>>> The IMX6Q/IMX6DL SoC's have a 2-bit temperature grade stored in OTP which
>>>>> is valid for all IMX6 SoC's (despite the fact that the IMXSDLRM and
>>>>> IMXSXRM do not document this - this has been proven via tests as well as
>>>>> verified by Freescale FAE).
>>>>>
>>>>> Instead of assuming a fixed 85C for passive cooling threshold and 105C for
>>>>> critical use the thermal grade for these configurations.
>>>>>
>>>>> We will set the critical to maxT - 5C and passive to maxT - 10C.
>>>
>>> I would like to chime in here if you don't mind.  I have been carrying
>>> a patch similar to this in the SolidRun repo to fix cooling issues
>>> that we have had.  I would recommend keeping the passive temp at maxT
>>> - 20C due to the thermal properties of the chip.  I have found that
>>> around 85-90C we can maintain a relatively steady thermal state with
>>> only passive cooling.  Generally with a hard non-NEON based cpu
>>> workload the iMX6 will level off at about 87C with all the cores
>>> clocked to 1Ghz, and sometimes dipping down to 800Mhz periodically.
>>> With a NEON based workload on all the cores it will push beyond this
>>> and generally end up finding steady state at about 800Mhz right around
>>> 90C.
>>>
>>> If you raise the initial passive threshold by 10C it will allow enough
>>> heat to build up in the chip that the only way to avoid reaching
>>> critical temps is by dropping the CPU down to its lowest frequency.
>>> This is not the best experience as then you have a much warmer chip
>>> and if the workload doesn't change you will just be switching between
>>> running at the highest cpu frequency or lowest which makes for a
>>> choppy experience.  A longer passive cooling zone allows the
>>> temperature of the chip to be regulated using only passive methods but
>>> without drastic performance drops.
>>>
>>> I am doing things a bit differently in my implementation as I setup a
>>> passive cooling zone for each cpu frequency, but that is just so you
>>> can have more control from userspace by changing the different passive
>>> trip points.
>>>
>>> -Jon
>>
>> Jon,
>>
>> I can agree with leaving a Max-20C passive delta. What do you think
>> about the critical threshold of Max-5C and rule of not allowing it to
>> be changed?
>>
>
> Tim,
>
> I definitely agree that the Critical temp should be a fixed point.  Is
> the purpose of lowering the critical threshold from the hardware
> default, to allow Linux to shutdown more cleanly rather than just have
> the hardware shutting down?  If that is the case then I think that is
> fine.  If it is to protect the SOC then that is unnecessary.  We have
> heated the SOCs to well beyond the critical threshold and they have
> survived just fine.
>
> This is a bit out of context but here is the formula I am using to
> figure out my trip points.  By default I use a linear set of trip
> points for passive cooling.
> https://github.com/linux4kix/linux-linaro-stable-mx6/commit/212c17d543739a5fe0bd75b66c10f05177e8bcb0
>
> The short of it is I set a trip delta of 6C and then figure out the
> lowest passive trip point as Critical - (#passive trip points * trip
> delta), where each cpu frequency stage is a passive trip point.  This
> will allow an 800Mhz SOC with 2 trip points to run at full speed
> longer than a 1.2Ghz with 4 trip points.  The idea being that the
> higher the clock rate means we will generate more heat and have more
> passive cooling levels so it is better to drop the top speed of the
> CPU earlier in order to let the passive cooling be effective and find
> a steady state.
>
> This may be a bit over the top but has fixed problems where long
> running processes would build up heat and eventually cause a thermal
> shutdown, but doesn't completely cripple the faster SOCs.

Jon,

Yes - the purpose of lowering the critical threshold from the hardware
default is to allow Linux to shutdown more cleanly.

If you agree with the fact that the patch here offers the improvement
of using OTG temperature grade as a basis can you ack it and if you
feel that the thresholds need to be adjusted perhaps propose a
follow-on patch? I feel people can debate the temperature delta's
endlessly but what I was really after here was to fix the fact that
all the processors are not temperature graded equally because they are
packaged differently (metal case on automotive offering better thermal
conductivity vs plastic case on consumer)

Regards,

Tim

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

* [PATCH v2] imx: thermal: use CPU temperature grade info for thresholds
  2015-07-28 14:50           ` Tim Harvey
@ 2015-07-28 15:01             ` Jon Nettleton
  2015-07-28 16:10               ` Jon Nettleton
  2015-07-28 16:12             ` Jon Nettleton
  1 sibling, 1 reply; 19+ messages in thread
From: Jon Nettleton @ 2015-07-28 15:01 UTC (permalink / raw)
  To: linux-arm-kernel

These changes need to be made to enable the canbus in the device-tree.
By default we have those pins assigned as GPIO.  As soon as I have the
device-tree overlay patches pushed this configuration will be more
dynamic, however now you must disable and enable the different iomux
pin functionality by hand in the device-tree.

diff --git a/arch/arm/boot/dts/imx6qdl-hummingboard.dtsi
b/arch/arm/boot/dts/imx6qdl-hummingboard.dtsi
index 7dcae42..308de69 100644
--- a/arch/arm/boot/dts/imx6qdl-hummingboard.dtsi
+++ b/arch/arm/boot/dts/imx6qdl-hummingboard.dtsi
@@ -168,7 +168,7 @@
 &flexcan1 {
        pinctrl-names = "default";
        pinctrl-0 = <&pinctrl_hummingboard_flexcan1>;
-       status = "disabled";
+       status = "okay";
 };

 &hdmi_core {
@@ -278,8 +278,9 @@
                                 MX6QDL_PAD_EIM_DA8__GPIO3_IO08 0x400130b1
                                 MX6QDL_PAD_EIM_DA7__GPIO3_IO07 0x400130b1
                                 MX6QDL_PAD_EIM_DA6__GPIO3_IO06 0x400130b1
-                                MX6QDL_PAD_SD3_CMD__GPIO7_IO02 0x400130b1
-                                MX6QDL_PAD_SD3_CLK__GPIO7_IO03 0x400130b1
+/*                               MX6QDL_PAD_SD3_CMD__GPIO7_IO02 0x400130b1
+ *                               MX6QDL_PAD_SD3_CLK__GPIO7_IO03 0x400130b1
+ */
                                 MX6QDL_PAD_EIM_DA3__GPIO3_IO03 0x400130b1
                         >;
                 };

On Tue, Jul 28, 2015 at 4:50 PM, Tim Harvey <tharvey@gateworks.com> wrote:
> On Tue, May 26, 2015 at 11:08 PM, Jon Nettleton <jon.nettleton@gmail.com> wrote:
>> On Tue, May 26, 2015 at 11:24 PM, Tim Harvey <tharvey@gateworks.com> wrote:
>>> On Sat, May 23, 2015 at 10:19 PM, Jon Nettleton <jon.nettleton@gmail.com> wrote:
>>>> On Sun, May 24, 2015 at 4:48 AM, Shawn Guo <shawn.guo@linaro.org> wrote:
>>>>> On Thu, May 21, 2015 at 04:45:47PM -0700, Tim Harvey wrote:
>>>>>> The IMX6Q/IMX6DL SoC's have a 2-bit temperature grade stored in OTP which
>>>>>> is valid for all IMX6 SoC's (despite the fact that the IMXSDLRM and
>>>>>> IMXSXRM do not document this - this has been proven via tests as well as
>>>>>> verified by Freescale FAE).
>>>>>>
>>>>>> Instead of assuming a fixed 85C for passive cooling threshold and 105C for
>>>>>> critical use the thermal grade for these configurations.
>>>>>>
>>>>>> We will set the critical to maxT - 5C and passive to maxT - 10C.
>>>>
>>>> I would like to chime in here if you don't mind.  I have been carrying
>>>> a patch similar to this in the SolidRun repo to fix cooling issues
>>>> that we have had.  I would recommend keeping the passive temp at maxT
>>>> - 20C due to the thermal properties of the chip.  I have found that
>>>> around 85-90C we can maintain a relatively steady thermal state with
>>>> only passive cooling.  Generally with a hard non-NEON based cpu
>>>> workload the iMX6 will level off at about 87C with all the cores
>>>> clocked to 1Ghz, and sometimes dipping down to 800Mhz periodically.
>>>> With a NEON based workload on all the cores it will push beyond this
>>>> and generally end up finding steady state at about 800Mhz right around
>>>> 90C.
>>>>
>>>> If you raise the initial passive threshold by 10C it will allow enough
>>>> heat to build up in the chip that the only way to avoid reaching
>>>> critical temps is by dropping the CPU down to its lowest frequency.
>>>> This is not the best experience as then you have a much warmer chip
>>>> and if the workload doesn't change you will just be switching between
>>>> running at the highest cpu frequency or lowest which makes for a
>>>> choppy experience.  A longer passive cooling zone allows the
>>>> temperature of the chip to be regulated using only passive methods but
>>>> without drastic performance drops.
>>>>
>>>> I am doing things a bit differently in my implementation as I setup a
>>>> passive cooling zone for each cpu frequency, but that is just so you
>>>> can have more control from userspace by changing the different passive
>>>> trip points.
>>>>
>>>> -Jon
>>>
>>> Jon,
>>>
>>> I can agree with leaving a Max-20C passive delta. What do you think
>>> about the critical threshold of Max-5C and rule of not allowing it to
>>> be changed?
>>>
>>
>> Tim,
>>
>> I definitely agree that the Critical temp should be a fixed point.  Is
>> the purpose of lowering the critical threshold from the hardware
>> default, to allow Linux to shutdown more cleanly rather than just have
>> the hardware shutting down?  If that is the case then I think that is
>> fine.  If it is to protect the SOC then that is unnecessary.  We have
>> heated the SOCs to well beyond the critical threshold and they have
>> survived just fine.
>>
>> This is a bit out of context but here is the formula I am using to
>> figure out my trip points.  By default I use a linear set of trip
>> points for passive cooling.
>> https://github.com/linux4kix/linux-linaro-stable-mx6/commit/212c17d543739a5fe0bd75b66c10f05177e8bcb0
>>
>> The short of it is I set a trip delta of 6C and then figure out the
>> lowest passive trip point as Critical - (#passive trip points * trip
>> delta), where each cpu frequency stage is a passive trip point.  This
>> will allow an 800Mhz SOC with 2 trip points to run at full speed
>> longer than a 1.2Ghz with 4 trip points.  The idea being that the
>> higher the clock rate means we will generate more heat and have more
>> passive cooling levels so it is better to drop the top speed of the
>> CPU earlier in order to let the passive cooling be effective and find
>> a steady state.
>>
>> This may be a bit over the top but has fixed problems where long
>> running processes would build up heat and eventually cause a thermal
>> shutdown, but doesn't completely cripple the faster SOCs.
>
> Jon,
>
> Yes - the purpose of lowering the critical threshold from the hardware
> default is to allow Linux to shutdown more cleanly.
>
> If you agree with the fact that the patch here offers the improvement
> of using OTG temperature grade as a basis can you ack it and if you
> feel that the thresholds need to be adjusted perhaps propose a
> follow-on patch? I feel people can debate the temperature delta's
> endlessly but what I was really after here was to fix the fact that
> all the processors are not temperature graded equally because they are
> packaged differently (metal case on automotive offering better thermal
> conductivity vs plastic case on consumer)
>
> Regards,
>
> Tim

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

* [PATCH v2] imx: thermal: use CPU temperature grade info for thresholds
  2015-07-28 15:01             ` Jon Nettleton
@ 2015-07-28 16:10               ` Jon Nettleton
  0 siblings, 0 replies; 19+ messages in thread
From: Jon Nettleton @ 2015-07-28 16:10 UTC (permalink / raw)
  To: linux-arm-kernel

Sorry about that guys.  My blank emails alt+tabs got mixed up.  ignore that.

On Tue, Jul 28, 2015 at 5:01 PM, Jon Nettleton <jon.nettleton@gmail.com> wrote:
> These changes need to be made to enable the canbus in the device-tree.
> By default we have those pins assigned as GPIO.  As soon as I have the
> device-tree overlay patches pushed this configuration will be more
> dynamic, however now you must disable and enable the different iomux
> pin functionality by hand in the device-tree.
>
> diff --git a/arch/arm/boot/dts/imx6qdl-hummingboard.dtsi
> b/arch/arm/boot/dts/imx6qdl-hummingboard.dtsi
> index 7dcae42..308de69 100644
> --- a/arch/arm/boot/dts/imx6qdl-hummingboard.dtsi
> +++ b/arch/arm/boot/dts/imx6qdl-hummingboard.dtsi
> @@ -168,7 +168,7 @@
>  &flexcan1 {
>         pinctrl-names = "default";
>         pinctrl-0 = <&pinctrl_hummingboard_flexcan1>;
> -       status = "disabled";
> +       status = "okay";
>  };
>
>  &hdmi_core {
> @@ -278,8 +278,9 @@
>                                  MX6QDL_PAD_EIM_DA8__GPIO3_IO08 0x400130b1
>                                  MX6QDL_PAD_EIM_DA7__GPIO3_IO07 0x400130b1
>                                  MX6QDL_PAD_EIM_DA6__GPIO3_IO06 0x400130b1
> -                                MX6QDL_PAD_SD3_CMD__GPIO7_IO02 0x400130b1
> -                                MX6QDL_PAD_SD3_CLK__GPIO7_IO03 0x400130b1
> +/*                               MX6QDL_PAD_SD3_CMD__GPIO7_IO02 0x400130b1
> + *                               MX6QDL_PAD_SD3_CLK__GPIO7_IO03 0x400130b1
> + */
>                                  MX6QDL_PAD_EIM_DA3__GPIO3_IO03 0x400130b1
>                          >;
>                  };
>
> On Tue, Jul 28, 2015 at 4:50 PM, Tim Harvey <tharvey@gateworks.com> wrote:
>> On Tue, May 26, 2015 at 11:08 PM, Jon Nettleton <jon.nettleton@gmail.com> wrote:
>>> On Tue, May 26, 2015 at 11:24 PM, Tim Harvey <tharvey@gateworks.com> wrote:
>>>> On Sat, May 23, 2015 at 10:19 PM, Jon Nettleton <jon.nettleton@gmail.com> wrote:
>>>>> On Sun, May 24, 2015 at 4:48 AM, Shawn Guo <shawn.guo@linaro.org> wrote:
>>>>>> On Thu, May 21, 2015 at 04:45:47PM -0700, Tim Harvey wrote:
>>>>>>> The IMX6Q/IMX6DL SoC's have a 2-bit temperature grade stored in OTP which
>>>>>>> is valid for all IMX6 SoC's (despite the fact that the IMXSDLRM and
>>>>>>> IMXSXRM do not document this - this has been proven via tests as well as
>>>>>>> verified by Freescale FAE).
>>>>>>>
>>>>>>> Instead of assuming a fixed 85C for passive cooling threshold and 105C for
>>>>>>> critical use the thermal grade for these configurations.
>>>>>>>
>>>>>>> We will set the critical to maxT - 5C and passive to maxT - 10C.
>>>>>
>>>>> I would like to chime in here if you don't mind.  I have been carrying
>>>>> a patch similar to this in the SolidRun repo to fix cooling issues
>>>>> that we have had.  I would recommend keeping the passive temp at maxT
>>>>> - 20C due to the thermal properties of the chip.  I have found that
>>>>> around 85-90C we can maintain a relatively steady thermal state with
>>>>> only passive cooling.  Generally with a hard non-NEON based cpu
>>>>> workload the iMX6 will level off at about 87C with all the cores
>>>>> clocked to 1Ghz, and sometimes dipping down to 800Mhz periodically.
>>>>> With a NEON based workload on all the cores it will push beyond this
>>>>> and generally end up finding steady state at about 800Mhz right around
>>>>> 90C.
>>>>>
>>>>> If you raise the initial passive threshold by 10C it will allow enough
>>>>> heat to build up in the chip that the only way to avoid reaching
>>>>> critical temps is by dropping the CPU down to its lowest frequency.
>>>>> This is not the best experience as then you have a much warmer chip
>>>>> and if the workload doesn't change you will just be switching between
>>>>> running at the highest cpu frequency or lowest which makes for a
>>>>> choppy experience.  A longer passive cooling zone allows the
>>>>> temperature of the chip to be regulated using only passive methods but
>>>>> without drastic performance drops.
>>>>>
>>>>> I am doing things a bit differently in my implementation as I setup a
>>>>> passive cooling zone for each cpu frequency, but that is just so you
>>>>> can have more control from userspace by changing the different passive
>>>>> trip points.
>>>>>
>>>>> -Jon
>>>>
>>>> Jon,
>>>>
>>>> I can agree with leaving a Max-20C passive delta. What do you think
>>>> about the critical threshold of Max-5C and rule of not allowing it to
>>>> be changed?
>>>>
>>>
>>> Tim,
>>>
>>> I definitely agree that the Critical temp should be a fixed point.  Is
>>> the purpose of lowering the critical threshold from the hardware
>>> default, to allow Linux to shutdown more cleanly rather than just have
>>> the hardware shutting down?  If that is the case then I think that is
>>> fine.  If it is to protect the SOC then that is unnecessary.  We have
>>> heated the SOCs to well beyond the critical threshold and they have
>>> survived just fine.
>>>
>>> This is a bit out of context but here is the formula I am using to
>>> figure out my trip points.  By default I use a linear set of trip
>>> points for passive cooling.
>>> https://github.com/linux4kix/linux-linaro-stable-mx6/commit/212c17d543739a5fe0bd75b66c10f05177e8bcb0
>>>
>>> The short of it is I set a trip delta of 6C and then figure out the
>>> lowest passive trip point as Critical - (#passive trip points * trip
>>> delta), where each cpu frequency stage is a passive trip point.  This
>>> will allow an 800Mhz SOC with 2 trip points to run at full speed
>>> longer than a 1.2Ghz with 4 trip points.  The idea being that the
>>> higher the clock rate means we will generate more heat and have more
>>> passive cooling levels so it is better to drop the top speed of the
>>> CPU earlier in order to let the passive cooling be effective and find
>>> a steady state.
>>>
>>> This may be a bit over the top but has fixed problems where long
>>> running processes would build up heat and eventually cause a thermal
>>> shutdown, but doesn't completely cripple the faster SOCs.
>>
>> Jon,
>>
>> Yes - the purpose of lowering the critical threshold from the hardware
>> default is to allow Linux to shutdown more cleanly.
>>
>> If you agree with the fact that the patch here offers the improvement
>> of using OTG temperature grade as a basis can you ack it and if you
>> feel that the thresholds need to be adjusted perhaps propose a
>> follow-on patch? I feel people can debate the temperature delta's
>> endlessly but what I was really after here was to fix the fact that
>> all the processors are not temperature graded equally because they are
>> packaged differently (metal case on automotive offering better thermal
>> conductivity vs plastic case on consumer)
>>
>> Regards,
>>
>> Tim

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

* [PATCH v2] imx: thermal: use CPU temperature grade info for thresholds
  2015-07-28 14:50           ` Tim Harvey
  2015-07-28 15:01             ` Jon Nettleton
@ 2015-07-28 16:12             ` Jon Nettleton
  1 sibling, 0 replies; 19+ messages in thread
From: Jon Nettleton @ 2015-07-28 16:12 UTC (permalink / raw)
  To: linux-arm-kernel

Tim,

Okay that all sounds fine.  I came into this half way through so I
missed the core of what you were trying to accomplish, sorry to
de-rail the conversation a bit.

I will gladly ACK the patch and will follow up with patches we can
discuss for additional changes.

-Jon

On Tue, Jul 28, 2015 at 4:50 PM, Tim Harvey <tharvey@gateworks.com> wrote:
> On Tue, May 26, 2015 at 11:08 PM, Jon Nettleton <jon.nettleton@gmail.com> wrote:
>> On Tue, May 26, 2015 at 11:24 PM, Tim Harvey <tharvey@gateworks.com> wrote:
>>> On Sat, May 23, 2015 at 10:19 PM, Jon Nettleton <jon.nettleton@gmail.com> wrote:
>>>> On Sun, May 24, 2015 at 4:48 AM, Shawn Guo <shawn.guo@linaro.org> wrote:
>>>>> On Thu, May 21, 2015 at 04:45:47PM -0700, Tim Harvey wrote:
>>>>>> The IMX6Q/IMX6DL SoC's have a 2-bit temperature grade stored in OTP which
>>>>>> is valid for all IMX6 SoC's (despite the fact that the IMXSDLRM and
>>>>>> IMXSXRM do not document this - this has been proven via tests as well as
>>>>>> verified by Freescale FAE).
>>>>>>
>>>>>> Instead of assuming a fixed 85C for passive cooling threshold and 105C for
>>>>>> critical use the thermal grade for these configurations.
>>>>>>
>>>>>> We will set the critical to maxT - 5C and passive to maxT - 10C.
>>>>
>>>> I would like to chime in here if you don't mind.  I have been carrying
>>>> a patch similar to this in the SolidRun repo to fix cooling issues
>>>> that we have had.  I would recommend keeping the passive temp at maxT
>>>> - 20C due to the thermal properties of the chip.  I have found that
>>>> around 85-90C we can maintain a relatively steady thermal state with
>>>> only passive cooling.  Generally with a hard non-NEON based cpu
>>>> workload the iMX6 will level off at about 87C with all the cores
>>>> clocked to 1Ghz, and sometimes dipping down to 800Mhz periodically.
>>>> With a NEON based workload on all the cores it will push beyond this
>>>> and generally end up finding steady state at about 800Mhz right around
>>>> 90C.
>>>>
>>>> If you raise the initial passive threshold by 10C it will allow enough
>>>> heat to build up in the chip that the only way to avoid reaching
>>>> critical temps is by dropping the CPU down to its lowest frequency.
>>>> This is not the best experience as then you have a much warmer chip
>>>> and if the workload doesn't change you will just be switching between
>>>> running at the highest cpu frequency or lowest which makes for a
>>>> choppy experience.  A longer passive cooling zone allows the
>>>> temperature of the chip to be regulated using only passive methods but
>>>> without drastic performance drops.
>>>>
>>>> I am doing things a bit differently in my implementation as I setup a
>>>> passive cooling zone for each cpu frequency, but that is just so you
>>>> can have more control from userspace by changing the different passive
>>>> trip points.
>>>>
>>>> -Jon
>>>
>>> Jon,
>>>
>>> I can agree with leaving a Max-20C passive delta. What do you think
>>> about the critical threshold of Max-5C and rule of not allowing it to
>>> be changed?
>>>
>>
>> Tim,
>>
>> I definitely agree that the Critical temp should be a fixed point.  Is
>> the purpose of lowering the critical threshold from the hardware
>> default, to allow Linux to shutdown more cleanly rather than just have
>> the hardware shutting down?  If that is the case then I think that is
>> fine.  If it is to protect the SOC then that is unnecessary.  We have
>> heated the SOCs to well beyond the critical threshold and they have
>> survived just fine.
>>
>> This is a bit out of context but here is the formula I am using to
>> figure out my trip points.  By default I use a linear set of trip
>> points for passive cooling.
>> https://github.com/linux4kix/linux-linaro-stable-mx6/commit/212c17d543739a5fe0bd75b66c10f05177e8bcb0
>>
>> The short of it is I set a trip delta of 6C and then figure out the
>> lowest passive trip point as Critical - (#passive trip points * trip
>> delta), where each cpu frequency stage is a passive trip point.  This
>> will allow an 800Mhz SOC with 2 trip points to run at full speed
>> longer than a 1.2Ghz with 4 trip points.  The idea being that the
>> higher the clock rate means we will generate more heat and have more
>> passive cooling levels so it is better to drop the top speed of the
>> CPU earlier in order to let the passive cooling be effective and find
>> a steady state.
>>
>> This may be a bit over the top but has fixed problems where long
>> running processes would build up heat and eventually cause a thermal
>> shutdown, but doesn't completely cripple the faster SOCs.
>
> Jon,
>
> Yes - the purpose of lowering the critical threshold from the hardware
> default is to allow Linux to shutdown more cleanly.
>
> If you agree with the fact that the patch here offers the improvement
> of using OTG temperature grade as a basis can you ack it and if you
> feel that the thresholds need to be adjusted perhaps propose a
> follow-on patch? I feel people can debate the temperature delta's
> endlessly but what I was really after here was to fix the fact that
> all the processors are not temperature graded equally because they are
> packaged differently (metal case on automotive offering better thermal
> conductivity vs plastic case on consumer)
>
> Regards,
>
> Tim

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

* [PATCH] imx: thermal: use CPU temperature grade info for thresholds
  2015-04-30 16:02 [PATCH] imx: thermal: use CPU temperature grade info for thresholds Tim Harvey
                   ` (3 preceding siblings ...)
  2015-05-21 23:45 ` [PATCH v2] " Tim Harvey
@ 2015-07-30  7:19 ` Jon Nettleton
  2015-10-13 14:16     ` Tim Harvey
  4 siblings, 1 reply; 19+ messages in thread
From: Jon Nettleton @ 2015-07-30  7:19 UTC (permalink / raw)
  To: linux-arm-kernel

On Thu, Apr 30, 2015 at 6:02 PM, Tim Harvey <tharvey@gateworks.com> wrote:
> The IMX6Q/IMX6DL SoC's have a 2-bit temperature grade stored in OTP. Instead
> of assuming 85C for passive cooling threshold and 105C for critical use
> the thermal grade for these configurations. For IMX6SX which does not have
> a temperature grade in OTP (according to the ref manual) assume 105C critical.
> We will default the user-settable passive threshold to crit - 20C.
>
> Cc: Anson Huang <b20788@freescale.com>
> Cc: Fabio Estevam <fabio.estevam@freescale.com>
> Cc: Shawn Guo <shawn.guo@linaro.org>
> Signed-off-by: Tim Harvey <tharvey@gateworks.com>
> ---
>  drivers/thermal/imx_thermal.c | 64 +++++++++++++++++++++++++++++++++----------
>  1 file changed, 49 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/thermal/imx_thermal.c b/drivers/thermal/imx_thermal.c
> index 2ccbc07..c6a4eed 100644
> --- a/drivers/thermal/imx_thermal.c
> +++ b/drivers/thermal/imx_thermal.c
> @@ -55,6 +55,7 @@
>  #define TEMPSENSE2_PANIC_VALUE_SHIFT   16
>  #define TEMPSENSE2_PANIC_VALUE_MASK    0xfff0000
>
> +#define OCOTP_MEM0                     0x0480
>  #define OCOTP_ANA1                     0x04e0
>
>  /* The driver supports 1 passive trip point and 1 critical trip point */
> @@ -64,12 +65,6 @@ enum imx_thermal_trip {
>         IMX_TRIP_NUM,
>  };
>
> -/*
> - * It defines the temperature in millicelsius for passive trip point
> - * that will trigger cooling action when crossed.
> - */
> -#define IMX_TEMP_PASSIVE               85000
> -
>  #define IMX_POLLING_DELAY              2000 /* millisecond */
>  #define IMX_PASSIVE_DELAY              1000
>
> @@ -106,6 +101,7 @@ struct imx_thermal_data {
>         int irq;
>         struct clk *thermal_clk;
>         const struct thermal_soc_data *socdata;
> +       const char *temp_grade;
>  };
>
>  static void imx_set_panic_temp(struct imx_thermal_data *data,
> @@ -289,7 +285,8 @@ static int imx_set_trip_temp(struct thermal_zone_device *tz, int trip,
>         if (trip == IMX_TRIP_CRITICAL)
>                 return -EPERM;
>
> -       if (temp > IMX_TEMP_PASSIVE)
> +       /* do not allow passive to be set higher than critical - 20C */
> +       if (temp > (data->temp_critical - (1000 * 20)))
>                 return -EINVAL;
>
>         data->temp_passive = temp;
> @@ -404,17 +401,50 @@ static int imx_get_sensor_data(struct platform_device *pdev)
>         data->c1 = temp64;
>         data->c2 = n1 * data->c1 + 1000 * t1;
>
> -       /*
> -        * Set the default passive cooling trip point,
> -        * can be changed from userspace.
> -        */
> -       data->temp_passive = IMX_TEMP_PASSIVE;
> +       /* Assume Extended Commercial temperature grade limits (105C max) */
> +       data->temp_grade = "unknown";
> +       data->temp_critical = 105000;
> +
> +       /* For IMX6Q use OTP for thermal grade */
> +       if (data->socdata->version == TEMPMON_IMX6Q) {
> +               ret = regmap_read(map, OCOTP_MEM0, &val);
> +               if (ret) {
> +                       dev_err(&pdev->dev, "failed to read temp grade: %d\n",
> +                               ret);
> +                       return ret;
> +               }
> +
> +               if (val == 0 || val == ~0) {
> +                       dev_err(&pdev->dev, "invalid temp grade data\n");
> +                       return -EINVAL;
> +               }
> +
> +               /* The maximum die temp is specified by the Temperature Grade */
> +               switch ((val >> 6) & 0x3) {
> +               case 0: /* Commercial (0 to 95C) */
> +                       data->temp_grade = "Commercial";
> +                       data->temp_critical = 95000;
> +                       break;
> +               case 1: /* Extended Commercial (-20 to 105C) */
> +                       data->temp_grade = "Extended Commercial";
> +                       data->temp_critical = 105000;
> +                       break;
> +               case 2: /* Industrial (-40 to 105C) */
> +                       data->temp_grade = "Industrial";
> +                       data->temp_critical = 105000;
> +                       break;
> +               case 3: /* Automotive (-40 to 125C) */
> +                       data->temp_grade = "Automotive";
> +                       data->temp_critical = 125000;
> +                       break;
> +               }
> +       }
>
>         /*
> -        * The maximum die temperature set to 20 C higher than
> -        * IMX_TEMP_PASSIVE.
> +        * Set the default passive cooling trip point to critical - 20C
> +        * (can be changed from userspace)
>          */
> -       data->temp_critical = 1000 * 20 + data->temp_passive;
> +       data->temp_passive = data->temp_critical - (1000 * 20);
>
>         return 0;
>  }
> @@ -559,6 +589,10 @@ static int imx_thermal_probe(struct platform_device *pdev)
>                 return ret;
>         }
>
> +       dev_info(&pdev->dev, "%s CPU temperature grade - "
> +                "thresholds: crit:%ldC passive:%ldC\n", data->temp_grade,
> +                data->temp_critical / 1000, data->temp_passive / 1000);
> +
>         /* Enable measurements at ~ 10 Hz */
>         regmap_write(map, TEMPSENSE1 + REG_CLR, TEMPSENSE1_MEASURE_FREQ);
>         measure_freq = DIV_ROUND_UP(32768, 10); /* 10 Hz */
> --
> 1.9.1
>
>

I think this is a fine start and we can build off it.

Acked-by: Jon Nettleton <jon@solid-run.com>

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

* Re: [PATCH] imx: thermal: use CPU temperature grade info for thresholds
  2015-07-30  7:19 ` [PATCH] " Jon Nettleton
@ 2015-10-13 14:16     ` Tim Harvey
  0 siblings, 0 replies; 19+ messages in thread
From: Tim Harvey @ 2015-10-13 14:16 UTC (permalink / raw)
  To: Zhang Rui, Eduardo Valentin
  Cc: linux-pm, Jon Nettleton, Fabio Estevam, Anson Huang, Shawn Guo,
	linux-arm-kernel

On Thu, Jul 30, 2015 at 12:19 AM, Jon Nettleton <jon.nettleton@gmail.com> wrote:
> On Thu, Apr 30, 2015 at 6:02 PM, Tim Harvey <tharvey@gateworks.com> wrote:
>> The IMX6Q/IMX6DL SoC's have a 2-bit temperature grade stored in OTP. Instead
>> of assuming 85C for passive cooling threshold and 105C for critical use
>> the thermal grade for these configurations. For IMX6SX which does not have
>> a temperature grade in OTP (according to the ref manual) assume 105C critical.
>> We will default the user-settable passive threshold to crit - 20C.
>>
>> Cc: Anson Huang <b20788@freescale.com>
>> Cc: Fabio Estevam <fabio.estevam@freescale.com>
>> Cc: Shawn Guo <shawn.guo@linaro.org>
>> Signed-off-by: Tim Harvey <tharvey@gateworks.com>
>> ---
>>  drivers/thermal/imx_thermal.c | 64 +++++++++++++++++++++++++++++++++----------
>>  1 file changed, 49 insertions(+), 15 deletions(-)
>>
>> diff --git a/drivers/thermal/imx_thermal.c b/drivers/thermal/imx_thermal.c
>> index 2ccbc07..c6a4eed 100644
>> --- a/drivers/thermal/imx_thermal.c
>> +++ b/drivers/thermal/imx_thermal.c
>> @@ -55,6 +55,7 @@
>>  #define TEMPSENSE2_PANIC_VALUE_SHIFT   16
>>  #define TEMPSENSE2_PANIC_VALUE_MASK    0xfff0000
>>
>> +#define OCOTP_MEM0                     0x0480
>>  #define OCOTP_ANA1                     0x04e0
>>
>>  /* The driver supports 1 passive trip point and 1 critical trip point */
>> @@ -64,12 +65,6 @@ enum imx_thermal_trip {
>>         IMX_TRIP_NUM,
>>  };
>>
>> -/*
>> - * It defines the temperature in millicelsius for passive trip point
>> - * that will trigger cooling action when crossed.
>> - */
>> -#define IMX_TEMP_PASSIVE               85000
>> -
>>  #define IMX_POLLING_DELAY              2000 /* millisecond */
>>  #define IMX_PASSIVE_DELAY              1000
>>
>> @@ -106,6 +101,7 @@ struct imx_thermal_data {
>>         int irq;
>>         struct clk *thermal_clk;
>>         const struct thermal_soc_data *socdata;
>> +       const char *temp_grade;
>>  };
>>
>>  static void imx_set_panic_temp(struct imx_thermal_data *data,
>> @@ -289,7 +285,8 @@ static int imx_set_trip_temp(struct thermal_zone_device *tz, int trip,
>>         if (trip == IMX_TRIP_CRITICAL)
>>                 return -EPERM;
>>
>> -       if (temp > IMX_TEMP_PASSIVE)
>> +       /* do not allow passive to be set higher than critical - 20C */
>> +       if (temp > (data->temp_critical - (1000 * 20)))
>>                 return -EINVAL;
>>
>>         data->temp_passive = temp;
>> @@ -404,17 +401,50 @@ static int imx_get_sensor_data(struct platform_device *pdev)
>>         data->c1 = temp64;
>>         data->c2 = n1 * data->c1 + 1000 * t1;
>>
>> -       /*
>> -        * Set the default passive cooling trip point,
>> -        * can be changed from userspace.
>> -        */
>> -       data->temp_passive = IMX_TEMP_PASSIVE;
>> +       /* Assume Extended Commercial temperature grade limits (105C max) */
>> +       data->temp_grade = "unknown";
>> +       data->temp_critical = 105000;
>> +
>> +       /* For IMX6Q use OTP for thermal grade */
>> +       if (data->socdata->version == TEMPMON_IMX6Q) {
>> +               ret = regmap_read(map, OCOTP_MEM0, &val);
>> +               if (ret) {
>> +                       dev_err(&pdev->dev, "failed to read temp grade: %d\n",
>> +                               ret);
>> +                       return ret;
>> +               }
>> +
>> +               if (val == 0 || val == ~0) {
>> +                       dev_err(&pdev->dev, "invalid temp grade data\n");
>> +                       return -EINVAL;
>> +               }
>> +
>> +               /* The maximum die temp is specified by the Temperature Grade */
>> +               switch ((val >> 6) & 0x3) {
>> +               case 0: /* Commercial (0 to 95C) */
>> +                       data->temp_grade = "Commercial";
>> +                       data->temp_critical = 95000;
>> +                       break;
>> +               case 1: /* Extended Commercial (-20 to 105C) */
>> +                       data->temp_grade = "Extended Commercial";
>> +                       data->temp_critical = 105000;
>> +                       break;
>> +               case 2: /* Industrial (-40 to 105C) */
>> +                       data->temp_grade = "Industrial";
>> +                       data->temp_critical = 105000;
>> +                       break;
>> +               case 3: /* Automotive (-40 to 125C) */
>> +                       data->temp_grade = "Automotive";
>> +                       data->temp_critical = 125000;
>> +                       break;
>> +               }
>> +       }
>>
>>         /*
>> -        * The maximum die temperature set to 20 C higher than
>> -        * IMX_TEMP_PASSIVE.
>> +        * Set the default passive cooling trip point to critical - 20C
>> +        * (can be changed from userspace)
>>          */
>> -       data->temp_critical = 1000 * 20 + data->temp_passive;
>> +       data->temp_passive = data->temp_critical - (1000 * 20);
>>
>>         return 0;
>>  }
>> @@ -559,6 +589,10 @@ static int imx_thermal_probe(struct platform_device *pdev)
>>                 return ret;
>>         }
>>
>> +       dev_info(&pdev->dev, "%s CPU temperature grade - "
>> +                "thresholds: crit:%ldC passive:%ldC\n", data->temp_grade,
>> +                data->temp_critical / 1000, data->temp_passive / 1000);
>> +
>>         /* Enable measurements at ~ 10 Hz */
>>         regmap_write(map, TEMPSENSE1 + REG_CLR, TEMPSENSE1_MEASURE_FREQ);
>>         measure_freq = DIV_ROUND_UP(32768, 10); /* 10 Hz */
>> --
>> 1.9.1
>>
>>
>
> I think this is a fine start and we can build off it.
>
> Acked-by: Jon Nettleton <jon@solid-run.com>

adding CC linux-pm@vger.kernel.org

Zhang / Eduardo,

This seems to have gotten missed - can you please review this for merge?

Regards,

Tim

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

* [PATCH] imx: thermal: use CPU temperature grade info for thresholds
@ 2015-10-13 14:16     ` Tim Harvey
  0 siblings, 0 replies; 19+ messages in thread
From: Tim Harvey @ 2015-10-13 14:16 UTC (permalink / raw)
  To: linux-arm-kernel

On Thu, Jul 30, 2015 at 12:19 AM, Jon Nettleton <jon.nettleton@gmail.com> wrote:
> On Thu, Apr 30, 2015 at 6:02 PM, Tim Harvey <tharvey@gateworks.com> wrote:
>> The IMX6Q/IMX6DL SoC's have a 2-bit temperature grade stored in OTP. Instead
>> of assuming 85C for passive cooling threshold and 105C for critical use
>> the thermal grade for these configurations. For IMX6SX which does not have
>> a temperature grade in OTP (according to the ref manual) assume 105C critical.
>> We will default the user-settable passive threshold to crit - 20C.
>>
>> Cc: Anson Huang <b20788@freescale.com>
>> Cc: Fabio Estevam <fabio.estevam@freescale.com>
>> Cc: Shawn Guo <shawn.guo@linaro.org>
>> Signed-off-by: Tim Harvey <tharvey@gateworks.com>
>> ---
>>  drivers/thermal/imx_thermal.c | 64 +++++++++++++++++++++++++++++++++----------
>>  1 file changed, 49 insertions(+), 15 deletions(-)
>>
>> diff --git a/drivers/thermal/imx_thermal.c b/drivers/thermal/imx_thermal.c
>> index 2ccbc07..c6a4eed 100644
>> --- a/drivers/thermal/imx_thermal.c
>> +++ b/drivers/thermal/imx_thermal.c
>> @@ -55,6 +55,7 @@
>>  #define TEMPSENSE2_PANIC_VALUE_SHIFT   16
>>  #define TEMPSENSE2_PANIC_VALUE_MASK    0xfff0000
>>
>> +#define OCOTP_MEM0                     0x0480
>>  #define OCOTP_ANA1                     0x04e0
>>
>>  /* The driver supports 1 passive trip point and 1 critical trip point */
>> @@ -64,12 +65,6 @@ enum imx_thermal_trip {
>>         IMX_TRIP_NUM,
>>  };
>>
>> -/*
>> - * It defines the temperature in millicelsius for passive trip point
>> - * that will trigger cooling action when crossed.
>> - */
>> -#define IMX_TEMP_PASSIVE               85000
>> -
>>  #define IMX_POLLING_DELAY              2000 /* millisecond */
>>  #define IMX_PASSIVE_DELAY              1000
>>
>> @@ -106,6 +101,7 @@ struct imx_thermal_data {
>>         int irq;
>>         struct clk *thermal_clk;
>>         const struct thermal_soc_data *socdata;
>> +       const char *temp_grade;
>>  };
>>
>>  static void imx_set_panic_temp(struct imx_thermal_data *data,
>> @@ -289,7 +285,8 @@ static int imx_set_trip_temp(struct thermal_zone_device *tz, int trip,
>>         if (trip == IMX_TRIP_CRITICAL)
>>                 return -EPERM;
>>
>> -       if (temp > IMX_TEMP_PASSIVE)
>> +       /* do not allow passive to be set higher than critical - 20C */
>> +       if (temp > (data->temp_critical - (1000 * 20)))
>>                 return -EINVAL;
>>
>>         data->temp_passive = temp;
>> @@ -404,17 +401,50 @@ static int imx_get_sensor_data(struct platform_device *pdev)
>>         data->c1 = temp64;
>>         data->c2 = n1 * data->c1 + 1000 * t1;
>>
>> -       /*
>> -        * Set the default passive cooling trip point,
>> -        * can be changed from userspace.
>> -        */
>> -       data->temp_passive = IMX_TEMP_PASSIVE;
>> +       /* Assume Extended Commercial temperature grade limits (105C max) */
>> +       data->temp_grade = "unknown";
>> +       data->temp_critical = 105000;
>> +
>> +       /* For IMX6Q use OTP for thermal grade */
>> +       if (data->socdata->version == TEMPMON_IMX6Q) {
>> +               ret = regmap_read(map, OCOTP_MEM0, &val);
>> +               if (ret) {
>> +                       dev_err(&pdev->dev, "failed to read temp grade: %d\n",
>> +                               ret);
>> +                       return ret;
>> +               }
>> +
>> +               if (val == 0 || val == ~0) {
>> +                       dev_err(&pdev->dev, "invalid temp grade data\n");
>> +                       return -EINVAL;
>> +               }
>> +
>> +               /* The maximum die temp is specified by the Temperature Grade */
>> +               switch ((val >> 6) & 0x3) {
>> +               case 0: /* Commercial (0 to 95C) */
>> +                       data->temp_grade = "Commercial";
>> +                       data->temp_critical = 95000;
>> +                       break;
>> +               case 1: /* Extended Commercial (-20 to 105C) */
>> +                       data->temp_grade = "Extended Commercial";
>> +                       data->temp_critical = 105000;
>> +                       break;
>> +               case 2: /* Industrial (-40 to 105C) */
>> +                       data->temp_grade = "Industrial";
>> +                       data->temp_critical = 105000;
>> +                       break;
>> +               case 3: /* Automotive (-40 to 125C) */
>> +                       data->temp_grade = "Automotive";
>> +                       data->temp_critical = 125000;
>> +                       break;
>> +               }
>> +       }
>>
>>         /*
>> -        * The maximum die temperature set to 20 C higher than
>> -        * IMX_TEMP_PASSIVE.
>> +        * Set the default passive cooling trip point to critical - 20C
>> +        * (can be changed from userspace)
>>          */
>> -       data->temp_critical = 1000 * 20 + data->temp_passive;
>> +       data->temp_passive = data->temp_critical - (1000 * 20);
>>
>>         return 0;
>>  }
>> @@ -559,6 +589,10 @@ static int imx_thermal_probe(struct platform_device *pdev)
>>                 return ret;
>>         }
>>
>> +       dev_info(&pdev->dev, "%s CPU temperature grade - "
>> +                "thresholds: crit:%ldC passive:%ldC\n", data->temp_grade,
>> +                data->temp_critical / 1000, data->temp_passive / 1000);
>> +
>>         /* Enable measurements at ~ 10 Hz */
>>         regmap_write(map, TEMPSENSE1 + REG_CLR, TEMPSENSE1_MEASURE_FREQ);
>>         measure_freq = DIV_ROUND_UP(32768, 10); /* 10 Hz */
>> --
>> 1.9.1
>>
>>
>
> I think this is a fine start and we can build off it.
>
> Acked-by: Jon Nettleton <jon@solid-run.com>

adding CC linux-pm at vger.kernel.org

Zhang / Eduardo,

This seems to have gotten missed - can you please review this for merge?

Regards,

Tim

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

end of thread, other threads:[~2015-10-13 14:16 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-04-30 16:02 [PATCH] imx: thermal: use CPU temperature grade info for thresholds Tim Harvey
2015-05-11 12:22 ` Shawn Guo
2015-05-11 12:26 ` Fabio Estevam
2015-05-11 13:31   ` Tim Harvey
2015-05-12  1:32     ` Eduardo Valentin
2015-05-11 18:55       ` Fabio Estevam
2015-05-20 22:08 ` Tim Harvey
2015-05-21 23:45 ` [PATCH v2] " Tim Harvey
2015-05-24  2:48   ` Shawn Guo
2015-05-24  5:19     ` Jon Nettleton
2015-05-26 21:24       ` Tim Harvey
2015-05-27  6:08         ` Jon Nettleton
2015-07-28 14:50           ` Tim Harvey
2015-07-28 15:01             ` Jon Nettleton
2015-07-28 16:10               ` Jon Nettleton
2015-07-28 16:12             ` Jon Nettleton
2015-07-30  7:19 ` [PATCH] " Jon Nettleton
2015-10-13 14:16   ` Tim Harvey
2015-10-13 14:16     ` Tim Harvey

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.