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

[Rafael J. Wysocki]

On Mon, Mar 9, 2026 at 5:17 PM Ilpo Järvinen
<ilpo.jarvinen@xxxxxxxxxxxxxxx> 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@xxxxxxxxx>
> > > >
> > > > 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@xxxxxxxxx>
> > > > ---
> > > >  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!