Skip to contentSkip to conversation

ACPI: Convert button and battery drivers to platform ones

PATCH Superseded Rafael J. Wysocki 1 participant 11 messages Dec 9, 2025 ACPI

v1 of 2 Superseded Latest is v2 →

lore
Share & tools
lore

Rafael J. Wysocki

author maintainer
#0 lore raw

PATCH 10/10 ACPI: Convert button and battery drivers to platform ones

Hi All,

While binding drivers directly to struct acpi_device objects allows basic functionality to be provided, at least in the majority of cases, there are some problems with it, related to general consistency, sysfs layout, power management operation ordering, and code cleanliness.

First of all, struct acpi_device objects represent firmware entities rather than hardware and in many cases they provide auxiliary information on devices enumerated independently (like PCI devices or CPUs). It is therefore generally questionable to assign resources to them or create class devices and similar under them because they don't provide functionality associated with those entities by themselves (for example, they don't generate wakeup or input events).

As a general rule, a struct acpi_device can only be a parent of another struct acpi_device. If that's not the case, the location of the child device in the device hierarchy is at least confusing and it may not be straightforward to identify the piece of hardware corresponding to that device.

Using system suspend and resume callbacks directly with struct acpi_device objects is questionable either because it may cause ordering problems to happen. Namely, struct acpi_device objects are registered before any devices corresponded to by them and they land on the PM list before all of those devices. Consequently, the execution ordering of their PM callbacks may be different from what is generally expected. Moreover, dependencies returned by _DEP objects don't generally affect struct acpi_device objects themselves, only the "physical" device objects associated with them, which potentially is one more source of inconsistency.

All of the above means that binding drivers to struct acpi_device "devices" should generally be avoided and so this series converts three generic ACPI device drivers, the button driver, the tiny power button driver, and the battery driver, to platform drivers.

Patches [01-03/10] are preliminary for the button driver conversions. Patch [01/10] causes platform devices to be registered for "fixed event device" buttons, patch [02/10] cleans up the "fixed event device" registration code, and patch [03/10] rearranges the notification handling code in the button driver to use internal "button" structures for passing data instead of struct acpi_device objects.

Patches [04-05/10] convert the two button drivers to platform ones and patches [06-07/10] do some cleanups on top of them.

Patches [08-09/10] are preliminary for the battery driver conversion which is carried out in patch [10/10].

Thanks!

author maintainer
#1 lore raw

PATCH 1/10 ACPI: scan: Register platform devices for fixed event buttons

From: Rafael J. Wysocki <[email protected]>

On platforms using ACPI, power and sleep buttons may be so called "fixed event devices" in which case they are hooked up directly to the Fixed Events register in the platform via dedicated lines and there are no corresponding device objects in the ACPI namespace. Nevertheless, in Linux they get corresponding struct acpi_device objects with special device IDs, either LNXPWRBN or LNXSLPBN, which are then used for driver binding in a ususal way.

However, the function creating those struct acpi_device objects for "fixed event device" buttons, acpi_bus_scan_fixed(), does not register platform devices for them, unlike the generic code handling device enumeration based on the ACPI namespace. Consequently, if an ACPI power or sleep button is represented by a device object in the ACPI namespace, it will get a corresponding platform device, but if it is a "fixed event device", it will not get one, which is inconsistent and prevents the ACPI power button driver from being converted into a platform driver.

For the sake of consistency and to allow the ACPI power button driver to become a platform one going forward, modify acpi_bus_scan_fixed() to register platform devices for "fixed event device" buttons and update ACPI platform device registration code to work with non-device ACPI object types, so it can handle the buttons in question.

No intentional functional impact.

Signed-off-by Rafael J. Wysocki <[email protected]>
---
 drivers/acpi/acpi_platform.c |   29 ++++++++++++++++-------------
 drivers/acpi/scan.c          |    4 ++++
 2 files changed, 20 insertions(+), 13 deletions(-)
drivers/acpi/acpi_platform.c +16 -13
@@ -138,21 +138,24 @@ struct platform_device *acpi_create_plat
 	}
 
 	INIT_LIST_HEAD(&resource_list);
-	count = acpi_dev_get_resources(adev, &resource_list, NULL, NULL);
-	if (count < 0)
-		return NULL;
-	if (count > 0) {
-		resources = kcalloc(count, sizeof(*resources), GFP_KERNEL);
-		if (!resources) {
+	if (adev->device_type == ACPI_BUS_TYPE_DEVICE) {
+		count = acpi_dev_get_resources(adev, &resource_list, NULL, NULL);
+		if (count < 0)
+			return NULL;
+
+		if (count > 0) {
+			resources = kcalloc(count, sizeof(*resources), GFP_KERNEL);
+			if (!resources) {
+				acpi_dev_free_resource_list(&resource_list);
+				return ERR_PTR(-ENOMEM);
+			}
+			count = 0;
+			list_for_each_entry(rentry, &resource_list, node)
+				acpi_platform_fill_resource(adev, rentry->res,
+							    &resources[count++]);
+
 			acpi_dev_free_resource_list(&resource_list);
-			return ERR_PTR(-ENOMEM);
 		}
-		count = 0;
-		list_for_each_entry(rentry, &resource_list, node)
-			acpi_platform_fill_resource(adev, rentry->res,
-						    &resources[count++]);
-
-		acpi_dev_free_resource_list(&resource_list);
 	}
 
 	memset(&pdevinfo, 0, sizeof(pdevinfo));
drivers/acpi/scan.c +4 -0
@@ -2660,6 +2660,8 @@ static void acpi_bus_scan_fixed(void)
 				device_init_wakeup(&adev->dev, true);
 			else
 				dev_dbg(&adev->dev, "No driver\n");
+
+			acpi_default_enumeration(adev);
 		}
 	}
 

@@ -2672,6 +2674,8 @@ static void acpi_bus_scan_fixed(void)
 			adev->flags.match_driver = true;
 			if (device_attach(&adev->dev) < 0)
 				dev_dbg(&adev->dev, "No driver\n");
+
+			acpi_default_enumeration(adev);
 		}
 	}
 }
author maintainer
#2 lore raw

PATCH 2/10 ACPI: scan: Reduce code duplication related to fixed event devices

In reply to Rafael J. Wysocki

From: Rafael J. Wysocki <[email protected]>

Move duplicate fixed event device registration code from acpi_bus_scan_fixed() into a new function called acpi_bus_add_fixed_device_object() and make acpi_bus_scan_fixed() invoke that function as needed.

No intentional functional impact.

Signed-off-by Rafael J. Wysocki <[email protected]>
---
 drivers/acpi/scan.c |   42 +++++++++++++++++-------------------------
 1 file changed, 17 insertions(+), 25 deletions(-)
drivers/acpi/scan.c +17 -25
@@ -2647,37 +2647,29 @@ int acpi_bus_register_early_device(int t
 }
 EXPORT_SYMBOL_GPL(acpi_bus_register_early_device);
 
-static void acpi_bus_scan_fixed(void)
+static void acpi_bus_add_fixed_device_object(enum acpi_bus_device_type type)
 {
-	if (!(acpi_gbl_FADT.flags & ACPI_FADT_POWER_BUTTON)) {
-		struct acpi_device *adev = NULL;
+	struct acpi_device *adev = NULL;
 
-		acpi_add_single_object(&adev, NULL, ACPI_BUS_TYPE_POWER_BUTTON,
-				       false);
-		if (adev) {
-			adev->flags.match_driver = true;
-			if (device_attach(&adev->dev) >= 0)
-				device_init_wakeup(&adev->dev, true);
-			else
-				dev_dbg(&adev->dev, "No driver\n");
+	acpi_add_single_object(&adev, NULL, type, false);
+	if (adev) {
+		adev->flags.match_driver = true;
+		if (device_attach(&adev->dev) >= 0)
+			device_init_wakeup(&adev->dev, true);
+		else
+			dev_dbg(&adev->dev, "No driver\n");
 
-			acpi_default_enumeration(adev);
-		}
+		acpi_default_enumeration(adev);
 	}
+}
 
-	if (!(acpi_gbl_FADT.flags & ACPI_FADT_SLEEP_BUTTON)) {
-		struct acpi_device *adev = NULL;
-
-		acpi_add_single_object(&adev, NULL, ACPI_BUS_TYPE_SLEEP_BUTTON,
-				       false);
-		if (adev) {
-			adev->flags.match_driver = true;
-			if (device_attach(&adev->dev) < 0)
-				dev_dbg(&adev->dev, "No driver\n");
+static void acpi_bus_scan_fixed(void)
+{
+	if (!(acpi_gbl_FADT.flags & ACPI_FADT_POWER_BUTTON))
+		acpi_bus_add_fixed_device_object(ACPI_BUS_TYPE_POWER_BUTTON);
 
-			acpi_default_enumeration(adev);
-		}
-	}
+	if (!(acpi_gbl_FADT.flags & ACPI_FADT_SLEEP_BUTTON))
+		acpi_bus_add_fixed_device_object(ACPI_BUS_TYPE_SLEEP_BUTTON);
 }
 
 static void __init acpi_get_spcr_uart_addr(void)
author maintainer
#3 lore raw

PATCH 3/10 ACPI: button: Adjust event notification routines

In reply to Rafael J. Wysocki

From: Rafael J. Wysocki <[email protected]>

Adjust the event notification routines in the ACPI button driver to take a struct acpi_button pointer as an argument istead of a struct acpi_device one where applicable, which allows the use of acpi_driver_data() to be limited and will facilitate subsequent changes.

No intentional functional impact.

Signed-off-by Rafael J. Wysocki <[email protected]>
---
 drivers/acpi/button.c |   67 ++++++++++++++++++++++++--------------------------
 1 file changed, 33 insertions(+), 34 deletions(-)
drivers/acpi/button.c +33 -34
@@ -169,6 +169,7 @@ static struct acpi_driver acpi_button_dr
 };
 
 struct acpi_button {
+	struct acpi_device *adev;
 	unsigned int type;
 	struct input_dev *input;
 	char phys[32];			/* for input device */

@@ -202,9 +203,9 @@ static int acpi_lid_evaluate_state(struc
 	return lid_state ? 1 : 0;
 }
 
-static int acpi_lid_notify_state(struct acpi_device *device, int state)
+static int acpi_lid_notify_state(struct acpi_button *button, int state)
 {
-	struct acpi_button *button = acpi_driver_data(device);
+	struct acpi_device *device = button->adev;
 	ktime_t next_report;
 	bool do_update;
 

@@ -287,18 +288,18 @@ static int acpi_lid_notify_state(struct
 static int __maybe_unused acpi_button_state_seq_show(struct seq_file *seq,
 						     void *offset)
 {
-	struct acpi_device *device = seq->private;
+	struct acpi_button *button = seq->private;
 	int state;
 
-	state = acpi_lid_evaluate_state(device);
+	state = acpi_lid_evaluate_state(button->adev);
 	seq_printf(seq, "state:      %s\n",
 		   state < 0 ? "unsupported" : (state ? "open" : "closed"));
 	return 0;
 }
 
-static int acpi_button_add_fs(struct acpi_device *device)
+static int acpi_button_add_fs(struct acpi_button *button)
 {
-	struct acpi_button *button = acpi_driver_data(device);
+	struct acpi_device *device = button->adev;
 	struct proc_dir_entry *entry = NULL;
 	int ret = 0;
 

@@ -333,7 +334,7 @@ static int acpi_button_add_fs(struct acp
 	/* create /proc/acpi/button/lid/LID/state */
 	entry = proc_create_single_data(ACPI_BUTTON_FILE_STATE, S_IRUGO,
 			acpi_device_dir(device), acpi_button_state_seq_show,
-			device);
+			button);
 	if (!entry) {
 		ret = -ENODEV;
 		goto remove_dev_dir;

@@ -355,9 +356,9 @@ remove_button_dir:
 	goto done;
 }
 
-static int acpi_button_remove_fs(struct acpi_device *device)
+static int acpi_button_remove_fs(struct acpi_button *button)
 {
-	struct acpi_button *button = acpi_driver_data(device);
+	struct acpi_device *device = button->adev;
 
 	if (button->type != ACPI_BUTTON_TYPE_LID)
 		return 0;

@@ -385,9 +386,10 @@ int acpi_lid_open(void)
 }
 EXPORT_SYMBOL(acpi_lid_open);
 
-static int acpi_lid_update_state(struct acpi_device *device,
+static int acpi_lid_update_state(struct acpi_button *button,
 				 bool signal_wakeup)
 {
+	struct acpi_device *device = button->adev;
 	int state;
 
 	state = acpi_lid_evaluate_state(device);

@@ -397,19 +399,17 @@ static int acpi_lid_update_state(struct
 	if (state && signal_wakeup)
 		acpi_pm_wakeup_event(&device->dev);
 
-	return acpi_lid_notify_state(device, state);
+	return acpi_lid_notify_state(button, state);
 }
 
-static void acpi_lid_initialize_state(struct acpi_device *device)
+static void acpi_lid_initialize_state(struct acpi_button *button)
 {
-	struct acpi_button *button = acpi_driver_data(device);
-
 	switch (lid_init_state) {
 	case ACPI_BUTTON_LID_INIT_OPEN:
-		(void)acpi_lid_notify_state(device, 1);
+		(void)acpi_lid_notify_state(button, 1);
 		break;
 	case ACPI_BUTTON_LID_INIT_METHOD:
-		(void)acpi_lid_update_state(device, false);
+		(void)acpi_lid_update_state(button, false);
 		break;
 	case ACPI_BUTTON_LID_INIT_IGNORE:
 	default:

@@ -421,8 +421,8 @@ static void acpi_lid_initialize_state(st
 
 static void acpi_lid_notify(acpi_handle handle, u32 event, void *data)
 {
-	struct acpi_device *device = data;
-	struct acpi_button *button;
+	struct acpi_button *button = data;
+	struct acpi_device *device = button->adev;
 
 	if (event != ACPI_BUTTON_NOTIFY_STATUS) {
 		acpi_handle_debug(device->handle, "Unsupported event [0x%x]\n",

@@ -430,17 +430,16 @@ static void acpi_lid_notify(acpi_handle
 		return;
 	}
 
-	button = acpi_driver_data(device);
 	if (!button->lid_state_initialized)
 		return;
 
-	acpi_lid_update_state(device, true);
+	acpi_lid_update_state(button, true);
 }
 
 static void acpi_button_notify(acpi_handle handle, u32 event, void *data)
 {
-	struct acpi_device *device = data;
-	struct acpi_button *button;
+	struct acpi_button *button = data;
+	struct acpi_device *device = button->adev;
 	struct input_dev *input;
 	int keycode;
 

@@ -457,7 +456,6 @@ static void acpi_button_notify(acpi_hand
 
 	acpi_pm_wakeup_event(&device->dev);
 
-	button = acpi_driver_data(device);
 	if (button->suspended || event == ACPI_BUTTON_NOTIFY_WAKE)
 		return;
 

@@ -505,7 +503,7 @@ static int acpi_button_resume(struct dev
 	if (button->type == ACPI_BUTTON_TYPE_LID) {
 		button->last_state = !!acpi_lid_evaluate_state(device);
 		button->last_time = ktime_get();
-		acpi_lid_initialize_state(device);
+		acpi_lid_initialize_state(button);
 	}
 
 	if (button->type == ACPI_BUTTON_TYPE_POWER) {

@@ -521,12 +519,12 @@ static int acpi_button_resume(struct dev
 
 static int acpi_lid_input_open(struct input_dev *input)
 {
-	struct acpi_device *device = input_get_drvdata(input);
-	struct acpi_button *button = acpi_driver_data(device);
+	struct acpi_button *button = input_get_drvdata(input);
+	struct acpi_device *device = button->adev;
 
 	button->last_state = !!acpi_lid_evaluate_state(device);
 	button->last_time = ktime_get();
-	acpi_lid_initialize_state(device);
+	acpi_lid_initialize_state(button);
 
 	return 0;
 }

@@ -551,6 +549,7 @@ static int acpi_button_add(struct acpi_d
 
 	device->driver_data = button;
 
+	button->adev = device;
 	button->input = input = input_allocate_device();
 	if (!input) {
 		error = -ENOMEM;

@@ -587,7 +586,7 @@ static int acpi_button_add(struct acpi_d
 	}
 
 	if (!error)
-		error = acpi_button_add_fs(device);
+		error = acpi_button_add_fs(button);
 
 	if (error) {
 		input_free_device(input);

@@ -617,7 +616,7 @@ static int acpi_button_add(struct acpi_d
 		break;
 	}
 
-	input_set_drvdata(input, device);
+	input_set_drvdata(input, button);
 	error = input_register_device(input);
 	if (error) {
 		input_free_device(input);

@@ -628,17 +627,17 @@ static int acpi_button_add(struct acpi_d
 	case ACPI_BUS_TYPE_POWER_BUTTON:
 		status = acpi_install_fixed_event_handler(ACPI_EVENT_POWER_BUTTON,
 							  acpi_button_event,
-							  device);
+							  button);
 		break;
 	case ACPI_BUS_TYPE_SLEEP_BUTTON:
 		status = acpi_install_fixed_event_handler(ACPI_EVENT_SLEEP_BUTTON,
 							  acpi_button_event,
-							  device);
+							  button);
 		break;
 	default:
 		status = acpi_install_notify_handler(device->handle,
 						     ACPI_ALL_NOTIFY, handler,
-						     device);
+						     button);
 		break;
 	}
 	if (ACPI_FAILURE(status)) {

@@ -661,7 +660,7 @@ static int acpi_button_add(struct acpi_d
 err_input_unregister:
 	input_unregister_device(input);
 err_remove_fs:
-	acpi_button_remove_fs(device);
+	acpi_button_remove_fs(button);
 err_free_button:
 	kfree(button);
 	return error;

@@ -689,7 +688,7 @@ static void acpi_button_remove(struct ac
 	}
 	acpi_os_wait_events_complete();
 
-	acpi_button_remove_fs(device);
+	acpi_button_remove_fs(button);
 	input_unregister_device(button->input);
 	kfree(button);
 }
author maintainer
#4 lore raw

PATCH 4/10 ACPI: button: Convert the driver to a platform one

In reply to Rafael J. Wysocki

From: Rafael J. Wysocki <[email protected]>

While binding drivers directly to struct acpi_device objects allows basic functionality to be provided, at least in the majority of cases, there are some problems with it, related to general consistency, sysfs layout, power management operation ordering, and code cleanliness.

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

While this is not expected to alter functionality, it changes sysfs layout and so it will be visible to user space.

Signed-off-by Rafael J. Wysocki <[email protected]>
---
 drivers/acpi/button.c |   65 +++++++++++++++++++++++++++-----------------------
 drivers/acpi/scan.c   |    9 ------
 2 files changed, 37 insertions(+), 37 deletions(-)
drivers/acpi/button.c +36 -29
@@ -19,6 +19,7 @@
 #include <linux/slab.h>
 #include <linux/acpi.h>
 #include <linux/dmi.h>
+#include <linux/platform_device.h>
 #include <acpi/button.h>
 
 #define ACPI_BUTTON_CLASS		"button"

@@ -145,8 +146,8 @@ static const struct dmi_system_id dmi_li
 	{}
 };
 
-static int acpi_button_add(struct acpi_device *device);
-static void acpi_button_remove(struct acpi_device *device);
+static int acpi_button_probe(struct platform_device *pdev);
+static void acpi_button_remove(struct platform_device *pdev);
 
 #ifdef CONFIG_PM_SLEEP
 static int acpi_button_suspend(struct device *dev);

@@ -157,19 +158,19 @@ static int acpi_button_resume(struct dev
 #endif
 static SIMPLE_DEV_PM_OPS(acpi_button_pm, acpi_button_suspend, acpi_button_resume);
 
-static struct acpi_driver acpi_button_driver = {
-	.name = "button",
-	.class = ACPI_BUTTON_CLASS,
-	.ids = button_device_ids,
-	.ops = {
-		.add = acpi_button_add,
-		.remove = acpi_button_remove,
+static struct platform_driver acpi_button_driver = {
+	.probe = acpi_button_probe,
+	.remove = acpi_button_remove,
+	.driver = {
+		.name = "acpi-button",
+		.acpi_match_table = button_device_ids,
+		.pm = &acpi_button_pm,
 	},
-	.drv.pm = &acpi_button_pm,
 };
 
 struct acpi_button {
 	struct acpi_device *adev;
+	struct platform_device *pdev;
 	unsigned int type;
 	struct input_dev *input;
 	char phys[32];			/* for input device */

@@ -397,7 +398,7 @@ static int acpi_lid_update_state(struct
 		return state;
 
 	if (state && signal_wakeup)
-		acpi_pm_wakeup_event(&device->dev);
+		acpi_pm_wakeup_event(&button->pdev->dev);
 
 	return acpi_lid_notify_state(button, state);
 }

@@ -454,7 +455,7 @@ static void acpi_button_notify(acpi_hand
 		return;
 	}
 
-	acpi_pm_wakeup_event(&device->dev);
+	acpi_pm_wakeup_event(&button->pdev->dev);
 
 	if (button->suspended || event == ACPI_BUTTON_NOTIFY_WAKE)
 		return;

@@ -486,8 +487,8 @@ static u32 acpi_button_event(void *data)
 #ifdef CONFIG_PM_SLEEP
 static int acpi_button_suspend(struct device *dev)
 {
-	struct acpi_device *device = to_acpi_device(dev);
-	struct acpi_button *button = acpi_driver_data(device);
+	struct platform_device *pdev = to_platform_device(dev);
+	struct acpi_button *button = platform_get_drvdata(pdev);
 
 	button->suspended = true;
 	return 0;

@@ -495,9 +496,10 @@ static int acpi_button_suspend(struct de
 
 static int acpi_button_resume(struct device *dev)
 {
+	struct platform_device *pdev = to_platform_device(dev);
+	struct acpi_button *button = platform_get_drvdata(pdev);
+	struct acpi_device *device = ACPI_COMPANION(dev);
 	struct input_dev *input;
-	struct acpi_device *device = to_acpi_device(dev);
-	struct acpi_button *button = acpi_driver_data(device);
 
 	button->suspended = false;
 	if (button->type == ACPI_BUTTON_TYPE_LID) {

@@ -529,8 +531,9 @@ static int acpi_lid_input_open(struct in
 	return 0;
 }
 
-static int acpi_button_add(struct acpi_device *device)
+static int acpi_button_probe(struct platform_device *pdev)
 {
+	struct acpi_device *device = ACPI_COMPANION(&pdev->dev);
 	acpi_notify_handler handler;
 	struct acpi_button *button;
 	struct input_dev *input;

@@ -547,8 +550,9 @@ static int acpi_button_add(struct acpi_d
 	if (!button)
 		return -ENOMEM;
 
-	device->driver_data = button;
+	platform_set_drvdata(pdev, button);
 
+	button->pdev = pdev;
 	button->adev = device;
 	button->input = input = input_allocate_device();
 	if (!input) {

@@ -599,7 +603,7 @@ static int acpi_button_add(struct acpi_d
 	input->phys = button->phys;
 	input->id.bustype = BUS_HOST;
 	input->id.product = button->type;
-	input->dev.parent = &device->dev;
+	input->dev.parent = &pdev->dev;
 
 	switch (button->type) {
 	case ACPI_BUTTON_TYPE_POWER:

@@ -653,7 +657,7 @@ static int acpi_button_add(struct acpi_d
 		lid_device = device;
 	}
 
-	device_init_wakeup(&device->dev, true);
+	device_init_wakeup(&pdev->dev, true);
 	pr_info("%s [%s]\n", name, acpi_device_bid(device));
 	return 0;
 

@@ -662,13 +666,15 @@ err_input_unregister:
 err_remove_fs:
 	acpi_button_remove_fs(button);
 err_free_button:
+	platform_set_drvdata(pdev, NULL);
 	kfree(button);
 	return error;
 }
 
-static void acpi_button_remove(struct acpi_device *device)
+static void acpi_button_remove(struct platform_device *pdev)
 {
-	struct acpi_button *button = acpi_driver_data(device);
+	struct acpi_device *device = ACPI_COMPANION(&pdev->dev);
+	struct acpi_button *button = platform_get_drvdata(pdev);
 
 	switch (device->device_type) {
 	case ACPI_BUS_TYPE_POWER_BUTTON:

@@ -690,6 +696,7 @@ static void acpi_button_remove(struct ac
 
 	acpi_button_remove_fs(button);
 	input_unregister_device(button->input);
+	platform_set_drvdata(pdev, NULL);
 	kfree(button);
 }
 

@@ -727,7 +734,7 @@ module_param_call(lid_init_state,
 		  NULL, 0644);
 MODULE_PARM_DESC(lid_init_state, "Behavior for reporting LID initial state");
 
-static int acpi_button_register_driver(struct acpi_driver *driver)
+static int __init acpi_button_init(void)
 {
 	const struct dmi_system_id *dmi_id;
 

@@ -743,20 +750,20 @@ static int acpi_button_register_driver(s
 	 * Modules such as nouveau.ko and i915.ko have a link time dependency
 	 * on acpi_lid_open(), and would therefore not be loadable on ACPI
 	 * capable kernels booted in non-ACPI mode if the return value of
-	 * acpi_bus_register_driver() is returned from here with ACPI disabled
+	 * platform_driver_register() is returned from here with ACPI disabled
 	 * when this driver is built as a module.
 	 */
 	if (acpi_disabled)
 		return 0;
 
-	return acpi_bus_register_driver(driver);
+	return platform_driver_register(&acpi_button_driver);
 }
 
-static void acpi_button_unregister_driver(struct acpi_driver *driver)
+static void __exit acpi_button_exit(void)
 {
 	if (!acpi_disabled)
-		acpi_bus_unregister_driver(driver);
+		platform_driver_unregister(&acpi_button_driver);
 }
 
-module_driver(acpi_button_driver, acpi_button_register_driver,
-	       acpi_button_unregister_driver);
+module_init(acpi_button_init);
+module_exit(acpi_button_exit);
author maintainer
#5 lore raw

PATCH 5/10 ACPI: tiny-power-button: Convert the driver to a platform one

In reply to Rafael J. Wysocki

From: Rafael J. Wysocki <[email protected]>

While binding drivers directly to struct acpi_device objects allows basic functionality to be provided, at least in the majority of cases, there are some problems with it, related to general consistency, sysfs layout, power management operation ordering, and code cleanliness.

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

While this is not expected to alter functionality, it changes sysfs layout and so it will be visible to user space.

Signed-off-by Rafael J. Wysocki <[email protected]>
---
 drivers/acpi/tiny-power-button.c |   27 +++++++++++++++------------
 1 file changed, 15 insertions(+), 12 deletions(-)
drivers/acpi/tiny-power-button.c +15 -12
@@ -1,7 +1,8 @@
 // SPDX-License-Identifier: GPL-2.0-or-later
+#include <linux/acpi.h>
 #include <linux/module.h>
+#include <linux/platform_device.h>
 #include <linux/sched/signal.h>
-#include <linux/acpi.h>
 #include <acpi/button.h>
 
 MODULE_AUTHOR("Josh Triplett");

@@ -35,8 +36,9 @@ static u32 acpi_tiny_power_button_event(
 	return ACPI_INTERRUPT_HANDLED;
 }
 
-static int acpi_tiny_power_button_add(struct acpi_device *device)
+static int acpi_tiny_power_button_probe(struct platform_device *pdev)
 {
+	struct acpi_device *device = ACPI_COMPANION(&pdev->dev);
 	acpi_status status;
 
 	if (device->device_type == ACPI_BUS_TYPE_POWER_BUTTON) {

@@ -55,8 +57,10 @@ static int acpi_tiny_power_button_add(st
 	return 0;
 }
 
-static void acpi_tiny_power_button_remove(struct acpi_device *device)
+static void acpi_tiny_power_button_remove(struct platform_device *pdev)
 {
+	struct acpi_device *device = ACPI_COMPANION(&pdev->dev);
+
 	if (device->device_type == ACPI_BUS_TYPE_POWER_BUTTON) {
 		acpi_remove_fixed_event_handler(ACPI_EVENT_POWER_BUTTON,
 						acpi_tiny_power_button_event);

@@ -67,14 +71,13 @@ static void acpi_tiny_power_button_remov
 	acpi_os_wait_events_complete();
 }
 
-static struct acpi_driver acpi_tiny_power_button_driver = {
-	.name = "tiny-power-button",
-	.class = "tiny-power-button",
-	.ids = tiny_power_button_device_ids,
-	.ops = {
-		.add = acpi_tiny_power_button_add,
-		.remove = acpi_tiny_power_button_remove,
-	},
+static struct platform_driver acpi_tiny_power_button_driver = {
+	.probe = acpi_tiny_power_button_probe,
+	.remove = acpi_tiny_power_button_remove,
+	.driver = {
+		.name = "acpi-tiny-power-button",
+		.acpi_match_table = tiny_power_button_device_ids,
+ 	},
 };
 
-module_acpi_driver(acpi_tiny_power_button_driver);
+module_platform_driver(acpi_tiny_power_button_driver);
author maintainer
#6 lore raw

PATCH 6/10 ACPI: scan: Do not bind ACPI drivers to fixed event buttons

In reply to Rafael J. Wysocki

From: Rafael J. Wysocki <[email protected]>

Both ACPI button drivers have been converted to platform ones, so there is no reason to attempt to bind an ACPI driver to a struct acpi_device representing a fixed event device button.

Update the relevant code accordingly.

No intentional functional impact.

Signed-off-by Rafael J. Wysocki <[email protected]>
---
 drivers/acpi/scan.c |    9 +--------
 1 file changed, 1 insertion(+), 8 deletions(-)
drivers/acpi/scan.c +1 -8
@@ -2652,15 +2652,8 @@ static void acpi_bus_add_fixed_device_ob
 	struct acpi_device *adev = NULL;
 
 	acpi_add_single_object(&adev, NULL, type, false);
-	if (adev) {
-		adev->flags.match_driver = true;
-		if (device_attach(&adev->dev) >= 0)
-			device_init_wakeup(&adev->dev, true);
-		else
-			dev_dbg(&adev->dev, "No driver\n");
-
+	if (adev)
 		acpi_default_enumeration(adev);
-	}
 }
 
 static void acpi_bus_scan_fixed(void)
author maintainer
#7 lore raw

PATCH 7/10 ACPI: scan: Do not mark button ACPI devices as wakeup-capable

In reply to Rafael J. Wysocki

From: Rafael J. Wysocki <[email protected]>

It is generally questionable to mark struct acpi_device "devices" as wakeup-capable because they represent firmware entities that by themselves have no wakeup capabilities.

It was done for struct acpi_device "devices" corresponding to buttons because the ACPI button driver was binding to them directly, but now that corresponding platform devices are created for the buttons and they are marked as wakeup-capable by the ACPI button driver, there is no reason to continue doing it.

Update acpi_wakeup_gpe_init() accordingly.

Signed-off-by Rafael J. Wysocki <[email protected]>
---
 drivers/acpi/scan.c |    1 -
 1 file changed, 1 deletion(-)
drivers/acpi/scan.c +0 -1
@@ -1022,7 +1022,6 @@ static bool acpi_wakeup_gpe_init(struct
 		    wakeup->sleep_state == ACPI_STATE_S5)
 			wakeup->sleep_state = ACPI_STATE_S4;
 		acpi_mark_gpe_for_wake(wakeup->gpe_device, wakeup->gpe_number);
-		device_set_wakeup_capable(&device->dev, true);
 		return true;
 	}
author maintainer
#8 lore raw

PATCH 8/10 ACPI: battery: Adjust event notification routine

In reply to Rafael J. Wysocki

From: Rafael J. Wysocki <[email protected]>

Adjust acpi_battery_notify() to cast its "data" argument to a struct acpi_battery pointer instead of a struct acpi_device one, which allows the use of acpi_driver_data() to be limited and will facilitate subsequent changes.

No intentional functional impact.

Signed-off-by Rafael J. Wysocki <[email protected]>
---
 drivers/acpi/battery.c |    6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)
drivers/acpi/battery.c +3 -3
@@ -1054,8 +1054,8 @@ static void acpi_battery_refresh(struct
 /* Driver Interface */
 static void acpi_battery_notify(acpi_handle handle, u32 event, void *data)
 {
-	struct acpi_device *device = data;
-	struct acpi_battery *battery = acpi_driver_data(device);
+	struct acpi_battery *battery = data;
+	struct acpi_device *device = battery->device;
 	struct power_supply *old;
 
 	if (!battery)

@@ -1249,7 +1249,7 @@ static int acpi_battery_add(struct acpi_
 	device_init_wakeup(&device->dev, 1);
 
 	result = acpi_dev_install_notify_handler(device, ACPI_ALL_NOTIFY,
-						 acpi_battery_notify, device);
+						 acpi_battery_notify, battery);
 	if (result)
 		goto fail_pm;
author maintainer
#9 lore raw

PATCH 9/10 ACPI: battery: Reduce code duplication related to cleanup

In reply to Rafael J. Wysocki

From: Rafael J. Wysocki <[email protected]>

Notice that sysfs_battery_cleanup() calls sysfs_remove_battery() under battery->update_lock which is also done in acpi_battery_remove(), so adjust the latter to use it.

No intentional functional impact.

Signed-off-by Rafael J. Wysocki <[email protected]>
---
 drivers/acpi/battery.c |    4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)
drivers/acpi/battery.c +1 -3
@@ -1279,9 +1279,7 @@ static void acpi_battery_remove(struct a
 	device_init_wakeup(&device->dev, 0);
 	unregister_pm_notifier(&battery->pm_nb);
 
-	guard(mutex)(&battery->update_lock);
-
-	sysfs_remove_battery(battery);
+	sysfs_battery_cleanup(battery);
 }
 
 /* this is needed to learn about changes made in suspended state */
author maintainer
#10 lore raw

PATCH 10/10 ACPI: battery: Convert the driver to a platform one

In reply to Rafael J. Wysocki

From: Rafael J. Wysocki <[email protected]>

While binding drivers directly to struct acpi_device objects allows basic functionality to be provided, at least in the majority of cases, there are some problems with it, related to general consistency, sysfs layout, power management operation ordering, and code cleanliness.

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

While this is not expected to alter functionality, it changes sysfs layout and so it will be visible to user space.

Signed-off-by Rafael J. Wysocki <[email protected]>
---
 drivers/acpi/battery.c |   63 +++++++++++++++++++++++--------------------------
 1 file changed, 30 insertions(+), 33 deletions(-)
drivers/acpi/battery.c +30 -33
@@ -17,6 +17,7 @@
 #include <linux/list.h>
 #include <linux/module.h>
 #include <linux/mutex.h>
+#include <linux/platform_device.h>
 #include <linux/slab.h>
 #include <linux/suspend.h>
 #include <linux/types.h>

@@ -1208,26 +1209,26 @@ static void sysfs_battery_cleanup(struct
 	sysfs_remove_battery(battery);
 }
 
-static int acpi_battery_add(struct acpi_device *device)
+static int acpi_battery_probe(struct platform_device *pdev)
 {
-	int result = 0;
+	struct acpi_device *device = ACPI_COMPANION(&pdev->dev);
 	struct acpi_battery *battery;
-
-	if (!device)
-		return -EINVAL;
+	int result;
 
 	if (device->dep_unmet)
 		return -EPROBE_DEFER;
 
-	battery = devm_kzalloc(&device->dev, sizeof(*battery), GFP_KERNEL);
+	battery = devm_kzalloc(&pdev->dev, sizeof(*battery), GFP_KERNEL);
 	if (!battery)
 		return -ENOMEM;
+
+	platform_set_drvdata(pdev, battery);
+
 	battery->device = device;
 	strscpy(acpi_device_name(device), ACPI_BATTERY_DEVICE_NAME);
 	strscpy(acpi_device_class(device), ACPI_BATTERY_CLASS);
-	device->driver_data = battery;
 
-	result = devm_mutex_init(&device->dev, &battery->update_lock);
+	result = devm_mutex_init(&pdev->dev, &battery->update_lock);
 	if (result)
 		return result;
 

@@ -1246,7 +1247,7 @@ static int acpi_battery_add(struct acpi_
 	if (result)
 		goto fail;
 
-	device_init_wakeup(&device->dev, 1);
+	device_init_wakeup(&pdev->dev, true);
 
 	result = acpi_dev_install_notify_handler(device, ACPI_ALL_NOTIFY,
 						 acpi_battery_notify, battery);

@@ -1256,41 +1257,38 @@ static int acpi_battery_add(struct acpi_
 	return 0;
 
 fail_pm:
-	device_init_wakeup(&device->dev, 0);
+	device_init_wakeup(&pdev->dev, false);
 	unregister_pm_notifier(&battery->pm_nb);
 fail:
 	sysfs_battery_cleanup(battery);
+	platform_set_drvdata(pdev, NULL);
 
 	return result;
 }
 
-static void acpi_battery_remove(struct acpi_device *device)
+static void acpi_battery_remove(struct platform_device *pdev)
 {
-	struct acpi_battery *battery;
+	struct acpi_device *device = ACPI_COMPANION(&pdev->dev);
+	struct acpi_battery *battery = platform_get_drvdata(pdev);
 
-	if (!device || !acpi_driver_data(device))
+	if (!device || !battery)
 		return;
 
-	battery = acpi_driver_data(device);
-
 	acpi_dev_remove_notify_handler(device, ACPI_ALL_NOTIFY,
 				       acpi_battery_notify);
 
-	device_init_wakeup(&device->dev, 0);
+	device_init_wakeup(&pdev->dev, false);
 	unregister_pm_notifier(&battery->pm_nb);
 
 	sysfs_battery_cleanup(battery);
+	platform_set_drvdata(pdev, NULL);
 }
 
 /* this is needed to learn about changes made in suspended state */
 static int acpi_battery_resume(struct device *dev)
 {
-	struct acpi_battery *battery;
-
-	if (!dev)
-		return -EINVAL;
+	struct acpi_battery *battery = platform_get_drvdata(to_platform_device(dev));
 
-	battery = acpi_driver_data(to_acpi_device(dev));
 	if (!battery)
 		return -EINVAL;
 

@@ -1304,16 +1302,15 @@ static int acpi_battery_resume(struct de
 
 static DEFINE_SIMPLE_DEV_PM_OPS(acpi_battery_pm, NULL, acpi_battery_resume);
 
-static struct acpi_driver acpi_battery_driver = {
-	.name = "battery",
-	.class = ACPI_BATTERY_CLASS,
-	.ids = battery_device_ids,
-	.ops = {
-		.add = acpi_battery_add,
-		.remove = acpi_battery_remove,
-		},
-	.drv.pm = pm_sleep_ptr(&acpi_battery_pm),
-	.drv.probe_type = PROBE_PREFER_ASYNCHRONOUS,
+static struct platform_driver acpi_battery_driver = {
+	.probe = acpi_battery_probe,
+	.remove = acpi_battery_remove,
+	.driver = {
+		.name = "acpi-battery",
+		.acpi_match_table = battery_device_ids,
+		.pm = pm_sleep_ptr(&acpi_battery_pm),
+		.probe_type = PROBE_PREFER_ASYNCHRONOUS,
+	},
 };
 
 static int __init acpi_battery_init(void)

@@ -1323,12 +1320,12 @@ static int __init acpi_battery_init(void
 
 	dmi_check_system(bat_dmi_table);
 
-	return acpi_bus_register_driver(&acpi_battery_driver);
+	return platform_driver_register(&acpi_battery_driver);
 }
 
 static void __exit acpi_battery_exit(void)
 {
-	acpi_bus_unregister_driver(&acpi_battery_driver);
+	platform_driver_unregister(&acpi_battery_driver);
 	battery_hook_exit();
 }