[PATCH v1 0/4] platform/x86: xo15-ebook: Bind to a platform device instead of an ACPI one

[PATCH v1 0/4] platform/x86: xo15-ebook: 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 xo15-ebook
driver on top of 3 preliminary changes.

Patch [1/4] fixes the handling of a wakeup source and ACPI GPE in the
driver during probe and removal.

Patch [2/4] cleans up label formatting in ebook_switch_add().

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

Thanks!

[PATCH v1 1/4] platform/x86: xo15-ebook: Fix wakeup source and GPE handling

[Rafael J. Wysocki]

From: "Rafael J. Wysocki" <rafael.j.wysocki@intel.com>

The device_set_wakeup_enable() call in ebook_switch_add() doesn't
actually do anything because power.can_wakeup is not set for ACPI
device objects.  Moreover, had it done anything, it would have
registered a wakeup source object that wouldn't have been used
going forward and that wakeup source would have been leaked after
driver removal because ebook_switch_remove() doesn't clean it up.
Accordingly, remove that call from ebook_switch_add().

Also prevent leaking an enabled ACPI GPE after removing the driver by
adding appropriate cleanup code to ebook_switch_remove().

Fixes: 89ca11771a4b ("OLPC XO-1.5 ebook switch driver")
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/platform/x86/xo15-ebook.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/platform/x86/xo15-ebook.c b/drivers/platform/x86/xo15-ebook.c
index 4d1b1b310cc5..1568169b7872 100644
--- a/drivers/platform/x86/xo15-ebook.c
+++ b/drivers/platform/x86/xo15-ebook.c
@@ -38,6 +38,7 @@ MODULE_DEVICE_TABLE(acpi, ebook_device_ids);
 struct ebook_switch {
 	struct input_dev *input;
 	char phys[32];			/* for input device */
+	bool gpe_enabled;
 };
 
 static int ebook_send_state(struct acpi_device *device)
@@ -128,7 +129,7 @@ static int ebook_switch_add(struct acpi_device *device)
 		/* Button's GPE is run-wake GPE */
 		acpi_enable_gpe(device->wakeup.gpe_device,
 				device->wakeup.gpe_number);
-		device_set_wakeup_enable(&device->dev, true);
+		button->gpe_enabled = true;
 	}
 
 	return 0;
@@ -144,6 +145,10 @@ static void ebook_switch_remove(struct acpi_device *device)
 {
 	struct ebook_switch *button = acpi_driver_data(device);
 
+	if (button->gpe_enabled)
+		acpi_disable_gpe(device->wakeup.gpe_device,
+				 device->wakeup.gpe_number);
+
 	input_unregister_device(button->input);
 	kfree(button);
 }

[PATCH v1 2/4] platform/x86: xo15-ebook: Fix formatting of labels

[Rafael J. Wysocki]

From: "Rafael J. Wysocki" <rafael.j.wysocki@intel.com>

Fix formatting of two labels in ebook_switch_add() to make that
function follow the current kernel coding style more closely.

No functional impact.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/platform/x86/xo15-ebook.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/platform/x86/xo15-ebook.c b/drivers/platform/x86/xo15-ebook.c
index 1568169b7872..616f4bb3461a 100644
--- a/drivers/platform/x86/xo15-ebook.c
+++ b/drivers/platform/x86/xo15-ebook.c
@@ -134,9 +134,9 @@ static int ebook_switch_add(struct acpi_device *device)
 
 	return 0;
 
- err_free_input:
+err_free_input:
 	input_free_device(input);
- err_free_button:
+err_free_button:
 	kfree(button);
 	return error;
 }

[PATCH v1 3/4] platform/x86: xo15-ebook: 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/xo15-ebook.c | 19 ++++++++++++++-----
 1 file changed, 14 insertions(+), 5 deletions(-)

diff --git a/drivers/platform/x86/xo15-ebook.c b/drivers/platform/x86/xo15-ebook.c
index 616f4bb3461a..8af1b9078db8 100644
--- a/drivers/platform/x86/xo15-ebook.c
+++ b/drivers/platform/x86/xo15-ebook.c
@@ -57,16 +57,15 @@ static int ebook_send_state(struct acpi_device *device)
 	return 0;
 }
 
-static void ebook_switch_notify(struct acpi_device *device, u32 event)
+static void ebook_switch_notify(acpi_handle handle, u32 event, void *data)
 {
 	switch (event) {
 	case ACPI_FIXED_HARDWARE_EVENT:
 	case XO15_EBOOK_NOTIFY_STATUS:
-		ebook_send_state(device);
+		ebook_send_state(data);
 		break;
 	default:
-		acpi_handle_debug(device->handle,
-				  "Unsupported event [0x%x]\n", event);
+		acpi_handle_debug(handle, "Unsupported event [0x%x]\n", event);
 		break;
 	}
 }
@@ -123,6 +122,11 @@ static int ebook_switch_add(struct acpi_device *device)
 	if (error)
 		goto err_free_input;
 
+	error = acpi_dev_install_notify_handler(device, ACPI_DEVICE_NOTIFY,
+						ebook_switch_notify, device);
+	if (error)
+		goto err_unregister_input;
+
 	ebook_send_state(device);
 
 	if (device->wakeup.flags.valid) {
@@ -139,6 +143,10 @@ static int ebook_switch_add(struct acpi_device *device)
 err_free_button:
 	kfree(button);
 	return error;
+
+err_unregister_input:
+	input_unregister_device(input);
+	goto err_free_button;
 }
 
 static void ebook_switch_remove(struct acpi_device *device)
@@ -149,6 +157,8 @@ static void ebook_switch_remove(struct acpi_device *device)
 		acpi_disable_gpe(device->wakeup.gpe_device,
 				 device->wakeup.gpe_number);
 
+	acpi_dev_remove_notify_handler(device, ACPI_DEVICE_NOTIFY,
+				       ebook_switch_notify);
 	input_unregister_device(button->input);
 	kfree(button);
 }
@@ -160,7 +170,6 @@ static struct acpi_driver xo15_ebook_driver = {
 	.ops = {
 		.add = ebook_switch_add,
 		.remove = ebook_switch_remove,
-		.notify = ebook_switch_notify,
 	},
 	.drv.pm = &ebook_switch_pm,
 };

[PATCH v1 4/4] platform/x86: xo15-ebook: 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 OLPC XO-1.5 ebook switch driver from an
ACPI driver to a platform one.

After this change, the input device registered by the driver will appear
under the platform device used for driver binding.

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/xo15-ebook.c | 44 ++++++++++++++++---------------
 1 file changed, 23 insertions(+), 21 deletions(-)

diff --git a/drivers/platform/x86/xo15-ebook.c b/drivers/platform/x86/xo15-ebook.c
index 8af1b9078db8..2ca15f04b2d9 100644
--- a/drivers/platform/x86/xo15-ebook.c
+++ b/drivers/platform/x86/xo15-ebook.c
@@ -15,6 +15,7 @@
 #include <linux/types.h>
 #include <linux/input.h>
 #include <linux/acpi.h>
+#include <linux/platform_device.h>
 
 #define MODULE_NAME "xo15-ebook"
 
@@ -41,13 +42,13 @@ struct ebook_switch {
 	bool gpe_enabled;
 };
 
-static int ebook_send_state(struct acpi_device *device)
+static int ebook_send_state(struct device *dev)
 {
-	struct ebook_switch *button = acpi_driver_data(device);
+	struct ebook_switch *button = dev_get_drvdata(dev);
 	unsigned long long state;
 	acpi_status status;
 
-	status = acpi_evaluate_integer(device->handle, "EBK", NULL, &state);
+	status = acpi_evaluate_integer(ACPI_HANDLE(dev), "EBK", NULL, &state);
 	if (ACPI_FAILURE(status))
 		return -EIO;
 
@@ -73,14 +74,15 @@ static void ebook_switch_notify(acpi_handle handle, u32 event, void *data)
 #ifdef CONFIG_PM_SLEEP
 static int ebook_switch_resume(struct device *dev)
 {
-	return ebook_send_state(to_acpi_device(dev));
+	return ebook_send_state(dev);
 }
 #endif
 
 static SIMPLE_DEV_PM_OPS(ebook_switch_pm, NULL, ebook_switch_resume);
 
-static int ebook_switch_add(struct acpi_device *device)
+static int ebook_switch_probe(struct platform_device *pdev)
 {
+	struct acpi_device *device = ACPI_COMPANION(&pdev->dev);
 	const struct acpi_device_id *id;
 	struct ebook_switch *button;
 	struct input_dev *input;
@@ -90,7 +92,7 @@ static int ebook_switch_add(struct acpi_device *device)
 	if (!button)
 		return -ENOMEM;
 
-	device->driver_data = button;
+	platform_set_drvdata(pdev, button);
 
 	button->input = input = input_allocate_device();
 	if (!input) {
@@ -100,7 +102,7 @@ static int ebook_switch_add(struct acpi_device *device)
 
 	id = acpi_match_acpi_device(ebook_device_ids, device);
 	if (!id) {
-		dev_err(&device->dev, "Unsupported hid\n");
+		dev_err(&pdev->dev, "Unsupported hid\n");
 		error = -ENODEV;
 		goto err_free_input;
 	}
@@ -113,7 +115,7 @@ static int ebook_switch_add(struct acpi_device *device)
 	input->name = acpi_device_name(device);
 	input->phys = button->phys;
 	input->id.bustype = BUS_HOST;
-	input->dev.parent = &device->dev;
+	input->dev.parent = &pdev->dev;
 
 	input->evbit[0] = BIT_MASK(EV_SW);
 	set_bit(SW_TABLET_MODE, input->swbit);
@@ -123,11 +125,11 @@ static int ebook_switch_add(struct acpi_device *device)
 		goto err_free_input;
 
 	error = acpi_dev_install_notify_handler(device, ACPI_DEVICE_NOTIFY,
-						ebook_switch_notify, device);
+						ebook_switch_notify, &pdev->dev);
 	if (error)
 		goto err_unregister_input;
 
-	ebook_send_state(device);
+	ebook_send_state(&pdev->dev);
 
 	if (device->wakeup.flags.valid) {
 		/* Button's GPE is run-wake GPE */
@@ -149,9 +151,10 @@ static int ebook_switch_add(struct acpi_device *device)
 	goto err_free_button;
 }
 
-static void ebook_switch_remove(struct acpi_device *device)
+static void ebook_switch_remove(struct platform_device *pdev)
 {
-	struct ebook_switch *button = acpi_driver_data(device);
+	struct ebook_switch *button = platform_get_drvdata(pdev);
+	struct acpi_device *device = ACPI_COMPANION(&pdev->dev);
 
 	if (button->gpe_enabled)
 		acpi_disable_gpe(device->wakeup.gpe_device,
@@ -163,14 +166,13 @@ static void ebook_switch_remove(struct acpi_device *device)
 	kfree(button);
 }
 
-static struct acpi_driver xo15_ebook_driver = {
-	.name = MODULE_NAME,
-	.class = XO15_EBOOK_CLASS,
-	.ids = ebook_device_ids,
-	.ops = {
-		.add = ebook_switch_add,
-		.remove = ebook_switch_remove,
+static struct platform_driver xo15_ebook_driver = {
+	.probe = ebook_switch_probe,
+	.remove = ebook_switch_remove,
+	.driver = {
+		.name = MODULE_NAME,
+		.acpi_match_table = ebook_device_ids,
+		.pm = &ebook_switch_pm,
 	},
-	.drv.pm = &ebook_switch_pm,
 };
-module_acpi_driver(xo15_ebook_driver);
+module_platform_driver(xo15_ebook_driver);

Re: [PATCH v1 3/4] platform/x86: xo15-ebook: Register ACPI notify handler directly

[Ilpo Järvinen]

On Fri, 8 May 2026, Rafael J. Wysocki wrote:

> 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/xo15-ebook.c | 19 ++++++++++++++-----
>  1 file changed, 14 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/platform/x86/xo15-ebook.c b/drivers/platform/x86/xo15-ebook.c
> index 616f4bb3461a..8af1b9078db8 100644
> --- a/drivers/platform/x86/xo15-ebook.c
> +++ b/drivers/platform/x86/xo15-ebook.c
> @@ -57,16 +57,15 @@ static int ebook_send_state(struct acpi_device *device)
>  	return 0;
>  }
>  
> -static void ebook_switch_notify(struct acpi_device *device, u32 event)
> +static void ebook_switch_notify(acpi_handle handle, u32 event, void *data)
>  {
>  	switch (event) {
>  	case ACPI_FIXED_HARDWARE_EVENT:
>  	case XO15_EBOOK_NOTIFY_STATUS:
> -		ebook_send_state(device);
> +		ebook_send_state(data);
>  		break;
>  	default:
> -		acpi_handle_debug(device->handle,
> -				  "Unsupported event [0x%x]\n", event);
> +		acpi_handle_debug(handle, "Unsupported event [0x%x]\n", event);
>  		break;
>  	}
>  }
> @@ -123,6 +122,11 @@ static int ebook_switch_add(struct acpi_device *device)
>  	if (error)
>  		goto err_free_input;
>  
> +	error = acpi_dev_install_notify_handler(device, ACPI_DEVICE_NOTIFY,
> +						ebook_switch_notify, device);
> +	if (error)
> +		goto err_unregister_input;
> +
>  	ebook_send_state(device);
>  
>  	if (device->wakeup.flags.valid) {
> @@ -139,6 +143,10 @@ static int ebook_switch_add(struct acpi_device *device)
>  err_free_button:
>  	kfree(button);
>  	return error;
> +
> +err_unregister_input:
> +	input_unregister_device(input);
> +	goto err_free_button;

The end result would be much simpler if there would be patch 5 to convert 
to devm_*().

>  }
>  
>  static void ebook_switch_remove(struct acpi_device *device)
> @@ -149,6 +157,8 @@ static void ebook_switch_remove(struct acpi_device *device)
>  		acpi_disable_gpe(device->wakeup.gpe_device,
>  				 device->wakeup.gpe_number);
>  
> +	acpi_dev_remove_notify_handler(device, ACPI_DEVICE_NOTIFY,
> +				       ebook_switch_notify);
>  	input_unregister_device(button->input);
>  	kfree(button);
>  }
> @@ -160,7 +170,6 @@ static struct acpi_driver xo15_ebook_driver = {
>  	.ops = {
>  		.add = ebook_switch_add,
>  		.remove = ebook_switch_remove,
> -		.notify = ebook_switch_notify,
>  	},
>  	.drv.pm = &ebook_switch_pm,
>  };
> 
> 
> 

-- 
 i.

Re: [PATCH v1 3/4] platform/x86: xo15-ebook: Register ACPI notify handler directly

[Rafael J. Wysocki]

On Mon, May 11, 2026 at 3:59 PM Ilpo Järvinen
<ilpo.jarvinen@linux.intel.com> wrote:
>
> On Fri, 8 May 2026, Rafael J. Wysocki wrote:
>
> > 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/xo15-ebook.c | 19 ++++++++++++++-----
> >  1 file changed, 14 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/platform/x86/xo15-ebook.c b/drivers/platform/x86/xo15-ebook.c
> > index 616f4bb3461a..8af1b9078db8 100644
> > --- a/drivers/platform/x86/xo15-ebook.c
> > +++ b/drivers/platform/x86/xo15-ebook.c
> > @@ -57,16 +57,15 @@ static int ebook_send_state(struct acpi_device *device)
> >       return 0;
> >  }
> >
> > -static void ebook_switch_notify(struct acpi_device *device, u32 event)
> > +static void ebook_switch_notify(acpi_handle handle, u32 event, void *data)
> >  {
> >       switch (event) {
> >       case ACPI_FIXED_HARDWARE_EVENT:
> >       case XO15_EBOOK_NOTIFY_STATUS:
> > -             ebook_send_state(device);
> > +             ebook_send_state(data);
> >               break;
> >       default:
> > -             acpi_handle_debug(device->handle,
> > -                               "Unsupported event [0x%x]\n", event);
> > +             acpi_handle_debug(handle, "Unsupported event [0x%x]\n", event);
> >               break;
> >       }
> >  }
> > @@ -123,6 +122,11 @@ static int ebook_switch_add(struct acpi_device *device)
> >       if (error)
> >               goto err_free_input;
> >
> > +     error = acpi_dev_install_notify_handler(device, ACPI_DEVICE_NOTIFY,
> > +                                             ebook_switch_notify, device);
> > +     if (error)
> > +             goto err_unregister_input;
> > +
> >       ebook_send_state(device);
> >
> >       if (device->wakeup.flags.valid) {
> > @@ -139,6 +143,10 @@ static int ebook_switch_add(struct acpi_device *device)
> >  err_free_button:
> >       kfree(button);
> >       return error;
> > +
> > +err_unregister_input:
> > +     input_unregister_device(input);
> > +     goto err_free_button;
>
> The end result would be much simpler if there would be patch 5 to convert
> to devm_*().

Well, I can add one.

Do you want me to resend an update of the series for this?

> >  }
> >
> >  static void ebook_switch_remove(struct acpi_device *device)
> > @@ -149,6 +157,8 @@ static void ebook_switch_remove(struct acpi_device *device)
> >               acpi_disable_gpe(device->wakeup.gpe_device,
> >                                device->wakeup.gpe_number);
> >
> > +     acpi_dev_remove_notify_handler(device, ACPI_DEVICE_NOTIFY,
> > +                                    ebook_switch_notify);
> >       input_unregister_device(button->input);
> >       kfree(button);
> >  }
> > @@ -160,7 +170,6 @@ static struct acpi_driver xo15_ebook_driver = {
> >       .ops = {
> >               .add = ebook_switch_add,
> >               .remove = ebook_switch_remove,
> > -             .notify = ebook_switch_notify,
> >       },
> >       .drv.pm = &ebook_switch_pm,
> >  };
> >
> >
> >
>
> --

Re: [PATCH v1 3/4] platform/x86: xo15-ebook: Register ACPI notify handler directly

[Ilpo Järvinen]

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

On Mon, 11 May 2026, Rafael J. Wysocki wrote:

> On Mon, May 11, 2026 at 3:59 PM Ilpo Järvinen
> <ilpo.jarvinen@linux.intel.com> wrote:
> >
> > On Fri, 8 May 2026, Rafael J. Wysocki wrote:
> >
> > > 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/xo15-ebook.c | 19 ++++++++++++++-----
> > >  1 file changed, 14 insertions(+), 5 deletions(-)
> > >
> > > diff --git a/drivers/platform/x86/xo15-ebook.c b/drivers/platform/x86/xo15-ebook.c
> > > index 616f4bb3461a..8af1b9078db8 100644
> > > --- a/drivers/platform/x86/xo15-ebook.c
> > > +++ b/drivers/platform/x86/xo15-ebook.c
> > > @@ -57,16 +57,15 @@ static int ebook_send_state(struct acpi_device *device)
> > >       return 0;
> > >  }
> > >
> > > -static void ebook_switch_notify(struct acpi_device *device, u32 event)
> > > +static void ebook_switch_notify(acpi_handle handle, u32 event, void *data)
> > >  {
> > >       switch (event) {
> > >       case ACPI_FIXED_HARDWARE_EVENT:
> > >       case XO15_EBOOK_NOTIFY_STATUS:
> > > -             ebook_send_state(device);
> > > +             ebook_send_state(data);
> > >               break;
> > >       default:
> > > -             acpi_handle_debug(device->handle,
> > > -                               "Unsupported event [0x%x]\n", event);
> > > +             acpi_handle_debug(handle, "Unsupported event [0x%x]\n", event);
> > >               break;
> > >       }
> > >  }
> > > @@ -123,6 +122,11 @@ static int ebook_switch_add(struct acpi_device *device)
> > >       if (error)
> > >               goto err_free_input;
> > >
> > > +     error = acpi_dev_install_notify_handler(device, ACPI_DEVICE_NOTIFY,
> > > +                                             ebook_switch_notify, device);
> > > +     if (error)
> > > +             goto err_unregister_input;
> > > +
> > >       ebook_send_state(device);
> > >
> > >       if (device->wakeup.flags.valid) {
> > > @@ -139,6 +143,10 @@ static int ebook_switch_add(struct acpi_device *device)
> > >  err_free_button:
> > >       kfree(button);
> > >       return error;
> > > +
> > > +err_unregister_input:
> > > +     input_unregister_device(input);
> > > +     goto err_free_button;
> >
> > The end result would be much simpler if there would be patch 5 to convert
> > to devm_*().
> 
> Well, I can add one.
> 
> Do you want me to resend an update of the series for this?

Ah, right. I took these now to review-ilpo-next.

(I was initially thinking of making a little bit different suggestion 
which would have required changes to these patches but then realized it 
was better to just add the devm_*() conversion after these).

-- 
 i.

Re: [PATCH v1 3/4] platform/x86: xo15-ebook: Register ACPI notify handler directly

[Rafael J. Wysocki]

On Mon, May 11, 2026 at 6:17 PM Ilpo Järvinen
<ilpo.jarvinen@linux.intel.com> wrote:
>
> On Mon, 11 May 2026, Rafael J. Wysocki wrote:
>
> > On Mon, May 11, 2026 at 3:59 PM Ilpo Järvinen
> > <ilpo.jarvinen@linux.intel.com> wrote:
> > >
> > > On Fri, 8 May 2026, Rafael J. Wysocki wrote:
> > >
> > > > 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/xo15-ebook.c | 19 ++++++++++++++-----
> > > >  1 file changed, 14 insertions(+), 5 deletions(-)
> > > >
> > > > diff --git a/drivers/platform/x86/xo15-ebook.c b/drivers/platform/x86/xo15-ebook.c
> > > > index 616f4bb3461a..8af1b9078db8 100644
> > > > --- a/drivers/platform/x86/xo15-ebook.c
> > > > +++ b/drivers/platform/x86/xo15-ebook.c
> > > > @@ -57,16 +57,15 @@ static int ebook_send_state(struct acpi_device *device)
> > > >       return 0;
> > > >  }
> > > >
> > > > -static void ebook_switch_notify(struct acpi_device *device, u32 event)
> > > > +static void ebook_switch_notify(acpi_handle handle, u32 event, void *data)
> > > >  {
> > > >       switch (event) {
> > > >       case ACPI_FIXED_HARDWARE_EVENT:
> > > >       case XO15_EBOOK_NOTIFY_STATUS:
> > > > -             ebook_send_state(device);
> > > > +             ebook_send_state(data);
> > > >               break;
> > > >       default:
> > > > -             acpi_handle_debug(device->handle,
> > > > -                               "Unsupported event [0x%x]\n", event);
> > > > +             acpi_handle_debug(handle, "Unsupported event [0x%x]\n", event);
> > > >               break;
> > > >       }
> > > >  }
> > > > @@ -123,6 +122,11 @@ static int ebook_switch_add(struct acpi_device *device)
> > > >       if (error)
> > > >               goto err_free_input;
> > > >
> > > > +     error = acpi_dev_install_notify_handler(device, ACPI_DEVICE_NOTIFY,
> > > > +                                             ebook_switch_notify, device);
> > > > +     if (error)
> > > > +             goto err_unregister_input;
> > > > +
> > > >       ebook_send_state(device);
> > > >
> > > >       if (device->wakeup.flags.valid) {
> > > > @@ -139,6 +143,10 @@ static int ebook_switch_add(struct acpi_device *device)
> > > >  err_free_button:
> > > >       kfree(button);
> > > >       return error;
> > > > +
> > > > +err_unregister_input:
> > > > +     input_unregister_device(input);
> > > > +     goto err_free_button;
> > >
> > > The end result would be much simpler if there would be patch 5 to convert
> > > to devm_*().
> >
> > Well, I can add one.
> >
> > Do you want me to resend an update of the series for this?
>
> Ah, right. I took these now to review-ilpo-next.
>
> (I was initially thinking of making a little bit different suggestion
> which would have required changes to these patches but then realized it
> was better to just add the devm_*() conversion after these).

So I guess I'll prepare a follow-up patch for the devm_ conversion.

Note though that there's no devm_ counterpart of
acpi_dev_install_notify_handler() ATM, so I may need to add one while
at it.

Re: [PATCH v1 3/4] platform/x86: xo15-ebook: Register ACPI notify handler directly

[Ilpo Järvinen]

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

On Mon, 11 May 2026, Rafael J. Wysocki wrote:

> On Mon, May 11, 2026 at 6:17 PM Ilpo Järvinen
> <ilpo.jarvinen@linux.intel.com> wrote:
> >
> > On Mon, 11 May 2026, Rafael J. Wysocki wrote:
> >
> > > On Mon, May 11, 2026 at 3:59 PM Ilpo Järvinen
> > > <ilpo.jarvinen@linux.intel.com> wrote:
> > > >
> > > > On Fri, 8 May 2026, Rafael J. Wysocki wrote:
> > > >
> > > > > 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/xo15-ebook.c | 19 ++++++++++++++-----
> > > > >  1 file changed, 14 insertions(+), 5 deletions(-)
> > > > >
> > > > > diff --git a/drivers/platform/x86/xo15-ebook.c b/drivers/platform/x86/xo15-ebook.c
> > > > > index 616f4bb3461a..8af1b9078db8 100644
> > > > > --- a/drivers/platform/x86/xo15-ebook.c
> > > > > +++ b/drivers/platform/x86/xo15-ebook.c
> > > > > @@ -57,16 +57,15 @@ static int ebook_send_state(struct acpi_device *device)
> > > > >       return 0;
> > > > >  }
> > > > >
> > > > > -static void ebook_switch_notify(struct acpi_device *device, u32 event)
> > > > > +static void ebook_switch_notify(acpi_handle handle, u32 event, void *data)
> > > > >  {
> > > > >       switch (event) {
> > > > >       case ACPI_FIXED_HARDWARE_EVENT:
> > > > >       case XO15_EBOOK_NOTIFY_STATUS:
> > > > > -             ebook_send_state(device);
> > > > > +             ebook_send_state(data);
> > > > >               break;
> > > > >       default:
> > > > > -             acpi_handle_debug(device->handle,
> > > > > -                               "Unsupported event [0x%x]\n", event);
> > > > > +             acpi_handle_debug(handle, "Unsupported event [0x%x]\n", event);
> > > > >               break;
> > > > >       }
> > > > >  }
> > > > > @@ -123,6 +122,11 @@ static int ebook_switch_add(struct acpi_device *device)
> > > > >       if (error)
> > > > >               goto err_free_input;
> > > > >
> > > > > +     error = acpi_dev_install_notify_handler(device, ACPI_DEVICE_NOTIFY,
> > > > > +                                             ebook_switch_notify, device);
> > > > > +     if (error)
> > > > > +             goto err_unregister_input;
> > > > > +
> > > > >       ebook_send_state(device);
> > > > >
> > > > >       if (device->wakeup.flags.valid) {
> > > > > @@ -139,6 +143,10 @@ static int ebook_switch_add(struct acpi_device *device)
> > > > >  err_free_button:
> > > > >       kfree(button);
> > > > >       return error;
> > > > > +
> > > > > +err_unregister_input:
> > > > > +     input_unregister_device(input);
> > > > > +     goto err_free_button;
> > > >
> > > > The end result would be much simpler if there would be patch 5 to convert
> > > > to devm_*().
> > >
> > > Well, I can add one.
> > >
> > > Do you want me to resend an update of the series for this?
> >
> > Ah, right. I took these now to review-ilpo-next.
> >
> > (I was initially thinking of making a little bit different suggestion
> > which would have required changes to these patches but then realized it
> > was better to just add the devm_*() conversion after these).
> 
> So I guess I'll prepare a follow-up patch for the devm_ conversion.
> 
> Note though that there's no devm_ counterpart of
> acpi_dev_install_notify_handler() ATM, so I may need to add one while
> at it.

While adding it will be able to remove acpi_dev_remove_notify_handler() 
call from ebook_switch_remove(), there's no such call in rollbacks for 
ebook_switch_probe() so it wouldn't be strictly required.

But it certainly wouldn't hurt to have one for other drivers.

-- 
 i.

Re: [PATCH v1 3/4] platform/x86: xo15-ebook: Register ACPI notify handler directly

[Rafael J. Wysocki]

On Mon, May 11, 2026 at 6:44 PM Ilpo Järvinen
<ilpo.jarvinen@linux.intel.com> wrote:
>
> On Mon, 11 May 2026, Rafael J. Wysocki wrote:
>
> > On Mon, May 11, 2026 at 6:17 PM Ilpo Järvinen
> > <ilpo.jarvinen@linux.intel.com> wrote:
> > >
> > > On Mon, 11 May 2026, Rafael J. Wysocki wrote:
> > >
> > > > On Mon, May 11, 2026 at 3:59 PM Ilpo Järvinen
> > > > <ilpo.jarvinen@linux.intel.com> wrote:
> > > > >
> > > > > On Fri, 8 May 2026, Rafael J. Wysocki wrote:
> > > > >
> > > > > > 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/xo15-ebook.c | 19 ++++++++++++++-----
> > > > > >  1 file changed, 14 insertions(+), 5 deletions(-)
> > > > > >
> > > > > > diff --git a/drivers/platform/x86/xo15-ebook.c b/drivers/platform/x86/xo15-ebook.c
> > > > > > index 616f4bb3461a..8af1b9078db8 100644
> > > > > > --- a/drivers/platform/x86/xo15-ebook.c
> > > > > > +++ b/drivers/platform/x86/xo15-ebook.c
> > > > > > @@ -57,16 +57,15 @@ static int ebook_send_state(struct acpi_device *device)
> > > > > >       return 0;
> > > > > >  }
> > > > > >
> > > > > > -static void ebook_switch_notify(struct acpi_device *device, u32 event)
> > > > > > +static void ebook_switch_notify(acpi_handle handle, u32 event, void *data)
> > > > > >  {
> > > > > >       switch (event) {
> > > > > >       case ACPI_FIXED_HARDWARE_EVENT:
> > > > > >       case XO15_EBOOK_NOTIFY_STATUS:
> > > > > > -             ebook_send_state(device);
> > > > > > +             ebook_send_state(data);
> > > > > >               break;
> > > > > >       default:
> > > > > > -             acpi_handle_debug(device->handle,
> > > > > > -                               "Unsupported event [0x%x]\n", event);
> > > > > > +             acpi_handle_debug(handle, "Unsupported event [0x%x]\n", event);
> > > > > >               break;
> > > > > >       }
> > > > > >  }
> > > > > > @@ -123,6 +122,11 @@ static int ebook_switch_add(struct acpi_device *device)
> > > > > >       if (error)
> > > > > >               goto err_free_input;
> > > > > >
> > > > > > +     error = acpi_dev_install_notify_handler(device, ACPI_DEVICE_NOTIFY,
> > > > > > +                                             ebook_switch_notify, device);
> > > > > > +     if (error)
> > > > > > +             goto err_unregister_input;
> > > > > > +
> > > > > >       ebook_send_state(device);
> > > > > >
> > > > > >       if (device->wakeup.flags.valid) {
> > > > > > @@ -139,6 +143,10 @@ static int ebook_switch_add(struct acpi_device *device)
> > > > > >  err_free_button:
> > > > > >       kfree(button);
> > > > > >       return error;
> > > > > > +
> > > > > > +err_unregister_input:
> > > > > > +     input_unregister_device(input);
> > > > > > +     goto err_free_button;
> > > > >
> > > > > The end result would be much simpler if there would be patch 5 to convert
> > > > > to devm_*().
> > > >
> > > > Well, I can add one.
> > > >
> > > > Do you want me to resend an update of the series for this?
> > >
> > > Ah, right. I took these now to review-ilpo-next.
> > >
> > > (I was initially thinking of making a little bit different suggestion
> > > which would have required changes to these patches but then realized it
> > > was better to just add the devm_*() conversion after these).
> >
> > So I guess I'll prepare a follow-up patch for the devm_ conversion.
> >
> > Note though that there's no devm_ counterpart of
> > acpi_dev_install_notify_handler() ATM, so I may need to add one while
> > at it.
>
> While adding it will be able to remove acpi_dev_remove_notify_handler()
> call from ebook_switch_remove(), there's no such call in rollbacks for
> ebook_switch_probe() so it wouldn't be strictly required.

Right.

> But it certainly wouldn't hurt to have one for other drivers.

Sure, I definitely have a plan to add one.