Hi, * Ladislav Michl <ladis@xxxxxxxxxxxxxx> [170310 15:06]: > Hi there, > > I'm using drivers/media/rc/gpio-ir-recv.c on DM3730 based board and getting > a lot of IR protocol decoding failures: > RC5(x/sz) decode failed at state 0 count 7 (855us space) > RC5(x/sz) decode failed at state 1 count 4 (1740us space) > RC5(x/sz) decode failed at state 4 count 1 (125000us space) > RC5(x/sz) decode failed at state 0 count 1 (182348us space) > RC5(x/sz) decode failed at state 0 count 1 (366us pulse) > etc... > Hacking gpio-ir-recv code to add debounce on GPIO pin makes situation > a bit better, but still bad enough. > It seems this problem is there since dawn of linux-omap time [1] and > can be easily reproduced requesting edge trigerred gpio interrupt, > feeding that gpio pin from signal generator and toggling another gpio > in interrupt hangler with scope hooked. > Last time patch [1] was updated to kernel 2.6.32 and you can find it > below. > > So it seems noone is really using edge triggered interrupts, right? Interesting, adding Grygorii to cc as well. > Now, patch bellow is for ancient omap1 variant (and I'd love to give it > a try with recent kernel, but everything I compiled so far is insanely > huge) and does not support both rising and falling edge. > > To make interrupt code reliable I do not see other option than to read > gpio input register and make loop similar as in patch bellow. However, > this was repeatedly rejected in the past. So, better ideas, anyone? > [1] http://marc.info/?l=linux-omap&m=119634410025393&w=2 Oh that thing yeah I remember testing it too. If we don't have any other options for edge interrupts I guess the loop is what we need to do. If you have some experimental patch to play with against the current mainline, please post it and I'll give it a try too. Regards, Tony > --- linux-2.6.32/arch/arm/plat-omap/gpio.c.orig 2009-12-29 20:56:29.000000000 +0100 > +++ linux-2.6.32/arch/arm/plat-omap/gpio.c 2009-12-29 20:56:44.000000000 +0100 > @@ -192,6 +192,7 @@ > u32 saved_risingdetect; > #endif > u32 level_mask; > + u32 serviced; > spinlock_t lock; > struct gpio_chip chip; > struct clk *dbck; > @@ -1204,6 +1205,28 @@ > spin_unlock_irqrestore(&bank->lock, flags); > } > > +static u32 _get_gpio_irq_status(struct gpio_bank *bank) > +{ > + u32 isr, ism, in, re; > + u32 base = bank->base; > + > + switch (bank->method) { > +#ifdef CONFIG_ARCH_OMAP15XX > + case METHOD_GPIO_1510: > + in = __raw_readl(base + OMAP1510_GPIO_DATA_INPUT); > + isr = __raw_readl(base + OMAP1510_GPIO_INT_STATUS); > + ism = __raw_readl(base + OMAP1510_GPIO_INT_MASK) | 0xffff0000; > + re = __raw_readl(base + OMAP1510_GPIO_INT_CONTROL); > + isr |= (~(in^re)) & (~ism); > + break; > +#endif > + default: > + BUG(); > + return 0; > + } > + return isr; > +} > + > /* > * We need to unmask the GPIO bank interrupt as soon as possible to > * avoid missing GPIO interrupts for other lines in the bank. > @@ -1218,11 +1241,13 @@ > void __iomem *isr_reg = NULL; > u32 isr; > unsigned int gpio_irq; > + unsigned long flags; > struct gpio_bank *bank; > u32 retrigger = 0; > int unmasked = 0; > > desc->chip->ack(irq); > + desc->chip->unmask(irq); > > bank = get_irq_data(irq); > #ifdef CONFIG_ARCH_OMAP1 > @@ -1249,54 +1274,29 @@ > if (bank->method == METHOD_GPIO_24XX) > isr_reg = bank->base + OMAP4_GPIO_IRQSTATUS0; > #endif > + spin_lock_irqsave(&bank->lock, flags); > while(1) { > - u32 isr_saved, level_mask = 0; > - u32 enabled; > + u32 isr; > + unsigned long serviced; > > - enabled = _get_gpio_irqbank_mask(bank); > - isr_saved = isr = __raw_readl(isr_reg) & enabled; > - > - if (cpu_is_omap15xx() && (bank->method == METHOD_MPUIO)) > - isr &= 0x0000ffff; > - > - if (cpu_class_is_omap2()) { > - level_mask = bank->level_mask & enabled; > - } > - > - /* clear edge sensitive interrupts before handler(s) are > - called so that we don't miss any interrupt occurred while > - executing them */ > - _enable_gpio_irqbank(bank, isr_saved & ~level_mask, 0); > - _clear_gpio_irqbank(bank, isr_saved & ~level_mask); > - _enable_gpio_irqbank(bank, isr_saved & ~level_mask, 1); > - > - /* if there is only edge sensitive GPIO pin interrupts > - configured, we could unmask GPIO bank interrupt immediately */ > - if (!level_mask && !unmasked) { > - unmasked = 1; > - desc->chip->unmask(irq); > - } > - > - isr |= retrigger; > - retrigger = 0; > + isr = _get_gpio_irq_status(bank) & ~bank->serviced; > if (!isr) > break; > > gpio_irq = bank->virtual_irq_start; > - for (; isr != 0; isr >>= 1, gpio_irq++) { > - if (!(isr & 1)) > - continue; > - > - generic_handle_irq(gpio_irq); > + serviced = 1; > + for (; isr != 0; gpio_irq++, serviced <<= 1 ) { > + if (isr & serviced) { > + bank->serviced |= serviced; > + spin_unlock_irqrestore(&bank->lock, flags); > + generic_handle_irq(gpio_irq); > + spin_lock_irqsave(&bank->lock, flags); > + bank->serviced &= ~serviced; > + isr &= ~serviced; > + } > } > } > - /* if bank has any level sensitive GPIO pin interrupt > - configured, we must unmask the bank interrupt only after > - handler(s) are executed in order to avoid spurious bank > - interrupt */ > - if (!unmasked) > - desc->chip->unmask(irq); > - > + spin_unlock_irqrestore(&bank->lock, flags); > } > > static void gpio_irq_shutdown(unsigned int irq) > @@ -1781,6 +1781,7 @@ > #endif > > bank->mod_usage = 0; > + bank->serviced = 0; > /* REVISIT eventually switch from OMAP-specific gpio structs > * over to the generic ones > */ > > -- > To unsubscribe from this list: send the line "unsubscribe linux-omap" in > the body of a message to majordomo@xxxxxxxxxxxxxxx > More majordomo info at http://vger.kernel.org/majordomo-info.html > -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
