| Message ID | 4822145.GXAFRqVoOG@kreacher |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [v3,1/8] thermal: core: Add mechanism for connecting trips with driver data | expand |
On 25/07/2023 14:06, Rafael J. Wysocki wrote: > From: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > > 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 <rafael.j.wysocki@intel.com> > --- > > 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)
On Tue, Aug 1, 2023 at 8:29 PM Daniel Lezcano <daniel.lezcano@linaro.org> wrote: > > On 25/07/2023 14:06, Rafael J. Wysocki wrote: > > From: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > > > > 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 <rafael.j.wysocki@intel.com> > > --- > > > > 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.
On 01/08/2023 21:05, Rafael J. Wysocki wrote: > On Tue, Aug 1, 2023 at 8:29 PM Daniel Lezcano <daniel.lezcano@linaro.org> wrote: >> >> On 25/07/2023 14:06, Rafael J. Wysocki wrote: >>> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com> >>> >>> 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 <rafael.j.wysocki@intel.com> >>> --- >>> >>> 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.
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);