Skip to contentSkip to conversation

pxa270 rtc wakeup event fix

PATCH New Nick Bane 5 participants 7 messages Apr 8, 2011 – Apr 12, 2011

Subsystems (3)
Arm (32-bit) Arm sub-architectures PXA2xx/PXA3xx support
lore
Share & tools
lore

Nick Bane

author
#0 lore raw

PATCH 1/1 pxa270 rtc wakeup event fix

This fixes the failure to register the IRQ_RTCAlrm alarm as a wakeup event. It is misinterpreted as a gpio irq not a PWER bitmask.

Nick Bane

Index: linux-2.6.38/arch/arm/mach-pxa/pxa27x.c
===================================================================
--- linux-2.6.38.orig/arch/arm/mach-pxa/pxa27x.c 2011-04-07
21:07:54.000000000 +0100
+++ linux-2.6.38/arch/arm/mach-pxa/pxa27x.c 2011-04-07
21:08:40.000000000 +0100
@@ -341,6 +341,11 @@
static inline void pxa27x_init_pm(void) {}
#endif

+static int is_gpio_irq(int irq)
+{
+ return (irq == IRQ_GPIO0) || (irq == IRQ_GPIO1) || (irq >=
PXA_GPIO_IRQ_BASE);
+}
+
/* PXA27x: Various gpios can issue wakeup events. This logic only
* handles the simple cases, not the WEMUX2 and WEMUX3 options */
@@ -349,7 +354,7 @@

      int gpio = IRQ_TO_GPIO(d->irq);
      uint32_t mask;

- if (gpio >= 0 && gpio < 128)
+ if (is_gpio_irq(d->irq) && gpio >= 0 && gpio < 128)

          return gpio_set_wake(gpio, on);

      if (d->irq == IRQ_KEYPAD)
#1 lore raw

PATCH 1/1 pxa270 rtc wakeup event fix

Quoted 4 lines · On Friday 08 April 2011 18:42:12 Nick Bane wrote:

This fixes the failure to register the IRQ_RTCAlrm alarm as a wakeup event. It is misinterpreted as a gpio irq not a PWER bitmask.

Nick Bane

CCing Haojian and Eric.

Quoted 3 lines

Index: linux-2.6.38/arch/arm/mach-pxa/pxa27x.c
===================================================================

Please use git format-patch

Thanks, cheers

Quoted 32 lines

--- linux-2.6.38.orig/arch/arm/mach-pxa/pxa27x.c 2011-04-07
21:07:54.000000000 +0100
+++ linux-2.6.38/arch/arm/mach-pxa/pxa27x.c 2011-04-07
21:08:40.000000000 +0100

(quoted diff) +5 -0
@@ -341,6 +341,11 @@
  static inline void pxa27x_init_pm(void) {}
  #endif
 
+static int is_gpio_irq(int irq)
+{
+    return (irq == IRQ_GPIO0) || (irq == IRQ_GPIO1) || (irq >=
 PXA_GPIO_IRQ_BASE);
+}
+
  /* PXA27x:  Various gpios can issue wakeup events.  This logic only
   * handles the simple cases, not the WEMUX2 and WEMUX3 options

   */
(quoted diff) +1 -1
@@ -349,7 +354,7 @@
      int gpio = IRQ_TO_GPIO(d->irq);
      uint32_t mask;
 
-    if (gpio >= 0 && gpio < 128)
+    if (is_gpio_irq(d->irq) && gpio >= 0 && gpio < 128)
          return gpio_set_wake(gpio, on);
 
      if (d->irq == IRQ_KEYPAD)

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

#2 lore raw

PATCH 1/1 pxa270 rtc wakeup event fix

Hi Nick,

Thanks for spotting this. The problem is really IRQ_TO_GPIO() macro doesn't behave as assumed. A better approach would be to return -1 when none-GPIO IRQs are mapped back, let me know if you are OK w/ the patch below:

(quoted diff) +0 -0

b/arch/arm/mach-pxa/include/mach/gpio.h
index b024a8b..b9f4ed1 100644

arch/arm/mach-pxa/include/mach/gpio.h +14 -1
@@ -103,7 +103,20 @@
 
 #define gpio_to_bank(gpio)	((gpio) >> 5)
 #define gpio_to_irq(gpio)	IRQ_GPIO(gpio)
-#define irq_to_gpio(irq)	IRQ_TO_GPIO(irq)
+
+static inline int irq_to_gpio(unsigned int irq)
+{
+	int gpio;
+
+	if (irq == IRQ_GPIO0 || irq == IRQ_GPIO1)
+		return irq - IRQ_GPIO0;
+
+	gpio = irq - PXA_GPIO_IRQ_BASE;
+	if (gpio >= 2 && gpio < NR_BUILTIN_GPIO)
+		return gpio;
+
+	return -1;
+}
 
 #ifdef CONFIG_CPU_PXA26x
 /* GPIO86/87/88/89 on PXA26x have their direction bits in GPDR2 inverted,
(quoted diff) +0 -0

b/arch/arm/mach-pxa/include/mach/irqs.h
index a4285fc..0384024 100644

arch/arm/mach-pxa/include/mach/irqs.h +0 -2
@@ -93,9 +93,6 @@
 #define GPIO_2_x_TO_IRQ(x)	(PXA_GPIO_IRQ_BASE + (x))
 #define IRQ_GPIO(x)	(((x) < 2) ? (IRQ_GPIO0 + (x)) : GPIO_2_x_TO_IRQ(x))
 
-#define IRQ_TO_GPIO_2_x(i)	((i) - PXA_GPIO_IRQ_BASE)
-#define IRQ_TO_GPIO(i)	(((i) < IRQ_GPIO(2)) ? ((i) - IRQ_GPIO0) :

IRQ_TO_GPIO_2_x(i))
-
/*
* The following interrupts are for board specific purposes. Since
* the kernel can only run on one machine@a time, we can re-use

arch/arm/mach-pxa/pxa25x.c +1 -1
index 6bde595..a4af8c5 100644
@@ -285,7 +285,7 @@ static inline void pxa25x_init_pm(void) {}
 
 static int pxa25x_set_wake(struct irq_data *d, unsigned int on)
 {
-	int gpio = IRQ_TO_GPIO(d->irq);
+	int gpio = irq_to_gpio(d->irq);
 	uint32_t mask = 0;
 
 	if (gpio >= 0 && gpio < 85)
arch/arm/mach-pxa/pxa27x.c +1 -1
index 1cb5d0f..909756e 100644
@@ -345,7 +345,7 @@ static inline void pxa27x_init_pm(void) {}
  */
 static int pxa27x_set_wake(struct irq_data *d, unsigned int on)
 {
-	int gpio = IRQ_TO_GPIO(d->irq);
+	int gpio = irq_to_gpio(d->irq);
 	uint32_t mask;
 
 	if (gpio >= 0 && gpio < 128)
#3 lore raw

PATCH 1/1 pxa270 rtc wakeup event fix

Quoted 8 lines

Hi Nick,

Thanks for spotting this. The problem is really IRQ_TO_GPIO() macro doesn't behave as assumed. A better approach would be to return -1 when none-GPIO IRQs are mapped back, let me know if you are OK w/ the patch below:

Hi Eric
That looks like the right answer.
Nick

Quoted 69 lines
arch/arm/mach-pxa/include/mach/gpio.h +14 -1
index b024a8b..b9f4ed1 100644
@@ -103,7 +103,20 @@
 
  #define gpio_to_bank(gpio)	((gpio)>>  5)
  #define gpio_to_irq(gpio)	IRQ_GPIO(gpio)
-#define irq_to_gpio(irq)	IRQ_TO_GPIO(irq)
+
+static inline int irq_to_gpio(unsigned int irq)
+{
+	int gpio;
+
+	if (irq == IRQ_GPIO0 || irq == IRQ_GPIO1)
+		return irq - IRQ_GPIO0;
+
+	gpio = irq - PXA_GPIO_IRQ_BASE;
+	if (gpio>= 2&&  gpio<  NR_BUILTIN_GPIO)
+		return gpio;
+
+	return -1;
+}
 
  #ifdef CONFIG_CPU_PXA26x
  /* GPIO86/87/88/89 on PXA26x have their direction bits in GPDR2 inverted,
arch/arm/mach-pxa/include/mach/irqs.h +0 -3
index a4285fc..0384024 100644
@@ -93,9 +93,6 @@
  #define GPIO_2_x_TO_IRQ(x)	(PXA_GPIO_IRQ_BASE + (x))
  #define IRQ_GPIO(x)	(((x)<  2) ? (IRQ_GPIO0 + (x)) : GPIO_2_x_TO_IRQ(x))
 
-#define IRQ_TO_GPIO_2_x(i)	((i) - PXA_GPIO_IRQ_BASE)
-#define IRQ_TO_GPIO(i)	(((i)<  IRQ_GPIO(2)) ? ((i) - IRQ_GPIO0) :
 IRQ_TO_GPIO_2_x(i))
-
  /*
   * The following interrupts are for board specific purposes. Since

* the kernel can only run on one machine at a time, we can re-use

arch/arm/mach-pxa/pxa25x.c +1 -1
index 6bde595..a4af8c5 100644
@@ -285,7 +285,7 @@ static inline void pxa25x_init_pm(void) {}
 
  static int pxa25x_set_wake(struct irq_data *d, unsigned int on)
  {
-	int gpio = IRQ_TO_GPIO(d->irq);
+	int gpio = irq_to_gpio(d->irq);
  	uint32_t mask = 0;
 
  	if (gpio>= 0&&  gpio<  85)
arch/arm/mach-pxa/pxa27x.c +1 -1
index 1cb5d0f..909756e 100644
@@ -345,7 +345,7 @@ static inline void pxa27x_init_pm(void) {}
   */
  static int pxa27x_set_wake(struct irq_data *d, unsigned int on)
  {
-	int gpio = IRQ_TO_GPIO(d->irq);
+	int gpio = irq_to_gpio(d->irq);
  	uint32_t mask;
 
  	if (gpio>= 0&&  gpio<  128)
#4 lore raw

PATCH 1/1 pxa270 rtc wakeup event fix

Hi Nick,

Quoted 18 lines · Nick Bane
Quoted 8 lines

Hi Nick,

Thanks for spotting this. The problem is really IRQ_TO_GPIO() macro doesn't behave as assumed. A better approach would be to return -1 when none-GPIO IRQs are mapped back, let me know if you are OK w/ the patch below:

Hi Eric
That looks like the right answer.
Nick

Quoted 7 lines
arch/arm/mach-pxa/include/mach/gpio.h +0 -0
index b024a8b..b9f4ed1 100644
@@ -103,7 +103,20 @@
Quoted 13 lines

arch/arm/mach-pxa/include/mach/gpio.h

Quoted 13 lines
arch/arm/mach-pxa/include/mach/gpio.h +10 -1
  #define gpio_to_bank(gpio)    ((gpio)>>  5)
  #define gpio_to_irq(gpio)    IRQ_GPIO(gpio)
-#define irq_to_gpio(irq)    IRQ_TO_GPIO(irq)
+
+static inline int irq_to_gpio(unsigned int irq)
+{
+    int gpio;
+
+    if (irq == IRQ_GPIO0 || irq == IRQ_GPIO1)
+        return irq - IRQ_GPIO0;
+
+    gpio = irq - PXA_GPIO_IRQ_BASE;
+    if (gpio>= 2&&  gpio<  NR_BUILTIN_GPIO)

This is a really nice white space game, your mailer plays ;) Really funny...

Quoted 56 lines
Quoted 41 lines
arch/arm/mach-pxa/include/mach/gpio.h +4 -0
+        return gpio;
+
+    return -1;
+}
 
  #ifdef CONFIG_CPU_PXA26x
  /* GPIO86/87/88/89 on PXA26x have their direction bits in GPDR2 inverted,
arch/arm/mach-pxa/include/mach/irqs.h +0 -3
index a4285fc..0384024 100644
@@ -93,9 +93,6 @@
  #define GPIO_2_x_TO_IRQ(x)    (PXA_GPIO_IRQ_BASE + (x))
  #define IRQ_GPIO(x)    (((x)<  2) ? (IRQ_GPIO0 + (x)) : GPIO_2_x_TO_IRQ(x))
 
-#define IRQ_TO_GPIO_2_x(i)    ((i) - PXA_GPIO_IRQ_BASE)
-#define IRQ_TO_GPIO(i)    (((i)<  IRQ_GPIO(2)) ? ((i) - IRQ_GPIO0) :
 IRQ_TO_GPIO_2_x(i))
-
  /*
   * The following interrupts are for board specific purposes. Since

* the kernel can only run on one machine at a time, we can re-use

arch/arm/mach-pxa/pxa25x.c +1 -1
index 6bde595..a4af8c5 100644
@@ -285,7 +285,7 @@ static inline void pxa25x_init_pm(void) {}
 
  static int pxa25x_set_wake(struct irq_data *d, unsigned int on)
  {
-    int gpio = IRQ_TO_GPIO(d->irq);
+    int gpio = irq_to_gpio(d->irq);
      uint32_t mask = 0;
 
      if (gpio>= 0&&  gpio<  85)
arch/arm/mach-pxa/pxa27x.c +0 -0
index 1cb5d0f..909756e 100644

arch/arm/mach-pxa/pxa27x.c static inline void pxa27x_init_pm(void) {} · line 351

Quoted 8 lines
arch/arm/mach-pxa/pxa27x.c +1 -1
@@ -345,7 +345,7 @@ static inline void pxa27x_init_pm(void) {}
   */
  static int pxa27x_set_wake(struct irq_data *d, unsigned int on)
  {
-    int gpio = IRQ_TO_GPIO(d->irq);
+    int gpio = irq_to_gpio(d->irq);
      uint32_t mask;
 
      if (gpio>= 0&&  gpio<  128)

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Signature · 2 lines
Regards,
Igor.
#5 lore raw

PATCH 1/1 pxa270 rtc wakeup event fix

In reply to Eric Miao
Quoted 13 lines · On Tuesday 12 April 2011 09:44:17 Eric Miao wrote:

Hi Nick,

Thanks for spotting this. The problem is really IRQ_TO_GPIO() macro doesn't behave as assumed. A better approach would be to return -1 when none-GPIO IRQs are mapped back, let me know if you are OK w/ the patch below:

arch/arm/mach-pxa/include/mach/gpio.h +0 -0
index b024a8b..b9f4ed1 100644

arch/arm/mach-pxa/include/mach/gpio.h line 107

Quoted 6 lines
arch/arm/mach-pxa/include/mach/gpio.h +2 -1
@@ -103,7 +103,20 @@
 
 #define gpio_to_bank(gpio)	((gpio) >> 5)
 #define gpio_to_irq(gpio)	IRQ_GPIO(gpio)
-#define irq_to_gpio(irq)	IRQ_TO_GPIO(irq)
+
+static inline int irq_to_gpio(unsigned int irq)

Maybe drop the explicit inline?

Cheers

Quoted 57 lines
arch/arm/mach-pxa/include/mach/gpio.h +12 -0
+{
+	int gpio;
+
+	if (irq == IRQ_GPIO0 || irq == IRQ_GPIO1)
+		return irq - IRQ_GPIO0;
+
+	gpio = irq - PXA_GPIO_IRQ_BASE;
+	if (gpio >= 2 && gpio < NR_BUILTIN_GPIO)
+		return gpio;
+
+	return -1;
+}
 
 #ifdef CONFIG_CPU_PXA26x
 /* GPIO86/87/88/89 on PXA26x have their direction bits in GPDR2 inverted,
arch/arm/mach-pxa/include/mach/irqs.h +0 -3
index a4285fc..0384024 100644
@@ -93,9 +93,6 @@
 #define GPIO_2_x_TO_IRQ(x)	(PXA_GPIO_IRQ_BASE + (x))
 #define IRQ_GPIO(x)	(((x) < 2) ? (IRQ_GPIO0 + (x)) : GPIO_2_x_TO_IRQ(x))
 
-#define IRQ_TO_GPIO_2_x(i)	((i) - PXA_GPIO_IRQ_BASE)
-#define IRQ_TO_GPIO(i)	(((i) < IRQ_GPIO(2)) ? ((i) - IRQ_GPIO0) :
 IRQ_TO_GPIO_2_x(i))
-
 /*
  * The following interrupts are for board specific purposes. Since

* the kernel can only run on one machine at a time, we can re-use

arch/arm/mach-pxa/pxa25x.c +1 -1
index 6bde595..a4af8c5 100644
@@ -285,7 +285,7 @@ static inline void pxa25x_init_pm(void) {}
 
 static int pxa25x_set_wake(struct irq_data *d, unsigned int on)
 {
-	int gpio = IRQ_TO_GPIO(d->irq);
+	int gpio = irq_to_gpio(d->irq);
 	uint32_t mask = 0;
 
 	if (gpio >= 0 && gpio < 85)
arch/arm/mach-pxa/pxa27x.c +1 -1
index 1cb5d0f..909756e 100644
@@ -345,7 +345,7 @@ static inline void pxa27x_init_pm(void) {}
  */
 static int pxa27x_set_wake(struct irq_data *d, unsigned int on)
 {
-	int gpio = IRQ_TO_GPIO(d->irq);
+	int gpio = irq_to_gpio(d->irq);
 	uint32_t mask;
 
 	if (gpio >= 0 && gpio < 128)
#6 lore raw

PATCH 1/1 pxa270 rtc wakeup event fix

Quoted 15 lines · Marek Vasut
Quoted 14 lines · On Tuesday 12 April 2011 09:44:17 Eric Miao wrote:

Hi Nick,

Thanks for spotting this. The problem is really IRQ_TO_GPIO() macro doesn't behave as assumed. A better approach would be to return -1 when none-GPIO IRQs are mapped back, let me know if you are OK w/ the patch below:

arch/arm/mach-pxa/include/mach/gpio.h +0 -0
index b024a8b..b9f4ed1 100644
@@ -103,7 +103,20 @@
Quoted 7 lines

arch/arm/mach-pxa/include/mach/gpio.h line 107

Quoted 5 lines
arch/arm/mach-pxa/include/mach/gpio.h +2 -1
 ?#define gpio_to_bank(gpio) ? ((gpio) >> 5)
 ?#define gpio_to_irq(gpio) ? ?IRQ_GPIO(gpio)
-#define irq_to_gpio(irq) ? ? IRQ_TO_GPIO(irq)
+
+static inline int irq_to_gpio(unsigned int irq)

Maybe drop the explicit inline?

It's in a header file, and has to be declared as inline to avoid the copy being duplicated in every .c files where this header is included.