[PATCH 1/2] ARM: PXA2xx: MFP: fix bug with MFP_LPM_KEEP_OUTPUT

[PATCH 1/2] ARM: PXA2xx: MFP: fix bug with MFP_LPM_KEEP_OUTPUT

[Igor Grinberg]

Pins that have MFP_LPM_KEEP_OUTPUT set and are configured for output
must retain the output state in low power mode.
Currently, the pin direction configuration is overrided with values
in gpdr_lpm[] array and do not obey the MFP_LPM_KEEP_OUTPUT setting.

Fix the above bug and add some documentation to clarify the
MFP_LPM_KEEP_OUTPUT setting purpose.

Reported-by: Paul Parsons <lost.distance@yahoo.com>
Signed-off-by: Igor Grinberg <grinberg@compulab.co.il>
Tested-by: Paul Parsons <lost.distance@yahoo.com>
---
 arch/arm/mach-pxa/include/mach/mfp-pxa2xx.h |    7 +++++++
 arch/arm/mach-pxa/mfp-pxa2xx.c              |   12 ++++++++++--
 2 files changed, 17 insertions(+), 2 deletions(-)

diff --git a/arch/arm/mach-pxa/include/mach/mfp-pxa2xx.h b/arch/arm/mach-pxa/include/mach/mfp-pxa2xx.h
index c54cef2..cbf51ae 100644
--- a/arch/arm/mach-pxa/include/mach/mfp-pxa2xx.h
+++ b/arch/arm/mach-pxa/include/mach/mfp-pxa2xx.h
@@ -17,6 +17,7 @@
  *
  * bit     23 - Input/Output (PXA2xx specific)
  * bit     24 - Wakeup Enable(PXA2xx specific)
+ * bit     25 - Keep Output  (PXA2xx specific)
  */
 
 #define MFP_DIR_IN		(0x0 << 23)
@@ -25,6 +26,12 @@
 #define MFP_DIR(x)		(((x) >> 23) & 0x1)
 
 #define MFP_LPM_CAN_WAKEUP	(0x1 << 24)
+
+/*
+ * MFP_LPM_KEEP_OUTPUT must be specified for pins that need to
+ * retain their last output level (low or high).
+ * Note: MFP_LPM_KEEP_OUTPUT has no effect on pins configured for input.
+ */
 #define MFP_LPM_KEEP_OUTPUT	(0x1 << 25)
 
 #define WAKEUP_ON_EDGE_RISE	(MFP_LPM_CAN_WAKEUP | MFP_LPM_EDGE_RISE)
diff --git a/arch/arm/mach-pxa/mfp-pxa2xx.c b/arch/arm/mach-pxa/mfp-pxa2xx.c
index b0a8428..d2373d7 100644
--- a/arch/arm/mach-pxa/mfp-pxa2xx.c
+++ b/arch/arm/mach-pxa/mfp-pxa2xx.c
@@ -366,14 +366,22 @@ static int pxa2xx_mfp_suspend(void)
 	}
 
 	for (i = 0; i <= gpio_to_bank(pxa_last_gpio); i++) {
-
 		saved_gafr[0][i] = GAFR_L(i);
 		saved_gafr[1][i] = GAFR_U(i);
 		saved_gpdr[i] = GPDR(i * 32);
 		saved_pgsr[i] = PGSR(i);
+	}
 
-		GPDR(i * 32) = gpdr_lpm[i];
+	/* set GPDR bits taking into account MFP_LPM_KEEP_OUTPUT */
+	for (i = 0; i < pxa_last_gpio; i++) {
+		if ((gpdr_lpm[gpio_to_bank(i)] & GPIO_bit(i)) ||
+		    ((gpio_desc[i].config & MFP_LPM_KEEP_OUTPUT) &&
+		     (saved_gpdr[gpio_to_bank(i)] & GPIO_bit(i))))
+			GPDR(i) |= GPIO_bit(i);
+		else
+			GPDR(i) &= ~GPIO_bit(i);
 	}
+
 	return 0;
 }
 
-- 
1.7.3.4

[PATCH 2/2] ARM: PXA2xx: MFP: fix potential direction bug

[Igor Grinberg]

Pins configured as input and have MFP_LPM_DRIVE_* flag set, can have a
wrong output value for some period of time (spike) during the suspend
sequence.
This can happen because the direction of the pins (GPDR) is set by
software and the output level is set by hardware (PGSR) at a later
stage.

Fix the above potential bug by setting the output levels first.
Also save the actual levels of the pins before the suspend and restore
them after the resume, but before the direction settings take place, so
the same bug as described above will not happen in the resume sequence.

Reported-by: Paul Parsons <lost.distance@yahoo.com>
Signed-off-by: Igor Grinberg <grinberg@compulab.co.il>
Tested-by: Paul Parsons <lost.distance@yahoo.com>
---
No regression has been seen after this patch.

 arch/arm/mach-pxa/mfp-pxa2xx.c |    9 +++++++++
 1 files changed, 9 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mach-pxa/mfp-pxa2xx.c b/arch/arm/mach-pxa/mfp-pxa2xx.c
index d2373d7..ef0426a 100644
--- a/arch/arm/mach-pxa/mfp-pxa2xx.c
+++ b/arch/arm/mach-pxa/mfp-pxa2xx.c
@@ -33,6 +33,8 @@
 #define BANK_OFF(n)	(((n) < 3) ? (n) << 2 : 0x100 + (((n) - 3) << 2))
 #define GPLR(x)		__REG2(0x40E00000, BANK_OFF((x) >> 5))
 #define GPDR(x)		__REG2(0x40E00000, BANK_OFF((x) >> 5) + 0x0c)
+#define GPSR(x)		__REG2(0x40E00000, BANK_OFF((x) >> 5) + 0x18)
+#define GPCR(x)		__REG2(0x40E00000, BANK_OFF((x) >> 5) + 0x24)
 
 #define PWER_WE35	(1 << 24)
 
@@ -348,6 +350,7 @@ static inline void pxa27x_mfp_init(void) {}
 #ifdef CONFIG_PM
 static unsigned long saved_gafr[2][4];
 static unsigned long saved_gpdr[4];
+static unsigned long saved_gplr[4];
 static unsigned long saved_pgsr[4];
 
 static int pxa2xx_mfp_suspend(void)
@@ -369,7 +372,11 @@ static int pxa2xx_mfp_suspend(void)
 		saved_gafr[0][i] = GAFR_L(i);
 		saved_gafr[1][i] = GAFR_U(i);
 		saved_gpdr[i] = GPDR(i * 32);
+		saved_gplr[i] = GPLR(i * 32);
 		saved_pgsr[i] = PGSR(i);
+
+		GPSR(i * 32) = PGSR(i);
+		GPCR(i * 32) = ~PGSR(i);
 	}
 
 	/* set GPDR bits taking into account MFP_LPM_KEEP_OUTPUT */
@@ -392,6 +399,8 @@ static void pxa2xx_mfp_resume(void)
 	for (i = 0; i <= gpio_to_bank(pxa_last_gpio); i++) {
 		GAFR_L(i) = saved_gafr[0][i];
 		GAFR_U(i) = saved_gafr[1][i];
+		GPSR(i * 32) = saved_gplr[i];
+		GPCR(i * 32) = ~saved_gplr[i];
 		GPDR(i * 32) = saved_gpdr[i];
 		PGSR(i) = saved_pgsr[i];
 	}
-- 
1.7.3.4

[PATCH 1/2] ARM: PXA2xx: MFP: fix bug with MFP_LPM_KEEP_OUTPUT

[Haojian Zhuang]

On Thu, Apr 12, 2012 at 8:43 PM, Igor Grinberg <grinberg@compulab.co.il> wrote:
> Pins that have MFP_LPM_KEEP_OUTPUT set and are configured for output
> must retain the output state in low power mode.
> Currently, the pin direction configuration is overrided with values
> in gpdr_lpm[] array and do not obey the MFP_LPM_KEEP_OUTPUT setting.
>
> Fix the above bug and add some documentation to clarify the
> MFP_LPM_KEEP_OUTPUT setting purpose.
>
> Reported-by: Paul Parsons <lost.distance@yahoo.com>
> Signed-off-by: Igor Grinberg <grinberg@compulab.co.il>
> Tested-by: Paul Parsons <lost.distance@yahoo.com>
> ---
> ?arch/arm/mach-pxa/include/mach/mfp-pxa2xx.h | ? ?7 +++++++
> ?arch/arm/mach-pxa/mfp-pxa2xx.c ? ? ? ? ? ? ?| ? 12 ++++++++++--
> ?2 files changed, 17 insertions(+), 2 deletions(-)
>
Applied

Regards
Haojian