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

[Rafael J. Wysocki]

On Tue, Mar 17, 2026 at 4:19 PM Ilpo Järvinen
<ilpo.jarvinen@xxxxxxxxxxxxxxx> wrote:
>
> On Mon, 9 Mar 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>
> > ---
> >
> > v1 -> v2: Call fujitsu_laptop_platform_remove() in acpi_fujitsu_laptop_add()
> >    error code path if notify handler registration fails (Ilpo)
>
> Hmm, did you forget to commit something as I don't see this addressed
> below?

No, I didn't.  It's there, you had a comment on it below even.

> > ---
> >  drivers/platform/x86/fujitsu-laptop.c | 32 ++++++++++++++++++++++-----
> >  1 file changed, 27 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/platform/x86/fujitsu-laptop.c b/drivers/platform/x86/fujitsu-laptop.c
> > index 1adce90ae3e6..7816a867a60c 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,13 @@ 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) {
> > +             fujitsu_laptop_platform_remove(device);
>
> Please add this into the rollback path instead and add another label
> there.

OK