| Message ID | 1418639312-12520-1-git-send-email-robin@protonic.nl |
|---|---|
| State | New, archived |
| Headers | show |
On Mon, Dec 15, 2014 at 11:28 AM, Robin van der Gracht <robin@protonic.nl> wrote: > The PIN2IRQ register should be used to mask an interrupt. Clearing a > bit in the IRQEN register only prevents the interrupt from propagating but > still allows hardware to set the status bit when triggered. So when > unmasking the interrupt, it will immediately re-trigger if an interrupt > condition occurred during masking. > > This is unwanted behavior especially when using level triggered > interrupts. In this case every interrupt triggers twice. If the > interrupt is handled in the handler, the second interrupt will be > the first one to be able to ack the interrupt. > > Signed-off-by: Robin van der Gracht <robin@protonic.nl> Apparently MXS is a popular GPIO controller without a real maintainer. Adding some random users to the To: line hoping we get some review of this patch. Yours, Linus Walleij -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Jan 13, 2015 at 07:34:31AM +0100, Linus Walleij wrote: > On Mon, Dec 15, 2014 at 11:28 AM, Robin van der Gracht > <robin@protonic.nl> wrote: > > > The PIN2IRQ register should be used to mask an interrupt. Clearing a No, PIN2IRQ implements a function select, not a mask. > > bit in the IRQEN register only prevents the interrupt from propagating but > > still allows hardware to set the status bit when triggered. Isn't it what a mask implementation supposed to do? > > So when > > unmasking the interrupt, it will immediately re-trigger if an interrupt > > condition occurred during masking. Per my understanding, this is the correct behavior. If you do not want that interrupt triggering, you should clear the interrupt status before unmasking the interrupt. Shawn > > > > This is unwanted behavior especially when using level triggered > > interrupts. In this case every interrupt triggers twice. If the > > interrupt is handled in the handler, the second interrupt will be > > the first one to be able to ack the interrupt. > > > > Signed-off-by: Robin van der Gracht <robin@protonic.nl> > > Apparently MXS is a popular GPIO controller without a real maintainer. > > Adding some random users to the To: line hoping we get some > review of this patch. > > Yours, > Linus Walleij -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/gpio/gpio-mxs.c b/drivers/gpio/gpio-mxs.c index 8ffdd7d..13aeae6 100644 --- a/drivers/gpio/gpio-mxs.c +++ b/drivers/gpio/gpio-mxs.c @@ -161,14 +161,12 @@ static void mxs_gpio_irq_handler(u32 irq, struct irq_desc *desc) desc->irq_data.chip->irq_ack(&desc->irq_data); - irq_stat = readl(port->base + PINCTRL_IRQSTAT(port)) & - readl(port->base + PINCTRL_IRQEN(port)); + irq_stat = readl(port->base + PINCTRL_IRQSTAT(port)); while (irq_stat != 0) { int irqoffset = fls(irq_stat) - 1; if (port->both_edges & (1 << irqoffset)) mxs_flip_edge(port, irqoffset); - generic_handle_irq(irq_find_mapping(port->domain, irqoffset)); irq_stat &= ~(1 << irqoffset); } @@ -212,7 +210,7 @@ static void __init mxs_gpio_init_gc(struct mxs_gpio_port *port, int irq_base) ct->chip.irq_set_type = mxs_gpio_set_irq_type; ct->chip.irq_set_wake = mxs_gpio_set_wake_irq; ct->regs.ack = PINCTRL_IRQSTAT(port) + MXS_CLR; - ct->regs.mask = PINCTRL_IRQEN(port); + ct->regs.mask = PINCTRL_PIN2IRQ(port); irq_setup_generic_chip(gc, IRQ_MSK(32), IRQ_GC_INIT_NESTED_LOCK, IRQ_NOREQUEST, 0); @@ -284,11 +282,11 @@ static int mxs_gpio_probe(struct platform_device *pdev) port->base = base; /* - * select the pin interrupt functionality but initially - * disable the interrupts + * enable interrupts but initially disable + * pin interrupt functionality */ - writel(~0U, port->base + PINCTRL_PIN2IRQ(port)); - writel(0, port->base + PINCTRL_IRQEN(port)); + writel(0, port->base + PINCTRL_PIN2IRQ(port)); + writel(~0U, port->base + PINCTRL_IRQEN(port)); /* clear address has to be used to clear IRQSTAT bits */ writel(~0U, port->base + PINCTRL_IRQSTAT(port) + MXS_CLR);
The PIN2IRQ register should be used to mask an interrupt. Clearing a bit in the IRQEN register only prevents the interrupt from propagating but still allows hardware to set the status bit when triggered. So when unmasking the interrupt, it will immediately re-trigger if an interrupt condition occurred during masking. This is unwanted behavior especially when using level triggered interrupts. In this case every interrupt triggers twice. If the interrupt is handled in the handler, the second interrupt will be the first one to be able to ack the interrupt. Signed-off-by: Robin van der Gracht <robin@protonic.nl> --- drivers/gpio/gpio-mxs.c | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-)