| Message ID | 1309513634-20971-10-git-send-email-tarun.kanti@ti.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Tarun Kanti DebBarma <tarun.kanti@ti.com> writes: > From: Charulatha V <charu@ti.com> > > By adding level and edge detection register offsets and then initializing them > correctly according to OMAP versions during device registrations we can now remove > lot of revision checks in these functions. > > Signed-off-by: Tarun Kanti DebBarma <tarun.kanti@ti.com> > Signed-off-by: Charulatha V <charu@ti.com> Found the bug causing GPIO IRQ triggering failures for Blaze and Zoom3 GPIO-based network IRQs... > @@ -400,12 +394,12 @@ static int gpio_irq_type(struct irq_data *d, unsigned type) > if (type & ~IRQ_TYPE_SENSE_MASK) > return -EINVAL; > > - /* OMAP1 allows only only edge triggering */ > - if (!cpu_class_is_omap2() > - && (type & (IRQ_TYPE_LEVEL_LOW|IRQ_TYPE_LEVEL_HIGH))) > + bank = irq_data_get_irq_chip_data(d); > + > + if (bank->regs->leveldetect0 && (type & > + (IRQ_TYPE_LEVEL_LOW|IRQ_TYPE_LEVEL_HIGH))) This check is not the same as the one it replaced. This check should be: if (!bank->regs->leveldetect0 && (type & (IRQ_TYPE_LEVEL_LOW | IRQ_TYPE_LEVEL_HIGH))) return -EINVAL; Note the formatting changes also for readability, but the bugfix is just adding the '!' before regs->leveldetect0. Kevin -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> -----Original Message----- > From: Hilman, Kevin > Sent: Wednesday, July 06, 2011 5:22 AM > To: DebBarma, Tarun Kanti > Cc: linux-omap@vger.kernel.org; Shilimkar, Santosh; tony@atomide.com; > Varadarajan, Charulatha > Subject: Re: [PATCH v3 09/20] GPIO: OMAP: Use level/edge detect reg > offsets > > Tarun Kanti DebBarma <tarun.kanti@ti.com> writes: > > > From: Charulatha V <charu@ti.com> > > > > By adding level and edge detection register offsets and then > initializing them > > correctly according to OMAP versions during device registrations we can > now remove > > lot of revision checks in these functions. > > > > Signed-off-by: Tarun Kanti DebBarma <tarun.kanti@ti.com> > > Signed-off-by: Charulatha V <charu@ti.com> > > Found the bug causing GPIO IRQ triggering failures for Blaze and Zoom3 > GPIO-based network IRQs... > > > @@ -400,12 +394,12 @@ static int gpio_irq_type(struct irq_data *d, > unsigned type) > > if (type & ~IRQ_TYPE_SENSE_MASK) > > return -EINVAL; > > > > - /* OMAP1 allows only only edge triggering */ > > - if (!cpu_class_is_omap2() > > - && (type & (IRQ_TYPE_LEVEL_LOW|IRQ_TYPE_LEVEL_HIGH))) > > + bank = irq_data_get_irq_chip_data(d); > > + > > + if (bank->regs->leveldetect0 && (type & > > + (IRQ_TYPE_LEVEL_LOW|IRQ_TYPE_LEVEL_HIGH))) > > This check is not the same as the one it replaced. This check should > be: > > if (!bank->regs->leveldetect0 && > (type & (IRQ_TYPE_LEVEL_LOW | IRQ_TYPE_LEVEL_HIGH))) > return -EINVAL; > > Note the formatting changes also for readability, but the bugfix is just > adding the '!' before regs->leveldetect0. Yes, thanks. I will make the correction. -- Tarun > > Kevin -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Tarun Kanti DebBarma <tarun.kanti@ti.com> writes: > From: Charulatha V <charu@ti.com> > > By adding level and edge detection register offsets and then initializing them > correctly according to OMAP versions during device registrations we can now remove > lot of revision checks in these functions. > > Signed-off-by: Tarun Kanti DebBarma <tarun.kanti@ti.com> > Signed-off-by: Charulatha V <charu@ti.com> Looks good. One very minor nit below... [...] > @@ -1247,40 +1240,18 @@ void omap2_gpio_prepare_for_idle(int off_mode) > if (!(bank->enabled_non_wakeup_gpios)) > goto save_gpio_ctx; > > - if (cpu_is_omap24xx() || cpu_is_omap34xx()) { > - bank->saved_datain = __raw_readl(bank->base + > - OMAP24XX_GPIO_DATAIN); > - l1 = __raw_readl(bank->base + > - OMAP24XX_GPIO_FALLINGDETECT); > - l2 = __raw_readl(bank->base + > - OMAP24XX_GPIO_RISINGDETECT); > - } > - > - if (cpu_is_omap44xx()) { > - bank->saved_datain = __raw_readl(bank->base + > - OMAP4_GPIO_DATAIN); > - l1 = __raw_readl(bank->base + > - OMAP4_GPIO_FALLINGDETECT); > - l2 = __raw_readl(bank->base + > - OMAP4_GPIO_RISINGDETECT); > - } > + bank->saved_datain = __raw_readl(bank->base + > + bank->regs->datain); > + l1 = __raw_readl(bank->base + bank->regs->fallingdetect); > + l2 = __raw_readl(bank->base + bank->regs->risingdetect); The l1 = 0, l2 = 0 assignments can now be removed at the beginning of this function also. Similarily for the 'l = 0' assignment in the runtime_resume function. Kevin -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[...] > > > > By adding level and edge detection register offsets and then > initializing them > > correctly according to OMAP versions during device registrations we can > now remove > > lot of revision checks in these functions. > > > > Signed-off-by: Tarun Kanti DebBarma <tarun.kanti@ti.com> > > Signed-off-by: Charulatha V <charu@ti.com> > > Looks good. One very minor nit below... > > [...] > > > @@ -1247,40 +1240,18 @@ void omap2_gpio_prepare_for_idle(int off_mode) > > if (!(bank->enabled_non_wakeup_gpios)) > > goto save_gpio_ctx; > > > > - if (cpu_is_omap24xx() || cpu_is_omap34xx()) { > > - bank->saved_datain = __raw_readl(bank->base + > > - OMAP24XX_GPIO_DATAIN); > > - l1 = __raw_readl(bank->base + > > - OMAP24XX_GPIO_FALLINGDETECT); > > - l2 = __raw_readl(bank->base + > > - OMAP24XX_GPIO_RISINGDETECT); > > - } > > - > > - if (cpu_is_omap44xx()) { > > - bank->saved_datain = __raw_readl(bank->base + > > - OMAP4_GPIO_DATAIN); > > - l1 = __raw_readl(bank->base + > > - OMAP4_GPIO_FALLINGDETECT); > > - l2 = __raw_readl(bank->base + > > - OMAP4_GPIO_RISINGDETECT); > > - } > > + bank->saved_datain = __raw_readl(bank->base + > > + bank->regs->datain); > > + l1 = __raw_readl(bank->base + bank->regs->fallingdetect); > > + l2 = __raw_readl(bank->base + bank->regs->risingdetect); > > The l1 = 0, l2 = 0 assignments can now be removed at the beginning of > this function also. Similarily for the 'l = 0' assignment in the > runtime_resume function. Ok. -- Tarun > > Kevin -- To unsubscribe from this list: send the line "unsubscribe linux-omap" 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/arch/arm/mach-omap2/gpio.c b/arch/arm/mach-omap2/gpio.c index 9387496..0c7e403 100644 --- a/arch/arm/mach-omap2/gpio.c +++ b/arch/arm/mach-omap2/gpio.c @@ -111,6 +111,10 @@ static int omap2_gpio_dev_init(struct omap_hwmod *oh, void *unused) pdata->regs->wkup_status = OMAP24XX_GPIO_WAKE_EN; pdata->regs->wkup_clear = OMAP24XX_GPIO_CLEARWKUENA; pdata->regs->wkup_set = OMAP24XX_GPIO_SETWKUENA; + pdata->regs->leveldetect0 = OMAP24XX_GPIO_LEVELDETECT0; + pdata->regs->leveldetect1 = OMAP24XX_GPIO_LEVELDETECT1; + pdata->regs->risingdetect = OMAP24XX_GPIO_RISINGDETECT; + pdata->regs->fallingdetect = OMAP24XX_GPIO_FALLINGDETECT; break; case 2: pdata->bank_type = METHOD_GPIO_44XX; @@ -131,6 +135,10 @@ static int omap2_gpio_dev_init(struct omap_hwmod *oh, void *unused) pdata->regs->wkup_status = OMAP4_GPIO_IRQWAKEN0; pdata->regs->wkup_clear = OMAP4_GPIO_IRQWAKEN0; pdata->regs->wkup_set = OMAP4_GPIO_IRQWAKEN0; + pdata->regs->leveldetect0 = OMAP4_GPIO_LEVELDETECT0; + pdata->regs->leveldetect1 = OMAP4_GPIO_LEVELDETECT1; + pdata->regs->risingdetect = OMAP4_GPIO_RISINGDETECT; + pdata->regs->fallingdetect = OMAP4_GPIO_FALLINGDETECT; break; default: WARN(1, "Invalid gpio bank_type\n"); diff --git a/arch/arm/plat-omap/include/plat/gpio.h b/arch/arm/plat-omap/include/plat/gpio.h index d941d92..a843669 100644 --- a/arch/arm/plat-omap/include/plat/gpio.h +++ b/arch/arm/plat-omap/include/plat/gpio.h @@ -192,6 +192,10 @@ struct omap_gpio_reg_offs { u16 wkup_status; u16 wkup_clear; u16 wkup_set; + u16 leveldetect0; + u16 leveldetect1; + u16 risingdetect; + u16 fallingdetect; bool irqenable_inv; }; diff --git a/drivers/gpio/gpio-omap.c b/drivers/gpio/gpio-omap.c index abb85df..66c837f 100644 --- a/drivers/gpio/gpio-omap.c +++ b/drivers/gpio/gpio-omap.c @@ -252,15 +252,9 @@ static inline void set_24xx_gpio_triggering(struct gpio_bank *bank, int gpio, bank->enabled_non_wakeup_gpios &= ~gpio_bit; } - if (cpu_is_omap44xx()) { - bank->level_mask = - __raw_readl(bank->base + OMAP4_GPIO_LEVELDETECT0) | - __raw_readl(bank->base + OMAP4_GPIO_LEVELDETECT1); - } else { - bank->level_mask = - __raw_readl(bank->base + OMAP24XX_GPIO_LEVELDETECT0) | - __raw_readl(bank->base + OMAP24XX_GPIO_LEVELDETECT1); - } + bank->level_mask = + __raw_readl(bank->base + bank->regs->leveldetect0) | + __raw_readl(bank->base + bank->regs->leveldetect1); } #endif @@ -400,12 +394,12 @@ static int gpio_irq_type(struct irq_data *d, unsigned type) if (type & ~IRQ_TYPE_SENSE_MASK) return -EINVAL; - /* OMAP1 allows only only edge triggering */ - if (!cpu_class_is_omap2() - && (type & (IRQ_TYPE_LEVEL_LOW|IRQ_TYPE_LEVEL_HIGH))) + bank = irq_data_get_irq_chip_data(d); + + if (bank->regs->leveldetect0 && (type & + (IRQ_TYPE_LEVEL_LOW|IRQ_TYPE_LEVEL_HIGH))) return -EINVAL; - bank = irq_data_get_irq_chip_data(d); spin_lock_irqsave(&bank->lock, flags); retval = _set_gpio_triggering(bank, GPIO_INDEX(bank, gpio), type); spin_unlock_irqrestore(&bank->lock, flags); @@ -652,9 +646,8 @@ static void gpio_irq_handler(unsigned int irq, struct irq_desc *desc) if (cpu_is_omap15xx() && (bank->method == METHOD_MPUIO)) isr &= 0x0000ffff; - if (cpu_class_is_omap2()) { + if (bank->level_mask) 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 @@ -1247,40 +1240,18 @@ void omap2_gpio_prepare_for_idle(int off_mode) if (!(bank->enabled_non_wakeup_gpios)) goto save_gpio_ctx; - if (cpu_is_omap24xx() || cpu_is_omap34xx()) { - bank->saved_datain = __raw_readl(bank->base + - OMAP24XX_GPIO_DATAIN); - l1 = __raw_readl(bank->base + - OMAP24XX_GPIO_FALLINGDETECT); - l2 = __raw_readl(bank->base + - OMAP24XX_GPIO_RISINGDETECT); - } - - if (cpu_is_omap44xx()) { - bank->saved_datain = __raw_readl(bank->base + - OMAP4_GPIO_DATAIN); - l1 = __raw_readl(bank->base + - OMAP4_GPIO_FALLINGDETECT); - l2 = __raw_readl(bank->base + - OMAP4_GPIO_RISINGDETECT); - } + bank->saved_datain = __raw_readl(bank->base + + bank->regs->datain); + l1 = __raw_readl(bank->base + bank->regs->fallingdetect); + l2 = __raw_readl(bank->base + bank->regs->risingdetect); bank->saved_fallingdetect = l1; bank->saved_risingdetect = l2; l1 &= ~bank->enabled_non_wakeup_gpios; l2 &= ~bank->enabled_non_wakeup_gpios; - if (cpu_is_omap24xx() || cpu_is_omap34xx()) { - __raw_writel(l1, bank->base + - OMAP24XX_GPIO_FALLINGDETECT); - __raw_writel(l2, bank->base + - OMAP24XX_GPIO_RISINGDETECT); - } - - if (cpu_is_omap44xx()) { - __raw_writel(l1, bank->base + OMAP4_GPIO_FALLINGDETECT); - __raw_writel(l2, bank->base + OMAP4_GPIO_RISINGDETECT); - } + __raw_writel(l1, bank->base + bank->regs->fallingdetect); + __raw_writel(l2, bank->base + bank->regs->risingdetect); save_gpio_ctx: if (bank->get_context_loss_count) @@ -1317,21 +1288,11 @@ void omap2_gpio_resume_after_idle(void) if (!(bank->enabled_non_wakeup_gpios)) continue; - if (cpu_is_omap24xx() || cpu_is_omap34xx()) { - __raw_writel(bank->saved_fallingdetect, - bank->base + OMAP24XX_GPIO_FALLINGDETECT); - __raw_writel(bank->saved_risingdetect, - bank->base + OMAP24XX_GPIO_RISINGDETECT); - l = __raw_readl(bank->base + OMAP24XX_GPIO_DATAIN); - } - - if (cpu_is_omap44xx()) { - __raw_writel(bank->saved_fallingdetect, - bank->base + OMAP4_GPIO_FALLINGDETECT); - __raw_writel(bank->saved_risingdetect, - bank->base + OMAP4_GPIO_RISINGDETECT); - l = __raw_readl(bank->base + OMAP4_GPIO_DATAIN); - } + __raw_writel(bank->saved_fallingdetect, + bank->base + bank->regs->fallingdetect); + __raw_writel(bank->saved_risingdetect, + bank->base + bank->regs->risingdetect); + l = __raw_readl(bank->base + bank->regs->datain); /* Check if any of the non-wakeup interrupt GPIOs have changed * state. If so, generate an IRQ by software. This is @@ -1359,35 +1320,28 @@ void omap2_gpio_resume_after_idle(void) if (gen) { u32 old0, old1; + old0 = __raw_readl(bank->base + + bank->regs->leveldetect0); + old1 = __raw_readl(bank->base + + bank->regs->leveldetect1); + + __raw_writel(old0, bank->base + + bank->regs->leveldetect0); + __raw_writel(old1, bank->base + + bank->regs->leveldetect1); if (cpu_is_omap24xx() || cpu_is_omap34xx()) { - old0 = __raw_readl(bank->base + - OMAP24XX_GPIO_LEVELDETECT0); - old1 = __raw_readl(bank->base + - OMAP24XX_GPIO_LEVELDETECT1); - __raw_writel(old0 | gen, bank->base + - OMAP24XX_GPIO_LEVELDETECT0); - __raw_writel(old1 | gen, bank->base + - OMAP24XX_GPIO_LEVELDETECT1); - __raw_writel(old0, bank->base + - OMAP24XX_GPIO_LEVELDETECT0); - __raw_writel(old1, bank->base + - OMAP24XX_GPIO_LEVELDETECT1); + old0 |= gen; + old1 |= gen; } if (cpu_is_omap44xx()) { - old0 = __raw_readl(bank->base + - OMAP4_GPIO_LEVELDETECT0); - old1 = __raw_readl(bank->base + - OMAP4_GPIO_LEVELDETECT1); - __raw_writel(old0 | l, bank->base + - OMAP4_GPIO_LEVELDETECT0); - __raw_writel(old1 | l, bank->base + - OMAP4_GPIO_LEVELDETECT1); - __raw_writel(old0, bank->base + - OMAP4_GPIO_LEVELDETECT0); - __raw_writel(old1, bank->base + - OMAP4_GPIO_LEVELDETECT1); + old0 |= l; + old1 |= l; } + __raw_writel(old0, bank->base + + bank->regs->leveldetect0); + __raw_writel(old1, bank->base + + bank->regs->leveldetect1); } } }