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

["Ilpo Järvinen" <ilpo.jarvinen@linux.intel.com>]

[-- Attachment #1: Type: text/plain, Size: 4976 bytes --]

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