| Message ID | 20230601131739.300760-21-michal.wilczynski@intel.com (mailing list archive) |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
| Series | Remove .notify callback in acpi_device_ops | expand |
On Thu, 1 Jun 2023, 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_device_install_event_handler() at the end of .add() callback. > Call acpi_device_remove_event_handler() at the beginning of .remove() > callback. Change arguments passed to the notify callback to match with > what's required by acpi_device_install_event_handler(). > > Signed-off-by: Michal Wilczynski <michal.wilczynski@intel.com> > --- > drivers/platform/x86/eeepc-laptop.c | 16 ++++++++++++---- > 1 file changed, 12 insertions(+), 4 deletions(-) > > diff --git a/drivers/platform/x86/eeepc-laptop.c b/drivers/platform/x86/eeepc-laptop.c > index 62b71e8e3567..bd6ada963d88 100644 > --- a/drivers/platform/x86/eeepc-laptop.c > +++ b/drivers/platform/x86/eeepc-laptop.c > @@ -1204,12 +1204,15 @@ static void eeepc_input_notify(struct eeepc_laptop *eeepc, int event) > pr_info("Unknown key %x pressed\n", event); > } > > -static void eeepc_acpi_notify(struct acpi_device *device, u32 event) > +static void eeepc_acpi_notify(acpi_handle handle, u32 event, void *data) > { > - struct eeepc_laptop *eeepc = acpi_driver_data(device); > int old_brightness, new_brightness; > + struct acpi_device *device = data; > + struct eeepc_laptop *eeepc; > u16 count; > > + eeepc = acpi_driver_data(device); > + > if (event > ACPI_MAX_SYS_NOTIFY) > return; > count = eeepc->event_count[event % 128]++; > @@ -1423,6 +1426,11 @@ static int eeepc_acpi_add(struct acpi_device *device) > goto fail_rfkill; > > eeepc_device_present = true; > + result = acpi_device_install_event_handler(device, ACPI_ALL_NOTIFY, > + eeepc_acpi_notify); > + if (result) > + goto fail_rfkill; This too is incorrectly rolled back. You shouldn't just copy the previous goto label like this but think what _more_ should be rolled back if that preceeding call succeeded. Usually, the remove function gives you good hint on what additional things should be rolled back. In here it looks like eeepc_rfkill_exit() would be needed too. > + > return 0; > > fail_rfkill: > @@ -1444,6 +1452,8 @@ static void eeepc_acpi_remove(struct acpi_device *device) > { > struct eeepc_laptop *eeepc = acpi_driver_data(device); > > + acpi_device_remove_event_handler(device, ACPI_ALL_NOTIFY, > + eeepc_acpi_notify); > eeepc_backlight_exit(eeepc); > eeepc_rfkill_exit(eeepc); > eeepc_input_exit(eeepc);
diff --git a/drivers/platform/x86/eeepc-laptop.c b/drivers/platform/x86/eeepc-laptop.c index 62b71e8e3567..bd6ada963d88 100644 --- a/drivers/platform/x86/eeepc-laptop.c +++ b/drivers/platform/x86/eeepc-laptop.c @@ -1204,12 +1204,15 @@ static void eeepc_input_notify(struct eeepc_laptop *eeepc, int event) pr_info("Unknown key %x pressed\n", event); } -static void eeepc_acpi_notify(struct acpi_device *device, u32 event) +static void eeepc_acpi_notify(acpi_handle handle, u32 event, void *data) { - struct eeepc_laptop *eeepc = acpi_driver_data(device); int old_brightness, new_brightness; + struct acpi_device *device = data; + struct eeepc_laptop *eeepc; u16 count; + eeepc = acpi_driver_data(device); + if (event > ACPI_MAX_SYS_NOTIFY) return; count = eeepc->event_count[event % 128]++; @@ -1423,6 +1426,11 @@ static int eeepc_acpi_add(struct acpi_device *device) goto fail_rfkill; eeepc_device_present = true; + result = acpi_device_install_event_handler(device, ACPI_ALL_NOTIFY, + eeepc_acpi_notify); + if (result) + goto fail_rfkill; + return 0; fail_rfkill: @@ -1444,6 +1452,8 @@ static void eeepc_acpi_remove(struct acpi_device *device) { struct eeepc_laptop *eeepc = acpi_driver_data(device); + acpi_device_remove_event_handler(device, ACPI_ALL_NOTIFY, + eeepc_acpi_notify); eeepc_backlight_exit(eeepc); eeepc_rfkill_exit(eeepc); eeepc_input_exit(eeepc); @@ -1465,11 +1475,9 @@ static struct acpi_driver eeepc_acpi_driver = { .class = EEEPC_ACPI_CLASS, .owner = THIS_MODULE, .ids = eeepc_device_ids, - .flags = ACPI_DRIVER_ALL_NOTIFY_EVENTS, .ops = { .add = eeepc_acpi_add, .remove = eeepc_acpi_remove, - .notify = eeepc_acpi_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_device_install_event_handler() at the end of .add() callback. Call acpi_device_remove_event_handler() at the beginning of .remove() callback. Change arguments passed to the notify callback to match with what's required by acpi_device_install_event_handler(). Signed-off-by: Michal Wilczynski <michal.wilczynski@intel.com> --- drivers/platform/x86/eeepc-laptop.c | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-)