Messages in this thread Patch in this message | | | From | "Zhang, Rui" <> | Subject | Re: [PATCH v2 3/3] thermal/core: Move the thermal trip code to a dedicated file | Date | Tue, 24 Jan 2023 06:38:54 +0000 |
| |
On Mon, 2023-01-23 at 16:27 +0100, Daniel Lezcano wrote: > The thermal_core.c files contains a lot of functions handling > different thermal components like the governors, the trip points, the > cooling device, the OF cooling device, etc ... > > This organization does not help to migrate to a more sane code where > there is a better self-encapsulation as all the components' internals > can be directly accessed from a single file. > > For the sake of clarity, let's move the thermal trip points code in a > dedicated thermal_trip.c file and add a function to browse all the > trip points like we do with the thermal zones, the govenors and the > cooling devices. > > The same can be done for the cooling devices and the governor code > but > that will come later as the current work in the thermal framework is > to fix the trip point handling and use a generic trip point > structure. > > No functional changes intended. > > Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
Reviewed-by: Zhang Rui <rui.zhang@intel.com>
thanks, rui
> --- > drivers/thermal/Makefile | 4 +- > drivers/thermal/thermal_core.c | 87 -------------- > drivers/thermal/thermal_core.h | 4 + > drivers/thermal/thermal_helpers.c | 62 ---------- > drivers/thermal/thermal_trip.c | 182 > ++++++++++++++++++++++++++++++ > 5 files changed, 188 insertions(+), 151 deletions(-) > create mode 100644 drivers/thermal/thermal_trip.c > > diff --git a/drivers/thermal/Makefile b/drivers/thermal/Makefile > index 2506c6c8ca83..2faf4651f34a 100644 > --- a/drivers/thermal/Makefile > +++ b/drivers/thermal/Makefile > @@ -4,8 +4,8 @@ > # > > obj-$(CONFIG_THERMAL) += thermal_sys.o > -thermal_sys-y += thermal_core.o > thermal_sysfs.o \ > - thermal_helpers.o > +thermal_sys-y += thermal_core.o > thermal_sysfs.o > +thermal_sys-y += thermal_trip.o > thermal_helpers.o > > # netlink interface to manage the thermal framework > thermal_sys-$(CONFIG_THERMAL_NETLINK) += > thermal_netlink.o > diff --git a/drivers/thermal/thermal_core.c > b/drivers/thermal/thermal_core.c > index fad0c4a07d16..4ee685043a3e 100644 > --- a/drivers/thermal/thermal_core.c > +++ b/drivers/thermal/thermal_core.c > @@ -1155,12 +1155,6 @@ static void thermal_set_delay_jiffies(unsigned > long *delay_jiffies, int delay_ms > *delay_jiffies = round_jiffies(*delay_jiffies); > } > > -int thermal_zone_get_num_trips(struct thermal_zone_device *tz) > -{ > - return tz->num_trips; > -} > -EXPORT_SYMBOL_GPL(thermal_zone_get_num_trips); > - > int thermal_zone_get_crit_temp(struct thermal_zone_device *tz, int > *temp) > { > int i, ret = -EINVAL; > @@ -1187,87 +1181,6 @@ int thermal_zone_get_crit_temp(struct > thermal_zone_device *tz, int *temp) > } > EXPORT_SYMBOL_GPL(thermal_zone_get_crit_temp); > > -int __thermal_zone_get_trip(struct thermal_zone_device *tz, int > trip_id, > - struct thermal_trip *trip) > -{ > - int ret; > - > - if (!tz || trip_id < 0 || trip_id >= tz->num_trips || !trip) > - return -EINVAL; > - > - if (tz->trips) { > - *trip = tz->trips[trip_id]; > - return 0; > - } > - > - if (tz->ops->get_trip_hyst) { > - ret = tz->ops->get_trip_hyst(tz, trip_id, &trip- > >hysteresis); > - if (ret) > - return ret; > - } else { > - trip->hysteresis = 0; > - } > - > - ret = tz->ops->get_trip_temp(tz, trip_id, &trip->temperature); > - if (ret) > - return ret; > - > - return tz->ops->get_trip_type(tz, trip_id, &trip->type); > -} > -EXPORT_SYMBOL_GPL(__thermal_zone_get_trip); > - > -int thermal_zone_get_trip(struct thermal_zone_device *tz, int > trip_id, > - struct thermal_trip *trip) > -{ > - int ret; > - > - mutex_lock(&tz->lock); > - ret = __thermal_zone_get_trip(tz, trip_id, trip); > - mutex_unlock(&tz->lock); > - > - return ret; > -} > -EXPORT_SYMBOL_GPL(thermal_zone_get_trip); > - > -int thermal_zone_set_trip(struct thermal_zone_device *tz, int > trip_id, > - const struct thermal_trip *trip) > -{ > - struct thermal_trip t; > - int ret; > - > - if (!tz->ops->set_trip_temp && !tz->ops->set_trip_hyst && !tz- > >trips) > - return -EINVAL; > - > - ret = __thermal_zone_get_trip(tz, trip_id, &t); > - if (ret) > - return ret; > - > - if (t.type != trip->type) > - return -EINVAL; > - > - if (t.temperature != trip->temperature && tz->ops- > >set_trip_temp) { > - ret = tz->ops->set_trip_temp(tz, trip_id, trip- > >temperature); > - if (ret) > - return ret; > - } > - > - if (t.hysteresis != trip->hysteresis && tz->ops->set_trip_hyst) > { > - ret = tz->ops->set_trip_hyst(tz, trip_id, trip- > >hysteresis); > - if (ret) > - return ret; > - } > - > - if (tz->trips && (t.temperature != trip->temperature || > t.hysteresis != trip->hysteresis)) > - tz->trips[trip_id] = *trip; > - > - thermal_notify_tz_trip_change(tz->id, trip_id, trip->type, > - trip->temperature, trip- > >hysteresis); > - > - __thermal_zone_device_update(tz, THERMAL_TRIP_CHANGED); > - > - return 0; > -} > - > /** > * thermal_zone_device_register_with_trips() - register a new > thermal zone device > * @type: the thermal zone device type > diff --git a/drivers/thermal/thermal_core.h > b/drivers/thermal/thermal_core.h > index 26350206a98d..7af54382e915 100644 > --- a/drivers/thermal/thermal_core.h > +++ b/drivers/thermal/thermal_core.h > @@ -52,6 +52,10 @@ int for_each_thermal_cooling_device(int > (*cb)(struct thermal_cooling_device *, > int for_each_thermal_governor(int (*cb)(struct thermal_governor *, > void *), > void *thermal_governor); > > +int __for_each_thermal_trip(struct thermal_zone_device *, > + int (*cb)(struct thermal_trip *, void *), > + void *); > + > struct thermal_zone_device *thermal_zone_get_by_id(int id); > > struct thermal_attr { > diff --git a/drivers/thermal/thermal_helpers.c > b/drivers/thermal/thermal_helpers.c > index 8977d5ddc23c..0f648131b0b5 100644 > --- a/drivers/thermal/thermal_helpers.c > +++ b/drivers/thermal/thermal_helpers.c > @@ -146,68 +146,6 @@ int thermal_zone_get_temp(struct > thermal_zone_device *tz, int *temp) > } > EXPORT_SYMBOL_GPL(thermal_zone_get_temp); > > -/** > - * __thermal_zone_set_trips - Computes the next trip points for the > driver > - * @tz: a pointer to a thermal zone device structure > - * > - * The function computes the next temperature boundaries by browsing > - * the trip points. The result is the closer low and high trip > points > - * to the current temperature. These values are passed to the > backend > - * driver to let it set its own notification mechanism (usually an > - * interrupt). > - * > - * This function must be called with tz->lock held. Both tz and tz- > >ops > - * must be valid pointers. > - * > - * It does not return a value > - */ > -void __thermal_zone_set_trips(struct thermal_zone_device *tz) > -{ > - struct thermal_trip trip; > - int low = -INT_MAX, high = INT_MAX; > - int i, ret; > - > - lockdep_assert_held(&tz->lock); > - > - if (!tz->ops->set_trips) > - return; > - > - for (i = 0; i < tz->num_trips; i++) { > - int trip_low; > - > - ret = __thermal_zone_get_trip(tz, i , &trip); > - if (ret) > - return; > - > - trip_low = trip.temperature - trip.hysteresis; > - > - if (trip_low < tz->temperature && trip_low > low) > - low = trip_low; > - > - if (trip.temperature > tz->temperature && > - trip.temperature < high) > - high = trip.temperature; > - } > - > - /* No need to change trip points */ > - if (tz->prev_low_trip == low && tz->prev_high_trip == high) > - return; > - > - tz->prev_low_trip = low; > - tz->prev_high_trip = high; > - > - dev_dbg(&tz->device, > - "new temperature boundaries: %d < x < %d\n", low, > high); > - > - /* > - * Set a temperature window. When this window is left the > driver > - * must inform the thermal core via thermal_zone_device_update. > - */ > - ret = tz->ops->set_trips(tz, low, high); > - if (ret) > - dev_err(&tz->device, "Failed to set trips: %d\n", ret); > -} > - > static void thermal_cdev_set_cur_state(struct thermal_cooling_device > *cdev, > int target) > { > diff --git a/drivers/thermal/thermal_trip.c > b/drivers/thermal/thermal_trip.c > new file mode 100644 > index 000000000000..5ce71f1cb1ba > --- /dev/null > +++ b/drivers/thermal/thermal_trip.c > @@ -0,0 +1,182 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * Copyright (C) 2008 Intel Corp > + * Copyright (C) 2008 Zhang Rui <rui.zhang@intel.com> > + * Copyright (C) 2008 Sujith Thomas <sujith.thomas@intel.com> > + * Copyright 2022 Linaro Limited > + * > + * Thermal trips handling > + */ > +#include "thermal_core.h" > + > +int __for_each_thermal_trip(struct thermal_zone_device *tz, > + int (*cb)(struct thermal_trip *, void *), > + void *data) > +{ > + int i, ret; > + struct thermal_trip trip; > + > + lockdep_assert_held(&tz->lock); > + > + for (i = 0; i < tz->num_trips; i++) { > + > + ret = __thermal_zone_get_trip(tz, i, &trip); > + if (ret) > + return ret; > + > + ret = cb(&trip, data); > + if (ret) > + return ret; > + } > + > + return 0; > +} > + > +int thermal_zone_get_num_trips(struct thermal_zone_device *tz) > +{ > + return tz->num_trips; > +} > +EXPORT_SYMBOL_GPL(thermal_zone_get_num_trips); > + > +/** > + * __thermal_zone_set_trips - Computes the next trip points for the > driver > + * @tz: a pointer to a thermal zone device structure > + * > + * The function computes the next temperature boundaries by browsing > + * the trip points. The result is the closer low and high trip > points > + * to the current temperature. These values are passed to the > backend > + * driver to let it set its own notification mechanism (usually an > + * interrupt). > + * > + * This function must be called with tz->lock held. Both tz and tz- > >ops > + * must be valid pointers. > + * > + * It does not return a value > + */ > +void __thermal_zone_set_trips(struct thermal_zone_device *tz) > +{ > + struct thermal_trip trip; > + int low = -INT_MAX, high = INT_MAX; > + int i, ret; > + > + lockdep_assert_held(&tz->lock); > + > + if (!tz->ops->set_trips) > + return; > + > + for (i = 0; i < tz->num_trips; i++) { > + int trip_low; > + > + ret = __thermal_zone_get_trip(tz, i , &trip); > + if (ret) > + return; > + > + trip_low = trip.temperature - trip.hysteresis; > + > + if (trip_low < tz->temperature && trip_low > low) > + low = trip_low; > + > + if (trip.temperature > tz->temperature && > + trip.temperature < high) > + high = trip.temperature; > + } > + > + /* No need to change trip points */ > + if (tz->prev_low_trip == low && tz->prev_high_trip == high) > + return; > + > + tz->prev_low_trip = low; > + tz->prev_high_trip = high; > + > + dev_dbg(&tz->device, > + "new temperature boundaries: %d < x < %d\n", low, > high); > + > + /* > + * Set a temperature window. When this window is left the > driver > + * must inform the thermal core via thermal_zone_device_update. > + */ > + ret = tz->ops->set_trips(tz, low, high); > + if (ret) > + dev_err(&tz->device, "Failed to set trips: %d\n", ret); > +} > + > +int __thermal_zone_get_trip(struct thermal_zone_device *tz, int > trip_id, > + struct thermal_trip *trip) > +{ > + int ret; > + > + if (!tz || trip_id < 0 || trip_id >= tz->num_trips || !trip) > + return -EINVAL; > + > + if (tz->trips) { > + *trip = tz->trips[trip_id]; > + return 0; > + } > + > + if (tz->ops->get_trip_hyst) { > + ret = tz->ops->get_trip_hyst(tz, trip_id, &trip- > >hysteresis); > + if (ret) > + return ret; > + } else { > + trip->hysteresis = 0; > + } > + > + ret = tz->ops->get_trip_temp(tz, trip_id, &trip->temperature); > + if (ret) > + return ret; > + > + return tz->ops->get_trip_type(tz, trip_id, &trip->type); > +} > +EXPORT_SYMBOL_GPL(__thermal_zone_get_trip); > + > +int thermal_zone_get_trip(struct thermal_zone_device *tz, int > trip_id, > + struct thermal_trip *trip) > +{ > + int ret; > + > + mutex_lock(&tz->lock); > + ret = __thermal_zone_get_trip(tz, trip_id, trip); > + mutex_unlock(&tz->lock); > + > + return ret; > +} > +EXPORT_SYMBOL_GPL(thermal_zone_get_trip); > + > +int thermal_zone_set_trip(struct thermal_zone_device *tz, int > trip_id, > + const struct thermal_trip *trip) > +{ > + struct thermal_trip t; > + int ret; > + > + if (!tz->ops->set_trip_temp && !tz->ops->set_trip_hyst && !tz- > >trips) > + return -EINVAL; > + > + ret = __thermal_zone_get_trip(tz, trip_id, &t); > + if (ret) > + return ret; > + > + if (t.type != trip->type) > + return -EINVAL; > + > + if (t.temperature != trip->temperature && tz->ops- > >set_trip_temp) { > + ret = tz->ops->set_trip_temp(tz, trip_id, trip- > >temperature); > + if (ret) > + return ret; > + } > + > + if (t.hysteresis != trip->hysteresis && tz->ops->set_trip_hyst) > { > + ret = tz->ops->set_trip_hyst(tz, trip_id, trip- > >hysteresis); > + if (ret) > + return ret; > + } > + > + if (tz->trips && (t.temperature != trip->temperature || > t.hysteresis != trip->hysteresis)) > + tz->trips[trip_id] = *trip; > + > + thermal_notify_tz_trip_change(tz->id, trip_id, trip->type, > + trip->temperature, trip- > >hysteresis); > + > + __thermal_zone_device_update(tz, THERMAL_TRIP_CHANGED); > + > + return 0; > +} | |