Re: [PATCH v3 09/14] platform/x86: acerhdf: Use the .should_bind() thermal zone callback

Peter Kästle
2024-08-19T20:24:10Z

On 19.08.24 18:19, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
>
> Make the acerhdf driver use the .should_bind() thermal zone
> callback to provide the thermal core with the information on whether or
> not to bind the given cooling device to the given trip point in the
> given thermal zone. If it returns 'true', the thermal core will bind
> the cooling device to the trip and the corresponding unbinding will be
> taken care of automatically by the core on the removal of the involved
> thermal zone or cooling device.
>
> The previously existing acerhdf_bind() function bound cooling devices
> to thermal trip point 0 only, so the new callback needs to return 'true'
> for trip point 0. However, it is straightforward to observe that trip
> point 0 is an active trip point and the only other trip point in the
> driver's thermal zone is a critical one, so it is sufficient to return
> 'true' from that callback if the type of the given trip point is
> THERMAL_TRIP_ACTIVE.
>
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> Acked-by: Hans de Goede <hdegoede@redhat.com>

Tested-by: Peter Kästle <peter@piie.net>

> ---
>
> v2 -> v3: Reorder (previously [12/17]) and add the ACK from Hans
>
> v1 -> v2: No changes
>
> This patch only depends on the [06/14] introducing the .should_bind()
> thermal zone callback:
>
> https://lore.kernel.org/linux-pm/9334403.CDJkKcVGEf@rjwysocki.net/
>
> ---
> drivers/platform/x86/acerhdf.c | 33 ++++++---------------------------
> 1 file changed, 6 insertions(+), 27 deletions(-)
>
> Index: linux-pm/drivers/platform/x86/acerhdf.c
> ===================================================================
> --- linux-pm.orig/drivers/platform/x86/acerhdf.c
> +++ linux-pm/drivers/platform/x86/acerhdf.c
> @@ -378,33 +378,13 @@ static int acerhdf_get_ec_temp(struct th
> return 0;
> }
>
> -static int acerhdf_bind(struct thermal_zone_device *thermal,
> - struct thermal_cooling_device *cdev)
> +static bool acerhdf_should_bind(struct thermal_zone_device *thermal,
> + const struct thermal_trip *trip,
> + struct thermal_cooling_device *cdev,
> + struct cooling_spec *c)
> {
> /* if the cooling device is the one from acerhdf bind it */
> - if (cdev != cl_dev)
> - return 0;
> -
> - if (thermal_zone_bind_cooling_device(thermal, 0, cdev,
> - THERMAL_NO_LIMIT, THERMAL_NO_LIMIT,
> - THERMAL_WEIGHT_DEFAULT)) {
> - pr_err("error binding cooling dev\n");
> - return -EINVAL;
> - }
> - return 0;
> -}
> -
> -static int acerhdf_unbind(struct thermal_zone_device *thermal,
> - struct thermal_cooling_device *cdev)
> -{
> - if (cdev != cl_dev)
> - return 0;
> -
> - if (thermal_zone_unbind_cooling_device(thermal, 0, cdev)) {
> - pr_err("error unbinding cooling dev\n");
> - return -EINVAL;
> - }
> - return 0;
> + return cdev == cl_dev && trip->type == THERMAL_TRIP_ACTIVE;
> }
>
> static inline void acerhdf_revert_to_bios_mode(void)
> @@ -447,8 +427,7 @@ static int acerhdf_get_crit_temp(struct
>
> /* bind callback functions to thermalzone */
> static struct thermal_zone_device_ops acerhdf_dev_ops = {
> - .bind = acerhdf_bind,
> - .unbind = acerhdf_unbind,
> + .should_bind = acerhdf_should_bind,
> .get_temp = acerhdf_get_ec_temp,
> .change_mode = acerhdf_change_mode,
> .get_crit_temp = acerhdf_get_crit_temp,
>
>
>