* [PATCH] pxa270 rtc wakeup event fix
@ 2011-04-08 16:42 Nick Bane
2011-04-08 19:36 ` Marek Vasut
0 siblings, 1 reply; 7+ messages in thread
From: Nick Bane @ 2011-04-08 16:42 UTC (permalink / raw)
To: linux-arm-kernel
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)
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH] pxa270 rtc wakeup event fix
2011-04-08 16:42 [PATCH] pxa270 rtc wakeup event fix Nick Bane
@ 2011-04-08 19:36 ` Marek Vasut
2011-04-12 7:44 ` Eric Miao
0 siblings, 1 reply; 7+ messages in thread
From: Marek Vasut @ 2011-04-08 19:36 UTC (permalink / raw)
To: linux-arm-kernel
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.
>
> Index: linux-2.6.38/arch/arm/mach-pxa/pxa27x.c
> ===================================================================
Please use git format-patch
Thanks, cheers
> --- 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)
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH] pxa270 rtc wakeup event fix
2011-04-08 19:36 ` Marek Vasut
@ 2011-04-12 7:44 ` Eric Miao
2011-04-12 8:28 ` Nick Bane
2011-04-12 10:53 ` Marek Vasut
0 siblings, 2 replies; 7+ messages in thread
From: Eric Miao @ 2011-04-12 7:44 UTC (permalink / raw)
To: linux-arm-kernel
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:
diff --git a/arch/arm/mach-pxa/include/mach/gpio.h
b/arch/arm/mach-pxa/include/mach/gpio.h
index b024a8b..b9f4ed1 100644
--- a/arch/arm/mach-pxa/include/mach/gpio.h
+++ b/arch/arm/mach-pxa/include/mach/gpio.h
@@ -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,
diff --git a/arch/arm/mach-pxa/include/mach/irqs.h
b/arch/arm/mach-pxa/include/mach/irqs.h
index a4285fc..0384024 100644
--- a/arch/arm/mach-pxa/include/mach/irqs.h
+++ b/arch/arm/mach-pxa/include/mach/irqs.h
@@ -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
diff --git a/arch/arm/mach-pxa/pxa25x.c b/arch/arm/mach-pxa/pxa25x.c
index 6bde595..a4af8c5 100644
--- a/arch/arm/mach-pxa/pxa25x.c
+++ b/arch/arm/mach-pxa/pxa25x.c
@@ -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)
diff --git a/arch/arm/mach-pxa/pxa27x.c b/arch/arm/mach-pxa/pxa27x.c
index 1cb5d0f..909756e 100644
--- a/arch/arm/mach-pxa/pxa27x.c
+++ b/arch/arm/mach-pxa/pxa27x.c
@@ -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)
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH] pxa270 rtc wakeup event fix
2011-04-12 7:44 ` Eric Miao
@ 2011-04-12 8:28 ` Nick Bane
2011-04-12 9:56 ` Igor Grinberg
2011-04-12 10:53 ` Marek Vasut
1 sibling, 1 reply; 7+ messages in thread
From: Nick Bane @ 2011-04-12 8:28 UTC (permalink / raw)
To: linux-arm-kernel
> 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
> diff --git a/arch/arm/mach-pxa/include/mach/gpio.h
> b/arch/arm/mach-pxa/include/mach/gpio.h
> index b024a8b..b9f4ed1 100644
> --- a/arch/arm/mach-pxa/include/mach/gpio.h
> +++ b/arch/arm/mach-pxa/include/mach/gpio.h
> @@ -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,
> diff --git a/arch/arm/mach-pxa/include/mach/irqs.h
> b/arch/arm/mach-pxa/include/mach/irqs.h
> index a4285fc..0384024 100644
> --- a/arch/arm/mach-pxa/include/mach/irqs.h
> +++ b/arch/arm/mach-pxa/include/mach/irqs.h
> @@ -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
> diff --git a/arch/arm/mach-pxa/pxa25x.c b/arch/arm/mach-pxa/pxa25x.c
> index 6bde595..a4af8c5 100644
> --- a/arch/arm/mach-pxa/pxa25x.c
> +++ b/arch/arm/mach-pxa/pxa25x.c
> @@ -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)
> diff --git a/arch/arm/mach-pxa/pxa27x.c b/arch/arm/mach-pxa/pxa27x.c
> index 1cb5d0f..909756e 100644
> --- a/arch/arm/mach-pxa/pxa27x.c
> +++ b/arch/arm/mach-pxa/pxa27x.c
> @@ -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)
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH] pxa270 rtc wakeup event fix
2011-04-12 8:28 ` Nick Bane
@ 2011-04-12 9:56 ` Igor Grinberg
0 siblings, 0 replies; 7+ messages in thread
From: Igor Grinberg @ 2011-04-12 9:56 UTC (permalink / raw)
To: linux-arm-kernel
Hi Nick,
On 04/12/11 11:28, Nick Bane 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:
>>
> Hi Eric
> That looks like the right answer.
> Nick
>> diff --git a/arch/arm/mach-pxa/include/mach/gpio.h
>> b/arch/arm/mach-pxa/include/mach/gpio.h
>> index b024a8b..b9f4ed1 100644
>> --- a/arch/arm/mach-pxa/include/mach/gpio.h
>> +++ b/arch/arm/mach-pxa/include/mach/gpio.h
>> @@ -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)
This is a really nice white space game, your mailer plays ;)
Really funny...
>> + return gpio;
>> +
>> + return -1;
>> +}
>>
>> #ifdef CONFIG_CPU_PXA26x
>> /* GPIO86/87/88/89 on PXA26x have their direction bits in GPDR2 inverted,
>> diff --git a/arch/arm/mach-pxa/include/mach/irqs.h
>> b/arch/arm/mach-pxa/include/mach/irqs.h
>> index a4285fc..0384024 100644
>> --- a/arch/arm/mach-pxa/include/mach/irqs.h
>> +++ b/arch/arm/mach-pxa/include/mach/irqs.h
>> @@ -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
>> diff --git a/arch/arm/mach-pxa/pxa25x.c b/arch/arm/mach-pxa/pxa25x.c
>> index 6bde595..a4af8c5 100644
>> --- a/arch/arm/mach-pxa/pxa25x.c
>> +++ b/arch/arm/mach-pxa/pxa25x.c
>> @@ -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)
>> diff --git a/arch/arm/mach-pxa/pxa27x.c b/arch/arm/mach-pxa/pxa27x.c
>> index 1cb5d0f..909756e 100644
>> --- a/arch/arm/mach-pxa/pxa27x.c
>> +++ b/arch/arm/mach-pxa/pxa27x.c
>> @@ -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
>
--
Regards,
Igor.
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH] pxa270 rtc wakeup event fix
2011-04-12 7:44 ` Eric Miao
2011-04-12 8:28 ` Nick Bane
@ 2011-04-12 10:53 ` Marek Vasut
2011-04-12 13:47 ` Eric Miao
1 sibling, 1 reply; 7+ messages in thread
From: Marek Vasut @ 2011-04-12 10:53 UTC (permalink / raw)
To: linux-arm-kernel
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:
>
> diff --git a/arch/arm/mach-pxa/include/mach/gpio.h
> b/arch/arm/mach-pxa/include/mach/gpio.h
> index b024a8b..b9f4ed1 100644
> --- a/arch/arm/mach-pxa/include/mach/gpio.h
> +++ b/arch/arm/mach-pxa/include/mach/gpio.h
> @@ -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
> +{
> + 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,
> diff --git a/arch/arm/mach-pxa/include/mach/irqs.h
> b/arch/arm/mach-pxa/include/mach/irqs.h
> index a4285fc..0384024 100644
> --- a/arch/arm/mach-pxa/include/mach/irqs.h
> +++ b/arch/arm/mach-pxa/include/mach/irqs.h
> @@ -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
> diff --git a/arch/arm/mach-pxa/pxa25x.c b/arch/arm/mach-pxa/pxa25x.c
> index 6bde595..a4af8c5 100644
> --- a/arch/arm/mach-pxa/pxa25x.c
> +++ b/arch/arm/mach-pxa/pxa25x.c
> @@ -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)
> diff --git a/arch/arm/mach-pxa/pxa27x.c b/arch/arm/mach-pxa/pxa27x.c
> index 1cb5d0f..909756e 100644
> --- a/arch/arm/mach-pxa/pxa27x.c
> +++ b/arch/arm/mach-pxa/pxa27x.c
> @@ -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)
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH] pxa270 rtc wakeup event fix
2011-04-12 10:53 ` Marek Vasut
@ 2011-04-12 13:47 ` Eric Miao
0 siblings, 0 replies; 7+ messages in thread
From: Eric Miao @ 2011-04-12 13:47 UTC (permalink / raw)
To: linux-arm-kernel
On Tue, Apr 12, 2011 at 6:53 PM, Marek Vasut <marek.vasut@gmail.com> wrote:
> 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:
>>
>> diff --git a/arch/arm/mach-pxa/include/mach/gpio.h
>> b/arch/arm/mach-pxa/include/mach/gpio.h
>> index b024a8b..b9f4ed1 100644
>> --- a/arch/arm/mach-pxa/include/mach/gpio.h
>> +++ b/arch/arm/mach-pxa/include/mach/gpio.h
>> @@ -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?
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.
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2011-04-12 13:47 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-04-08 16:42 [PATCH] pxa270 rtc wakeup event fix Nick Bane
2011-04-08 19:36 ` Marek Vasut
2011-04-12 7:44 ` Eric Miao
2011-04-12 8:28 ` Nick Bane
2011-04-12 9:56 ` Igor Grinberg
2011-04-12 10:53 ` Marek Vasut
2011-04-12 13:47 ` Eric Miao
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).