On Tue, Nov 26, 2013 at 01:27:21PM +0100, Linus Walleij wrote: > On Tue, Nov 26, 2013 at 12:49 PM, Mika Westerberg > <[email protected]> wrote: > > On Tue, Nov 26, 2013 at 11:23:39AM +0100, Linus Walleij wrote: > > >> +static unsigned int lp_irq_startup(struct irq_data *d) > >> +{ > >> + struct lp_gpio *lg = irq_data_get_irq_chip_data(d); > >> + > >> + if (gpio_lock_as_irq(&lg->chip, irqd_to_hwirq(d))) > >> + dev_err(lg->chip.dev, > >> + "unable to lock HW IRQ %lu for IRQ\n", > >> + irqd_to_hwirq(d)); > >> + lp_irq_enable(d); > > > > I may be missing something but doesn't this now end up calling > > lp_irq_enable() twice? First in ->irq_startup() and then later on in > > ->irq_enable(). > > kernel/irq/chip.c: > > int irq_startup(struct irq_desc *desc, bool resend) > { > int ret = 0; > > irq_state_clr_disabled(desc); > desc->depth = 0; > > if (desc->irq_data.chip->irq_startup) { > ret = desc->irq_data.chip->irq_startup(&desc->irq_data); > irq_state_clr_masked(desc); > } else { > irq_enable(desc); > } > if (resend) > check_irq_resend(desc, desc->irq_data.irq); > return ret; > } > > If this hook exists, calls irq_startup() on the chip. > Else, calls irq_enable() which looks like this: > > void irq_enable(struct irq_desc *desc) > { > irq_state_clr_disabled(desc); > if (desc->irq_data.chip->irq_enable) > desc->irq_data.chip->irq_enable(&desc->irq_data); > else > desc->irq_data.chip->irq_unmask(&desc->irq_data); > irq_state_clr_masked(desc); > } > > I.e. calls .enable() or .unmask(). > > So there is a strict semantic requirement that if you implement > startup() it should perform the same as .enable(), or .unmask() > depending on whether the former is implemented.
Thanks for the explanation, got it now :) > I think this patch is OK ... can you test it on the Lynxpoint? Tried on haswell/lynxpoint with a slightly modified i2c-hid driver (so that it is able to use GPIOs as interrupts) and here's what I got: # cat /sys/kernel/debug/gpio GPIOs 162-255, platform/INT33C7:00, INT33C7:00: gpio-217 (hid-irq ) in hi IRQ Also the device itself works, so Tested-by: Mika Westerberg <[email protected]> -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
