gpio: pxa: Set PXA GPIO irq_chip IRQCHIP_SKIP_SET_WAKE flag
Subsystems (5)
PATCH 1/1 gpio: pxa: Set PXA GPIO irq_chip IRQCHIP_SKIP_SET_WAKE flag
The PXA GPIO irq_chip structure (pxa_muxed_gpio_chip) sets neither:
1. The irq_set_wake() handler.
2. The IRQCHIP_SKIP_SET_WAKE flag.
Consequently any attempt to configure the PXA GPIOs as wakeup sources results in Unbalanced IRQ warnings when the system is woken from sleep mode:
WARNING: at kernel/irq/manage.c:520 irq_set_irq_wake+0xc4/0xf8() Unbalanced IRQ 96 wake disable ... WARNING: at kernel/irq/manage.c:520 irq_set_irq_wake+0xc4/0xf8() Unbalanced IRQ 190 wake disable ... WARNING: at kernel/irq/manage.c:520 irq_set_irq_wake+0xc4/0xf8()
Unbalanced IRQ 195 wake disable
...
This patch sets the pxa_muxed_gpio_chip IRQCHIP_SKIP_SET_WAKE flag, thereby eliminating the Unbalanced IRQ warnings.
--- drivers/gpio/gpio-pxa.c | 1 + 1 files changed, 1 insertions(+), 0 deletions(-)
drivers/gpio/gpio-pxa.c +1 -0
@@ -427,6 +427,7 @@ static struct irq_chip pxa_muxed_gpio_chip = { .irq_mask = pxa_mask_muxed_gpio, .irq_unmask = pxa_unmask_muxed_gpio, .irq_set_type = pxa_gpio_irq_type, + .flags = IRQCHIP_SKIP_SET_WAKE, };static int pxa_gpio_nums (void )
Signature · 1 line
1.7.3.4
PATCH 1/1 gpio: pxa: Set PXA GPIO irq_chip IRQCHIP_SKIP_SET_WAKE flag
Quoted 50 lines · Paul Parsons
The PXA GPIO
irq_chipstructure
(pxa_muxed_gpio_chip) sets neither:
1. Theirq_set_wake()handler.
2. TheIRQCHIP_SKIP_SET_WAKEflag.Consequently any attempt to configure the PXA GPIOs as wakeup sources
results in Unbalanced IRQ warnings when the system is woken from sleep
mode:WARNING: at kernel/irq/manage.c:520 irq_set_irq_wake+0xc4/0xf8() Unbalanced IRQ 96 wake disable ... WARNING: at kernel/irq/manage.c:520 irq_set_irq_wake+0xc4/0xf8() Unbalanced IRQ 190 wake disable ... WARNING: at kernel/irq/manage.c:520 irq_set_irq_wake+0xc4/0xf8()Unbalanced IRQ 195 wake disable
...This patch sets the
pxa_muxed_gpio_chipIRQCHIP_SKIP_SET_WAKEflag,
thereby eliminating the Unbalanced IRQ warnings.Signed-off-by Paul Parsons <[email protected]>---
drivers/gpio/gpio-pxa.c|? ? 1 +1 files changed, 1 insertions(+), 0 deletions(-)drivers/gpio/gpio-pxa.c +0
-0@@ -427,6 +427,7 @@ static struct irq_chip pxa_muxed_gpio_chip = { ??? .irq_mask??? = pxa_mask_muxed_gpio, ??? .irq_unmask??? = pxa_unmask_muxed_gpio, ??? .irq_set_type??? =
pxa_gpio_irq_type,
+??? .flags???
??? =IRQCHIP_SKIP_SET_WAKE,
};
Quoted 2 lines
static int
pxa_gpio_nums(void)
Hmm, now I'm wondering if an irq_chip irq_set_wake() handler should be provided instead.
This question arises because of the gpio_keys driver, which provides a per-button wakeup flag (in struct gpio_keys_button).
Without an irq_set_wake() handler, the per-button wakeup flag seems to serve no useful purpose: the flag is used to call enable_irq_wake() from gpio_keys_suspend(), but enable_irq_wake() does nothing useful without a handler.
However if an irq_set_wake() handler were to be provided, and if that handler were to call gpio_set_wake(), then the per-button wakeup flag would indeed determine which buttons act as wakeup sources.
For example:
drivers/gpio/gpio-pxa.c +15 -0
@@ -23,6 +23,9 @@#include <linux/slab.h> #include <mach/irqs.h> +#if defined(CONFIG_PXA25x) || defined(CONFIG_PXA27x) +#include <mach/mfp-pxa2xx.h> +#endif /* * We handle the GPIOs by banks, each bank covers up to 32 GPIOs with
@@ -421,12 +424,24 @@ static void pxa_unmask_muxed_gpio(struct irq_data *d)update_edge_detect (c); } +#if defined(CONFIG_PXA25x) || defined(CONFIG_PXA27x) +static int pxa_gpio_irq_wake (struct irq_data *d,unsigned int on) +{ +int gpio =pxa_irq_to_gpio (d->irq); + +return gpio_set_wake (gpio, on); +} +#endif +static struct irq_chip pxa_muxed_gpio_chip = { .name ="GPIO" , .irq_ack = pxa_ack_muxed_gpio, .irq_mask = pxa_mask_muxed_gpio, .irq_unmask = pxa_unmask_muxed_gpio, .irq_set_type = pxa_gpio_irq_type, +#if defined(CONFIG_PXA25x) || defined(CONFIG_PXA27x) + .irq_set_wake = pxa_gpio_irq_wake, +#endif };static int pxa_gpio_nums (void )
So the underlying question is:
Was it the intention that gpio_set_wake() be called from irq_chip irq_set_wake() handlers?
Regards,
Paul
Robert Jarzmik
maintainerPATCH 1/1 gpio: pxa: Set PXA GPIO irq_chip IRQCHIP_SKIP_SET_WAKE flag
Quoted 5 lines · Paul Parsons
Quoted 4 lines · Paul Parsons
The PXA GPIO
irq_chipstructure
(pxa_muxed_gpio_chip) sets neither:
1. Theirq_set_wake()handler.
2. TheIRQCHIP_SKIP_SET_WAKEflag.
Hi Paul and others,
I tried the v3.4-rc2 today, and it _seems_ gpio button wakeups don't work anymore (ie. wakeup set up by gpio-keys driver). More specifically, the GPIO0 (power button) doesn't wakeup my board from "echo mem > /proc/power/state".
I haven't investigated much, but tracing PWER register on suspend, while GPIO0 (power button) was set as a wakeup source gives 0x80100000, which lacks the bit0 setting (ie. GPIO0 as a wakeup source).
I don't know it this is a recent or known issue, or if I'm have missed a change in pxa/gpio-keys driver.
Would anyone have an idea what's happening or should I look for the issue myself ?
Cheers.
Signature · 1 line
Robert
PATCH 1/1 gpio: pxa: Set PXA GPIO irq_chip IRQCHIP_SKIP_SET_WAKE flag
Hello Robert,
Quoted 5 lines · Robert Jarzmik
Quoted 4 lines
Quoted 4 lines
The PXA GPIO
irq_chipstructure
(pxa_muxed_gpio_chip) sets neither:
1. Theirq_set_wake()handler.
2. TheIRQCHIP_SKIP_SET_WAKEflag.
Quoted 8 lines
Hi Paul and others,
I tried the v3.4-rc2 today, and it _seems_ gpio button wakeups don't work
anymore (ie. wakeup set up by gpio-keys driver). More specifically, the GPIO0
(power button) doesn't wakeup my board from "echo mem >/proc/power/state".
Which board are you using?
Quoted 6 lines
I haven't investigated much, but tracing PWER register on suspend,
while GPIO0 (power button) was set as a wakeup source gives0x80100000, which
lacks the bit0 setting (ie. GPIO0 as a wakeup source).
PWER bit 0 (WE0) has a reset value of 1 on the PXA250/PXA270. If your WE0 is 0 then presumably either your bootloader or kernel has cleared it.
Quoted 4 lines
I don't know it this is a recent or known issue, or if I'm have missed a change in
pxa/gpio-keys driver.
I can only comment on the hx4700 platform, which has 4 pending patches for suspend/resume, plus 1 more still to be submitted. These fix hx4700 suspend/resume, which has never worked. None of the patches touch the gpio-keys driver, however.
Quoted 3 lines
Would anyone have an idea what's happening or should I look for the issue myself ?
If you are using the hx4700 then I can point you towards the pending patches.
Regards,
Paul
PATCH 1/1 gpio: pxa: Set PXA GPIO irq_chip IRQCHIP_SKIP_SET_WAKE flag
Quoted 6 lines · Paul Parsons
Quoted 6 lines
I tried the v3.4-rc2 today, and it _seems_ gpio button wakeups don't work
anymore (ie. wakeup set up by gpio-keys driver). More specifically, the GPIO0
(power button) doesn't wakeup my board from "echo mem >/proc/power/state".
...
Quoted 10 lines
Quoted 3 lines
I don't know it this is a recent or known issue, or if I'm have missed a change in
pxa/gpio-keys driver.I can only comment on the hx4700 platform, which has 4 pending
patches for suspend/resume, plus 1 more still to be submitted.
These fix hx4700 suspend/resume, which has never worked. None of the patches touch the gpio-keys driver, however.
Just to clarify, the generic pxa/gpio suspend/resume support does seem to work OK; the hx4700 patches are platform specific, apart from this SKIP_SET_WAKE patch to fix the Unbalanced IRQ warnings.
Consequently it seems likely that your problem is also platform specific.
Regards,
Paul
Robert Jarzmik
maintainerPATCH 1/1 gpio: pxa: Set PXA GPIO irq_chip IRQCHIP_SKIP_SET_WAKE flag
Quoted 4 lines · Paul Parsons
--- On Wed, 18/4/12, Paul Parsons <[email protected]> wrote:
Just to clarify, the generic pxa/gpio suspend/resume support does seem to work OK; the hx4700 patches are platform specific, apart from thisSKIP_SET_WAKEpatch to fix the Unbalanced IRQ warnings.
OK, I'll check.
My platform is mioa701. In my case, the suspend/resume is working for RTC wakeup, but not for GPIO0. This code was working before (that's vague, I know, I suspect 3.0 was working), and is not working anymore. The gpio_keys_button structure has still its 'wakeup' field ...
Quoted 2 lines
Consequently it seems likely that your problem is also platform specific.
OK. So you confirm that on hx4700, pressing PowerButton while in suspend to ram brings back to life your device ?
Cheers.
Signature · 1 line
Robert
PATCH 1/1 gpio: pxa: Set PXA GPIO irq_chip IRQCHIP_SKIP_SET_WAKE flag
Quoted 17 lines · Robert Jarzmik
Quoted 1 line
Just to clarify, the generic pxa/gpio suspend/resume
support does
Quoted 1 line
seem to work OK; the hx4700 patches are platform
specific, apart
Quoted 1 line
from this
SKIP_SET_WAKEpatch to fix the Unbalanced IRQwarnings.
OK, I'll check.
My platform is mioa701. In my case, the suspend/resume is working for RTC
wakeup, but not for GPIO0. This code was working before (that's vague, I know, I
suspect 3.0 was working), and is not working anymore. Thegpio_keys_button
structure has still its 'wakeup' field ...
Quoted 6 lines
Quoted 1 line
Consequently it seems likely that your problem is also
platform
Quoted 1 line
specific.
OK. So you confirm that on hx4700, pressing PowerButton while in suspend to ram
brings back to life your device ?
Yes.
I had a quick look at arch/arm/mach-pxa/mioa701.c and noticed that it didn't call gpio_set_wake() for GPIO0. Does this help:
arch/arm/mach-pxa/mioa701.c +1 -0
@@ -726,6 +726,7 @@ static void __init mioa701_machine_init(void)pxa2xx_mfp_config (ARRAY_AND_SIZE (mioa701_pin_config)); +gpio_set_wake (GPIO0_KEY_POWER ,1 );pxa_set_ffuart_info (NULL );pxa_set_btuart_info (NULL );pxa_set_stuart_info (NULL );
Regards,
Paul
Robert Jarzmik
maintainerPATCH 1/1 gpio: pxa: Set PXA GPIO irq_chip IRQCHIP_SKIP_SET_WAKE flag
Quoted 9 lines · Paul Parsons
I had a quick look at
arch/arm/mach-pxa/mioa701.cand noticed that it didn't callgpio_set_wake()for GPIO0. Does this help:arch/arm/mach-pxa/mioa701.c +0
-0
arch/arm/mach-pxa/mioa701.c static void __init mioa701_machine_init(void) · line 732
Quoted 7 lines
arch/arm/mach-pxa/mioa701.c +1
-0@@ -726,6 +726,7 @@ static void __init mioa701_machine_init(void)pxa2xx_mfp_config (ARRAY_AND_SIZE (mioa701_pin_config)); +gpio_set_wake (GPIO0_KEY_POWER ,1 );pxa_set_ffuart_info (NULL );pxa_set_btuart_info (NULL );pxa_set_stuart_info (NULL );
Yes, the wakeup works again with the gpio_set_wake() call.
But that shouldn't be necessary. The legacy method that worked, if I remember correctly, was :
- suspend is triggered
- gpio_keys.c: gpio_keys_suspend() is called
- as mioa701 setup the platform data for gpio_keys_data as :
MIO_KEY(KEY_EXIT, GPIO0_KEY_POWER, "Power button", 1),
=> implies struct gpio_keys_button has the button with .wakeup=1
This triggers in gpio_keys_suspend():
-> enable_irq_wake(bdata->irq)
This enable_irq_wake() should in the end call gpio_set_wake(0, 1), but apparently it doesn't for some reason. This the part I have to check.
Cheers.
Signature · 1 line
Robert
PATCH 1/1 gpio: pxa: Set PXA GPIO irq_chip IRQCHIP_SKIP_SET_WAKE flag
Quoted 37 lines · Robert Jarzmik
Review
Quoted 5 lines
I had a quick look at
arch/arm/mach-pxa/mioa701.cand noticed that it didn't callgpio_set_wake()for GPIO0. Does this help:(quoted diff) +0
-0
b/arch/arm/mach-pxa/mioa701.carch/arm/mach-pxa/mioa701.c static void __init
Quoted 4 lines
index 061d570..23190bf 100644
arch/arm/mach-pxa/mioa701.c +0
-0@@ -726,6 +726,7 @@ static void __init
mioa701_machine_init(void)Quoted 3 lines
?
?
? ???pxa2xx_mfp_config(ARRAY_AND_SIZE(mioa701_pin_config));
Quoted 4 lines
+???
gpio_set_wake(GPIO0_KEY_POWER, 1);
? ???pxa_set_ffuart_info(NULL);
? ???pxa_set_btuart_info(NULL);
? ???pxa_set_stuart_info(NULL);Yes, the wakeup works again with the
gpio_set_wake()call. But that shouldn't be necessary. The legacy method that worked, if I remember
correctly, was :
- suspend is triggered
-gpio_keys.c:gpio_keys_suspend()is called
- as mioa701 setup the platform data forgpio_keys_dataas
:
???MIO_KEY(KEY_EXIT,GPIO0_KEY_POWER, "Power button", 1),
? ???=> implies structgpio_keys_buttonhas the button with .wakeup=1 ???This triggers ingpio_keys_suspend():
? ? ? ? ? ? ? ?
? ???->enable_irq_wake(bdata->irq)
Quoted 3 lines
This
enable_irq_wake()should in the end callgpio_set_wake(0, 1), but apparently
it doesn't for some reason. This the part I have to check.
I posted a question about that 6 days ago: whether the irq_chip irq_set_wake() handler (which is called from enable_irq_wake()) should call gpio_set_wake():
http://lists.infradead.org/pipermail/linux-arm-kernel/2012-April/093953.html
Regards,
Paul
Robert Jarzmik
maintainerPATCH 1/1 gpio: pxa: Set PXA GPIO irq_chip IRQCHIP_SKIP_SET_WAKE flag
Haojian,
I feel there's some change in PXA gpio wakeup handling that's weird.
The effect : gpio_keys driver can't wakeup from suspend to RAM on GPIO0 activation
A quick analysis (thanks Paul for direction) :
- gpio_keys() calls irq_set_wake(96, 1) (ie. gpio0)
- gpio-pxa driver doesn't care (as it doesn't have any irq_set_wake function) (see in [1])
Now if I'm directing the mail at is because you're the only one who commited between v3.2 and v3.3 in gpio-pxa driver ([2]).
Could you tell me if removing irq_set_wake() was intentional ?
Cheers.
Signature · 12 lines
Robert [1] rj at beldin:~/mio_linux/kernel$ git show v3.2:drivers/gpio/gpio-pxa.c | grep set_wake void __init pxa_init_gpio(int mux_irq, int start, int end, set_wake_t fn) pxa_muxed_gpio_chip.irq_set_wake = fn; rj at beldin:~/mio_linux/kernel$ git show v3.3:drivers/gpio/gpio-pxa.c | grep set_wake rj at beldin:~/mio_linux/kernel$ [2] rj at beldin:~/mio_linux/kernel$ git log v3.3 ^v3.2 --pretty=format:%an drivers/gpio/gpio-pxa.c | sort -u Haojian Zhuang
Haojian Zhuang
maintainerPATCH 1/1 gpio: pxa: Set PXA GPIO irq_chip IRQCHIP_SKIP_SET_WAKE flag
Quoted 30 lines · Robert Jarzmik
Haojian,
I feel there's some change in PXA gpio wakeup handling that's weird.
The effect :
gpio_keysdriver can't wakeup from suspend to RAM on GPIO0 activationA quick analysis (thanks Paul for direction) :
?-gpio_keys()callsirq_set_wake(96, 1)(ie. gpio0)
?- gpio-pxa driver doesn't care (as it doesn't have anyirq_set_wakefunction) ? (see in [1])Now if I'm directing the mail at is because you're the only one who commited between v3.2 and v3.3 in gpio-pxa driver ([2]).
Could you tell me if removing
irq_set_wake()was intentional ?Cheers.
--
Robert[1]
rj at beldin:~/mio_linux/kernel$ git show v3.2:drivers/gpio/gpio-pxa.c | grepset_wakevoid__initpxa_init_gpio(int mux_irq, int start, int end, set_wake_t fn)? ? ? ?pxa_muxed_gpio_chip.irq_set_wake = fn;
rj at beldin:~/mio_linux/kernel$ git show v3.3:drivers/gpio/gpio-pxa.c | grepset_wakerj at beldin:~/mio_linux/kernel$
Quoted 3 lines
[2]
rj at beldin:~/mio_linux/kernel$ git log v3.3 ^v3.2 --pretty=format:%andrivers/gpio/gpio-pxa.c| sort -u Haojian Zhuang
Yes, I need to fix this. The set_wake() function will be transfered to gpio-pxa driver via platform data.
Thanks
Haojian
Robert Jarzmik
maintainerPATCH 1/1 arm/pxa: fix gpio wakeup setting
In 3.3, gpio wakeup setting was broken. The call enable_irq_wake() didn't set up the PXA gpio registers (PWER, ...) anymore.
Fix it at least for pxa27x. The driver doesn't seem to be used in pxa25x (weird ...), and the fix doesn't extend to pxa3xx and pxa95x (which don't have a gpio_set_wake() available).
--- arch/arm/mach-pxa/pxa27x.c | 6 +++++- drivers/gpio/gpio-pxa.c | 21 +++++++++++++++++++-- include/linux/gpio-pxa.h | 4 ++++ 3 files changed, 28 insertions(+), 3 deletions(-)
arch/arm/mach-pxa/pxa27x.c +5 -1
@@ -421,8 +421,11 @@ void __init pxa27x_set_i2c_power_info(struct i2c_pxa_platform_data *info)pxa_register_device (&pxa27x_device_i2c_power, info); } +static struct pxa_gpio_platform_data pxa27x_gpio_info__initdata = { + .gpio_set_wake = gpio_set_wake, +}; +static struct platform_device *devices[]__initdata = { - &pxa_device_gpio, &pxa27x_device_udc, &pxa_device_pmu, &pxa_device_i2s,
@@ -458,6 +461,7 @@ static int __init pxa27x_init(void)register_syscore_ops (&pxa2xx_mfp_syscore_ops);register_syscore_ops (&pxa2xx_clock_syscore_ops); +pxa_register_device (&pxa_device_gpio, &pxa27x_gpio_info); ret =platform_add_devices (devices,ARRAY_SIZE (devices)); }
drivers/gpio/gpio-pxa.c +19 -2
@@ -64,6 +64,7 @@ struct pxa_gpio_chip {unsigned long irq_mask;unsigned long irq_edge_rise;unsigned long irq_edge_fall; +int (*set_wake)(unsigned int gpio,unsigned int on);#ifdef CONFIG_PM unsigned long saved_gplr;
@@ -269,7 +270,8 @@ static void pxa_gpio_set(struct gpio_chip *chip, unsigned offset, int value) (value ? GPSR_OFFSET : GPCR_OFFSET)); } -static int __devinit pxa_init_gpio_chip (int gpio_end) +static int __devinit pxa_init_gpio_chip (int gpio_end, +int (*set_wake)(unsigned int ,unsigned int )) {int i, gpio, nbanks =gpio_to_bank (gpio_end) +1 ;struct pxa_gpio_chip *chips;
@@ -285,6 +287,7 @@ static int __devinit pxa_init_gpio_chip(int gpio_end)sprintf (chips[i].label,"gpio-%d" , i); chips[i].regbase = gpio_reg_base +BANK_OFF (i); + chips[i].set_wake = set_wake; c->base = gpio; c->label = chips[i].label;
@@ -412,6 +415,17 @@ static void pxa_mask_muxed_gpio(struct irq_data *d)writel_relaxed (gfer, c->regbase +GFER_OFFSET ); } +static int pxa_gpio_set_wake (struct irq_data *d,unsigned int on) +{ +int gpio =pxa_irq_to_gpio (d->irq); +struct pxa_gpio_chip *c =gpio_to_pxachip (gpio); + +if (c->set_wake) +return c->set_wake (gpio, on); +else +return 0 ; +} +static void pxa_unmask_muxed_gpio (struct irq_data *d) {int gpio =pxa_irq_to_gpio (d->irq);
@@ -427,6 +441,7 @@ static struct irq_chip pxa_muxed_gpio_chip = { .irq_mask = pxa_mask_muxed_gpio, .irq_unmask = pxa_unmask_muxed_gpio, .irq_set_type = pxa_gpio_irq_type, + .irq_set_wake = pxa_gpio_set_wake, };static int pxa_gpio_nums (void )
@@ -471,6 +486,7 @@ static int __devinit pxa_gpio_probe(struct platform_device *pdev)struct pxa_gpio_chip *c;struct resource *res;struct clk *clk; +struct pxa_gpio_platform_data *info;int gpio, irq, ret;int irq0 =0 , irq1 =0 , irq_mux, gpio_offset =0 ;
@@ -516,7 +532,8 @@ static int __devinit pxa_gpio_probe(struct platform_device *pdev) }/* Initialize GPIO chips */ -pxa_init_gpio_chip (pxa_last_gpio); + info =dev_get_platdata (&pdev->dev); +pxa_init_gpio_chip (pxa_last_gpio, info ? info->gpio_set_wake :NULL );/* clear all GPIO edge detects */ for_each_gpio_chip (gpio, c) {
include/linux/gpio-pxa.h +4 -0
@@ -13,4 +13,8 @@ extern int pxa_last_gpio;extern int pxa_irq_to_gpio (int irq); +struct pxa_gpio_platform_data { +int (*gpio_set_wake)(unsigned int gpio,unsigned int on); +}; +#endif /* __GPIO_PXA_H */
Signature · 1 line
1.7.5.4
Robert Jarzmik
maintainerPATCH 1/1 arm/pxa: fix gpio wakeup setting
Quoted 8 lines · Robert Jarzmik
In 3.3, gpio wakeup setting was broken. The call
enable_irq_wake()didn't set up the PXA gpio registers (PWER, ...) anymore.Fix it at least for pxa27x. The driver doesn't seem to be used in pxa25x (weird ...), and the fix doesn't extend to pxa3xx and pxa95x (which don't have a
gpio_set_wake()available).
Haojian, ping.
I'd like this to be included in 3.4, are you available nowadays ?
Cheers.
--
Robert
PATCH 1/1 arm/pxa: fix gpio wakeup setting
Quoted 11 lines · Robert Jarzmik
Quoted 8 lines · Robert Jarzmik
In 3.3, gpio wakeup setting was broken. The call
enable_irq_wake()didn't set up the PXA gpio registers (PWER, ...) anymore.Fix it at least for pxa27x. The driver doesn't seem to be used in pxa25x (weird ...), and the fix doesn't extend to pxa3xx and pxa95x (which don't have a
gpio_set_wake()available).
Quoted 7 lines
Haojian, ping.
I'd like this to be included in 3.4, are you available nowadays ?Cheers.
--
Robert
Applied
Regards
Haojian