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

["Ilpo Järvinen" <ilpo.jarvinen@linux.intel.com>]

[-- Attachment #1: Type: text/plain, Size: 3834 bytes --]

On Tue, 17 Mar 2026, Rafael J. Wysocki wrote:

> On Tue, Mar 17, 2026 at 4:19 PM Ilpo Järvinen
> <ilpo.jarvinen@linux.intel.com> wrote:
> >
> > On Mon, 9 Mar 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>
> > > ---
> > >
> > > 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.

Yes, sorry, I forgot I'd already written this (and to remove it), only 
after having written this I noticed it was done differently than what I 
had expected.

-- 
 i.


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