On Fri, Jun 16, 2023 at 6:51 PM Michal Wilczynski <michal.wilczynski@xxxxxxxxx> 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. > > While at it, fix lack of whitespaces in .remove() callback. > > Suggested-by: Rafael J. Wysocki <rafael.j.wysocki@xxxxxxxxx> > Signed-off-by: Michal Wilczynski <michal.wilczynski@xxxxxxxxx> > --- > drivers/acpi/battery.c | 30 ++++++++++++++++++++++++------ > 1 file changed, 24 insertions(+), 6 deletions(-) > > diff --git a/drivers/acpi/battery.c b/drivers/acpi/battery.c > index 9c67ed02d797..6337e7b1f6e1 100644 > --- a/drivers/acpi/battery.c > +++ b/drivers/acpi/battery.c > @@ -1034,11 +1034,14 @@ static void acpi_battery_refresh(struct acpi_battery *battery) > } > > /* Driver Interface */ > -static void acpi_battery_notify(struct acpi_device *device, u32 event) > +static void acpi_battery_notify(acpi_handle handle, u32 event, void *data) > { > - struct acpi_battery *battery = acpi_driver_data(device); > + struct acpi_device *device = data; > + struct acpi_battery *battery; > struct power_supply *old; > > + battery = acpi_driver_data(device); > + > if (!battery) > return; > old = battery->bat; > @@ -1212,13 +1215,23 @@ static int acpi_battery_add(struct acpi_device *device) > > device_init_wakeup(&device->dev, 1); > > - return result; > + result = acpi_dev_install_notify_handler(device, > + ACPI_ALL_NOTIFY, > + acpi_battery_notify); > + if (result) > + goto fail_deinit_wakup_and_unregister; You could call the label "fail_pm", for example, which would be more concise and so slightly easier to follow, without any loss of clarity AFAICS. > + > + return 0; > > +fail_deinit_wakup_and_unregister: > + device_init_wakeup(&device->dev, 0); > + unregister_pm_notifier(&battery->pm_nb); > fail: > sysfs_remove_battery(battery); > mutex_destroy(&battery->lock); > mutex_destroy(&battery->sysfs_lock); > kfree(battery); > + > return result; > } > > @@ -1228,10 +1241,17 @@ static void acpi_battery_remove(struct acpi_device *device) > > if (!device || !acpi_driver_data(device)) > return; > - device_init_wakeup(&device->dev, 0); > + > battery = acpi_driver_data(device); > + > + acpi_dev_remove_notify_handler(device, > + ACPI_ALL_NOTIFY, > + acpi_battery_notify); > + > + device_init_wakeup(&device->dev, 0); > unregister_pm_notifier(&battery->pm_nb); > sysfs_remove_battery(battery); > + > mutex_destroy(&battery->lock); > mutex_destroy(&battery->sysfs_lock); > kfree(battery); > @@ -1264,11 +1284,9 @@ static struct acpi_driver acpi_battery_driver = { > .name = "battery", > .class = ACPI_BATTERY_CLASS, > .ids = battery_device_ids, > - .flags = ACPI_DRIVER_ALL_NOTIFY_EVENTS, > .ops = { > .add = acpi_battery_add, > .remove = acpi_battery_remove, > - .notify = acpi_battery_notify, > }, > .drv.pm = &acpi_battery_pm, > }; > -- > 2.41.0 >