Re: [PATCH v1 04/10] ACPI: processor: thermal: Use thermal_cooling_device_create()

[Armin Wolf <W_Armin@gmx.de>]

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

> From: "Rafael J. Wysocki" <rafael.j.wysocki@intel.com>
>
> Instead of using thermal_cooling_device_register() for registering
> a cooling device in the ACPI processor driver, make it use
> thermal_cooling_device_create() and pass a pointer to the processor
> device representing the given CPU to that function as the cooling
> 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 companion ACPI
> device.  That ACPI device is reachable through the "firmware_node"
> symbolic link in the parent's sysfs directory regardless.
>
> Moreover, since the cooling 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.
>
> While at it, check for error pointer values in addition to checking
> for NULL in acpi_processor_thermal_exit() to avoid dereferncing them
> mistakenly.

Reviewed-by: Armin Wolf <W_Armin@gmx.de>

> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> ---
>   drivers/acpi/processor_driver.c  |  4 +--
>   drivers/acpi/processor_thermal.c | 47 +++++---------------------------
>   include/acpi/processor.h         |  3 +-
>   3 files changed, 10 insertions(+), 44 deletions(-)
>
> diff --git a/drivers/acpi/processor_driver.c b/drivers/acpi/processor_driver.c
> index cdc2ae1632b2..90c14480393d 100644
> --- a/drivers/acpi/processor_driver.c
> +++ b/drivers/acpi/processor_driver.c
> @@ -179,7 +179,7 @@ static int __acpi_processor_start(struct acpi_device *device)
>   	return 0;
>   
>   err_thermal_exit:
> -	acpi_processor_thermal_exit(pr, device);
> +	acpi_processor_thermal_exit(pr);
>   err_power_exit:
>   	acpi_processor_power_exit(pr);
>   	return result;
> @@ -203,7 +203,7 @@ static int acpi_processor_stop(struct device *dev)
>   
>   	acpi_cppc_processor_exit(pr);
>   
> -	acpi_processor_thermal_exit(pr, device);
> +	acpi_processor_thermal_exit(pr);
>   
>   	return 0;
>   }
> diff --git a/drivers/acpi/processor_thermal.c b/drivers/acpi/processor_thermal.c
> index c7b1dc5687ec..6cd47551844a 100644
> --- a/drivers/acpi/processor_thermal.c
> +++ b/drivers/acpi/processor_thermal.c
> @@ -311,52 +311,19 @@ const struct thermal_cooling_device_ops processor_cooling_ops = {
>   int acpi_processor_thermal_init(struct acpi_processor *pr,
>   				struct acpi_device *device)
>   {
> -	int result = 0;
> -
> -	pr->cdev = thermal_cooling_device_register("Processor", device,
> -						   &processor_cooling_ops);
> -	if (IS_ERR(pr->cdev)) {
> -		result = PTR_ERR(pr->cdev);
> -		return result;
> -	}
> -
> -	dev_dbg(&device->dev, "registered as cooling_device%d\n",
> -		pr->cdev->id);
> -
> -	result = sysfs_create_link(&device->dev.kobj,
> -				   &pr->cdev->device.kobj,
> -				   "thermal_cooling");
> -	if (result) {
> -		dev_err(&device->dev,
> -			"Failed to create sysfs link 'thermal_cooling'\n");
> -		goto err_thermal_unregister;
> -	}
> +	pr->cdev = thermal_cooling_device_create(pr->dev, "Processor", device,
> +						 &processor_cooling_ops);
> +	if (IS_ERR(pr->cdev))
> +		return PTR_ERR(pr->cdev);
>   
> -	result = sysfs_create_link(&pr->cdev->device.kobj,
> -				   &device->dev.kobj,
> -				   "device");
> -	if (result) {
> -		dev_err(&pr->cdev->device,
> -			"Failed to create sysfs link 'device'\n");
> -		goto err_remove_sysfs_thermal;
> -	}
> +	dev_dbg(pr->dev, "registered as cooling_device%d\n", pr->cdev->id);
>   
>   	return 0;
> -
> -err_remove_sysfs_thermal:
> -	sysfs_remove_link(&device->dev.kobj, "thermal_cooling");
> -err_thermal_unregister:
> -	thermal_cooling_device_unregister(pr->cdev);
> -
> -	return result;
>   }
>   
> -void acpi_processor_thermal_exit(struct acpi_processor *pr,
> -				 struct acpi_device *device)
> +void acpi_processor_thermal_exit(struct acpi_processor *pr)
>   {
> -	if (pr->cdev) {
> -		sysfs_remove_link(&device->dev.kobj, "thermal_cooling");
> -		sysfs_remove_link(&pr->cdev->device.kobj, "device");
> +	if (!IS_ERR_OR_NULL(pr->cdev)) {
>   		thermal_cooling_device_unregister(pr->cdev);
>   		pr->cdev = NULL;
>   	}
> diff --git a/include/acpi/processor.h b/include/acpi/processor.h
> index 554be224ce76..656aaf74fb18 100644
> --- a/include/acpi/processor.h
> +++ b/include/acpi/processor.h
> @@ -429,8 +429,7 @@ int acpi_processor_ffh_lpi_enter(struct acpi_lpi_state *lpi);
>   /* in processor_thermal.c */
>   int acpi_processor_thermal_init(struct acpi_processor *pr,
>   				struct acpi_device *device);
> -void acpi_processor_thermal_exit(struct acpi_processor *pr,
> -				 struct acpi_device *device);
> +void acpi_processor_thermal_exit(struct acpi_processor *pr);
>   extern const struct thermal_cooling_device_ops processor_cooling_ops;
>   #ifdef CONFIG_CPU_FREQ
>   void acpi_thermal_cpufreq_init(struct cpufreq_policy *policy);