[PATCH v1 00/10] ACPI: thermal: Register thermal class cooling devices under parents

[PATCH v1 00/10] ACPI: thermal: Register thermal class cooling devices under parents

[Rafael J. Wysocki]

Hi All,

This series generally changes the way in which thermal class cooling devices
are used by ACPI device drivers registering them (ACPI processor, ACPI fan,
and ACPI video) and by the ACPI thermal zone device driver.

First, it causes the thermal class cooling devices to be registered under
parents in specific locations within the device hierarchy which allows
their relationships with other devices to be recognized more easily.  It
also drops custom sysfs attributes registered by the users of the thermal
class cooling devices because they are not particularly useful any more
when those devices get proper parents.

Second, it modifies the ACPI thermal zone device driver to use thermal class
cooling device parent information for binding cooling devices to trip points
in thermal zones, which was previously done with the help of the devdata
pointers of the cooling devices and effectively prevented drivers from using
the devdata pointers for their own purposes as originally intended.

Finally, it updates the ACPI processor and ACPI fan drivers to use the
devdata pointers of thermal class cooling devices in a bit more efficient
way.

The first two patches in the series are preliminary fixes preventing
Sashiko from reporting "pre-existing issues" in the subsequent patches.

The third patch is a thermal core update allowing thermal class cooling
devices to be registered under proper parents.

The other patches update the ACPI processor, ACPI video, and ACPI fan
drivers as per the above.

The details are covered by individual patch changelogs.

Thanks!

[PATCH v1 01/10] ACPI: fan: Fix memory leak due to leftover devm_kcalloc() argument

[Rafael J. Wysocki]

From: "Rafael J. Wysocki" <rafael.j.wysocki@intel.com>

An ACPI device object's dev field in passed as the first argument to
devm_kcalloc() in acpi_fan_get_fps() which is incorrect and leads to
a memory leak on driver probe errors and removal because the driver
is not bound to that ACPI device.

Address this by replacing that pointer with a pointer to the device the
driver is actually bound to.

While at it, drop a redundant error message after a memory allocation
failure (that also gets printed relative to the ACPI device).

Fixes: d91a1d129b63 ("ACPI: fan: Use platform device for devres-related actions")
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/acpi/fan_core.c | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/drivers/acpi/fan_core.c b/drivers/acpi/fan_core.c
index 624d0736b581..a31d7beca5d0 100644
--- a/drivers/acpi/fan_core.c
+++ b/drivers/acpi/fan_core.c
@@ -284,7 +284,7 @@ static int acpi_fan_speed_cmp(const void *a, const void *b)
 	return fps1->speed - fps2->speed;
 }
 
-static int acpi_fan_get_fps(struct acpi_device *device)
+static int acpi_fan_get_fps(struct device *dev, struct acpi_device *device)
 {
 	struct acpi_fan *fan = acpi_driver_data(device);
 	struct acpi_buffer buffer = { ACPI_ALLOCATE_BUFFER, NULL };
@@ -304,11 +304,8 @@ static int acpi_fan_get_fps(struct acpi_device *device)
 	}
 
 	fan->fps_count = obj->package.count - 1; /* minus revision field */
-	fan->fps = devm_kcalloc(&device->dev,
-				fan->fps_count, sizeof(struct acpi_fan_fps),
-				GFP_KERNEL);
+	fan->fps = devm_kcalloc(dev, fan->fps_count, sizeof(*fan->fps), GFP_KERNEL);
 	if (!fan->fps) {
-		dev_err(&device->dev, "Not enough memory\n");
 		status = -ENOMEM;
 		goto err;
 	}
@@ -522,7 +519,7 @@ static int acpi_fan_probe(struct platform_device *pdev)
 		if (result)
 			return result;
 
-		result = acpi_fan_get_fps(device);
+		result = acpi_fan_get_fps(&pdev->dev, device);
 		if (result)
 			return result;
 	}
-- 
2.51.0

[PATCH v1 02/10] ACPI: video: Fix backlight unregistration ordering

[Rafael J. Wysocki]

From: "Rafael J. Wysocki" <rafael.j.wysocki@intel.com>

In acpi_video_dev_unregister_backlight(), the sysfs interface of the
cooling class device may access the brightness object under the
backlight device's ACPI companion, so that object cannot be freed
before unregistering the cooling class device.

Adjust the code to take that into account.

Fixes:
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/acpi/acpi_video.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/acpi/acpi_video.c b/drivers/acpi/acpi_video.c
index 4d6fd9f6e9ad..6cfe390411c1 100644
--- a/drivers/acpi/acpi_video.c
+++ b/drivers/acpi/acpi_video.c
@@ -1825,6 +1825,12 @@ 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;
+	}
 	if (device->backlight) {
 		backlight_device_unregister(device->backlight);
 		device->backlight = NULL;
@@ -1834,12 +1840,6 @@ static void acpi_video_dev_unregister_backlight(struct acpi_video_device *device
 		kfree(device->brightness);
 		device->brightness = NULL;
 	}
-	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;
-	}
 }
 
 static int acpi_video_bus_unregister_backlight(struct acpi_video_bus *video)
-- 
2.51.0

[PATCH v1 03/10] thermal: core: Introduce thermal_cooling_device_create()

[Rafael J. Wysocki]

From: "Rafael J. Wysocki" <rafael.j.wysocki@intel.com>

Currently, thermal cooling devices have no parents, but it would be
generally useful to be able to create them under specific parents in
the device hierarchy (for instance, it may help to identify the device
representing the actual cooling hardware).

To make that possible, add thermal_cooling_device_create() that will
work like thermal_cooling_device_register() except that it will take
an additional parent argument (which may be NULL).

Redefine thermal_cooling_device_register() as a static inline wrapper
around thermal_cooling_device_create().

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/thermal/thermal_core.c | 28 ++++++++++++++++------------
 drivers/thermal/thermal_core.h |  3 ++-
 drivers/thermal/thermal_of.c   |  2 +-
 include/linux/thermal.h        | 19 ++++++++++++++-----
 4 files changed, 33 insertions(+), 19 deletions(-)

diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c
index 82e2f0d8a26d..ac928c199829 100644
--- a/drivers/thermal/thermal_core.c
+++ b/drivers/thermal/thermal_core.c
@@ -1005,7 +1005,8 @@ thermal_cooling_device_alloc(const char *type, const struct thermal_cooling_devi
 	return ERR_PTR(ret);
 }
 
-int thermal_cooling_device_add(struct thermal_cooling_device *cdev, void *devdata)
+int thermal_cooling_device_add(struct thermal_cooling_device *cdev,
+			       struct device *parent, void *devdata)
 {
 	unsigned long current_state;
 	int ret;
@@ -1013,6 +1014,7 @@ int thermal_cooling_device_add(struct thermal_cooling_device *cdev, void *devdat
 	mutex_init(&cdev->lock);
 	INIT_LIST_HEAD(&cdev->thermal_instances);
 	cdev->updated = false;
+	cdev->device.parent = parent;
 	cdev->device.class = &thermal_class;
 	cdev->device.release = thermal_cdev_release;
 	device_initialize(&cdev->device);
@@ -1062,21 +1064,23 @@ int thermal_cooling_device_add(struct thermal_cooling_device *cdev, void *devdat
 }
 
 /**
- * thermal_cooling_device_register() - register a new thermal cooling device
+ * thermal_cooling_device_create() - register a new thermal cooling device
+ * @parent:	parent device (optional).
  * @type:	the thermal cooling device type.
  * @devdata:	device private data.
  * @ops:	standard thermal cooling devices callbacks.
  *
- * This interface function adds a new thermal cooling device (fan/processor/...)
- * to /sys/class/thermal/ folder as cooling_device[0-*]. It tries to bind itself
- * to all the thermal zone devices registered at the same time.
+ * Allocate and register a new thermal cooling device under the given parent (if
+ * not NULL) and with the given type, device data, and operations.  During the
+ * registration, it will be matched against all of the registered thermal zones
+ * and it will be bound to the matching ones.
  *
- * Return: a pointer to the created struct thermal_cooling_device or an
- * ERR_PTR. Caller must check return value with IS_ERR*() helpers.
+ * Return: A pointer to the created struct thermal_cooling_device or an ERR_PTR.
+ * Callers must use IS_ERR*() helpers to check the return value.
  */
-struct thermal_cooling_device *
-thermal_cooling_device_register(const char *type, void *devdata,
-				const struct thermal_cooling_device_ops *ops)
+struct thermal_cooling_device *thermal_cooling_device_create(
+			struct device *parent, const char *type, void *devdata,
+			const struct thermal_cooling_device_ops *ops)
 {
 	struct thermal_cooling_device *cdev;
 	int ret;
@@ -1085,13 +1089,13 @@ thermal_cooling_device_register(const char *type, void *devdata,
 	if (IS_ERR(cdev))
 		return cdev;
 
-	ret = thermal_cooling_device_add(cdev, devdata);
+	ret = thermal_cooling_device_add(cdev, parent, devdata);
 	if (ret)
 		return ERR_PTR(ret);
 
 	return cdev;
 }
-EXPORT_SYMBOL_GPL(thermal_cooling_device_register);
+EXPORT_SYMBOL_GPL(thermal_cooling_device_create);
 
 static void thermal_cooling_device_release(void *data)
 {
diff --git a/drivers/thermal/thermal_core.h b/drivers/thermal/thermal_core.h
index e98b0aa5aacc..7ef7c6cab437 100644
--- a/drivers/thermal/thermal_core.h
+++ b/drivers/thermal/thermal_core.h
@@ -270,7 +270,8 @@ void thermal_governor_update_tz(struct thermal_zone_device *tz,
 struct thermal_cooling_device *
 thermal_cooling_device_alloc(const char *type, const struct thermal_cooling_device_ops *ops);
 
-int thermal_cooling_device_add(struct thermal_cooling_device *cdev, void *devdata);
+int thermal_cooling_device_add(struct thermal_cooling_device *cdev,
+			       struct device *parent, void *devdata);
 
 /* Helpers */
 #define for_each_trip_desc(__tz, __td)	\
diff --git a/drivers/thermal/thermal_of.c b/drivers/thermal/thermal_of.c
index 0217a49b08ae..14dfac9359b8 100644
--- a/drivers/thermal/thermal_of.c
+++ b/drivers/thermal/thermal_of.c
@@ -561,7 +561,7 @@ thermal_of_cooling_device_register(struct device_node *np, u32 cdev_id,
 	cdev->np = np;
 	cdev->cdev_id = cdev_id;
 
-	ret = thermal_cooling_device_add(cdev, devdata);
+	ret = thermal_cooling_device_add(cdev, NULL, devdata);
 	if (ret)
 		return ERR_PTR(ret);
 
diff --git a/include/linux/thermal.h b/include/linux/thermal.h
index 083b4f533933..306ad17aed89 100644
--- a/include/linux/thermal.h
+++ b/include/linux/thermal.h
@@ -293,8 +293,9 @@ struct device *thermal_zone_device(struct thermal_zone_device *tzd);
 void thermal_zone_device_update(struct thermal_zone_device *,
 				enum thermal_notify_event);
 
-struct thermal_cooling_device *thermal_cooling_device_register(const char *,
-		void *, const struct thermal_cooling_device_ops *);
+struct thermal_cooling_device *thermal_cooling_device_create(
+			struct device *parent, const char *type, void *devdata,
+			const struct thermal_cooling_device_ops *ops);
 
 struct thermal_cooling_device *
 devm_thermal_cooling_device_register(struct device *dev, const char *type, void *devdata,
@@ -340,9 +341,9 @@ static inline void thermal_zone_device_update(struct thermal_zone_device *tz,
 					      enum thermal_notify_event event)
 { }
 
-static inline struct thermal_cooling_device *
-thermal_cooling_device_register(const char *type, void *devdata,
-	const struct thermal_cooling_device_ops *ops)
+static inline struct thermal_cooling_device *thermal_cooling_device_create(
+			struct device *parent, const char *type, void *devdata,
+			const struct thermal_cooling_device_ops *ops)
 { return ERR_PTR(-ENODEV); }
 
 static inline struct thermal_cooling_device *
@@ -391,4 +392,12 @@ static inline void thermal_pm_prepare(void) {}
 static inline void thermal_pm_complete(void) {}
 #endif /* CONFIG_THERMAL */
 
+static inline struct thermal_cooling_device *thermal_cooling_device_register(
+				const char *type, void *devdata,
+				const struct thermal_cooling_device_ops *ops)
+{
+	return thermal_cooling_device_create(NULL, type, devdata, ops);
+}
+
+
 #endif /* __THERMAL_H__ */
-- 
2.51.0

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

[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.

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);
-- 
2.51.0

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

[Rafael J. Wysocki]

From: "Rafael J. Wysocki" <rafael.j.wysocki@intel.com>

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.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 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;
 	}
-- 
2.51.0

[PATCH v1 06/10] ACPI: fan: Use thermal_cooling_device_create()

[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 fan driver, make it use
thermal_cooling_device_create() and pass a pointer to the platform
device representing the given fan 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 that
symbolic link manually.

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
to it.  Accordingly, also remove the code creating that symbolic link.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/acpi/fan_core.c | 24 +-----------------------
 1 file changed, 1 insertion(+), 23 deletions(-)

diff --git a/drivers/acpi/fan_core.c b/drivers/acpi/fan_core.c
index a31d7beca5d0..29ca97503deb 100644
--- a/drivers/acpi/fan_core.c
+++ b/drivers/acpi/fan_core.c
@@ -564,8 +564,7 @@ static int acpi_fan_probe(struct platform_device *pdev)
 	else
 		name = acpi_device_bid(device);
 
-	cdev = thermal_cooling_device_register(name, device,
-						&fan_cooling_ops);
+	cdev = thermal_cooling_device_create(&pdev->dev, name, device, &fan_cooling_ops);
 	if (IS_ERR(cdev)) {
 		result = PTR_ERR(cdev);
 		goto err_end;
@@ -574,28 +573,9 @@ static int acpi_fan_probe(struct platform_device *pdev)
 	dev_dbg(&pdev->dev, "registered as cooling_device%d\n", cdev->id);
 
 	fan->cdev = cdev;
-	result = sysfs_create_link(&pdev->dev.kobj,
-				   &cdev->device.kobj,
-				   "thermal_cooling");
-	if (result) {
-		dev_err(&pdev->dev, "Failed to create sysfs link 'thermal_cooling'\n");
-		goto err_unregister;
-	}
-
-	result = sysfs_create_link(&cdev->device.kobj,
-				   &pdev->dev.kobj,
-				   "device");
-	if (result) {
-		dev_err(&pdev->dev, "Failed to create sysfs link 'device'\n");
-		goto err_remove_link;
-	}
 
 	return 0;
 
-err_remove_link:
-	sysfs_remove_link(&pdev->dev.kobj, "thermal_cooling");
-err_unregister:
-	thermal_cooling_device_unregister(cdev);
 err_end:
 	if (fan->has_fst)
 		acpi_fan_delete_attributes(device);
@@ -612,8 +592,6 @@ static void acpi_fan_remove(struct platform_device *pdev)
 
 		acpi_fan_delete_attributes(device);
 	}
-	sysfs_remove_link(&pdev->dev.kobj, "thermal_cooling");
-	sysfs_remove_link(&fan->cdev->device.kobj, "device");
 	thermal_cooling_device_unregister(fan->cdev);
 }
 
-- 
2.51.0

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

[Rafael J. Wysocki]

From: "Rafael J. Wysocki" <rafael.j.wysocki@intel.com>

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@intel.com>
---
 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);
 
-		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;
 	}
 
-- 
2.51.0

[PATCH v1 08/10] ACPI: processor: thermal: Use more suitable cooling device data

[Rafael J. Wysocki]

From: "Rafael J. Wysocki" <rafael.j.wysocki@intel.com>

Instead of passing an ACPI device object pointer as devdata to
thermal_cooling_device_create(), make acpi_processor_thermal_init()
pass a pointer to the struct acpi_processor representing the given CPU
to it, which allows the callback functions in processor_cooling_ops to
be simplified and the second argument of acpi_processor_thermal_init()
and acpi_processor_thermal_exit() to be dropped.

No intentional functional impact.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/acpi/processor_driver.c  |  2 +-
 drivers/acpi/processor_thermal.c | 35 +++++---------------------------
 include/acpi/processor.h         |  3 +--
 3 files changed, 7 insertions(+), 33 deletions(-)

diff --git a/drivers/acpi/processor_driver.c b/drivers/acpi/processor_driver.c
index 90c14480393d..bdd1529d79f0 100644
--- a/drivers/acpi/processor_driver.c
+++ b/drivers/acpi/processor_driver.c
@@ -164,7 +164,7 @@ static int __acpi_processor_start(struct acpi_device *device)
 
 	acpi_pss_perf_init(pr);
 
-	result = acpi_processor_thermal_init(pr, device);
+	result = acpi_processor_thermal_init(pr);
 	if (result)
 		goto err_power_exit;
 
diff --git a/drivers/acpi/processor_thermal.c b/drivers/acpi/processor_thermal.c
index 6cd47551844a..11036f0d2b94 100644
--- a/drivers/acpi/processor_thermal.c
+++ b/drivers/acpi/processor_thermal.c
@@ -235,15 +235,7 @@ static int
 processor_get_max_state(struct thermal_cooling_device *cdev,
 			unsigned long *state)
 {
-	struct acpi_device *device = cdev->devdata;
-	struct acpi_processor *pr;
-
-	if (!device)
-		return -EINVAL;
-
-	pr = acpi_driver_data(device);
-	if (!pr)
-		return -EINVAL;
+	struct acpi_processor *pr = cdev->devdata;
 
 	*state = acpi_processor_max_state(pr);
 	return 0;
@@ -253,15 +245,7 @@ static int
 processor_get_cur_state(struct thermal_cooling_device *cdev,
 			unsigned long *cur_state)
 {
-	struct acpi_device *device = cdev->devdata;
-	struct acpi_processor *pr;
-
-	if (!device)
-		return -EINVAL;
-
-	pr = acpi_driver_data(device);
-	if (!pr)
-		return -EINVAL;
+	struct acpi_processor *pr = cdev->devdata;
 
 	*cur_state = cpufreq_get_cur_state(pr->id);
 	if (pr->flags.throttling)
@@ -273,18 +257,10 @@ static int
 processor_set_cur_state(struct thermal_cooling_device *cdev,
 			unsigned long state)
 {
-	struct acpi_device *device = cdev->devdata;
-	struct acpi_processor *pr;
+	struct acpi_processor *pr = cdev->devdata;
 	int result = 0;
 	int max_pstate;
 
-	if (!device)
-		return -EINVAL;
-
-	pr = acpi_driver_data(device);
-	if (!pr)
-		return -EINVAL;
-
 	max_pstate = cpufreq_get_max_state(pr->id);
 
 	if (state > acpi_processor_max_state(pr))
@@ -308,10 +284,9 @@ const struct thermal_cooling_device_ops processor_cooling_ops = {
 	.set_cur_state = processor_set_cur_state,
 };
 
-int acpi_processor_thermal_init(struct acpi_processor *pr,
-				struct acpi_device *device)
+int acpi_processor_thermal_init(struct acpi_processor *pr)
 {
-	pr->cdev = thermal_cooling_device_create(pr->dev, "Processor", device,
+	pr->cdev = thermal_cooling_device_create(pr->dev, "Processor", pr,
 						 &processor_cooling_ops);
 	if (IS_ERR(pr->cdev))
 		return PTR_ERR(pr->cdev);
diff --git a/include/acpi/processor.h b/include/acpi/processor.h
index 656aaf74fb18..b5447af4d40b 100644
--- a/include/acpi/processor.h
+++ b/include/acpi/processor.h
@@ -427,8 +427,7 @@ int acpi_processor_ffh_lpi_enter(struct acpi_lpi_state *lpi);
 #endif /* CONFIG_ACPI_PROCESSOR_IDLE */
 
 /* in processor_thermal.c */
-int acpi_processor_thermal_init(struct acpi_processor *pr,
-				struct acpi_device *device);
+int acpi_processor_thermal_init(struct acpi_processor *pr);
 void acpi_processor_thermal_exit(struct acpi_processor *pr);
 extern const struct thermal_cooling_device_ops processor_cooling_ops;
 #ifdef CONFIG_CPU_FREQ
-- 
2.51.0

[PATCH v1 09/10] ACPI: fan: Store ACPI device pointer in struct acpi_fan

[Rafael J. Wysocki]

From: "Rafael J. Wysocki" <rafael.j.wysocki@intel.com>

In preparation for subsequent changes, instead of storing a fan ACPI
handle in struct acpi_fan, store a pointer to the corresponding struct
acpi_device in it.

No intentional functional impact.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/acpi/fan.h       |  2 +-
 drivers/acpi/fan_core.c  | 11 ++++++-----
 drivers/acpi/fan_hwmon.c |  2 +-
 3 files changed, 8 insertions(+), 7 deletions(-)

diff --git a/drivers/acpi/fan.h b/drivers/acpi/fan.h
index e20d6ad9df80..3faa247af514 100644
--- a/drivers/acpi/fan.h
+++ b/drivers/acpi/fan.h
@@ -52,7 +52,7 @@ struct acpi_fan_fst {
 };
 
 struct acpi_fan {
-	acpi_handle handle;
+	struct acpi_device *adev;
 	bool acpi4;
 	bool has_fst;
 	struct acpi_fan_fif fif;
diff --git a/drivers/acpi/fan_core.c b/drivers/acpi/fan_core.c
index 29ca97503deb..8b20b8a55c13 100644
--- a/drivers/acpi/fan_core.c
+++ b/drivers/acpi/fan_core.c
@@ -340,17 +340,18 @@ static int acpi_fan_dsm_init(struct device *dev)
 		},
 	};
 	struct acpi_fan *fan = dev_get_drvdata(dev);
+	acpi_handle fan_handle = fan->adev->handle;
 	union acpi_object *obj;
 	int ret = 0;
 
-	if (!acpi_check_dsm(fan->handle, &acpi_fan_microsoft_guid, 0,
+	if (!acpi_check_dsm(fan_handle, &acpi_fan_microsoft_guid, 0,
 			    BIT(ACPI_FAN_DSM_GET_TRIP_POINT_GRANULARITY) |
 			    BIT(ACPI_FAN_DSM_SET_TRIP_POINTS)))
 		return 0;
 
 	dev_info(dev, "Using Microsoft fan extensions\n");
 
-	obj = acpi_evaluate_dsm_typed(fan->handle, &acpi_fan_microsoft_guid, 0,
+	obj = acpi_evaluate_dsm_typed(fan_handle, &acpi_fan_microsoft_guid, 0,
 				      ACPI_FAN_DSM_GET_TRIP_POINT_GRANULARITY, &dummy,
 				      ACPI_TYPE_INTEGER);
 	if (!obj)
@@ -392,8 +393,8 @@ static int acpi_fan_dsm_set_trip_points(struct device *dev, u64 upper, u64 lower
 	};
 	union acpi_object *obj;
 
-	obj = acpi_evaluate_dsm(fan->handle, &acpi_fan_microsoft_guid, 0,
-				ACPI_FAN_DSM_SET_TRIP_POINTS, &in);
+	obj = acpi_evaluate_dsm(fan->adev->handle, &acpi_fan_microsoft_guid,
+				0, ACPI_FAN_DSM_SET_TRIP_POINTS, &in);
 	kfree(obj);
 
 	return 0;
@@ -503,7 +504,7 @@ static int acpi_fan_probe(struct platform_device *pdev)
 		return -ENOMEM;
 	}
 
-	fan->handle = device->handle;
+	fan->adev = device;
 	device->driver_data = fan;
 	platform_set_drvdata(pdev, fan);
 
diff --git a/drivers/acpi/fan_hwmon.c b/drivers/acpi/fan_hwmon.c
index d3374f8f524b..c5d8419f41c5 100644
--- a/drivers/acpi/fan_hwmon.c
+++ b/drivers/acpi/fan_hwmon.c
@@ -94,7 +94,7 @@ static int acpi_fan_hwmon_read(struct device *dev, enum hwmon_sensor_types type,
 	struct acpi_fan_fst fst;
 	int ret;
 
-	ret = acpi_fan_get_fst(fan->handle, &fst);
+	ret = acpi_fan_get_fst(fan->adev->handle, &fst);
 	if (ret < 0)
 		return ret;
 
-- 
2.51.0

[PATCH v1 10/10] ACPI: fan: Use more suitable cooling device data

[Rafael J. Wysocki]

From: "Rafael J. Wysocki" <rafael.j.wysocki@intel.com>

Instead of passing an ACPI device object pointer as devdata to
thermal_cooling_device_create(), make acpi_fan_probe() pass a pointer
to struct acpi_fan to it, which allows the callback functions in
fan_cooling_ops to be simplified.

Also avoid using acpi_driver_data() in two functions invoked by the
cooling device callbacks by passing struct acpi_fan pointers instead
of struct acpi_device pointers to them.

No intentional functional impact.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/acpi/fan_core.c | 27 ++++++++++++---------------
 1 file changed, 12 insertions(+), 15 deletions(-)

diff --git a/drivers/acpi/fan_core.c b/drivers/acpi/fan_core.c
index 8b20b8a55c13..5ad65975fbb4 100644
--- a/drivers/acpi/fan_core.c
+++ b/drivers/acpi/fan_core.c
@@ -54,8 +54,7 @@ MODULE_DEVICE_TABLE(acpi, fan_device_ids);
 static int fan_get_max_state(struct thermal_cooling_device *cdev, unsigned long
 			     *state)
 {
-	struct acpi_device *device = cdev->devdata;
-	struct acpi_fan *fan = acpi_driver_data(device);
+	struct acpi_fan *fan = cdev->devdata;
 
 	if (fan->acpi4) {
 		if (fan->fif.fine_grain_ctrl)
@@ -105,9 +104,9 @@ int acpi_fan_get_fst(acpi_handle handle, struct acpi_fan_fst *fst)
 	return ret;
 }
 
-static int fan_get_state_acpi4(struct acpi_device *device, unsigned long *state)
+static int fan_get_state_acpi4(struct acpi_fan *fan, unsigned long *state)
 {
-	struct acpi_fan *fan = acpi_driver_data(device);
+	struct acpi_device *device = fan->adev;
 	struct acpi_fan_fst fst;
 	int status, i;
 
@@ -159,13 +158,12 @@ static int fan_get_state(struct acpi_device *device, unsigned long *state)
 static int fan_get_cur_state(struct thermal_cooling_device *cdev, unsigned long
 			     *state)
 {
-	struct acpi_device *device = cdev->devdata;
-	struct acpi_fan *fan = acpi_driver_data(device);
+	struct acpi_fan *fan = cdev->devdata;
 
 	if (fan->acpi4)
-		return fan_get_state_acpi4(device, state);
+		return fan_get_state_acpi4(fan, state);
 	else
-		return fan_get_state(device, state);
+		return fan_get_state(fan->adev, state);
 }
 
 static int fan_set_state(struct acpi_device *device, unsigned long state)
@@ -177,9 +175,9 @@ static int fan_set_state(struct acpi_device *device, unsigned long state)
 				     state ? ACPI_STATE_D0 : ACPI_STATE_D3_COLD);
 }
 
-static int fan_set_state_acpi4(struct acpi_device *device, unsigned long state)
+static int fan_set_state_acpi4(struct acpi_fan *fan, unsigned long state)
 {
-	struct acpi_fan *fan = acpi_driver_data(device);
+	struct acpi_device *device = fan->adev;
 	acpi_status status;
 	u64 value = state;
 	int max_state;
@@ -213,13 +211,12 @@ static int fan_set_state_acpi4(struct acpi_device *device, unsigned long state)
 static int
 fan_set_cur_state(struct thermal_cooling_device *cdev, unsigned long state)
 {
-	struct acpi_device *device = cdev->devdata;
-	struct acpi_fan *fan = acpi_driver_data(device);
+	struct acpi_fan *fan = cdev->devdata;
 
 	if (fan->acpi4)
-		return fan_set_state_acpi4(device, state);
+		return fan_set_state_acpi4(fan, state);
 	else
-		return fan_set_state(device, state);
+		return fan_set_state(fan->adev, state);
 }
 
 static const struct thermal_cooling_device_ops fan_cooling_ops = {
@@ -565,7 +562,7 @@ static int acpi_fan_probe(struct platform_device *pdev)
 	else
 		name = acpi_device_bid(device);
 
-	cdev = thermal_cooling_device_create(&pdev->dev, name, device, &fan_cooling_ops);
+	cdev = thermal_cooling_device_create(&pdev->dev, name, fan, &fan_cooling_ops);
 	if (IS_ERR(cdev)) {
 		result = PTR_ERR(cdev);
 		goto err_end;
-- 
2.51.0

Re: [PATCH v1 02/10] ACPI: video: Fix backlight unregistration ordering

[Andy Shevchenko]

On Fri, Sep 11, 2026 at 03:01:30PM +0200, Rafael J. Wysocki wrote:

> In acpi_video_dev_unregister_backlight(), the sysfs interface of the
> cooling class device may access the brightness object under the
> backlight device's ACPI companion, so that object cannot be freed
> before unregistering the cooling class device.
> 
> Adjust the code to take that into account.
> 
> Fixes:

Fixes?

> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v1 01/10] ACPI: fan: Fix memory leak due to leftover devm_kcalloc() argument

[Andy Shevchenko]

On Fri, Sep 11, 2026 at 03:00:51PM +0200, Rafael J. Wysocki wrote:

> An ACPI device object's dev field in passed as the first argument to
> devm_kcalloc() in acpi_fan_get_fps() which is incorrect and leads to
> a memory leak on driver probe errors and removal because the driver
> is not bound to that ACPI device.
> 
> Address this by replacing that pointer with a pointer to the device the
> driver is actually bound to.
> 
> While at it, drop a redundant error message after a memory allocation
> failure (that also gets printed relative to the ACPI device).

...

>  	fan->fps_count = obj->package.count - 1; /* minus revision field */
> -	fan->fps = devm_kcalloc(&device->dev,
> -				fan->fps_count, sizeof(struct acpi_fan_fps),
> -				GFP_KERNEL);
> +	fan->fps = devm_kcalloc(dev, fan->fps_count, sizeof(*fan->fps), GFP_KERNEL);
>  	if (!fan->fps) {
> -		dev_err(&device->dev, "Not enough memory\n");
>  		status = -ENOMEM;
>  		goto err;

I was about ranting on goto after devm_*(), but looking at the context,
I understand why it's not a problem. While at it, a side note: perhaps it makes
sense to use ACPI_FREE(obj) instead of kfree()? Or even better to have __free()
version of it, so we can declare the object with autoclean.

-- 
With Best Regards,
Andy Shevchenko

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

[Andy Shevchenko]

On Fri, Sep 11, 2026 at 03:02:53PM +0200, Rafael J. Wysocki wrote:

> 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

Typo: dereferencing

> mistakenly.

...

Is any user space ABI breakage expected as an outcome of this change?

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v1 02/10] ACPI: video: Fix backlight unregistration ordering

[Rafael J. Wysocki (Intel)]

On Fri, Sep 11, 2026 at 6:27 PM Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
>
> On Fri, Sep 11, 2026 at 03:01:30PM +0200, Rafael J. Wysocki wrote:
>
> > In acpi_video_dev_unregister_backlight(), the sysfs interface of the
> > cooling class device may access the brightness object under the
> > backlight device's ACPI companion, so that object cannot be freed
> > before unregistering the cooling class device.
> >
> > Adjust the code to take that into account.
> >
> > Fixes:
>
> Fixes?

I thought I added it, but clearly I didn't.

It should be

Fixes: 67b662e189f4 ("ACPI / video: seperate backlight control and
event interface")

> > Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

Will fix when applying unless there will be a v2.

Thanks!

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

[Andy Shevchenko]

On Fri, Sep 11, 2026 at 03:05:07PM +0200, Rafael J. Wysocki wrote:

> 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.

...

> +	parent_handle = ACPI_HANDLE(parent);
>  
> -		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)

device_match_acpi_handle() ?

		if (device_match_acpi_handle(parent, acpi_trip->devices.handles[i]))

>  			return true;
>  	}

-- 
With Best Regards,
Andy Shevchenko

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

[Rafael J. Wysocki (Intel)]

On Fri, Sep 11, 2026 at 6:32 PM Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
>
> On Fri, Sep 11, 2026 at 03:02:53PM +0200, Rafael J. Wysocki wrote:
>
> > 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
>
> Typo: dereferencing

Noted, thanks!

> > mistakenly.
>
> ...
>
> Is any user space ABI breakage expected as an outcome of this change?

Not really.

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

[Rafael J. Wysocki (Intel)]

On Fri, Sep 11, 2026 at 6:36 PM Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
>
> On Fri, Sep 11, 2026 at 03:05:07PM +0200, Rafael J. Wysocki wrote:
>
> > 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.
>
> ...
>
> > +     parent_handle = ACPI_HANDLE(parent);
> >
> > -             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)
>
> device_match_acpi_handle() ?

That would cause ACPI_HANDLE() to be evaluated
acpi_trip->devices.count times for the parent whereas only one
evaluation is necessary, so not really.

>                 if (device_match_acpi_handle(parent, acpi_trip->devices.handles[i]))
>
> >                       return true;
> >       }

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

[Andy Shevchenko]

On Fri, Sep 11, 2026 at 06:36:28PM +0200, Rafael J. Wysocki (Intel) wrote:
> On Fri, Sep 11, 2026 at 6:32 PM Andy Shevchenko
> <andriy.shevchenko@linux.intel.com> wrote:
> > On Fri, Sep 11, 2026 at 03:02:53PM +0200, Rafael J. Wysocki wrote:

...

> > Is any user space ABI breakage expected as an outcome of this change?
> 
> Not really.

Maybe makes sense to state this clearly in the cover letter?

-- 
With Best Regards,
Andy Shevchenko

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

[Rafael J. Wysocki (Intel)]

On Fri, Sep 11, 2026 at 6:47 PM Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
>
> On Fri, Sep 11, 2026 at 06:36:28PM +0200, Rafael J. Wysocki (Intel) wrote:
> > On Fri, Sep 11, 2026 at 6:32 PM Andy Shevchenko
> > <andriy.shevchenko@linux.intel.com> wrote:
> > > On Fri, Sep 11, 2026 at 03:02:53PM +0200, Rafael J. Wysocki wrote:
>
> ...
>
> > > Is any user space ABI breakage expected as an outcome of this change?
> >
> > Not really.
>
> Maybe makes sense to state this clearly in the cover letter?

I guess you specifically mean the manually created symbolic links in
sysfs that get removed.

If so, I'm not aware of anyone using them for anything, but the lack
of observation is not proof of nonexistence.

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

[Andy Shevchenko]

On Fri, Sep 11, 2026 at 07:01:23PM +0200, Rafael J. Wysocki (Intel) wrote:
> On Fri, Sep 11, 2026 at 6:47 PM Andy Shevchenko
> <andriy.shevchenko@linux.intel.com> wrote:
> > On Fri, Sep 11, 2026 at 06:36:28PM +0200, Rafael J. Wysocki (Intel) wrote:
> > > On Fri, Sep 11, 2026 at 6:32 PM Andy Shevchenko
> > > <andriy.shevchenko@linux.intel.com> wrote:
> > > > On Fri, Sep 11, 2026 at 03:02:53PM +0200, Rafael J. Wysocki wrote:

...

> > > > Is any user space ABI breakage expected as an outcome of this change?
> > >
> > > Not really.
> >
> > Maybe makes sense to state this clearly in the cover letter?
> 
> I guess you specifically mean the manually created symbolic links in
> sysfs that get removed.
> 
> If so, I'm not aware of anyone using them for anything, but the lack
> of observation is not proof of nonexistence.

Yes, I refer to the changes in the sysfs layout / presence / absence.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v1 01/10] ACPI: fan: Fix memory leak due to leftover devm_kcalloc() argument

[Armin Wolf]

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

> From: "Rafael J. Wysocki" <rafael.j.wysocki@intel.com>
>
> An ACPI device object's dev field in passed as the first argument to
> devm_kcalloc() in acpi_fan_get_fps() which is incorrect and leads to
> a memory leak on driver probe errors and removal because the driver
> is not bound to that ACPI device.
>
> Address this by replacing that pointer with a pointer to the device the
> driver is actually bound to.
>
> While at it, drop a redundant error message after a memory allocation
> failure (that also gets printed relative to the ACPI device).

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

> Fixes: d91a1d129b63 ("ACPI: fan: Use platform device for devres-related actions")
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> ---
>   drivers/acpi/fan_core.c | 9 +++------
>   1 file changed, 3 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/acpi/fan_core.c b/drivers/acpi/fan_core.c
> index 624d0736b581..a31d7beca5d0 100644
> --- a/drivers/acpi/fan_core.c
> +++ b/drivers/acpi/fan_core.c
> @@ -284,7 +284,7 @@ static int acpi_fan_speed_cmp(const void *a, const void *b)
>   	return fps1->speed - fps2->speed;
>   }
>   
> -static int acpi_fan_get_fps(struct acpi_device *device)
> +static int acpi_fan_get_fps(struct device *dev, struct acpi_device *device)
>   {
>   	struct acpi_fan *fan = acpi_driver_data(device);
>   	struct acpi_buffer buffer = { ACPI_ALLOCATE_BUFFER, NULL };
> @@ -304,11 +304,8 @@ static int acpi_fan_get_fps(struct acpi_device *device)
>   	}
>   
>   	fan->fps_count = obj->package.count - 1; /* minus revision field */
> -	fan->fps = devm_kcalloc(&device->dev,
> -				fan->fps_count, sizeof(struct acpi_fan_fps),
> -				GFP_KERNEL);
> +	fan->fps = devm_kcalloc(dev, fan->fps_count, sizeof(*fan->fps), GFP_KERNEL);
>   	if (!fan->fps) {
> -		dev_err(&device->dev, "Not enough memory\n");
>   		status = -ENOMEM;
>   		goto err;
>   	}
> @@ -522,7 +519,7 @@ static int acpi_fan_probe(struct platform_device *pdev)
>   		if (result)
>   			return result;
>   
> -		result = acpi_fan_get_fps(device);
> +		result = acpi_fan_get_fps(&pdev->dev, device);
>   		if (result)
>   			return result;
>   	}

Re: [PATCH v1 02/10] ACPI: video: Fix backlight unregistration ordering

[Armin Wolf]

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

> From: "Rafael J. Wysocki" <rafael.j.wysocki@intel.com>
>
> In acpi_video_dev_unregister_backlight(), the sysfs interface of the
> cooling class device may access the brightness object under the
> backlight device's ACPI companion, so that object cannot be freed
> before unregistering the cooling class device.
>
> Adjust the code to take that into account.

Ignoring the broken fixes tag already found by Andy:

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

> Fixes:
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> ---
>   drivers/acpi/acpi_video.c | 12 ++++++------
>   1 file changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/acpi/acpi_video.c b/drivers/acpi/acpi_video.c
> index 4d6fd9f6e9ad..6cfe390411c1 100644
> --- a/drivers/acpi/acpi_video.c
> +++ b/drivers/acpi/acpi_video.c
> @@ -1825,6 +1825,12 @@ 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;
> +	}
>   	if (device->backlight) {
>   		backlight_device_unregister(device->backlight);
>   		device->backlight = NULL;
> @@ -1834,12 +1840,6 @@ static void acpi_video_dev_unregister_backlight(struct acpi_video_device *device
>   		kfree(device->brightness);
>   		device->brightness = NULL;
>   	}
> -	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;
> -	}
>   }
>   
>   static int acpi_video_bus_unregister_backlight(struct acpi_video_bus *video)

Re: [PATCH v1 03/10] thermal: core: Introduce thermal_cooling_device_create()

[Armin Wolf]

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

> From: "Rafael J. Wysocki" <rafael.j.wysocki@intel.com>
>
> Currently, thermal cooling devices have no parents, but it would be
> generally useful to be able to create them under specific parents in
> the device hierarchy (for instance, it may help to identify the device
> representing the actual cooling hardware).
>
> To make that possible, add thermal_cooling_device_create() that will
> work like thermal_cooling_device_register() except that it will take
> an additional parent argument (which may be NULL).
>
> Redefine thermal_cooling_device_register() as a static inline wrapper
> around thermal_cooling_device_create().

Hi,

i like your idea with the new registration function, this gives us more time
for migrating the other drivers. Should we mark the original function as
deprecated?

For the patch itself:

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

> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> ---
>   drivers/thermal/thermal_core.c | 28 ++++++++++++++++------------
>   drivers/thermal/thermal_core.h |  3 ++-
>   drivers/thermal/thermal_of.c   |  2 +-
>   include/linux/thermal.h        | 19 ++++++++++++++-----
>   4 files changed, 33 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c
> index 82e2f0d8a26d..ac928c199829 100644
> --- a/drivers/thermal/thermal_core.c
> +++ b/drivers/thermal/thermal_core.c
> @@ -1005,7 +1005,8 @@ thermal_cooling_device_alloc(const char *type, const struct thermal_cooling_devi
>   	return ERR_PTR(ret);
>   }
>   
> -int thermal_cooling_device_add(struct thermal_cooling_device *cdev, void *devdata)
> +int thermal_cooling_device_add(struct thermal_cooling_device *cdev,
> +			       struct device *parent, void *devdata)
>   {
>   	unsigned long current_state;
>   	int ret;
> @@ -1013,6 +1014,7 @@ int thermal_cooling_device_add(struct thermal_cooling_device *cdev, void *devdat
>   	mutex_init(&cdev->lock);
>   	INIT_LIST_HEAD(&cdev->thermal_instances);
>   	cdev->updated = false;
> +	cdev->device.parent = parent;
>   	cdev->device.class = &thermal_class;
>   	cdev->device.release = thermal_cdev_release;
>   	device_initialize(&cdev->device);
> @@ -1062,21 +1064,23 @@ int thermal_cooling_device_add(struct thermal_cooling_device *cdev, void *devdat
>   }
>   
>   /**
> - * thermal_cooling_device_register() - register a new thermal cooling device
> + * thermal_cooling_device_create() - register a new thermal cooling device
> + * @parent:	parent device (optional).
>    * @type:	the thermal cooling device type.
>    * @devdata:	device private data.
>    * @ops:	standard thermal cooling devices callbacks.
>    *
> - * This interface function adds a new thermal cooling device (fan/processor/...)
> - * to /sys/class/thermal/ folder as cooling_device[0-*]. It tries to bind itself
> - * to all the thermal zone devices registered at the same time.
> + * Allocate and register a new thermal cooling device under the given parent (if
> + * not NULL) and with the given type, device data, and operations.  During the
> + * registration, it will be matched against all of the registered thermal zones
> + * and it will be bound to the matching ones.
>    *
> - * Return: a pointer to the created struct thermal_cooling_device or an
> - * ERR_PTR. Caller must check return value with IS_ERR*() helpers.
> + * Return: A pointer to the created struct thermal_cooling_device or an ERR_PTR.
> + * Callers must use IS_ERR*() helpers to check the return value.
>    */
> -struct thermal_cooling_device *
> -thermal_cooling_device_register(const char *type, void *devdata,
> -				const struct thermal_cooling_device_ops *ops)
> +struct thermal_cooling_device *thermal_cooling_device_create(
> +			struct device *parent, const char *type, void *devdata,
> +			const struct thermal_cooling_device_ops *ops)
>   {
>   	struct thermal_cooling_device *cdev;
>   	int ret;
> @@ -1085,13 +1089,13 @@ thermal_cooling_device_register(const char *type, void *devdata,
>   	if (IS_ERR(cdev))
>   		return cdev;
>   
> -	ret = thermal_cooling_device_add(cdev, devdata);
> +	ret = thermal_cooling_device_add(cdev, parent, devdata);
>   	if (ret)
>   		return ERR_PTR(ret);
>   
>   	return cdev;
>   }
> -EXPORT_SYMBOL_GPL(thermal_cooling_device_register);
> +EXPORT_SYMBOL_GPL(thermal_cooling_device_create);
>   
>   static void thermal_cooling_device_release(void *data)
>   {
> diff --git a/drivers/thermal/thermal_core.h b/drivers/thermal/thermal_core.h
> index e98b0aa5aacc..7ef7c6cab437 100644
> --- a/drivers/thermal/thermal_core.h
> +++ b/drivers/thermal/thermal_core.h
> @@ -270,7 +270,8 @@ void thermal_governor_update_tz(struct thermal_zone_device *tz,
>   struct thermal_cooling_device *
>   thermal_cooling_device_alloc(const char *type, const struct thermal_cooling_device_ops *ops);
>   
> -int thermal_cooling_device_add(struct thermal_cooling_device *cdev, void *devdata);
> +int thermal_cooling_device_add(struct thermal_cooling_device *cdev,
> +			       struct device *parent, void *devdata);
>   
>   /* Helpers */
>   #define for_each_trip_desc(__tz, __td)	\
> diff --git a/drivers/thermal/thermal_of.c b/drivers/thermal/thermal_of.c
> index 0217a49b08ae..14dfac9359b8 100644
> --- a/drivers/thermal/thermal_of.c
> +++ b/drivers/thermal/thermal_of.c
> @@ -561,7 +561,7 @@ thermal_of_cooling_device_register(struct device_node *np, u32 cdev_id,
>   	cdev->np = np;
>   	cdev->cdev_id = cdev_id;
>   
> -	ret = thermal_cooling_device_add(cdev, devdata);
> +	ret = thermal_cooling_device_add(cdev, NULL, devdata);
>   	if (ret)
>   		return ERR_PTR(ret);
>   
> diff --git a/include/linux/thermal.h b/include/linux/thermal.h
> index 083b4f533933..306ad17aed89 100644
> --- a/include/linux/thermal.h
> +++ b/include/linux/thermal.h
> @@ -293,8 +293,9 @@ struct device *thermal_zone_device(struct thermal_zone_device *tzd);
>   void thermal_zone_device_update(struct thermal_zone_device *,
>   				enum thermal_notify_event);
>   
> -struct thermal_cooling_device *thermal_cooling_device_register(const char *,
> -		void *, const struct thermal_cooling_device_ops *);
> +struct thermal_cooling_device *thermal_cooling_device_create(
> +			struct device *parent, const char *type, void *devdata,
> +			const struct thermal_cooling_device_ops *ops);
>   
>   struct thermal_cooling_device *
>   devm_thermal_cooling_device_register(struct device *dev, const char *type, void *devdata,
> @@ -340,9 +341,9 @@ static inline void thermal_zone_device_update(struct thermal_zone_device *tz,
>   					      enum thermal_notify_event event)
>   { }
>   
> -static inline struct thermal_cooling_device *
> -thermal_cooling_device_register(const char *type, void *devdata,
> -	const struct thermal_cooling_device_ops *ops)
> +static inline struct thermal_cooling_device *thermal_cooling_device_create(
> +			struct device *parent, const char *type, void *devdata,
> +			const struct thermal_cooling_device_ops *ops)
>   { return ERR_PTR(-ENODEV); }
>   
>   static inline struct thermal_cooling_device *
> @@ -391,4 +392,12 @@ static inline void thermal_pm_prepare(void) {}
>   static inline void thermal_pm_complete(void) {}
>   #endif /* CONFIG_THERMAL */
>   
> +static inline struct thermal_cooling_device *thermal_cooling_device_register(
> +				const char *type, void *devdata,
> +				const struct thermal_cooling_device_ops *ops)
> +{
> +	return thermal_cooling_device_create(NULL, type, devdata, ops);
> +}
> +
> +
>   #endif /* __THERMAL_H__ */

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

[Armin Wolf]

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);

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

[Armin Wolf]

Am 11.09.26 um 20:06 schrieb Andy Shevchenko:

> On Fri, Sep 11, 2026 at 07:01:23PM +0200, Rafael J. Wysocki (Intel) wrote:
>> On Fri, Sep 11, 2026 at 6:47 PM Andy Shevchenko
>> <andriy.shevchenko@linux.intel.com> wrote:
>>> On Fri, Sep 11, 2026 at 06:36:28PM +0200, Rafael J. Wysocki (Intel) wrote:
>>>> On Fri, Sep 11, 2026 at 6:32 PM Andy Shevchenko
>>>> <andriy.shevchenko@linux.intel.com> wrote:
>>>>> On Fri, Sep 11, 2026 at 03:02:53PM +0200, Rafael J. Wysocki wrote:
> ...
>
>>>>> Is any user space ABI breakage expected as an outcome of this change?
>>>> Not really.
>>> Maybe makes sense to state this clearly in the cover letter?
>> I guess you specifically mean the manually created symbolic links in
>> sysfs that get removed.
>>
>> If so, I'm not aware of anyone using them for anything, but the lack
>> of observation is not proof of nonexistence.
> Yes, I refer to the changes in the sysfs layout / presence / absence.

AFAIK those custom sysfs attributes are not even documented under Documentation/ABI, and
thermald also does not seem to use them. I thus think we can safely remove them.

Thanks,
Armin Wolf

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@intel.com>
>
> 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@gmx.de>

> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> ---
>   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;
>   	}

Re: [PATCH v1 06/10] ACPI: fan: Use thermal_cooling_device_create()

[Armin Wolf]

Am 11.09.26 um 15:04 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 fan driver, make it use
> thermal_cooling_device_create() and pass a pointer to the platform
> device representing the given fan 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 that
> symbolic link manually.
>
> 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
> to it.  Accordingly, also remove the code creating that symbolic link.

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

> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> ---
>   drivers/acpi/fan_core.c | 24 +-----------------------
>   1 file changed, 1 insertion(+), 23 deletions(-)
>
> diff --git a/drivers/acpi/fan_core.c b/drivers/acpi/fan_core.c
> index a31d7beca5d0..29ca97503deb 100644
> --- a/drivers/acpi/fan_core.c
> +++ b/drivers/acpi/fan_core.c
> @@ -564,8 +564,7 @@ static int acpi_fan_probe(struct platform_device *pdev)
>   	else
>   		name = acpi_device_bid(device);
>   
> -	cdev = thermal_cooling_device_register(name, device,
> -						&fan_cooling_ops);
> +	cdev = thermal_cooling_device_create(&pdev->dev, name, device, &fan_cooling_ops);
>   	if (IS_ERR(cdev)) {
>   		result = PTR_ERR(cdev);
>   		goto err_end;
> @@ -574,28 +573,9 @@ static int acpi_fan_probe(struct platform_device *pdev)
>   	dev_dbg(&pdev->dev, "registered as cooling_device%d\n", cdev->id);
>   
>   	fan->cdev = cdev;
> -	result = sysfs_create_link(&pdev->dev.kobj,
> -				   &cdev->device.kobj,
> -				   "thermal_cooling");
> -	if (result) {
> -		dev_err(&pdev->dev, "Failed to create sysfs link 'thermal_cooling'\n");
> -		goto err_unregister;
> -	}
> -
> -	result = sysfs_create_link(&cdev->device.kobj,
> -				   &pdev->dev.kobj,
> -				   "device");
> -	if (result) {
> -		dev_err(&pdev->dev, "Failed to create sysfs link 'device'\n");
> -		goto err_remove_link;
> -	}
>   
>   	return 0;
>   
> -err_remove_link:
> -	sysfs_remove_link(&pdev->dev.kobj, "thermal_cooling");
> -err_unregister:
> -	thermal_cooling_device_unregister(cdev);
>   err_end:
>   	if (fan->has_fst)
>   		acpi_fan_delete_attributes(device);
> @@ -612,8 +592,6 @@ static void acpi_fan_remove(struct platform_device *pdev)
>   
>   		acpi_fan_delete_attributes(device);
>   	}
> -	sysfs_remove_link(&pdev->dev.kobj, "thermal_cooling");
> -	sysfs_remove_link(&fan->cdev->device.kobj, "device");
>   	thermal_cooling_device_unregister(fan->cdev);
>   }
>   

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@intel.com>
>
> 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@intel.com>
> ---
>   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@gmx.de>

> -		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;
>   	}
>   

Re: [PATCH v1 08/10] ACPI: processor: thermal: Use more suitable cooling device data

[Armin Wolf]

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

> From: "Rafael J. Wysocki" <rafael.j.wysocki@intel.com>
>
> Instead of passing an ACPI device object pointer as devdata to
> thermal_cooling_device_create(), make acpi_processor_thermal_init()
> pass a pointer to the struct acpi_processor representing the given CPU
> to it, which allows the callback functions in processor_cooling_ops to
> be simplified and the second argument of acpi_processor_thermal_init()
> and acpi_processor_thermal_exit() to be dropped.
>
> No intentional functional impact.

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

>
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> ---
>   drivers/acpi/processor_driver.c  |  2 +-
>   drivers/acpi/processor_thermal.c | 35 +++++---------------------------
>   include/acpi/processor.h         |  3 +--
>   3 files changed, 7 insertions(+), 33 deletions(-)
>
> diff --git a/drivers/acpi/processor_driver.c b/drivers/acpi/processor_driver.c
> index 90c14480393d..bdd1529d79f0 100644
> --- a/drivers/acpi/processor_driver.c
> +++ b/drivers/acpi/processor_driver.c
> @@ -164,7 +164,7 @@ static int __acpi_processor_start(struct acpi_device *device)
>   
>   	acpi_pss_perf_init(pr);
>   
> -	result = acpi_processor_thermal_init(pr, device);
> +	result = acpi_processor_thermal_init(pr);
>   	if (result)
>   		goto err_power_exit;
>   
> diff --git a/drivers/acpi/processor_thermal.c b/drivers/acpi/processor_thermal.c
> index 6cd47551844a..11036f0d2b94 100644
> --- a/drivers/acpi/processor_thermal.c
> +++ b/drivers/acpi/processor_thermal.c
> @@ -235,15 +235,7 @@ static int
>   processor_get_max_state(struct thermal_cooling_device *cdev,
>   			unsigned long *state)
>   {
> -	struct acpi_device *device = cdev->devdata;
> -	struct acpi_processor *pr;
> -
> -	if (!device)
> -		return -EINVAL;
> -
> -	pr = acpi_driver_data(device);
> -	if (!pr)
> -		return -EINVAL;
> +	struct acpi_processor *pr = cdev->devdata;
>   
>   	*state = acpi_processor_max_state(pr);
>   	return 0;
> @@ -253,15 +245,7 @@ static int
>   processor_get_cur_state(struct thermal_cooling_device *cdev,
>   			unsigned long *cur_state)
>   {
> -	struct acpi_device *device = cdev->devdata;
> -	struct acpi_processor *pr;
> -
> -	if (!device)
> -		return -EINVAL;
> -
> -	pr = acpi_driver_data(device);
> -	if (!pr)
> -		return -EINVAL;
> +	struct acpi_processor *pr = cdev->devdata;
>   
>   	*cur_state = cpufreq_get_cur_state(pr->id);
>   	if (pr->flags.throttling)
> @@ -273,18 +257,10 @@ static int
>   processor_set_cur_state(struct thermal_cooling_device *cdev,
>   			unsigned long state)
>   {
> -	struct acpi_device *device = cdev->devdata;
> -	struct acpi_processor *pr;
> +	struct acpi_processor *pr = cdev->devdata;
>   	int result = 0;
>   	int max_pstate;
>   
> -	if (!device)
> -		return -EINVAL;
> -
> -	pr = acpi_driver_data(device);
> -	if (!pr)
> -		return -EINVAL;
> -
>   	max_pstate = cpufreq_get_max_state(pr->id);
>   
>   	if (state > acpi_processor_max_state(pr))
> @@ -308,10 +284,9 @@ const struct thermal_cooling_device_ops processor_cooling_ops = {
>   	.set_cur_state = processor_set_cur_state,
>   };
>   
> -int acpi_processor_thermal_init(struct acpi_processor *pr,
> -				struct acpi_device *device)
> +int acpi_processor_thermal_init(struct acpi_processor *pr)
>   {
> -	pr->cdev = thermal_cooling_device_create(pr->dev, "Processor", device,
> +	pr->cdev = thermal_cooling_device_create(pr->dev, "Processor", pr,
>   						 &processor_cooling_ops);
>   	if (IS_ERR(pr->cdev))
>   		return PTR_ERR(pr->cdev);
> diff --git a/include/acpi/processor.h b/include/acpi/processor.h
> index 656aaf74fb18..b5447af4d40b 100644
> --- a/include/acpi/processor.h
> +++ b/include/acpi/processor.h
> @@ -427,8 +427,7 @@ int acpi_processor_ffh_lpi_enter(struct acpi_lpi_state *lpi);
>   #endif /* CONFIG_ACPI_PROCESSOR_IDLE */
>   
>   /* in processor_thermal.c */
> -int acpi_processor_thermal_init(struct acpi_processor *pr,
> -				struct acpi_device *device);
> +int acpi_processor_thermal_init(struct acpi_processor *pr);
>   void acpi_processor_thermal_exit(struct acpi_processor *pr);
>   extern const struct thermal_cooling_device_ops processor_cooling_ops;
>   #ifdef CONFIG_CPU_FREQ

Re: [PATCH v1 09/10] ACPI: fan: Store ACPI device pointer in struct acpi_fan

[Armin Wolf]

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

> From: "Rafael J. Wysocki" <rafael.j.wysocki@intel.com>
>
> In preparation for subsequent changes, instead of storing a fan ACPI
> handle in struct acpi_fan, store a pointer to the corresponding struct
> acpi_device in it.
>
> No intentional functional impact.

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

> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> ---
>   drivers/acpi/fan.h       |  2 +-
>   drivers/acpi/fan_core.c  | 11 ++++++-----
>   drivers/acpi/fan_hwmon.c |  2 +-
>   3 files changed, 8 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/acpi/fan.h b/drivers/acpi/fan.h
> index e20d6ad9df80..3faa247af514 100644
> --- a/drivers/acpi/fan.h
> +++ b/drivers/acpi/fan.h
> @@ -52,7 +52,7 @@ struct acpi_fan_fst {
>   };
>   
>   struct acpi_fan {
> -	acpi_handle handle;
> +	struct acpi_device *adev;
>   	bool acpi4;
>   	bool has_fst;
>   	struct acpi_fan_fif fif;
> diff --git a/drivers/acpi/fan_core.c b/drivers/acpi/fan_core.c
> index 29ca97503deb..8b20b8a55c13 100644
> --- a/drivers/acpi/fan_core.c
> +++ b/drivers/acpi/fan_core.c
> @@ -340,17 +340,18 @@ static int acpi_fan_dsm_init(struct device *dev)
>   		},
>   	};
>   	struct acpi_fan *fan = dev_get_drvdata(dev);
> +	acpi_handle fan_handle = fan->adev->handle;
>   	union acpi_object *obj;
>   	int ret = 0;
>   
> -	if (!acpi_check_dsm(fan->handle, &acpi_fan_microsoft_guid, 0,
> +	if (!acpi_check_dsm(fan_handle, &acpi_fan_microsoft_guid, 0,
>   			    BIT(ACPI_FAN_DSM_GET_TRIP_POINT_GRANULARITY) |
>   			    BIT(ACPI_FAN_DSM_SET_TRIP_POINTS)))
>   		return 0;
>   
>   	dev_info(dev, "Using Microsoft fan extensions\n");
>   
> -	obj = acpi_evaluate_dsm_typed(fan->handle, &acpi_fan_microsoft_guid, 0,
> +	obj = acpi_evaluate_dsm_typed(fan_handle, &acpi_fan_microsoft_guid, 0,
>   				      ACPI_FAN_DSM_GET_TRIP_POINT_GRANULARITY, &dummy,
>   				      ACPI_TYPE_INTEGER);
>   	if (!obj)
> @@ -392,8 +393,8 @@ static int acpi_fan_dsm_set_trip_points(struct device *dev, u64 upper, u64 lower
>   	};
>   	union acpi_object *obj;
>   
> -	obj = acpi_evaluate_dsm(fan->handle, &acpi_fan_microsoft_guid, 0,
> -				ACPI_FAN_DSM_SET_TRIP_POINTS, &in);
> +	obj = acpi_evaluate_dsm(fan->adev->handle, &acpi_fan_microsoft_guid,
> +				0, ACPI_FAN_DSM_SET_TRIP_POINTS, &in);
>   	kfree(obj);
>   
>   	return 0;
> @@ -503,7 +504,7 @@ static int acpi_fan_probe(struct platform_device *pdev)
>   		return -ENOMEM;
>   	}
>   
> -	fan->handle = device->handle;
> +	fan->adev = device;
>   	device->driver_data = fan;
>   	platform_set_drvdata(pdev, fan);
>   
> diff --git a/drivers/acpi/fan_hwmon.c b/drivers/acpi/fan_hwmon.c
> index d3374f8f524b..c5d8419f41c5 100644
> --- a/drivers/acpi/fan_hwmon.c
> +++ b/drivers/acpi/fan_hwmon.c
> @@ -94,7 +94,7 @@ static int acpi_fan_hwmon_read(struct device *dev, enum hwmon_sensor_types type,
>   	struct acpi_fan_fst fst;
>   	int ret;
>   
> -	ret = acpi_fan_get_fst(fan->handle, &fst);
> +	ret = acpi_fan_get_fst(fan->adev->handle, &fst);
>   	if (ret < 0)
>   		return ret;
>   

Re: [PATCH v1 10/10] ACPI: fan: Use more suitable cooling device data

[Armin Wolf]

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

> From: "Rafael J. Wysocki" <rafael.j.wysocki@intel.com>
>
> Instead of passing an ACPI device object pointer as devdata to
> thermal_cooling_device_create(), make acpi_fan_probe() pass a pointer
> to struct acpi_fan to it, which allows the callback functions in
> fan_cooling_ops to be simplified.
>
> Also avoid using acpi_driver_data() in two functions invoked by the
> cooling device callbacks by passing struct acpi_fan pointers instead
> of struct acpi_device pointers to them.
>
> No intentional functional impact.

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

> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> ---
>   drivers/acpi/fan_core.c | 27 ++++++++++++---------------
>   1 file changed, 12 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/acpi/fan_core.c b/drivers/acpi/fan_core.c
> index 8b20b8a55c13..5ad65975fbb4 100644
> --- a/drivers/acpi/fan_core.c
> +++ b/drivers/acpi/fan_core.c
> @@ -54,8 +54,7 @@ MODULE_DEVICE_TABLE(acpi, fan_device_ids);
>   static int fan_get_max_state(struct thermal_cooling_device *cdev, unsigned long
>   			     *state)
>   {
> -	struct acpi_device *device = cdev->devdata;
> -	struct acpi_fan *fan = acpi_driver_data(device);
> +	struct acpi_fan *fan = cdev->devdata;
>   
>   	if (fan->acpi4) {
>   		if (fan->fif.fine_grain_ctrl)
> @@ -105,9 +104,9 @@ int acpi_fan_get_fst(acpi_handle handle, struct acpi_fan_fst *fst)
>   	return ret;
>   }
>   
> -static int fan_get_state_acpi4(struct acpi_device *device, unsigned long *state)
> +static int fan_get_state_acpi4(struct acpi_fan *fan, unsigned long *state)
>   {
> -	struct acpi_fan *fan = acpi_driver_data(device);
> +	struct acpi_device *device = fan->adev;
>   	struct acpi_fan_fst fst;
>   	int status, i;
>   
> @@ -159,13 +158,12 @@ static int fan_get_state(struct acpi_device *device, unsigned long *state)
>   static int fan_get_cur_state(struct thermal_cooling_device *cdev, unsigned long
>   			     *state)
>   {
> -	struct acpi_device *device = cdev->devdata;
> -	struct acpi_fan *fan = acpi_driver_data(device);
> +	struct acpi_fan *fan = cdev->devdata;
>   
>   	if (fan->acpi4)
> -		return fan_get_state_acpi4(device, state);
> +		return fan_get_state_acpi4(fan, state);
>   	else
> -		return fan_get_state(device, state);
> +		return fan_get_state(fan->adev, state);
>   }
>   
>   static int fan_set_state(struct acpi_device *device, unsigned long state)
> @@ -177,9 +175,9 @@ static int fan_set_state(struct acpi_device *device, unsigned long state)
>   				     state ? ACPI_STATE_D0 : ACPI_STATE_D3_COLD);
>   }
>   
> -static int fan_set_state_acpi4(struct acpi_device *device, unsigned long state)
> +static int fan_set_state_acpi4(struct acpi_fan *fan, unsigned long state)
>   {
> -	struct acpi_fan *fan = acpi_driver_data(device);
> +	struct acpi_device *device = fan->adev;
>   	acpi_status status;
>   	u64 value = state;
>   	int max_state;
> @@ -213,13 +211,12 @@ static int fan_set_state_acpi4(struct acpi_device *device, unsigned long state)
>   static int
>   fan_set_cur_state(struct thermal_cooling_device *cdev, unsigned long state)
>   {
> -	struct acpi_device *device = cdev->devdata;
> -	struct acpi_fan *fan = acpi_driver_data(device);
> +	struct acpi_fan *fan = cdev->devdata;
>   
>   	if (fan->acpi4)
> -		return fan_set_state_acpi4(device, state);
> +		return fan_set_state_acpi4(fan, state);
>   	else
> -		return fan_set_state(device, state);
> +		return fan_set_state(fan->adev, state);
>   }
>   
>   static const struct thermal_cooling_device_ops fan_cooling_ops = {
> @@ -565,7 +562,7 @@ static int acpi_fan_probe(struct platform_device *pdev)
>   	else
>   		name = acpi_device_bid(device);
>   
> -	cdev = thermal_cooling_device_create(&pdev->dev, name, device, &fan_cooling_ops);
> +	cdev = thermal_cooling_device_create(&pdev->dev, name, fan, &fan_cooling_ops);
>   	if (IS_ERR(cdev)) {
>   		result = PTR_ERR(cdev);
>   		goto err_end;

Re: [PATCH v1 00/10] ACPI: thermal: Register thermal class cooling devices under parents

[Armin Wolf]

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

> Hi All,
>
> This series generally changes the way in which thermal class cooling devices
> are used by ACPI device drivers registering them (ACPI processor, ACPI fan,
> and ACPI video) and by the ACPI thermal zone device driver.
>
> First, it causes the thermal class cooling devices to be registered under
> parents in specific locations within the device hierarchy which allows
> their relationships with other devices to be recognized more easily.  It
> also drops custom sysfs attributes registered by the users of the thermal
> class cooling devices because they are not particularly useful any more
> when those devices get proper parents.
>
> Second, it modifies the ACPI thermal zone device driver to use thermal class
> cooling device parent information for binding cooling devices to trip points
> in thermal zones, which was previously done with the help of the devdata
> pointers of the cooling devices and effectively prevented drivers from using
> the devdata pointers for their own purposes as originally intended.
>
> Finally, it updates the ACPI processor and ACPI fan drivers to use the
> devdata pointers of thermal class cooling devices in a bit more efficient
> way.
>
> The first two patches in the series are preliminary fixes preventing
> Sashiko from reporting "pre-existing issues" in the subsequent patches.
>
> The third patch is a thermal core update allowing thermal class cooling
> devices to be registered under proper parents.
>
> The other patches update the ACPI processor, ACPI video, and ACPI fan
> drivers as per the above.
>
> The details are covered by individual patch changelogs.
>
> Thanks!

Nice patch series, it looks mostly good to me. Do you plan something similar for
thermal zones?

Thanks,
Armin Wolf

Re: [PATCH v1 00/10] ACPI: thermal: Register thermal class cooling devices under parents

[Rafael J. Wysocki (Intel)]

On Fri, Sep 11, 2026 at 11:35 PM Armin Wolf <W_Armin@gmx.de> wrote:
>
> Am 11.09.26 um 15:00 schrieb Rafael J. Wysocki:
>
> > Hi All,
> >
> > This series generally changes the way in which thermal class cooling devices
> > are used by ACPI device drivers registering them (ACPI processor, ACPI fan,
> > and ACPI video) and by the ACPI thermal zone device driver.
> >
> > First, it causes the thermal class cooling devices to be registered under
> > parents in specific locations within the device hierarchy which allows
> > their relationships with other devices to be recognized more easily.  It
> > also drops custom sysfs attributes registered by the users of the thermal
> > class cooling devices because they are not particularly useful any more
> > when those devices get proper parents.
> >
> > Second, it modifies the ACPI thermal zone device driver to use thermal class
> > cooling device parent information for binding cooling devices to trip points
> > in thermal zones, which was previously done with the help of the devdata
> > pointers of the cooling devices and effectively prevented drivers from using
> > the devdata pointers for their own purposes as originally intended.
> >
> > Finally, it updates the ACPI processor and ACPI fan drivers to use the
> > devdata pointers of thermal class cooling devices in a bit more efficient
> > way.
> >
> > The first two patches in the series are preliminary fixes preventing
> > Sashiko from reporting "pre-existing issues" in the subsequent patches.
> >
> > The third patch is a thermal core update allowing thermal class cooling
> > devices to be registered under proper parents.
> >
> > The other patches update the ACPI processor, ACPI video, and ACPI fan
> > drivers as per the above.
> >
> > The details are covered by individual patch changelogs.
> >
> > Thanks!
>
> Nice patch series, it looks mostly good to me.

Thanks!

> Do you plan something similar for thermal zones?

I do overall, but that is a bit harder due to the hwmon involvement.

Re: [PATCH v1 01/10] ACPI: fan: Fix memory leak due to leftover devm_kcalloc() argument

[Rafael J. Wysocki (Intel)]

On Fri, Sep 11, 2026 at 6:29 PM Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
>
> On Fri, Sep 11, 2026 at 03:00:51PM +0200, Rafael J. Wysocki wrote:
>
> > An ACPI device object's dev field in passed as the first argument to
> > devm_kcalloc() in acpi_fan_get_fps() which is incorrect and leads to
> > a memory leak on driver probe errors and removal because the driver
> > is not bound to that ACPI device.
> >
> > Address this by replacing that pointer with a pointer to the device the
> > driver is actually bound to.
> >
> > While at it, drop a redundant error message after a memory allocation
> > failure (that also gets printed relative to the ACPI device).
>
> ...
>
> >       fan->fps_count = obj->package.count - 1; /* minus revision field */
> > -     fan->fps = devm_kcalloc(&device->dev,
> > -                             fan->fps_count, sizeof(struct acpi_fan_fps),
> > -                             GFP_KERNEL);
> > +     fan->fps = devm_kcalloc(dev, fan->fps_count, sizeof(*fan->fps), GFP_KERNEL);
> >       if (!fan->fps) {
> > -             dev_err(&device->dev, "Not enough memory\n");
> >               status = -ENOMEM;
> >               goto err;
>
> I was about ranting on goto after devm_*(), but looking at the context,
> I understand why it's not a problem. While at it, a side note: perhaps it makes
> sense to use ACPI_FREE(obj) instead of kfree()?

It should be ACPI_FREE() strictly speaking.

> Or even better to have __free() version of it, so we can declare the object with autoclean.

That one is a bit tricky, but I think I have an idea how to do it.
I'll post something next week.

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

[Andy Shevchenko]

On Fri, Sep 11, 2026 at 11:14:09PM +0200, Armin Wolf wrote:
> Am 11.09.26 um 20:06 schrieb Andy Shevchenko:
> > On Fri, Sep 11, 2026 at 07:01:23PM +0200, Rafael J. Wysocki (Intel) wrote:
> > > On Fri, Sep 11, 2026 at 6:47 PM Andy Shevchenko
> > > <andriy.shevchenko@linux.intel.com> wrote:
> > > > On Fri, Sep 11, 2026 at 06:36:28PM +0200, Rafael J. Wysocki (Intel) wrote:
> > > > > On Fri, Sep 11, 2026 at 6:32 PM Andy Shevchenko
> > > > > <andriy.shevchenko@linux.intel.com> wrote:
> > > > > > On Fri, Sep 11, 2026 at 03:02:53PM +0200, Rafael J. Wysocki wrote:

...

> > > > > > Is any user space ABI breakage expected as an outcome of this change?
> > > > > Not really.
> > > > Maybe makes sense to state this clearly in the cover letter?
> > > I guess you specifically mean the manually created symbolic links in
> > > sysfs that get removed.
> > > 
> > > If so, I'm not aware of anyone using them for anything, but the lack
> > > of observation is not proof of nonexistence.
> > Yes, I refer to the changes in the sysfs layout / presence / absence.
> 
> AFAIK those custom sysfs attributes are not even documented under Documentation/ABI, and
> thermald also does not seem to use them. I thus think we can safely remove them.

There is Debian source code browser (that covers hundreds of OSS projects) [1],
have you tried to seek any matches there? Otherwise the assumption that nobody
uses them might be wrong even if they were never documented.

[1]: https://codesearch.debian.net/

-- 
With Best Regards,
Andy Shevchenko

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

[Rafael J. Wysocki (Intel)]

On Sun, Sep 13, 2026 at 10:15 AM Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
>
> On Fri, Sep 11, 2026 at 11:14:09PM +0200, Armin Wolf wrote:
> > Am 11.09.26 um 20:06 schrieb Andy Shevchenko:
> > > On Fri, Sep 11, 2026 at 07:01:23PM +0200, Rafael J. Wysocki (Intel) wrote:
> > > > On Fri, Sep 11, 2026 at 6:47 PM Andy Shevchenko
> > > > <andriy.shevchenko@linux.intel.com> wrote:
> > > > > On Fri, Sep 11, 2026 at 06:36:28PM +0200, Rafael J. Wysocki (Intel) wrote:
> > > > > > On Fri, Sep 11, 2026 at 6:32 PM Andy Shevchenko
> > > > > > <andriy.shevchenko@linux.intel.com> wrote:
> > > > > > > On Fri, Sep 11, 2026 at 03:02:53PM +0200, Rafael J. Wysocki wrote:
>
> ...
>
> > > > > > > Is any user space ABI breakage expected as an outcome of this change?
> > > > > > Not really.
> > > > > Maybe makes sense to state this clearly in the cover letter?
> > > > I guess you specifically mean the manually created symbolic links in
> > > > sysfs that get removed.
> > > >
> > > > If so, I'm not aware of anyone using them for anything, but the lack
> > > > of observation is not proof of nonexistence.
> > > Yes, I refer to the changes in the sysfs layout / presence / absence.
> >
> > AFAIK those custom sysfs attributes are not even documented under Documentation/ABI, and
> > thermald also does not seem to use them. I thus think we can safely remove them.
>
> There is Debian source code browser (that covers hundreds of OSS projects) [1],
> have you tried to seek any matches there? Otherwise the assumption that nobody
> uses them might be wrong even if they were never documented.
>
> [1]: https://codesearch.debian.net/

It is mostly relevant whether or not they are used by someone today
and not whether or not they have ever been used at all.

We'll find out.

If anyone complains and they are unable to cope with that, adding
"thermal_cooling" back should not be a problem (although I'd rather
not do it unless absolutely necessary).