On Tue, 17 Mar 2026, Rafael J. Wysocki wrote: > On Tue, Mar 17, 2026 at 6:48 PM Ilpo Järvinen > wrote: > > > > On Thu, 12 Mar 2026, Rafael J. Wysocki wrote: > > > > > From: "Rafael J. Wysocki" > > > > > > 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 > > > --- > > > 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 > > > #include > > > #include > > > +#include > > > #include > > > #include > > > #include > > > @@ -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.