Re: [PATCH v1 05/10] ACPI: video: Use thermal_cooling_device_create()

[Armin Wolf]

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

> From: "Rafael J. Wysocki" <rafael.j.wysocki@xxxxxxxxx>
>
> Instead of using thermal_cooling_device_register() for registering a
> backlight cooling device in the ACPI video bus driver, make it use
> thermal_cooling_device_create() and pass a pointer to the LCD device
> to that function as the cooling class device's parent.  That will
> cause the cooling device's sysfs directory to be created under
> the parent's sysfs directory (among other things).
>
> Since creating a class device under a parent causes a "device" symbolic
> link from the sysfs directory of the class device to the sysfs directory
> of the parent to appear automatically, remove the code creating the
> "device" symbolic link from the sysfs directory of the cooling device
> in question to the sysfs directory of the parent's ACPI companion
> device.  That companion device is reachable through the "firmware_node"
> symbolic link in the parent's sysfs directory regardless.
>
> Moreover, since the cooling class device is now located in sysfs under
> its parent and it can be easily identified as a cooling device, there is
> no need to create a "thermal_cooling" symbolic link from its parent's
> ACPI companion to it.  Accordingly, also remove the code creating that
> symbolic link.

Reviewed-by: Armin Wolf <W_Armin@xxxxxx>

> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@xxxxxxxxx>
> ---
>   drivers/acpi/acpi_video.c | 35 ++++++++++++++++-------------------
>   1 file changed, 16 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/acpi/acpi_video.c b/drivers/acpi/acpi_video.c
> index 6cfe390411c1..2a2822516665 100644
> --- a/drivers/acpi/acpi_video.c
> +++ b/drivers/acpi/acpi_video.c
> @@ -1700,9 +1700,9 @@ static int acpi_video_resume(struct notifier_block *nb,
>   
>   static void acpi_video_dev_register_backlight(struct acpi_video_device *device)
>   {
> +	struct device *phys_dev, *parent = NULL;
>   	struct backlight_properties props;
>   	struct pci_dev *pdev;
> -	struct device *parent = NULL;
>   	int result;
>   	static int count;
>   	char *name;
> @@ -1729,11 +1729,10 @@ static void acpi_video_dev_register_backlight(struct acpi_video_device *device)
>   						      device,
>   						      &acpi_backlight_ops,
>   						      &props);
> -	put_device(parent);
>   	kfree(name);
>   	if (IS_ERR(device->backlight)) {
>   		device->backlight = NULL;
> -		return;
> +		goto put_parent;
>   	}
>   
>   	/*
> @@ -1743,8 +1742,12 @@ static void acpi_video_dev_register_backlight(struct acpi_video_device *device)
>   	device->backlight->props.brightness =
>   			acpi_video_get_brightness(device->backlight);
>   
> -	device->cooling_dev = thermal_cooling_device_register("LCD", device,
> -							      &video_cooling_ops);
> +	phys_dev = acpi_bus_get_primary_device(device->dev);
> +	if (!phys_dev)
> +		phys_dev = get_device(parent);
> +
> +	device->cooling_dev = thermal_cooling_device_create(phys_dev, "LCD", device,
> +							    &video_cooling_ops);
>   	if (IS_ERR(device->cooling_dev)) {
>   		/*
>   		 * Set cooling_dev to NULL so we don't crash trying to free it.
> @@ -1753,21 +1756,17 @@ static void acpi_video_dev_register_backlight(struct acpi_video_device *device)
>   		 * -- dtor
>   		 */
>   		device->cooling_dev = NULL;
> -		return;
> +		goto put_phys_dev;
>   	}
>   
> -	dev_info(&device->dev->dev, "registered as cooling_device%d\n",
> -		 device->cooling_dev->id);
> -	result = sysfs_create_link(&device->dev->dev.kobj,
> -			&device->cooling_dev->device.kobj,
> -			"thermal_cooling");
> -	if (result)
> -		pr_info("sysfs link creation failed\n");
> +	dev_info(&device->cooling_dev->device, "Using ACPI device %s\n",
> +		acpi_dev_name(device->dev));
>   
> -	result = sysfs_create_link(&device->cooling_dev->device.kobj,
> -			&device->dev->dev.kobj, "device");
> -	if (result)
> -		pr_info("Reverse sysfs link creation failed\n");
> +put_phys_dev:
> +	put_device(phys_dev);
> +
> +put_parent:
> +	put_device(parent);
>   }
>   
>   static void acpi_video_run_bcl_for_osi(struct acpi_video_bus *video)
> @@ -1826,8 +1825,6 @@ static int acpi_video_bus_register_backlight(struct acpi_video_bus *video)
>   static void acpi_video_dev_unregister_backlight(struct acpi_video_device *device)
>   {
>   	if (device->cooling_dev) {
> -		sysfs_remove_link(&device->dev->dev.kobj, "thermal_cooling");
> -		sysfs_remove_link(&device->cooling_dev->device.kobj, "device");
>   		thermal_cooling_device_unregister(device->cooling_dev);
>   		device->cooling_dev = NULL;
>   	}