ARM: omap4: gpio: fix setting IRQWAKEN bits

PATCH Superseded Colin Cross 5 participants 7 messages Jun 4, 2011 – Jun 6, 2011

Subsystems (2)

Arm (32-bit) Arm sub-architectures

lore

Share & tools

lore

Colin Cross

author

2011-06-04 19:03 UTC #0 lore raw

PATCH 1/1 ARM: omap4: gpio: fix setting IRQWAKEN bits

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 <[email protected]>

---
 arch/arm/plat-omap/gpio.c |   14 +++++---------
 1 files changed, 5 insertions(+), 9 deletions(-)

arch/arm/plat-omap/gpio.c +5 -9

index c985652..23ac7b6 100644
@@ -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

Signature · 1 line

1.7.4.1

Colin Cross

author

2011-06-05 02:37 UTC #1 lore raw

Quoted 31 lines · Colin Cross

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 <[email protected]>
---
?arch/arm/plat-omap/gpio.c | ? 14 +++++---------
?1 files changed, 5 insertions(+), 9 deletions(-)
arch/arm/plat-omap/gpio.c +0 -4
index c985652..23ac7b6 100644
@@ -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 {

arch/arm/plat-omap/gpio.c static inline void set_24xx_gpio_triggering(struct gpio_bank *bank, int gpio, · line 569

Quoted 10 lines

arch/arm/plat-omap/gpio.c +5 -5

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

Quoted 7 lines

arch/arm/plat-omap/gpio.c +0 -0
 ? ? ? ? ? ? ? ?} else {
? ? ? ? ? ? ? ? ? ? ? ?/*
? ? ? ? ? ? ? ? ? ? ? ? * GPIO wakeup request can only be generated on edge --
1.7.4.1

Kevin Hilman

2011-06-06 22:32 UTC #2 lore raw

PATCH 1/1 ARM: omap4: gpio: fix setting IRQWAKEN bits

Quoted 9 lines · Colin Cross

Quoted 8 lines · Colin Cross
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 <[email protected]>

[...]

Quoted 3 lines

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.

Thanks,

Kevin

Todd Poynor

2011-06-06 23:05 UTC #3 lore raw

Quoted 15 lines · Kevin Hilman

Quoted 9 lines · Colin Cross
Quoted 8 lines · Colin Cross
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 <[email protected]>
[...]
Quoted 1 line

Quoted 6 lines

Quoted 2 lines
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

Kevin Hilman

2011-06-06 23:25 UTC #4 lore raw

Quoted 25 lines · Todd Poynor

Quoted 15 lines · Kevin Hilman
Quoted 9 lines · Colin Cross
Quoted 8 lines · Colin Cross
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 <[email protected]>
[...]
Quoted 1 line
Quoted 6 lines
Quoted 2 lines
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

Quoted 7 lines

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

Santosh Shilimkar

Acked-by on v1 1/1

2011-06-06 07:02 UTC #5 lore raw

In reply to Colin Cross

Quoted 8 lines · Colin Cross

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<[email protected]>

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 <[email protected]>

DebBarma, Tarun Kanti

2011-06-06 07:23 UTC #6 lore raw

Quoted 17 lines

-----Original Message-----
From: linux-omap-owner at vger.kernel.org [mailto:linux-omap- owner at vger.kernel.org] On Behalf Of Shilimkar, Santosh
Sent: Monday, June 06, 2011 12:32 PM
To: Colin Cross
Cc: linux-omap at vger.kernel.org; Tony Lindgren; Russell King; linux- kernel at vger.kernel.org; linux-arm-kernel at lists.infradead.org
Subject: Re: [PATCH] ARM: omap4: gpio: fix setting IRQWAKEN bits
Quoted 7 lines · Colin Cross
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.

Quoted 7 lines

Quoted 1 line
Signed-off-by Colin Cross<[email protected]>
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 <[email protected]>

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

Quoted 5 lines

--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo at vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html