| Message ID | 20230616165034.3630141-10-michal.wilczynski@intel.com |
|---|---|
| State | New |
| Headers | show |
| Series | Remove .notify callback in acpi_device_ops | expand |
On Fri, Jun 16, 2023 at 6:51 PM Michal Wilczynski <michal.wilczynski@intel.com> wrote: > > Currently logic for installing notifications from ACPI devices is > implemented using notify callback in struct acpi_driver. Preparations > are being made to replace acpi_driver with more generic struct > platform_driver, which doesn't contain notify callback. Furthermore > as of now handlers are being called indirectly through > acpi_notify_device(), which decreases performance. > > Call acpi_dev_install_notify_handler() at the end of .add() callback. > Call acpi_dev_remove_notify_handler() at the beginning of .remove() > callback. Change arguments passed to the notify function to match with > what's required by acpi_install_notify_handler(). Remove .notify > callback initialization in acpi_driver. > > Suggested-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > Signed-off-by: Michal Wilczynski <michal.wilczynski@intel.com> > --- > drivers/acpi/nfit/core.c | 24 ++++++++++++++++++------ > 1 file changed, 18 insertions(+), 6 deletions(-) > > diff --git a/drivers/acpi/nfit/core.c b/drivers/acpi/nfit/core.c > index 95930e9d776c..a281bdfee8a0 100644 > --- a/drivers/acpi/nfit/core.c > +++ b/drivers/acpi/nfit/core.c > @@ -3312,11 +3312,13 @@ void acpi_nfit_shutdown(void *data) > } > EXPORT_SYMBOL_GPL(acpi_nfit_shutdown); > > -static void acpi_nfit_notify(struct acpi_device *adev, u32 event) > +static void acpi_nfit_notify(acpi_handle handle, u32 event, void *data) > { > - device_lock(&adev->dev); > - __acpi_nfit_notify(&adev->dev, adev->handle, event); > - device_unlock(&adev->dev); It's totally not necessary to rename the ACPI device variable here. Just add struct acpi_device *adev = data; to this function. > + struct acpi_device *device = data; > + > + device_lock(&device->dev); > + __acpi_nfit_notify(&device->dev, handle, event); > + device_unlock(&device->dev); > } > > static int acpi_nfit_add(struct acpi_device *adev) > @@ -3375,12 +3377,23 @@ static int acpi_nfit_add(struct acpi_device *adev) > > if (rc) > return rc; > - return devm_add_action_or_reset(dev, acpi_nfit_shutdown, acpi_desc); > + > + rc = devm_add_action_or_reset(dev, acpi_nfit_shutdown, acpi_desc); > + if (rc) > + return rc; > + > + return acpi_dev_install_notify_handler(adev, > + ACPI_DEVICE_NOTIFY, > + acpi_nfit_notify); > } > > static void acpi_nfit_remove(struct acpi_device *adev) > { > /* see acpi_nfit_unregister */ > + > + acpi_dev_remove_notify_handler(adev, > + ACPI_DEVICE_NOTIFY, > + acpi_nfit_notify); > } > > static void acpi_nfit_update_notify(struct device *dev, acpi_handle handle) > @@ -3465,7 +3478,6 @@ static struct acpi_driver acpi_nfit_driver = { > .ops = { > .add = acpi_nfit_add, > .remove = acpi_nfit_remove, > - .notify = acpi_nfit_notify, > }, > }; > > --
Michal Wilczynski wrote: > Currently logic for installing notifications from ACPI devices is > implemented using notify callback in struct acpi_driver. Preparations > are being made to replace acpi_driver with more generic struct > platform_driver, which doesn't contain notify callback. Furthermore > as of now handlers are being called indirectly through > acpi_notify_device(), which decreases performance. > > Call acpi_dev_install_notify_handler() at the end of .add() callback. > Call acpi_dev_remove_notify_handler() at the beginning of .remove() > callback. Change arguments passed to the notify function to match with > what's required by acpi_install_notify_handler(). Remove .notify > callback initialization in acpi_driver. > > Suggested-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > Signed-off-by: Michal Wilczynski <michal.wilczynski@intel.com> > --- > drivers/acpi/nfit/core.c | 24 ++++++++++++++++++------ > 1 file changed, 18 insertions(+), 6 deletions(-) > > diff --git a/drivers/acpi/nfit/core.c b/drivers/acpi/nfit/core.c > index 95930e9d776c..a281bdfee8a0 100644 > --- a/drivers/acpi/nfit/core.c > +++ b/drivers/acpi/nfit/core.c > @@ -3312,11 +3312,13 @@ void acpi_nfit_shutdown(void *data) > } > EXPORT_SYMBOL_GPL(acpi_nfit_shutdown); > > -static void acpi_nfit_notify(struct acpi_device *adev, u32 event) > +static void acpi_nfit_notify(acpi_handle handle, u32 event, void *data) > { > - device_lock(&adev->dev); > - __acpi_nfit_notify(&adev->dev, adev->handle, event); > - device_unlock(&adev->dev); > + struct acpi_device *device = data; > + > + device_lock(&device->dev); > + __acpi_nfit_notify(&device->dev, handle, event); > + device_unlock(&device->dev); > } > > static int acpi_nfit_add(struct acpi_device *adev) > @@ -3375,12 +3377,23 @@ static int acpi_nfit_add(struct acpi_device *adev) > > if (rc) > return rc; > - return devm_add_action_or_reset(dev, acpi_nfit_shutdown, acpi_desc); > + > + rc = devm_add_action_or_reset(dev, acpi_nfit_shutdown, acpi_desc); > + if (rc) > + return rc; > + > + return acpi_dev_install_notify_handler(adev, > + ACPI_DEVICE_NOTIFY, > + acpi_nfit_notify); > } > > static void acpi_nfit_remove(struct acpi_device *adev) > { > /* see acpi_nfit_unregister */ > + > + acpi_dev_remove_notify_handler(adev, > + ACPI_DEVICE_NOTIFY, > + acpi_nfit_notify); Please use devm to trigger this release rather than making acpi_nfit_remove() contain any logic. An additional cleanup opportunity with the ->add() path fully devm instrumented would be to just delete acpi_nfit_remove() since it is optional and serves no purpose.
On 6/29/2023 6:18 PM, Rafael J. Wysocki wrote: > On Fri, Jun 16, 2023 at 6:51 PM Michal Wilczynski > <michal.wilczynski@intel.com> wrote: >> Currently logic for installing notifications from ACPI devices is >> implemented using notify callback in struct acpi_driver. Preparations >> are being made to replace acpi_driver with more generic struct >> platform_driver, which doesn't contain notify callback. Furthermore >> as of now handlers are being called indirectly through >> acpi_notify_device(), which decreases performance. >> >> Call acpi_dev_install_notify_handler() at the end of .add() callback. >> Call acpi_dev_remove_notify_handler() at the beginning of .remove() >> callback. Change arguments passed to the notify function to match with >> what's required by acpi_install_notify_handler(). Remove .notify >> callback initialization in acpi_driver. >> >> Suggested-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com> >> Signed-off-by: Michal Wilczynski <michal.wilczynski@intel.com> >> --- >> drivers/acpi/nfit/core.c | 24 ++++++++++++++++++------ >> 1 file changed, 18 insertions(+), 6 deletions(-) >> >> diff --git a/drivers/acpi/nfit/core.c b/drivers/acpi/nfit/core.c >> index 95930e9d776c..a281bdfee8a0 100644 >> --- a/drivers/acpi/nfit/core.c >> +++ b/drivers/acpi/nfit/core.c >> @@ -3312,11 +3312,13 @@ void acpi_nfit_shutdown(void *data) >> } >> EXPORT_SYMBOL_GPL(acpi_nfit_shutdown); >> >> -static void acpi_nfit_notify(struct acpi_device *adev, u32 event) >> +static void acpi_nfit_notify(acpi_handle handle, u32 event, void *data) >> { >> - device_lock(&adev->dev); >> - __acpi_nfit_notify(&adev->dev, adev->handle, event); >> - device_unlock(&adev->dev); > It's totally not necessary to rename the ACPI device variable here. > > Just add > > struct acpi_device *adev = data; > > to this function. Sure, is adev a preferred name for acpi_device ? I've seen a mix of different naming in drivers, some use device, adev, acpi_dev and so on. I suppose it's not a big deal, but it would be good to know. > >> + struct acpi_device *device = data; >> + >> + device_lock(&device->dev); >> + __acpi_nfit_notify(&device->dev, handle, event); >> + device_unlock(&device->dev); >> } >> >> static int acpi_nfit_add(struct acpi_device *adev) >> @@ -3375,12 +3377,23 @@ static int acpi_nfit_add(struct acpi_device *adev) >> >> if (rc) >> return rc; >> - return devm_add_action_or_reset(dev, acpi_nfit_shutdown, acpi_desc); >> + >> + rc = devm_add_action_or_reset(dev, acpi_nfit_shutdown, acpi_desc); >> + if (rc) >> + return rc; >> + >> + return acpi_dev_install_notify_handler(adev, >> + ACPI_DEVICE_NOTIFY, >> + acpi_nfit_notify); >> } >> >> static void acpi_nfit_remove(struct acpi_device *adev) >> { >> /* see acpi_nfit_unregister */ >> + >> + acpi_dev_remove_notify_handler(adev, >> + ACPI_DEVICE_NOTIFY, >> + acpi_nfit_notify); >> } >> >> static void acpi_nfit_update_notify(struct device *dev, acpi_handle handle) >> @@ -3465,7 +3478,6 @@ static struct acpi_driver acpi_nfit_driver = { >> .ops = { >> .add = acpi_nfit_add, >> .remove = acpi_nfit_remove, >> - .notify = acpi_nfit_notify, >> }, >> }; >> >> --
On Fri, Jun 30, 2023 at 11:55 AM Wilczynski, Michal <michal.wilczynski@intel.com> wrote: > > > > On 6/29/2023 6:18 PM, Rafael J. Wysocki wrote: > > On Fri, Jun 16, 2023 at 6:51 PM Michal Wilczynski > > <michal.wilczynski@intel.com> wrote: > >> Currently logic for installing notifications from ACPI devices is > >> implemented using notify callback in struct acpi_driver. Preparations > >> are being made to replace acpi_driver with more generic struct > >> platform_driver, which doesn't contain notify callback. Furthermore > >> as of now handlers are being called indirectly through > >> acpi_notify_device(), which decreases performance. > >> > >> Call acpi_dev_install_notify_handler() at the end of .add() callback. > >> Call acpi_dev_remove_notify_handler() at the beginning of .remove() > >> callback. Change arguments passed to the notify function to match with > >> what's required by acpi_install_notify_handler(). Remove .notify > >> callback initialization in acpi_driver. > >> > >> Suggested-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > >> Signed-off-by: Michal Wilczynski <michal.wilczynski@intel.com> > >> --- > >> drivers/acpi/nfit/core.c | 24 ++++++++++++++++++------ > >> 1 file changed, 18 insertions(+), 6 deletions(-) > >> > >> diff --git a/drivers/acpi/nfit/core.c b/drivers/acpi/nfit/core.c > >> index 95930e9d776c..a281bdfee8a0 100644 > >> --- a/drivers/acpi/nfit/core.c > >> +++ b/drivers/acpi/nfit/core.c > >> @@ -3312,11 +3312,13 @@ void acpi_nfit_shutdown(void *data) > >> } > >> EXPORT_SYMBOL_GPL(acpi_nfit_shutdown); > >> > >> -static void acpi_nfit_notify(struct acpi_device *adev, u32 event) > >> +static void acpi_nfit_notify(acpi_handle handle, u32 event, void *data) > >> { > >> - device_lock(&adev->dev); > >> - __acpi_nfit_notify(&adev->dev, adev->handle, event); > >> - device_unlock(&adev->dev); > > It's totally not necessary to rename the ACPI device variable here. > > > > Just add > > > > struct acpi_device *adev = data; > > > > to this function. > > Sure, is adev a preferred name for acpi_device ? In new code, it is. In the existing code, it depends. If you do a one-line change, it is better to retain the original naming (for the sake of clarity of the change itself). If you rearrange it completely, you may as well change the names while at it. And there is a spectrum in between. > I've seen a mix of different naming > in drivers, some use device, adev, acpi_dev and so on. I suppose it's not a big deal, but > it would be good to know. Personally, I prefer adev, but this isn't a very strong preference. Using "device" as a name of a struct acpi_device object (or a pointer to one of these for that matter) is slightly misleading IMV, because those things represent AML entities rather than actual hardware.
On 6/29/2023 10:54 PM, Dan Williams wrote: > Michal Wilczynski wrote: >> Currently logic for installing notifications from ACPI devices is >> implemented using notify callback in struct acpi_driver. Preparations >> are being made to replace acpi_driver with more generic struct >> platform_driver, which doesn't contain notify callback. Furthermore >> as of now handlers are being called indirectly through >> acpi_notify_device(), which decreases performance. >> >> Call acpi_dev_install_notify_handler() at the end of .add() callback. >> Call acpi_dev_remove_notify_handler() at the beginning of .remove() >> callback. Change arguments passed to the notify function to match with >> what's required by acpi_install_notify_handler(). Remove .notify >> callback initialization in acpi_driver. >> >> Suggested-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com> >> Signed-off-by: Michal Wilczynski <michal.wilczynski@intel.com> >> --- >> drivers/acpi/nfit/core.c | 24 ++++++++++++++++++------ >> 1 file changed, 18 insertions(+), 6 deletions(-) >> >> diff --git a/drivers/acpi/nfit/core.c b/drivers/acpi/nfit/core.c >> index 95930e9d776c..a281bdfee8a0 100644 >> --- a/drivers/acpi/nfit/core.c >> +++ b/drivers/acpi/nfit/core.c >> @@ -3312,11 +3312,13 @@ void acpi_nfit_shutdown(void *data) >> } >> EXPORT_SYMBOL_GPL(acpi_nfit_shutdown); >> >> -static void acpi_nfit_notify(struct acpi_device *adev, u32 event) >> +static void acpi_nfit_notify(acpi_handle handle, u32 event, void *data) >> { >> - device_lock(&adev->dev); >> - __acpi_nfit_notify(&adev->dev, adev->handle, event); >> - device_unlock(&adev->dev); >> + struct acpi_device *device = data; >> + >> + device_lock(&device->dev); >> + __acpi_nfit_notify(&device->dev, handle, event); >> + device_unlock(&device->dev); >> } >> >> static int acpi_nfit_add(struct acpi_device *adev) >> @@ -3375,12 +3377,23 @@ static int acpi_nfit_add(struct acpi_device *adev) >> >> if (rc) >> return rc; >> - return devm_add_action_or_reset(dev, acpi_nfit_shutdown, acpi_desc); >> + >> + rc = devm_add_action_or_reset(dev, acpi_nfit_shutdown, acpi_desc); >> + if (rc) >> + return rc; >> + >> + return acpi_dev_install_notify_handler(adev, >> + ACPI_DEVICE_NOTIFY, >> + acpi_nfit_notify); >> } >> >> static void acpi_nfit_remove(struct acpi_device *adev) >> { >> /* see acpi_nfit_unregister */ >> + >> + acpi_dev_remove_notify_handler(adev, >> + ACPI_DEVICE_NOTIFY, >> + acpi_nfit_notify); > Please use devm to trigger this release rather than making > acpi_nfit_remove() contain any logic. I think adding separate devm action to remove event handler is not necessary. I'll put the removal in the beggining of acpi_nfit_shutdown() if you don't object. > > An additional cleanup opportunity with the ->add() path fully devm > instrumented would be to just delete acpi_nfit_remove() since it is > optional and serves no purpose. Will do, Thanks !
Wilczynski, Michal wrote: > > > On 6/29/2023 10:54 PM, Dan Williams wrote: > > Michal Wilczynski wrote: > >> Currently logic for installing notifications from ACPI devices is > >> implemented using notify callback in struct acpi_driver. Preparations > >> are being made to replace acpi_driver with more generic struct > >> platform_driver, which doesn't contain notify callback. Furthermore > >> as of now handlers are being called indirectly through > >> acpi_notify_device(), which decreases performance. > >> > >> Call acpi_dev_install_notify_handler() at the end of .add() callback. > >> Call acpi_dev_remove_notify_handler() at the beginning of .remove() > >> callback. Change arguments passed to the notify function to match with > >> what's required by acpi_install_notify_handler(). Remove .notify > >> callback initialization in acpi_driver. > >> > >> Suggested-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > >> Signed-off-by: Michal Wilczynski <michal.wilczynski@intel.com> > >> --- > >> drivers/acpi/nfit/core.c | 24 ++++++++++++++++++------ > >> 1 file changed, 18 insertions(+), 6 deletions(-) > >> > >> diff --git a/drivers/acpi/nfit/core.c b/drivers/acpi/nfit/core.c > >> index 95930e9d776c..a281bdfee8a0 100644 > >> --- a/drivers/acpi/nfit/core.c > >> +++ b/drivers/acpi/nfit/core.c > >> @@ -3312,11 +3312,13 @@ void acpi_nfit_shutdown(void *data) > >> } > >> EXPORT_SYMBOL_GPL(acpi_nfit_shutdown); > >> > >> -static void acpi_nfit_notify(struct acpi_device *adev, u32 event) > >> +static void acpi_nfit_notify(acpi_handle handle, u32 event, void *data) > >> { > >> - device_lock(&adev->dev); > >> - __acpi_nfit_notify(&adev->dev, adev->handle, event); > >> - device_unlock(&adev->dev); > >> + struct acpi_device *device = data; > >> + > >> + device_lock(&device->dev); > >> + __acpi_nfit_notify(&device->dev, handle, event); > >> + device_unlock(&device->dev); > >> } > >> > >> static int acpi_nfit_add(struct acpi_device *adev) > >> @@ -3375,12 +3377,23 @@ static int acpi_nfit_add(struct acpi_device *adev) > >> > >> if (rc) > >> return rc; > >> - return devm_add_action_or_reset(dev, acpi_nfit_shutdown, acpi_desc); > >> + > >> + rc = devm_add_action_or_reset(dev, acpi_nfit_shutdown, acpi_desc); > >> + if (rc) > >> + return rc; > >> + > >> + return acpi_dev_install_notify_handler(adev, > >> + ACPI_DEVICE_NOTIFY, > >> + acpi_nfit_notify); > >> } > >> > >> static void acpi_nfit_remove(struct acpi_device *adev) > >> { > >> /* see acpi_nfit_unregister */ > >> + > >> + acpi_dev_remove_notify_handler(adev, > >> + ACPI_DEVICE_NOTIFY, > >> + acpi_nfit_notify); > > Please use devm to trigger this release rather than making > > acpi_nfit_remove() contain any logic. > > I think adding separate devm action to remove event handler is not > necessary. I'll put the removal in the beggining of acpi_nfit_shutdown() if you > don't object. How do you plan to handle an acpi_dev_install_notify_handler() failure? acpi_nfit_shutdown() will need to have extra logic to know that it can skip acpi_dev_remove_notify_handler() in some cases and not other.. Maybe it is ok to remove a handler that was never installed, but I would rather not go look that up. A devm callback for acpi_dev_remove_notify_handler() avoids that.
On 6/30/2023 7:19 PM, Dan Williams wrote: > Wilczynski, Michal wrote: >> >> On 6/29/2023 10:54 PM, Dan Williams wrote: >>> Michal Wilczynski wrote: >>>> Currently logic for installing notifications from ACPI devices is >>>> implemented using notify callback in struct acpi_driver. Preparations >>>> are being made to replace acpi_driver with more generic struct >>>> platform_driver, which doesn't contain notify callback. Furthermore >>>> as of now handlers are being called indirectly through >>>> acpi_notify_device(), which decreases performance. >>>> >>>> Call acpi_dev_install_notify_handler() at the end of .add() callback. >>>> Call acpi_dev_remove_notify_handler() at the beginning of .remove() >>>> callback. Change arguments passed to the notify function to match with >>>> what's required by acpi_install_notify_handler(). Remove .notify >>>> callback initialization in acpi_driver. >>>> >>>> Suggested-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com> >>>> Signed-off-by: Michal Wilczynski <michal.wilczynski@intel.com> >>>> --- >>>> drivers/acpi/nfit/core.c | 24 ++++++++++++++++++------ >>>> 1 file changed, 18 insertions(+), 6 deletions(-) >>>> >>>> diff --git a/drivers/acpi/nfit/core.c b/drivers/acpi/nfit/core.c >>>> index 95930e9d776c..a281bdfee8a0 100644 >>>> --- a/drivers/acpi/nfit/core.c >>>> +++ b/drivers/acpi/nfit/core.c >>>> @@ -3312,11 +3312,13 @@ void acpi_nfit_shutdown(void *data) >>>> } >>>> EXPORT_SYMBOL_GPL(acpi_nfit_shutdown); >>>> >>>> -static void acpi_nfit_notify(struct acpi_device *adev, u32 event) >>>> +static void acpi_nfit_notify(acpi_handle handle, u32 event, void *data) >>>> { >>>> - device_lock(&adev->dev); >>>> - __acpi_nfit_notify(&adev->dev, adev->handle, event); >>>> - device_unlock(&adev->dev); >>>> + struct acpi_device *device = data; >>>> + >>>> + device_lock(&device->dev); >>>> + __acpi_nfit_notify(&device->dev, handle, event); >>>> + device_unlock(&device->dev); >>>> } >>>> >>>> static int acpi_nfit_add(struct acpi_device *adev) >>>> @@ -3375,12 +3377,23 @@ static int acpi_nfit_add(struct acpi_device *adev) >>>> >>>> if (rc) >>>> return rc; >>>> - return devm_add_action_or_reset(dev, acpi_nfit_shutdown, acpi_desc); >>>> + >>>> + rc = devm_add_action_or_reset(dev, acpi_nfit_shutdown, acpi_desc); >>>> + if (rc) >>>> + return rc; >>>> + >>>> + return acpi_dev_install_notify_handler(adev, >>>> + ACPI_DEVICE_NOTIFY, >>>> + acpi_nfit_notify); >>>> } >>>> >>>> static void acpi_nfit_remove(struct acpi_device *adev) >>>> { >>>> /* see acpi_nfit_unregister */ >>>> + >>>> + acpi_dev_remove_notify_handler(adev, >>>> + ACPI_DEVICE_NOTIFY, >>>> + acpi_nfit_notify); >>> Please use devm to trigger this release rather than making >>> acpi_nfit_remove() contain any logic. >> I think adding separate devm action to remove event handler is not >> necessary. I'll put the removal in the beggining of acpi_nfit_shutdown() if you >> don't object. > How do you plan to handle an acpi_dev_install_notify_handler() failure? > acpi_nfit_shutdown() will need to have extra logic to know that it can > skip acpi_dev_remove_notify_handler() in some cases and not other.. > Maybe it is ok to remove a handler that was never installed, but I would > rather not go look that up. A devm callback for > acpi_dev_remove_notify_handler() avoids that. Sure, I looked at the code and it seems to me that trying to remove a callback that doesn't exist shouldn't cause any problems. But maybe it's not very elegant and we shouldn't rely on that behavior. Will add separate devm action for that then.
diff --git a/drivers/acpi/nfit/core.c b/drivers/acpi/nfit/core.c index 95930e9d776c..a281bdfee8a0 100644 --- a/drivers/acpi/nfit/core.c +++ b/drivers/acpi/nfit/core.c @@ -3312,11 +3312,13 @@ void acpi_nfit_shutdown(void *data) } EXPORT_SYMBOL_GPL(acpi_nfit_shutdown); -static void acpi_nfit_notify(struct acpi_device *adev, u32 event) +static void acpi_nfit_notify(acpi_handle handle, u32 event, void *data) { - device_lock(&adev->dev); - __acpi_nfit_notify(&adev->dev, adev->handle, event); - device_unlock(&adev->dev); + struct acpi_device *device = data; + + device_lock(&device->dev); + __acpi_nfit_notify(&device->dev, handle, event); + device_unlock(&device->dev); } static int acpi_nfit_add(struct acpi_device *adev) @@ -3375,12 +3377,23 @@ static int acpi_nfit_add(struct acpi_device *adev) if (rc) return rc; - return devm_add_action_or_reset(dev, acpi_nfit_shutdown, acpi_desc); + + rc = devm_add_action_or_reset(dev, acpi_nfit_shutdown, acpi_desc); + if (rc) + return rc; + + return acpi_dev_install_notify_handler(adev, + ACPI_DEVICE_NOTIFY, + acpi_nfit_notify); } static void acpi_nfit_remove(struct acpi_device *adev) { /* see acpi_nfit_unregister */ + + acpi_dev_remove_notify_handler(adev, + ACPI_DEVICE_NOTIFY, + acpi_nfit_notify); } static void acpi_nfit_update_notify(struct device *dev, acpi_handle handle) @@ -3465,7 +3478,6 @@ static struct acpi_driver acpi_nfit_driver = { .ops = { .add = acpi_nfit_add, .remove = acpi_nfit_remove, - .notify = acpi_nfit_notify, }, };
Currently logic for installing notifications from ACPI devices is implemented using notify callback in struct acpi_driver. Preparations are being made to replace acpi_driver with more generic struct platform_driver, which doesn't contain notify callback. Furthermore as of now handlers are being called indirectly through acpi_notify_device(), which decreases performance. Call acpi_dev_install_notify_handler() at the end of .add() callback. Call acpi_dev_remove_notify_handler() at the beginning of .remove() callback. Change arguments passed to the notify function to match with what's required by acpi_install_notify_handler(). Remove .notify callback initialization in acpi_driver. Suggested-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com> Signed-off-by: Michal Wilczynski <michal.wilczynski@intel.com> --- drivers/acpi/nfit/core.c | 24 ++++++++++++++++++------ 1 file changed, 18 insertions(+), 6 deletions(-)