[PATCH v1 0/3] platform/x86: panasonic-laptop: Bind to a platform device instead of an ACPI one

[PATCH v1 0/3] platform/x86: panasonic-laptop: 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 Panasonic
hotkey and LCD brightness control driver.

Patch [1/3] removes some redundant checks from the driver.

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

Thanks!

[PATCH v1 1/3] platform/x86: panasonic-laptop: Remove redundant checks from 3 functions

[Rafael J. Wysocki]

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

The device pointer cannot be NULL in acpi_pcc_hotkey_add() and
acpi_pcc_hotkey_remove() because these functions are ACPI driver
callbacks and NULL is never passed to any of them as an argument.

Likewise, acpi_pcc_hotkey_resume() is a resume callback of a
device driver and NULL is never passed to it as an argument, so
the dev pointer in it cannot be NULL.

Moreover, since acpi_pcc_hotkey_remove() and acpi_pcc_hotkey_resume()
can only run after acpi_pcc_hotkey_add() has completed successfully,
the acpi_driver_data() of the device object used by them cannot be
NULL when they run.

Drop all of the redundant NULL checks of the pointers mentioned above.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/platform/x86/panasonic-laptop.c | 15 +--------------
 1 file changed, 1 insertion(+), 14 deletions(-)

diff --git a/drivers/platform/x86/panasonic-laptop.c b/drivers/platform/x86/panasonic-laptop.c
index d923ddaa4849..64195ff0f40e 100644
--- a/drivers/platform/x86/panasonic-laptop.c
+++ b/drivers/platform/x86/panasonic-laptop.c
@@ -968,14 +968,7 @@ static int acpi_pcc_init_input(struct pcc_acpi *pcc)
 #ifdef CONFIG_PM_SLEEP
 static int acpi_pcc_hotkey_resume(struct device *dev)
 {
-	struct pcc_acpi *pcc;
-
-	if (!dev)
-		return -EINVAL;
-
-	pcc = acpi_driver_data(to_acpi_device(dev));
-	if (!pcc)
-		return -EINVAL;
+	struct pcc_acpi *pcc = acpi_driver_data(to_acpi_device(dev));
 
 	if (pcc->num_sifr > SINF_MUTE)
 		acpi_pcc_write_sset(pcc, SINF_MUTE, pcc->mute);
@@ -997,9 +990,6 @@ static int acpi_pcc_hotkey_add(struct acpi_device *device)
 	struct pcc_acpi *pcc;
 	int num_sifr, result;
 
-	if (!device)
-		return -EINVAL;
-
 	num_sifr = acpi_pcc_get_sqty(device);
 
 	/*
@@ -1123,9 +1113,6 @@ static void acpi_pcc_hotkey_remove(struct acpi_device *device)
 {
 	struct pcc_acpi *pcc = acpi_driver_data(device);
 
-	if (!device || !pcc)
-		return;
-
 	i8042_remove_filter(panasonic_i8042_filter);
 
 	if (pcc->platform) {
-- 
2.51.0

[PATCH v1 2/3] platform/x86: panasonic-laptop: 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/panasonic-laptop.c | 20 +++++++++++++++-----
 1 file changed, 15 insertions(+), 5 deletions(-)

diff --git a/drivers/platform/x86/panasonic-laptop.c b/drivers/platform/x86/panasonic-laptop.c
index 64195ff0f40e..4bd83b987761 100644
--- a/drivers/platform/x86/panasonic-laptop.c
+++ b/drivers/platform/x86/panasonic-laptop.c
@@ -185,7 +185,7 @@ enum SINF_BITS { SINF_NUM_BATTERIES = 0,
 
 static int acpi_pcc_hotkey_add(struct acpi_device *device);
 static void acpi_pcc_hotkey_remove(struct acpi_device *device);
-static void acpi_pcc_hotkey_notify(struct acpi_device *device, u32 event);
+static void acpi_pcc_hotkey_notify(acpi_handle handle, u32 event, void *data);
 
 static const struct acpi_device_id pcc_device_ids[] = {
 	{ "MAT0012", 0},
@@ -208,7 +208,6 @@ static struct acpi_driver acpi_pcc_driver = {
 	.ops =		{
 				.add =		acpi_pcc_hotkey_add,
 				.remove =	acpi_pcc_hotkey_remove,
-				.notify =	acpi_pcc_hotkey_notify,
 			},
 	.drv.pm =	&acpi_pcc_hotkey_pm,
 };
@@ -869,9 +868,9 @@ static void acpi_pcc_generate_keyinput(struct pcc_acpi *pcc)
 		pr_err("Unknown hotkey event: 0x%04llx\n", result);
 }
 
-static void acpi_pcc_hotkey_notify(struct acpi_device *device, u32 event)
+static void acpi_pcc_hotkey_notify(acpi_handle handle, u32 event, void *data)
 {
-	struct pcc_acpi *pcc = acpi_driver_data(device);
+	struct pcc_acpi *pcc = data;
 
 	switch (event) {
 	case HKEY_NOTIFY:
@@ -1073,13 +1072,18 @@ static int acpi_pcc_hotkey_add(struct acpi_device *device)
 	if (result)
 		goto out_backlight;
 
+	result = acpi_dev_install_notify_handler(device, ACPI_DEVICE_NOTIFY,
+						 acpi_pcc_hotkey_notify, pcc);
+	if (result)
+		goto out_sysfs;
+
 	/* optical drive initialization */
 	if (ACPI_SUCCESS(check_optd_present())) {
 		pcc->platform = platform_device_register_simple("panasonic",
 			PLATFORM_DEVID_NONE, NULL, 0);
 		if (IS_ERR(pcc->platform)) {
 			result = PTR_ERR(pcc->platform);
-			goto out_sysfs;
+			goto out_notify_handler;
 		}
 		result = device_create_file(&pcc->platform->dev,
 			&dev_attr_cdpower);
@@ -1095,6 +1099,9 @@ static int acpi_pcc_hotkey_add(struct acpi_device *device)
 
 out_platform:
 	platform_device_unregister(pcc->platform);
+out_notify_handler:
+	acpi_dev_remove_notify_handler(device, ACPI_DEVICE_NOTIFY,
+				       acpi_pcc_hotkey_notify);
 out_sysfs:
 	sysfs_remove_group(&device->dev.kobj, &pcc_attr_group);
 out_backlight:
@@ -1123,6 +1130,9 @@ static void acpi_pcc_hotkey_remove(struct acpi_device *device)
 
 	sysfs_remove_group(&device->dev.kobj, &pcc_attr_group);
 
+	acpi_dev_remove_notify_handler(device, ACPI_DEVICE_NOTIFY,
+				       acpi_pcc_hotkey_notify);
+
 	backlight_device_unregister(pcc->backlight);
 
 	input_unregister_device(pcc->input_dev);
-- 
2.51.0

[PATCH v1 3/3] platform/x86: panasonic-laptop: 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 Panasonic laptop ACPI driver to a platform
one.

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

To maintain backwards compatibility with possibly existing user space,
the sysfs attributes created by the driver under the ACPI device object
used by it are not relocated.  Accordingly, the driver will continue to
use the driver_data pointer in struct acpi_device which needs to be
cleared on driver removal.

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/panasonic-laptop.c | 33 ++++++++++++++-----------
 1 file changed, 18 insertions(+), 15 deletions(-)

diff --git a/drivers/platform/x86/panasonic-laptop.c b/drivers/platform/x86/panasonic-laptop.c
index 4bd83b987761..632d05adb6f3 100644
--- a/drivers/platform/x86/panasonic-laptop.c
+++ b/drivers/platform/x86/panasonic-laptop.c
@@ -183,8 +183,8 @@ enum SINF_BITS { SINF_NUM_BATTERIES = 0,
 	};
 /* R1 handles SINF_AC_CUR_BRIGHT as SINF_CUR_BRIGHT, doesn't know AC state */
 
-static int acpi_pcc_hotkey_add(struct acpi_device *device);
-static void acpi_pcc_hotkey_remove(struct acpi_device *device);
+static int acpi_pcc_hotkey_probe(struct platform_device *pdev);
+static void acpi_pcc_hotkey_remove(struct platform_device *pdev);
 static void acpi_pcc_hotkey_notify(acpi_handle handle, u32 event, void *data);
 
 static const struct acpi_device_id pcc_device_ids[] = {
@@ -201,15 +201,14 @@ static int acpi_pcc_hotkey_resume(struct device *dev);
 #endif
 static SIMPLE_DEV_PM_OPS(acpi_pcc_hotkey_pm, NULL, acpi_pcc_hotkey_resume);
 
-static struct acpi_driver acpi_pcc_driver = {
-	.name =		ACPI_PCC_DRIVER_NAME,
-	.class =	ACPI_PCC_CLASS,
-	.ids =		pcc_device_ids,
-	.ops =		{
-				.add =		acpi_pcc_hotkey_add,
-				.remove =	acpi_pcc_hotkey_remove,
-			},
-	.drv.pm =	&acpi_pcc_hotkey_pm,
+static struct platform_driver acpi_pcc_driver = {
+	.probe = acpi_pcc_hotkey_probe,
+	.remove = acpi_pcc_hotkey_remove,
+	.driver = {
+		.name = ACPI_PCC_DRIVER_NAME,
+		.acpi_match_table = pcc_device_ids,
+		.pm = &acpi_pcc_hotkey_pm,
+	},
 };
 
 static const struct key_entry panasonic_keymap[] = {
@@ -967,7 +966,7 @@ static int acpi_pcc_init_input(struct pcc_acpi *pcc)
 #ifdef CONFIG_PM_SLEEP
 static int acpi_pcc_hotkey_resume(struct device *dev)
 {
-	struct pcc_acpi *pcc = acpi_driver_data(to_acpi_device(dev));
+	struct pcc_acpi *pcc = acpi_driver_data(ACPI_COMPANION(dev));
 
 	if (pcc->num_sifr > SINF_MUTE)
 		acpi_pcc_write_sset(pcc, SINF_MUTE, pcc->mute);
@@ -983,8 +982,9 @@ static int acpi_pcc_hotkey_resume(struct device *dev)
 }
 #endif
 
-static int acpi_pcc_hotkey_add(struct acpi_device *device)
+static int acpi_pcc_hotkey_probe(struct platform_device *pdev)
 {
+	struct acpi_device *device = ACPI_COMPANION(&pdev->dev);
 	struct backlight_properties props;
 	struct pcc_acpi *pcc;
 	int num_sifr, result;
@@ -1116,8 +1116,9 @@ static int acpi_pcc_hotkey_add(struct acpi_device *device)
 	return result;
 }
 
-static void acpi_pcc_hotkey_remove(struct acpi_device *device)
+static void acpi_pcc_hotkey_remove(struct platform_device *pdev)
 {
+	struct acpi_device *device = ACPI_COMPANION(&pdev->dev);
 	struct pcc_acpi *pcc = acpi_driver_data(device);
 
 	i8042_remove_filter(panasonic_i8042_filter);
@@ -1137,8 +1138,10 @@ static void acpi_pcc_hotkey_remove(struct acpi_device *device)
 
 	input_unregister_device(pcc->input_dev);
 
+	device->driver_data = NULL;
+
 	kfree(pcc->sinf);
 	kfree(pcc);
 }
 
-module_acpi_driver(acpi_pcc_driver);
+module_platform_driver(acpi_pcc_driver);
-- 
2.51.0

Re: [PATCH v1 2/3] platform/x86: panasonic-laptop: Register ACPI notify handler directly

[Ilpo Järvinen]

On Thu, 12 Mar 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/panasonic-laptop.c | 20 +++++++++++++++-----
>  1 file changed, 15 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/platform/x86/panasonic-laptop.c b/drivers/platform/x86/panasonic-laptop.c
> index 64195ff0f40e..4bd83b987761 100644
> --- a/drivers/platform/x86/panasonic-laptop.c
> +++ b/drivers/platform/x86/panasonic-laptop.c
> @@ -185,7 +185,7 @@ enum SINF_BITS { SINF_NUM_BATTERIES = 0,
>  
>  static int acpi_pcc_hotkey_add(struct acpi_device *device);
>  static void acpi_pcc_hotkey_remove(struct acpi_device *device);
> -static void acpi_pcc_hotkey_notify(struct acpi_device *device, u32 event);
> +static void acpi_pcc_hotkey_notify(acpi_handle handle, u32 event, void *data);
>  
>  static const struct acpi_device_id pcc_device_ids[] = {
>  	{ "MAT0012", 0},
> @@ -208,7 +208,6 @@ static struct acpi_driver acpi_pcc_driver = {
>  	.ops =		{
>  				.add =		acpi_pcc_hotkey_add,
>  				.remove =	acpi_pcc_hotkey_remove,
> -				.notify =	acpi_pcc_hotkey_notify,
>  			},
>  	.drv.pm =	&acpi_pcc_hotkey_pm,
>  };
> @@ -869,9 +868,9 @@ static void acpi_pcc_generate_keyinput(struct pcc_acpi *pcc)
>  		pr_err("Unknown hotkey event: 0x%04llx\n", result);
>  }
>  
> -static void acpi_pcc_hotkey_notify(struct acpi_device *device, u32 event)
> +static void acpi_pcc_hotkey_notify(acpi_handle handle, u32 event, void *data)
>  {
> -	struct pcc_acpi *pcc = acpi_driver_data(device);
> +	struct pcc_acpi *pcc = data;
>  
>  	switch (event) {
>  	case HKEY_NOTIFY:
> @@ -1073,13 +1072,18 @@ static int acpi_pcc_hotkey_add(struct acpi_device *device)
>  	if (result)
>  		goto out_backlight;
>  
> +	result = acpi_dev_install_notify_handler(device, ACPI_DEVICE_NOTIFY,
> +						 acpi_pcc_hotkey_notify, pcc);
> +	if (result)
> +		goto out_sysfs;
> +
>  	/* optical drive initialization */
>  	if (ACPI_SUCCESS(check_optd_present())) {
>  		pcc->platform = platform_device_register_simple("panasonic",
>  			PLATFORM_DEVID_NONE, NULL, 0);
>  		if (IS_ERR(pcc->platform)) {
>  			result = PTR_ERR(pcc->platform);
> -			goto out_sysfs;
> +			goto out_notify_handler;
>  		}
>  		result = device_create_file(&pcc->platform->dev,
>  			&dev_attr_cdpower);
> @@ -1095,6 +1099,9 @@ static int acpi_pcc_hotkey_add(struct acpi_device *device)
>  
>  out_platform:
>  	platform_device_unregister(pcc->platform);
> +out_notify_handler:
> +	acpi_dev_remove_notify_handler(device, ACPI_DEVICE_NOTIFY,
> +				       acpi_pcc_hotkey_notify);
>  out_sysfs:
>  	sysfs_remove_group(&device->dev.kobj, &pcc_attr_group);
>  out_backlight:
> @@ -1123,6 +1130,9 @@ static void acpi_pcc_hotkey_remove(struct acpi_device *device)
>  
>  	sysfs_remove_group(&device->dev.kobj, &pcc_attr_group);
>  
> +	acpi_dev_remove_notify_handler(device, ACPI_DEVICE_NOTIFY,
> +				       acpi_pcc_hotkey_notify);
> +

This diverges from the setup order. Is that intentional? (I can see the 
current order is not exactly reverse of the setup order but it still 
looks it's possible to place the added remove call better than here.)

This driver also likely has pre-existing rollback problems related to 
pcc_register_optd_notifier/pcc_unregister_optd_notifier().

>  	backlight_device_unregister(pcc->backlight);
>  
>  	input_unregister_device(pcc->input_dev);
> 

-- 
 i.

Re: [PATCH v1 2/3] platform/x86: panasonic-laptop: Register ACPI notify handler directly

[Rafael J. Wysocki]

On Tue, Mar 17, 2026 at 6:35 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>
> >
> > 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/panasonic-laptop.c | 20 +++++++++++++++-----
> >  1 file changed, 15 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/platform/x86/panasonic-laptop.c b/drivers/platform/x86/panasonic-laptop.c
> > index 64195ff0f40e..4bd83b987761 100644
> > --- a/drivers/platform/x86/panasonic-laptop.c
> > +++ b/drivers/platform/x86/panasonic-laptop.c
> > @@ -185,7 +185,7 @@ enum SINF_BITS { SINF_NUM_BATTERIES = 0,
> >
> >  static int acpi_pcc_hotkey_add(struct acpi_device *device);
> >  static void acpi_pcc_hotkey_remove(struct acpi_device *device);
> > -static void acpi_pcc_hotkey_notify(struct acpi_device *device, u32 event);
> > +static void acpi_pcc_hotkey_notify(acpi_handle handle, u32 event, void *data);
> >
> >  static const struct acpi_device_id pcc_device_ids[] = {
> >       { "MAT0012", 0},
> > @@ -208,7 +208,6 @@ static struct acpi_driver acpi_pcc_driver = {
> >       .ops =          {
> >                               .add =          acpi_pcc_hotkey_add,
> >                               .remove =       acpi_pcc_hotkey_remove,
> > -                             .notify =       acpi_pcc_hotkey_notify,
> >                       },
> >       .drv.pm =       &acpi_pcc_hotkey_pm,
> >  };
> > @@ -869,9 +868,9 @@ static void acpi_pcc_generate_keyinput(struct pcc_acpi *pcc)
> >               pr_err("Unknown hotkey event: 0x%04llx\n", result);
> >  }
> >
> > -static void acpi_pcc_hotkey_notify(struct acpi_device *device, u32 event)
> > +static void acpi_pcc_hotkey_notify(acpi_handle handle, u32 event, void *data)
> >  {
> > -     struct pcc_acpi *pcc = acpi_driver_data(device);
> > +     struct pcc_acpi *pcc = data;
> >
> >       switch (event) {
> >       case HKEY_NOTIFY:
> > @@ -1073,13 +1072,18 @@ static int acpi_pcc_hotkey_add(struct acpi_device *device)
> >       if (result)
> >               goto out_backlight;
> >
> > +     result = acpi_dev_install_notify_handler(device, ACPI_DEVICE_NOTIFY,
> > +                                              acpi_pcc_hotkey_notify, pcc);
> > +     if (result)
> > +             goto out_sysfs;
> > +
> >       /* optical drive initialization */
> >       if (ACPI_SUCCESS(check_optd_present())) {
> >               pcc->platform = platform_device_register_simple("panasonic",
> >                       PLATFORM_DEVID_NONE, NULL, 0);
> >               if (IS_ERR(pcc->platform)) {
> >                       result = PTR_ERR(pcc->platform);
> > -                     goto out_sysfs;
> > +                     goto out_notify_handler;
> >               }
> >               result = device_create_file(&pcc->platform->dev,
> >                       &dev_attr_cdpower);
> > @@ -1095,6 +1099,9 @@ static int acpi_pcc_hotkey_add(struct acpi_device *device)
> >
> >  out_platform:
> >       platform_device_unregister(pcc->platform);
> > +out_notify_handler:
> > +     acpi_dev_remove_notify_handler(device, ACPI_DEVICE_NOTIFY,
> > +                                    acpi_pcc_hotkey_notify);
> >  out_sysfs:
> >       sysfs_remove_group(&device->dev.kobj, &pcc_attr_group);
> >  out_backlight:
> > @@ -1123,6 +1130,9 @@ static void acpi_pcc_hotkey_remove(struct acpi_device *device)
> >
> >       sysfs_remove_group(&device->dev.kobj, &pcc_attr_group);
> >
> > +     acpi_dev_remove_notify_handler(device, ACPI_DEVICE_NOTIFY,
> > +                                    acpi_pcc_hotkey_notify);
> > +
>
> This diverges from the setup order. Is that intentional? (I can see the
> current order is not exactly reverse of the setup order but it still
> looks it's possible to place the added remove call better than here.)
>
> This driver also likely has pre-existing rollback problems related to
> pcc_register_optd_notifier/pcc_unregister_optd_notifier().

So it would be good to fix them while at it.

OK, I'll have a look.

> >       backlight_device_unregister(pcc->backlight);
> >
> >       input_unregister_device(pcc->input_dev);
> >
>
> --
>  i.
>