[PATCH v1 3/5] platform/x86: fujitsu: Register ACPI notify handlers directly

Posted by Rafael J. Wysocki 1 month, 1 week ago

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

To facilitate subsequent conversion of the driver to using struct
platform_driver instead of struct acpi_driver, make it install its ACPI
notify handlers directly instead of using struct acpi_driver .notify()
callbacks.

No intentional functional impact.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/platform/x86/fujitsu-laptop.c | 30 ++++++++++++++++++++++-----
 1 file changed, 25 insertions(+), 5 deletions(-)

diff --git a/drivers/platform/x86/fujitsu-laptop.c b/drivers/platform/x86/fujitsu-laptop.c
index 1adce90ae3e6..cb13c06b8f35 100644
--- a/drivers/platform/x86/fujitsu-laptop.c
+++ b/drivers/platform/x86/fujitsu-laptop.c
@@ -502,8 +502,9 @@ static int fujitsu_backlight_register(struct acpi_device *device)
 
 /* Brightness notify */
 
-static void acpi_fujitsu_bl_notify(struct acpi_device *device, u32 event)
+static void acpi_fujitsu_bl_notify(acpi_handle handle, u32 event, void *data)
 {
+	struct acpi_device *device = data;
 	struct fujitsu_bl *priv = acpi_driver_data(device);
 	int oldb, newb;
 
@@ -558,7 +559,18 @@ static int acpi_fujitsu_bl_add(struct acpi_device *device)
 	if (ret)
 		return ret;
 
-	return fujitsu_backlight_register(device);
+	ret = fujitsu_backlight_register(device);
+	if (ret)
+		return ret;
+
+	return acpi_dev_install_notify_handler(device, ACPI_DEVICE_NOTIFY,
+					       acpi_fujitsu_bl_notify, device);
+}
+
+static void acpi_fujitsu_bl_remove(struct acpi_device *device)
+{
+	acpi_dev_remove_notify_handler(device, ACPI_DEVICE_NOTIFY,
+				       acpi_fujitsu_bl_notify);
 }
 
 /* ACPI device for hotkey handling */
@@ -941,8 +953,9 @@ static void acpi_fujitsu_laptop_release(struct acpi_device *device)
 	}
 }
 
-static void acpi_fujitsu_laptop_notify(struct acpi_device *device, u32 event)
+static void acpi_fujitsu_laptop_notify(acpi_handle handle, u32 event, void *data)
 {
+	struct acpi_device *device = data;
 	struct fujitsu_laptop *priv = acpi_driver_data(device);
 	unsigned long flags;
 	int scancode, i = 0;
@@ -1056,6 +1069,11 @@ static int acpi_fujitsu_laptop_add(struct acpi_device *device)
 	if (ret)
 		goto err_free_fifo;
 
+	ret = acpi_dev_install_notify_handler(device, ACPI_DEVICE_NOTIFY,
+					      acpi_fujitsu_laptop_notify, device);
+	if (ret)
+		goto err_free_fifo;
+
 	ret = fujitsu_battery_charge_control_add(device);
 	if (ret < 0)
 		pr_warn("Unable to register battery charge control: %d\n", ret);
@@ -1074,6 +1092,9 @@ static void acpi_fujitsu_laptop_remove(struct acpi_device *device)
 
 	fujitsu_battery_charge_control_remove(device);
 
+	acpi_dev_remove_notify_handler(device, ACPI_DEVICE_NOTIFY,
+				       acpi_fujitsu_laptop_notify);
+
 	fujitsu_laptop_platform_remove(device);
 
 	kfifo_free(&priv->fifo);
@@ -1092,7 +1113,7 @@ static struct acpi_driver acpi_fujitsu_bl_driver = {
 	.ids = fujitsu_bl_device_ids,
 	.ops = {
 		.add = acpi_fujitsu_bl_add,
-		.notify = acpi_fujitsu_bl_notify,
+		.remove = acpi_fujitsu_bl_remove,
 		},
 };
 
@@ -1108,7 +1129,6 @@ static struct acpi_driver acpi_fujitsu_laptop_driver = {
 	.ops = {
 		.add = acpi_fujitsu_laptop_add,
 		.remove = acpi_fujitsu_laptop_remove,
-		.notify = acpi_fujitsu_laptop_notify,
 		},
 };
 
-- 
2.51.0

Re: [PATCH v1 3/5] platform/x86: fujitsu: Register ACPI notify handlers directly

Posted by Ilpo Järvinen 4 weeks ago

On Wed, 25 Feb 2026, Rafael J. Wysocki wrote:

> From: "Rafael J. Wysocki" <rafael.j.wysocki@intel.com>
> 
> To facilitate subsequent conversion of the driver to using struct
> platform_driver instead of struct acpi_driver, make it install its ACPI
> notify handlers directly instead of using struct acpi_driver .notify()
> callbacks.
> 
> No intentional functional impact.
> 
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> ---
>  drivers/platform/x86/fujitsu-laptop.c | 30 ++++++++++++++++++++++-----
>  1 file changed, 25 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/platform/x86/fujitsu-laptop.c b/drivers/platform/x86/fujitsu-laptop.c
> index 1adce90ae3e6..cb13c06b8f35 100644
> --- a/drivers/platform/x86/fujitsu-laptop.c
> +++ b/drivers/platform/x86/fujitsu-laptop.c
> @@ -502,8 +502,9 @@ static int fujitsu_backlight_register(struct acpi_device *device)
>  
>  /* Brightness notify */
>  
> -static void acpi_fujitsu_bl_notify(struct acpi_device *device, u32 event)
> +static void acpi_fujitsu_bl_notify(acpi_handle handle, u32 event, void *data)
>  {
> +	struct acpi_device *device = data;
>  	struct fujitsu_bl *priv = acpi_driver_data(device);
>  	int oldb, newb;
>  
> @@ -558,7 +559,18 @@ static int acpi_fujitsu_bl_add(struct acpi_device *device)
>  	if (ret)
>  		return ret;
>  
> -	return fujitsu_backlight_register(device);
> +	ret = fujitsu_backlight_register(device);
> +	if (ret)
> +		return ret;
> +
> +	return acpi_dev_install_notify_handler(device, ACPI_DEVICE_NOTIFY,
> +					       acpi_fujitsu_bl_notify, device);
> +}
> +
> +static void acpi_fujitsu_bl_remove(struct acpi_device *device)
> +{
> +	acpi_dev_remove_notify_handler(device, ACPI_DEVICE_NOTIFY,
> +				       acpi_fujitsu_bl_notify);
>  }
>  
>  /* ACPI device for hotkey handling */
> @@ -941,8 +953,9 @@ static void acpi_fujitsu_laptop_release(struct acpi_device *device)
>  	}
>  }
>  
> -static void acpi_fujitsu_laptop_notify(struct acpi_device *device, u32 event)
> +static void acpi_fujitsu_laptop_notify(acpi_handle handle, u32 event, void *data)
>  {
> +	struct acpi_device *device = data;
>  	struct fujitsu_laptop *priv = acpi_driver_data(device);
>  	unsigned long flags;
>  	int scancode, i = 0;
> @@ -1056,6 +1069,11 @@ static int acpi_fujitsu_laptop_add(struct acpi_device *device)
>  	if (ret)
>  		goto err_free_fifo;
>  
> +	ret = acpi_dev_install_notify_handler(device, ACPI_DEVICE_NOTIFY,
> +					      acpi_fujitsu_laptop_notify, device);
> +	if (ret)
> +		goto err_free_fifo;

Hi Rafael,

Is the rollback path still correct after adding this here?

-- 
 i.

> +
>  	ret = fujitsu_battery_charge_control_add(device);
>  	if (ret < 0)
>  		pr_warn("Unable to register battery charge control: %d\n", ret);
> @@ -1074,6 +1092,9 @@ static void acpi_fujitsu_laptop_remove(struct acpi_device *device)
>  
>  	fujitsu_battery_charge_control_remove(device);
>  
> +	acpi_dev_remove_notify_handler(device, ACPI_DEVICE_NOTIFY,
> +				       acpi_fujitsu_laptop_notify);
> +
>  	fujitsu_laptop_platform_remove(device);
>  
>  	kfifo_free(&priv->fifo);
> @@ -1092,7 +1113,7 @@ static struct acpi_driver acpi_fujitsu_bl_driver = {
>  	.ids = fujitsu_bl_device_ids,
>  	.ops = {
>  		.add = acpi_fujitsu_bl_add,
> -		.notify = acpi_fujitsu_bl_notify,
> +		.remove = acpi_fujitsu_bl_remove,
>  		},
>  };
>  
> @@ -1108,7 +1129,6 @@ static struct acpi_driver acpi_fujitsu_laptop_driver = {
>  	.ops = {
>  		.add = acpi_fujitsu_laptop_add,
>  		.remove = acpi_fujitsu_laptop_remove,
> -		.notify = acpi_fujitsu_laptop_notify,
>  		},
>  };
>  
>

Re: [PATCH v1 3/5] platform/x86: fujitsu: Register ACPI notify handlers directly

Posted by Rafael J. Wysocki 4 weeks ago

On Monday, March 9, 2026 2:00:31 PM CET Ilpo Järvinen wrote:
> On Wed, 25 Feb 2026, Rafael J. Wysocki wrote:
> 
> > From: "Rafael J. Wysocki" <rafael.j.wysocki@intel.com>
> > 
> > To facilitate subsequent conversion of the driver to using struct
> > platform_driver instead of struct acpi_driver, make it install its ACPI
> > notify handlers directly instead of using struct acpi_driver .notify()
> > callbacks.
> > 
> > No intentional functional impact.
> > 
> > Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> > ---
> >  drivers/platform/x86/fujitsu-laptop.c | 30 ++++++++++++++++++++++-----
> >  1 file changed, 25 insertions(+), 5 deletions(-)
> > 
> > diff --git a/drivers/platform/x86/fujitsu-laptop.c b/drivers/platform/x86/fujitsu-laptop.c
> > index 1adce90ae3e6..cb13c06b8f35 100644
> > --- a/drivers/platform/x86/fujitsu-laptop.c
> > +++ b/drivers/platform/x86/fujitsu-laptop.c
> > @@ -502,8 +502,9 @@ static int fujitsu_backlight_register(struct acpi_device *device)
> >  
> >  /* Brightness notify */
> >  
> > -static void acpi_fujitsu_bl_notify(struct acpi_device *device, u32 event)
> > +static void acpi_fujitsu_bl_notify(acpi_handle handle, u32 event, void *data)
> >  {
> > +	struct acpi_device *device = data;
> >  	struct fujitsu_bl *priv = acpi_driver_data(device);
> >  	int oldb, newb;
> >  
> > @@ -558,7 +559,18 @@ static int acpi_fujitsu_bl_add(struct acpi_device *device)
> >  	if (ret)
> >  		return ret;
> >  
> > -	return fujitsu_backlight_register(device);
> > +	ret = fujitsu_backlight_register(device);
> > +	if (ret)
> > +		return ret;
> > +
> > +	return acpi_dev_install_notify_handler(device, ACPI_DEVICE_NOTIFY,
> > +					       acpi_fujitsu_bl_notify, device);
> > +}
> > +
> > +static void acpi_fujitsu_bl_remove(struct acpi_device *device)
> > +{
> > +	acpi_dev_remove_notify_handler(device, ACPI_DEVICE_NOTIFY,
> > +				       acpi_fujitsu_bl_notify);
> >  }
> >  
> >  /* ACPI device for hotkey handling */
> > @@ -941,8 +953,9 @@ static void acpi_fujitsu_laptop_release(struct acpi_device *device)
> >  	}
> >  }
> >  
> > -static void acpi_fujitsu_laptop_notify(struct acpi_device *device, u32 event)
> > +static void acpi_fujitsu_laptop_notify(acpi_handle handle, u32 event, void *data)
> >  {
> > +	struct acpi_device *device = data;
> >  	struct fujitsu_laptop *priv = acpi_driver_data(device);
> >  	unsigned long flags;
> >  	int scancode, i = 0;
> > @@ -1056,6 +1069,11 @@ static int acpi_fujitsu_laptop_add(struct acpi_device *device)
> >  	if (ret)
> >  		goto err_free_fifo;
> >  
> > +	ret = acpi_dev_install_notify_handler(device, ACPI_DEVICE_NOTIFY,
> > +					      acpi_fujitsu_laptop_notify, device);
> > +	if (ret)
> > +		goto err_free_fifo;
> 
> Hi Rafael,
> 
> Is the rollback path still correct after adding this here?

Why wouldn't it be?  There's only kfifo_free() in it.

> > +
> >  	ret = fujitsu_battery_charge_control_add(device);
> >  	if (ret < 0)
> >  		pr_warn("Unable to register battery charge control: %d\n", ret);
> > @@ -1074,6 +1092,9 @@ static void acpi_fujitsu_laptop_remove(struct acpi_device *device)
> >  
> >  	fujitsu_battery_charge_control_remove(device);
> >  
> > +	acpi_dev_remove_notify_handler(device, ACPI_DEVICE_NOTIFY,
> > +				       acpi_fujitsu_laptop_notify);
> > +
> >  	fujitsu_laptop_platform_remove(device);
> >  
> >  	kfifo_free(&priv->fifo);
> > @@ -1092,7 +1113,7 @@ static struct acpi_driver acpi_fujitsu_bl_driver = {
> >  	.ids = fujitsu_bl_device_ids,
> >  	.ops = {
> >  		.add = acpi_fujitsu_bl_add,
> > -		.notify = acpi_fujitsu_bl_notify,
> > +		.remove = acpi_fujitsu_bl_remove,
> >  		},
> >  };
> >  
> > @@ -1108,7 +1129,6 @@ static struct acpi_driver acpi_fujitsu_laptop_driver = {
> >  	.ops = {
> >  		.add = acpi_fujitsu_laptop_add,
> >  		.remove = acpi_fujitsu_laptop_remove,
> > -		.notify = acpi_fujitsu_laptop_notify,
> >  		},
> >  };
> >  
> > 
> 
>

Re: [PATCH v1 3/5] platform/x86: fujitsu: Register ACPI notify handlers directly

Posted by Ilpo Järvinen 4 weeks ago

On Mon, 9 Mar 2026, Rafael J. Wysocki wrote:
> On Monday, March 9, 2026 2:00:31 PM CET Ilpo Järvinen wrote:
> > On Wed, 25 Feb 2026, Rafael J. Wysocki wrote:
> > 
> > > From: "Rafael J. Wysocki" <rafael.j.wysocki@intel.com>
> > > 
> > > To facilitate subsequent conversion of the driver to using struct
> > > platform_driver instead of struct acpi_driver, make it install its ACPI
> > > notify handlers directly instead of using struct acpi_driver .notify()
> > > callbacks.
> > > 
> > > No intentional functional impact.
> > > 
> > > Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> > > ---
> > >  drivers/platform/x86/fujitsu-laptop.c | 30 ++++++++++++++++++++++-----
> > >  1 file changed, 25 insertions(+), 5 deletions(-)
> > > 
> > > diff --git a/drivers/platform/x86/fujitsu-laptop.c b/drivers/platform/x86/fujitsu-laptop.c
> > > index 1adce90ae3e6..cb13c06b8f35 100644
> > > --- a/drivers/platform/x86/fujitsu-laptop.c
> > > +++ b/drivers/platform/x86/fujitsu-laptop.c
> > > @@ -502,8 +502,9 @@ static int fujitsu_backlight_register(struct acpi_device *device)
> > >  
> > >  /* Brightness notify */
> > >  
> > > -static void acpi_fujitsu_bl_notify(struct acpi_device *device, u32 event)
> > > +static void acpi_fujitsu_bl_notify(acpi_handle handle, u32 event, void *data)
> > >  {
> > > +	struct acpi_device *device = data;
> > >  	struct fujitsu_bl *priv = acpi_driver_data(device);
> > >  	int oldb, newb;
> > >  
> > > @@ -558,7 +559,18 @@ static int acpi_fujitsu_bl_add(struct acpi_device *device)
> > >  	if (ret)
> > >  		return ret;
> > >  
> > > -	return fujitsu_backlight_register(device);
> > > +	ret = fujitsu_backlight_register(device);
> > > +	if (ret)
> > > +		return ret;
> > > +
> > > +	return acpi_dev_install_notify_handler(device, ACPI_DEVICE_NOTIFY,
> > > +					       acpi_fujitsu_bl_notify, device);
> > > +}
> > > +
> > > +static void acpi_fujitsu_bl_remove(struct acpi_device *device)
> > > +{
> > > +	acpi_dev_remove_notify_handler(device, ACPI_DEVICE_NOTIFY,
> > > +				       acpi_fujitsu_bl_notify);
> > >  }
> > >  
> > >  /* ACPI device for hotkey handling */
> > > @@ -941,8 +953,9 @@ static void acpi_fujitsu_laptop_release(struct acpi_device *device)
> > >  	}
> > >  }
> > >  
> > > -static void acpi_fujitsu_laptop_notify(struct acpi_device *device, u32 event)
> > > +static void acpi_fujitsu_laptop_notify(acpi_handle handle, u32 event, void *data)
> > >  {
> > > +	struct acpi_device *device = data;
> > >  	struct fujitsu_laptop *priv = acpi_driver_data(device);
> > >  	unsigned long flags;
> > >  	int scancode, i = 0;
> > > @@ -1056,6 +1069,11 @@ static int acpi_fujitsu_laptop_add(struct acpi_device *device)
> > >  	if (ret)
> > >  		goto err_free_fifo;
> > >  
> > > +	ret = acpi_dev_install_notify_handler(device, ACPI_DEVICE_NOTIFY,
> > > +					      acpi_fujitsu_laptop_notify, device);
> > > +	if (ret)
> > > +		goto err_free_fifo;
> > 
> > Hi Rafael,
> > 
> > Is the rollback path still correct after adding this here?
> 
> Why wouldn't it be?  There's only kfifo_free() in it.

So it's intentional to not add fujitsu_laptop_platform_remove() there?

> > > +
> > >  	ret = fujitsu_battery_charge_control_add(device);
> > >  	if (ret < 0)
> > >  		pr_warn("Unable to register battery charge control: %d\n", ret);
> > > @@ -1074,6 +1092,9 @@ static void acpi_fujitsu_laptop_remove(struct acpi_device *device)
> > >  
> > >  	fujitsu_battery_charge_control_remove(device);
> > >  
> > > +	acpi_dev_remove_notify_handler(device, ACPI_DEVICE_NOTIFY,
> > > +				       acpi_fujitsu_laptop_notify);
> > > +
> > >  	fujitsu_laptop_platform_remove(device);
> > >  
> > >  	kfifo_free(&priv->fifo);
> > > @@ -1092,7 +1113,7 @@ static struct acpi_driver acpi_fujitsu_bl_driver = {
> > >  	.ids = fujitsu_bl_device_ids,
> > >  	.ops = {
> > >  		.add = acpi_fujitsu_bl_add,
> > > -		.notify = acpi_fujitsu_bl_notify,
> > > +		.remove = acpi_fujitsu_bl_remove,
> > >  		},
> > >  };
> > >  
> > > @@ -1108,7 +1129,6 @@ static struct acpi_driver acpi_fujitsu_laptop_driver = {
> > >  	.ops = {
> > >  		.add = acpi_fujitsu_laptop_add,
> > >  		.remove = acpi_fujitsu_laptop_remove,
> > > -		.notify = acpi_fujitsu_laptop_notify,
> > >  		},
> > >  };
> > >  
> > > 
> > 
> > 
> 
> 
> 
> 

-- 
 i.

Re: [PATCH v1 3/5] platform/x86: fujitsu: Register ACPI notify handlers directly

Posted by Rafael J. Wysocki 4 weeks ago

On Mon, Mar 9, 2026 at 5:17 PM Ilpo Järvinen
<ilpo.jarvinen@linux.intel.com> wrote:
>
> On Mon, 9 Mar 2026, Rafael J. Wysocki wrote:
> > On Monday, March 9, 2026 2:00:31 PM CET Ilpo Järvinen wrote:
> > > On Wed, 25 Feb 2026, Rafael J. Wysocki wrote:
> > >
> > > > From: "Rafael J. Wysocki" <rafael.j.wysocki@intel.com>
> > > >
> > > > To facilitate subsequent conversion of the driver to using struct
> > > > platform_driver instead of struct acpi_driver, make it install its ACPI
> > > > notify handlers directly instead of using struct acpi_driver .notify()
> > > > callbacks.
> > > >
> > > > No intentional functional impact.
> > > >
> > > > Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> > > > ---
> > > >  drivers/platform/x86/fujitsu-laptop.c | 30 ++++++++++++++++++++++-----
> > > >  1 file changed, 25 insertions(+), 5 deletions(-)
> > > >
> > > > diff --git a/drivers/platform/x86/fujitsu-laptop.c b/drivers/platform/x86/fujitsu-laptop.c
> > > > index 1adce90ae3e6..cb13c06b8f35 100644
> > > > --- a/drivers/platform/x86/fujitsu-laptop.c
> > > > +++ b/drivers/platform/x86/fujitsu-laptop.c
> > > > @@ -502,8 +502,9 @@ static int fujitsu_backlight_register(struct acpi_device *device)
> > > >
> > > >  /* Brightness notify */
> > > >
> > > > -static void acpi_fujitsu_bl_notify(struct acpi_device *device, u32 event)
> > > > +static void acpi_fujitsu_bl_notify(acpi_handle handle, u32 event, void *data)
> > > >  {
> > > > + struct acpi_device *device = data;
> > > >   struct fujitsu_bl *priv = acpi_driver_data(device);
> > > >   int oldb, newb;
> > > >
> > > > @@ -558,7 +559,18 @@ static int acpi_fujitsu_bl_add(struct acpi_device *device)
> > > >   if (ret)
> > > >           return ret;
> > > >
> > > > - return fujitsu_backlight_register(device);
> > > > + ret = fujitsu_backlight_register(device);
> > > > + if (ret)
> > > > +         return ret;
> > > > +
> > > > + return acpi_dev_install_notify_handler(device, ACPI_DEVICE_NOTIFY,
> > > > +                                        acpi_fujitsu_bl_notify, device);
> > > > +}
> > > > +
> > > > +static void acpi_fujitsu_bl_remove(struct acpi_device *device)
> > > > +{
> > > > + acpi_dev_remove_notify_handler(device, ACPI_DEVICE_NOTIFY,
> > > > +                                acpi_fujitsu_bl_notify);
> > > >  }
> > > >
> > > >  /* ACPI device for hotkey handling */
> > > > @@ -941,8 +953,9 @@ static void acpi_fujitsu_laptop_release(struct acpi_device *device)
> > > >   }
> > > >  }
> > > >
> > > > -static void acpi_fujitsu_laptop_notify(struct acpi_device *device, u32 event)
> > > > +static void acpi_fujitsu_laptop_notify(acpi_handle handle, u32 event, void *data)
> > > >  {
> > > > + struct acpi_device *device = data;
> > > >   struct fujitsu_laptop *priv = acpi_driver_data(device);
> > > >   unsigned long flags;
> > > >   int scancode, i = 0;
> > > > @@ -1056,6 +1069,11 @@ static int acpi_fujitsu_laptop_add(struct acpi_device *device)
> > > >   if (ret)
> > > >           goto err_free_fifo;
> > > >
> > > > + ret = acpi_dev_install_notify_handler(device, ACPI_DEVICE_NOTIFY,
> > > > +                                       acpi_fujitsu_laptop_notify, device);
> > > > + if (ret)
> > > > +         goto err_free_fifo;
> > >
> > > Hi Rafael,
> > >
> > > Is the rollback path still correct after adding this here?
> >
> > Why wouldn't it be?  There's only kfifo_free() in it.
>
> So it's intentional to not add fujitsu_laptop_platform_remove() there?

Ah, yes, it needs to be called when acpi_dev_install_notify_handler() fails.

A fix would be relatively straightforward, but I guess I need to
resubmit the whole series with this fixed.

Will do, thanks!