RE: [RFC] [PATCH 1/3] OMAP4: Keyboard Controller Support

Shilimkar, Santosh
2010-05-12T06:54:45Z

> -----Original Message-----
> From: Dmitry Torokhov [mailto:dmitry.torokhov@gmail.com]
> Sent: Wednesday, May 12, 2010 12:05 PM
> To: Shilimkar, Santosh
> Cc: Arce, Abraham; linux-input@vger.kernel.org; linux-omap@vger.kernel.org
> Subject: Re: [RFC] [PATCH 1/3] OMAP4: Keyboard Controller Support
>
> On Wed, May 12, 2010 at 11:49:45AM +0530, Shilimkar, Santosh wrote:
> > > -----Original Message-----
> > > From: Dmitry Torokhov [mailto:dmitry.torokhov@gmail.com]
> > > Sent: Wednesday, May 12, 2010 11:33 AM
> > > To: Shilimkar, Santosh
> > > Cc: Arce, Abraham; linux-input@vger.kernel.org; linux-omap@vger.kernel.org
> > > Subject: Re: [RFC] [PATCH 1/3] OMAP4: Keyboard Controller Support
> > >
> > > On Wed, May 12, 2010 at 11:15:11AM +0530, Shilimkar, Santosh wrote:
> > > > > -----Original Message-----
> > > > > From: linux-omap-owner@vger.kernel.org [mailto:linux-omap-owner@vger.kernel.org] On Behalf Of
> > > Arce,
> > > > > Abraham
> > > > > Sent: Wednesday, May 12, 2010 11:10 AM
> > > > > To: Dmitry Torokhov
> > > > > Cc: linux-input@vger.kernel.org; linux-omap@vger.kernel.org
> > > > > Subject: RE: [RFC] [PATCH 1/3] OMAP4: Keyboard Controller Support
> > > > >
> > > > > Dmitry,
> > > > >
> > > > > 2 comments + one question before sending next version...
> > > > >
> > > > > [...]
> > > > >
> > > > > > > > > > +static irqreturn_t omap_keypad_threaded(int irq, void *dev_id)
> > > > > > > > > > +{
> > > > > > > > >
> > > > > > > > > Why is iti threaded? I fo not see anything that will sleep.
> > > > > > >
> > > > > > >
> > > > > > > It was implemented based on previous comments...
> > > > > > >
> > > > > >
> > > > > > Would you point me to that comment? Like I said, I do not see anything
> > > > > > that would possibly sleep in this routine so you don't need to use
> > > > > > threaded interrupt.
> > > > >
> > > > > Using now request_irq based on your comments. In same omap_keypad_interrupt
> disable/clear/enable
> > > > > interrupts will be executed
> > > > >
> > > > > [...]
> > > > >
> > > > Sorry for jumping into the comments late. Thought this was sorted out. Key scanning
> > > > and debounce timeouts etc still there. Having all these things in ISR itself isn't good
> > > > idea.
> > > >
> > > > Dmitry,
> > > > Don't you think its optimal to push the key-scanning and debounce timeout code
> > > > part of bottom half ??
> > > >
> > >
> > > If you need debounce then you need to fire a timer and keep doing this
> > > until interrupt (or key state) settles. It really depends on the device.
> > >
> > The OMAP4 keypad controller has internal timeout mechanism and doesn't need any
> > external timer for this.
> >
>
> Then I do not understand the question... If hardware takes care of
> debouncing then just read the state and report the data.
>
You have point. Debounce shouldn't be an issue. If the key scan isn't taking
Much time then I agree with your suggestion.

Abraham, Can you please how much time you are spending in the key scan loops ?

Regards,
Santosh