Re: [PATCH]input: Change timer function to workqueue for gpio_keys driver

[Alek Du <alek.du@intel.com>]

On Thu, 25 Jun 2009 18:29:25 +0800
Jani Nikula <ext-jani.1.nikula@nokia.com> wrote:

> On Fri, Jun 12, 2009 at 8:40 PM, Trilok Soni<soni.trilok@gmail.com> wrote:
> >>  static irqreturn_t gpio_keys_isr(int irq, void *dev_id)
> >>  {
> >>        struct gpio_button_data *bdata = dev_id;
> >> @@ -62,10 +61,10 @@ static irqreturn_t gpio_keys_isr(int irq, void *dev_id)
> >>        BUG_ON(irq != gpio_to_irq(button->gpio));
> >>
> >>        if (button->debounce_interval)
> >> -               mod_timer(&bdata->timer,
> >> -                       jiffies + msecs_to_jiffies(button->debounce_interval));
> >> +               schedule_delayed_work(&bdata->work,
> >> +                       msecs_to_jiffies(button->debounce_interval));
> >>        else
> >> -               gpio_keys_report_event(bdata);
> >> +               schedule_work(&bdata->work.work);
> >>
> >>        return IRQ_HANDLED;
> >>  }
> 
> Correct me if I'm wrong, but as far as I can tell,
> schedule_delayed_work doesn't modify the timer if the work was already
> pending. The result is not the same as with the timer. This breaks the
> debouncing.

No. The workqueue is per button, if the work is already pending, then last
key press is not handled yet. That keeps the debouncing. Why you want the second
key press to break the first one? The second key press should be ignored, that's
the meaning of debouncing right?

> 
> It looks like a slightly modified version of this patch has already
> been committed [1], but it has the same problem.
> 
> [1] 0b346838c5862bfe911432956a106d602535d030 Input: gpio-keys - change
> timer to workqueue

Yes, the patch is already in Linus tree.


Thanks,
Alek
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html