[PATCH v1 0/3] hwmon: (acpi_power_meter) Bind the driver to a platform device

[PATCH v1 0/3] hwmon: (acpi_power_meter) Bind the driver to a platform device

[Rafael J. Wysocki]

Hi All,

This series is part of a larger effort to switch over all drivers using
the struct acpi_driver interface to the more common struct platform_driver
interface and eliminate the former.  The background is explained in
Documentation/driver-api/acpi/acpi-drivers.rst and in the changelog of
the patch that introduced the above document:

https://lore.kernel.org/all/2396510.ElGaqSPkdT@rafael.j.wysocki/

The bottom line is that the kernel would be better off without struct
acpi_driver and so it is better to get rid of it.

This series carries out driver conversion of the hwmon acpi_power_meter
driver.

Patch [1/3] removes some redundant checks from three functions in the
driver to facilitate subsequent changes.

Patch [2/3] updates the driver to install an ACPI notify handler by itself
instead of using the .notify() callback from struct acpi_driver, which is
requisite for the driver conversion.

Patch [3/3] converts the driver to using struct platform_driver for device
binding.

@Jaroslaw, testing this on your systems where the acpi_power_meter driver
is used would be much appreciated.

Thanks!

[PATCH v1 1/3] hwmon: (acpi_power_meter) Drop redundant checks from three functions

[Rafael J. Wysocki]

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

Since acpi_power_meter_notify() and acpi_power_meter_remove() are
.notify() and .remove() callback functions of an ACPI driver,
respectively, the first argument of the former and the only argument
of the latter cannot be NULL.  Likewise, the acpi_power_meter_resume()
argument cannot be NULL because it is a system resume callback
function.

Moreover, since all of these functions can only run after
acpi_power_meter_add() has returned 0, the driver_data field in the
struct acpi_device object used by them cannot be NULL either.

Accordingly, drop the redundant "device" checks against NULL from
acpi_power_meter_notify() and acpi_power_meter_remove(), drop the
redundant "dev" check against NULL from acpi_power_meter_resume(),
and drop the redundant acpi_driver_data() checks against NULL from
all of these functions.

Additionally, combine the initialization of the "resource" local
variable in acpi_power_meter_notify() and acpi_power_meter_remove()
with its declaration.

No intentional functional impact.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/hwmon/acpi_power_meter.c | 18 ++----------------
 1 file changed, 2 insertions(+), 16 deletions(-)

diff --git a/drivers/hwmon/acpi_power_meter.c b/drivers/hwmon/acpi_power_meter.c
index 1e3fab5f7946..49e57c20ef70 100644
--- a/drivers/hwmon/acpi_power_meter.c
+++ b/drivers/hwmon/acpi_power_meter.c
@@ -816,14 +816,9 @@ static int read_capabilities(struct acpi_power_meter_resource *resource)
 /* Handle ACPI event notifications */
 static void acpi_power_meter_notify(struct acpi_device *device, u32 event)
 {
-	struct acpi_power_meter_resource *resource;
+	struct acpi_power_meter_resource *resource = acpi_driver_data(device);
 	int res;
 
-	if (!device || !acpi_driver_data(device))
-		return;
-
-	resource = acpi_driver_data(device);
-
 	guard(mutex)(&acpi_notify_lock);
 
 	switch (event) {
@@ -956,12 +951,8 @@ static int acpi_power_meter_add(struct acpi_device *device)
 
 static void acpi_power_meter_remove(struct acpi_device *device)
 {
-	struct acpi_power_meter_resource *resource;
+	struct acpi_power_meter_resource *resource = acpi_driver_data(device);
 
-	if (!device || !acpi_driver_data(device))
-		return;
-
-	resource = acpi_driver_data(device);
 	if (!IS_ERR(resource->hwmon_dev))
 		hwmon_device_unregister(resource->hwmon_dev);
 
@@ -975,12 +966,7 @@ static int acpi_power_meter_resume(struct device *dev)
 {
 	struct acpi_power_meter_resource *resource;
 
-	if (!dev)
-		return -EINVAL;
-
 	resource = acpi_driver_data(to_acpi_device(dev));
-	if (!resource)
-		return -EINVAL;
 
 	free_capabilities(resource);
 	read_capabilities(resource);
-- 
2.51.0

[PATCH v1 2/3] hwmon: (acpi_power_meter) Register ACPI notify handler directly

[Rafael J. Wysocki]

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

To facilitate subsequent conversion of the driver to a platform one,
make it install an ACPI notify handler directly instead of using
a .notify() callback in struct acpi_driver.

No intentional functional impact.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/hwmon/acpi_power_meter.c | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/drivers/hwmon/acpi_power_meter.c b/drivers/hwmon/acpi_power_meter.c
index 49e57c20ef70..c010f55f7c7b 100644
--- a/drivers/hwmon/acpi_power_meter.c
+++ b/drivers/hwmon/acpi_power_meter.c
@@ -814,8 +814,9 @@ static int read_capabilities(struct acpi_power_meter_resource *resource)
 }
 
 /* Handle ACPI event notifications */
-static void acpi_power_meter_notify(struct acpi_device *device, u32 event)
+static void acpi_power_meter_notify(acpi_handle handle, u32 event, void *data)
 {
+	struct acpi_device *device = data;
 	struct acpi_power_meter_resource *resource = acpi_driver_data(device);
 	int res;
 
@@ -936,9 +937,16 @@ static int acpi_power_meter_add(struct acpi_device *device)
 		goto exit_remove;
 	}
 
+	res = acpi_dev_install_notify_handler(device, ACPI_DEVICE_NOTIFY,
+					      acpi_power_meter_notify, device);
+	if (res)
+		goto exit_hwmon;
+
 	res = 0;
 	goto exit;
 
+exit_hwmon:
+	hwmon_device_unregister(resource->hwmon_dev);
 exit_remove:
 	remove_domain_devices(resource);
 exit_free_capability:
@@ -953,6 +961,9 @@ static void acpi_power_meter_remove(struct acpi_device *device)
 {
 	struct acpi_power_meter_resource *resource = acpi_driver_data(device);
 
+	acpi_dev_remove_notify_handler(device, ACPI_DEVICE_NOTIFY,
+				       acpi_power_meter_notify);
+
 	if (!IS_ERR(resource->hwmon_dev))
 		hwmon_device_unregister(resource->hwmon_dev);
 
@@ -984,7 +995,6 @@ static struct acpi_driver acpi_power_meter_driver = {
 	.ops = {
 		.add = acpi_power_meter_add,
 		.remove = acpi_power_meter_remove,
-		.notify = acpi_power_meter_notify,
 		},
 	.drv.pm = pm_sleep_ptr(&acpi_power_meter_pm),
 };
-- 
2.51.0

[PATCH v1 3/3] hwmon: (acpi_power_meter) Convert ACPI driver to a platform one

[Rafael J. Wysocki]

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

In all cases in which a struct acpi_driver is used for binding a driver
to an ACPI device object, a corresponding platform device is created by
the ACPI core and that device is regarded as a proper representation of
underlying hardware.  Accordingly, a struct platform_driver should be
used by driver code to bind to that device.  There are multiple reasons
why drivers should not bind directly to ACPI device objects [1].

Overall, it is better to bind drivers to platform devices than to their
ACPI companions, so convert the hwmon ACPI power meter driver to a
platform one.

After this change, the subordinate hwmon device will be registered
under the platform device representing the ACPI power meter, sysfs
notifications will trigger on that device, and diagnostic messages
will be printed relative to it instead of its ACPI companion.

Link: https://lore.kernel.org/all/2396510.ElGaqSPkdT@rafael.j.wysocki/ [1]
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/hwmon/acpi_power_meter.c | 76 ++++++++++++++++----------------
 1 file changed, 37 insertions(+), 39 deletions(-)

diff --git a/drivers/hwmon/acpi_power_meter.c b/drivers/hwmon/acpi_power_meter.c
index c010f55f7c7b..be7f702dcde9 100644
--- a/drivers/hwmon/acpi_power_meter.c
+++ b/drivers/hwmon/acpi_power_meter.c
@@ -18,6 +18,7 @@
 #include <linux/time.h>
 #include <linux/err.h>
 #include <linux/acpi.h>
+#include <linux/platform_device.h>
 
 #define ACPI_POWER_METER_NAME		"power_meter"
 #define ACPI_POWER_METER_DEVICE_NAME	"Power Meter"
@@ -816,8 +817,8 @@ static int read_capabilities(struct acpi_power_meter_resource *resource)
 /* Handle ACPI event notifications */
 static void acpi_power_meter_notify(acpi_handle handle, u32 event, void *data)
 {
-	struct acpi_device *device = data;
-	struct acpi_power_meter_resource *resource = acpi_driver_data(device);
+	struct device *dev = data;
+	struct acpi_power_meter_resource *resource = dev_get_drvdata(dev);
 	int res;
 
 	guard(mutex)(&acpi_notify_lock);
@@ -833,43 +834,43 @@ static void acpi_power_meter_notify(acpi_handle handle, u32 event, void *data)
 		remove_domain_devices(resource);
 		res = read_capabilities(resource);
 		if (res)
-			dev_err_once(&device->dev, "read capabilities failed.\n");
+			dev_err_once(dev, "read capabilities failed.\n");
 		res = read_domain_devices(resource);
 		if (res && res != -ENODEV)
-			dev_err_once(&device->dev, "read domain devices failed.\n");
+			dev_err_once(dev, "read domain devices failed.\n");
 
 		mutex_unlock(&resource->lock);
 
 		resource->hwmon_dev =
-			hwmon_device_register_with_info(&device->dev,
+			hwmon_device_register_with_info(dev,
 							ACPI_POWER_METER_NAME,
 							resource,
 							&power_meter_chip_info,
 							power_extra_groups);
 		if (IS_ERR(resource->hwmon_dev))
-			dev_err_once(&device->dev, "register hwmon device failed.\n");
+			dev_err_once(dev, "register hwmon device failed.\n");
 
 		break;
 	case METER_NOTIFY_TRIP:
-		sysfs_notify(&device->dev.kobj, NULL, POWER_AVERAGE_NAME);
+		sysfs_notify(&dev->kobj, NULL, POWER_AVERAGE_NAME);
 		break;
 	case METER_NOTIFY_CAP:
 		mutex_lock(&resource->lock);
 		res = update_cap(resource);
 		if (res)
-			dev_err_once(&device->dev, "update cap failed when capping value is changed.\n");
+			dev_err_once(dev, "update cap failed when capping value is changed.\n");
 		mutex_unlock(&resource->lock);
-		sysfs_notify(&device->dev.kobj, NULL, POWER_CAP_NAME);
+		sysfs_notify(&dev->kobj, NULL, POWER_CAP_NAME);
 		break;
 	case METER_NOTIFY_INTERVAL:
-		sysfs_notify(&device->dev.kobj, NULL, POWER_AVG_INTERVAL_NAME);
+		sysfs_notify(&dev->kobj, NULL, POWER_AVG_INTERVAL_NAME);
 		break;
 	case METER_NOTIFY_CAPPING:
 		mutex_lock(&resource->lock);
 		resource->power_alarm = true;
 		mutex_unlock(&resource->lock);
-		sysfs_notify(&device->dev.kobj, NULL, POWER_ALARM_NAME);
-		dev_info(&device->dev, "Capping in progress.\n");
+		sysfs_notify(&dev->kobj, NULL, POWER_ALARM_NAME);
+		dev_info(dev, "Capping in progress.\n");
 		break;
 	default:
 		WARN(1, "Unexpected event %d\n", event);
@@ -877,16 +878,15 @@ static void acpi_power_meter_notify(acpi_handle handle, u32 event, void *data)
 	}
 
 	acpi_bus_generate_netlink_event(ACPI_POWER_METER_CLASS,
-					dev_name(&device->dev), event, 0);
+					dev_name(&resource->acpi_dev->dev),
+					event, 0);
 }
 
-static int acpi_power_meter_add(struct acpi_device *device)
+static int acpi_power_meter_probe(struct platform_device *pdev)
 {
-	int res;
+	struct acpi_device *device = ACPI_COMPANION(&pdev->dev);
 	struct acpi_power_meter_resource *resource;
-
-	if (!device)
-		return -EINVAL;
+	int res;
 
 	resource = kzalloc_obj(*resource);
 	if (!resource)
@@ -897,7 +897,8 @@ static int acpi_power_meter_add(struct acpi_device *device)
 	mutex_init(&resource->lock);
 	strscpy(acpi_device_name(device), ACPI_POWER_METER_DEVICE_NAME);
 	strscpy(acpi_device_class(device), ACPI_POWER_METER_CLASS);
-	device->driver_data = resource;
+
+	platform_set_drvdata(pdev, resource);
 
 #if IS_REACHABLE(CONFIG_ACPI_IPMI)
 	/*
@@ -910,7 +911,7 @@ static int acpi_power_meter_add(struct acpi_device *device)
 		struct acpi_device *ipi_device = acpi_dev_get_first_match_dev("IPI0001", NULL, -1);
 
 		if (ipi_device && acpi_wait_for_acpi_ipmi())
-			dev_warn(&device->dev, "Waiting for ACPI IPMI timeout");
+			dev_warn(&pdev->dev, "Waiting for ACPI IPMI timeout");
 		acpi_dev_put(ipi_device);
 	}
 #endif
@@ -928,7 +929,7 @@ static int acpi_power_meter_add(struct acpi_device *device)
 		goto exit_free_capability;
 
 	resource->hwmon_dev =
-		hwmon_device_register_with_info(&device->dev,
+		hwmon_device_register_with_info(&pdev->dev,
 						ACPI_POWER_METER_NAME, resource,
 						&power_meter_chip_info,
 						power_extra_groups);
@@ -938,7 +939,7 @@ static int acpi_power_meter_add(struct acpi_device *device)
 	}
 
 	res = acpi_dev_install_notify_handler(device, ACPI_DEVICE_NOTIFY,
-					      acpi_power_meter_notify, device);
+					      acpi_power_meter_notify, &pdev->dev);
 	if (res)
 		goto exit_hwmon;
 
@@ -957,11 +958,11 @@ static int acpi_power_meter_add(struct acpi_device *device)
 	return res;
 }
 
-static void acpi_power_meter_remove(struct acpi_device *device)
+static void acpi_power_meter_remove(struct platform_device *pdev)
 {
-	struct acpi_power_meter_resource *resource = acpi_driver_data(device);
+	struct acpi_power_meter_resource *resource = platform_get_drvdata(pdev);
 
-	acpi_dev_remove_notify_handler(device, ACPI_DEVICE_NOTIFY,
+	acpi_dev_remove_notify_handler(resource->acpi_dev, ACPI_DEVICE_NOTIFY,
 				       acpi_power_meter_notify);
 
 	if (!IS_ERR(resource->hwmon_dev))
@@ -975,9 +976,7 @@ static void acpi_power_meter_remove(struct acpi_device *device)
 
 static int acpi_power_meter_resume(struct device *dev)
 {
-	struct acpi_power_meter_resource *resource;
-
-	resource = acpi_driver_data(to_acpi_device(dev));
+	struct acpi_power_meter_resource *resource = dev_get_drvdata(dev);
 
 	free_capabilities(resource);
 	read_capabilities(resource);
@@ -988,15 +987,14 @@ static int acpi_power_meter_resume(struct device *dev)
 static DEFINE_SIMPLE_DEV_PM_OPS(acpi_power_meter_pm, NULL,
 				acpi_power_meter_resume);
 
-static struct acpi_driver acpi_power_meter_driver = {
-	.name = "power_meter",
-	.class = ACPI_POWER_METER_CLASS,
-	.ids = power_meter_ids,
-	.ops = {
-		.add = acpi_power_meter_add,
-		.remove = acpi_power_meter_remove,
-		},
-	.drv.pm = pm_sleep_ptr(&acpi_power_meter_pm),
+static struct platform_driver acpi_power_meter_driver = {
+	.probe = acpi_power_meter_probe,
+	.remove = acpi_power_meter_remove,
+	.driver = {
+		.name = "acpi-power-meter",
+		.acpi_match_table = power_meter_ids,
+		.pm = &acpi_power_meter_pm,
+	},
 };
 
 /* Module init/exit routines */
@@ -1025,7 +1023,7 @@ static int __init acpi_power_meter_init(void)
 
 	dmi_check_system(pm_dmi_table);
 
-	result = acpi_bus_register_driver(&acpi_power_meter_driver);
+	result = platform_driver_register(&acpi_power_meter_driver);
 	if (result < 0)
 		return result;
 
@@ -1034,7 +1032,7 @@ static int __init acpi_power_meter_init(void)
 
 static void __exit acpi_power_meter_exit(void)
 {
-	acpi_bus_unregister_driver(&acpi_power_meter_driver);
+	platform_driver_unregister(&acpi_power_meter_driver);
 }
 
 MODULE_AUTHOR("Darrick J. Wong <darrick.wong@oracle.com>");
-- 
2.51.0

Re: [PATCH v1 1/3] hwmon: (acpi_power_meter) Drop redundant checks from three functions

[Guenter Roeck]

On Sun, Mar 01, 2026 at 02:17:19PM +0100, Rafael J. Wysocki wrote:
> From: "Rafael J. Wysocki" <rafael.j.wysocki@intel.com>
> 
> Since acpi_power_meter_notify() and acpi_power_meter_remove() are
> .notify() and .remove() callback functions of an ACPI driver,
> respectively, the first argument of the former and the only argument
> of the latter cannot be NULL.  Likewise, the acpi_power_meter_resume()
> argument cannot be NULL because it is a system resume callback
> function.
> 
> Moreover, since all of these functions can only run after
> acpi_power_meter_add() has returned 0, the driver_data field in the
> struct acpi_device object used by them cannot be NULL either.
> 
> Accordingly, drop the redundant "device" checks against NULL from
> acpi_power_meter_notify() and acpi_power_meter_remove(), drop the
> redundant "dev" check against NULL from acpi_power_meter_resume(),
> and drop the redundant acpi_driver_data() checks against NULL from
> all of these functions.
> 
> Additionally, combine the initialization of the "resource" local
> variable in acpi_power_meter_notify() and acpi_power_meter_remove()
> with its declaration.
> 
> No intentional functional impact.
> 
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

Applied.

Thanks,
Guenter

Re: [PATCH v1 2/3] hwmon: (acpi_power_meter) Register ACPI notify handler directly

[Guenter Roeck]

On Sun, Mar 01, 2026 at 02:18:05PM +0100, Rafael J. Wysocki wrote:
> From: "Rafael J. Wysocki" <rafael.j.wysocki@intel.com>
> 
> To facilitate subsequent conversion of the driver to a platform one,
> make it install an ACPI notify handler directly instead of using
> a .notify() callback in struct acpi_driver.
> 
> No intentional functional impact.
> 
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

Applied.

Thanks,
Guenter

Re: [PATCH v1 3/3] hwmon: (acpi_power_meter) Convert ACPI driver to a platform one

[Guenter Roeck]

On Sun, Mar 01, 2026 at 02:18:49PM +0100, Rafael J. Wysocki wrote:
> From: "Rafael J. Wysocki" <rafael.j.wysocki@intel.com>
> 
> In all cases in which a struct acpi_driver is used for binding a driver
> to an ACPI device object, a corresponding platform device is created by
> the ACPI core and that device is regarded as a proper representation of
> underlying hardware.  Accordingly, a struct platform_driver should be
> used by driver code to bind to that device.  There are multiple reasons
> why drivers should not bind directly to ACPI device objects [1].
> 
> Overall, it is better to bind drivers to platform devices than to their
> ACPI companions, so convert the hwmon ACPI power meter driver to a
> platform one.
> 
> After this change, the subordinate hwmon device will be registered
> under the platform device representing the ACPI power meter, sysfs
> notifications will trigger on that device, and diagnostic messages
> will be printed relative to it instead of its ACPI companion.
> 
> Link: https://lore.kernel.org/all/2396510.ElGaqSPkdT@rafael.j.wysocki/ [1]
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

Thanks a lot for this cleanup work.

I decided to apply the entire series right away to hopefully get some test
feedback before the next commit window.

Thanks,
Guenter