| 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: <bb766307-2d9c-fc6d-588a-9394a2b852e8@redhat.com>
Date: Tue, 8 Oct 2019 15:03:12 +0200
From: Hans de Goede <hdegoede@...hat.com>
To: Dmitry Torokhov <dmitry.torokhov@...il.com>,
Darren Hart <dvhart@...radead.org>,
Andy Shevchenko <andy@...radead.org>
Cc: platform-driver-x86@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2] platform/x86: peaq-wmi: switch to using polled mode of
input devices
Hi,
On 01-10-2019 20:58, Dmitry Torokhov wrote:
> We have added polled mode to the normal input devices with the intent of
> retiring input_polled_dev. This converts peaq-wmi driver to use the
> polling mode of standard input devices and removes dependency on
> INPUT_POLLDEV.
>
> Because the new polling coded does not allow peeking inside the poller
> structure to get the poll interval, we change the "debounce" process to
> operate on the time basis, instead of counting events.
>
> We also fix error handling during initialization, as previously we leaked
> input device structure when we failed to register it.
>
> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@...il.com>
Patch looks good to me and I've also given this a test-run on the
hw which uses this driver:
Reviewed-by: Hans de Goede <hdegoede@...hat.com>
Tested-by: Hans de Goede <hdegoede@...hat.com>
Regards,
Hans
> ---
>
> v2: include input.h instead of input-polldev.h
>
> drivers/platform/x86/Kconfig | 1 -
> drivers/platform/x86/peaq-wmi.c | 66 +++++++++++++++++++++------------
> 2 files changed, 42 insertions(+), 25 deletions(-)
>
> diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
> index f0a93f630455..c703c78c59f3 100644
> --- a/drivers/platform/x86/Kconfig
> +++ b/drivers/platform/x86/Kconfig
> @@ -804,7 +804,6 @@ config PEAQ_WMI
> tristate "PEAQ 2-in-1 WMI hotkey driver"
> depends on ACPI_WMI
> depends on INPUT
> - select INPUT_POLLDEV
> help
> Say Y here if you want to support WMI-based hotkeys on PEAQ 2-in-1s.
>
> diff --git a/drivers/platform/x86/peaq-wmi.c b/drivers/platform/x86/peaq-wmi.c
> index fdeb3624c529..cf9c44c20a82 100644
> --- a/drivers/platform/x86/peaq-wmi.c
> +++ b/drivers/platform/x86/peaq-wmi.c
> @@ -6,7 +6,7 @@
>
> #include <linux/acpi.h>
> #include <linux/dmi.h>
> -#include <linux/input-polldev.h>
> +#include <linux/input.h>
> #include <linux/kernel.h>
> #include <linux/module.h>
>
> @@ -18,8 +18,7 @@
>
> MODULE_ALIAS("wmi:"PEAQ_DOLBY_BUTTON_GUID);
>
> -static unsigned int peaq_ignore_events_counter;
> -static struct input_polled_dev *peaq_poll_dev;
> +static struct input_dev *peaq_poll_dev;
>
> /*
> * The Dolby button (yes really a Dolby button) causes an ACPI variable to get
> @@ -28,8 +27,10 @@ static struct input_polled_dev *peaq_poll_dev;
> * (if polling after the release) or twice (polling between press and release).
> * We ignore events for 0.5s after the first event to avoid reporting 2 presses.
> */
> -static void peaq_wmi_poll(struct input_polled_dev *dev)
> +static void peaq_wmi_poll(struct input_dev *input_dev)
> {
> + static unsigned long last_event_time;
> + static bool had_events;
> union acpi_object obj;
> acpi_status status;
> u32 dummy = 0;
> @@ -44,22 +45,25 @@ static void peaq_wmi_poll(struct input_polled_dev *dev)
> return;
>
> if (obj.type != ACPI_TYPE_INTEGER) {
> - dev_err(&peaq_poll_dev->input->dev,
> + dev_err(&input_dev->dev,
> "Error WMBC did not return an integer\n");
> return;
> }
>
> - if (peaq_ignore_events_counter && peaq_ignore_events_counter--)
> + if (!obj.integer.value)
> return;
>
> - if (obj.integer.value) {
> - input_event(peaq_poll_dev->input, EV_KEY, KEY_SOUND, 1);
> - input_sync(peaq_poll_dev->input);
> - input_event(peaq_poll_dev->input, EV_KEY, KEY_SOUND, 0);
> - input_sync(peaq_poll_dev->input);
> - peaq_ignore_events_counter = max(1u,
> - PEAQ_POLL_IGNORE_MS / peaq_poll_dev->poll_interval);
> - }
> + if (had_events && time_before(jiffies, last_event_time +
> + msecs_to_jiffies(PEAQ_POLL_IGNORE_MS)))
> + return;
> +
> + input_event(input_dev, EV_KEY, KEY_SOUND, 1);
> + input_sync(input_dev);
> + input_event(input_dev, EV_KEY, KEY_SOUND, 0);
> + input_sync(input_dev);
> +
> + last_event_time = jiffies;
> + had_events = true;
> }
>
> /* Some other devices (Shuttle XS35) use the same WMI GUID for other purposes */
> @@ -75,6 +79,8 @@ static const struct dmi_system_id peaq_dmi_table[] __initconst = {
>
> static int __init peaq_wmi_init(void)
> {
> + int err;
> +
> /* WMI GUID is not unique, also check for a DMI match */
> if (!dmi_check_system(peaq_dmi_table))
> return -ENODEV;
> @@ -82,24 +88,36 @@ static int __init peaq_wmi_init(void)
> if (!wmi_has_guid(PEAQ_DOLBY_BUTTON_GUID))
> return -ENODEV;
>
> - peaq_poll_dev = input_allocate_polled_device();
> + peaq_poll_dev = input_allocate_device();
> if (!peaq_poll_dev)
> return -ENOMEM;
>
> - peaq_poll_dev->poll = peaq_wmi_poll;
> - peaq_poll_dev->poll_interval = PEAQ_POLL_INTERVAL_MS;
> - peaq_poll_dev->poll_interval_max = PEAQ_POLL_MAX_MS;
> - peaq_poll_dev->input->name = "PEAQ WMI hotkeys";
> - peaq_poll_dev->input->phys = "wmi/input0";
> - peaq_poll_dev->input->id.bustype = BUS_HOST;
> - input_set_capability(peaq_poll_dev->input, EV_KEY, KEY_SOUND);
> + peaq_poll_dev->name = "PEAQ WMI hotkeys";
> + peaq_poll_dev->phys = "wmi/input0";
> + peaq_poll_dev->id.bustype = BUS_HOST;
> + input_set_capability(peaq_poll_dev, EV_KEY, KEY_SOUND);
> +
> + err = input_setup_polling(peaq_poll_dev, peaq_wmi_poll);
> + if (err)
> + goto err_out;
> +
> + input_set_poll_interval(peaq_poll_dev, PEAQ_POLL_INTERVAL_MS);
> + input_set_max_poll_interval(peaq_poll_dev, PEAQ_POLL_MAX_MS);
> +
> + err = input_register_device(peaq_poll_dev);
> + if (err)
> + goto err_out;
> +
> + return 0;
>
> - return input_register_polled_device(peaq_poll_dev);
> +err_out:
> + input_free_device(peaq_poll_dev);
> + return err;
> }
>
> static void __exit peaq_wmi_exit(void)
> {
> - input_unregister_polled_device(peaq_poll_dev);
> + input_unregister_device(peaq_poll_dev);
> }
>
> module_init(peaq_wmi_init);
>
Powered by blists - more mailing lists