Re: [PATCH v3 07/14] thermal: ACPI: Use the .should_bind() thermal zone callback

[Zhang, Rui]

On Mon, 2024-08-19 at 18:02 +0200, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@xxxxxxxxx>
> 
> Make the ACPI thermal zone 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.
> 
> This replaces the .bind() and .unbind() thermal zone callbacks which
> allows the code to be simplified quite significantly while providing
> the same functionality.
> 
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@xxxxxxxxx>

Reviewed-by: Zhang Rui <rui.zhang@xxxxxxxxx>

thanks,
rui
> ---
> 
> v1 -> v3: No changes (previously [09/17])
> 
> This patch only depends on the previous one introducing the
> .should_bind()
> thermal zone callback.
> 
> ---
>  drivers/acpi/thermal.c |   64 ++++++--------------------------------
> -----------
>  1 file changed, 9 insertions(+), 55 deletions(-)
> 
> Index: linux-pm/drivers/acpi/thermal.c
> ===================================================================
> --- linux-pm.orig/drivers/acpi/thermal.c
> +++ linux-pm/drivers/acpi/thermal.c
> @@ -558,77 +558,31 @@ static void acpi_thermal_zone_device_cri
>         thermal_zone_device_critical(thermal);
>  }
>  
> -struct acpi_thermal_bind_data {
> -       struct thermal_zone_device *thermal;
> -       struct thermal_cooling_device *cdev;
> -       bool bind;
> -};
> -
> -static int bind_unbind_cdev_cb(struct thermal_trip *trip, void *arg)
> +static bool acpi_thermal_should_bind_cdev(struct thermal_zone_device
> *thermal,
> +                                         const struct thermal_trip
> *trip,
> +                                         struct
> thermal_cooling_device *cdev,
> +                                         struct cooling_spec *c)
>  {
>         struct acpi_thermal_trip *acpi_trip = trip->priv;
> -       struct acpi_thermal_bind_data *bd = arg;
> -       struct thermal_zone_device *thermal = bd->thermal;
> -       struct thermal_cooling_device *cdev = bd->cdev;
>         struct acpi_device *cdev_adev = cdev->devdata;
>         int i;
>  
>         /* Skip critical and hot trips. */
>         if (!acpi_trip)
> -               return 0;
> +               return false;
>  
>         for (i = 0; i < acpi_trip->devices.count; i++) {
>                 acpi_handle handle = acpi_trip->devices.handles[i];
> -               struct acpi_device *adev =
> acpi_fetch_acpi_dev(handle);
> -
> -               if (adev != cdev_adev)
> -                       continue;
>  
> -               if (bd->bind) {
> -                       int ret;
> -
> -                       ret = thermal_bind_cdev_to_trip(thermal,
> trip, cdev,
> -
>                                                        THERMAL_NO_LIMI
> T,
> -
>                                                        THERMAL_NO_LIMI
> T,
> -
>                                                        THERMAL_WEIGHT_
> DEFAULT);
> -                       if (ret)
> -                               return ret;
> -               } else {
> -                       thermal_unbind_cdev_from_trip(thermal, trip,
> cdev);
> -               }
> +               if (acpi_fetch_acpi_dev(handle) == cdev_adev)
> +                       return true;
>         }
>  
> -       return 0;
> -}
> -
> -static int acpi_thermal_bind_unbind_cdev(struct thermal_zone_device
> *thermal,
> -                                        struct
> thermal_cooling_device *cdev,
> -                                        bool bind)
> -{
> -       struct acpi_thermal_bind_data bd = {
> -               .thermal = thermal, .cdev = cdev, .bind = bind
> -       };
> -
> -       return thermal_zone_for_each_trip(thermal,
> bind_unbind_cdev_cb, &bd);
> -}
> -
> -static int
> -acpi_thermal_bind_cooling_device(struct thermal_zone_device
> *thermal,
> -                                struct thermal_cooling_device *cdev)
> -{
> -       return acpi_thermal_bind_unbind_cdev(thermal, cdev, true);
> -}
> -
> -static int
> -acpi_thermal_unbind_cooling_device(struct thermal_zone_device
> *thermal,
> -                                  struct thermal_cooling_device
> *cdev)
> -{
> -       return acpi_thermal_bind_unbind_cdev(thermal, cdev, false);
> +       return false;
>  }
>  
>  static const struct thermal_zone_device_ops acpi_thermal_zone_ops =
> {
> -       .bind = acpi_thermal_bind_cooling_device,
> -       .unbind = acpi_thermal_unbind_cooling_device,
> +       .should_bind = acpi_thermal_should_bind_cdev,
>         .get_temp = thermal_get_temp,
>         .get_trend = thermal_get_trend,
>         .hot = acpi_thermal_zone_device_hot,
> 
> 
>