[PATCH v1 0/2] platform/x86: system76: Bind to a platform device instead of an ACPI one

[PATCH v1 0/2] platform/x86: system76: Bind to a platform device instead of an ACPI one

[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 platform x86 System76
ACPI driver.

Patch [1/2] 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 [2/2] converts the driver to using struct platform_driver for device
binding.

Thanks!

[PATCH v1 1/2] platform/x86: system76: 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/platform/x86/system76_acpi.c | 17 +++++++++++++----
 1 file changed, 13 insertions(+), 4 deletions(-)

diff --git a/drivers/platform/x86/system76_acpi.c b/drivers/platform/x86/system76_acpi.c
index 3da753b3d00d..35e294a55fa3 100644
--- a/drivers/platform/x86/system76_acpi.c
+++ b/drivers/platform/x86/system76_acpi.c
@@ -644,11 +644,10 @@ static void input_key(struct system76_data *data, unsigned int code)
 }
 
 // Handle ACPI notification
-static void system76_notify(struct acpi_device *acpi_dev, u32 event)
+static void system76_notify(acpi_handle handle, u32 event, void *context)
 {
-	struct system76_data *data;
+	struct system76_data *data = context;
 
-	data = acpi_driver_data(acpi_dev);
 	switch (event) {
 	case 0x80:
 		kb_led_hotkey_hardware(data);
@@ -777,6 +776,15 @@ static int system76_add(struct acpi_device *acpi_dev)
 		system76_battery_init();
 	}
 
+	err = acpi_dev_install_notify_handler(acpi_dev, ACPI_DEVICE_NOTIFY,
+					      system76_notify, data);
+	if (err) {
+		if (data->has_open_ec)
+			system76_battery_exit();
+
+		goto error;
+	}
+
 	return 0;
 
 error:
@@ -792,6 +800,8 @@ static void system76_remove(struct acpi_device *acpi_dev)
 {
 	struct system76_data *data;
 
+	acpi_dev_remove_notify_handler(acpi_dev, ACPI_DEVICE_NOTIFY, system76_notify);
+
 	data = acpi_driver_data(acpi_dev);
 
 	if (data->has_open_ec) {
@@ -813,7 +823,6 @@ static struct acpi_driver system76_driver = {
 	.ops = {
 		.add = system76_add,
 		.remove = system76_remove,
-		.notify = system76_notify,
 	},
 };
 module_acpi_driver(system76_driver);
-- 
2.51.0

[PATCH v1 2/2] platform/x86: system76: 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 System76 ACPI driver to a platform one.

After this change, the subordinate hwmon, input and LED class 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/system76_acpi.c | 52 +++++++++++++++-------------
 1 file changed, 27 insertions(+), 25 deletions(-)

diff --git a/drivers/platform/x86/system76_acpi.c b/drivers/platform/x86/system76_acpi.c
index 35e294a55fa3..b3a4254156ae 100644
--- a/drivers/platform/x86/system76_acpi.c
+++ b/drivers/platform/x86/system76_acpi.c
@@ -18,6 +18,7 @@
 #include <linux/leds.h>
 #include <linux/module.h>
 #include <linux/pci_ids.h>
+#include <linux/platform_device.h>
 #include <linux/power_supply.h>
 #include <linux/sysfs.h>
 #include <linux/types.h>
@@ -670,16 +671,19 @@ static void system76_notify(acpi_handle handle, u32 event, void *context)
 	}
 }
 
-// Add a System76 ACPI device
-static int system76_add(struct acpi_device *acpi_dev)
+// Probe a System76 platform device
+static int system76_probe(struct platform_device *pdev)
 {
+	struct acpi_device *acpi_dev = ACPI_COMPANION(&pdev->dev);
 	struct system76_data *data;
 	int err;
 
-	data = devm_kzalloc(&acpi_dev->dev, sizeof(*data), GFP_KERNEL);
+	data = devm_kzalloc(&pdev->dev, sizeof(*data), GFP_KERNEL);
 	if (!data)
 		return -ENOMEM;
-	acpi_dev->driver_data = data;
+
+	platform_set_drvdata(pdev, data);
+
 	data->acpi_dev = acpi_dev;
 
 	// Some models do not run open EC firmware. Check for an ACPI method
@@ -695,7 +699,7 @@ static int system76_add(struct acpi_device *acpi_dev)
 	data->ap_led.brightness_set_blocking = ap_led_set;
 	data->ap_led.max_brightness = 1;
 	data->ap_led.default_trigger = "rfkill-none";
-	err = devm_led_classdev_register(&acpi_dev->dev, &data->ap_led);
+	err = devm_led_classdev_register(&pdev->dev, &data->ap_led);
 	if (err)
 		return err;
 
@@ -739,19 +743,19 @@ static int system76_add(struct acpi_device *acpi_dev)
 	}
 
 	if (data->kbled_type != KBLED_NONE) {
-		err = devm_led_classdev_register(&acpi_dev->dev, &data->kb_led);
+		err = devm_led_classdev_register(&pdev->dev, &data->kb_led);
 		if (err)
 			return err;
 	}
 
-	data->input = devm_input_allocate_device(&acpi_dev->dev);
+	data->input = devm_input_allocate_device(&pdev->dev);
 	if (!data->input)
 		return -ENOMEM;
 
 	data->input->name = "System76 ACPI Hotkeys";
 	data->input->phys = "system76_acpi/input0";
 	data->input->id.bustype = BUS_HOST;
-	data->input->dev.parent = &acpi_dev->dev;
+	data->input->dev.parent = &pdev->dev;
 	input_set_capability(data->input, EV_KEY, KEY_SCREENLOCK);
 
 	err = input_register_device(data->input);
@@ -767,7 +771,7 @@ static int system76_add(struct acpi_device *acpi_dev)
 		if (err)
 			goto error;
 
-		data->therm = devm_hwmon_device_register_with_info(&acpi_dev->dev,
+		data->therm = devm_hwmon_device_register_with_info(&pdev->dev,
 			"system76_acpi", data, &thermal_chip_info, NULL);
 		err = PTR_ERR_OR_ZERO(data->therm);
 		if (err)
@@ -795,14 +799,13 @@ static int system76_add(struct acpi_device *acpi_dev)
 	return err;
 }
 
-// Remove a System76 ACPI device
-static void system76_remove(struct acpi_device *acpi_dev)
+// Remove a System76 platform device
+static void system76_remove(struct platform_device *pdev)
 {
-	struct system76_data *data;
-
-	acpi_dev_remove_notify_handler(acpi_dev, ACPI_DEVICE_NOTIFY, system76_notify);
+	struct system76_data *data = platform_get_drvdata(pdev);
 
-	data = acpi_driver_data(acpi_dev);
+	acpi_dev_remove_notify_handler(ACPI_COMPANION(&pdev->dev),
+				       ACPI_DEVICE_NOTIFY, system76_notify);
 
 	if (data->has_open_ec) {
 		system76_battery_exit();
@@ -810,22 +813,21 @@ static void system76_remove(struct acpi_device *acpi_dev)
 		kfree(data->ntmp);
 	}
 
-	devm_led_classdev_unregister(&acpi_dev->dev, &data->ap_led);
-	devm_led_classdev_unregister(&acpi_dev->dev, &data->kb_led);
+	devm_led_classdev_unregister(&pdev->dev, &data->ap_led);
+	devm_led_classdev_unregister(&pdev->dev, &data->kb_led);
 
 	system76_get(data, "FINI");
 }
 
-static struct acpi_driver system76_driver = {
-	.name = "System76 ACPI Driver",
-	.class = "hotkey",
-	.ids = device_ids,
-	.ops = {
-		.add = system76_add,
-		.remove = system76_remove,
+static struct platform_driver system76_driver = {
+	.probe = system76_probe,
+	.remove = system76_remove,
+	.driver = {
+		.name = "System76 ACPI Driver",
+		.acpi_match_table = device_ids,
 	},
 };
-module_acpi_driver(system76_driver);
+module_platform_driver(system76_driver);
 
 MODULE_DESCRIPTION("System76 ACPI Driver");
 MODULE_AUTHOR("Jeremy Soller <jeremy@system76.com>");
-- 
2.51.0

Re: [PATCH v1 2/2] platform/x86: system76: Convert ACPI driver to a platform one

[Jeremy Soller]

Hello Rafael,

This change looks good to me, is there anything else you need from me as the module author?

Thanks,

Jeremy

Re: [PATCH v1 2/2] platform/x86: system76: Convert ACPI driver to a platform one

[Rafael J. Wysocki]

Hi,

On Thu, Mar 12, 2026 at 5:09 PM Jeremy Soller <jeremy@system76.com> wrote:
>
> Hello Rafael,
>
> This change looks good to me, is there anything else you need from me as the module author?

No, I don't think so.

Thank you!

Re: [PATCH v1 2/2] platform/x86: system76: Convert ACPI driver to a platform one

[Ilpo Järvinen]

On Thu, 12 Mar 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 System76 ACPI driver to a platform one.
> 
> After this change, the subordinate hwmon, input and LED class 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/system76_acpi.c | 52 +++++++++++++++-------------
>  1 file changed, 27 insertions(+), 25 deletions(-)
> 
> diff --git a/drivers/platform/x86/system76_acpi.c b/drivers/platform/x86/system76_acpi.c
> index 35e294a55fa3..b3a4254156ae 100644
> --- a/drivers/platform/x86/system76_acpi.c
> +++ b/drivers/platform/x86/system76_acpi.c
> @@ -18,6 +18,7 @@
>  #include <linux/leds.h>
>  #include <linux/module.h>
>  #include <linux/pci_ids.h>
> +#include <linux/platform_device.h>
>  #include <linux/power_supply.h>
>  #include <linux/sysfs.h>
>  #include <linux/types.h>
> @@ -670,16 +671,19 @@ static void system76_notify(acpi_handle handle, u32 event, void *context)
>  	}
>  }
>  
> -// Add a System76 ACPI device
> -static int system76_add(struct acpi_device *acpi_dev)
> +// Probe a System76 platform device
> +static int system76_probe(struct platform_device *pdev)
>  {
> +	struct acpi_device *acpi_dev = ACPI_COMPANION(&pdev->dev);
>  	struct system76_data *data;
>  	int err;
>  
> -	data = devm_kzalloc(&acpi_dev->dev, sizeof(*data), GFP_KERNEL);
> +	data = devm_kzalloc(&pdev->dev, sizeof(*data), GFP_KERNEL);
>  	if (!data)
>  		return -ENOMEM;
> -	acpi_dev->driver_data = data;
> +
> +	platform_set_drvdata(pdev, data);
> +
>  	data->acpi_dev = acpi_dev;
>  
>  	// Some models do not run open EC firmware. Check for an ACPI method
> @@ -695,7 +699,7 @@ static int system76_add(struct acpi_device *acpi_dev)
>  	data->ap_led.brightness_set_blocking = ap_led_set;
>  	data->ap_led.max_brightness = 1;
>  	data->ap_led.default_trigger = "rfkill-none";
> -	err = devm_led_classdev_register(&acpi_dev->dev, &data->ap_led);
> +	err = devm_led_classdev_register(&pdev->dev, &data->ap_led);
>  	if (err)
>  		return err;
>  
> @@ -739,19 +743,19 @@ static int system76_add(struct acpi_device *acpi_dev)
>  	}
>  
>  	if (data->kbled_type != KBLED_NONE) {
> -		err = devm_led_classdev_register(&acpi_dev->dev, &data->kb_led);
> +		err = devm_led_classdev_register(&pdev->dev, &data->kb_led);
>  		if (err)
>  			return err;
>  	}
>  
> -	data->input = devm_input_allocate_device(&acpi_dev->dev);
> +	data->input = devm_input_allocate_device(&pdev->dev);
>  	if (!data->input)
>  		return -ENOMEM;
>  
>  	data->input->name = "System76 ACPI Hotkeys";
>  	data->input->phys = "system76_acpi/input0";
>  	data->input->id.bustype = BUS_HOST;
> -	data->input->dev.parent = &acpi_dev->dev;
> +	data->input->dev.parent = &pdev->dev;
>  	input_set_capability(data->input, EV_KEY, KEY_SCREENLOCK);
>  
>  	err = input_register_device(data->input);
> @@ -767,7 +771,7 @@ static int system76_add(struct acpi_device *acpi_dev)
>  		if (err)
>  			goto error;
>  
> -		data->therm = devm_hwmon_device_register_with_info(&acpi_dev->dev,
> +		data->therm = devm_hwmon_device_register_with_info(&pdev->dev,
>  			"system76_acpi", data, &thermal_chip_info, NULL);
>  		err = PTR_ERR_OR_ZERO(data->therm);
>  		if (err)
> @@ -795,14 +799,13 @@ static int system76_add(struct acpi_device *acpi_dev)
>  	return err;
>  }
>  
> -// Remove a System76 ACPI device
> -static void system76_remove(struct acpi_device *acpi_dev)
> +// Remove a System76 platform device
> +static void system76_remove(struct platform_device *pdev)
>  {
> -	struct system76_data *data;
> -
> -	acpi_dev_remove_notify_handler(acpi_dev, ACPI_DEVICE_NOTIFY, system76_notify);
> +	struct system76_data *data = platform_get_drvdata(pdev);
>  
> -	data = acpi_driver_data(acpi_dev);
> +	acpi_dev_remove_notify_handler(ACPI_COMPANION(&pdev->dev),
> +				       ACPI_DEVICE_NOTIFY, system76_notify);
>  
>  	if (data->has_open_ec) {
>  		system76_battery_exit();
> @@ -810,22 +813,21 @@ static void system76_remove(struct acpi_device *acpi_dev)
>  		kfree(data->ntmp);
>  	}
>  
> -	devm_led_classdev_unregister(&acpi_dev->dev, &data->ap_led);
> -	devm_led_classdev_unregister(&acpi_dev->dev, &data->kb_led);
> +	devm_led_classdev_unregister(&pdev->dev, &data->ap_led);
> +	devm_led_classdev_unregister(&pdev->dev, &data->kb_led);

With devm_* being used, why are these needed?

>  	system76_get(data, "FINI");
>  }
>  
> -static struct acpi_driver system76_driver = {
> -	.name = "System76 ACPI Driver",
> -	.class = "hotkey",
> -	.ids = device_ids,
> -	.ops = {
> -		.add = system76_add,
> -		.remove = system76_remove,
> +static struct platform_driver system76_driver = {
> +	.probe = system76_probe,
> +	.remove = system76_remove,
> +	.driver = {
> +		.name = "System76 ACPI Driver",
> +		.acpi_match_table = device_ids,
>  	},
>  };
> -module_acpi_driver(system76_driver);
> +module_platform_driver(system76_driver);
>  
>  MODULE_DESCRIPTION("System76 ACPI Driver");
>  MODULE_AUTHOR("Jeremy Soller <jeremy@system76.com>");
> 

-- 
 i.

Re: [PATCH v1 2/2] platform/x86: system76: Convert ACPI driver to a platform one

[Rafael J. Wysocki]

On Tue, Mar 17, 2026 at 6:48 PM Ilpo Järvinen
<ilpo.jarvinen@linux.intel.com> wrote:
>
> On Thu, 12 Mar 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 System76 ACPI driver to a platform one.
> >
> > After this change, the subordinate hwmon, input and LED class 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/system76_acpi.c | 52 +++++++++++++++-------------
> >  1 file changed, 27 insertions(+), 25 deletions(-)
> >
> > diff --git a/drivers/platform/x86/system76_acpi.c b/drivers/platform/x86/system76_acpi.c
> > index 35e294a55fa3..b3a4254156ae 100644
> > --- a/drivers/platform/x86/system76_acpi.c
> > +++ b/drivers/platform/x86/system76_acpi.c
> > @@ -18,6 +18,7 @@
> >  #include <linux/leds.h>
> >  #include <linux/module.h>
> >  #include <linux/pci_ids.h>
> > +#include <linux/platform_device.h>
> >  #include <linux/power_supply.h>
> >  #include <linux/sysfs.h>
> >  #include <linux/types.h>
> > @@ -670,16 +671,19 @@ static void system76_notify(acpi_handle handle, u32 event, void *context)
> >       }
> >  }
> >
> > -// Add a System76 ACPI device
> > -static int system76_add(struct acpi_device *acpi_dev)
> > +// Probe a System76 platform device
> > +static int system76_probe(struct platform_device *pdev)
> >  {
> > +     struct acpi_device *acpi_dev = ACPI_COMPANION(&pdev->dev);
> >       struct system76_data *data;
> >       int err;
> >
> > -     data = devm_kzalloc(&acpi_dev->dev, sizeof(*data), GFP_KERNEL);
> > +     data = devm_kzalloc(&pdev->dev, sizeof(*data), GFP_KERNEL);
> >       if (!data)
> >               return -ENOMEM;
> > -     acpi_dev->driver_data = data;
> > +
> > +     platform_set_drvdata(pdev, data);
> > +
> >       data->acpi_dev = acpi_dev;
> >
> >       // Some models do not run open EC firmware. Check for an ACPI method
> > @@ -695,7 +699,7 @@ static int system76_add(struct acpi_device *acpi_dev)
> >       data->ap_led.brightness_set_blocking = ap_led_set;
> >       data->ap_led.max_brightness = 1;
> >       data->ap_led.default_trigger = "rfkill-none";
> > -     err = devm_led_classdev_register(&acpi_dev->dev, &data->ap_led);
> > +     err = devm_led_classdev_register(&pdev->dev, &data->ap_led);
> >       if (err)
> >               return err;
> >
> > @@ -739,19 +743,19 @@ static int system76_add(struct acpi_device *acpi_dev)
> >       }
> >
> >       if (data->kbled_type != KBLED_NONE) {
> > -             err = devm_led_classdev_register(&acpi_dev->dev, &data->kb_led);
> > +             err = devm_led_classdev_register(&pdev->dev, &data->kb_led);
> >               if (err)
> >                       return err;
> >       }
> >
> > -     data->input = devm_input_allocate_device(&acpi_dev->dev);
> > +     data->input = devm_input_allocate_device(&pdev->dev);
> >       if (!data->input)
> >               return -ENOMEM;
> >
> >       data->input->name = "System76 ACPI Hotkeys";
> >       data->input->phys = "system76_acpi/input0";
> >       data->input->id.bustype = BUS_HOST;
> > -     data->input->dev.parent = &acpi_dev->dev;
> > +     data->input->dev.parent = &pdev->dev;
> >       input_set_capability(data->input, EV_KEY, KEY_SCREENLOCK);
> >
> >       err = input_register_device(data->input);
> > @@ -767,7 +771,7 @@ static int system76_add(struct acpi_device *acpi_dev)
> >               if (err)
> >                       goto error;
> >
> > -             data->therm = devm_hwmon_device_register_with_info(&acpi_dev->dev,
> > +             data->therm = devm_hwmon_device_register_with_info(&pdev->dev,
> >                       "system76_acpi", data, &thermal_chip_info, NULL);
> >               err = PTR_ERR_OR_ZERO(data->therm);
> >               if (err)
> > @@ -795,14 +799,13 @@ static int system76_add(struct acpi_device *acpi_dev)
> >       return err;
> >  }
> >
> > -// Remove a System76 ACPI device
> > -static void system76_remove(struct acpi_device *acpi_dev)
> > +// Remove a System76 platform device
> > +static void system76_remove(struct platform_device *pdev)
> >  {
> > -     struct system76_data *data;
> > -
> > -     acpi_dev_remove_notify_handler(acpi_dev, ACPI_DEVICE_NOTIFY, system76_notify);
> > +     struct system76_data *data = platform_get_drvdata(pdev);
> >
> > -     data = acpi_driver_data(acpi_dev);
> > +     acpi_dev_remove_notify_handler(ACPI_COMPANION(&pdev->dev),
> > +                                    ACPI_DEVICE_NOTIFY, system76_notify);
> >
> >       if (data->has_open_ec) {
> >               system76_battery_exit();
> > @@ -810,22 +813,21 @@ static void system76_remove(struct acpi_device *acpi_dev)
> >               kfree(data->ntmp);
> >       }
> >
> > -     devm_led_classdev_unregister(&acpi_dev->dev, &data->ap_led);
> > -     devm_led_classdev_unregister(&acpi_dev->dev, &data->kb_led);
> > +     devm_led_classdev_unregister(&pdev->dev, &data->ap_led);
> > +     devm_led_classdev_unregister(&pdev->dev, &data->kb_led);
>
> With devm_* being used, why are these needed?

I just preserved the existing pattern.  I can remove it, but that
would be a separate patch, wouldn't it?

> >       system76_get(data, "FINI");
> >  }
> >
> > -static struct acpi_driver system76_driver = {
> > -     .name = "System76 ACPI Driver",
> > -     .class = "hotkey",
> > -     .ids = device_ids,
> > -     .ops = {
> > -             .add = system76_add,
> > -             .remove = system76_remove,
> > +static struct platform_driver system76_driver = {
> > +     .probe = system76_probe,
> > +     .remove = system76_remove,
> > +     .driver = {
> > +             .name = "System76 ACPI Driver",
> > +             .acpi_match_table = device_ids,
> >       },
> >  };
> > -module_acpi_driver(system76_driver);
> > +module_platform_driver(system76_driver);
> >
> >  MODULE_DESCRIPTION("System76 ACPI Driver");
> >  MODULE_AUTHOR("Jeremy Soller <jeremy@system76.com>");
> >
>
> --
>  i.
>

Re: [PATCH v1 2/2] platform/x86: system76: Convert ACPI driver to a platform one

[Ilpo Järvinen]

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

On Tue, 17 Mar 2026, Rafael J. Wysocki wrote:

> On Tue, Mar 17, 2026 at 6:48 PM Ilpo Järvinen
> <ilpo.jarvinen@linux.intel.com> wrote:
> >
> > On Thu, 12 Mar 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 System76 ACPI driver to a platform one.
> > >
> > > After this change, the subordinate hwmon, input and LED class 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/system76_acpi.c | 52 +++++++++++++++-------------
> > >  1 file changed, 27 insertions(+), 25 deletions(-)
> > >
> > > diff --git a/drivers/platform/x86/system76_acpi.c b/drivers/platform/x86/system76_acpi.c
> > > index 35e294a55fa3..b3a4254156ae 100644
> > > --- a/drivers/platform/x86/system76_acpi.c
> > > +++ b/drivers/platform/x86/system76_acpi.c
> > > @@ -18,6 +18,7 @@
> > >  #include <linux/leds.h>
> > >  #include <linux/module.h>
> > >  #include <linux/pci_ids.h>
> > > +#include <linux/platform_device.h>
> > >  #include <linux/power_supply.h>
> > >  #include <linux/sysfs.h>
> > >  #include <linux/types.h>
> > > @@ -670,16 +671,19 @@ static void system76_notify(acpi_handle handle, u32 event, void *context)
> > >       }
> > >  }
> > >
> > > -// Add a System76 ACPI device
> > > -static int system76_add(struct acpi_device *acpi_dev)
> > > +// Probe a System76 platform device
> > > +static int system76_probe(struct platform_device *pdev)
> > >  {
> > > +     struct acpi_device *acpi_dev = ACPI_COMPANION(&pdev->dev);
> > >       struct system76_data *data;
> > >       int err;
> > >
> > > -     data = devm_kzalloc(&acpi_dev->dev, sizeof(*data), GFP_KERNEL);
> > > +     data = devm_kzalloc(&pdev->dev, sizeof(*data), GFP_KERNEL);
> > >       if (!data)
> > >               return -ENOMEM;
> > > -     acpi_dev->driver_data = data;
> > > +
> > > +     platform_set_drvdata(pdev, data);
> > > +
> > >       data->acpi_dev = acpi_dev;
> > >
> > >       // Some models do not run open EC firmware. Check for an ACPI method
> > > @@ -695,7 +699,7 @@ static int system76_add(struct acpi_device *acpi_dev)
> > >       data->ap_led.brightness_set_blocking = ap_led_set;
> > >       data->ap_led.max_brightness = 1;
> > >       data->ap_led.default_trigger = "rfkill-none";
> > > -     err = devm_led_classdev_register(&acpi_dev->dev, &data->ap_led);
> > > +     err = devm_led_classdev_register(&pdev->dev, &data->ap_led);
> > >       if (err)
> > >               return err;
> > >
> > > @@ -739,19 +743,19 @@ static int system76_add(struct acpi_device *acpi_dev)
> > >       }
> > >
> > >       if (data->kbled_type != KBLED_NONE) {
> > > -             err = devm_led_classdev_register(&acpi_dev->dev, &data->kb_led);
> > > +             err = devm_led_classdev_register(&pdev->dev, &data->kb_led);
> > >               if (err)
> > >                       return err;
> > >       }
> > >
> > > -     data->input = devm_input_allocate_device(&acpi_dev->dev);
> > > +     data->input = devm_input_allocate_device(&pdev->dev);
> > >       if (!data->input)
> > >               return -ENOMEM;
> > >
> > >       data->input->name = "System76 ACPI Hotkeys";
> > >       data->input->phys = "system76_acpi/input0";
> > >       data->input->id.bustype = BUS_HOST;
> > > -     data->input->dev.parent = &acpi_dev->dev;
> > > +     data->input->dev.parent = &pdev->dev;
> > >       input_set_capability(data->input, EV_KEY, KEY_SCREENLOCK);
> > >
> > >       err = input_register_device(data->input);
> > > @@ -767,7 +771,7 @@ static int system76_add(struct acpi_device *acpi_dev)
> > >               if (err)
> > >                       goto error;
> > >
> > > -             data->therm = devm_hwmon_device_register_with_info(&acpi_dev->dev,
> > > +             data->therm = devm_hwmon_device_register_with_info(&pdev->dev,
> > >                       "system76_acpi", data, &thermal_chip_info, NULL);
> > >               err = PTR_ERR_OR_ZERO(data->therm);
> > >               if (err)
> > > @@ -795,14 +799,13 @@ static int system76_add(struct acpi_device *acpi_dev)
> > >       return err;
> > >  }
> > >
> > > -// Remove a System76 ACPI device
> > > -static void system76_remove(struct acpi_device *acpi_dev)
> > > +// Remove a System76 platform device
> > > +static void system76_remove(struct platform_device *pdev)
> > >  {
> > > -     struct system76_data *data;
> > > -
> > > -     acpi_dev_remove_notify_handler(acpi_dev, ACPI_DEVICE_NOTIFY, system76_notify);
> > > +     struct system76_data *data = platform_get_drvdata(pdev);
> > >
> > > -     data = acpi_driver_data(acpi_dev);
> > > +     acpi_dev_remove_notify_handler(ACPI_COMPANION(&pdev->dev),
> > > +                                    ACPI_DEVICE_NOTIFY, system76_notify);
> > >
> > >       if (data->has_open_ec) {
> > >               system76_battery_exit();
> > > @@ -810,22 +813,21 @@ static void system76_remove(struct acpi_device *acpi_dev)
> > >               kfree(data->ntmp);
> > >       }
> > >
> > > -     devm_led_classdev_unregister(&acpi_dev->dev, &data->ap_led);
> > > -     devm_led_classdev_unregister(&acpi_dev->dev, &data->kb_led);
> > > +     devm_led_classdev_unregister(&pdev->dev, &data->ap_led);
> > > +     devm_led_classdev_unregister(&pdev->dev, &data->kb_led);
> >
> > With devm_* being used, why are these needed?
> 
> I just preserved the existing pattern.  I can remove it, but that
> would be a separate patch, wouldn't it?

Yes, to own patch.

-- 
 i.