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

Posted by Rafael J. Wysocki 3 weeks, 5 days ago

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

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

Posted by Ilpo Järvinen 2 weeks, 6 days ago

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

Posted by Rafael J. Wysocki 2 weeks, 6 days ago

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.
>