Messages in this thread | | | Date | Mon, 7 Aug 2023 18:17:53 +0200 | Subject | Re: [PATCH v4 04/10] thermal: core: Add thermal_zone_update_trip_temp() helper routine | From | Daniel Lezcano <> |
| |
On 07/08/2023 17:40, Rafael J. Wysocki wrote: > On Mon, Aug 7, 2023 at 1:34 PM Daniel Lezcano <daniel.lezcano@linaro.org> wrote: >> >> On 04/08/2023 23:05, Rafael J. Wysocki wrote: >>> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com> >>> >>> Introduce a helper routine called thermal_zone_update_trip_temp() that >>> can be used to update a trip point's temperature with the help of a >>> pointer to local data associated with that trip point provided by >>> the thermal driver that created it. >>> >>> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com> >>> --- >>> >>> New patch in v4. >>> >>> --- >>> drivers/thermal/thermal_trip.c | 37 +++++++++++++++++++++++++++++++++++++ >>> include/linux/thermal.h | 4 ++++ >>> 2 files changed, 41 insertions(+) >>> >>> Index: linux-pm/drivers/thermal/thermal_trip.c >>> =================================================================== >>> --- linux-pm.orig/drivers/thermal/thermal_trip.c >>> +++ linux-pm/drivers/thermal/thermal_trip.c >>> @@ -180,3 +180,40 @@ int thermal_zone_set_trip(struct thermal >>> >>> return 0; >>> } >>> + >>> +/** >>> + * thermal_zone_update_trip_temp - Update the trip point temperature. >>> + * @tz: Thermal zone. >>> + * @trip_priv: Trip tag. >>> + * @temp: New trip temperature. >>> + * >>> + * This only works for thermal zones using trip tables and its caller must >>> + * ensure that the zone lock is held before using it. >>> + * >>> + * @trip_priv is expected to be the value that has been stored by the driver >>> + * in the struct thermal_trip representing the trip point in question, so it >>> + * can be matched against the value of the priv field in that structure. >>> + * >>> + * If @trip_priv does not match any trip point in the trip table of @tz, >>> + * nothing happens. >>> + */ >>> +void thermal_zone_update_trip_temp(struct thermal_zone_device *tz, >>> + void *trip_priv, int temperature) >>> +{ >>> + int i; >>> + >>> + lockdep_assert_held(&tz->lock); >>> + >>> + if (!tz->trips || !trip_priv) >>> + return; >>> + >>> + for (i = 0; i < tz->num_trips; i++) { >>> + struct thermal_trip *trip = &tz->trips[i]; >>> + >>> + if (trip->priv == trip_priv) { >>> + trip->temperature = temperature; >>> + return; >>> + } >>> + } >>> +} >>> +EXPORT_SYMBOL_GPL(thermal_zone_update_trip_temp); >> >> This function would imply the comparator is always trip->priv but if we >> want another comparison eg. trip->priv->id, that won't be possible. >> >> Actually, I think you can reuse an existing function with a simple >> change, for_each_thermal_trip() located in thermal_core.h. > > for_each_thermal_trip() is only defined in tools/lib/thermal/thermal.c > AFAICS, but this one could actually work, so I can copy that > definition to somewhere else. > > But I suppose that you mean __for_each_thermal_trip() which won't > work, because it makes a copy of the trip and passes that to the > callback, but the callback would need to update the temperature of the > original trip. > > It would work if it passed the original trip to the caller, so I can > add something like that.
As there is no user of this function yet, I think you can change that to use the trip array instead of the __thermal_zone_get_trip(). This one was used to have a compatibility with thermal zones using get_trip_* ops but that is not really needed and with your series only one driver will remain before dropping these ops.
>> The changes would be renaming it without the '__' prefix and moving it >> in include/linux/thermal.h. >> >> Then the comparison function and the temperature change can be an ACPI >> driver specific callback passed as parameter to for_each_thermal_zone > > I guess you mean for_each_thermal_trip().
Yes, __for_each_thermal_trip()
> As per the above, not really, but I can do something along these lines.
-- <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs
Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook | <http://twitter.com/#!/linaroorg> Twitter | <http://www.linaro.org/linaro-blog/> Blog
| |