Messages in this thread | | | Subject | Re: [PATCH v3 3/4] thermal/core: Build ascending ordered indexes for the trip points | From | Zhang Rui <> | Date | Mon, 18 Jul 2022 13:28:55 +0800 |
| |
On Fri, 2022-07-15 at 23:09 +0200, Daniel Lezcano wrote: > By convention the trips points are declared in the ascending > temperature order. However, no specification for the device tree, > ACPI > or documentation tells the trip points must be ordered this way. > > In the other hand, we need those to be ordered to browse them at the > thermal events. But if we assume they are ordered and change the code > based on this assumption, any platform with shuffled trip points > description will be broken (if they exist). > > Instead of taking the risk of breaking the existing platforms, use an > array of temperature ordered trip identifiers and make it available > for the code needing to browse the trip points in an ordered way. > > Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org> > --- > drivers/thermal/thermal_core.c | 62 +++++++++++++++++++++++++++----- > -- > include/linux/thermal.h | 2 ++ > 2 files changed, 52 insertions(+), 12 deletions(-) > > diff --git a/drivers/thermal/thermal_core.c > b/drivers/thermal/thermal_core.c > index f66036b3daae..f02f38b66445 100644 > --- a/drivers/thermal/thermal_core.c > +++ b/drivers/thermal/thermal_core.c > @@ -355,7 +355,8 @@ static void handle_critical_trips(struct > thermal_zone_device *tz, > } > > static void handle_thermal_trip_crossed(struct thermal_zone_device > *tz, int trip, > - int trip_temp, int trip_hyst, > enum thermal_trip_type trip_type) > + int trip_temp, int trip_hyst, > + enum thermal_trip_type > trip_type) > { > if (tz->last_temperature == THERMAL_TEMP_INVALID) > return; > @@ -1165,6 +1166,46 @@ static void bind_tz(struct thermal_zone_device > *tz) > mutex_unlock(&thermal_list_lock); > } > > +static void sort_trips_indexes(struct thermal_zone_device *tz) > +{ > + int i, j; > + > + for (i = 0; i < tz->trips; i++) > + tz->trips_indexes[i] = i; > + > + for (i = 0; i < tz->trips; i++) { > + for (j = i + 1; j < tz->trips; j++) { > + int t1, t2; > + > + tz->ops->get_trip_temp(tz, tz- > >trips_indexes[i], &t1);
This line can be moved to the upper loop.
> + tz->ops->get_trip_temp(tz, tz- > >trips_indexes[j], &t2); > +
what about the disabled trip points?
we should ignore those trip points and check the return value to make sure we're comparing the valid trip_temp values.
thanks, rui
> + if (t1 > t2) > + swap(tz->trips_indexes[i], tz- > >trips_indexes[j]); > + } > + } > +} > + > +static int thermal_zone_device_trip_init(struct thermal_zone_device > *tz) > +{ > + enum thermal_trip_type trip_type; > + int trip_temp, i; > + > + tz->trips_indexes = kzalloc(tz->trips * sizeof(int), > GFP_KERNEL); > + if (!tz->trips_indexes) > + return -ENOMEM; > + > + for (i = 0; i < tz->trips; i++) { > + if (tz->ops->get_trip_type(tz, i, &trip_type) || > + tz->ops->get_trip_temp(tz, i, &trip_temp) || > !trip_temp) > + set_bit(i, &tz->trips_disabled); > + } > + > + sort_trips_indexes(tz); > + > + return 0; > +} > + > /** > * thermal_zone_device_register() - register a new thermal zone > device > * @type: the thermal zone device type > @@ -1196,11 +1237,8 @@ thermal_zone_device_register(const char *type, > int trips, int mask, > int polling_delay) > { > struct thermal_zone_device *tz; > - enum thermal_trip_type trip_type; > - int trip_temp; > int id; > int result; > - int count; > struct thermal_governor *governor; > > if (!type || strlen(type) == 0) { > @@ -1272,12 +1310,9 @@ thermal_zone_device_register(const char *type, > int trips, int mask, > if (result) > goto release_device; > > - for (count = 0; count < trips; count++) { > - if (tz->ops->get_trip_type(tz, count, &trip_type) || > - tz->ops->get_trip_temp(tz, count, &trip_temp) || > - !trip_temp) > - set_bit(count, &tz->trips_disabled); > - } > + result = thermal_zone_device_trip_init(tz); > + if (result) > + goto unregister; > > /* Update 'this' zone's governor information */ > mutex_lock(&thermal_governor_lock); > @@ -1290,7 +1325,7 @@ thermal_zone_device_register(const char *type, > int trips, int mask, > result = thermal_set_governor(tz, governor); > if (result) { > mutex_unlock(&thermal_governor_lock); > - goto unregister; > + goto kfree_indexes; > } > > mutex_unlock(&thermal_governor_lock); > @@ -1298,7 +1333,7 @@ thermal_zone_device_register(const char *type, > int trips, int mask, > if (!tz->tzp || !tz->tzp->no_hwmon) { > result = thermal_add_hwmon_sysfs(tz); > if (result) > - goto unregister; > + goto kfree_indexes; > } > > mutex_lock(&thermal_list_lock); > @@ -1319,6 +1354,8 @@ thermal_zone_device_register(const char *type, > int trips, int mask, > > return tz; > > +kfree_indexes: > + kfree(tz->trips_indexes); > unregister: > device_del(&tz->device); > release_device: > @@ -1387,6 +1424,7 @@ void thermal_zone_device_unregister(struct > thermal_zone_device *tz) > thermal_remove_hwmon_sysfs(tz); > ida_simple_remove(&thermal_tz_ida, tz->id); > ida_destroy(&tz->ida); > + kfree(tz->trips_indexes); > mutex_destroy(&tz->lock); > device_unregister(&tz->device); > > diff --git a/include/linux/thermal.h b/include/linux/thermal.h > index 231bac2768fb..4c3b72536772 100644 > --- a/include/linux/thermal.h > +++ b/include/linux/thermal.h > @@ -112,6 +112,7 @@ struct thermal_cooling_device { > * @mode: current mode of this thermal zone > * @devdata: private pointer for device private data > * @trips: number of trip points the thermal zone supports > + * @trips_indexes: an array of sorted trip points indexes > * @trips_disabled; bitmap for disabled trips > * @passive_delay_jiffies: number of jiffies to wait between polls > when > * performing passive cooling. > @@ -152,6 +153,7 @@ struct thermal_zone_device { > enum thermal_device_mode mode; > void *devdata; > int trips; > + int *trips_indexes; > unsigned long trips_disabled; /* bitmap for disabled trips > */ > unsigned long passive_delay_jiffies; > unsigned long polling_delay_jiffies;
| |