| Message ID | 1307214239-16316-1-git-send-email-ccross@android.com (mailing list archive) |
|---|---|
| State | Not Applicable |
| Delegated to: | Kevin Hilman |
| Headers | show |
On Sat, Jun 4, 2011 at 12:03 PM, Colin Cross <ccross@android.com> wrote: > Setting the IRQWAKEN bit was overwriting previous IRQWAKEN bits, > causing only the last bit set to take effect, resulting in lost > wakeups when the GPIO controller is in idle. > > Replace direct writes to IRQWAKEN with writes to SETWKUENA and > CLEARWKUEN. > > Signed-off-by: Colin Cross <ccross@android.com> > --- > arch/arm/plat-omap/gpio.c | 14 +++++--------- > 1 files changed, 5 insertions(+), 9 deletions(-) > > diff --git a/arch/arm/plat-omap/gpio.c b/arch/arm/plat-omap/gpio.c > index c985652..23ac7b6 100644 > --- a/arch/arm/plat-omap/gpio.c > +++ b/arch/arm/plat-omap/gpio.c > @@ -539,7 +539,6 @@ static inline void set_24xx_gpio_triggering(struct gpio_bank *bank, int gpio, > { > void __iomem *base = bank->base; > u32 gpio_bit = 1 << gpio; > - u32 val; > > if (cpu_is_omap44xx()) { > MOD_REG_BIT(OMAP4_GPIO_LEVELDETECT0, gpio_bit, > @@ -563,14 +562,11 @@ static inline void set_24xx_gpio_triggering(struct gpio_bank *bank, int gpio, > if (likely(!(bank->non_wakeup_gpios & gpio_bit))) { > if (cpu_is_omap44xx()) { > if (trigger != 0) > - __raw_writel(1 << gpio, bank->base+ > - OMAP4_GPIO_IRQWAKEN0); > - else { > - val = __raw_readl(bank->base + > - OMAP4_GPIO_IRQWAKEN0); > - __raw_writel(val & (~(1 << gpio)), bank->base + > - OMAP4_GPIO_IRQWAKEN0); > - } > + __raw_writel(gpio_bit, > + bank->base + OMAP4_GPIO_SETWKUENA); > + else > + __raw_writel(gpio_bit, > + bank->base + OMAP4_GPIO_CLEARWKUENA); Todd pointed out that the OMAP4 TRM says not to use SETWKUENA and CLEARWKUENA. I'll send another patch that applies to v3.0-rc1 that uses MOD_REG_BIT on IRQWAKEN_0, and another patch that adds the necessary locking around the read-modify-writes in _set_gpio_triggering. > } else { > /* > * GPIO wakeup request can only be generated on edge > -- > 1.7.4.1 > > -- 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
On 6/5/2011 12:33 AM, Colin Cross wrote: > Setting the IRQWAKEN bit was overwriting previous IRQWAKEN bits, > causing only the last bit set to take effect, resulting in lost > wakeups when the GPIO controller is in idle. > > Replace direct writes to IRQWAKEN with writes to SETWKUENA and > CLEARWKUEN. > > Signed-off-by: Colin Cross<ccross@android.com> Looks good. This needs to be fixed on top of the clean-up branch where GPIO movemnt to driver/gpio/* happening. For this change, Acked-by: Santosh Shilimkar <santosh.shilimkar@ti.com> -- 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: linux-omap-owner@vger.kernel.org [mailto:linux-omap- > owner@vger.kernel.org] On Behalf Of Shilimkar, Santosh > Sent: Monday, June 06, 2011 12:32 PM > To: Colin Cross > Cc: linux-omap@vger.kernel.org; Tony Lindgren; Russell King; linux- > kernel@vger.kernel.org; linux-arm-kernel@lists.infradead.org > Subject: Re: [PATCH] ARM: omap4: gpio: fix setting IRQWAKEN bits > > On 6/5/2011 12:33 AM, Colin Cross wrote: > > Setting the IRQWAKEN bit was overwriting previous IRQWAKEN bits, > > causing only the last bit set to take effect, resulting in lost > > wakeups when the GPIO controller is in idle. > > > > Replace direct writes to IRQWAKEN with writes to SETWKUENA and > > CLEARWKUEN. > > > > Signed-off-by: Colin Cross<ccross@android.com> > > Looks good. This needs to be fixed on top of the clean-up > branch where GPIO movemnt to driver/gpio/* happening. > > For this change, > Acked-by: Santosh Shilimkar <santosh.shilimkar@ti.com> This is also available as part of following cleanup series we are doing. However, since this is a defect we can prioritize it over the cleanup. -- Tarun > > -- > 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 -- 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
On Mon, Jun 06, 2011 at 03:32:29PM -0700, Kevin Hilman wrote: > Colin Cross <ccross@android.com> writes: > > > On Sat, Jun 4, 2011 at 12:03 PM, Colin Cross <ccross@android.com> wrote: > >> Setting the IRQWAKEN bit was overwriting previous IRQWAKEN bits, > >> causing only the last bit set to take effect, resulting in lost > >> wakeups when the GPIO controller is in idle. > >> > >> Replace direct writes to IRQWAKEN with writes to SETWKUENA and > >> CLEARWKUEN. > >> > >> Signed-off-by: Colin Cross <ccross@android.com> > > [...] > > > > > Todd pointed out that the OMAP4 TRM says not to use SETWKUENA and > > CLEARWKUENA. > > In my GPIO cleanups, I was wondering why the set/clear registers were > not used here. Todd, can you give the TRM version & reference for this? > I didn't find anything after a quick scan/search. http://focus.ti.com/general/docs/wtbu/wtbudocumentcenter.tsp?templateId=6123&navigationId=12037 OMAP4460 Multimedia Device Silicon Revision 1.0 Version E February 2011–Revised May 2011 p. 5357 Table 25-68. GPIO_CLEARWKUPENA "Caution: This register is kept for backward software compatibility. Use GPIO_IRQWAKEN_0 and GPIO_IRQWAKEN_1 instead." OMAP4430 TRM is similar. Todd -- 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
Todd Poynor <toddpoynor@google.com> writes: > On Mon, Jun 06, 2011 at 03:32:29PM -0700, Kevin Hilman wrote: >> Colin Cross <ccross@android.com> writes: >> >> > On Sat, Jun 4, 2011 at 12:03 PM, Colin Cross <ccross@android.com> wrote: >> >> Setting the IRQWAKEN bit was overwriting previous IRQWAKEN bits, >> >> causing only the last bit set to take effect, resulting in lost >> >> wakeups when the GPIO controller is in idle. >> >> >> >> Replace direct writes to IRQWAKEN with writes to SETWKUENA and >> >> CLEARWKUEN. >> >> >> >> Signed-off-by: Colin Cross <ccross@android.com> >> >> [...] >> >> > >> > Todd pointed out that the OMAP4 TRM says not to use SETWKUENA and >> > CLEARWKUENA. >> >> In my GPIO cleanups, I was wondering why the set/clear registers were >> not used here. Todd, can you give the TRM version & reference for this? >> I didn't find anything after a quick scan/search. > > http://focus.ti.com/general/docs/wtbu/wtbudocumentcenter.tsp?templateId=6123&navigationId=12037 > > OMAP4460 Multimedia Device Silicon Revision 1.0 Version E > February 2011–Revised May 2011 > > p. 5357 Table 25-68. GPIO_CLEARWKUPENA > > "Caution: This register is kept for backward software compatibility. Use > GPIO_IRQWAKEN_0 and GPIO_IRQWAKEN_1 instead." OK, I see it now. Thanks, 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/plat-omap/gpio.c b/arch/arm/plat-omap/gpio.c index c985652..23ac7b6 100644 --- a/arch/arm/plat-omap/gpio.c +++ b/arch/arm/plat-omap/gpio.c @@ -539,7 +539,6 @@ static inline void set_24xx_gpio_triggering(struct gpio_bank *bank, int gpio, { void __iomem *base = bank->base; u32 gpio_bit = 1 << gpio; - u32 val; if (cpu_is_omap44xx()) { MOD_REG_BIT(OMAP4_GPIO_LEVELDETECT0, gpio_bit, @@ -563,14 +562,11 @@ static inline void set_24xx_gpio_triggering(struct gpio_bank *bank, int gpio, if (likely(!(bank->non_wakeup_gpios & gpio_bit))) { if (cpu_is_omap44xx()) { if (trigger != 0) - __raw_writel(1 << gpio, bank->base+ - OMAP4_GPIO_IRQWAKEN0); - else { - val = __raw_readl(bank->base + - OMAP4_GPIO_IRQWAKEN0); - __raw_writel(val & (~(1 << gpio)), bank->base + - OMAP4_GPIO_IRQWAKEN0); - } + __raw_writel(gpio_bit, + bank->base + OMAP4_GPIO_SETWKUENA); + else + __raw_writel(gpio_bit, + bank->base + OMAP4_GPIO_CLEARWKUENA); } else { /* * GPIO wakeup request can only be generated on edge
Setting the IRQWAKEN bit was overwriting previous IRQWAKEN bits, causing only the last bit set to take effect, resulting in lost wakeups when the GPIO controller is in idle. Replace direct writes to IRQWAKEN with writes to SETWKUENA and CLEARWKUEN. Signed-off-by: Colin Cross <ccross@android.com> --- arch/arm/plat-omap/gpio.c | 14 +++++--------- 1 files changed, 5 insertions(+), 9 deletions(-)