Re: [PATCH v4 3/3] Input: gpio_keys.c: Enable use with non-local GPIO chips.

[David Jander <david.jander@protonic.nl>]

On Fri, 16 Mar 2012 10:18:15 +0000
Ben Dooks <ben@trinity.fluff.org> wrote:

> On Fri, Mar 16, 2012 at 12:20:04AM -0700, Dmitry Torokhov wrote:
> > Hi David,
> > 
> > On Tue, Jun 14, 2011 at 11:08:11AM +0200, David Jander wrote:
> > > Use a threaded interrupt handler in order to permit the handler to use
> > > a GPIO driver that causes things like I2C transactions being done inside
> > > the handler context.
> > > Also, gpio_keys_init needs to be declared as a late_initcall, to make sure
> > > all needed GPIO drivers have been loaded if the drivers are built into the
> > > kernel.
> > 
> > Don't want to resurrect the whole initcall discussion, but could you
> > tell me again why the interrup handler needs to be threaded? We do not
> > access hardware from it, hardware is accessed from workqueue context.
> > Here is the ISR in its entirety:
> > 
> > static irqreturn_t gpio_keys_isr(int irq, void *dev_id)
> > {
> >         struct gpio_button_data *bdata = dev_id;
> >         const struct gpio_keys_button *button = bdata->button;
> > 
> >         BUG_ON(irq != gpio_to_irq(button->gpio));
> 
> Why on earth do we need this? this looks like something that is not
> necessary and in my view a waste of cpu cycles.

No idea... catch some weird (hardware-/setup-)bug? Not _that_ many CPU cycles
anyway, plus I am not the author of that line.... maybe ask Uwe Kleine-König
(CC'd)?

> >         if (bdata->timer_debounce)
> >                 mod_timer(&bdata->timer,
> >                         jiffies + msecs_to_jiffies(bdata->timer_debounce));
> >         else
> >                 schedule_work(&bdata->work);
> > 
> >         return IRQ_HANDLED;
> > }
> > 
> > It looks to me that non-threaded handler would work as well? Or
> > gpio_to_irq() can sleep with certain chips?
> 
> See above comment, I'd go with just remove it and unthread.

Not unthread, but use request_any_context_irq(), please!

Best regards,

-- 
David Jander
Protonic Holland.
--
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