linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] ARM: omap4: gpio: fix setting IRQWAKEN bits
@ 2011-06-04 19:03 Colin Cross
  2011-06-05  2:37 ` Colin Cross
  2011-06-06  7:02 ` Santosh Shilimkar
  0 siblings, 2 replies; 6+ messages in thread
From: Colin Cross @ 2011-06-04 19:03 UTC (permalink / raw)
  To: linux-omap
  Cc: Colin Cross, Tony Lindgren, Russell King, linux-arm-kernel, linux-kernel

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);
 		} else {
 			/*
 			 * GPIO wakeup request can only be generated on edge
-- 
1.7.4.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] ARM: omap4: gpio: fix setting IRQWAKEN bits
  2011-06-04 19:03 [PATCH] ARM: omap4: gpio: fix setting IRQWAKEN bits Colin Cross
@ 2011-06-05  2:37 ` Colin Cross
       [not found]   ` <87r576wpz6.fsf@ti.com>
  2011-06-06  7:02 ` Santosh Shilimkar
  1 sibling, 1 reply; 6+ messages in thread
From: Colin Cross @ 2011-06-05  2:37 UTC (permalink / raw)
  To: l-o
  Cc: Colin Cross, Tony Lindgren, Russell King, linux-arm-kernel, lkml,
	Todd Poynor

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
>
>

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] ARM: omap4: gpio: fix setting IRQWAKEN bits
  2011-06-04 19:03 [PATCH] ARM: omap4: gpio: fix setting IRQWAKEN bits Colin Cross
  2011-06-05  2:37 ` Colin Cross
@ 2011-06-06  7:02 ` Santosh Shilimkar
  2011-06-06  7:23   ` DebBarma, Tarun Kanti
  1 sibling, 1 reply; 6+ messages in thread
From: Santosh Shilimkar @ 2011-06-06  7:02 UTC (permalink / raw)
  To: Colin Cross
  Cc: linux-omap, Tony Lindgren, Russell King, linux-kernel, linux-arm-kernel

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>	


^ permalink raw reply	[flat|nested] 6+ messages in thread

* RE: [PATCH] ARM: omap4: gpio: fix setting IRQWAKEN bits
  2011-06-06  7:02 ` Santosh Shilimkar
@ 2011-06-06  7:23   ` DebBarma, Tarun Kanti
  0 siblings, 0 replies; 6+ messages in thread
From: DebBarma, Tarun Kanti @ 2011-06-06  7:23 UTC (permalink / raw)
  To: Shilimkar, Santosh, Colin Cross
  Cc: linux-omap, Tony Lindgren, Russell King, linux-kernel, linux-arm-kernel

> -----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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] ARM: omap4: gpio: fix setting IRQWAKEN bits
       [not found]   ` <87r576wpz6.fsf@ti.com>
@ 2011-06-06 23:05     ` Todd Poynor
  2011-06-06 23:25       ` Kevin Hilman
  0 siblings, 1 reply; 6+ messages in thread
From: Todd Poynor @ 2011-06-06 23:05 UTC (permalink / raw)
  To: Kevin Hilman
  Cc: l-o, Tony Lindgren, Russell King, linux-arm-kernel, lkml, Colin Cross

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] ARM: omap4: gpio: fix setting IRQWAKEN bits
  2011-06-06 23:05     ` Todd Poynor
@ 2011-06-06 23:25       ` Kevin Hilman
  0 siblings, 0 replies; 6+ messages in thread
From: Kevin Hilman @ 2011-06-06 23:25 UTC (permalink / raw)
  To: Todd Poynor
  Cc: l-o, Tony Lindgren, Russell King, linux-arm-kernel, lkml, Colin Cross

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


^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2011-06-06 23:26 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-06-04 19:03 [PATCH] ARM: omap4: gpio: fix setting IRQWAKEN bits Colin Cross
2011-06-05  2:37 ` Colin Cross
     [not found]   ` <87r576wpz6.fsf@ti.com>
2011-06-06 23:05     ` Todd Poynor
2011-06-06 23:25       ` Kevin Hilman
2011-06-06  7:02 ` Santosh Shilimkar
2011-06-06  7:23   ` DebBarma, Tarun Kanti

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).