| Message ID | 1308111806-29152-5-git-send-email-tarun.kanti@ti.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Kevin Hilman |
| Headers | show |
Tarun Kanti DebBarma <tarun.kanti@ti.com> writes: > From: Charulatha V <charu@ti.com> > > In gpio_irq_handler, isr register is always accessed as 32 bit register and only > for OMAP15xx the first 16 MSBs are masked. Correct this by using the appropriate > readl/readw registers as per the bank width. > > Signed-off-by: Charulatha V <charu@ti.com> > --- > drivers/gpio/gpio-omap.c | 8 +++++--- > 1 files changed, 5 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpio/gpio-omap.c b/drivers/gpio/gpio-omap.c > index c6987f2..01b568f 100644 > --- a/drivers/gpio/gpio-omap.c > +++ b/drivers/gpio/gpio-omap.c > @@ -590,10 +590,12 @@ static void gpio_irq_handler(unsigned int irq, struct irq_desc *desc) > u32 enabled; > > 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 (bank->width == 32) > + isr = __raw_readl(isr_reg) & enabled; > + else if (bank->width == 16) > + isr = (__raw_readw(isr_reg) & enabled) & 0x0000ffff; Minor nit: is the '& 0xffff' really needed. The 'enabled' mask is already masked using the bank width. Kevin > + isr_saved = isr; > > if (bank->regs->leveldetect0) > level_mask = bank->level_mask & enabled; -- 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> > > In gpio_irq_handler, isr register is always accessed as 32 bit register and only > for OMAP15xx the first 16 MSBs are masked. Correct this by using the appropriate > readl/readw registers as per the bank width. > > Signed-off-by: Charulatha V <charu@ti.com> > --- > drivers/gpio/gpio-omap.c | 8 +++++--- > 1 files changed, 5 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpio/gpio-omap.c b/drivers/gpio/gpio-omap.c > index c6987f2..01b568f 100644 > --- a/drivers/gpio/gpio-omap.c > +++ b/drivers/gpio/gpio-omap.c > @@ -590,10 +590,12 @@ static void gpio_irq_handler(unsigned int irq, struct irq_desc *desc) > u32 enabled; > > 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 (bank->width == 32) > + isr = __raw_readl(isr_reg) & enabled; > + else if (bank->width == 16) > + isr = (__raw_readw(isr_reg) & enabled) & 0x0000ffff; > + isr_saved = isr; Come to think of it, even when bank->width is 16, all the OMAP1 registers are 4-byte aligned, so just use a 4-byte read. The 'enabled' mask is already taking care to mask for bank width. 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: Thursday, June 16, 2011 11:28 PM > To: DebBarma, Tarun Kanti > Cc: linux-omap@vger.kernel.org; Shilimkar, Santosh; tony@atomide.com > Subject: Re: [PATCH v2 14/18] GPIO: OMAP: Fix use of readl/readw to access > isr_reg > > Tarun Kanti DebBarma <tarun.kanti@ti.com> writes: > > > From: Charulatha V <charu@ti.com> > > > > In gpio_irq_handler, isr register is always accessed as 32 bit register > and only > > for OMAP15xx the first 16 MSBs are masked. Correct this by using the > appropriate > > readl/readw registers as per the bank width. > > > > Signed-off-by: Charulatha V <charu@ti.com> > > --- > > drivers/gpio/gpio-omap.c | 8 +++++--- > > 1 files changed, 5 insertions(+), 3 deletions(-) > > > > diff --git a/drivers/gpio/gpio-omap.c b/drivers/gpio/gpio-omap.c > > index c6987f2..01b568f 100644 > > --- a/drivers/gpio/gpio-omap.c > > +++ b/drivers/gpio/gpio-omap.c > > @@ -590,10 +590,12 @@ static void gpio_irq_handler(unsigned int irq, > struct irq_desc *desc) > > u32 enabled; > > > > 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 (bank->width == 32) > > + isr = __raw_readl(isr_reg) & enabled; > > + else if (bank->width == 16) > > + isr = (__raw_readw(isr_reg) & enabled) & 0x0000ffff; > > + isr_saved = isr; > > Come to think of it, even when bank->width is 16, all the OMAP1 > registers are 4-byte aligned, so just use a 4-byte read. The 'enabled' > mask is already taking care to mask for bank width. Yes, that makes sense. I will change. -- 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
> -----Original Message----- > From: Hilman, Kevin > Sent: Thursday, June 16, 2011 11:24 PM > To: DebBarma, Tarun Kanti > Cc: linux-omap@vger.kernel.org; Shilimkar, Santosh; tony@atomide.com > Subject: Re: [PATCH v2 14/18] GPIO: OMAP: Fix use of readl/readw to access > isr_reg > > Tarun Kanti DebBarma <tarun.kanti@ti.com> writes: > > > From: Charulatha V <charu@ti.com> > > > > In gpio_irq_handler, isr register is always accessed as 32 bit register > and only > > for OMAP15xx the first 16 MSBs are masked. Correct this by using the > appropriate > > readl/readw registers as per the bank width. > > > > Signed-off-by: Charulatha V <charu@ti.com> > > --- > > drivers/gpio/gpio-omap.c | 8 +++++--- > > 1 files changed, 5 insertions(+), 3 deletions(-) > > > > diff --git a/drivers/gpio/gpio-omap.c b/drivers/gpio/gpio-omap.c > > index c6987f2..01b568f 100644 > > --- a/drivers/gpio/gpio-omap.c > > +++ b/drivers/gpio/gpio-omap.c > > @@ -590,10 +590,12 @@ static void gpio_irq_handler(unsigned int irq, > struct irq_desc *desc) > > u32 enabled; > > > > 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 (bank->width == 32) > > + isr = __raw_readl(isr_reg) & enabled; > > + else if (bank->width == 16) > > + isr = (__raw_readw(isr_reg) & enabled) & 0x0000ffff; > > Minor nit: is the '& 0xffff' really needed. The 'enabled' mask is > already masked using the bank width. Right, that's you pointed out. Thanks. -- Tarun > > Kevin > > > + isr_saved = isr; > > > > if (bank->regs->leveldetect0) > > level_mask = bank->level_mask & enabled; -- 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/drivers/gpio/gpio-omap.c b/drivers/gpio/gpio-omap.c index c6987f2..01b568f 100644 --- a/drivers/gpio/gpio-omap.c +++ b/drivers/gpio/gpio-omap.c @@ -590,10 +590,12 @@ static void gpio_irq_handler(unsigned int irq, struct irq_desc *desc) u32 enabled; 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 (bank->width == 32) + isr = __raw_readl(isr_reg) & enabled; + else if (bank->width == 16) + isr = (__raw_readw(isr_reg) & enabled) & 0x0000ffff; + isr_saved = isr; if (bank->regs->leveldetect0) level_mask = bank->level_mask & enabled;