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.
> 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.
--
Ben Dooks, ben@xxxxxxxxx, http://www.fluff.org/ben/
Large Hadron Colada: A large Pina Colada that makes the universe disappear.
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
