Cory Maccarrone <[email protected]> writes: > Some chips, namely any OMAP1 chips using METHOD_MPUIO, > OMAP15xx and OMAP7xx, cannot be setup to respond to on-chip GPIO > interrupts in both rising and falling edge directions -- they can > only respond to one direction or the other, depending on how the > ICR is configured. > > This change implements a toggle function that will modify the ICR > to flip the direction of interrupt for IRQs that are requested with > both rising and falling flags. The toggle function is a noop for > chips and GPIOs it does not apply to.
I think this needs a little more description. In particular you should describe that this toggle happens on *every* interrupt, and why. Also, it's not exactly a noop for non-OMAP1. More on this below... > Signed-off-by: Cory Maccarrone <[email protected]> > --- > arch/arm/plat-omap/gpio.c | 60 > +++++++++++++++++++++++++++++++++++++++++++++ > 1 files changed, 60 insertions(+), 0 deletions(-) > > diff --git a/arch/arm/plat-omap/gpio.c b/arch/arm/plat-omap/gpio.c > index 055160e..b594398 100644 > --- a/arch/arm/plat-omap/gpio.c > +++ b/arch/arm/plat-omap/gpio.c > @@ -749,6 +749,53 @@ static inline void set_24xx_gpio_triggering(struct > gpio_bank *bank, int gpio, > } > #endif > > +/* > + * This only applies to chips that can't do both rising and falling edge > + * detection at once. For all other chips, this function is a noop. > + */ Not exactly a noop. This function is called for non-OMAP1 as well... > +static void _toggle_gpio_edge_triggering(struct gpio_bank *bank, int gpio) > +{ > + void __iomem *reg = bank->base; > + u32 l = 0; > + > + switch (bank->method) { > +#ifdef CONFIG_ARCH_OMAP1 > + case METHOD_MPUIO: > + reg += OMAP_MPUIO_GPIO_INT_EDGE; > + l = __raw_readl(reg); > + if ((l >> gpio) & 1) > + l &= ~(1 << gpio); > + else > + l |= 1 << gpio; > + break; > +#endif > +#ifdef CONFIG_ARCH_OMAP15XX > + case METHOD_GPIO_1510: > + reg += OMAP1510_GPIO_INT_CONTROL; > + l = __raw_readl(reg); > + if ((l >> gpio) & 1) > + l &= ~(1 << gpio); > + else > + l |= 1 << gpio; > + break; > +#endif > +#if defined(CONFIG_ARCH_OMAP730) || defined(CONFIG_ARCH_OMAP850) > + case METHOD_GPIO_7XX: > + reg += OMAP7XX_GPIO_INT_CONTROL; > + l = __raw_readl(reg); > + if ((l >> gpio) & 1) { > + l &= ~(1 << gpio); > + } else { > + l |= 1 << gpio; > + } > + break; > +#endif > + default: > + return; > + } > + __raw_writel(l, reg); ...and will write a zero to offset zero of the GPIO bank regs (GPIO_REVISION reg). While ths is a read-only reg and *should* have no effect, the point is that this function should not be called for non-OMAP1. > +} > + > static int _set_gpio_triggering(struct gpio_bank *bank, int gpio, int > trigger) > { > void __iomem *reg = bank->base; > @@ -1284,9 +1331,22 @@ static void gpio_irq_handler(unsigned int irq, struct > irq_desc *desc) > > gpio_irq = bank->virtual_irq_start; > for (; isr != 0; isr >>= 1, gpio_irq++) { > + struct irq_desc *desc = irq_to_desc(gpio_irq); > if (!(isr & 1)) > continue; > > + /* > + * Some chips can't respond to both rising and falling > at the > + * same time. If this irq was requested with both > flags, we > + * need to flip the ICR data for the IRQ to respond to > the > + * IRQ for the opposite direction. > + */ > + if (desc->status & IRQ_TYPE_EDGE_FALLING && > + desc->status & IRQ_TYPE_EDGE_RISING) > + _toggle_gpio_edge_triggering( > + get_irq_chip_data(gpio_irq), > + > get_gpio_index(irq_to_gpio(gpio_irq))); > + Rather than check the flags on every interrupt, it may be better to create a per-bank mask for GPIOs that need this treatment. The code that sets that flag could be conditional on OMAP1 and also on whether both edges are set. > generic_handle_irq(gpio_irq); > } > } Kevin -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
