From patchwork Sun Mar 7 22:22:38 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Paul Cercueil X-Patchwork-Id: 12121127 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-13.8 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 0F3E4C433E9 for ; Sun, 7 Mar 2021 22:23:59 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id D7AE465151 for ; Sun, 7 Mar 2021 22:23:58 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S231265AbhCGWX0 (ORCPT ); Sun, 7 Mar 2021 17:23:26 -0500 Received: from aposti.net ([89.234.176.197]:34036 "EHLO aposti.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S231388AbhCGWWv (ORCPT ); Sun, 7 Mar 2021 17:22:51 -0500 From: Paul Cercueil To: Dmitry Torokhov Cc: od@zcrc.me, linux-input@vger.kernel.org, linux-kernel@vger.kernel.org, Paul Cercueil Subject: [PATCH v3 1/3] input: gpio-keys: Remove extra call to input_sync Date: Sun, 7 Mar 2021 22:22:38 +0000 Message-Id: <20210307222240.380583-1-paul@crapouillou.net> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: linux-input@vger.kernel.org The input_sync() function is already called after the loop in gpio_keys_report_state(), so it does not need to be called after each iteration within gpio_keys_gpio_report_event(). Signed-off-by: Paul Cercueil --- Notes: v2: Keep the input_sync() within gpio_keys_report_state() so that it's not called at every iteration of the loop. v3: No change drivers/input/keyboard/gpio_keys.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/input/keyboard/gpio_keys.c b/drivers/input/keyboard/gpio_keys.c index 77bac4ddf324..7fcb2c35c5cc 100644 --- a/drivers/input/keyboard/gpio_keys.c +++ b/drivers/input/keyboard/gpio_keys.c @@ -373,7 +373,6 @@ static void gpio_keys_gpio_report_event(struct gpio_button_data *bdata) } else { input_event(input, type, *bdata->code, state); } - input_sync(input); } static void gpio_keys_gpio_work_func(struct work_struct *work) @@ -382,6 +381,7 @@ static void gpio_keys_gpio_work_func(struct work_struct *work) container_of(work, struct gpio_button_data, work.work); gpio_keys_gpio_report_event(bdata); + input_sync(bdata->input); if (bdata->button->wakeup) pm_relax(bdata->input->dev.parent); From patchwork Sun Mar 7 22:22:39 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Paul Cercueil X-Patchwork-Id: 12121131 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-13.8 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 06DB3C433DB for ; Sun, 7 Mar 2021 22:23:59 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id C2F4C65147 for ; Sun, 7 Mar 2021 22:23:58 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S231584AbhCGWX1 (ORCPT ); Sun, 7 Mar 2021 17:23:27 -0500 Received: from aposti.net ([89.234.176.197]:34046 "EHLO aposti.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S231250AbhCGWW5 (ORCPT ); Sun, 7 Mar 2021 17:22:57 -0500 From: Paul Cercueil To: Dmitry Torokhov Cc: od@zcrc.me, linux-input@vger.kernel.org, linux-kernel@vger.kernel.org, Paul Cercueil Subject: [PATCH v3 2/3] input: gpio-keys: Use hrtimer for release timer Date: Sun, 7 Mar 2021 22:22:39 +0000 Message-Id: <20210307222240.380583-2-paul@crapouillou.net> In-Reply-To: <20210307222240.380583-1-paul@crapouillou.net> References: <20210307222240.380583-1-paul@crapouillou.net> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: linux-input@vger.kernel.org Dealing with input, timing is important; if the button should be released in one millisecond, then it should be done in one millisecond and not a hundred milliseconds. Therefore, the standard timer API is not really suitable for this task. Convert the gpio-keys driver to use a hrtimer instead of the standard timer to address this issue. Note that by using a hard IRQ for the hrtimer callback, we can get rid of the spin_lock_irqsave() and spin_unlock_irqrestore(). Signed-off-by: Paul Cercueil --- Notes: v2-v3: No change drivers/input/keyboard/gpio_keys.c | 27 ++++++++++++++++----------- 1 file changed, 16 insertions(+), 11 deletions(-) diff --git a/drivers/input/keyboard/gpio_keys.c b/drivers/input/keyboard/gpio_keys.c index 7fcb2c35c5cc..4b92f49decef 100644 --- a/drivers/input/keyboard/gpio_keys.c +++ b/drivers/input/keyboard/gpio_keys.c @@ -8,6 +8,7 @@ #include +#include #include #include #include @@ -36,7 +37,7 @@ struct gpio_button_data { unsigned short *code; - struct timer_list release_timer; + struct hrtimer release_timer; unsigned int release_delay; /* in msecs, for IRQ-only buttons */ struct delayed_work work; @@ -146,7 +147,7 @@ static void gpio_keys_disable_button(struct gpio_button_data *bdata) if (bdata->gpiod) cancel_delayed_work_sync(&bdata->work); else - del_timer_sync(&bdata->release_timer); + hrtimer_cancel(&bdata->release_timer); bdata->disabled = true; } @@ -415,19 +416,20 @@ static irqreturn_t gpio_keys_gpio_isr(int irq, void *dev_id) return IRQ_HANDLED; } -static void gpio_keys_irq_timer(struct timer_list *t) +static enum hrtimer_restart gpio_keys_irq_timer(struct hrtimer *t) { - struct gpio_button_data *bdata = from_timer(bdata, t, release_timer); + struct gpio_button_data *bdata = container_of(t, + struct gpio_button_data, + release_timer); struct input_dev *input = bdata->input; - unsigned long flags; - spin_lock_irqsave(&bdata->lock, flags); if (bdata->key_pressed) { input_event(input, EV_KEY, *bdata->code, 0); input_sync(input); bdata->key_pressed = false; } - spin_unlock_irqrestore(&bdata->lock, flags); + + return HRTIMER_NORESTART; } static irqreturn_t gpio_keys_irq_isr(int irq, void *dev_id) @@ -457,8 +459,9 @@ static irqreturn_t gpio_keys_irq_isr(int irq, void *dev_id) } if (bdata->release_delay) - mod_timer(&bdata->release_timer, - jiffies + msecs_to_jiffies(bdata->release_delay)); + hrtimer_start(&bdata->release_timer, + ms_to_ktime(bdata->release_delay), + HRTIMER_MODE_REL_HARD); out: spin_unlock_irqrestore(&bdata->lock, flags); return IRQ_HANDLED; @@ -471,7 +474,7 @@ static void gpio_keys_quiesce_key(void *data) if (bdata->gpiod) cancel_delayed_work_sync(&bdata->work); else - del_timer_sync(&bdata->release_timer); + hrtimer_cancel(&bdata->release_timer); } static int gpio_keys_setup_key(struct platform_device *pdev, @@ -595,7 +598,9 @@ static int gpio_keys_setup_key(struct platform_device *pdev, } bdata->release_delay = button->debounce_interval; - timer_setup(&bdata->release_timer, gpio_keys_irq_timer, 0); + hrtimer_init(&bdata->release_timer, + CLOCK_REALTIME, HRTIMER_MODE_REL_HARD); + bdata->release_timer.function = gpio_keys_irq_timer; isr = gpio_keys_irq_isr; irqflags = 0; From patchwork Sun Mar 7 22:22:40 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Paul Cercueil X-Patchwork-Id: 12121129 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-13.8 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id E44D2C433E0 for ; Sun, 7 Mar 2021 22:23:58 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id B0617650ED for ; Sun, 7 Mar 2021 22:23:58 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S231625AbhCGWX1 (ORCPT ); Sun, 7 Mar 2021 17:23:27 -0500 Received: from aposti.net ([89.234.176.197]:34054 "EHLO aposti.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S231405AbhCGWXD (ORCPT ); Sun, 7 Mar 2021 17:23:03 -0500 From: Paul Cercueil To: Dmitry Torokhov Cc: od@zcrc.me, linux-input@vger.kernel.org, linux-kernel@vger.kernel.org, Paul Cercueil Subject: [PATCH v3 3/3] input: gpio-keys: Use hrtimer for software debounce, if possible Date: Sun, 7 Mar 2021 22:22:40 +0000 Message-Id: <20210307222240.380583-3-paul@crapouillou.net> In-Reply-To: <20210307222240.380583-1-paul@crapouillou.net> References: <20210307222240.380583-1-paul@crapouillou.net> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: linux-input@vger.kernel.org We want to be able to report the input event as soon as the debounce delay elapsed. However, the current code does not really ensure that, as it uses the jiffies-based schedule_delayed_work() API. With a small enough HZ value (HZ <= 100), this results in some input events being lost, when a key is quickly pressed then released (on a human's time scale). Switching to hrtimers fixes this issue, and will work even on extremely low HZ values (tested at HZ=24). This is however only possible if reading the GPIO is possible without sleeping. If this condition is not met, the previous approach of using a jiffies-based timer is taken. Signed-off-by: Paul Cercueil --- Notes: v2: HRTIMER_MODE_REL_SOFT -> HRTIMER_MODE_REL v3: Only use a hrtimer-based timer if we know that reading the GPIO will never sleep. drivers/input/keyboard/gpio_keys.c | 69 ++++++++++++++++++++++++------ 1 file changed, 56 insertions(+), 13 deletions(-) diff --git a/drivers/input/keyboard/gpio_keys.c b/drivers/input/keyboard/gpio_keys.c index 4b92f49decef..046d9dffa171 100644 --- a/drivers/input/keyboard/gpio_keys.c +++ b/drivers/input/keyboard/gpio_keys.c @@ -41,6 +41,7 @@ struct gpio_button_data { unsigned int release_delay; /* in msecs, for IRQ-only buttons */ struct delayed_work work; + struct hrtimer debounce_timer; unsigned int software_debounce; /* in msecs, for GPIO-driven buttons */ unsigned int irq; @@ -49,6 +50,7 @@ struct gpio_button_data { bool disabled; bool key_pressed; bool suspended; + bool debounce_use_hrtimer; }; struct gpio_keys_drvdata { @@ -144,10 +146,12 @@ static void gpio_keys_disable_button(struct gpio_button_data *bdata) */ disable_irq(bdata->irq); - if (bdata->gpiod) - cancel_delayed_work_sync(&bdata->work); - else + if (!bdata->gpiod) hrtimer_cancel(&bdata->release_timer); + else if (bdata->debounce_use_hrtimer) + hrtimer_cancel(&bdata->debounce_timer); + else + cancel_delayed_work_sync(&bdata->work); bdata->disabled = true; } @@ -361,7 +365,10 @@ static void gpio_keys_gpio_report_event(struct gpio_button_data *bdata) unsigned int type = button->type ?: EV_KEY; int state; - state = gpiod_get_value_cansleep(bdata->gpiod); + if (bdata->debounce_use_hrtimer) + state = gpiod_get_value(bdata->gpiod); + else + state = gpiod_get_value_cansleep(bdata->gpiod); if (state < 0) { dev_err(input->dev.parent, "failed to get gpio state: %d\n", state); @@ -376,11 +383,8 @@ static void gpio_keys_gpio_report_event(struct gpio_button_data *bdata) } } -static void gpio_keys_gpio_work_func(struct work_struct *work) +static void gpio_keys_debounce_event(struct gpio_button_data *bdata) { - struct gpio_button_data *bdata = - container_of(work, struct gpio_button_data, work.work); - gpio_keys_gpio_report_event(bdata); input_sync(bdata->input); @@ -388,6 +392,26 @@ static void gpio_keys_gpio_work_func(struct work_struct *work) pm_relax(bdata->input->dev.parent); } +static void gpio_keys_gpio_work_func(struct work_struct *work) +{ + struct gpio_button_data *bdata = container_of(work, + struct gpio_button_data, + work.work); + + gpio_keys_debounce_event(bdata); +} + +static enum hrtimer_restart gpio_keys_debounce_timer(struct hrtimer *t) +{ + struct gpio_button_data *bdata = container_of(t, + struct gpio_button_data, + debounce_timer); + + gpio_keys_debounce_event(bdata); + + return HRTIMER_NORESTART; +} + static irqreturn_t gpio_keys_gpio_isr(int irq, void *dev_id) { struct gpio_button_data *bdata = dev_id; @@ -409,9 +433,15 @@ static irqreturn_t gpio_keys_gpio_isr(int irq, void *dev_id) } } - mod_delayed_work(system_wq, - &bdata->work, - msecs_to_jiffies(bdata->software_debounce)); + if (bdata->debounce_use_hrtimer) { + hrtimer_start(&bdata->debounce_timer, + ms_to_ktime(bdata->software_debounce), + HRTIMER_MODE_REL); + } else { + mod_delayed_work(system_wq, + &bdata->work, + msecs_to_jiffies(bdata->software_debounce)); + } return IRQ_HANDLED; } @@ -471,10 +501,12 @@ static void gpio_keys_quiesce_key(void *data) { struct gpio_button_data *bdata = data; - if (bdata->gpiod) + if (bdata->gpiod) { + hrtimer_cancel(&bdata->debounce_timer); cancel_delayed_work_sync(&bdata->work); - else + } else { hrtimer_cancel(&bdata->release_timer); + } } static int gpio_keys_setup_key(struct platform_device *pdev, @@ -546,6 +578,13 @@ static int gpio_keys_setup_key(struct platform_device *pdev, if (error < 0) bdata->software_debounce = button->debounce_interval; + + /* + * If reading the GPIO won't sleep, we can use a hrtimer + * instead of a standard timer for the software + * debounce, to reduce the latency as much as possible. + */ + bdata->debounce_use_hrtimer = !gpiod_cansleep(bdata->gpiod); } if (button->irq) { @@ -564,6 +603,10 @@ static int gpio_keys_setup_key(struct platform_device *pdev, INIT_DELAYED_WORK(&bdata->work, gpio_keys_gpio_work_func); + hrtimer_init(&bdata->debounce_timer, + CLOCK_REALTIME, HRTIMER_MODE_REL); + bdata->debounce_timer.function = gpio_keys_debounce_timer; + isr = gpio_keys_gpio_isr; irqflags = IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING;