[PATCH] OMAP3: GPIO fixes for off-mode

[PATCH] OMAP3: GPIO fixes for off-mode

[Tero Kristo]

Off mode is now using the omap2 retention fix code for scanning GPIOs
during off-mode transitions. All the *non_wakeup_gpios variables
are now used for off-mode transition tracking on OMAP3. This patch fixes
cases where GPIO state changes are missed during off-mode.

Signed-off-by: Tero Kristo <tero.kristo@nokia.com>
---
 arch/arm/mach-omap2/pm34xx.c |   10 ++++++----
 arch/arm/plat-omap/gpio.c    |   29 ++++++++++++++++++++---------
 2 files changed, 26 insertions(+), 13 deletions(-)

diff --git a/arch/arm/mach-omap2/pm34xx.c b/arch/arm/mach-omap2/pm34xx.c
index c67d770..66f2b22 100644
--- a/arch/arm/mach-omap2/pm34xx.c
+++ b/arch/arm/mach-omap2/pm34xx.c
@@ -330,14 +330,15 @@ void omap_sram_idle(void)
 	core_next_state = pwrdm_read_next_pwrst(core_pwrdm);
 	if (per_next_state < PWRDM_POWER_ON) {
 		omap_uart_prepare_idle(2);
-		omap2_gpio_prepare_for_retention();
 		if (per_next_state == PWRDM_POWER_OFF) {
 			if (core_next_state == PWRDM_POWER_ON) {
 				per_next_state = PWRDM_POWER_RET;
 				pwrdm_set_next_pwrst(per_pwrdm, per_next_state);
 				per_state_modified = 1;
-			} else
+			} else {
+				omap2_gpio_prepare_for_retention();
 				omap3_per_save_context();
+			}
 		}
 	}
 
@@ -411,9 +412,10 @@ void omap_sram_idle(void)
 	/* PER */
 	if (per_next_state < PWRDM_POWER_ON) {
 		per_prev_state = pwrdm_read_prev_pwrst(per_pwrdm);
-		if (per_prev_state == PWRDM_POWER_OFF)
+		if (per_prev_state == PWRDM_POWER_OFF) {
 			omap3_per_restore_context();
-		omap2_gpio_resume_after_retention();
+			omap2_gpio_resume_after_retention();
+		}
 		omap_uart_resume_idle(2);
 		if (per_state_modified)
 			pwrdm_set_next_pwrst(per_pwrdm, PWRDM_POWER_OFF);
diff --git a/arch/arm/plat-omap/gpio.c b/arch/arm/plat-omap/gpio.c
index 83a5953..db40037 100644
--- a/arch/arm/plat-omap/gpio.c
+++ b/arch/arm/plat-omap/gpio.c
@@ -540,7 +540,9 @@ static inline void set_24xx_gpio_triggering(struct gpio_bank *bank, int gpio,
 		else
 			__raw_writel(1 << gpio, bank->base
 					+ OMAP24XX_GPIO_CLEARWKUENA);
-	} else {
+	}
+	/* This part needs to be executed always for OMAP34xx */
+	if (cpu_is_omap34xx() || (bank->non_wakeup_gpios & gpio_bit)) {
 		if (trigger != 0)
 			bank->enabled_non_wakeup_gpios |= gpio_bit;
 		else
@@ -1456,7 +1458,8 @@ static int __init _omap_gpio_init(void)
 
 			/* Initialize interface clock ungated, module enabled */
 			__raw_writel(0, bank->base + OMAP24XX_GPIO_CTRL);
-			if (i < ARRAY_SIZE(non_wakeup_gpios))
+			if (cpu_is_omap24xx() &&
+			    i < ARRAY_SIZE(non_wakeup_gpios))
 				bank->non_wakeup_gpios = non_wakeup_gpios[i];
 			gpio_count = 32;
 		}
@@ -1626,10 +1629,13 @@ static int workaround_enabled;
 void omap2_gpio_prepare_for_retention(void)
 {
 	int i, c = 0;
+	int min = 0;
 
+	if (cpu_is_omap34xx())
+		min = 1;
 	/* Remove triggering for all non-wakeup GPIOs.  Otherwise spurious
 	 * IRQs will be generated.  See OMAP2420 Errata item 1.101. */
-	for (i = 0; i < gpio_bank_count; i++) {
+	for (i = min; i < gpio_bank_count; i++) {
 		struct gpio_bank *bank = &gpio_bank[i];
 		u32 l1, l2;
 
@@ -1644,10 +1650,12 @@ void omap2_gpio_prepare_for_retention(void)
 		bank->saved_risingdetect = l2;
 		l1 &= ~bank->enabled_non_wakeup_gpios;
 		l2 &= ~bank->enabled_non_wakeup_gpios;
-#if defined(CONFIG_ARCH_OMAP24XX) || defined(CONFIG_ARCH_OMAP34XX)
-		__raw_writel(l1, bank->base + OMAP24XX_GPIO_FALLINGDETECT);
-		__raw_writel(l2, bank->base + OMAP24XX_GPIO_RISINGDETECT);
-#endif
+		if (cpu_is_omap24xx()) {
+			__raw_writel(l1, bank->base +
+					OMAP24XX_GPIO_FALLINGDETECT);
+			__raw_writel(l2, bank->base +
+					OMAP24XX_GPIO_RISINGDETECT);
+		}
 		c++;
 	}
 	if (!c) {
@@ -1660,10 +1668,13 @@ void omap2_gpio_prepare_for_retention(void)
 void omap2_gpio_resume_after_retention(void)
 {
 	int i;
+	int min = 0;
 
 	if (!workaround_enabled)
 		return;
-	for (i = 0; i < gpio_bank_count; i++) {
+	if (cpu_is_omap34xx())
+		min = 1;
+	for (i = min; i < gpio_bank_count; i++) {
 		struct gpio_bank *bank = &gpio_bank[i];
 		u32 l;
 
@@ -1683,7 +1694,7 @@ void omap2_gpio_resume_after_retention(void)
 		l = __raw_readl(bank->base + OMAP24XX_GPIO_DATAIN);
 #endif
 		l ^= bank->saved_datain;
-		l &= bank->non_wakeup_gpios;
+		l &= bank->enabled_non_wakeup_gpios;
 		if (l) {
 			u32 old0, old1;
 #if defined(CONFIG_ARCH_OMAP24XX) || defined(CONFIG_ARCH_OMAP34XX)
-- 
1.5.4.3

Re: [PATCH] OMAP3: GPIO fixes for off-mode

[Kevin Hilman]

Tero Kristo <tero.kristo@nokia.com> writes:

> From:	Tero Kristo <tero.kristo@nokia.com>
> Subject: [PATCH] OMAP3: GPIO fixes for off-mode
> To:	linux-omap@vger.kernel.org
> Date:	Mon, 22 Dec 2008 14:27:12 +0200
>
> Off mode is now using the omap2 retention fix code for scanning GPIOs
> during off-mode transitions. All the *non_wakeup_gpios variables
> are now used for off-mode transition tracking on OMAP3. This patch fixes
> cases where GPIO state changes are missed during off-mode.
>
> Signed-off-by: Tero Kristo <tero.kristo@nokia.com>

Looks good now, thanks.  Pulling into PM branch.

Kevin

> ---
>  arch/arm/mach-omap2/pm34xx.c |   10 ++++++----
>  arch/arm/plat-omap/gpio.c    |   29 ++++++++++++++++++++---------
>  2 files changed, 26 insertions(+), 13 deletions(-)
>
> diff --git a/arch/arm/mach-omap2/pm34xx.c b/arch/arm/mach-omap2/pm34xx.c
> index c67d770..66f2b22 100644
> --- a/arch/arm/mach-omap2/pm34xx.c
> +++ b/arch/arm/mach-omap2/pm34xx.c
> @@ -330,14 +330,15 @@ void omap_sram_idle(void)
>  	core_next_state = pwrdm_read_next_pwrst(core_pwrdm);
>  	if (per_next_state < PWRDM_POWER_ON) {
>  		omap_uart_prepare_idle(2);
> -		omap2_gpio_prepare_for_retention();
>  		if (per_next_state == PWRDM_POWER_OFF) {
>  			if (core_next_state == PWRDM_POWER_ON) {
>  				per_next_state = PWRDM_POWER_RET;
>  				pwrdm_set_next_pwrst(per_pwrdm, per_next_state);
>  				per_state_modified = 1;
> -			} else
> +			} else {
> +				omap2_gpio_prepare_for_retention();
>  				omap3_per_save_context();
> +			}
>  		}
>  	}
>  
> @@ -411,9 +412,10 @@ void omap_sram_idle(void)
>  	/* PER */
>  	if (per_next_state < PWRDM_POWER_ON) {
>  		per_prev_state = pwrdm_read_prev_pwrst(per_pwrdm);
> -		if (per_prev_state == PWRDM_POWER_OFF)
> +		if (per_prev_state == PWRDM_POWER_OFF) {
>  			omap3_per_restore_context();
> -		omap2_gpio_resume_after_retention();
> +			omap2_gpio_resume_after_retention();
> +		}
>  		omap_uart_resume_idle(2);
>  		if (per_state_modified)
>  			pwrdm_set_next_pwrst(per_pwrdm, PWRDM_POWER_OFF);
> diff --git a/arch/arm/plat-omap/gpio.c b/arch/arm/plat-omap/gpio.c
> index 83a5953..db40037 100644
> --- a/arch/arm/plat-omap/gpio.c
> +++ b/arch/arm/plat-omap/gpio.c
> @@ -540,7 +540,9 @@ static inline void set_24xx_gpio_triggering(struct gpio_bank *bank, int gpio,
>  		else
>  			__raw_writel(1 << gpio, bank->base
>  					+ OMAP24XX_GPIO_CLEARWKUENA);
> -	} else {
> +	}
> +	/* This part needs to be executed always for OMAP34xx */
> +	if (cpu_is_omap34xx() || (bank->non_wakeup_gpios & gpio_bit)) {
>  		if (trigger != 0)
>  			bank->enabled_non_wakeup_gpios |= gpio_bit;
>  		else
> @@ -1456,7 +1458,8 @@ static int __init _omap_gpio_init(void)
>  
>  			/* Initialize interface clock ungated, module enabled */
>  			__raw_writel(0, bank->base + OMAP24XX_GPIO_CTRL);
> -			if (i < ARRAY_SIZE(non_wakeup_gpios))

Resending: OMAP3 GPIO fixes for off-mode

[Tero Kristo]

Resending this patch as the previous version breaks after the PER vs. CORE
dependency patch. Any comments to this by the way?

-Tero

[PATCH] OMAP3: GPIO fixes for off-mode

[Tero Kristo]

Off mode is now using the omap2 retention fix code for scanning GPIOs
only during off-mode transitions. All the *non_wakeup_gpios variables
are now used for off-mode transition tracking on OMAP3. This patch fixes
cases where GPIO state changes are missed during off-mode.

Signed-off-by: Tero Kristo <tero.kristo@nokia.com>
---
 arch/arm/mach-omap2/pm34xx.c |   10 ++++++----
 arch/arm/plat-omap/gpio.c    |   18 ++++++++++++++++--
 2 files changed, 22 insertions(+), 6 deletions(-)

diff --git a/arch/arm/mach-omap2/pm34xx.c b/arch/arm/mach-omap2/pm34xx.c
index 152b4e7..082e4f4 100644
--- a/arch/arm/mach-omap2/pm34xx.c
+++ b/arch/arm/mach-omap2/pm34xx.c
@@ -324,14 +324,15 @@ void omap_sram_idle(void)
 	core_next_state = pwrdm_read_next_pwrst(core_pwrdm);
 	if (per_next_state < PWRDM_POWER_ON) {
 		omap_uart_prepare_idle(2);
-		omap2_gpio_prepare_for_retention();
 		if (per_next_state == PWRDM_POWER_OFF) {
 			if (core_next_state == PWRDM_POWER_ON) {
 				per_next_state = PWRDM_POWER_RET;
 				pwrdm_set_next_pwrst(per_pwrdm, per_next_state);
 				per_state_modified = 1;
-			} else
+			} else {
+				omap2_gpio_prepare_for_retention();
 				omap3_per_save_context();
+			}
 		}
 	}
 
@@ -404,9 +405,10 @@ void omap_sram_idle(void)
 	/* PER */
 	if (per_next_state < PWRDM_POWER_ON) {
 		per_prev_state = pwrdm_read_prev_pwrst(per_pwrdm);
-		if (per_prev_state == PWRDM_POWER_OFF)
+		if (per_prev_state == PWRDM_POWER_OFF) {
 			omap3_per_restore_context();
-		omap2_gpio_resume_after_retention();
+			omap2_gpio_resume_after_retention();
+		}
 		omap_uart_resume_idle(2);
 		if (per_state_modified)
 			pwrdm_set_next_pwrst(per_pwrdm, PWRDM_POWER_OFF);
diff --git a/arch/arm/plat-omap/gpio.c b/arch/arm/plat-omap/gpio.c
index 62349fd..b45bf9d 100644
--- a/arch/arm/plat-omap/gpio.c
+++ b/arch/arm/plat-omap/gpio.c
@@ -556,7 +556,9 @@ static inline void set_24xx_gpio_triggering(struct gpio_bank *bank, int gpio,
 		else
 			__raw_writel(1 << gpio, bank->base
 					+ OMAP24XX_GPIO_CLEARWKUENA);
+#ifdef CONFIG_ARCH_OMAP24XX
 	} else {
+#endif
 		if (trigger != 0)
 			bank->enabled_non_wakeup_gpios |= gpio_bit;
 		else
@@ -1485,9 +1487,11 @@ static int __init _omap_gpio_init(void)
 
 #if defined(CONFIG_ARCH_OMAP24XX) || defined(CONFIG_ARCH_OMAP34XX)
 		if (bank->method == METHOD_GPIO_24XX) {
+#ifdef CONFIG_ARCH_OMAP24XX
 			static const u32 non_wakeup_gpios[] = {
 				0xe203ffc0, 0x08700040
 			};
+#endif
 
 			__raw_writel(0x00000000, bank->base + OMAP24XX_GPIO_IRQENABLE1);
 			__raw_writel(0xffffffff, bank->base + OMAP24XX_GPIO_IRQSTATUS1);
@@ -1495,8 +1499,10 @@ static int __init _omap_gpio_init(void)
 
 			/* Initialize interface clock ungated, module enabled */
 			__raw_writel(0, bank->base + OMAP24XX_GPIO_CTRL);
+#ifdef CONFIG_ARCH_OMAP24XX
 			if (i < ARRAY_SIZE(non_wakeup_gpios))
 				bank->non_wakeup_gpios = non_wakeup_gpios[i];
+#endif
 			gpio_count = 32;
 		}
 #endif
@@ -1666,7 +1672,11 @@ void omap2_gpio_prepare_for_retention(void)
 
 	/* Remove triggering for all non-wakeup GPIOs.  Otherwise spurious
 	 * IRQs will be generated.  See OMAP2420 Errata item 1.101. */
+#ifdef CONFIG_ARCH_OMAP24XX
 	for (i = 0; i < gpio_bank_count; i++) {
+#else
+	for (i = 1; i < gpio_bank_count; i++) {
+#endif
 		struct gpio_bank *bank = &gpio_bank[i];
 		u32 l1, l2;
 
@@ -1681,7 +1691,7 @@ void omap2_gpio_prepare_for_retention(void)
 		bank->saved_risingdetect = l2;
 		l1 &= ~bank->enabled_non_wakeup_gpios;
 		l2 &= ~bank->enabled_non_wakeup_gpios;
-#if defined(CONFIG_ARCH_OMAP24XX) || defined(CONFIG_ARCH_OMAP34XX)
+#if defined(CONFIG_ARCH_OMAP24XX)
 		__raw_writel(l1, bank->base + OMAP24XX_GPIO_FALLINGDETECT);
 		__raw_writel(l2, bank->base + OMAP24XX_GPIO_RISINGDETECT);
 #endif
@@ -1700,7 +1710,11 @@ void omap2_gpio_resume_after_retention(void)
 
 	if (!workaround_enabled)
 		return;
+#ifdef CONFIG_ARCH_OMAP24XX
 	for (i = 0; i < gpio_bank_count; i++) {
+#else
+	for (i = 1; i < gpio_bank_count; i++) {
+#endif
 		struct gpio_bank *bank = &gpio_bank[i];
 		u32 l;
 
@@ -1720,7 +1734,7 @@ void omap2_gpio_resume_after_retention(void)
 		l = __raw_readl(bank->base + OMAP24XX_GPIO_DATAIN);
 #endif
 		l ^= bank->saved_datain;
-		l &= bank->non_wakeup_gpios;
+		l &= bank->enabled_non_wakeup_gpios;
 		if (l) {
 			u32 old0, old1;
 #if defined(CONFIG_ARCH_OMAP24XX) || defined(CONFIG_ARCH_OMAP34XX)
-- 
1.5.4.3

Re: [PATCH] OMAP3: GPIO fixes for off-mode

[Kevin Hilman]

Tero Kristo <tero.kristo@nokia.com> writes:

> Off mode is now using the omap2 retention fix code for scanning GPIOs
> only during off-mode transitions. All the *non_wakeup_gpios variables
> are now used for off-mode transition tracking on OMAP3. This patch fixes
> cases where GPIO state changes are missed during off-mode.
>
> Signed-off-by: Tero Kristo <tero.kristo@nokia.com>

Can you rework this slightly to use runtime cpu_is_* checks instead
of compile-time #ifdefs?

This will currently not do the "right thing" on a kernel compiled
with both omap2 and omap3 support.

Thanks,

Kevin

> ---
>  arch/arm/mach-omap2/pm34xx.c |   10 ++++++----
>  arch/arm/plat-omap/gpio.c    |   18 ++++++++++++++++--
>  2 files changed, 22 insertions(+), 6 deletions(-)
>
> diff --git a/arch/arm/mach-omap2/pm34xx.c b/arch/arm/mach-omap2/pm34xx.c
> index 152b4e7..082e4f4 100644
> --- a/arch/arm/mach-omap2/pm34xx.c
> +++ b/arch/arm/mach-omap2/pm34xx.c
> @@ -324,14 +324,15 @@ void omap_sram_idle(void)
>  	core_next_state = pwrdm_read_next_pwrst(core_pwrdm);
>  	if (per_next_state < PWRDM_POWER_ON) {
>  		omap_uart_prepare_idle(2);
> -		omap2_gpio_prepare_for_retention();
>  		if (per_next_state == PWRDM_POWER_OFF) {
>  			if (core_next_state == PWRDM_POWER_ON) {
>  				per_next_state = PWRDM_POWER_RET;
>  				pwrdm_set_next_pwrst(per_pwrdm, per_next_state);
>  				per_state_modified = 1;
> -			} else
> +			} else {
> +				omap2_gpio_prepare_for_retention();
>  				omap3_per_save_context();
> +			}
>  		}
>  	}
>  
> @@ -404,9 +405,10 @@ void omap_sram_idle(void)
>  	/* PER */
>  	if (per_next_state < PWRDM_POWER_ON) {
>  		per_prev_state = pwrdm_read_prev_pwrst(per_pwrdm);
> -		if (per_prev_state == PWRDM_POWER_OFF)
> +		if (per_prev_state == PWRDM_POWER_OFF) {
>  			omap3_per_restore_context();
> -		omap2_gpio_resume_after_retention();
> +			omap2_gpio_resume_after_retention();
> +		}
>  		omap_uart_resume_idle(2);
>  		if (per_state_modified)
>  			pwrdm_set_next_pwrst(per_pwrdm, PWRDM_POWER_OFF);
> diff --git a/arch/arm/plat-omap/gpio.c b/arch/arm/plat-omap/gpio.c
> index 62349fd..b45bf9d 100644
> --- a/arch/arm/plat-omap/gpio.c
> +++ b/arch/arm/plat-omap/gpio.c
> @@ -556,7 +556,9 @@ static inline void set_24xx_gpio_triggering(struct gpio_bank *bank, int gpio,
>  		else
>  			__raw_writel(1 << gpio, bank->base
>  					+ OMAP24XX_GPIO_CLEARWKUENA);
> +#ifdef CONFIG_ARCH_OMAP24XX
>  	} else {
> +#endif
>  		if (trigger != 0)
>  			bank->enabled_non_wakeup_gpios |= gpio_bit;
>  		else
> @@ -1485,9 +1487,11 @@ static int __init _omap_gpio_init(void)
>  
>  #if defined(CONFIG_ARCH_OMAP24XX) || defined(CONFIG_ARCH_OMAP34XX)
>  		if (bank->method == METHOD_GPIO_24XX) {
> +#ifdef CONFIG_ARCH_OMAP24XX
>  			static const u32 non_wakeup_gpios[] = {
>  				0xe203ffc0, 0x08700040
>  			};
> +#endif
>  
>  			__raw_writel(0x00000000, bank->base + OMAP24XX_GPIO_IRQENABLE1);
>  			__raw_writel(0xffffffff, bank->base + OMAP24XX_GPIO_IRQSTATUS1);
> @@ -1495,8 +1499,10 @@ static int __init _omap_gpio_init(void)
>  
>  			/* Initialize interface clock ungated, module enabled */
>  			__raw_writel(0, bank->base + OMAP24XX_GPIO_CTRL);
> +#ifdef CONFIG_ARCH_OMAP24XX
>  			if (i < ARRAY_SIZE(non_wakeup_gpios))
>  				bank->non_wakeup_gpios = non_wakeup_gpios[i];
> +#endif
>  			gpio_count = 32;
>  		}
>  #endif
> @@ -1666,7 +1672,11 @@ void omap2_gpio_prepare_for_retention(void)
>  
>  	/* Remove triggering for all non-wakeup GPIOs.  Otherwise spurious
>  	 * IRQs will be generated.  See OMAP2420 Errata item 1.101. */
> +#ifdef CONFIG_ARCH_OMAP24XX
>  	for (i = 0; i < gpio_bank_count; i++) {
> +#else
> +	for (i = 1; i < gpio_bank_count; i++) {
> +#endif
>  		struct gpio_bank *bank = &gpio_bank[i];
>  		u32 l1, l2;
>  
> @@ -1681,7 +1691,7 @@ void omap2_gpio_prepare_for_retention(void)
>  		bank->saved_risingdetect = l2;
>  		l1 &= ~bank->enabled_non_wakeup_gpios;
>  		l2 &= ~bank->enabled_non_wakeup_gpios;
> -#if defined(CONFIG_ARCH_OMAP24XX) || defined(CONFIG_ARCH_OMAP34XX)
> +#if defined(CONFIG_ARCH_OMAP24XX)
>  		__raw_writel(l1, bank->base + OMAP24XX_GPIO_FALLINGDETECT);
>  		__raw_writel(l2, bank->base + OMAP24XX_GPIO_RISINGDETECT);
>  #endif
> @@ -1700,7 +1710,11 @@ void omap2_gpio_resume_after_retention(void)
>  
>  	if (!workaround_enabled)
>  		return;
> +#ifdef CONFIG_ARCH_OMAP24XX
>  	for (i = 0; i < gpio_bank_count; i++) {
> +#else
> +	for (i = 1; i < gpio_bank_count; i++) {
> +#endif
>  		struct gpio_bank *bank = &gpio_bank[i];
>  		u32 l;
>  
> @@ -1720,7 +1734,7 @@ void omap2_gpio_resume_after_retention(void)
>  		l = __raw_readl(bank->base + OMAP24XX_GPIO_DATAIN);
>  #endif
>  		l ^= bank->saved_datain;
> -		l &= bank->non_wakeup_gpios;
> +		l &= bank->enabled_non_wakeup_gpios;
>  		if (l) {
>  			u32 old0, old1;
>  #if defined(CONFIG_ARCH_OMAP24XX) || defined(CONFIG_ARCH_OMAP34XX)
> -- 
> 1.5.4.3
>
> --
> 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

Re: [PATCH] OMAP3: GPIO fixes for off-mode

[David Brownell]

On Monday 01 December 2008, Tero Kristo wrote:
> Off mode is now using the omap2 retention fix code for scanning GPIOs
> only during off-mode transitions. All the *non_wakeup_gpios variables
> are now used for off-mode transition tracking on OMAP3. This patch fixes
> cases where GPIO state changes are missed during off-mode.

I second the "no #ifdefs" comment ...

Could you uplevel your description here a bit?  I'm trying to
understand if what this does is complete and correct.

My understanding is that we currently have several mechanisms
interacting to affect things that relate to the OMAP3-only
"off" modes for pins used as GPIOs:

 - irq_chip.set_wake() calls, for GPIOs used as IRQs.
   We should assume that if the IRQ is wake-enabled, that
   applies to OFF mode too.  (AFAICT, this mechanism is
   not handled by this patch.)

 - Hmm, and because a goal is to transparently enter OFF
   modes to save power, rather than only after drivers
   get suspend() calls that tend to trigger set_wake(),
   an un-commented conclusion seems to be that all GPIOs
   used as IRQs implicitly act like set_wake() was called.
   (Something which *is* partially addressed here.)

 - omap_cfg_reg() configuration for any pin can include
   its OFF-mode parameters.  Virtually unused ... and
   not addressed by this patch, so I'm puzzled how pins
   are expected to be kept active with just this patch.

 - OMAP2-specific bug workarounds, some GPIOs can't be
   used for wakeup, ergo bank->non_wakeup_gpios.  This
   is resolved for OMAP3, yes?

When I looked at this issue a while back, I came to believe
that we'd need to map GPIOs to their config registers so
we could diddle the OFF-mode bits.  And I don't see such
a mapping (table) here...

- Dave

RE: [PATCH] OMAP3: GPIO fixes for off-mode

[Tero.Kristo]

Hi,

>On Monday 01 December 2008, Tero Kristo wrote:
>> Off mode is now using the omap2 retention fix code for 
>scanning GPIOs 
>> only during off-mode transitions. All the *non_wakeup_gpios 
>variables 
>> are now used for off-mode transition tracking on OMAP3. This patch 
>> fixes cases where GPIO state changes are missed during off-mode.
>
>I second the "no #ifdefs" comment ...

I agree the code looks horrible at the moment with the multitude of
#ifdef:s, I just did not feel comfortable fixing all of those, so I
continued the way it has been written so far...

>Could you uplevel your description here a bit?  I'm trying to 
>understand if what this does is complete and correct.
>
>My understanding is that we currently have several mechanisms 
>interacting to affect things that relate to the OMAP3-only 
>"off" modes for pins used as GPIOs:
>
> - irq_chip.set_wake() calls, for GPIOs used as IRQs.
>   We should assume that if the IRQ is wake-enabled, that
>   applies to OFF mode too.  (AFAICT, this mechanism is
>   not handled by this patch.)

Off mode wakeup requires setting up IO pads according to the wakeup
scheme you want to have. This is rather nasty limitation of the HW and
you need to be careful how you configure these things. This patch
assumes you have configured your IO pads in a way that you get wakeups
for your GPIOs during off, and when you eventually wakeup, it will
scan all GPIOs to see if any GPIO "interrupt" has happened. You will
not get edge sensitive interrupts from GPIOs during off-mode, because
the hardware block handling the edge sensing has no power at all.

> - Hmm, and because a goal is to transparently enter OFF
>   modes to save power, rather than only after drivers
>   get suspend() calls that tend to trigger set_wake(),
>   an un-commented conclusion seems to be that all GPIOs
>   used as IRQs implicitly act like set_wake() was called.
>   (Something which *is* partially addressed here.)

Yes, this is correct. I think this is somewhat against the design of
the omap2 code, as the GPIOs can wakeup the device from both suspend
and dynamic idle. However, this is mainly caused by the IO pad wakeup
mechanism of omap3, which does not care whether you are in dynamic idle
or in suspend. Handling this correctly would require creating IO pad
control mechanism to the kernel, something that I am not sure if we
want to do (we would need to have separate IO pad config for suspend
and dynamic idle.)

>
> - omap_cfg_reg() configuration for any pin can include
>   its OFF-mode parameters.  Virtually unused ... and
>   not addressed by this patch, so I'm puzzled how pins
>   are expected to be kept active with just this patch.

Mux configuration for OMAP3 is handled mostly by boot loader at the
moment, this is the behavior at least on TI SDP implementation.

>
> - OMAP2-specific bug workarounds, some GPIOs can't be
>   used for wakeup, ergo bank->non_wakeup_gpios.  This
>   is resolved for OMAP3, yes?

Well, basically only bank 0 can be used for wakeup in omap3, wakeups
for all the rest are handled via IO pad.

>
>When I looked at this issue a while back, I came to believe 
>that we'd need to map GPIOs to their config registers so we 
>could diddle the OFF-mode bits.  And I don't see such a 
>mapping (table) here...

Yep, if we want to have different configs for suspend and dynamic
idle, this is something we should do. However, having around 190
pins in OMAP3 you can potentially configure as GPIOs, I am really
hesitant writing such piece of code.

[PATCH] OMAP3: GPIO fixes for off-mode

[Tero Kristo]

Off mode is now using the omap2 retention fix code for scanning GPIOs
only during off-mode transitions. All the *non_wakeup_gpios variables
are now used for off-mode transition tracking on OMAP3. This patch fixes
cases where GPIO state changes are missed during off-mode.

Signed-off-by: Tero Kristo <tero.kristo@nokia.com>
---
 arch/arm/mach-omap2/pm34xx.c |   10 ++++++----
 arch/arm/plat-omap/gpio.c    |   18 ++++++++++++++++--
 2 files changed, 22 insertions(+), 6 deletions(-)

diff --git a/arch/arm/mach-omap2/pm34xx.c b/arch/arm/mach-omap2/pm34xx.c
index 5723240..2aaa631 100644
--- a/arch/arm/mach-omap2/pm34xx.c
+++ b/arch/arm/mach-omap2/pm34xx.c
@@ -321,9 +321,10 @@ void omap_sram_idle(void)
 	per_next_state = pwrdm_read_next_pwrst(per_pwrdm);
 	if (per_next_state < PWRDM_POWER_ON) {
 		omap_uart_prepare_idle(2);
-		omap2_gpio_prepare_for_retention();
-		if (per_next_state == PWRDM_POWER_OFF)
+		if (per_next_state == PWRDM_POWER_OFF) {
+			omap2_gpio_prepare_for_retention();
 			omap3_per_save_context();
+		}
 	}
 
 	/* CORE */
@@ -396,9 +397,10 @@ void omap_sram_idle(void)
 	/* PER */
 	if (per_next_state < PWRDM_POWER_ON) {
 		per_prev_state = pwrdm_read_prev_pwrst(per_pwrdm);
-		if (per_prev_state == PWRDM_POWER_OFF)
+		if (per_prev_state == PWRDM_POWER_OFF) {
 			omap3_per_restore_context();
-		omap2_gpio_resume_after_retention();
+			omap2_gpio_resume_after_retention();
+		}
 		omap_uart_resume_idle(2);
 	}
 
diff --git a/arch/arm/plat-omap/gpio.c b/arch/arm/plat-omap/gpio.c
index 62349fd..b45bf9d 100644
--- a/arch/arm/plat-omap/gpio.c
+++ b/arch/arm/plat-omap/gpio.c
@@ -556,7 +556,9 @@ static inline void set_24xx_gpio_triggering(struct gpio_bank *bank, int gpio,
 		else
 			__raw_writel(1 << gpio, bank->base
 					+ OMAP24XX_GPIO_CLEARWKUENA);
+#ifdef CONFIG_ARCH_OMAP24XX
 	} else {
+#endif
 		if (trigger != 0)
 			bank->enabled_non_wakeup_gpios |= gpio_bit;
 		else
@@ -1485,9 +1487,11 @@ static int __init _omap_gpio_init(void)
 
 #if defined(CONFIG_ARCH_OMAP24XX) || defined(CONFIG_ARCH_OMAP34XX)
 		if (bank->method == METHOD_GPIO_24XX) {
+#ifdef CONFIG_ARCH_OMAP24XX
 			static const u32 non_wakeup_gpios[] = {
 				0xe203ffc0, 0x08700040
 			};
+#endif
 
 			__raw_writel(0x00000000, bank->base + OMAP24XX_GPIO_IRQENABLE1);
 			__raw_writel(0xffffffff, bank->base + OMAP24XX_GPIO_IRQSTATUS1);
@@ -1495,8 +1499,10 @@ static int __init _omap_gpio_init(void)
 
 			/* Initialize interface clock ungated, module enabled */
 			__raw_writel(0, bank->base + OMAP24XX_GPIO_CTRL);
+#ifdef CONFIG_ARCH_OMAP24XX
 			if (i < ARRAY_SIZE(non_wakeup_gpios))
 				bank->non_wakeup_gpios = non_wakeup_gpios[i];
+#endif
 			gpio_count = 32;
 		}
 #endif
@@ -1666,7 +1672,11 @@ void omap2_gpio_prepare_for_retention(void)
 
 	/* Remove triggering for all non-wakeup GPIOs.  Otherwise spurious
 	 * IRQs will be generated.  See OMAP2420 Errata item 1.101. */
+#ifdef CONFIG_ARCH_OMAP24XX
 	for (i = 0; i < gpio_bank_count; i++) {
+#else
+	for (i = 1; i < gpio_bank_count; i++) {
+#endif
 		struct gpio_bank *bank = &gpio_bank[i];
 		u32 l1, l2;
 
@@ -1681,7 +1691,7 @@ void omap2_gpio_prepare_for_retention(void)
 		bank->saved_risingdetect = l2;
 		l1 &= ~bank->enabled_non_wakeup_gpios;
 		l2 &= ~bank->enabled_non_wakeup_gpios;
-#if defined(CONFIG_ARCH_OMAP24XX) || defined(CONFIG_ARCH_OMAP34XX)
+#if defined(CONFIG_ARCH_OMAP24XX)
 		__raw_writel(l1, bank->base + OMAP24XX_GPIO_FALLINGDETECT);
 		__raw_writel(l2, bank->base + OMAP24XX_GPIO_RISINGDETECT);
 #endif
@@ -1700,7 +1710,11 @@ void omap2_gpio_resume_after_retention(void)
 
 	if (!workaround_enabled)
 		return;
+#ifdef CONFIG_ARCH_OMAP24XX
 	for (i = 0; i < gpio_bank_count; i++) {
+#else
+	for (i = 1; i < gpio_bank_count; i++) {
+#endif
 		struct gpio_bank *bank = &gpio_bank[i];
 		u32 l;
 
@@ -1720,7 +1734,7 @@ void omap2_gpio_resume_after_retention(void)
 		l = __raw_readl(bank->base + OMAP24XX_GPIO_DATAIN);
 #endif
 		l ^= bank->saved_datain;
-		l &= bank->non_wakeup_gpios;
+		l &= bank->enabled_non_wakeup_gpios;
 		if (l) {
 			u32 old0, old1;
 #if defined(CONFIG_ARCH_OMAP24XX) || defined(CONFIG_ARCH_OMAP34XX)
-- 
1.5.4.3