| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <1AE640813FDE7649BE1B193DEA596E886CEC614F@SHSMSX101.ccr.corp.intel.com>
Date: Mon, 5 Jun 2017 04:28:12 +0000
From: "Zheng, Lv" <lv.zheng@...el.com>
To: Benjamin Tissoires <benjamin.tissoires@...hat.com>,
"Wysocki, Rafael J" <rafael.j.wysocki@...el.com>,
"Rafael J . Wysocki" <rjw@...ysocki.net>,
"Brown, Len" <len.brown@...el.com>, Lv Zheng <zetalog@...il.com>,
"Peter Hutterer" <peter.hutterer@...-t.net>
CC: "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"linux-acpi@...r.kernel.org" <linux-acpi@...r.kernel.org>,
"systemd-devel@...ts.freedesktop.org"
<systemd-devel@...ts.freedesktop.org>,
"linux-input@...r.kernel.org" <linux-input@...r.kernel.org>
Subject: RE: [WIP PATCH 3/4] ACPI: button: Let input filter out the LID
events
Hi, Benjamin
> From: Benjamin Tissoires [mailto:benjamin.tissoires@...hat.com]
> Subject: [WIP PATCH 3/4] ACPI: button: Let input filter out the LID events
>
> The input stack already filters out the LID events. So instead of
> filtering them out at the source, we can hook up after the input
> processing and propagate the lid switch events when the input stack
> tells us to.
>
> An other benefit is that if userspace (think libinput) "fixes" the lid
> switch state by some heuristics, this new state is forwarded to the
> listeners in the kernel.
See my comments to PATCH 4.
IMO, it sounds better that
1. ACPI lid works as a driver of SW_LID, and
2. i915 registers notification (the only user) via input layer.
So it looks i915 rather than button driver should call input_register_handler().
And input layer may help to provide a simplified interface for drivers to register key notifications.
Thanks and best regards
Lv
>
> Signed-off-by: Benjamin Tissoires <benjamin.tissoires@...hat.com>
> ---
> drivers/acpi/button.c | 156 ++++++++++++++++++++++++++++++++++++++++++++------
> 1 file changed, 139 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/acpi/button.c b/drivers/acpi/button.c
> index 9ad7604..03e5981 100644
> --- a/drivers/acpi/button.c
> +++ b/drivers/acpi/button.c
> @@ -109,8 +109,6 @@ struct acpi_button {
> struct input_dev *input;
> char phys[32]; /* for input device */
> unsigned long pushed;
> - int last_state;
> - ktime_t last_time;
> bool suspended;
> };
>
> @@ -184,7 +182,6 @@ static int acpi_lid_evaluate_state(struct acpi_device *device)
> static int acpi_lid_notify_state(struct acpi_device *device, int state)
> {
> struct acpi_button *button = acpi_driver_data(device);
> - int ret;
>
> /* button_input_lock must be held */
>
> @@ -205,20 +202,129 @@ static int acpi_lid_notify_state(struct acpi_device *device, int state)
> if (state)
> pm_wakeup_hard_event(&device->dev);
>
> - ret = blocking_notifier_call_chain(&acpi_lid_notifier, state, device);
> - if (ret == NOTIFY_DONE)
> - ret = blocking_notifier_call_chain(&acpi_lid_notifier, state,
> - device);
> - if (ret == NOTIFY_DONE || ret == NOTIFY_OK) {
> - /*
> - * It is also regarded as success if the notifier_chain
> - * returns NOTIFY_OK or NOTIFY_DONE.
> - */
> - ret = 0;
> + return 0;
> +}
> +
> +/*
> + * Pass incoming event to all connected clients.
> + */
> +static void acpi_button_lid_events(struct input_handle *handle,
> + const struct input_value *vals,
> + unsigned int count)
> +{
> + const struct input_value *v;
> + int state = -1;
> + int ret;
> +
> + for (v = vals; v != vals + count; v++) {
> + switch (v->type) {
> + case EV_SYN:
> + if (v->code == SYN_REPORT && state >= 0) {
> + ret = blocking_notifier_call_chain(&acpi_lid_notifier,
> + state,
> + lid_device);
> + if (ret == NOTIFY_DONE)
> + ret = blocking_notifier_call_chain(&acpi_lid_notifier,
> + state,
> + lid_device);
> + if (ret == NOTIFY_DONE || ret == NOTIFY_OK) {
> + /*
> + * It is also regarded as success if
> + * the notifier_chain returns NOTIFY_OK
> + * or NOTIFY_DONE.
> + */
> + ret = 0;
> + }
> + }
> + break;
> + case EV_SW:
> + if (v->code == SW_LID)
> + state = !v->value;
> + break;
> + }
> }
> - return ret;
> }
>
> +static int acpi_button_lid_connect(struct input_handler *handler,
> + struct input_dev *dev,
> + const struct input_device_id *id)
> +{
> + struct input_handle *handle;
> + int error;
> +
> + handle = kzalloc(sizeof(struct input_handle), GFP_KERNEL);
> + if (!handle)
> + return -ENOMEM;
> +
> + handle->dev = dev;
> + handle->handler = handler;
> + handle->name = "acpi-button-lid";
> +
> + error = input_register_handle(handle);
> + if (error) {
> + dev_err(&lid_device->dev, "Error installing input handle\n");
> + goto err_free;
> + }
> +
> + error = input_open_device(handle);
> + if (error) {
> + dev_err(&lid_device->dev, "Failed to open input device\n");
> + goto err_unregister;
> + }
> +
> + return 0;
> +
> + err_unregister:
> + input_unregister_handle(handle);
> + err_free:
> + kfree(handle);
> + return error;
> +}
> +
> +static void acpi_button_lid_disconnect(struct input_handle *handle)
> +{
> + input_close_device(handle);
> + input_unregister_handle(handle);
> + kfree(handle);
> +}
> +
> +bool acpi_button_lid_match(struct input_handler *handler,
> + struct input_dev *dev)
> +{
> + struct acpi_button *button;
> +
> + if (!lid_device)
> + return false;
> +
> + button = acpi_driver_data(lid_device);
> +
> + if (dev != button->input)
> + return false;
> +
> + return true;
> +}
> +
> +static const struct input_device_id acpi_button_lid_ids[] = {
> + {
> + .flags = INPUT_DEVICE_ID_MATCH_EVBIT,
> + .evbit = { BIT_MASK(EV_SW) },
> + .swbit = { BIT_MASK(SW_LID) },
> + },
> + { },
> +};
> +
> +MODULE_DEVICE_TABLE(input, acpi_button_lid_ids);
> +
> +static struct input_handler acpi_button_lid_handler = {
> + .match = acpi_button_lid_match,
> + .connect = acpi_button_lid_connect,
> + .disconnect = acpi_button_lid_disconnect,
> + .events = acpi_button_lid_events,
> + .name = "acpi-lid-callchain",
> + .id_table = acpi_button_lid_ids,
> +};
> +
> +
> static int acpi_button_state_seq_show(struct seq_file *seq, void *offset)
> {
> struct acpi_device *device = seq->private;
> @@ -581,8 +687,6 @@ static int acpi_button_add(struct acpi_device *device)
> strcpy(name, ACPI_BUTTON_DEVICE_NAME_LID);
> sprintf(class, "%s/%s",
> ACPI_BUTTON_CLASS, ACPI_BUTTON_SUBCLASS_LID);
> - button->last_state = !!acpi_lid_evaluate_state(device);
> - button->last_time = ktime_get();
> } else {
> printk(KERN_ERR PREFIX "Unsupported hid [%s]\n", hid);
> error = -ENODEV;
> @@ -674,4 +778,22 @@ module_param_call(lid_init_state,
> NULL, 0644);
> MODULE_PARM_DESC(lid_init_state, "Behavior for reporting LID initial state");
>
> -module_acpi_driver(acpi_button_driver);
> +static int __init acpi_button_init(void)
> +{
> + int error;
> +
> + error = input_register_handler(&acpi_button_lid_handler);
> + if (error)
> + return error;
> +
> + return acpi_bus_register_driver(&acpi_button_driver);
> +}
> +
> +static void __exit acpi_button_exit(void)
> +{
> + acpi_bus_unregister_driver(&acpi_button_driver);
> + input_unregister_handler(&acpi_button_lid_handler);
> +}
> +
> +module_init(acpi_button_init);
> +module_exit(acpi_button_exit);
> --
> 2.9.4
Powered by blists - more mailing lists