All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v1] thermal: imx: Update critical temp threshold
@ 2022-04-20  9:13 ` Francesco Dolcini
  0 siblings, 0 replies; 16+ messages in thread
From: Francesco Dolcini @ 2022-04-20  9:13 UTC (permalink / raw)
  To: Rafael J . Wysocki, Daniel Lezcano, linux-pm
  Cc: Francesco Dolcini, Amit Kucheria, Shawn Guo, Sascha Hauer,
	Pengutronix Kernel Team, Fabio Estevam, NXP Linux Team,
	linux-arm-kernel, Tim Harvey, Jon Nettleton

Increase the critical temperature threshold to the datasheet defined
value according to the temperature grade of the SoC, increasing the
actual critical temperature value of 5 degrees.

Without this change the emergency shutdown will trigger earlier then
required affecting applications that are expected to be working on this
close to the limit, but yet valid, temperature range.

Signed-off-by: Francesco Dolcini <francesco.dolcini@toradex.com>
---

Not sure if there is an alternative to this patch, the critical threshold seems
to be read-only and it is not possible to just change it from user space that
would be my preferred solution.

According to the original discussion [1] the reasoning was the following:

On Tue, Jul 28, 2015 at 4:50 PM, Tim Harvey <tharvey@gateworks.com> wrote:
> Yes - the purpose of lowering the critical threshold from the hardware
> default is to allow Linux to shutdown more cleanly.

But I do not understand it.

[1] https://lore.kernel.org/all/CAJ+vNU1PQZa9KoCU9o_ws6jAAjhGVJby-1P583SVejT5TrAFTQ@mail.gmail.com/

---
 drivers/thermal/imx_thermal.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/thermal/imx_thermal.c b/drivers/thermal/imx_thermal.c
index 16663373b682..75a631a23e61 100644
--- a/drivers/thermal/imx_thermal.c
+++ b/drivers/thermal/imx_thermal.c
@@ -504,10 +504,10 @@ static void imx_init_temp_grade(struct platform_device *pdev, u32 ocotp_mem0)
 	}
 
 	/*
-	 * Set the critical trip point at 5 °C under max
+	 * Set the critical trip point at max
 	 * Set the passive trip point at 10 °C under max (changeable via sysfs)
 	 */
-	data->temp_critical = data->temp_max - (1000 * 5);
+	data->temp_critical = data->temp_max;
 	data->temp_passive = data->temp_max - (1000 * 10);
 }
 
-- 
2.25.1


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

* [PATCH v1] thermal: imx: Update critical temp threshold
@ 2022-04-20  9:13 ` Francesco Dolcini
  0 siblings, 0 replies; 16+ messages in thread
From: Francesco Dolcini @ 2022-04-20  9:13 UTC (permalink / raw)
  To: Rafael J . Wysocki, Daniel Lezcano, linux-pm
  Cc: Francesco Dolcini, Amit Kucheria, Shawn Guo, Sascha Hauer,
	Pengutronix Kernel Team, Fabio Estevam, NXP Linux Team,
	linux-arm-kernel, Tim Harvey, Jon Nettleton

Increase the critical temperature threshold to the datasheet defined
value according to the temperature grade of the SoC, increasing the
actual critical temperature value of 5 degrees.

Without this change the emergency shutdown will trigger earlier then
required affecting applications that are expected to be working on this
close to the limit, but yet valid, temperature range.

Signed-off-by: Francesco Dolcini <francesco.dolcini@toradex.com>
---

Not sure if there is an alternative to this patch, the critical threshold seems
to be read-only and it is not possible to just change it from user space that
would be my preferred solution.

According to the original discussion [1] the reasoning was the following:

On Tue, Jul 28, 2015 at 4:50 PM, Tim Harvey <tharvey@gateworks.com> wrote:
> Yes - the purpose of lowering the critical threshold from the hardware
> default is to allow Linux to shutdown more cleanly.

But I do not understand it.

[1] https://lore.kernel.org/all/CAJ+vNU1PQZa9KoCU9o_ws6jAAjhGVJby-1P583SVejT5TrAFTQ@mail.gmail.com/

---
 drivers/thermal/imx_thermal.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/thermal/imx_thermal.c b/drivers/thermal/imx_thermal.c
index 16663373b682..75a631a23e61 100644
--- a/drivers/thermal/imx_thermal.c
+++ b/drivers/thermal/imx_thermal.c
@@ -504,10 +504,10 @@ static void imx_init_temp_grade(struct platform_device *pdev, u32 ocotp_mem0)
 	}
 
 	/*
-	 * Set the critical trip point at 5 °C under max
+	 * Set the critical trip point at max
 	 * Set the passive trip point at 10 °C under max (changeable via sysfs)
 	 */
-	data->temp_critical = data->temp_max - (1000 * 5);
+	data->temp_critical = data->temp_max;
 	data->temp_passive = data->temp_max - (1000 * 10);
 }
 
-- 
2.25.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* Re: [PATCH v1] thermal: imx: Update critical temp threshold
  2022-04-20  9:13 ` Francesco Dolcini
@ 2022-05-09  9:55   ` Daniel Lezcano
  -1 siblings, 0 replies; 16+ messages in thread
From: Daniel Lezcano @ 2022-05-09  9:55 UTC (permalink / raw)
  To: Francesco Dolcini, Rafael J . Wysocki, linux-pm
  Cc: Amit Kucheria, Shawn Guo, Sascha Hauer, Pengutronix Kernel Team,
	Fabio Estevam, NXP Linux Team, linux-arm-kernel, Tim Harvey,
	Jon Nettleton

Hi Francesco,


On 20/04/2022 11:13, Francesco Dolcini wrote:
> Increase the critical temperature threshold to the datasheet defined
> value according to the temperature grade of the SoC, increasing the
> actual critical temperature value of 5 degrees.
> 
> Without this change the emergency shutdown will trigger earlier then
> required affecting applications that are expected to be working on this
> close to the limit, but yet valid, temperature range.
> 
> Signed-off-by: Francesco Dolcini <francesco.dolcini@toradex.com>
> ---
> 
> Not sure if there is an alternative to this patch, the critical threshold seems
> to be read-only and it is not possible to just change it from user space that
> would be my preferred solution.
> 
> According to the original discussion [1] the reasoning was the following:
> 
> On Tue, Jul 28, 2015 at 4:50 PM, Tim Harvey <tharvey@gateworks.com> wrote:
>> Yes - the purpose of lowering the critical threshold from the hardware
>> default is to allow Linux to shutdown more cleanly.
> 
> But I do not understand it.

Shawn, Sascha ? any comment ?

> [1] https://lore.kernel.org/all/CAJ+vNU1PQZa9KoCU9o_ws6jAAjhGVJby-1P583SVejT5TrAFTQ@mail.gmail.com/
> 
> ---
>   drivers/thermal/imx_thermal.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/thermal/imx_thermal.c b/drivers/thermal/imx_thermal.c
> index 16663373b682..75a631a23e61 100644
> --- a/drivers/thermal/imx_thermal.c
> +++ b/drivers/thermal/imx_thermal.c
> @@ -504,10 +504,10 @@ static void imx_init_temp_grade(struct platform_device *pdev, u32 ocotp_mem0)
>   	}
>   
>   	/*
> -	 * Set the critical trip point at 5 °C under max
> +	 * Set the critical trip point at max
>   	 * Set the passive trip point at 10 °C under max (changeable via sysfs)
>   	 */
> -	data->temp_critical = data->temp_max - (1000 * 5);
> +	data->temp_critical = data->temp_max;
>   	data->temp_passive = data->temp_max - (1000 * 10);
>   }
>   


-- 
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* Re: [PATCH v1] thermal: imx: Update critical temp threshold
@ 2022-05-09  9:55   ` Daniel Lezcano
  0 siblings, 0 replies; 16+ messages in thread
From: Daniel Lezcano @ 2022-05-09  9:55 UTC (permalink / raw)
  To: Francesco Dolcini, Rafael J . Wysocki, linux-pm
  Cc: Amit Kucheria, Shawn Guo, Sascha Hauer, Pengutronix Kernel Team,
	Fabio Estevam, NXP Linux Team, linux-arm-kernel, Tim Harvey,
	Jon Nettleton

Hi Francesco,


On 20/04/2022 11:13, Francesco Dolcini wrote:
> Increase the critical temperature threshold to the datasheet defined
> value according to the temperature grade of the SoC, increasing the
> actual critical temperature value of 5 degrees.
> 
> Without this change the emergency shutdown will trigger earlier then
> required affecting applications that are expected to be working on this
> close to the limit, but yet valid, temperature range.
> 
> Signed-off-by: Francesco Dolcini <francesco.dolcini@toradex.com>
> ---
> 
> Not sure if there is an alternative to this patch, the critical threshold seems
> to be read-only and it is not possible to just change it from user space that
> would be my preferred solution.
> 
> According to the original discussion [1] the reasoning was the following:
> 
> On Tue, Jul 28, 2015 at 4:50 PM, Tim Harvey <tharvey@gateworks.com> wrote:
>> Yes - the purpose of lowering the critical threshold from the hardware
>> default is to allow Linux to shutdown more cleanly.
> 
> But I do not understand it.

Shawn, Sascha ? any comment ?

> [1] https://lore.kernel.org/all/CAJ+vNU1PQZa9KoCU9o_ws6jAAjhGVJby-1P583SVejT5TrAFTQ@mail.gmail.com/
> 
> ---
>   drivers/thermal/imx_thermal.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/thermal/imx_thermal.c b/drivers/thermal/imx_thermal.c
> index 16663373b682..75a631a23e61 100644
> --- a/drivers/thermal/imx_thermal.c
> +++ b/drivers/thermal/imx_thermal.c
> @@ -504,10 +504,10 @@ static void imx_init_temp_grade(struct platform_device *pdev, u32 ocotp_mem0)
>   	}
>   
>   	/*
> -	 * Set the critical trip point at 5 °C under max
> +	 * Set the critical trip point at max
>   	 * Set the passive trip point at 10 °C under max (changeable via sysfs)
>   	 */
> -	data->temp_critical = data->temp_max - (1000 * 5);
> +	data->temp_critical = data->temp_max;
>   	data->temp_passive = data->temp_max - (1000 * 10);
>   }
>   


-- 
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog

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

* Re: [PATCH v1] thermal: imx: Update critical temp threshold
  2022-05-09  9:55   ` Daniel Lezcano
@ 2022-05-12  7:36     ` Francesco Dolcini
  -1 siblings, 0 replies; 16+ messages in thread
From: Francesco Dolcini @ 2022-05-12  7:36 UTC (permalink / raw)
  To: Daniel Lezcano, Sascha Hauer, Shawn Guo
  Cc: Francesco Dolcini, Rafael J . Wysocki, linux-pm, Amit Kucheria,
	Pengutronix Kernel Team, Fabio Estevam, NXP Linux Team,
	linux-arm-kernel, Tim Harvey, Jon Nettleton

Hello Daniel, Sasha, Shawn and all

On Mon, May 09, 2022 at 11:55:20AM +0200, Daniel Lezcano wrote:
> On 20/04/2022 11:13, Francesco Dolcini wrote:
> > Increase the critical temperature threshold to the datasheet defined
> > value according to the temperature grade of the SoC, increasing the
> > actual critical temperature value of 5 degrees.
> > 
> > Without this change the emergency shutdown will trigger earlier then
> > required affecting applications that are expected to be working on this
> > close to the limit, but yet valid, temperature range.
> > 
> > Signed-off-by: Francesco Dolcini <francesco.dolcini@toradex.com>
> > ---
> > 
> > Not sure if there is an alternative to this patch, the critical threshold seems
> > to be read-only and it is not possible to just change it from user space that
> > would be my preferred solution.
> > 
> > According to the original discussion [1] the reasoning was the following:
> > 
> > On Tue, Jul 28, 2015 at 4:50 PM, Tim Harvey <tharvey@gateworks.com> wrote:
> > > Yes - the purpose of lowering the critical threshold from the hardware
> > > default is to allow Linux to shutdown more cleanly.
> > 
> > But I do not understand it.
> 
> Shawn, Sascha ? any comment ?

Just one small addition, we (Toradex) are using this modified critical
threshold since quite some time, on multiple i.MX[67]* SOC, and we
regularly run stress tests on commercial/IT part on the whole
temperature working range (ambient temperature up to 85 degrees for IT
modules) in climate chambers and I'm not aware of any issue reported
because of that (indeed, it is the other way around, without this change
we had issues).

Francesco


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* Re: [PATCH v1] thermal: imx: Update critical temp threshold
@ 2022-05-12  7:36     ` Francesco Dolcini
  0 siblings, 0 replies; 16+ messages in thread
From: Francesco Dolcini @ 2022-05-12  7:36 UTC (permalink / raw)
  To: Daniel Lezcano, Sascha Hauer, Shawn Guo
  Cc: Francesco Dolcini, Rafael J . Wysocki, linux-pm, Amit Kucheria,
	Pengutronix Kernel Team, Fabio Estevam, NXP Linux Team,
	linux-arm-kernel, Tim Harvey, Jon Nettleton

Hello Daniel, Sasha, Shawn and all

On Mon, May 09, 2022 at 11:55:20AM +0200, Daniel Lezcano wrote:
> On 20/04/2022 11:13, Francesco Dolcini wrote:
> > Increase the critical temperature threshold to the datasheet defined
> > value according to the temperature grade of the SoC, increasing the
> > actual critical temperature value of 5 degrees.
> > 
> > Without this change the emergency shutdown will trigger earlier then
> > required affecting applications that are expected to be working on this
> > close to the limit, but yet valid, temperature range.
> > 
> > Signed-off-by: Francesco Dolcini <francesco.dolcini@toradex.com>
> > ---
> > 
> > Not sure if there is an alternative to this patch, the critical threshold seems
> > to be read-only and it is not possible to just change it from user space that
> > would be my preferred solution.
> > 
> > According to the original discussion [1] the reasoning was the following:
> > 
> > On Tue, Jul 28, 2015 at 4:50 PM, Tim Harvey <tharvey@gateworks.com> wrote:
> > > Yes - the purpose of lowering the critical threshold from the hardware
> > > default is to allow Linux to shutdown more cleanly.
> > 
> > But I do not understand it.
> 
> Shawn, Sascha ? any comment ?

Just one small addition, we (Toradex) are using this modified critical
threshold since quite some time, on multiple i.MX[67]* SOC, and we
regularly run stress tests on commercial/IT part on the whole
temperature working range (ambient temperature up to 85 degrees for IT
modules) in climate chambers and I'm not aware of any issue reported
because of that (indeed, it is the other way around, without this change
we had issues).

Francesco


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

* Re: [PATCH v1] thermal: imx: Update critical temp threshold
  2022-05-12  7:36     ` Francesco Dolcini
@ 2022-05-12 10:08       ` Lucas Stach
  -1 siblings, 0 replies; 16+ messages in thread
From: Lucas Stach @ 2022-05-12 10:08 UTC (permalink / raw)
  To: Francesco Dolcini, Daniel Lezcano, Sascha Hauer, Shawn Guo
  Cc: linux-pm, Tim Harvey, Amit Kucheria, Jon Nettleton,
	NXP Linux Team, Pengutronix Kernel Team, Rafael J . Wysocki,
	Fabio Estevam, linux-arm-kernel

Am Donnerstag, dem 12.05.2022 um 09:36 +0200 schrieb Francesco Dolcini:
> Hello Daniel, Sasha, Shawn and all
> 
> On Mon, May 09, 2022 at 11:55:20AM +0200, Daniel Lezcano wrote:
> > On 20/04/2022 11:13, Francesco Dolcini wrote:
> > > Increase the critical temperature threshold to the datasheet defined
> > > value according to the temperature grade of the SoC, increasing the
> > > actual critical temperature value of 5 degrees.
> > > 
> > > Without this change the emergency shutdown will trigger earlier then
> > > required affecting applications that are expected to be working on this
> > > close to the limit, but yet valid, temperature range.
> > > 
> > > Signed-off-by: Francesco Dolcini <francesco.dolcini@toradex.com>
> > > ---
> > > 
> > > Not sure if there is an alternative to this patch, the critical threshold seems
> > > to be read-only and it is not possible to just change it from user space that
> > > would be my preferred solution.
> > > 
> > > According to the original discussion [1] the reasoning was the following:
> > > 
> > > On Tue, Jul 28, 2015 at 4:50 PM, Tim Harvey <tharvey@gateworks.com> wrote:
> > > > Yes - the purpose of lowering the critical threshold from the hardware
> > > > default is to allow Linux to shutdown more cleanly.
> > > 
> > > But I do not understand it.
> > 
> > Shawn, Sascha ? any comment ?
> 
> Just one small addition, we (Toradex) are using this modified critical
> threshold since quite some time, on multiple i.MX[67]* SOC, and we
> regularly run stress tests on commercial/IT part on the whole
> temperature working range (ambient temperature up to 85 degrees for IT
> modules) in climate chambers and I'm not aware of any issue reported
> because of that (indeed, it is the other way around, without this change
> we had issues).

That is really an overall system design issue. Most chips will probably
work fine when going over the critical temperature, as this is mostly
set due to device lifetime constraints, not because the chip fails at
this temperature. However the chip is only guaranteed to work at up to
the critical temperature, so one could argue that starting a orderly
shutdown when the critical temperature is reached is already too late,
as the temperature may rise further during the time taken to shut down
the system. Also device leakage increases a lot at those critical
temperatures, so the system may fail not because the chip is
malfunctioning, but the board power supply may not be able to supply
the increased current required.

Really I think there is no right or wrong here. I believe that this
needs to be up to the system integrator, so the critical temperature
should be writable by userspace in the constraints set by the fuses.

Regards,
Lucas


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

* Re: [PATCH v1] thermal: imx: Update critical temp threshold
@ 2022-05-12 10:08       ` Lucas Stach
  0 siblings, 0 replies; 16+ messages in thread
From: Lucas Stach @ 2022-05-12 10:08 UTC (permalink / raw)
  To: Francesco Dolcini, Daniel Lezcano, Sascha Hauer, Shawn Guo
  Cc: linux-pm, Tim Harvey, Amit Kucheria, Jon Nettleton,
	NXP Linux Team, Pengutronix Kernel Team, Rafael J . Wysocki,
	Fabio Estevam, linux-arm-kernel

Am Donnerstag, dem 12.05.2022 um 09:36 +0200 schrieb Francesco Dolcini:
> Hello Daniel, Sasha, Shawn and all
> 
> On Mon, May 09, 2022 at 11:55:20AM +0200, Daniel Lezcano wrote:
> > On 20/04/2022 11:13, Francesco Dolcini wrote:
> > > Increase the critical temperature threshold to the datasheet defined
> > > value according to the temperature grade of the SoC, increasing the
> > > actual critical temperature value of 5 degrees.
> > > 
> > > Without this change the emergency shutdown will trigger earlier then
> > > required affecting applications that are expected to be working on this
> > > close to the limit, but yet valid, temperature range.
> > > 
> > > Signed-off-by: Francesco Dolcini <francesco.dolcini@toradex.com>
> > > ---
> > > 
> > > Not sure if there is an alternative to this patch, the critical threshold seems
> > > to be read-only and it is not possible to just change it from user space that
> > > would be my preferred solution.
> > > 
> > > According to the original discussion [1] the reasoning was the following:
> > > 
> > > On Tue, Jul 28, 2015 at 4:50 PM, Tim Harvey <tharvey@gateworks.com> wrote:
> > > > Yes - the purpose of lowering the critical threshold from the hardware
> > > > default is to allow Linux to shutdown more cleanly.
> > > 
> > > But I do not understand it.
> > 
> > Shawn, Sascha ? any comment ?
> 
> Just one small addition, we (Toradex) are using this modified critical
> threshold since quite some time, on multiple i.MX[67]* SOC, and we
> regularly run stress tests on commercial/IT part on the whole
> temperature working range (ambient temperature up to 85 degrees for IT
> modules) in climate chambers and I'm not aware of any issue reported
> because of that (indeed, it is the other way around, without this change
> we had issues).

That is really an overall system design issue. Most chips will probably
work fine when going over the critical temperature, as this is mostly
set due to device lifetime constraints, not because the chip fails at
this temperature. However the chip is only guaranteed to work at up to
the critical temperature, so one could argue that starting a orderly
shutdown when the critical temperature is reached is already too late,
as the temperature may rise further during the time taken to shut down
the system. Also device leakage increases a lot at those critical
temperatures, so the system may fail not because the chip is
malfunctioning, but the board power supply may not be able to supply
the increased current required.

Really I think there is no right or wrong here. I believe that this
needs to be up to the system integrator, so the critical temperature
should be writable by userspace in the constraints set by the fuses.

Regards,
Lucas


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* Re: [PATCH v1] thermal: imx: Update critical temp threshold
  2022-05-12 10:08       ` Lucas Stach
@ 2022-05-12 10:24         ` Francesco Dolcini
  -1 siblings, 0 replies; 16+ messages in thread
From: Francesco Dolcini @ 2022-05-12 10:24 UTC (permalink / raw)
  To: Lucas Stach
  Cc: Francesco Dolcini, Daniel Lezcano, Sascha Hauer, Shawn Guo,
	linux-pm, Tim Harvey, Amit Kucheria, Jon Nettleton,
	NXP Linux Team, Pengutronix Kernel Team, Rafael J . Wysocki,
	Fabio Estevam, linux-arm-kernel

Hello Lucas,

On Thu, May 12, 2022 at 12:08:08PM +0200, Lucas Stach wrote:
> Am Donnerstag, dem 12.05.2022 um 09:36 +0200 schrieb Francesco Dolcini:
> > Hello Daniel, Sasha, Shawn and all
> > 
> > On Mon, May 09, 2022 at 11:55:20AM +0200, Daniel Lezcano wrote:
> > > On 20/04/2022 11:13, Francesco Dolcini wrote:
> > > > Increase the critical temperature threshold to the datasheet defined
> > > > value according to the temperature grade of the SoC, increasing the
> > > > actual critical temperature value of 5 degrees.
> > > > 
> > > > Without this change the emergency shutdown will trigger earlier then
> > > > required affecting applications that are expected to be working on this
> > > > close to the limit, but yet valid, temperature range.
> > > > 
> > > > Signed-off-by: Francesco Dolcini <francesco.dolcini@toradex.com>
> > > > ---
> > > > 
> > > > Not sure if there is an alternative to this patch, the critical threshold seems
> > > > to be read-only and it is not possible to just change it from user space that
> > > > would be my preferred solution.
> > > > 
> > > > According to the original discussion [1] the reasoning was the following:
> > > > 
> > > > On Tue, Jul 28, 2015 at 4:50 PM, Tim Harvey <tharvey@gateworks.com> wrote:
> > > > > Yes - the purpose of lowering the critical threshold from the hardware
> > > > > default is to allow Linux to shutdown more cleanly.
> > > > 
> > > > But I do not understand it.
> > > 
> > > Shawn, Sascha ? any comment ?
> > 
> > Just one small addition, we (Toradex) are using this modified critical
> > threshold since quite some time, on multiple i.MX[67]* SOC, and we
> > regularly run stress tests on commercial/IT part on the whole
> > temperature working range (ambient temperature up to 85 degrees for IT
> > modules) in climate chambers and I'm not aware of any issue reported
> > because of that (indeed, it is the other way around, without this change
> > we had issues).
> 
> That is really an overall system design issue. Most chips will probably
> work fine when going over the critical temperature, as this is mostly
> set due to device lifetime constraints, not because the chip fails at
> this temperature. However the chip is only guaranteed to work at up to
> the critical temperature, so one could argue that starting a orderly
> shutdown when the critical temperature is reached is already too late,
> as the temperature may rise further during the time taken to shut down
> the system. Also device leakage increases a lot at those critical
> temperatures, so the system may fail not because the chip is
> malfunctioning, but the board power supply may not be able to supply
> the increased current required.
> 
> Really I think there is no right or wrong here. I believe that this
> needs to be up to the system integrator, so the critical temperature
> should be writable by userspace in the constraints set by the fuses.

I agree 95% with you. The 5% I do not agree is that the final system
integrator should be allowed to go even above the fuses constraints.
Sometime is better to take the chance of burning a chip than shutting
the system down.

Anyway, would it be fine to have a patch that make the critical
threshold write-able (in my initial message I mentioned this as my
preferred solution also)? If anybody has a pointer on how
to do it, it would be great, I'm not familiar with that code.

Francesco


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

* Re: [PATCH v1] thermal: imx: Update critical temp threshold
@ 2022-05-12 10:24         ` Francesco Dolcini
  0 siblings, 0 replies; 16+ messages in thread
From: Francesco Dolcini @ 2022-05-12 10:24 UTC (permalink / raw)
  To: Lucas Stach
  Cc: Francesco Dolcini, Daniel Lezcano, Sascha Hauer, Shawn Guo,
	linux-pm, Tim Harvey, Amit Kucheria, Jon Nettleton,
	NXP Linux Team, Pengutronix Kernel Team, Rafael J . Wysocki,
	Fabio Estevam, linux-arm-kernel

Hello Lucas,

On Thu, May 12, 2022 at 12:08:08PM +0200, Lucas Stach wrote:
> Am Donnerstag, dem 12.05.2022 um 09:36 +0200 schrieb Francesco Dolcini:
> > Hello Daniel, Sasha, Shawn and all
> > 
> > On Mon, May 09, 2022 at 11:55:20AM +0200, Daniel Lezcano wrote:
> > > On 20/04/2022 11:13, Francesco Dolcini wrote:
> > > > Increase the critical temperature threshold to the datasheet defined
> > > > value according to the temperature grade of the SoC, increasing the
> > > > actual critical temperature value of 5 degrees.
> > > > 
> > > > Without this change the emergency shutdown will trigger earlier then
> > > > required affecting applications that are expected to be working on this
> > > > close to the limit, but yet valid, temperature range.
> > > > 
> > > > Signed-off-by: Francesco Dolcini <francesco.dolcini@toradex.com>
> > > > ---
> > > > 
> > > > Not sure if there is an alternative to this patch, the critical threshold seems
> > > > to be read-only and it is not possible to just change it from user space that
> > > > would be my preferred solution.
> > > > 
> > > > According to the original discussion [1] the reasoning was the following:
> > > > 
> > > > On Tue, Jul 28, 2015 at 4:50 PM, Tim Harvey <tharvey@gateworks.com> wrote:
> > > > > Yes - the purpose of lowering the critical threshold from the hardware
> > > > > default is to allow Linux to shutdown more cleanly.
> > > > 
> > > > But I do not understand it.
> > > 
> > > Shawn, Sascha ? any comment ?
> > 
> > Just one small addition, we (Toradex) are using this modified critical
> > threshold since quite some time, on multiple i.MX[67]* SOC, and we
> > regularly run stress tests on commercial/IT part on the whole
> > temperature working range (ambient temperature up to 85 degrees for IT
> > modules) in climate chambers and I'm not aware of any issue reported
> > because of that (indeed, it is the other way around, without this change
> > we had issues).
> 
> That is really an overall system design issue. Most chips will probably
> work fine when going over the critical temperature, as this is mostly
> set due to device lifetime constraints, not because the chip fails at
> this temperature. However the chip is only guaranteed to work at up to
> the critical temperature, so one could argue that starting a orderly
> shutdown when the critical temperature is reached is already too late,
> as the temperature may rise further during the time taken to shut down
> the system. Also device leakage increases a lot at those critical
> temperatures, so the system may fail not because the chip is
> malfunctioning, but the board power supply may not be able to supply
> the increased current required.
> 
> Really I think there is no right or wrong here. I believe that this
> needs to be up to the system integrator, so the critical temperature
> should be writable by userspace in the constraints set by the fuses.

I agree 95% with you. The 5% I do not agree is that the final system
integrator should be allowed to go even above the fuses constraints.
Sometime is better to take the chance of burning a chip than shutting
the system down.

Anyway, would it be fine to have a patch that make the critical
threshold write-able (in my initial message I mentioned this as my
preferred solution also)? If anybody has a pointer on how
to do it, it would be great, I'm not familiar with that code.

Francesco


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* Re: [PATCH v1] thermal: imx: Update critical temp threshold
  2022-05-12 10:24         ` Francesco Dolcini
@ 2022-05-12 10:52           ` Daniel Lezcano
  -1 siblings, 0 replies; 16+ messages in thread
From: Daniel Lezcano @ 2022-05-12 10:52 UTC (permalink / raw)
  To: Francesco Dolcini, Lucas Stach
  Cc: Sascha Hauer, Shawn Guo, linux-pm, Tim Harvey, Amit Kucheria,
	Jon Nettleton, NXP Linux Team, Pengutronix Kernel Team,
	Rafael J . Wysocki, Fabio Estevam, linux-arm-kernel

On 12/05/2022 12:24, Francesco Dolcini wrote:
> Hello Lucas,
> 
> On Thu, May 12, 2022 at 12:08:08PM +0200, Lucas Stach wrote:
>> Am Donnerstag, dem 12.05.2022 um 09:36 +0200 schrieb Francesco Dolcini:

[ ... ]

> Anyway, would it be fine to have a patch that make the critical
> threshold write-able (in my initial message I mentioned this as my
> preferred solution also)? If anybody has a pointer on how
> to do it, it would be great, I'm not familiar with that code.

What about a module param ?



-- 
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog

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

* Re: [PATCH v1] thermal: imx: Update critical temp threshold
@ 2022-05-12 10:52           ` Daniel Lezcano
  0 siblings, 0 replies; 16+ messages in thread
From: Daniel Lezcano @ 2022-05-12 10:52 UTC (permalink / raw)
  To: Francesco Dolcini, Lucas Stach
  Cc: Sascha Hauer, Shawn Guo, linux-pm, Tim Harvey, Amit Kucheria,
	Jon Nettleton, NXP Linux Team, Pengutronix Kernel Team,
	Rafael J . Wysocki, Fabio Estevam, linux-arm-kernel

On 12/05/2022 12:24, Francesco Dolcini wrote:
> Hello Lucas,
> 
> On Thu, May 12, 2022 at 12:08:08PM +0200, Lucas Stach wrote:
>> Am Donnerstag, dem 12.05.2022 um 09:36 +0200 schrieb Francesco Dolcini:

[ ... ]

> Anyway, would it be fine to have a patch that make the critical
> threshold write-able (in my initial message I mentioned this as my
> preferred solution also)? If anybody has a pointer on how
> to do it, it would be great, I'm not familiar with that code.

What about a module param ?



-- 
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* Re: [PATCH v1] thermal: imx: Update critical temp threshold
  2022-05-12 10:52           ` Daniel Lezcano
@ 2022-05-12 13:56             ` Francesco Dolcini
  -1 siblings, 0 replies; 16+ messages in thread
From: Francesco Dolcini @ 2022-05-12 13:56 UTC (permalink / raw)
  To: Daniel Lezcano
  Cc: Francesco Dolcini, Lucas Stach, Sascha Hauer, Shawn Guo,
	linux-pm, Tim Harvey, Amit Kucheria, Jon Nettleton,
	NXP Linux Team, Pengutronix Kernel Team, Rafael J . Wysocki,
	Fabio Estevam, linux-arm-kernel

On Thu, May 12, 2022 at 12:52:46PM +0200, Daniel Lezcano wrote:
> On 12/05/2022 12:24, Francesco Dolcini wrote:
> > Hello Lucas,
> > 
> > On Thu, May 12, 2022 at 12:08:08PM +0200, Lucas Stach wrote:
> > > Am Donnerstag, dem 12.05.2022 um 09:36 +0200 schrieb Francesco Dolcini:
> 
> [ ... ]
> 
> > Anyway, would it be fine to have a patch that make the critical
> > threshold write-able (in my initial message I mentioned this as my
> > preferred solution also)? If anybody has a pointer on how
> > to do it, it would be great, I'm not familiar with that code.
> 
> What about a module param ?

I would be happy to just be able to write to the `critical`
trip_point_temp file in sysfs, however the thermal
framework seems to enforce the critical threshold being read only (only
`get_trip_temp` callback available). Is there any way to change this? 

# cat /sys/class/thermal/thermal_zone0/trip_point_1_type
critical
# ls -l /sys/class/thermal/thermal_zone0/trip_point_1_temp
-r--r--r-- 1 root root 4096 May  6 14:33 /sys/class/thermal/thermal_zone0/trip_point_1_temp

Francesco


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

* Re: [PATCH v1] thermal: imx: Update critical temp threshold
@ 2022-05-12 13:56             ` Francesco Dolcini
  0 siblings, 0 replies; 16+ messages in thread
From: Francesco Dolcini @ 2022-05-12 13:56 UTC (permalink / raw)
  To: Daniel Lezcano
  Cc: Francesco Dolcini, Lucas Stach, Sascha Hauer, Shawn Guo,
	linux-pm, Tim Harvey, Amit Kucheria, Jon Nettleton,
	NXP Linux Team, Pengutronix Kernel Team, Rafael J . Wysocki,
	Fabio Estevam, linux-arm-kernel

On Thu, May 12, 2022 at 12:52:46PM +0200, Daniel Lezcano wrote:
> On 12/05/2022 12:24, Francesco Dolcini wrote:
> > Hello Lucas,
> > 
> > On Thu, May 12, 2022 at 12:08:08PM +0200, Lucas Stach wrote:
> > > Am Donnerstag, dem 12.05.2022 um 09:36 +0200 schrieb Francesco Dolcini:
> 
> [ ... ]
> 
> > Anyway, would it be fine to have a patch that make the critical
> > threshold write-able (in my initial message I mentioned this as my
> > preferred solution also)? If anybody has a pointer on how
> > to do it, it would be great, I'm not familiar with that code.
> 
> What about a module param ?

I would be happy to just be able to write to the `critical`
trip_point_temp file in sysfs, however the thermal
framework seems to enforce the critical threshold being read only (only
`get_trip_temp` callback available). Is there any way to change this? 

# cat /sys/class/thermal/thermal_zone0/trip_point_1_type
critical
# ls -l /sys/class/thermal/thermal_zone0/trip_point_1_temp
-r--r--r-- 1 root root 4096 May  6 14:33 /sys/class/thermal/thermal_zone0/trip_point_1_temp

Francesco


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* Re: [PATCH v1] thermal: imx: Update critical temp threshold
  2022-05-12 13:56             ` Francesco Dolcini
@ 2022-05-13 16:25               ` Daniel Lezcano
  -1 siblings, 0 replies; 16+ messages in thread
From: Daniel Lezcano @ 2022-05-13 16:25 UTC (permalink / raw)
  To: Francesco Dolcini
  Cc: Lucas Stach, Sascha Hauer, Shawn Guo, linux-pm, Tim Harvey,
	Amit Kucheria, Jon Nettleton, NXP Linux Team,
	Pengutronix Kernel Team, Rafael J . Wysocki, Fabio Estevam,
	linux-arm-kernel

On 12/05/2022 15:56, Francesco Dolcini wrote:
> On Thu, May 12, 2022 at 12:52:46PM +0200, Daniel Lezcano wrote:
>> On 12/05/2022 12:24, Francesco Dolcini wrote:
>>> Hello Lucas,
>>>
>>> On Thu, May 12, 2022 at 12:08:08PM +0200, Lucas Stach wrote:
>>>> Am Donnerstag, dem 12.05.2022 um 09:36 +0200 schrieb Francesco Dolcini:
>>
>> [ ... ]
>>
>>> Anyway, would it be fine to have a patch that make the critical
>>> threshold write-able (in my initial message I mentioned this as my
>>> preferred solution also)? If anybody has a pointer on how
>>> to do it, it would be great, I'm not familiar with that code.
>>
>> What about a module param ?
> 
> I would be happy to just be able to write to the `critical`
> trip_point_temp file in sysfs, however the thermal
> framework seems to enforce the critical threshold being read only (only
> `get_trip_temp` callback available). Is there any way to change this?

No, critical trip point is not a system setting but a hardware setting.

It should be set via the DT or a module parameter.


> # cat /sys/class/thermal/thermal_zone0/trip_point_1_type
> critical
> # ls -l /sys/class/thermal/thermal_zone0/trip_point_1_temp
> -r--r--r-- 1 root root 4096 May  6 14:33 /sys/class/thermal/thermal_zone0/trip_point_1_temp





-- 
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog

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

* Re: [PATCH v1] thermal: imx: Update critical temp threshold
@ 2022-05-13 16:25               ` Daniel Lezcano
  0 siblings, 0 replies; 16+ messages in thread
From: Daniel Lezcano @ 2022-05-13 16:25 UTC (permalink / raw)
  To: Francesco Dolcini
  Cc: Lucas Stach, Sascha Hauer, Shawn Guo, linux-pm, Tim Harvey,
	Amit Kucheria, Jon Nettleton, NXP Linux Team,
	Pengutronix Kernel Team, Rafael J . Wysocki, Fabio Estevam,
	linux-arm-kernel

On 12/05/2022 15:56, Francesco Dolcini wrote:
> On Thu, May 12, 2022 at 12:52:46PM +0200, Daniel Lezcano wrote:
>> On 12/05/2022 12:24, Francesco Dolcini wrote:
>>> Hello Lucas,
>>>
>>> On Thu, May 12, 2022 at 12:08:08PM +0200, Lucas Stach wrote:
>>>> Am Donnerstag, dem 12.05.2022 um 09:36 +0200 schrieb Francesco Dolcini:
>>
>> [ ... ]
>>
>>> Anyway, would it be fine to have a patch that make the critical
>>> threshold write-able (in my initial message I mentioned this as my
>>> preferred solution also)? If anybody has a pointer on how
>>> to do it, it would be great, I'm not familiar with that code.
>>
>> What about a module param ?
> 
> I would be happy to just be able to write to the `critical`
> trip_point_temp file in sysfs, however the thermal
> framework seems to enforce the critical threshold being read only (only
> `get_trip_temp` callback available). Is there any way to change this?

No, critical trip point is not a system setting but a hardware setting.

It should be set via the DT or a module parameter.


> # cat /sys/class/thermal/thermal_zone0/trip_point_1_type
> critical
> # ls -l /sys/class/thermal/thermal_zone0/trip_point_1_temp
> -r--r--r-- 1 root root 4096 May  6 14:33 /sys/class/thermal/thermal_zone0/trip_point_1_temp





-- 
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

end of thread, other threads:[~2022-05-13 16:26 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-04-20  9:13 [PATCH v1] thermal: imx: Update critical temp threshold Francesco Dolcini
2022-04-20  9:13 ` Francesco Dolcini
2022-05-09  9:55 ` Daniel Lezcano
2022-05-09  9:55   ` Daniel Lezcano
2022-05-12  7:36   ` Francesco Dolcini
2022-05-12  7:36     ` Francesco Dolcini
2022-05-12 10:08     ` Lucas Stach
2022-05-12 10:08       ` Lucas Stach
2022-05-12 10:24       ` Francesco Dolcini
2022-05-12 10:24         ` Francesco Dolcini
2022-05-12 10:52         ` Daniel Lezcano
2022-05-12 10:52           ` Daniel Lezcano
2022-05-12 13:56           ` Francesco Dolcini
2022-05-12 13:56             ` Francesco Dolcini
2022-05-13 16:25             ` Daniel Lezcano
2022-05-13 16:25               ` Daniel Lezcano

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.