Re: [PATCH v1 07/10] ACPI: thermal: Use cooling device parent for thermal zone binding

[Armin Wolf]

Am 11.09.26 um 15:05 schrieb Rafael J. Wysocki:

> From: "Rafael J. Wysocki" <rafael.j.wysocki@xxxxxxxxx>
>
> The ACPI thermal zone .should_bind() callback function,
> acpi_thermal_should_bind_cdev(), expects the given cooling device's
> devdata to point to an ACPI device object whose ACPI handle should be
> compared with ACPI handles in a list associated with the given trip
> point.  That is not particularly straightforward and it effectively
> requires the drivers of ACPI cooling devices to populate the devdata
> with addresses of the ACPI companions of the devices they bind to.
> Consequently, the devdata cannot be used by the driver for its own
> needs which is its intended purpose.
>
> That can be overcome with the help of the observation that the
> ACPI device objects to be matched against the lists of ACPI handles
> associated with trip points are in fact the ACPI companions of the
> parents of cooling devices.  Thus instead of using the given cooling
> device's devdata, it is sufficient to obtain the ACPI handle of its
> parent and compare that ACPI handle with the ones in the list
> associated with the given trip point.
>
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@xxxxxxxxx>
> ---
>   drivers/acpi/thermal.c | 13 +++++++------
>   1 file changed, 7 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/acpi/thermal.c b/drivers/acpi/thermal.c
> index dd7666c176a0..dea28d674407 100644
> --- a/drivers/acpi/thermal.c
> +++ b/drivers/acpi/thermal.c
> @@ -564,17 +564,18 @@ static bool acpi_thermal_should_bind_cdev(struct thermal_zone_device *thermal,
>   					  struct cooling_spec *c)
>   {
>   	struct acpi_thermal_trip *acpi_trip = trip->priv;
> -	struct acpi_device *cdev_adev = cdev->devdata;
> +	struct device *parent = cdev->device.parent;
> +	acpi_handle parent_handle;
>   	int i;
>   
> -	/* Skip critical and hot trips. */
> -	if (!acpi_trip)
> +	/* Skip critical and hot trips and parentless cooling devices. */
> +	if (!acpi_trip || !parent)
>   		return false;
>   
> -	for (i = 0; i < acpi_trip->devices.count; i++) {
> -		acpi_handle handle = acpi_trip->devices.handles[i];
> +	parent_handle = ACPI_HANDLE(parent);
>   

Please check parent_handle for NULL here so we can return early. With this being fixed:

Reviewed-by: Armin Wolf <W_Armin@xxxxxx>

> -		if (acpi_fetch_acpi_dev(handle) == cdev_adev)
> +	for (i = 0; i < acpi_trip->devices.count; i++) {
> +		if (acpi_trip->devices.handles[i] == parent_handle)
>   			return true;
>   	}
>