| Message ID | 20240729150259.1089814-2-daniel.lezcano@linaro.org (mailing list archive) |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
| Series | Add thermal thresholds support | expand |
On Monday, July 29, 2024 5:02:50 PM CEST Daniel Lezcano wrote: > In order to set the scene for the thresholds support which have to > manipulate the low and high temperature boundaries for the interrupt > support, we must pass the low and high value the incoming thresholds > routine. > > Instead of looping in the trip descriptors in > thermal_zone_device_update(), we move the loop in the > handle_thermal_trip() function and use it to set the low and high > values. > > As these variables can be set directly in the handle_thermal_trip(), > we can get rid of a descriptors loop found in the thermal_set_trips() > function as low and high are set in handle_thermal_trip(). > > Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org> > --- > drivers/thermal/thermal_core.c | 104 +++++++++++++++++++-------------- > drivers/thermal/thermal_core.h | 2 +- > drivers/thermal/thermal_trip.c | 12 +--- > 3 files changed, 62 insertions(+), 56 deletions(-) > > diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c > index 95c399f94744..5cfa2a706e96 100644 > --- a/drivers/thermal/thermal_core.c > +++ b/drivers/thermal/thermal_core.c > @@ -425,59 +425,74 @@ static void handle_critical_trips(struct thermal_zone_device *tz, > } > > static void handle_thermal_trip(struct thermal_zone_device *tz, > - struct thermal_trip_desc *td, > struct list_head *way_up_list, > - struct list_head *way_down_list) > + struct list_head *way_down_list, > + int *low, int *high) > { > - const struct thermal_trip *trip = &td->trip; > + struct thermal_trip_desc *td; > int old_threshold; > > - if (trip->temperature == THERMAL_TEMP_INVALID) > - return; > + for_each_trip_desc(tz, td) { > > - /* > - * If the trip temperature or hysteresis has been updated recently, > - * the threshold needs to be computed again using the new values. > - * However, its initial value still reflects the old ones and that > - * is what needs to be compared with the previous zone temperature > - * to decide which action to take. > - */ > - old_threshold = td->threshold; > - td->threshold = trip->temperature; > + const struct thermal_trip *trip = &td->trip; > + > + if (trip->temperature == THERMAL_TEMP_INVALID) > + continue; > + > + if (tz->last_temperature < old_threshold || > + tz->last_temperature == THERMAL_TEMP_INIT) > + continue; > > - if (tz->last_temperature >= old_threshold && > - tz->last_temperature != THERMAL_TEMP_INIT) { > /* > - * Mitigation is under way, so it needs to stop if the zone > - * temperature falls below the low temperature of the trip. > - * In that case, the trip temperature becomes the new threshold. > + * If the trip temperature or hysteresis has been updated recently, > + * the threshold needs to be computed again using the new values. > + * However, its initial value still reflects the old ones and that > + * is what needs to be compared with the previous zone temperature > + * to decide which action to take. > */ > - if (tz->temperature < trip->temperature - trip->hysteresis) { > - list_add(&td->notify_list_node, way_down_list); > - td->notify_temp = trip->temperature - trip->hysteresis; > + old_threshold = td->threshold; > + td->threshold = trip->temperature; > > - if (trip->type == THERMAL_TRIP_PASSIVE) { > - tz->passive--; > - WARN_ON(tz->passive < 0); > + if (tz->last_temperature >= old_threshold && > + tz->last_temperature != THERMAL_TEMP_INVALID) { > + /* > + * Mitigation is under way, so it needs to stop if the zone > + * temperature falls below the low temperature of the trip. > + * In that case, the trip temperature becomes the new threshold. > + */ > + if (tz->temperature < trip->temperature - trip->hysteresis) { > + list_add(&td->notify_list_node, way_down_list); > + td->notify_temp = trip->temperature - trip->hysteresis; > + > + if (trip->type == THERMAL_TRIP_PASSIVE) { > + tz->passive--; > + WARN_ON(tz->passive < 0); > + } > + } else { > + td->threshold -= trip->hysteresis; > } > - } else { > + } else if (tz->temperature >= trip->temperature) { > + /* > + * There is no mitigation under way, so it needs to be started > + * if the zone temperature exceeds the trip one. The new > + * threshold is then set to the low temperature of the trip. > + */ > + list_add_tail(&td->notify_list_node, way_up_list); > + td->notify_temp = trip->temperature; > td->threshold -= trip->hysteresis; > + > + if (trip->type == THERMAL_TRIP_PASSIVE) > + tz->passive++; > + else if (trip->type == THERMAL_TRIP_CRITICAL || > + trip->type == THERMAL_TRIP_HOT) > + handle_critical_trips(tz, trip); > } > - } else if (tz->temperature >= trip->temperature) { > - /* > - * There is no mitigation under way, so it needs to be started > - * if the zone temperature exceeds the trip one. The new > - * threshold is then set to the low temperature of the trip. > - */ > - list_add_tail(&td->notify_list_node, way_up_list); > - td->notify_temp = trip->temperature; > - td->threshold -= trip->hysteresis; > - > - if (trip->type == THERMAL_TRIP_PASSIVE) > - tz->passive++; > - else if (trip->type == THERMAL_TRIP_CRITICAL || > - trip->type == THERMAL_TRIP_HOT) > - handle_critical_trips(tz, trip); > + > + if (td->threshold < tz->temperature && td->threshold > *low) > + *low = td->threshold; > + > + if (td->threshold > tz->temperature && td->threshold < *high) > + *high = td->threshold; > } > } > > @@ -545,6 +560,8 @@ void __thermal_zone_device_update(struct thermal_zone_device *tz, > { > struct thermal_governor *governor = thermal_get_tz_governor(tz); > struct thermal_trip_desc *td; > + int low = -INT_MAX, high = INT_MAX; > + > LIST_HEAD(way_down_list); > LIST_HEAD(way_up_list); > int temp, ret; > @@ -580,10 +597,9 @@ void __thermal_zone_device_update(struct thermal_zone_device *tz, > > tz->notify_event = event; > > - for_each_trip_desc(tz, td) > - handle_thermal_trip(tz, td, &way_up_list, &way_down_list); > + handle_thermal_trip(tz, &way_up_list, &way_down_list, &low, &high); > > - thermal_zone_set_trips(tz); > + thermal_zone_set_trips(tz, low, high); > > list_sort(NULL, &way_up_list, thermal_trip_notify_cmp); > list_for_each_entry(td, &way_up_list, notify_list_node) > diff --git a/drivers/thermal/thermal_core.h b/drivers/thermal/thermal_core.h > index 4cf2b7230d04..67a09f90eb95 100644 > --- a/drivers/thermal/thermal_core.h > +++ b/drivers/thermal/thermal_core.h > @@ -259,7 +259,7 @@ void thermal_governor_update_tz(struct thermal_zone_device *tz, > > const char *thermal_trip_type_name(enum thermal_trip_type trip_type); > > -void thermal_zone_set_trips(struct thermal_zone_device *tz); > +void thermal_zone_set_trips(struct thermal_zone_device *tz, int low, int high); > int thermal_zone_trip_id(const struct thermal_zone_device *tz, > const struct thermal_trip *trip); > void thermal_zone_trip_updated(struct thermal_zone_device *tz, > diff --git a/drivers/thermal/thermal_trip.c b/drivers/thermal/thermal_trip.c > index c0b679b846b3..af0f9f5ae0de 100644 > --- a/drivers/thermal/thermal_trip.c > +++ b/drivers/thermal/thermal_trip.c > @@ -76,10 +76,8 @@ EXPORT_SYMBOL_GPL(thermal_zone_get_num_trips); > * > * It does not return a value > */ > -void thermal_zone_set_trips(struct thermal_zone_device *tz) > +void thermal_zone_set_trips(struct thermal_zone_device *tz, int low, int high) > { > - const struct thermal_trip_desc *td; > - int low = -INT_MAX, high = INT_MAX; > int ret; > > lockdep_assert_held(&tz->lock); > @@ -87,14 +85,6 @@ void thermal_zone_set_trips(struct thermal_zone_device *tz) > if (!tz->ops.set_trips) > return; > > - for_each_trip_desc(tz, td) { > - if (td->threshold < tz->temperature && td->threshold > low) > - low = td->threshold; > - > - if (td->threshold > tz->temperature && td->threshold < high) > - high = td->threshold; > - } > - > /* No need to change trip points */ > if (tz->prev_low_trip == low && tz->prev_high_trip == high) > return; > Well, why not do the (untested) change below instead, which is way simpler? The thermal_zone_set_trips() kerneldoc needs to be either updated or dropped because it is not applicable any more after this and I think it's better to just drop it. --- drivers/thermal/thermal_core.c | 12 ++++++++++-- drivers/thermal/thermal_core.h | 2 +- drivers/thermal/thermal_trip.c | 27 +-------------------------- 3 files changed, 12 insertions(+), 29 deletions(-) Index: linux-pm/drivers/thermal/thermal_core.c =================================================================== --- linux-pm.orig/drivers/thermal/thermal_core.c +++ linux-pm/drivers/thermal/thermal_core.c @@ -551,6 +551,7 @@ void __thermal_zone_device_update(struct struct thermal_trip_desc *td; LIST_HEAD(way_down_list); LIST_HEAD(way_up_list); + int low = -INT_MAX, high = INT_MAX; int temp, ret; if (tz->suspended) @@ -584,10 +585,17 @@ void __thermal_zone_device_update(struct tz->notify_event = event; - for_each_trip_desc(tz, td) + for_each_trip_desc(tz, td) { handle_thermal_trip(tz, td, &way_up_list, &way_down_list); - thermal_zone_set_trips(tz); + if (td->threshold < tz->temperature && td->threshold > low) + low = td->threshold; + + if (td->threshold >= tz->temperature && td->threshold < high) + high = td->threshold; + } + + thermal_zone_set_trips(tz, low, high); list_sort(NULL, &way_up_list, thermal_trip_notify_cmp); list_for_each_entry(td, &way_up_list, notify_list_node) Index: linux-pm/drivers/thermal/thermal_core.h =================================================================== --- linux-pm.orig/drivers/thermal/thermal_core.h +++ linux-pm/drivers/thermal/thermal_core.h @@ -254,7 +254,7 @@ void thermal_governor_update_tz(struct t const char *thermal_trip_type_name(enum thermal_trip_type trip_type); -void thermal_zone_set_trips(struct thermal_zone_device *tz); +void thermal_zone_set_trips(struct thermal_zone_device *tz, int low, int high); int thermal_zone_trip_id(const struct thermal_zone_device *tz, const struct thermal_trip *trip); int __thermal_zone_get_temp(struct thermal_zone_device *tz, int *temp); Index: linux-pm/drivers/thermal/thermal_trip.c =================================================================== --- linux-pm.orig/drivers/thermal/thermal_trip.c +++ linux-pm/drivers/thermal/thermal_trip.c @@ -55,25 +55,8 @@ int thermal_zone_for_each_trip(struct th } EXPORT_SYMBOL_GPL(thermal_zone_for_each_trip); -/** - * 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) +void thermal_zone_set_trips(struct thermal_zone_device *tz, int low, int high) { - const struct thermal_trip_desc *td; - int low = -INT_MAX, high = INT_MAX; int ret; lockdep_assert_held(&tz->lock); @@ -81,14 +64,6 @@ void thermal_zone_set_trips(struct therm if (!tz->ops.set_trips) return; - for_each_trip_desc(tz, td) { - if (td->threshold < tz->temperature && td->threshold > low) - low = td->threshold; - - if (td->threshold > tz->temperature && td->threshold < high) - high = td->threshold; - } - /* No need to change trip points */ if (tz->prev_low_trip == low && tz->prev_high_trip == high) return;
Hi Daniel, kernel test robot noticed the following build warnings: [auto build test WARNING on rafael-pm/thermal] [also build test WARNING on linus/master v6.11-rc1 next-20240729] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch#_base_tree_information] url: https://github.com/intel-lab-lkp/linux/commits/Daniel-Lezcano/thermal-core-Encapsulate-more-handle_thermal_trip/20240730-005842 base: https://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git thermal patch link: https://lore.kernel.org/r/20240729150259.1089814-2-daniel.lezcano%40linaro.org patch subject: [PATCH v1 1/7] thermal/core: Encapsulate more handle_thermal_trip config: x86_64-defconfig (https://download.01.org/0day-ci/archive/20240730/202407300733.L6f9tFOM-lkp@intel.com/config) compiler: gcc-13 (Ubuntu 13.2.0-4ubuntu3) 13.2.0 reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240730/202407300733.L6f9tFOM-lkp@intel.com/reproduce) If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags | Reported-by: kernel test robot <lkp@intel.com> | Closes: https://lore.kernel.org/oe-kbuild-all/202407300733.L6f9tFOM-lkp@intel.com/ All warnings (new ones prefixed by >>): >> drivers/thermal/thermal_trip.c:80: warning: Function parameter or struct member 'low' not described in 'thermal_zone_set_trips' >> drivers/thermal/thermal_trip.c:80: warning: Function parameter or struct member 'high' not described in 'thermal_zone_set_trips' vim +80 drivers/thermal/thermal_trip.c 5b8de18ee9027c Daniel Lezcano 2023-01-23 63 5b8de18ee9027c Daniel Lezcano 2023-01-23 64 /** 28d5cc12671c8b Rafael J. Wysocki 2024-06-07 65 * thermal_zone_set_trips - Computes the next trip points for the driver 5b8de18ee9027c Daniel Lezcano 2023-01-23 66 * @tz: a pointer to a thermal zone device structure 5b8de18ee9027c Daniel Lezcano 2023-01-23 67 * 5b8de18ee9027c Daniel Lezcano 2023-01-23 68 * The function computes the next temperature boundaries by browsing 5b8de18ee9027c Daniel Lezcano 2023-01-23 69 * the trip points. The result is the closer low and high trip points 5b8de18ee9027c Daniel Lezcano 2023-01-23 70 * to the current temperature. These values are passed to the backend 5b8de18ee9027c Daniel Lezcano 2023-01-23 71 * driver to let it set its own notification mechanism (usually an 5b8de18ee9027c Daniel Lezcano 2023-01-23 72 * interrupt). 5b8de18ee9027c Daniel Lezcano 2023-01-23 73 * 5b8de18ee9027c Daniel Lezcano 2023-01-23 74 * This function must be called with tz->lock held. Both tz and tz->ops 5b8de18ee9027c Daniel Lezcano 2023-01-23 75 * must be valid pointers. 5b8de18ee9027c Daniel Lezcano 2023-01-23 76 * 5b8de18ee9027c Daniel Lezcano 2023-01-23 77 * It does not return a value 5b8de18ee9027c Daniel Lezcano 2023-01-23 78 */ 4c090556217f53 Daniel Lezcano 2024-07-29 79 void thermal_zone_set_trips(struct thermal_zone_device *tz, int low, int high) 5b8de18ee9027c Daniel Lezcano 2023-01-23 @80 { 0c0c4740c9d266 Rafael J. Wysocki 2023-12-04 81 int ret; 5b8de18ee9027c Daniel Lezcano 2023-01-23 82 5b8de18ee9027c Daniel Lezcano 2023-01-23 83 lockdep_assert_held(&tz->lock); 5b8de18ee9027c Daniel Lezcano 2023-01-23 84 698a1eb1f75eb6 Rafael J. Wysocki 2024-02-22 85 if (!tz->ops.set_trips) 5b8de18ee9027c Daniel Lezcano 2023-01-23 86 return; 5b8de18ee9027c Daniel Lezcano 2023-01-23 87 5b8de18ee9027c Daniel Lezcano 2023-01-23 88 /* No need to change trip points */ 5b8de18ee9027c Daniel Lezcano 2023-01-23 89 if (tz->prev_low_trip == low && tz->prev_high_trip == high) 5b8de18ee9027c Daniel Lezcano 2023-01-23 90 return; 5b8de18ee9027c Daniel Lezcano 2023-01-23 91 5b8de18ee9027c Daniel Lezcano 2023-01-23 92 tz->prev_low_trip = low; 5b8de18ee9027c Daniel Lezcano 2023-01-23 93 tz->prev_high_trip = high; 5b8de18ee9027c Daniel Lezcano 2023-01-23 94 5b8de18ee9027c Daniel Lezcano 2023-01-23 95 dev_dbg(&tz->device, 5b8de18ee9027c Daniel Lezcano 2023-01-23 96 "new temperature boundaries: %d < x < %d\n", low, high); 5b8de18ee9027c Daniel Lezcano 2023-01-23 97 5b8de18ee9027c Daniel Lezcano 2023-01-23 98 /* 5b8de18ee9027c Daniel Lezcano 2023-01-23 99 * Set a temperature window. When this window is left the driver 5b8de18ee9027c Daniel Lezcano 2023-01-23 100 * must inform the thermal core via thermal_zone_device_update. 5b8de18ee9027c Daniel Lezcano 2023-01-23 101 */ 698a1eb1f75eb6 Rafael J. Wysocki 2024-02-22 102 ret = tz->ops.set_trips(tz, low, high); 5b8de18ee9027c Daniel Lezcano 2023-01-23 103 if (ret) 5b8de18ee9027c Daniel Lezcano 2023-01-23 104 dev_err(&tz->device, "Failed to set trips: %d\n", ret); 5b8de18ee9027c Daniel Lezcano 2023-01-23 105 } 5b8de18ee9027c Daniel Lezcano 2023-01-23 106
Hi Rafael, On 29/07/2024 18:57, Rafael J. Wysocki wrote: > On Monday, July 29, 2024 5:02:50 PM CEST Daniel Lezcano wrote: >> In order to set the scene for the thresholds support which have to >> manipulate the low and high temperature boundaries for the interrupt >> support, we must pass the low and high value the incoming thresholds >> routine. >> >> Instead of looping in the trip descriptors in >> thermal_zone_device_update(), we move the loop in the >> handle_thermal_trip() function and use it to set the low and high >> values. >> >> As these variables can be set directly in the handle_thermal_trip(), >> we can get rid of a descriptors loop found in the thermal_set_trips() >> function as low and high are set in handle_thermal_trip(). >> >> Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org> >> --- [ ... ] >> - for_each_trip_desc(tz, td) >> - handle_thermal_trip(tz, td, &way_up_list, &way_down_list); >> + handle_thermal_trip(tz, &way_up_list, &way_down_list, &low, &high); >> [ ... ] > Well, why not do the (untested) change below instead, which is way simpler? Right, I did your proposed changed initially. The patch looks very complicated but it is just the result of the difference between the change above and below. It is code reorg, without functional changes (except two loops => one loop). It looked to me more consistent to move the for_each_trip_desc() inside handle_thermal_trip() in order to: - encapsulate the trip code more and have one line call - be consistent with the thermal_threshold_handle() which is also one line call If you prefer, I can split the changes, but it is extra work for little benefit. I pushed the changes in the git tree, the resulting code from these changes are: https://git.kernel.org/pub/scm/linux/kernel/git/thermal/linux.git/tree/drivers/thermal/thermal_core.c?h=thermal/threshold-patchset#n427 and https://git.kernel.org/pub/scm/linux/kernel/git/thermal/linux.git/tree/drivers/thermal/thermal_core.c?h=thermal/threshold-patchset#n600 Let me know if you prefer to do a smaller change or go forward in the code encapsulation > The thermal_zone_set_trips() kerneldoc needs to be either updated or dropped > because it is not applicable any more after this and I think it's better to > just drop it. Sure > - for_each_trip_desc(tz, td) > + for_each_trip_desc(tz, td) { > handle_thermal_trip(tz, td, &way_up_list, &way_down_list); [ ... ]
Hi Daniel, On Tue, Jul 30, 2024 at 11:40 AM Daniel Lezcano <daniel.lezcano@linaro.org> wrote: > > > Hi Rafael, > > On 29/07/2024 18:57, Rafael J. Wysocki wrote: > > On Monday, July 29, 2024 5:02:50 PM CEST Daniel Lezcano wrote: > >> In order to set the scene for the thresholds support which have to > >> manipulate the low and high temperature boundaries for the interrupt > >> support, we must pass the low and high value the incoming thresholds > >> routine. > >> > >> Instead of looping in the trip descriptors in > >> thermal_zone_device_update(), we move the loop in the > >> handle_thermal_trip() function and use it to set the low and high > >> values. > >> > >> As these variables can be set directly in the handle_thermal_trip(), > >> we can get rid of a descriptors loop found in the thermal_set_trips() > >> function as low and high are set in handle_thermal_trip(). > >> > >> Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org> > >> --- > > [ ... ] > > >> - for_each_trip_desc(tz, td) > >> - handle_thermal_trip(tz, td, &way_up_list, &way_down_list); > >> + handle_thermal_trip(tz, &way_up_list, &way_down_list, &low, &high); > >> > > [ ... ] > > > Well, why not do the (untested) change below instead, which is way simpler? > > Right, I did your proposed changed initially. OK > The patch looks very > complicated but it is just the result of the difference between the > change above and below. It is code reorg, without functional changes > (except two loops => one loop). > > It looked to me more consistent to move the for_each_trip_desc() inside > handle_thermal_trip() in order to: > - encapsulate the trip code more and have one line call I don't agree with this. The purpose is questionable and the code becomes more complex overall. And I'm totally not a fan of passing values through pointers if that is avoidable. > - be consistent with the thermal_threshold_handle() which is also one > line call That is a somewhat different story. > If you prefer, I can split the changes, but it is extra work for little > benefit. I pushed the changes in the git tree, the resulting code from > these changes are: > > https://git.kernel.org/pub/scm/linux/kernel/git/thermal/linux.git/tree/drivers/thermal/thermal_core.c?h=thermal/threshold-patchset#n427 > > and > > https://git.kernel.org/pub/scm/linux/kernel/git/thermal/linux.git/tree/drivers/thermal/thermal_core.c?h=thermal/threshold-patchset#n600 > > Let me know if you prefer to do a smaller change or go forward in the > code encapsulation I'd prefer to make a smaller change (obviously). > > The thermal_zone_set_trips() kerneldoc needs to be either updated or dropped > > because it is not applicable any more after this and I think it's better to > > just drop it. > > Sure BTW, there is a problem in thermal_zone_set_trips() now if the zone temperature is exactly equal to one of the trip's thresholds because it will then skip that trip. Say there are three trips, A, B, C sorted in ascending temperature order with no hysteresis. Say the zone temperature is exactly equal to the temperature of B. thermal_zone_set_trips() will set the boundaries to A and C, so the crossing of B will not be caught. IMV, both comparisons with the zone temperature in thermal_zone_set_trips() need to be made not strict, that is if (td->threshold <= tz->temperature && td->threshold > low) low = td->threshold; if (td->threshold => tz->temperature && td->threshold < high) high = td->threshold; in order to catch all of the trip crossing events. Of course, in the example above, this will cause thermal_zone_set_trips() to set both the boundaries to B, but that's OK because it will trigger an interrupt when the zone temperature becomes different from the B threshold regardless of which way it goes and that will allow a new interval to be set (aither {A, B} if it goes down or {B, C} if it goes up). IMV this change needs to be made in -stable, so I'll send a patch for it to be applied before any other changes in thermal_zone_set_trips().
diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c index 95c399f94744..5cfa2a706e96 100644 --- a/drivers/thermal/thermal_core.c +++ b/drivers/thermal/thermal_core.c @@ -425,59 +425,74 @@ static void handle_critical_trips(struct thermal_zone_device *tz, } static void handle_thermal_trip(struct thermal_zone_device *tz, - struct thermal_trip_desc *td, struct list_head *way_up_list, - struct list_head *way_down_list) + struct list_head *way_down_list, + int *low, int *high) { - const struct thermal_trip *trip = &td->trip; + struct thermal_trip_desc *td; int old_threshold; - if (trip->temperature == THERMAL_TEMP_INVALID) - return; + for_each_trip_desc(tz, td) { - /* - * If the trip temperature or hysteresis has been updated recently, - * the threshold needs to be computed again using the new values. - * However, its initial value still reflects the old ones and that - * is what needs to be compared with the previous zone temperature - * to decide which action to take. - */ - old_threshold = td->threshold; - td->threshold = trip->temperature; + const struct thermal_trip *trip = &td->trip; + + if (trip->temperature == THERMAL_TEMP_INVALID) + continue; + + if (tz->last_temperature < old_threshold || + tz->last_temperature == THERMAL_TEMP_INIT) + continue; - if (tz->last_temperature >= old_threshold && - tz->last_temperature != THERMAL_TEMP_INIT) { /* - * Mitigation is under way, so it needs to stop if the zone - * temperature falls below the low temperature of the trip. - * In that case, the trip temperature becomes the new threshold. + * If the trip temperature or hysteresis has been updated recently, + * the threshold needs to be computed again using the new values. + * However, its initial value still reflects the old ones and that + * is what needs to be compared with the previous zone temperature + * to decide which action to take. */ - if (tz->temperature < trip->temperature - trip->hysteresis) { - list_add(&td->notify_list_node, way_down_list); - td->notify_temp = trip->temperature - trip->hysteresis; + old_threshold = td->threshold; + td->threshold = trip->temperature; - if (trip->type == THERMAL_TRIP_PASSIVE) { - tz->passive--; - WARN_ON(tz->passive < 0); + if (tz->last_temperature >= old_threshold && + tz->last_temperature != THERMAL_TEMP_INVALID) { + /* + * Mitigation is under way, so it needs to stop if the zone + * temperature falls below the low temperature of the trip. + * In that case, the trip temperature becomes the new threshold. + */ + if (tz->temperature < trip->temperature - trip->hysteresis) { + list_add(&td->notify_list_node, way_down_list); + td->notify_temp = trip->temperature - trip->hysteresis; + + if (trip->type == THERMAL_TRIP_PASSIVE) { + tz->passive--; + WARN_ON(tz->passive < 0); + } + } else { + td->threshold -= trip->hysteresis; } - } else { + } else if (tz->temperature >= trip->temperature) { + /* + * There is no mitigation under way, so it needs to be started + * if the zone temperature exceeds the trip one. The new + * threshold is then set to the low temperature of the trip. + */ + list_add_tail(&td->notify_list_node, way_up_list); + td->notify_temp = trip->temperature; td->threshold -= trip->hysteresis; + + if (trip->type == THERMAL_TRIP_PASSIVE) + tz->passive++; + else if (trip->type == THERMAL_TRIP_CRITICAL || + trip->type == THERMAL_TRIP_HOT) + handle_critical_trips(tz, trip); } - } else if (tz->temperature >= trip->temperature) { - /* - * There is no mitigation under way, so it needs to be started - * if the zone temperature exceeds the trip one. The new - * threshold is then set to the low temperature of the trip. - */ - list_add_tail(&td->notify_list_node, way_up_list); - td->notify_temp = trip->temperature; - td->threshold -= trip->hysteresis; - - if (trip->type == THERMAL_TRIP_PASSIVE) - tz->passive++; - else if (trip->type == THERMAL_TRIP_CRITICAL || - trip->type == THERMAL_TRIP_HOT) - handle_critical_trips(tz, trip); + + if (td->threshold < tz->temperature && td->threshold > *low) + *low = td->threshold; + + if (td->threshold > tz->temperature && td->threshold < *high) + *high = td->threshold; } } @@ -545,6 +560,8 @@ void __thermal_zone_device_update(struct thermal_zone_device *tz, { struct thermal_governor *governor = thermal_get_tz_governor(tz); struct thermal_trip_desc *td; + int low = -INT_MAX, high = INT_MAX; + LIST_HEAD(way_down_list); LIST_HEAD(way_up_list); int temp, ret; @@ -580,10 +597,9 @@ void __thermal_zone_device_update(struct thermal_zone_device *tz, tz->notify_event = event; - for_each_trip_desc(tz, td) - handle_thermal_trip(tz, td, &way_up_list, &way_down_list); + handle_thermal_trip(tz, &way_up_list, &way_down_list, &low, &high); - thermal_zone_set_trips(tz); + thermal_zone_set_trips(tz, low, high); list_sort(NULL, &way_up_list, thermal_trip_notify_cmp); list_for_each_entry(td, &way_up_list, notify_list_node) diff --git a/drivers/thermal/thermal_core.h b/drivers/thermal/thermal_core.h index 4cf2b7230d04..67a09f90eb95 100644 --- a/drivers/thermal/thermal_core.h +++ b/drivers/thermal/thermal_core.h @@ -259,7 +259,7 @@ void thermal_governor_update_tz(struct thermal_zone_device *tz, const char *thermal_trip_type_name(enum thermal_trip_type trip_type); -void thermal_zone_set_trips(struct thermal_zone_device *tz); +void thermal_zone_set_trips(struct thermal_zone_device *tz, int low, int high); int thermal_zone_trip_id(const struct thermal_zone_device *tz, const struct thermal_trip *trip); void thermal_zone_trip_updated(struct thermal_zone_device *tz, diff --git a/drivers/thermal/thermal_trip.c b/drivers/thermal/thermal_trip.c index c0b679b846b3..af0f9f5ae0de 100644 --- a/drivers/thermal/thermal_trip.c +++ b/drivers/thermal/thermal_trip.c @@ -76,10 +76,8 @@ EXPORT_SYMBOL_GPL(thermal_zone_get_num_trips); * * It does not return a value */ -void thermal_zone_set_trips(struct thermal_zone_device *tz) +void thermal_zone_set_trips(struct thermal_zone_device *tz, int low, int high) { - const struct thermal_trip_desc *td; - int low = -INT_MAX, high = INT_MAX; int ret; lockdep_assert_held(&tz->lock); @@ -87,14 +85,6 @@ void thermal_zone_set_trips(struct thermal_zone_device *tz) if (!tz->ops.set_trips) return; - for_each_trip_desc(tz, td) { - if (td->threshold < tz->temperature && td->threshold > low) - low = td->threshold; - - if (td->threshold > tz->temperature && td->threshold < high) - high = td->threshold; - } - /* No need to change trip points */ if (tz->prev_low_trip == low && tz->prev_high_trip == high) return;
In order to set the scene for the thresholds support which have to manipulate the low and high temperature boundaries for the interrupt support, we must pass the low and high value the incoming thresholds routine. Instead of looping in the trip descriptors in thermal_zone_device_update(), we move the loop in the handle_thermal_trip() function and use it to set the low and high values. As these variables can be set directly in the handle_thermal_trip(), we can get rid of a descriptors loop found in the thermal_set_trips() function as low and high are set in handle_thermal_trip(). Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org> --- drivers/thermal/thermal_core.c | 104 +++++++++++++++++++-------------- drivers/thermal/thermal_core.h | 2 +- drivers/thermal/thermal_trip.c | 12 +--- 3 files changed, 62 insertions(+), 56 deletions(-)