| Message ID | 12296181.O9o76ZdvQC@kreacher (mailing list archive) |
|---|---|
| State | Mainlined, archived |
| Headers | show |
| Series | [v1] thermal: core: Drop trips_disabled bitmask | expand |
On Tue, Sep 19, 2023 at 08:54:37PM +0200, Rafael J. Wysocki wrote: > From: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > > After recent changes, thermal_zone_get_trip() cannot fail, as invoked > from thermal_zone_device_register_with_trips(), so the only role of > the trips_disabled bitmask is struct thermal_zone_device is to make > handle_thermal_trip() skip trip points whose temperature was initially > zero. However, since the unit of temperature in the thermal core is > millicelsius, zero may very well be a valid temperature value at least > in some usage scenarios and the trip temperature may as well change > later. Thus there is no reason to permanently disable trip points > with initial temperature equal to zero. > > Accordingly, drop the trips_disabled bitmask along with the code > related to it. > > Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com> I guess I was copied because of commit f1b80a3878b2 ("thermal: core: Restore behavior regarding invalid trip points"). Since then we stopped relying on this behavior with commit 5601ef91fba8 ("mlxsw: core_thermal: Use static trip points for transceiver modules"). Tested your patch and didn't see any regressions: Tested-by: Ido Schimmel <idosch@nvidia.com> Thanks
On Wed, Sep 20, 2023 at 10:12 AM Ido Schimmel <idosch@nvidia.com> wrote: > > On Tue, Sep 19, 2023 at 08:54:37PM +0200, Rafael J. Wysocki wrote: > > From: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > > > > After recent changes, thermal_zone_get_trip() cannot fail, as invoked > > from thermal_zone_device_register_with_trips(), so the only role of > > the trips_disabled bitmask is struct thermal_zone_device is to make > > handle_thermal_trip() skip trip points whose temperature was initially > > zero. However, since the unit of temperature in the thermal core is > > millicelsius, zero may very well be a valid temperature value at least > > in some usage scenarios and the trip temperature may as well change > > later. Thus there is no reason to permanently disable trip points > > with initial temperature equal to zero. > > > > Accordingly, drop the trips_disabled bitmask along with the code > > related to it. > > > > Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > > I guess I was copied because of commit f1b80a3878b2 ("thermal: core: > Restore behavior regarding invalid trip points"). That's correct. > Since then we stopped > relying on this behavior with commit 5601ef91fba8 ("mlxsw: core_thermal: > Use static trip points for transceiver modules"). > > Tested your patch and didn't see any regressions: > > Tested-by: Ido Schimmel <idosch@nvidia.com> Thank you!
On 19/09/2023 20:54, Rafael J. Wysocki wrote: > From: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > > After recent changes, thermal_zone_get_trip() cannot fail, as invoked > from thermal_zone_device_register_with_trips(), so the only role of > the trips_disabled bitmask is struct thermal_zone_device is to make > handle_thermal_trip() skip trip points whose temperature was initially > zero. However, since the unit of temperature in the thermal core is > millicelsius, zero may very well be a valid temperature value at least > in some usage scenarios and the trip temperature may as well change > later. Thus there is no reason to permanently disable trip points > with initial temperature equal to zero. > > Accordingly, drop the trips_disabled bitmask along with the code > related to it. > > Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com> The logical change after changing how the trip point are handled ;) Acked-by: Daniel Lezcano <daniel.lezcano@linaro.org>
Index: linux-pm/drivers/thermal/thermal_core.c =================================================================== --- linux-pm.orig/drivers/thermal/thermal_core.c +++ linux-pm/drivers/thermal/thermal_core.c @@ -347,10 +347,6 @@ static void handle_thermal_trip(struct t { struct thermal_trip trip; - /* Ignore disabled trip points */ - if (test_bit(trip_id, &tz->trips_disabled)) - return; - __thermal_zone_get_trip(tz, trip_id, &trip); if (trip.temperature == THERMAL_TEMP_INVALID) @@ -1231,7 +1227,6 @@ thermal_zone_device_register_with_trips( struct thermal_zone_device *tz; int id; int result; - int count; struct thermal_governor *governor; if (!type || strlen(type) == 0) { @@ -1328,14 +1323,6 @@ thermal_zone_device_register_with_trips( if (result) goto release_device; - for (count = 0; count < num_trips; count++) { - struct thermal_trip trip; - - result = thermal_zone_get_trip(tz, count, &trip); - if (result || !trip.temperature) - set_bit(count, &tz->trips_disabled); - } - /* Update 'this' zone's governor information */ mutex_lock(&thermal_governor_lock); Index: linux-pm/include/linux/thermal.h =================================================================== --- linux-pm.orig/include/linux/thermal.h +++ linux-pm/include/linux/thermal.h @@ -122,7 +122,6 @@ struct thermal_cooling_device { * @devdata: private pointer for device private data * @trips: an array of struct thermal_trip * @num_trips: number of trip points the thermal zone supports - * @trips_disabled; bitmap for disabled trips * @passive_delay_jiffies: number of jiffies to wait between polls when * performing passive cooling. * @polling_delay_jiffies: number of jiffies to wait between polls when @@ -163,7 +162,6 @@ struct thermal_zone_device { void *devdata; struct thermal_trip *trips; int num_trips; - unsigned long trips_disabled; /* bitmap for disabled trips */ unsigned long passive_delay_jiffies; unsigned long polling_delay_jiffies; int temperature;