On Fri, 16 Mar 2012 00:20:04 -0700 Dmitry Torokhov <dmitry.torokhov@xxxxxxxxx> 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: Sorry, the reason described is apparently not very clear. The real reason seems to be that I would like this driver to work with I2C GPIO expanders, and its the GPIO expanders "interrupt controller" which has itself a threaded handler (due to I2C transfers done in it to ack an IRQ). So this is actually a nested and threaded interrupt controller (because the IRQ line of the GPIO expander is connected to a different GPIO acting itself also as interrupt line). In irq/manage.c, function __setup_irq(): ... /* * Check whether the interrupt nests into another interrupt * thread. */ nested = irq_settings_is_nested_thread(desc); if (nested) { if (!new->thread_fn) { ret = -EINVAL; goto out_mput; } ... This is were requesting a non-threaded IRQ from this GPIO controller will fail. I know this is not a trivial setup, but IMHO it is very useful (for connecting keyboards), and a nice demonstration of the powerful features this GPIO driver has :-) > 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)); > > 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? Not in my case. I just checked again. If I change request_threaded_irq() to request_irq(), I get this: ... [ 6.409810] gpio-keys gpio_keys.0: Unable to claim irq 0; error -22 [ 6.416106] gpio-keys: probe of gpio_keys.0 failed with error -22 ... This error -22 (-EINVAL) is returned from __setup_irq() (see above). BTW: The connections of CPU-GPIO -> IRQ of PCA9539 -> GPIO -> gpio_key is entirely done in the device tree, which is also sort of cool ;-) 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@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html