2023-07-25 12:40:38

by Rafael J. Wysocki

[permalink] [raw]
Subject: [PATCH v3 2/8] thermal: core: Do not handle trip points with invalid temperature

From: Rafael J. Wysocki <[email protected]>

Trip points with temperature set to THERMAL_TEMP_INVALID are as good as
disabled, so make handle_thermal_trip() ignore them.

Signed-off-by: Rafael J. Wysocki <[email protected]>
---

v2 -> v3: No changes.

v1 -> v2: No changes.

---
drivers/thermal/thermal_core.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)

Index: linux-pm/drivers/thermal/thermal_core.c
===================================================================
--- linux-pm.orig/drivers/thermal/thermal_core.c
+++ linux-pm/drivers/thermal/thermal_core.c
@@ -348,7 +348,8 @@ static void handle_thermal_trip(struct t
struct thermal_trip trip;

/* Ignore disabled trip points */
- if (test_bit(trip_id, &tz->trips_disabled))
+ if (test_bit(trip_id, &tz->trips_disabled) ||
+ trip.temperature == THERMAL_TEMP_INVALID)
return;

__thermal_zone_get_trip(tz, trip_id, &trip);





2023-08-01 19:07:36

by Daniel Lezcano

[permalink] [raw]
Subject: Re: [PATCH v3 2/8] thermal: core: Do not handle trip points with invalid temperature

On 25/07/2023 14:06, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <[email protected]>
>
> Trip points with temperature set to THERMAL_TEMP_INVALID are as good as
> disabled, so make handle_thermal_trip() ignore them.
>
> Signed-off-by: Rafael J. Wysocki <[email protected]>
> ---
>
> v2 -> v3: No changes.
>
> v1 -> v2: No changes.
>
> ---
> drivers/thermal/thermal_core.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> Index: linux-pm/drivers/thermal/thermal_core.c
> ===================================================================
> --- linux-pm.orig/drivers/thermal/thermal_core.c
> +++ linux-pm/drivers/thermal/thermal_core.c
> @@ -348,7 +348,8 @@ static void handle_thermal_trip(struct t
> struct thermal_trip trip;
>
> /* Ignore disabled trip points */
> - if (test_bit(trip_id, &tz->trips_disabled))
> + if (test_bit(trip_id, &tz->trips_disabled) ||
> + trip.temperature == THERMAL_TEMP_INVALID)
> return;

This will set the temperature to THERMAL_TEMP_INVALID at each thermal
zone update.

It would make more sense to set it when setting the disabled bit at init
time, no?

But is that something we really want to do ? The trip point will be
reordered due to the temperature change (-273°C)

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


2023-08-01 19:28:15

by Rafael J. Wysocki

[permalink] [raw]
Subject: Re: [PATCH v3 2/8] thermal: core: Do not handle trip points with invalid temperature

On Tue, Aug 1, 2023 at 8:29 PM Daniel Lezcano <[email protected]> wrote:
>
> On 25/07/2023 14:06, Rafael J. Wysocki wrote:
> > From: Rafael J. Wysocki <[email protected]>
> >
> > Trip points with temperature set to THERMAL_TEMP_INVALID are as good as
> > disabled, so make handle_thermal_trip() ignore them.
> >
> > Signed-off-by: Rafael J. Wysocki <[email protected]>
> > ---
> >
> > v2 -> v3: No changes.
> >
> > v1 -> v2: No changes.
> >
> > ---
> > drivers/thermal/thermal_core.c | 3 ++-
> > 1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > Index: linux-pm/drivers/thermal/thermal_core.c
> > ===================================================================
> > --- linux-pm.orig/drivers/thermal/thermal_core.c
> > +++ linux-pm/drivers/thermal/thermal_core.c
> > @@ -348,7 +348,8 @@ static void handle_thermal_trip(struct t
> > struct thermal_trip trip;
> >
> > /* Ignore disabled trip points */
> > - if (test_bit(trip_id, &tz->trips_disabled))
> > + if (test_bit(trip_id, &tz->trips_disabled) ||
> > + trip.temperature == THERMAL_TEMP_INVALID)
> > return;
>
> This will set the temperature to THERMAL_TEMP_INVALID at each thermal
> zone update.

What do you mean?

It doesn't set anything.

> It would make more sense to set it when setting the disabled bit at init
> time, no?
>
> But is that something we really want to do ? The trip point will be
> reordered due to the temperature change (-273°C)

Again, I'm not sure what you mean.

2023-08-01 21:25:30

by Daniel Lezcano

[permalink] [raw]
Subject: Re: [PATCH v3 2/8] thermal: core: Do not handle trip points with invalid temperature

On 01/08/2023 21:05, Rafael J. Wysocki wrote:
> On Tue, Aug 1, 2023 at 8:29 PM Daniel Lezcano <[email protected]> wrote:
>>
>> On 25/07/2023 14:06, Rafael J. Wysocki wrote:
>>> From: Rafael J. Wysocki <[email protected]>
>>>
>>> Trip points with temperature set to THERMAL_TEMP_INVALID are as good as
>>> disabled, so make handle_thermal_trip() ignore them.
>>>
>>> Signed-off-by: Rafael J. Wysocki <[email protected]>
>>> ---
>>>
>>> v2 -> v3: No changes.
>>>
>>> v1 -> v2: No changes.
>>>
>>> ---
>>> drivers/thermal/thermal_core.c | 3 ++-
>>> 1 file changed, 2 insertions(+), 1 deletion(-)
>>>
>>> Index: linux-pm/drivers/thermal/thermal_core.c
>>> ===================================================================
>>> --- linux-pm.orig/drivers/thermal/thermal_core.c
>>> +++ linux-pm/drivers/thermal/thermal_core.c
>>> @@ -348,7 +348,8 @@ static void handle_thermal_trip(struct t
>>> struct thermal_trip trip;
>>>
>>> /* Ignore disabled trip points */
>>> - if (test_bit(trip_id, &tz->trips_disabled))
>>> + if (test_bit(trip_id, &tz->trips_disabled) ||
>>> + trip.temperature == THERMAL_TEMP_INVALID)
>>> return;
>>
>> This will set the temperature to THERMAL_TEMP_INVALID at each thermal
>> zone update.
>
> What do you mean?
>
> It doesn't set anything.

Oh never mind, I read '=' not '=='

>> It would make more sense to set it when setting the disabled bit at init
>> time, no?
>>
>> But is that something we really want to do ? The trip point will be
>> reordered due to the temperature change (-273°C)
>
> Again, I'm not sure what you mean.

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