[PATCH v1 4/4] platform/x86: asus-wireless: Convert ACPI driver to a platform one

Posted by Rafael J. Wysocki 1 month, 2 weeks ago

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 Asus wireless ACPI driver to a platform
one.

After this change, the subordinate input and LED devices will be
registered under the platform device used for driver binding instead of
its ACPI companion.

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

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

diff --git a/drivers/platform/x86/asus-wireless.c b/drivers/platform/x86/asus-wireless.c
index 45d41875c515..2b494bf3cba8 100644
--- a/drivers/platform/x86/asus-wireless.c
+++ b/drivers/platform/x86/asus-wireless.c
@@ -12,6 +12,7 @@
 #include <linux/acpi.h>
 #include <linux/input.h>
 #include <linux/pci_ids.h>
+#include <linux/platform_device.h>
 #include <linux/leds.h>
 
 struct hswc_params {
@@ -124,19 +125,22 @@ static void asus_wireless_notify(acpi_handle handle, u32 event, void *context)
 	input_sync(data->idev);
 }
 
-static int asus_wireless_add(struct acpi_device *adev)
+static int asus_wireless_probe(struct platform_device *pdev)
 {
+	struct acpi_device *adev = ACPI_COMPANION(&pdev->dev);
 	struct asus_wireless_data *data;
 	const struct acpi_device_id *id;
 	int err;
 
-	data = devm_kzalloc(&adev->dev, sizeof(*data), GFP_KERNEL);
+	data = devm_kzalloc(&pdev->dev, sizeof(*data), GFP_KERNEL);
 	if (!data)
 		return -ENOMEM;
-	adev->driver_data = data;
+
+	platform_set_drvdata(pdev, data);
+
 	data->adev = adev;
 
-	data->idev = devm_input_allocate_device(&adev->dev);
+	data->idev = devm_input_allocate_device(&pdev->dev);
 	if (!data->idev)
 		return -ENOMEM;
 	data->idev->name = "Asus Wireless Radio Control";
@@ -165,14 +169,14 @@ static int asus_wireless_add(struct acpi_device *adev)
 	data->led.flags = LED_CORE_SUSPENDRESUME;
 	data->led.max_brightness = 1;
 	data->led.default_trigger = "rfkill-none";
-	err = devm_led_classdev_register(&adev->dev, &data->led);
+	err = devm_led_classdev_register(&pdev->dev, &data->led);
 	if (err)
 		goto err;
 
 	err = acpi_dev_install_notify_handler(adev, ACPI_DEVICE_NOTIFY,
 					      asus_wireless_notify, data);
 	if (err) {
-		devm_led_classdev_unregister(&adev->dev, &data->led);
+		devm_led_classdev_unregister(&pdev->dev, &data->led);
 		goto err;
 	}
 	return 0;
@@ -182,28 +186,27 @@ static int asus_wireless_add(struct acpi_device *adev)
 	return err;
 }
 
-static void asus_wireless_remove(struct acpi_device *adev)
+static void asus_wireless_remove(struct platform_device *pdev)
 {
-	struct asus_wireless_data *data = acpi_driver_data(adev);
+	struct asus_wireless_data *data = platform_get_drvdata(pdev);
 
-	acpi_dev_remove_notify_handler(adev, ACPI_DEVICE_NOTIFY,
+	acpi_dev_remove_notify_handler(data->adev, ACPI_DEVICE_NOTIFY,
 				       asus_wireless_notify);
 	if (data->wq) {
-		devm_led_classdev_unregister(&adev->dev, &data->led);
+		devm_led_classdev_unregister(&pdev->dev, &data->led);
 		destroy_workqueue(data->wq);
 	}
 }
 
-static struct acpi_driver asus_wireless_driver = {
-	.name = "Asus Wireless Radio Control Driver",
-	.class = "hotkey",
-	.ids = device_ids,
-	.ops = {
-		.add = asus_wireless_add,
-		.remove = asus_wireless_remove,
+static struct platform_driver asus_wireless_driver = {
+	.probe = asus_wireless_probe,
+	.remove = asus_wireless_remove,
+	.driver = {
+		.name = "Asus Wireless Radio Control Driver",
+		.acpi_match_table = device_ids,
 	},
 };
-module_acpi_driver(asus_wireless_driver);
+module_platform_driver(asus_wireless_driver);
 
 MODULE_DESCRIPTION("Asus Wireless Radio Control Driver");
 MODULE_AUTHOR("João Paulo Rechi Vita <jprvita@gmail.com>");
-- 
2.51.0

Re: [PATCH v1 4/4] platform/x86: asus-wireless: Convert ACPI driver to a platform one

Posted by Ilpo Järvinen 1 week, 1 day ago

On Sat, 28 Feb 2026, 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 Asus wireless ACPI driver to a platform
> one.
> 
> After this change, the subordinate input and LED devices will be
> registered under the platform device used for driver binding instead of
> its ACPI companion.
> 
> While this is not expected to alter functionality, it changes sysfs
> layout and so it will be visible to user space.
> 
> Link: https://lore.kernel.org/all/2396510.ElGaqSPkdT@rafael.j.wysocki/ [1]
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> ---
>  drivers/platform/x86/asus-wireless.c | 39 +++++++++++++++-------------
>  1 file changed, 21 insertions(+), 18 deletions(-)
> 
> diff --git a/drivers/platform/x86/asus-wireless.c b/drivers/platform/x86/asus-wireless.c
> index 45d41875c515..2b494bf3cba8 100644
> --- a/drivers/platform/x86/asus-wireless.c
> +++ b/drivers/platform/x86/asus-wireless.c
> @@ -12,6 +12,7 @@
>  #include <linux/acpi.h>
>  #include <linux/input.h>
>  #include <linux/pci_ids.h>
> +#include <linux/platform_device.h>
>  #include <linux/leds.h>
>  
>  struct hswc_params {
> @@ -124,19 +125,22 @@ static void asus_wireless_notify(acpi_handle handle, u32 event, void *context)
>  	input_sync(data->idev);
>  }
>  
> -static int asus_wireless_add(struct acpi_device *adev)
> +static int asus_wireless_probe(struct platform_device *pdev)
>  {
> +	struct acpi_device *adev = ACPI_COMPANION(&pdev->dev);
>  	struct asus_wireless_data *data;
>  	const struct acpi_device_id *id;
>  	int err;
>  
> -	data = devm_kzalloc(&adev->dev, sizeof(*data), GFP_KERNEL);
> +	data = devm_kzalloc(&pdev->dev, sizeof(*data), GFP_KERNEL);
>  	if (!data)
>  		return -ENOMEM;
> -	adev->driver_data = data;
> +
> +	platform_set_drvdata(pdev, data);
> +
>  	data->adev = adev;
>  
> -	data->idev = devm_input_allocate_device(&adev->dev);
> +	data->idev = devm_input_allocate_device(&pdev->dev);
>  	if (!data->idev)
>  		return -ENOMEM;
>  	data->idev->name = "Asus Wireless Radio Control";
> @@ -165,14 +169,14 @@ static int asus_wireless_add(struct acpi_device *adev)
>  	data->led.flags = LED_CORE_SUSPENDRESUME;
>  	data->led.max_brightness = 1;
>  	data->led.default_trigger = "rfkill-none";
> -	err = devm_led_classdev_register(&adev->dev, &data->led);
> +	err = devm_led_classdev_register(&pdev->dev, &data->led);
>  	if (err)
>  		goto err;
>  
>  	err = acpi_dev_install_notify_handler(adev, ACPI_DEVICE_NOTIFY,
>  					      asus_wireless_notify, data);
>  	if (err) {
> -		devm_led_classdev_unregister(&adev->dev, &data->led);
> +		devm_led_classdev_unregister(&pdev->dev, &data->led);

I've taken this series now into the review-ilpo-next branch.

However, here are manual devm_*() rollbacks which could probably be 
eliminated as well but you can do them on top of this.

--
 i.

>  		goto err;
>  	}
>  	return 0;
> @@ -182,28 +186,27 @@ static int asus_wireless_add(struct acpi_device *adev)
>  	return err;
>  }
>  
> -static void asus_wireless_remove(struct acpi_device *adev)
> +static void asus_wireless_remove(struct platform_device *pdev)
>  {
> -	struct asus_wireless_data *data = acpi_driver_data(adev);
> +	struct asus_wireless_data *data = platform_get_drvdata(pdev);
>  
> -	acpi_dev_remove_notify_handler(adev, ACPI_DEVICE_NOTIFY,
> +	acpi_dev_remove_notify_handler(data->adev, ACPI_DEVICE_NOTIFY,
>  				       asus_wireless_notify);
>  	if (data->wq) {
> -		devm_led_classdev_unregister(&adev->dev, &data->led);
> +		devm_led_classdev_unregister(&pdev->dev, &data->led);
>  		destroy_workqueue(data->wq);
>  	}
>  }
>  
> -static struct acpi_driver asus_wireless_driver = {
> -	.name = "Asus Wireless Radio Control Driver",
> -	.class = "hotkey",
> -	.ids = device_ids,
> -	.ops = {
> -		.add = asus_wireless_add,
> -		.remove = asus_wireless_remove,
> +static struct platform_driver asus_wireless_driver = {
> +	.probe = asus_wireless_probe,
> +	.remove = asus_wireless_remove,
> +	.driver = {
> +		.name = "Asus Wireless Radio Control Driver",
> +		.acpi_match_table = device_ids,
>  	},
>  };
> -module_acpi_driver(asus_wireless_driver);
> +module_platform_driver(asus_wireless_driver);
>  
>  MODULE_DESCRIPTION("Asus Wireless Radio Control Driver");
>  MODULE_AUTHOR("João Paulo Rechi Vita <jprvita@gmail.com>");
>

Re: [PATCH v1 4/4] platform/x86: asus-wireless: Convert ACPI driver to a platform one

Posted by Denis Benato 1 month, 2 weeks ago

On 2/28/26 16:12, 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 Asus wireless ACPI driver to a platform
> one.
>
> After this change, the subordinate input and LED devices will be
> registered under the platform device used for driver binding instead of
> its ACPI companion.
>
> While this is not expected to alter functionality, it changes sysfs
> layout and so it will be visible to user space.
>
> Link: https://lore.kernel.org/all/2396510.ElGaqSPkdT@rafael.j.wysocki/ [1]
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Reviewed-by: Denis Benato <denis.benato@linux.dev>
> ---
>  drivers/platform/x86/asus-wireless.c | 39 +++++++++++++++-------------
>  1 file changed, 21 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/platform/x86/asus-wireless.c b/drivers/platform/x86/asus-wireless.c
> index 45d41875c515..2b494bf3cba8 100644
> --- a/drivers/platform/x86/asus-wireless.c
> +++ b/drivers/platform/x86/asus-wireless.c
> @@ -12,6 +12,7 @@
>  #include <linux/acpi.h>
>  #include <linux/input.h>
>  #include <linux/pci_ids.h>
> +#include <linux/platform_device.h>
>  #include <linux/leds.h>
>  
>  struct hswc_params {
> @@ -124,19 +125,22 @@ static void asus_wireless_notify(acpi_handle handle, u32 event, void *context)
>  	input_sync(data->idev);
>  }
>  
> -static int asus_wireless_add(struct acpi_device *adev)
> +static int asus_wireless_probe(struct platform_device *pdev)
>  {
> +	struct acpi_device *adev = ACPI_COMPANION(&pdev->dev);
>  	struct asus_wireless_data *data;
>  	const struct acpi_device_id *id;
>  	int err;
>  
> -	data = devm_kzalloc(&adev->dev, sizeof(*data), GFP_KERNEL);
> +	data = devm_kzalloc(&pdev->dev, sizeof(*data), GFP_KERNEL);
>  	if (!data)
>  		return -ENOMEM;
> -	adev->driver_data = data;
> +
> +	platform_set_drvdata(pdev, data);
> +
>  	data->adev = adev;
>  
> -	data->idev = devm_input_allocate_device(&adev->dev);
> +	data->idev = devm_input_allocate_device(&pdev->dev);
>  	if (!data->idev)
>  		return -ENOMEM;
>  	data->idev->name = "Asus Wireless Radio Control";
> @@ -165,14 +169,14 @@ static int asus_wireless_add(struct acpi_device *adev)
>  	data->led.flags = LED_CORE_SUSPENDRESUME;
>  	data->led.max_brightness = 1;
>  	data->led.default_trigger = "rfkill-none";
> -	err = devm_led_classdev_register(&adev->dev, &data->led);
> +	err = devm_led_classdev_register(&pdev->dev, &data->led);
>  	if (err)
>  		goto err;
>  
>  	err = acpi_dev_install_notify_handler(adev, ACPI_DEVICE_NOTIFY,
>  					      asus_wireless_notify, data);
>  	if (err) {
> -		devm_led_classdev_unregister(&adev->dev, &data->led);
> +		devm_led_classdev_unregister(&pdev->dev, &data->led);
>  		goto err;
>  	}
>  	return 0;
> @@ -182,28 +186,27 @@ static int asus_wireless_add(struct acpi_device *adev)
>  	return err;
>  }
>  
> -static void asus_wireless_remove(struct acpi_device *adev)
> +static void asus_wireless_remove(struct platform_device *pdev)
>  {
> -	struct asus_wireless_data *data = acpi_driver_data(adev);
> +	struct asus_wireless_data *data = platform_get_drvdata(pdev);
>  
> -	acpi_dev_remove_notify_handler(adev, ACPI_DEVICE_NOTIFY,
> +	acpi_dev_remove_notify_handler(data->adev, ACPI_DEVICE_NOTIFY,
>  				       asus_wireless_notify);
>  	if (data->wq) {
> -		devm_led_classdev_unregister(&adev->dev, &data->led);
> +		devm_led_classdev_unregister(&pdev->dev, &data->led);
>  		destroy_workqueue(data->wq);
>  	}
>  }
>  
> -static struct acpi_driver asus_wireless_driver = {
> -	.name = "Asus Wireless Radio Control Driver",
> -	.class = "hotkey",
> -	.ids = device_ids,
> -	.ops = {
> -		.add = asus_wireless_add,
> -		.remove = asus_wireless_remove,
> +static struct platform_driver asus_wireless_driver = {
> +	.probe = asus_wireless_probe,
> +	.remove = asus_wireless_remove,
> +	.driver = {
> +		.name = "Asus Wireless Radio Control Driver",
> +		.acpi_match_table = device_ids,
>  	},
>  };
> -module_acpi_driver(asus_wireless_driver);
> +module_platform_driver(asus_wireless_driver);
>  
>  MODULE_DESCRIPTION("Asus Wireless Radio Control Driver");
>  MODULE_AUTHOR("João Paulo Rechi Vita <jprvita@gmail.com>");