All of lore.kernel.org
 help / color / mirror / Atom feed
From: Phil Carmody <ext-phil.2.carmody@nokia.com>
To: ext Alek Du <alek.du@intel.com>
Cc: "Nikula Jani.1 (EXT-Nixu/Helsinki)" <ext-jani.1.nikula@nokia.com>,
	LKML <linux-kernel@vger.kernel.org>,
	Trilok Soni <soni.trilok@gmail.com>,
	"linux-input@vger.kernel.org" <linux-input@vger.kernel.org>,
	Dmitry Torokhov <dmitry.torokhov@gmail.com>,
	"ben-linux@fluff.org" <ben-linux@fluff.org>
Subject: Re: [PATCH]input: Change timer function to workqueue for gpio_keys driver
Date: Thu, 25 Jun 2009 19:09:14 +0300	[thread overview]
Message-ID: <1245946154.9103.742.camel@pcarmody-desktop> (raw)
In-Reply-To: <20090625234846.1b987069@dxy.sh.intel.com>

On Thu, 2009-06-25 at 17:48 +0200, ext Alek Du wrote:
> On Thu, 25 Jun 2009 23:42:08 +0800
> Jani Nikula <ext-jani.1.nikula@nokia.com> wrote:
> 
> > On Thu, 2009-06-25 at 17:09 +0200, ext Alek Du wrote:
> > > On Thu, 25 Jun 2009 23:05:55 +0800
> > > Jani Nikula <ext-jani.1.nikula@nokia.com> wrote:
> > > > Actually scrap that, the input layer already ignores events with no
> > > > state changes, right?
> > > > 
> > > Yes, correct. I just want to reply your previous mail, but seems you find that. :-)
> > 
> > The point about your patch breaking debouncing is still valid, though.
> > 
> >
> How? If IRQ triggered then the delay work scheduled, after debouncing time, in work, it checks GPIO pin state again,
> if pin is active, send "1" to input layer -- key pressed, if de-active, send "0" -- no event. 
> 
> I really did test on my board, could you also try it out?

This is not a matter of testing, this error can be seen simply by
algorithm analysis - that's how Jani and I discovered the problem in the
first place.

If you stopped calling the delay after the first transition "debouncing
time" and simply called it a "delay" you might more easily see that it
does *no* debouncing at all. Imagine putting noise on the line
constantly - the original code's timer would never expire. Your timer
will expire after a delay, and while the line is still toggling
frantically - you've not debounced.

Please investigate the meaning and implications of "debouncing" before
claiming your code does it.

Phil


  reply	other threads:[~2009-06-25 16:07 UTC|newest]

Thread overview: 30+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-06-08  7:24 [PATCH]input: Change timer function to workqueue for gpio_keys driver Alek Du
2009-06-12 17:40 ` Trilok Soni
2009-06-12 17:40   ` Trilok Soni
2009-06-25 10:29   ` Jani Nikula
2009-06-25 10:29     ` Jani Nikula
2009-06-25 13:06     ` Alek Du
2009-06-25 13:06       ` Alek Du
2009-06-25 13:31       ` Jani Nikula
2009-06-25 14:08         ` Alek Du
2009-06-25 14:52           ` Jani Nikula
2009-06-25 15:05             ` Jani Nikula
2009-06-25 15:09               ` Alek Du
2009-06-25 15:42                 ` Jani Nikula
2009-06-25 15:48                   ` Alek Du
2009-06-25 16:09                     ` Phil Carmody [this message]
2009-06-25 16:23                       ` Alek Du
2009-06-25 16:42                         ` Dmitry Torokhov
2009-06-25 16:42                           ` Dmitry Torokhov
2009-06-26 12:15                         ` [PATCH 0/2] fix gpio-keys debouncing and timer sleep issues Jani Nikula
2009-06-26 12:15                           ` [PATCH 1/2] Revert "Input: gpio-keys - change timer to workqueue" Jani Nikula
2009-06-26 12:15                           ` [PATCH 2/2] input: gpio-keys: avoid possibility of sleeping in timer function Jani Nikula
2009-06-29 10:30                             ` Alek Du
2009-06-29 10:30                               ` Alek Du
2009-06-26 12:50                           ` [PATCH 0/2] fix gpio-keys debouncing and timer sleep issues Du, Alek
2009-06-26 12:50                             ` Du, Alek
2009-06-29  5:59                           ` Dmitry Torokhov
2009-06-29 10:32                             ` Jani Nikula
  -- strict thread matches above, loose matches on Subject: below --
2009-06-08  5:54 [PATCH] input: Change timer function to workqueue for gpio_keys driver Alek Du
2009-06-08  6:04 ` Trilok Soni
2009-06-08  6:04   ` Trilok Soni

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1245946154.9103.742.camel@pcarmody-desktop \
    --to=ext-phil.2.carmody@nokia.com \
    --cc=alek.du@intel.com \
    --cc=ben-linux@fluff.org \
    --cc=dmitry.torokhov@gmail.com \
    --cc=ext-jani.1.nikula@nokia.com \
    --cc=linux-input@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=soni.trilok@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.