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