Messages in this thread | | | From | "Zhang, Rui" <> | Subject | Re: [PATCH 5/5] thermal/core: Sort the trip points when registering a thermal zone | Date | Thu, 19 Jan 2023 07:22:59 +0000 |
| |
On Wed, 2023-01-18 at 22:11 +0100, Daniel Lezcano wrote: > Most of the drivers are converted to use the generic thermal trip > points. They register a thermal zone with an array of trip points. > > However, we don't have the guarantee the trip points are ordered. The > main goal of moving to the generic trip points is to provide a common > structure, ordered, so we can fix sanely how the trip points are > crossed. As a reminder, the detection is fuzzy when the trip points > are defined with hysteresis values, we can have duplicated or > inconsistent trip events. > > This change sorts the trip points array when it is registered with > the > thermal zone. The direction of the ordering is descending because > when > we browse the trip points, we want to check the highest trip points > first as they are higher in temperature, thus higher in priority. > > A pr_info() trace tells the trip points have been ordered if it > appears they are not sorted initially. > > Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org> > --- > drivers/thermal/thermal_core.c | 3 +++ > drivers/thermal/thermal_core.h | 1 + > drivers/thermal/thermal_trip.c | 28 ++++++++++++++++++++++++++++ > 3 files changed, 32 insertions(+) > > diff --git a/drivers/thermal/thermal_core.c > b/drivers/thermal/thermal_core.c > index d0577685085a..394770591771 100644 > --- a/drivers/thermal/thermal_core.c > +++ b/drivers/thermal/thermal_core.c > @@ -1255,6 +1255,9 @@ thermal_zone_device_register_with_trips(const > char *type, struct thermal_trip *t > if (num_trips > 0 && (!ops->get_trip_type || !ops- > >get_trip_temp) && !trips) > return ERR_PTR(-EINVAL); > > + if (trips && thermal_trip_sort(trips, num_trips)) > + pr_info("Thermal trips sorted for thermal zone '%s'\n", > type); > + > tz = kzalloc(sizeof(*tz), GFP_KERNEL); > if (!tz) > return ERR_PTR(-ENOMEM); > diff --git a/drivers/thermal/thermal_core.h > b/drivers/thermal/thermal_core.h > index 26350206a98d..4688107fda1d 100644 > --- a/drivers/thermal/thermal_core.h > +++ b/drivers/thermal/thermal_core.h > @@ -117,6 +117,7 @@ void __thermal_zone_set_trips(struct > thermal_zone_device *tz); > int __thermal_zone_get_trip(struct thermal_zone_device *tz, int > trip_id, > struct thermal_trip *trip); > int __thermal_zone_get_temp(struct thermal_zone_device *tz, int > *temp); > +int thermal_trip_sort(struct thermal_trip *trips, int num_trips); > > /* sysfs I/F */ > int thermal_zone_create_device_groups(struct thermal_zone_device *, > int); > diff --git a/drivers/thermal/thermal_trip.c > b/drivers/thermal/thermal_trip.c > index 2ef61ff7ffc3..924998f09a5a 100644 > --- a/drivers/thermal/thermal_trip.c > +++ b/drivers/thermal/thermal_trip.c > @@ -9,6 +9,34 @@ > */ > #include "thermal_core.h" > > +/* > + * The trip points must be ordered in the descending order so when > we > + * browse the trip points we will hit the critical, hot and then the > + * passive/active trip points. The critical trip point being the > first > + * one to be handled. > + */ > +int thermal_trip_sort(struct thermal_trip *trips, int num_trips) > +{ > + struct thermal_trip tt; > + int sorted = 0; > + int i, j; > + > + for (i = 0; i < num_trips; i++) { > + > + for (j = i + 1; j < num_trips; j++) { > + > + if (trips[i].temperature < > trips[j].temperature) { > + tt = trips[i]; > + trips[i] = trips[j]; > + trips[j] = tt; > + sorted++; > + } > + } > + } > + > + return sorted; > +} > + When this happens, the index(trip_id) of each trip is changed, but we pass the new trip_id to .get_trip_temp()/.set_trip_temp() callbacks.
This will confuse the drivers and update the wrong trips, right?
IMO, we need a map between thermal core trips and unsorted driver trips.
thanks, rui
> int __for_each_thermal_trip(struct thermal_zone_device *tz, > int (*cb)(struct thermal_trip *, > int trip_id, void *),
| |