| Message ID | 20230601131719.300720-1-michal.wilczynski@intel.com (mailing list archive) |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
| Series | Remove .notify callback in acpi_device_ops | expand |
On Thursday, June 1, 2023 3:17:19 PM CEST Michal Wilczynski wrote: > Currently acpi_device_install_notify_handler() and > acpi_device_remove_notify_handler() always install acpi_notify_device() > as a function handler, and only then the real .notify callback gets > called. This is not efficient and doesn't provide any real advantage. > > Introduce new acpi_device_install_event_handler() and > acpi_device_remove_event_handler(). Those functions are replacing old > installers, and after all drivers switch to the new model, old installers > will be removed at the end of the patchset. > > Make new installer/removal function arguments to take function pointer as > an argument instead of using .notify callback. Introduce new variable in > struct acpi_device, as fixed events still needs to be handled by an > intermediary that would schedule them for later execution. This is due to > fixed hardware event handlers being executed in interrupt context. > > Make acpi_device_install_event_handler() and > acpi_device_remove_event_handler() non-static, and export symbols. This > will allow the drivers to call them directly, instead of relying on > .notify callback. > > Signed-off-by: Michal Wilczynski <michal.wilczynski@intel.com> > --- > drivers/acpi/bus.c | 59 ++++++++++++++++++++++++++++++++++++++++- > include/acpi/acpi_bus.h | 7 +++++ > 2 files changed, 65 insertions(+), 1 deletion(-) > > diff --git a/drivers/acpi/bus.c b/drivers/acpi/bus.c > index d161ff707de4..cf2c2bfe29a0 100644 > --- a/drivers/acpi/bus.c > +++ b/drivers/acpi/bus.c > @@ -535,7 +535,7 @@ static void acpi_notify_device_fixed(void *data) > struct acpi_device *device = data; > > /* Fixed hardware devices have no handles */ > - acpi_notify_device(NULL, ACPI_FIXED_HARDWARE_EVENT, device); > + device->fixed_event_notify(NULL, ACPI_FIXED_HARDWARE_EVENT, device); > } > > static u32 acpi_device_fixed_event(void *data) > @@ -550,11 +550,13 @@ static int acpi_device_install_notify_handler(struct acpi_device *device, > acpi_status status; > > if (device->device_type == ACPI_BUS_TYPE_POWER_BUTTON) { > + device->fixed_event_notify = acpi_notify_device; > status = > acpi_install_fixed_event_handler(ACPI_EVENT_POWER_BUTTON, > acpi_device_fixed_event, > device); > } else if (device->device_type == ACPI_BUS_TYPE_SLEEP_BUTTON) { > + device->fixed_event_notify = acpi_notify_device; > status = > acpi_install_fixed_event_handler(ACPI_EVENT_SLEEP_BUTTON, > acpi_device_fixed_event, > @@ -579,9 +581,11 @@ static void acpi_device_remove_notify_handler(struct acpi_device *device, > if (device->device_type == ACPI_BUS_TYPE_POWER_BUTTON) { > acpi_remove_fixed_event_handler(ACPI_EVENT_POWER_BUTTON, > acpi_device_fixed_event); > + device->fixed_event_notify = NULL; > } else if (device->device_type == ACPI_BUS_TYPE_SLEEP_BUTTON) { > acpi_remove_fixed_event_handler(ACPI_EVENT_SLEEP_BUTTON, > acpi_device_fixed_event); > + device->fixed_event_notify = NULL; > } else { > u32 type = acpi_drv->flags & ACPI_DRIVER_ALL_NOTIFY_EVENTS ? > ACPI_ALL_NOTIFY : ACPI_DEVICE_NOTIFY; > @@ -592,6 +596,59 @@ static void acpi_device_remove_notify_handler(struct acpi_device *device, > acpi_os_wait_events_complete(); > } > > +int acpi_device_install_event_handler(struct acpi_device *device, > + u32 type, > + void (*notify)(acpi_handle, u32, void*)) > +{ > + acpi_status status; > + > + if (!notify) > + return -EINVAL; > + > + if (device->device_type == ACPI_BUS_TYPE_POWER_BUTTON) { > + device->fixed_event_notify = notify; > + status = > + acpi_install_fixed_event_handler(ACPI_EVENT_POWER_BUTTON, > + acpi_device_fixed_event, > + device); > + } else if (device->device_type == ACPI_BUS_TYPE_SLEEP_BUTTON) { > + device->fixed_event_notify = notify; > + status = > + acpi_install_fixed_event_handler(ACPI_EVENT_SLEEP_BUTTON, > + acpi_device_fixed_event, > + device); > + } else { > + status = acpi_install_notify_handler(device->handle, type, > + notify, > + device); > + } > + > + if (ACPI_FAILURE(status)) > + return -EINVAL; > + return 0; > +} > +EXPORT_SYMBOL(acpi_device_install_event_handler); > + > +void acpi_device_remove_event_handler(struct acpi_device *device, > + u32 type, > + void (*notify)(acpi_handle, u32, void*)) > +{ > + if (device->device_type == ACPI_BUS_TYPE_POWER_BUTTON) { > + acpi_remove_fixed_event_handler(ACPI_EVENT_POWER_BUTTON, > + acpi_device_fixed_event); > + device->fixed_event_notify = NULL; > + } else if (device->device_type == ACPI_BUS_TYPE_SLEEP_BUTTON) { > + acpi_remove_fixed_event_handler(ACPI_EVENT_SLEEP_BUTTON, > + acpi_device_fixed_event); > + device->fixed_event_notify = NULL; > + } else { > + acpi_remove_notify_handler(device->handle, type, > + notify); > + } > + acpi_os_wait_events_complete(); > +} > +EXPORT_SYMBOL(acpi_device_remove_event_handler); > + > /* Handle events targeting \_SB device (at present only graceful shutdown) */ > > #define ACPI_SB_NOTIFY_SHUTDOWN_REQUEST 0x81 > diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h > index a6affc0550b0..7fb411438b6f 100644 > --- a/include/acpi/acpi_bus.h > +++ b/include/acpi/acpi_bus.h > @@ -387,6 +387,7 @@ struct acpi_device { > struct list_head physical_node_list; > struct mutex physical_node_lock; > void (*remove)(struct acpi_device *); > + void (*fixed_event_notify)(acpi_handle handle, u32 type, void *data); This is a rather confusing change, because ->remove() above is not a driver callback, whereas the new one would be. Moreover, it is rather wasteful, because the only devices needing it are buttons, so for all of the other ACPI device objects the new callback pointer would always be NULL. Finally, it is not necessary even. The key observation here is that there are only 2 drivers handling power and sleep buttons that use ACPI fixed events: the ACPI button driver (button.c in drivers/acpi) and the "tiny power button" driver (tiny-power-button.c in drivers/acpi). All of the other drivers don't need the "fixed event notify" thing and these two can be modified to take care of all of it by themselves. So if something like the below is done prior to the rest of your series, the rest will be about acpi_install/remove_notify_handler() only and you won't even need the wrapper routines any more: driver may just be switched over to using the ACPICA functions directly. [This patch is untested and is really 3 patches in one, but since I've cut it already, I'll send it properly next week after some button driver testing.] --- drivers/acpi/bus.c | 53 +++++----------------------------- drivers/acpi/button.c | 60 +++++++++++++++++++++++++++++++++------ drivers/acpi/tiny-power-button.c | 49 ++++++++++++++++++++++++++----- 3 files changed, 101 insertions(+), 61 deletions(-) Index: linux-pm/drivers/acpi/tiny-power-button.c =================================================================== --- linux-pm.orig/drivers/acpi/tiny-power-button.c +++ linux-pm/drivers/acpi/tiny-power-button.c @@ -19,18 +19,52 @@ static const struct acpi_device_id tiny_ }; MODULE_DEVICE_TABLE(acpi, tiny_power_button_device_ids); -static int acpi_noop_add(struct acpi_device *device) +static void acpi_tiny_power_button_notify(acpi_handle handle, u32 event, void *data) { - return 0; + kill_cad_pid(power_signal, 1); } -static void acpi_noop_remove(struct acpi_device *device) +static void acpi_tiny_power_button_notify_run(void *not_used) { + acpi_tiny_power_button_notify(NULL, ACPI_FIXED_HARDWARE_EVENT, NULL); } -static void acpi_tiny_power_button_notify(struct acpi_device *device, u32 event) +static u32 acpi_tiny_power_button_event(void *not_used) { - kill_cad_pid(power_signal, 1); + acpi_os_execute(OSL_NOTIFY_HANDLER, acpi_tiny_power_button_notify_run, NULL); + return ACPI_INTERRUPT_HANDLED; +} + +static int acpi_tiny_power_button_add(struct acpi_device *device) +{ + acpi_status status; + + if (device->device_type == ACPI_BUS_TYPE_POWER_BUTTON) { + status = acpi_install_fixed_event_handler(ACPI_EVENT_POWER_BUTTON, + acpi_tiny_power_button_event, + NULL); + } else { + status = acpi_install_notify_handler(device->handle, + ACPI_DEVICE_NOTIFY, + acpi_tiny_power_button_notify, + NULL); + } + if (ACPI_FAILURE(status)) + return -ENODEV; + + return 0; +} + +static void acpi_tiny_power_button_remove(struct acpi_device *device) +{ + if (device->device_type == ACPI_BUS_TYPE_POWER_BUTTON) { + acpi_remove_fixed_event_handler(ACPI_EVENT_POWER_BUTTON, + acpi_tiny_power_button_event); + } else { + acpi_remove_notify_handler(device->handle, ACPI_DEVICE_NOTIFY, + acpi_tiny_power_button_notify); + } + acpi_os_wait_events_complete(); } static struct acpi_driver acpi_tiny_power_button_driver = { @@ -38,9 +72,8 @@ static struct acpi_driver acpi_tiny_powe .class = "tiny-power-button", .ids = tiny_power_button_device_ids, .ops = { - .add = acpi_noop_add, - .remove = acpi_noop_remove, - .notify = acpi_tiny_power_button_notify, + .add = acpi_tiny_power_button_add, + .remove = acpi_tiny_power_button_remove, }, }; Index: linux-pm/drivers/acpi/button.c =================================================================== --- linux-pm.orig/drivers/acpi/button.c +++ linux-pm/drivers/acpi/button.c @@ -135,7 +135,6 @@ static const struct dmi_system_id dmi_li static int acpi_button_add(struct acpi_device *device); static void acpi_button_remove(struct acpi_device *device); -static void acpi_button_notify(struct acpi_device *device, u32 event); #ifdef CONFIG_PM_SLEEP static int acpi_button_suspend(struct device *dev); @@ -153,7 +152,6 @@ static struct acpi_driver acpi_button_dr .ops = { .add = acpi_button_add, .remove = acpi_button_remove, - .notify = acpi_button_notify, }, .drv.pm = &acpi_button_pm, }; @@ -409,15 +407,13 @@ static void acpi_lid_initialize_state(st button->lid_state_initialized = true; } -static void acpi_button_notify(struct acpi_device *device, u32 event) +static void acpi_button_notify(acpi_handle handle, u32 event, void *data) { + struct acpi_device *device = data; struct acpi_button *button = acpi_driver_data(device); struct input_dev *input; switch (event) { - case ACPI_FIXED_HARDWARE_EVENT: - event = ACPI_BUTTON_NOTIFY_STATUS; - fallthrough; case ACPI_BUTTON_NOTIFY_STATUS: input = button->input; if (button->type == ACPI_BUTTON_TYPE_LID) { @@ -450,6 +446,17 @@ static void acpi_button_notify(struct ac } } +static void acpi_button_notify_run(void *data) +{ + acpi_button_notify(NULL, ACPI_BUTTON_NOTIFY_STATUS, data); +} + +static u32 acpi_button_event(void *data) +{ + acpi_os_execute(OSL_NOTIFY_HANDLER, acpi_button_notify_run, data); + return ACPI_INTERRUPT_HANDLED; +} + #ifdef CONFIG_PM_SLEEP static int acpi_button_suspend(struct device *dev) { @@ -492,6 +499,7 @@ static int acpi_button_add(struct acpi_d struct acpi_button *button; struct input_dev *input; const char *hid = acpi_device_hid(device); + acpi_status status; char *name, *class; int error; @@ -568,6 +576,26 @@ static int acpi_button_add(struct acpi_d error = input_register_device(input); if (error) goto err_remove_fs; + + if (device->device_type == ACPI_BUS_TYPE_POWER_BUTTON) { + status = acpi_install_fixed_event_handler(ACPI_EVENT_POWER_BUTTON, + acpi_button_event, + device); + } else if (device->device_type == ACPI_BUS_TYPE_SLEEP_BUTTON) { + status = acpi_install_fixed_event_handler(ACPI_EVENT_SLEEP_BUTTON, + acpi_button_event, + device); + } else { + status = acpi_install_notify_handler(device->handle, + ACPI_DEVICE_NOTIFY, + acpi_button_notify, + device); + } + if (ACPI_FAILURE(status)) { + error = -ENODEV; + goto err_input_unregister; + } + if (button->type == ACPI_BUTTON_TYPE_LID) { /* * This assumes there's only one lid device, or if there are @@ -580,11 +608,13 @@ static int acpi_button_add(struct acpi_d pr_info("%s [%s]\n", name, acpi_device_bid(device)); return 0; - err_remove_fs: +err_input_unregister: + input_unregister_device(input); +err_remove_fs: acpi_button_remove_fs(device); - err_free_input: +err_free_input: input_free_device(input); - err_free_button: +err_free_button: kfree(button); return error; } @@ -593,6 +623,18 @@ static void acpi_button_remove(struct ac { struct acpi_button *button = acpi_driver_data(device); + if (device->device_type == ACPI_BUS_TYPE_POWER_BUTTON) { + acpi_remove_fixed_event_handler(ACPI_EVENT_POWER_BUTTON, + acpi_button_event); + } else if (device->device_type == ACPI_BUS_TYPE_SLEEP_BUTTON) { + acpi_remove_fixed_event_handler(ACPI_EVENT_SLEEP_BUTTON, + acpi_button_event); + } else { + acpi_remove_notify_handler(device->handle, ACPI_DEVICE_NOTIFY, + acpi_button_notify); + } + acpi_os_wait_events_complete(); + acpi_button_remove_fs(device); input_unregister_device(button->input); kfree(button); Index: linux-pm/drivers/acpi/bus.c =================================================================== --- linux-pm.orig/drivers/acpi/bus.c +++ linux-pm/drivers/acpi/bus.c @@ -530,65 +530,30 @@ static void acpi_notify_device(acpi_hand acpi_drv->ops.notify(device, event); } -static void acpi_notify_device_fixed(void *data) -{ - struct acpi_device *device = data; - - /* Fixed hardware devices have no handles */ - acpi_notify_device(NULL, ACPI_FIXED_HARDWARE_EVENT, device); -} - -static u32 acpi_device_fixed_event(void *data) -{ - acpi_os_execute(OSL_NOTIFY_HANDLER, acpi_notify_device_fixed, data); - return ACPI_INTERRUPT_HANDLED; -} - static int acpi_device_install_notify_handler(struct acpi_device *device, struct acpi_driver *acpi_drv) { - acpi_status status; - - if (device->device_type == ACPI_BUS_TYPE_POWER_BUTTON) { - status = - acpi_install_fixed_event_handler(ACPI_EVENT_POWER_BUTTON, - acpi_device_fixed_event, - device); - } else if (device->device_type == ACPI_BUS_TYPE_SLEEP_BUTTON) { - status = - acpi_install_fixed_event_handler(ACPI_EVENT_SLEEP_BUTTON, - acpi_device_fixed_event, - device); - } else { - u32 type = acpi_drv->flags & ACPI_DRIVER_ALL_NOTIFY_EVENTS ? + u32 type = acpi_drv->flags & ACPI_DRIVER_ALL_NOTIFY_EVENTS ? ACPI_ALL_NOTIFY : ACPI_DEVICE_NOTIFY; + acpi_status status; - status = acpi_install_notify_handler(device->handle, type, - acpi_notify_device, - device); - } - + status = acpi_install_notify_handler(device->handle, type, + acpi_notify_device, device); if (ACPI_FAILURE(status)) return -EINVAL; + return 0; } static void acpi_device_remove_notify_handler(struct acpi_device *device, struct acpi_driver *acpi_drv) { - if (device->device_type == ACPI_BUS_TYPE_POWER_BUTTON) { - acpi_remove_fixed_event_handler(ACPI_EVENT_POWER_BUTTON, - acpi_device_fixed_event); - } else if (device->device_type == ACPI_BUS_TYPE_SLEEP_BUTTON) { - acpi_remove_fixed_event_handler(ACPI_EVENT_SLEEP_BUTTON, - acpi_device_fixed_event); - } else { - u32 type = acpi_drv->flags & ACPI_DRIVER_ALL_NOTIFY_EVENTS ? + u32 type = acpi_drv->flags & ACPI_DRIVER_ALL_NOTIFY_EVENTS ? ACPI_ALL_NOTIFY : ACPI_DEVICE_NOTIFY; - acpi_remove_notify_handler(device->handle, type, - acpi_notify_device); - } + acpi_remove_notify_handler(device->handle, type, + acpi_notify_device); + acpi_os_wait_events_complete(); }
On 6/2/2023 8:34 PM, Rafael J. Wysocki wrote: > On Thursday, June 1, 2023 3:17:19 PM CEST Michal Wilczynski wrote: >> Currently acpi_device_install_notify_handler() and >> acpi_device_remove_notify_handler() always install acpi_notify_device() >> as a function handler, and only then the real .notify callback gets >> called. This is not efficient and doesn't provide any real advantage. >> >> Introduce new acpi_device_install_event_handler() and >> acpi_device_remove_event_handler(). Those functions are replacing old >> installers, and after all drivers switch to the new model, old installers >> will be removed at the end of the patchset. >> >> Make new installer/removal function arguments to take function pointer as >> an argument instead of using .notify callback. Introduce new variable in >> struct acpi_device, as fixed events still needs to be handled by an >> intermediary that would schedule them for later execution. This is due to >> fixed hardware event handlers being executed in interrupt context. >> >> Make acpi_device_install_event_handler() and >> acpi_device_remove_event_handler() non-static, and export symbols. This >> will allow the drivers to call them directly, instead of relying on >> .notify callback. >> >> Signed-off-by: Michal Wilczynski <michal.wilczynski@intel.com> >> --- >> drivers/acpi/bus.c | 59 ++++++++++++++++++++++++++++++++++++++++- >> include/acpi/acpi_bus.h | 7 +++++ >> 2 files changed, 65 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/acpi/bus.c b/drivers/acpi/bus.c >> index d161ff707de4..cf2c2bfe29a0 100644 >> --- a/drivers/acpi/bus.c >> +++ b/drivers/acpi/bus.c >> @@ -535,7 +535,7 @@ static void acpi_notify_device_fixed(void *data) >> struct acpi_device *device = data; >> >> /* Fixed hardware devices have no handles */ >> - acpi_notify_device(NULL, ACPI_FIXED_HARDWARE_EVENT, device); >> + device->fixed_event_notify(NULL, ACPI_FIXED_HARDWARE_EVENT, device); >> } >> >> static u32 acpi_device_fixed_event(void *data) >> @@ -550,11 +550,13 @@ static int acpi_device_install_notify_handler(struct acpi_device *device, >> acpi_status status; >> >> if (device->device_type == ACPI_BUS_TYPE_POWER_BUTTON) { >> + device->fixed_event_notify = acpi_notify_device; >> status = >> acpi_install_fixed_event_handler(ACPI_EVENT_POWER_BUTTON, >> acpi_device_fixed_event, >> device); >> } else if (device->device_type == ACPI_BUS_TYPE_SLEEP_BUTTON) { >> + device->fixed_event_notify = acpi_notify_device; >> status = >> acpi_install_fixed_event_handler(ACPI_EVENT_SLEEP_BUTTON, >> acpi_device_fixed_event, >> @@ -579,9 +581,11 @@ static void acpi_device_remove_notify_handler(struct acpi_device *device, >> if (device->device_type == ACPI_BUS_TYPE_POWER_BUTTON) { >> acpi_remove_fixed_event_handler(ACPI_EVENT_POWER_BUTTON, >> acpi_device_fixed_event); >> + device->fixed_event_notify = NULL; >> } else if (device->device_type == ACPI_BUS_TYPE_SLEEP_BUTTON) { >> acpi_remove_fixed_event_handler(ACPI_EVENT_SLEEP_BUTTON, >> acpi_device_fixed_event); >> + device->fixed_event_notify = NULL; >> } else { >> u32 type = acpi_drv->flags & ACPI_DRIVER_ALL_NOTIFY_EVENTS ? >> ACPI_ALL_NOTIFY : ACPI_DEVICE_NOTIFY; >> @@ -592,6 +596,59 @@ static void acpi_device_remove_notify_handler(struct acpi_device *device, >> acpi_os_wait_events_complete(); >> } >> >> +int acpi_device_install_event_handler(struct acpi_device *device, >> + u32 type, >> + void (*notify)(acpi_handle, u32, void*)) >> +{ >> + acpi_status status; >> + >> + if (!notify) >> + return -EINVAL; >> + >> + if (device->device_type == ACPI_BUS_TYPE_POWER_BUTTON) { >> + device->fixed_event_notify = notify; >> + status = >> + acpi_install_fixed_event_handler(ACPI_EVENT_POWER_BUTTON, >> + acpi_device_fixed_event, >> + device); >> + } else if (device->device_type == ACPI_BUS_TYPE_SLEEP_BUTTON) { >> + device->fixed_event_notify = notify; >> + status = >> + acpi_install_fixed_event_handler(ACPI_EVENT_SLEEP_BUTTON, >> + acpi_device_fixed_event, >> + device); >> + } else { >> + status = acpi_install_notify_handler(device->handle, type, >> + notify, >> + device); >> + } >> + >> + if (ACPI_FAILURE(status)) >> + return -EINVAL; >> + return 0; >> +} >> +EXPORT_SYMBOL(acpi_device_install_event_handler); >> + >> +void acpi_device_remove_event_handler(struct acpi_device *device, >> + u32 type, >> + void (*notify)(acpi_handle, u32, void*)) >> +{ >> + if (device->device_type == ACPI_BUS_TYPE_POWER_BUTTON) { >> + acpi_remove_fixed_event_handler(ACPI_EVENT_POWER_BUTTON, >> + acpi_device_fixed_event); >> + device->fixed_event_notify = NULL; >> + } else if (device->device_type == ACPI_BUS_TYPE_SLEEP_BUTTON) { >> + acpi_remove_fixed_event_handler(ACPI_EVENT_SLEEP_BUTTON, >> + acpi_device_fixed_event); >> + device->fixed_event_notify = NULL; >> + } else { >> + acpi_remove_notify_handler(device->handle, type, >> + notify); >> + } >> + acpi_os_wait_events_complete(); >> +} >> +EXPORT_SYMBOL(acpi_device_remove_event_handler); >> + >> /* Handle events targeting \_SB device (at present only graceful shutdown) */ >> >> #define ACPI_SB_NOTIFY_SHUTDOWN_REQUEST 0x81 >> diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h >> index a6affc0550b0..7fb411438b6f 100644 >> --- a/include/acpi/acpi_bus.h >> +++ b/include/acpi/acpi_bus.h >> @@ -387,6 +387,7 @@ struct acpi_device { >> struct list_head physical_node_list; >> struct mutex physical_node_lock; >> void (*remove)(struct acpi_device *); >> + void (*fixed_event_notify)(acpi_handle handle, u32 type, void *data); Hi, Thank for you review, > This is a rather confusing change, because ->remove() above is not a driver > callback, whereas the new one would be. > > Moreover, it is rather wasteful, because the only devices needing it are > buttons, so for all of the other ACPI device objects the new callback pointer > would always be NULL. > > Finally, it is not necessary even. I was thinking about resolving this somehow in compile-time, but I guess was a bit afraid of refactoring too much code - didn't want to break anything. > > The key observation here is that there are only 2 drivers handling power and > sleep buttons that use ACPI fixed events: the ACPI button driver (button.c in > drivers/acpi) and the "tiny power button" driver (tiny-power-button.c in > drivers/acpi). All of the other drivers don't need the "fixed event notify" > thing and these two can be modified to take care of all of it by themselves. > > So if something like the below is done prior to the rest of your series, the > rest will be about acpi_install/remove_notify_handler() only and you won't > even need the wrapper routines any more: driver may just be switched over > to using the ACPICA functions directly. Sure, will get your patch, apply it before my series and fix individual drivers to use acpica functions directly. Thank you for your help ! Regards, Michał > > [This patch is untested and is really 3 patches in one, but since I've cut it > already, I'll send it properly next week after some button driver testing.] > > --- > drivers/acpi/bus.c | 53 +++++----------------------------- > drivers/acpi/button.c | 60 +++++++++++++++++++++++++++++++++------ > drivers/acpi/tiny-power-button.c | 49 ++++++++++++++++++++++++++----- > 3 files changed, 101 insertions(+), 61 deletions(-) > > Index: linux-pm/drivers/acpi/tiny-power-button.c > =================================================================== > --- linux-pm.orig/drivers/acpi/tiny-power-button.c > +++ linux-pm/drivers/acpi/tiny-power-button.c > @@ -19,18 +19,52 @@ static const struct acpi_device_id tiny_ > }; > MODULE_DEVICE_TABLE(acpi, tiny_power_button_device_ids); > > -static int acpi_noop_add(struct acpi_device *device) > +static void acpi_tiny_power_button_notify(acpi_handle handle, u32 event, void *data) > { > - return 0; > + kill_cad_pid(power_signal, 1); > } > > -static void acpi_noop_remove(struct acpi_device *device) > +static void acpi_tiny_power_button_notify_run(void *not_used) > { > + acpi_tiny_power_button_notify(NULL, ACPI_FIXED_HARDWARE_EVENT, NULL); > } > > -static void acpi_tiny_power_button_notify(struct acpi_device *device, u32 event) > +static u32 acpi_tiny_power_button_event(void *not_used) > { > - kill_cad_pid(power_signal, 1); > + acpi_os_execute(OSL_NOTIFY_HANDLER, acpi_tiny_power_button_notify_run, NULL); > + return ACPI_INTERRUPT_HANDLED; > +} > + > +static int acpi_tiny_power_button_add(struct acpi_device *device) > +{ > + acpi_status status; > + > + if (device->device_type == ACPI_BUS_TYPE_POWER_BUTTON) { > + status = acpi_install_fixed_event_handler(ACPI_EVENT_POWER_BUTTON, > + acpi_tiny_power_button_event, > + NULL); > + } else { > + status = acpi_install_notify_handler(device->handle, > + ACPI_DEVICE_NOTIFY, > + acpi_tiny_power_button_notify, > + NULL); > + } > + if (ACPI_FAILURE(status)) > + return -ENODEV; > + > + return 0; > +} > + > +static void acpi_tiny_power_button_remove(struct acpi_device *device) > +{ > + if (device->device_type == ACPI_BUS_TYPE_POWER_BUTTON) { > + acpi_remove_fixed_event_handler(ACPI_EVENT_POWER_BUTTON, > + acpi_tiny_power_button_event); > + } else { > + acpi_remove_notify_handler(device->handle, ACPI_DEVICE_NOTIFY, > + acpi_tiny_power_button_notify); > + } > + acpi_os_wait_events_complete(); > } > > static struct acpi_driver acpi_tiny_power_button_driver = { > @@ -38,9 +72,8 @@ static struct acpi_driver acpi_tiny_powe > .class = "tiny-power-button", > .ids = tiny_power_button_device_ids, > .ops = { > - .add = acpi_noop_add, > - .remove = acpi_noop_remove, > - .notify = acpi_tiny_power_button_notify, > + .add = acpi_tiny_power_button_add, > + .remove = acpi_tiny_power_button_remove, > }, > }; > > Index: linux-pm/drivers/acpi/button.c > =================================================================== > --- linux-pm.orig/drivers/acpi/button.c > +++ linux-pm/drivers/acpi/button.c > @@ -135,7 +135,6 @@ static const struct dmi_system_id dmi_li > > static int acpi_button_add(struct acpi_device *device); > static void acpi_button_remove(struct acpi_device *device); > -static void acpi_button_notify(struct acpi_device *device, u32 event); > > #ifdef CONFIG_PM_SLEEP > static int acpi_button_suspend(struct device *dev); > @@ -153,7 +152,6 @@ static struct acpi_driver acpi_button_dr > .ops = { > .add = acpi_button_add, > .remove = acpi_button_remove, > - .notify = acpi_button_notify, > }, > .drv.pm = &acpi_button_pm, > }; > @@ -409,15 +407,13 @@ static void acpi_lid_initialize_state(st > button->lid_state_initialized = true; > } > > -static void acpi_button_notify(struct acpi_device *device, u32 event) > +static void acpi_button_notify(acpi_handle handle, u32 event, void *data) > { > + struct acpi_device *device = data; > struct acpi_button *button = acpi_driver_data(device); > struct input_dev *input; > > switch (event) { > - case ACPI_FIXED_HARDWARE_EVENT: > - event = ACPI_BUTTON_NOTIFY_STATUS; > - fallthrough; > case ACPI_BUTTON_NOTIFY_STATUS: > input = button->input; > if (button->type == ACPI_BUTTON_TYPE_LID) { > @@ -450,6 +446,17 @@ static void acpi_button_notify(struct ac > } > } > > +static void acpi_button_notify_run(void *data) > +{ > + acpi_button_notify(NULL, ACPI_BUTTON_NOTIFY_STATUS, data); > +} > + > +static u32 acpi_button_event(void *data) > +{ > + acpi_os_execute(OSL_NOTIFY_HANDLER, acpi_button_notify_run, data); > + return ACPI_INTERRUPT_HANDLED; > +} > + > #ifdef CONFIG_PM_SLEEP > static int acpi_button_suspend(struct device *dev) > { > @@ -492,6 +499,7 @@ static int acpi_button_add(struct acpi_d > struct acpi_button *button; > struct input_dev *input; > const char *hid = acpi_device_hid(device); > + acpi_status status; > char *name, *class; > int error; > > @@ -568,6 +576,26 @@ static int acpi_button_add(struct acpi_d > error = input_register_device(input); > if (error) > goto err_remove_fs; > + > + if (device->device_type == ACPI_BUS_TYPE_POWER_BUTTON) { > + status = acpi_install_fixed_event_handler(ACPI_EVENT_POWER_BUTTON, > + acpi_button_event, > + device); > + } else if (device->device_type == ACPI_BUS_TYPE_SLEEP_BUTTON) { > + status = acpi_install_fixed_event_handler(ACPI_EVENT_SLEEP_BUTTON, > + acpi_button_event, > + device); > + } else { > + status = acpi_install_notify_handler(device->handle, > + ACPI_DEVICE_NOTIFY, > + acpi_button_notify, > + device); > + } > + if (ACPI_FAILURE(status)) { > + error = -ENODEV; > + goto err_input_unregister; > + } > + > if (button->type == ACPI_BUTTON_TYPE_LID) { > /* > * This assumes there's only one lid device, or if there are > @@ -580,11 +608,13 @@ static int acpi_button_add(struct acpi_d > pr_info("%s [%s]\n", name, acpi_device_bid(device)); > return 0; > > - err_remove_fs: > +err_input_unregister: > + input_unregister_device(input); > +err_remove_fs: > acpi_button_remove_fs(device); > - err_free_input: > +err_free_input: > input_free_device(input); > - err_free_button: > +err_free_button: > kfree(button); > return error; > } > @@ -593,6 +623,18 @@ static void acpi_button_remove(struct ac > { > struct acpi_button *button = acpi_driver_data(device); > > + if (device->device_type == ACPI_BUS_TYPE_POWER_BUTTON) { > + acpi_remove_fixed_event_handler(ACPI_EVENT_POWER_BUTTON, > + acpi_button_event); > + } else if (device->device_type == ACPI_BUS_TYPE_SLEEP_BUTTON) { > + acpi_remove_fixed_event_handler(ACPI_EVENT_SLEEP_BUTTON, > + acpi_button_event); > + } else { > + acpi_remove_notify_handler(device->handle, ACPI_DEVICE_NOTIFY, > + acpi_button_notify); > + } > + acpi_os_wait_events_complete(); > + > acpi_button_remove_fs(device); > input_unregister_device(button->input); > kfree(button); > Index: linux-pm/drivers/acpi/bus.c > =================================================================== > --- linux-pm.orig/drivers/acpi/bus.c > +++ linux-pm/drivers/acpi/bus.c > @@ -530,65 +530,30 @@ static void acpi_notify_device(acpi_hand > acpi_drv->ops.notify(device, event); > } > > -static void acpi_notify_device_fixed(void *data) > -{ > - struct acpi_device *device = data; > - > - /* Fixed hardware devices have no handles */ > - acpi_notify_device(NULL, ACPI_FIXED_HARDWARE_EVENT, device); > -} > - > -static u32 acpi_device_fixed_event(void *data) > -{ > - acpi_os_execute(OSL_NOTIFY_HANDLER, acpi_notify_device_fixed, data); > - return ACPI_INTERRUPT_HANDLED; > -} > - > static int acpi_device_install_notify_handler(struct acpi_device *device, > struct acpi_driver *acpi_drv) > { > - acpi_status status; > - > - if (device->device_type == ACPI_BUS_TYPE_POWER_BUTTON) { > - status = > - acpi_install_fixed_event_handler(ACPI_EVENT_POWER_BUTTON, > - acpi_device_fixed_event, > - device); > - } else if (device->device_type == ACPI_BUS_TYPE_SLEEP_BUTTON) { > - status = > - acpi_install_fixed_event_handler(ACPI_EVENT_SLEEP_BUTTON, > - acpi_device_fixed_event, > - device); > - } else { > - u32 type = acpi_drv->flags & ACPI_DRIVER_ALL_NOTIFY_EVENTS ? > + u32 type = acpi_drv->flags & ACPI_DRIVER_ALL_NOTIFY_EVENTS ? > ACPI_ALL_NOTIFY : ACPI_DEVICE_NOTIFY; > + acpi_status status; > > - status = acpi_install_notify_handler(device->handle, type, > - acpi_notify_device, > - device); > - } > - > + status = acpi_install_notify_handler(device->handle, type, > + acpi_notify_device, device); > if (ACPI_FAILURE(status)) > return -EINVAL; > + > return 0; > } > > static void acpi_device_remove_notify_handler(struct acpi_device *device, > struct acpi_driver *acpi_drv) > { > - if (device->device_type == ACPI_BUS_TYPE_POWER_BUTTON) { > - acpi_remove_fixed_event_handler(ACPI_EVENT_POWER_BUTTON, > - acpi_device_fixed_event); > - } else if (device->device_type == ACPI_BUS_TYPE_SLEEP_BUTTON) { > - acpi_remove_fixed_event_handler(ACPI_EVENT_SLEEP_BUTTON, > - acpi_device_fixed_event); > - } else { > - u32 type = acpi_drv->flags & ACPI_DRIVER_ALL_NOTIFY_EVENTS ? > + u32 type = acpi_drv->flags & ACPI_DRIVER_ALL_NOTIFY_EVENTS ? > ACPI_ALL_NOTIFY : ACPI_DEVICE_NOTIFY; > > - acpi_remove_notify_handler(device->handle, type, > - acpi_notify_device); > - } > + acpi_remove_notify_handler(device->handle, type, > + acpi_notify_device); > + > acpi_os_wait_events_complete(); > } > > > >
On Mon, Jun 5, 2023 at 10:44 AM Wilczynski, Michal <michal.wilczynski@intel.com> wrote: > > On 6/2/2023 8:34 PM, Rafael J. Wysocki wrote: > > On Thursday, June 1, 2023 3:17:19 PM CEST Michal Wilczynski wrote: > >> Currently acpi_device_install_notify_handler() and > >> acpi_device_remove_notify_handler() always install acpi_notify_device() > >> as a function handler, and only then the real .notify callback gets > >> called. This is not efficient and doesn't provide any real advantage. > >> > >> Introduce new acpi_device_install_event_handler() and > >> acpi_device_remove_event_handler(). Those functions are replacing old > >> installers, and after all drivers switch to the new model, old installers > >> will be removed at the end of the patchset. > >> > >> Make new installer/removal function arguments to take function pointer as > >> an argument instead of using .notify callback. Introduce new variable in > >> struct acpi_device, as fixed events still needs to be handled by an > >> intermediary that would schedule them for later execution. This is due to > >> fixed hardware event handlers being executed in interrupt context. > >> > >> Make acpi_device_install_event_handler() and > >> acpi_device_remove_event_handler() non-static, and export symbols. This > >> will allow the drivers to call them directly, instead of relying on > >> .notify callback. > >> > >> Signed-off-by: Michal Wilczynski <michal.wilczynski@intel.com> > >> --- > >> drivers/acpi/bus.c | 59 ++++++++++++++++++++++++++++++++++++++++- > >> include/acpi/acpi_bus.h | 7 +++++ > >> 2 files changed, 65 insertions(+), 1 deletion(-) > >> > >> diff --git a/drivers/acpi/bus.c b/drivers/acpi/bus.c > >> index d161ff707de4..cf2c2bfe29a0 100644 > >> --- a/drivers/acpi/bus.c > >> +++ b/drivers/acpi/bus.c > >> @@ -535,7 +535,7 @@ static void acpi_notify_device_fixed(void *data) > >> struct acpi_device *device = data; > >> > >> /* Fixed hardware devices have no handles */ > >> - acpi_notify_device(NULL, ACPI_FIXED_HARDWARE_EVENT, device); > >> + device->fixed_event_notify(NULL, ACPI_FIXED_HARDWARE_EVENT, device); > >> } > >> > >> static u32 acpi_device_fixed_event(void *data) > >> @@ -550,11 +550,13 @@ static int acpi_device_install_notify_handler(struct acpi_device *device, > >> acpi_status status; > >> > >> if (device->device_type == ACPI_BUS_TYPE_POWER_BUTTON) { > >> + device->fixed_event_notify = acpi_notify_device; > >> status = > >> acpi_install_fixed_event_handler(ACPI_EVENT_POWER_BUTTON, > >> acpi_device_fixed_event, > >> device); > >> } else if (device->device_type == ACPI_BUS_TYPE_SLEEP_BUTTON) { > >> + device->fixed_event_notify = acpi_notify_device; > >> status = > >> acpi_install_fixed_event_handler(ACPI_EVENT_SLEEP_BUTTON, > >> acpi_device_fixed_event, > >> @@ -579,9 +581,11 @@ static void acpi_device_remove_notify_handler(struct acpi_device *device, > >> if (device->device_type == ACPI_BUS_TYPE_POWER_BUTTON) { > >> acpi_remove_fixed_event_handler(ACPI_EVENT_POWER_BUTTON, > >> acpi_device_fixed_event); > >> + device->fixed_event_notify = NULL; > >> } else if (device->device_type == ACPI_BUS_TYPE_SLEEP_BUTTON) { > >> acpi_remove_fixed_event_handler(ACPI_EVENT_SLEEP_BUTTON, > >> acpi_device_fixed_event); > >> + device->fixed_event_notify = NULL; > >> } else { > >> u32 type = acpi_drv->flags & ACPI_DRIVER_ALL_NOTIFY_EVENTS ? > >> ACPI_ALL_NOTIFY : ACPI_DEVICE_NOTIFY; > >> @@ -592,6 +596,59 @@ static void acpi_device_remove_notify_handler(struct acpi_device *device, > >> acpi_os_wait_events_complete(); > >> } > >> > >> +int acpi_device_install_event_handler(struct acpi_device *device, > >> + u32 type, > >> + void (*notify)(acpi_handle, u32, void*)) > >> +{ > >> + acpi_status status; > >> + > >> + if (!notify) > >> + return -EINVAL; > >> + > >> + if (device->device_type == ACPI_BUS_TYPE_POWER_BUTTON) { > >> + device->fixed_event_notify = notify; > >> + status = > >> + acpi_install_fixed_event_handler(ACPI_EVENT_POWER_BUTTON, > >> + acpi_device_fixed_event, > >> + device); > >> + } else if (device->device_type == ACPI_BUS_TYPE_SLEEP_BUTTON) { > >> + device->fixed_event_notify = notify; > >> + status = > >> + acpi_install_fixed_event_handler(ACPI_EVENT_SLEEP_BUTTON, > >> + acpi_device_fixed_event, > >> + device); > >> + } else { > >> + status = acpi_install_notify_handler(device->handle, type, > >> + notify, > >> + device); > >> + } > >> + > >> + if (ACPI_FAILURE(status)) > >> + return -EINVAL; > >> + return 0; > >> +} > >> +EXPORT_SYMBOL(acpi_device_install_event_handler); > >> + > >> +void acpi_device_remove_event_handler(struct acpi_device *device, > >> + u32 type, > >> + void (*notify)(acpi_handle, u32, void*)) > >> +{ > >> + if (device->device_type == ACPI_BUS_TYPE_POWER_BUTTON) { > >> + acpi_remove_fixed_event_handler(ACPI_EVENT_POWER_BUTTON, > >> + acpi_device_fixed_event); > >> + device->fixed_event_notify = NULL; > >> + } else if (device->device_type == ACPI_BUS_TYPE_SLEEP_BUTTON) { > >> + acpi_remove_fixed_event_handler(ACPI_EVENT_SLEEP_BUTTON, > >> + acpi_device_fixed_event); > >> + device->fixed_event_notify = NULL; > >> + } else { > >> + acpi_remove_notify_handler(device->handle, type, > >> + notify); > >> + } > >> + acpi_os_wait_events_complete(); > >> +} > >> +EXPORT_SYMBOL(acpi_device_remove_event_handler); > >> + > >> /* Handle events targeting \_SB device (at present only graceful shutdown) */ > >> > >> #define ACPI_SB_NOTIFY_SHUTDOWN_REQUEST 0x81 > >> diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h > >> index a6affc0550b0..7fb411438b6f 100644 > >> --- a/include/acpi/acpi_bus.h > >> +++ b/include/acpi/acpi_bus.h > >> @@ -387,6 +387,7 @@ struct acpi_device { > >> struct list_head physical_node_list; > >> struct mutex physical_node_lock; > >> void (*remove)(struct acpi_device *); > >> + void (*fixed_event_notify)(acpi_handle handle, u32 type, void *data); > > > Hi, > Thank for you review, > > > This is a rather confusing change, because ->remove() above is not a driver > > callback, whereas the new one would be. > > > > Moreover, it is rather wasteful, because the only devices needing it are > > buttons, so for all of the other ACPI device objects the new callback pointer > > would always be NULL. > > > > Finally, it is not necessary even. > > I was thinking about resolving this somehow in compile-time, but I guess was a bit > afraid of refactoring too much code - didn't want to break anything. > > > > > The key observation here is that there are only 2 drivers handling power and > > sleep buttons that use ACPI fixed events: the ACPI button driver (button.c in > > drivers/acpi) and the "tiny power button" driver (tiny-power-button.c in > > drivers/acpi). All of the other drivers don't need the "fixed event notify" > > thing and these two can be modified to take care of all of it by themselves. > > > > So if something like the below is done prior to the rest of your series, the > > rest will be about acpi_install/remove_notify_handler() only and you won't > > even need the wrapper routines any more: driver may just be switched over > > to using the ACPICA functions directly. > > Sure, will get your patch, apply it before my series and fix individual drivers to use acpica > functions directly. I have posted this series which replaces it in the meantime: https://lore.kernel.org/linux-acpi/1847933.atdPhlSkOF@kreacher Moreover, I think that there's still a reason to use the wrappers. Namely, the unregistration part needs to call acpi_os_wait_events_complete() after the notify handler has been unregistered and it's better to avoid code duplication related to that. Also the registration wrapper can be something like: int acpi_dev_install_notify_handler(struct acpi_device *adev, acpi_notify_handler handler, u32 handler_type) { if (ACPI_FAILURE(acpi_install_notify_handler(adev->handle, handler_type, handler, adev))) return -ENODEV; return 0; } which would be simpler to use than the "raw" acpi_install_notify_handler() and using it would avoid a tiny bit of code duplication IMV.
diff --git a/drivers/acpi/bus.c b/drivers/acpi/bus.c index d161ff707de4..cf2c2bfe29a0 100644 --- a/drivers/acpi/bus.c +++ b/drivers/acpi/bus.c @@ -535,7 +535,7 @@ static void acpi_notify_device_fixed(void *data) struct acpi_device *device = data; /* Fixed hardware devices have no handles */ - acpi_notify_device(NULL, ACPI_FIXED_HARDWARE_EVENT, device); + device->fixed_event_notify(NULL, ACPI_FIXED_HARDWARE_EVENT, device); } static u32 acpi_device_fixed_event(void *data) @@ -550,11 +550,13 @@ static int acpi_device_install_notify_handler(struct acpi_device *device, acpi_status status; if (device->device_type == ACPI_BUS_TYPE_POWER_BUTTON) { + device->fixed_event_notify = acpi_notify_device; status = acpi_install_fixed_event_handler(ACPI_EVENT_POWER_BUTTON, acpi_device_fixed_event, device); } else if (device->device_type == ACPI_BUS_TYPE_SLEEP_BUTTON) { + device->fixed_event_notify = acpi_notify_device; status = acpi_install_fixed_event_handler(ACPI_EVENT_SLEEP_BUTTON, acpi_device_fixed_event, @@ -579,9 +581,11 @@ static void acpi_device_remove_notify_handler(struct acpi_device *device, if (device->device_type == ACPI_BUS_TYPE_POWER_BUTTON) { acpi_remove_fixed_event_handler(ACPI_EVENT_POWER_BUTTON, acpi_device_fixed_event); + device->fixed_event_notify = NULL; } else if (device->device_type == ACPI_BUS_TYPE_SLEEP_BUTTON) { acpi_remove_fixed_event_handler(ACPI_EVENT_SLEEP_BUTTON, acpi_device_fixed_event); + device->fixed_event_notify = NULL; } else { u32 type = acpi_drv->flags & ACPI_DRIVER_ALL_NOTIFY_EVENTS ? ACPI_ALL_NOTIFY : ACPI_DEVICE_NOTIFY; @@ -592,6 +596,59 @@ static void acpi_device_remove_notify_handler(struct acpi_device *device, acpi_os_wait_events_complete(); } +int acpi_device_install_event_handler(struct acpi_device *device, + u32 type, + void (*notify)(acpi_handle, u32, void*)) +{ + acpi_status status; + + if (!notify) + return -EINVAL; + + if (device->device_type == ACPI_BUS_TYPE_POWER_BUTTON) { + device->fixed_event_notify = notify; + status = + acpi_install_fixed_event_handler(ACPI_EVENT_POWER_BUTTON, + acpi_device_fixed_event, + device); + } else if (device->device_type == ACPI_BUS_TYPE_SLEEP_BUTTON) { + device->fixed_event_notify = notify; + status = + acpi_install_fixed_event_handler(ACPI_EVENT_SLEEP_BUTTON, + acpi_device_fixed_event, + device); + } else { + status = acpi_install_notify_handler(device->handle, type, + notify, + device); + } + + if (ACPI_FAILURE(status)) + return -EINVAL; + return 0; +} +EXPORT_SYMBOL(acpi_device_install_event_handler); + +void acpi_device_remove_event_handler(struct acpi_device *device, + u32 type, + void (*notify)(acpi_handle, u32, void*)) +{ + if (device->device_type == ACPI_BUS_TYPE_POWER_BUTTON) { + acpi_remove_fixed_event_handler(ACPI_EVENT_POWER_BUTTON, + acpi_device_fixed_event); + device->fixed_event_notify = NULL; + } else if (device->device_type == ACPI_BUS_TYPE_SLEEP_BUTTON) { + acpi_remove_fixed_event_handler(ACPI_EVENT_SLEEP_BUTTON, + acpi_device_fixed_event); + device->fixed_event_notify = NULL; + } else { + acpi_remove_notify_handler(device->handle, type, + notify); + } + acpi_os_wait_events_complete(); +} +EXPORT_SYMBOL(acpi_device_remove_event_handler); + /* Handle events targeting \_SB device (at present only graceful shutdown) */ #define ACPI_SB_NOTIFY_SHUTDOWN_REQUEST 0x81 diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h index a6affc0550b0..7fb411438b6f 100644 --- a/include/acpi/acpi_bus.h +++ b/include/acpi/acpi_bus.h @@ -387,6 +387,7 @@ struct acpi_device { struct list_head physical_node_list; struct mutex physical_node_lock; void (*remove)(struct acpi_device *); + void (*fixed_event_notify)(acpi_handle handle, u32 type, void *data); }; /* Non-device subnode */ @@ -513,6 +514,12 @@ void acpi_bus_private_data_handler(acpi_handle, void *); int acpi_bus_get_private_data(acpi_handle, void **); int acpi_bus_attach_private_data(acpi_handle, void *); void acpi_bus_detach_private_data(acpi_handle); +int acpi_device_install_event_handler(struct acpi_device *device, + u32 type, + void (*notify)(acpi_handle, u32, void*)); +void acpi_device_remove_event_handler(struct acpi_device *device, + u32 type, + void (*notify)(acpi_handle, u32, void*)); extern int acpi_notifier_call_chain(struct acpi_device *, u32, u32); extern int register_acpi_notifier(struct notifier_block *); extern int unregister_acpi_notifier(struct notifier_block *);
Currently acpi_device_install_notify_handler() and acpi_device_remove_notify_handler() always install acpi_notify_device() as a function handler, and only then the real .notify callback gets called. This is not efficient and doesn't provide any real advantage. Introduce new acpi_device_install_event_handler() and acpi_device_remove_event_handler(). Those functions are replacing old installers, and after all drivers switch to the new model, old installers will be removed at the end of the patchset. Make new installer/removal function arguments to take function pointer as an argument instead of using .notify callback. Introduce new variable in struct acpi_device, as fixed events still needs to be handled by an intermediary that would schedule them for later execution. This is due to fixed hardware event handlers being executed in interrupt context. Make acpi_device_install_event_handler() and acpi_device_remove_event_handler() non-static, and export symbols. This will allow the drivers to call them directly, instead of relying on .notify callback. Signed-off-by: Michal Wilczynski <michal.wilczynski@intel.com> --- drivers/acpi/bus.c | 59 ++++++++++++++++++++++++++++++++++++++++- include/acpi/acpi_bus.h | 7 +++++ 2 files changed, 65 insertions(+), 1 deletion(-)