| 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: <20170606103134.GC14880@mail.corp.redhat.com>
Date: Tue, 6 Jun 2017 12:31:34 +0200
From: Benjamin Tissoires <benjamin.tissoires@...hat.com>
To: "Zheng, Lv" <lv.zheng@...el.com>,
Dmitry Torokhov <dmitry.torokhov@...il.com>
Cc: "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>,
"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
On Jun 05 2017 or thereabouts, Zheng, Lv wrote:
> 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.
Sounds good.
Dmitry, would a simplified API for other drivers to listen to input
events be something you would agree on?
Cheers,
Benjamin
>
> 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