Messages in this thread | | | Date | Wed, 29 Nov 2023 21:43:30 +0000 | Subject | Re: [PATCH v3 2/2] thermal: trip: Rework thermal_zone_set_trip() and its callers | From | Lukasz Luba <> |
| |
On 11/29/23 13:38, Rafael J. Wysocki wrote: > From: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > > Both trip_point_temp_store() and trip_point_hyst_store() use > thermal_zone_set_trip() to update a given trip point, but none of them > actually needs to change more than one field in struct thermal_trip > representing it. However, each of them effectively calls > __thermal_zone_get_trip() twice in a row for the same trip index value, > once directly and once via thermal_zone_set_trip(), which is not > particularly efficient, and the way in which thermal_zone_set_trip() > carries out the update is not particularly straightforward. > > Moreover, some checks done by them both need not go under the thermal > zone lock and code duplication between them can be reduced quite a bit > by moving the majority of logic into thermal_zone_set_trip(). > > Rework all of the above functions to address the above. > > No intentional functional impact. > > Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > --- > > v2 -> v3: Fix missing return statement in thermal_zone_set_trip() (Lukasz). > > v1 -> v2: > * Fix 2 typos in the changelog (Lukasz). > * Split one change into the [1/2]. > > --- > drivers/thermal/thermal_core.h | 9 ++++++ > drivers/thermal/thermal_sysfs.c | 52 ++++++++-------------------------- > drivers/thermal/thermal_trip.c | 60 +++++++++++++++++++++++++++------------- > include/linux/thermal.h | 3 -- > 4 files changed, 62 insertions(+), 62 deletions(-) >
That looks OK. I have also checked those places were we set the callbacks. In mainline we only use set_trip_temp() callback. I don't know what is Daniel's idea for the patch, but LGTM.
Reviewed-by: Lukasz Luba <lukasz.luba@arm.com>
also tested both patches on two arm32, arm64 boards
Tested-by: Lukasz Luba <lukasz.luba@arm.com>
| |