Skip to contentSkip to conversation

pxa/hx4700: Avoid unbalanced irq wakeup enables/disables

PATCH New Paul Parsons 6 participants 12 messages May 5, 2011 – Feb 27, 2012

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

Paul Parsons

author
#0 lore raw

PATCH 1/1 pxa/hx4700: Avoid unbalanced irq wakeup enables/disables

Resuming a suspended hx4700 results in Unbalanced IRQ warnings:

WARNING: at kernel/irq/manage.c:507 irq_set_irq_wake+0xe0/0xec()

Unbalanced IRQ 246 wake disable
..

Likewise for IRQ 241 and IRQ 243. The 3 ASIC3 GPIO buttons - RECORD/CALENDAR/HOME - each fail in a call to enable_irq_wake() in gpio_keys_suspend() because of an absent irq_set_wake() handler in the asic3_gpio_irq_chip structure. Matching calls to disable_irq_wake() in gpio_keys_resume() then print the warnings. Since we should never need to resume via the 3 ASIC3 GPIO buttons, nor 2 of the 3 builtin GPIO buttons - MAIL/CONTACTS, the simplest remedy for the warnings is to enable irq wakeup on the POWER button only.

Signed-off-by Paul Parsons <[email protected]>

---

arch/arm/mach-pxa/hx4700.c +1 -1
@@ -191,7 +191,7 @@ static struct pxaficp_platform_data ficp
 		.active_low = _active_low,		\
 		.desc       = _desc,			\
 		.type       = EV_KEY,			\
-		.wakeup     = 1,			\
+		.wakeup     = KEY_##_code == KEY_POWER,	\
 	}
 
 static struct gpio_keys_button gpio_keys_buttons[] = {
#1 lore raw

PATCH 1/1 pxa/hx4700: Avoid unbalanced irq wakeup enables/disables

Quoted 7 lines · Paul Parsons

Resuming a suspended hx4700 results in Unbalanced IRQ warnings:

WARNING: at kernel/irq/manage.c:507 irq_set_irq_wake+0xe0/0xec()

Unbalanced IRQ 246 wake disable
..

Likewise for IRQ 241 and IRQ 243. The 3 ASIC3 GPIO buttons - RECORD/CALENDAR/HOME - each fail in a call to enable_irq_wake() in gpio_keys_suspend() because of an absent irq_set_wake() handler in the asic3_gpio_irq_chip structure. Matching calls to disable_irq_wake() in gpio_keys_resume() then print the warnings. Since we should never need to resume via the 3 ASIC3 GPIO buttons, nor 2 of the 3 builtin GPIO buttons - MAIL/CONTACTS, the simplest remedy for the warnings is to enable irq wakeup on the POWER button only.

Not sure if it's right to allow only POWER button to wake the device. In both Windows CE and handhelds.org 2.6.21 kernel any button can wake device, and even start corresponding action (i.e. run some app) after wakeup. I think we should just disable wakeup on ASIC3 button for now, and/or think about how to implement .set_wake in ASIC3.

For first part I mean something like this (not sure if it still applies, but you should get the idea):

Date: Sat, 31 Jul 2010 11:55:25 +0400
Subject: [PATCH 17/47] pxa/hx4700: don't wake on ASIC3 buttons

Currently ASIC3 driver doesn't support set_wake on its GPIOs, which leads to "Unbalanced IRQ ### wake disable" warnings on wakeup. Workaround this by simply not using buttons connected to ASIC3 gpios as wakeup sources.

Signed-off-by Dmitry Artamonow <[email protected]>
---
 arch/arm/mach-pxa/hx4700.c |   16 ++++++++--------
 1 files changed, 8 insertions(+), 8 deletions(-)
arch/arm/mach-pxa/hx4700.c +8 -8
index fa33fef..6c78f93 100644
@@ -202,23 +202,23 @@ static struct pxaficp_platform_data ficp_info = {
  * GPIO Keys
  */
 
-#define INIT_KEY(_code, _gpio, _active_low, _desc)	\
+#define INIT_KEY(_code, _gpio, _active_low, _wake, _desc)	\
 	{						\
 		.code       = KEY_##_code,		\
 		.gpio       = _gpio,			\
 		.active_low = _active_low,		\
 		.desc       = _desc,			\
 		.type       = EV_KEY,			\
-		.wakeup     = 1,			\
+		.wakeup     = _wake,			\
 	}
 
 static struct gpio_keys_button gpio_keys_buttons[] = {
-	INIT_KEY(POWER,       GPIO0_HX4700_nKEY_POWER,   1, "Power button"),
-	INIT_KEY(MAIL,        GPIO94_HX4700_KEY_MAIL,    0, "Mail button"),
-	INIT_KEY(ADDRESSBOOK, GPIO99_HX4700_KEY_CONTACTS,0, "Contacts button"),
-	INIT_KEY(RECORD,      GPIOD6_nKEY_RECORD,        1, "Record button"),
-	INIT_KEY(CALENDAR,    GPIOD1_nKEY_CALENDAR,      1, "Calendar button"),
-	INIT_KEY(HOMEPAGE,    GPIOD3_nKEY_HOME,          1, "Home button"),
+	INIT_KEY(POWER,       GPIO0_HX4700_nKEY_POWER,   1, 1, "Power button"),
+	INIT_KEY(MAIL,        GPIO94_HX4700_KEY_MAIL,    0, 1, "Mail button"),
+	INIT_KEY(ADDRESSBOOK, GPIO99_HX4700_KEY_CONTACTS,0, 1, "Contacts button"),
+	INIT_KEY(RECORD,      GPIOD6_nKEY_RECORD,        1, 0, "Record button"),
+	INIT_KEY(CALENDAR,    GPIOD1_nKEY_CALENDAR,      1, 0, "Calendar button"),
+	INIT_KEY(HOMEPAGE,    GPIOD3_nKEY_HOME,          1, 0, "Home button"),
 };
 
 static struct gpio_keys_platform_data gpio_keys_data = {
Signature · 2 lines
Best regards,
Dmitry "MAD" Artamonow
author
#2 lore raw

PATCH 1/1 pxa/hx4700: Avoid unbalanced irq wakeup enables/disables

Hi Dmitry,

I tried your patch and found that the MAIL and CONTACTS buttons did not resume the unit. The same is true of the handhelds.org 2.6.21-hh20 kernel. I don't have WinCE any more so am unable to test that.

The reason appears to be that neither GPIO94 (MAIL) nor GPIO99 (CONTACTS) can be enabled as a wake-up source in the PWER register.

Regards,
Paul

Quoted 140 lines · Dmitry Artamonow

From: Dmitry Artamonow <[email protected]>
Subject: Re: [PATCH] pxa/hx4700: Avoid unbalanced irq wakeup enables/disables
To: "Paul Parsons" <[email protected]>
Cc: linux-arm-kernel at lists.infradead.org, eric.y.miao at gmail.com, philipp.zabel at gmail.com
Date: Friday, 13 May, 2011, 11:13

Quoted 1 line · Paul Parsons

Resuming a suspended hx4700 results in Unbalanced IRQ

warnings:

Quoted 2 lines
WARNING: at kernel/irq/manage.c:507

irq_set_irq_wake+0xe0/0xec()

Quoted 4 lines

Unbalanced IRQ 246 wake disable
..

Likewise for IRQ 241 and IRQ 243. The 3 ASIC3 GPIO

buttons - RECORD/CALENDAR/HOME - each fail in a call to enable_irq_wake() in gpio_keys_suspend() because of an absent irq_set_wake() handler in the asic3_gpio_irq_chip structure. Matching calls to disable_irq_wake() in gpio_keys_resume() then print the warnings. Since we should never need to resume via the 3 ASIC3 GPIO buttons, nor 2 of the 3 builtin GPIO buttons - MAIL/CONTACTS, the simplest remedy for the warnings is to enable irq wakeup on the POWER button only.

Not sure if it's right to allow only POWER button to wake the device.
In both Windows CE and handhelds.org 2.6.21 kernel any button can wake
device, and even start corresponding action (i.e. run some app) after wakeup.
I think we should just disable wakeup on ASIC3 button for now, and/or think
about how to implement .set_wake in ASIC3.

For first part I mean something like this (not sure if it still applies,
but you should get the idea):

Date: Sat, 31 Jul 2010 11:55:25 +0400
Subject: [PATCH 17/47] pxa/hx4700: don't wake on ASIC3 buttons

Currently ASIC3 driver doesn't support set_wake on its GPIOs,
which leads to "Unbalanced IRQ ### wake disable" warnings on wakeup.
Workaround this by simply not using buttons connected to ASIC3
gpios as wakeup sources.

Signed-off-by Dmitry Artamonow <[email protected]>

---
arch/arm/mach-pxa/hx4700.c |???16
++++++++--------

 1 files changed, 8 insertions(+), 8 deletions(-)
arch/arm/mach-pxa/hx4700.c +0 -0
index fa33fef..6c78f93 100644
@@ -202,23 +202,23 @@ static struct pxaficp_platform_data
 ficp_info = {

? * GPIO Keys
? */

-#define INIT_KEY(_code, _gpio, _active_low, _desc)??? \
+#define INIT_KEY(_code, _gpio, _active_low, _wake, _desc)??? \
??? {??? ???
??? ??? ???
??? \
??? ??? .code? ?
???= KEY_##_code,???
??? \
??? ??? .gpio? ?
???= _gpio,???
??? ??? \
??? ??? .active_low =
_active_low,??? ??? \
??? ??? .desc? ?
???= _desc,???
??? ??? \
??? ??? .type? ?
???= EV_KEY,???
??? ??? \
-??? ??? .wakeup?
???= 1,??? ???
??? \
+??? ??? .wakeup?
???= _wake,???
??? ??? \
??? }

static struct gpio_keys_button gpio_keys_buttons[] = {
-??? INIT_KEY(POWER,? ? ???GPIO0_HX4700_nKEY_POWER,???1,
"Power button"),
-??? INIT_KEY(MAIL,? ? ?
? GPIO94_HX4700_KEY_MAIL,? ? 0, "Mail button"),
-??? INIT_KEY(ADDRESSBOOK, GPIO99_HX4700_KEY_CONTACTS,0, "Contacts button"), -??? INIT_KEY(RECORD,? ? ? GPIOD6_nKEY_RECORD,? ? ? ? 1, "Record button"),
-??? INIT_KEY(CALENDAR,? ? GPIOD1_nKEY_CALENDAR,? ? ? 1, "Calendar button"),
-??? INIT_KEY(HOMEPAGE,? ?
GPIOD3_nKEY_HOME,? ? ? ? ? 1, "Home
button"),
+??? INIT_KEY(POWER,? ? ???GPIO0_HX4700_nKEY_POWER,???1,
1, "Power button"),
+??? INIT_KEY(MAIL,? ? ?
? GPIO94_HX4700_KEY_MAIL,? ? 0, 1, "Mail button"),
+??? INIT_KEY(ADDRESSBOOK, GPIO99_HX4700_KEY_CONTACTS,0, 1, "Contacts button"), +??? INIT_KEY(RECORD,? ? ? GPIOD6_nKEY_RECORD,? ? ? ? 1, 0, "Record button"),
+??? INIT_KEY(CALENDAR,? ? GPIOD1_nKEY_CALENDAR,? ? ? 1, 0, "Calendar button"),
+??? INIT_KEY(HOMEPAGE,? ?
GPIOD3_nKEY_HOME,? ? ? ? ? 1, 0,
"Home button"),
};

static struct gpio_keys_platform_data gpio_keys_data = {

Signature · 2 lines
Best regards,
Dmitry "MAD" Artamonow
#3 lore raw

PATCH 1/1 pxa/hx4700: Avoid unbalanced irq wakeup enables/disables

Quoted 5 lines · Paul Parsons

Hi Dmitry,

I tried your patch and found that the MAIL and CONTACTS buttons did not resume the unit. The same is true of the handhelds.org 2.6.21-hh20 kernel. I don't have WinCE any more so am unable to test that.

The reason appears to be that neither GPIO94 (MAIL) nor GPIO99 (CONTACTS) can be enabled as a wake-up source in the PWER register.

Well, you right here - GPIO94 and GPIO99 can't be enabled as wake-up sources in PWER on PXA270. But they can be enabled in PKWR, according to section 3.8.1.15 of PXA27x Developer's Manual. And there's a proof: WinCE happily wakes on MAIL/CONTACTS presses. Though probably these pins will need some configuration in MFP table to work as wake-up sources...

Anyway, I have no strong opinion about whether to disable just ASIC3 buttons, or to leave only POWER as wakeup source. So I think your patch is fine as the solution to unbalanced irq wakeup problem and may go in, unless Philipp or Eric have objections.

Signature · 2 lines
Best regards,
Dmitry "MAD" Artamonow
author
#4 lore raw

PATCH 1/1 pxa/hx4700: Avoid unbalanced irq wakeup enables/disables

Hi Dmitry,

Quoted 10 lines

Well, you right here - GPIO94 and GPIO99 can't be enabled as wake-up
sources in PWER on PXA270. But they can be enabled in PKWR, according to
section 3.8.1.15 of PXA27x Developer's Manual. And there's a proof: WinCE
happily wakes on MAIL/CONTACTS presses. Though probably these pins will
need some configuration in MFP table to work as wake-up sources...

Ah, another wake-up enable register; I missed that! Yes, that should work via tweaks to the MFP table.

Quoted 7 lines

Anyway, I have no strong opinion about whether to disable just ASIC3
buttons, or to leave only POWER as wakeup source. So I think your patch is
fine as the solution to unbalanced irq wakeup problem and may go in, unless
Philipp or Eric have objections.

My feeling is that it should be all or nothing; we either get all other 5 buttons working as wake-ups or we disable them. Perhaps Russell's comments are the clincher.

While on the subject, it turns out that the reset button (= GPIO reset) also acts as a wake-up on my unit. But the PXA27x manual says this:

"In standby, sleep, and deep-sleep modes, GPIO reset is first treated as a wake-up event."

Does this mean that both the SMR and GPR bits the RCSR register are set? And if so, does the bootloader then decide whether to wake-up the unit or whether to reset it?

Regards,
Paul

#5 lore raw

PATCH 1/1 pxa/hx4700: Avoid unbalanced irq wakeup enables/disables

Quoted 23 lines · Paul Parsons

Hi Dmitry,

Quoted 10 lines

Well, you right here - GPIO94 and GPIO99 can't be enabled as wake-up
sources in PWER on PXA270. But they can be enabled in PKWR, according to
section 3.8.1.15 of PXA27x Developer's Manual. And there's a proof: WinCE
happily wakes on MAIL/CONTACTS presses. Though probably these pins will
need some configuration in MFP table to work as wake-up sources...

Ah, another wake-up enable register; I missed that! Yes, that should work via tweaks to the MFP table.

Quoted 7 lines

Anyway, I have no strong opinion about whether to disable just ASIC3
buttons, or to leave only POWER as wakeup source. So I think your patch is
fine as the solution to unbalanced irq wakeup problem and may go in, unless
Philipp or Eric have objections.

Quoted 2 lines

My feeling is that it should be all or nothing; we either get all other 5 buttons working as wake-ups or we disable them. Perhaps Russell's comments are the clincher.

To use GPIOs in PKWR, you need to do two things:

1. WAKEUP_ON_LEVEL_HIGH or'ed with the pin configuration (see mfp-pxa2xx.h)
2. enable_irq_wakeup() will eventually invoke gpio_set_wake() in mfp-pxa2xx.c, and that function will handle the PKWR setting.

Quoted 6 lines

While on the subject, it turns out that the reset button (= GPIO reset) also acts as a wake-up on my unit. But the PXA27x manual says this:

"In standby, sleep, and deep-sleep modes, GPIO reset is first treated as a wake-up event."

Does this mean that both the SMR and GPR bits the RCSR register are set? And if so, does the bootloader then decide whether to wake-up the unit or whether to reset it?

The full statement is:

In standby, sleep, and deep-sleep modes, GPIO reset is first treated as a wake-up event. It is then
recognized as a GPIO reset if held asserted for more than 1 ms after all power supplies are stable.

And my understanding of this is - an assertion on GPIO<1> will firstly be treated as a wake-up event and cause the sleep-exit. SMR in RCSR will be set. A normal resume process begins from software perspective. If the assertion is a pulse less than 1ms, no reset will happen. However, if the assertion is more than 1ms, then GPIO reset will happen wherever the software resuming process is. SMR will most likely be cleared during this reset process.

In other word, the whole process is transparent to software including the bootloader.

Quoted 3 lines

Regards,
Paul

#6 lore raw

PATCH 1/1 pxa/hx4700: Avoid unbalanced irq wakeup enables/disables

In reply to Dmitry Artamonow
Quoted 3 lines · Dmitry Artamonow

Not sure if it's right to allow only POWER button to wake the device. In both Windows CE and handhelds.org 2.6.21 kernel any button can wake device, and even start corresponding action (i.e. run some app) after wakeup.

Ah, I'm glad you mentioned that.

This 'feature' is really annoying, as it keeps resulting in my iPAQ battery being drained completely. My iPAQ now lives plugged into its charger because that's the only way I can ensure that the programs and data on it stay there. If I put it anywhere else, a button will get pressed unnoticed and then I'll have to reload everything back on to it.

So really, whether a non-power button wakes up or not should be ultimately end-user configurable. Wish it was in wince.

#7 lore raw

PATCH 1/1 pxa/hx4700: Avoid unbalanced irq wakeup enables/disables

In reply to Paul Parsons

Am Donnerstag, den 05.05.2011, 01:10 +0000 schrieb Paul Parsons:

Quoted 14 lines

Resuming a suspended hx4700 results in Unbalanced IRQ warnings:

WARNING: at kernel/irq/manage.c:507 irq_set_irq_wake+0xe0/0xec()

Unbalanced IRQ 246 wake disable
..

Likewise for IRQ 241 and IRQ 243. The 3 ASIC3 GPIO buttons - RECORD/CALENDAR/HOME - each fail in a call to enable_irq_wake() in gpio_keys_suspend() because of an absent irq_set_wake() handler in the asic3_gpio_irq_chip structure. Matching calls to disable_irq_wake() in gpio_keys_resume() then print the warnings. Since we should never need to resume via the 3 ASIC3 GPIO buttons, nor 2 of the 3 builtin GPIO buttons - MAIL/CONTACTS, the simplest remedy for the warnings is to enable irq wakeup on the POWER button only.

Signed-off-by Paul Parsons <[email protected]>

---

arch/arm/mach-pxa/hx4700.c static struct pxaficp_platform_data ficp · line 197

Quoted 8 lines
arch/arm/mach-pxa/hx4700.c +1 -1
@@ -191,7 +191,7 @@ static struct pxaficp_platform_data ficp
 		.active_low = _active_low,		\
 		.desc       = _desc,			\
 		.type       = EV_KEY,			\
-		.wakeup     = 1,			\
+		.wakeup     = KEY_##_code == KEY_POWER,	\
 	}
 
 static struct gpio_keys_button gpio_keys_buttons[] = {

On my hx4700 iPAQ running Windows Mobile 5, gpio button wakeup is configurable under "Start -> Settings -> Personal -> Buttons -> Lock". There is a checkbox "Disable all buttons except power button" which enables/disables the keypad and ASIC3 GPIO buttons as wakeup sources.

To achieve the same functionality on linux, we could split the power button and the other buttons into separate gpio_keys devices and let userspace disable the other buttons as wakeup sources via the device's pm_wakeup interface. For that we need

Given that ASIC3 doesn't support irq_set_wake for now,

Acked-by Philipp Zabel <[email protected]>

regards
Philipp

maintainer
#8 lore raw

PATCH 1/1 pxa/hx4700: Avoid unbalanced irq wakeup enables/disables

Quoted 29 lines · Philipp Zabel

Am Donnerstag, den 05.05.2011, 01:10 +0000 schrieb Paul Parsons:

Quoted 14 lines

Resuming a suspended hx4700 results in Unbalanced IRQ warnings:

WARNING: at kernel/irq/manage.c:507 irq_set_irq_wake+0xe0/0xec()

Unbalanced IRQ 246 wake disable
..

Likewise for IRQ 241 and IRQ 243. The 3 ASIC3 GPIO buttons - RECORD/CALENDAR/HOME - each fail in a call to enable_irq_wake() in gpio_keys_suspend() because of an absent irq_set_wake() handler in the asic3_gpio_irq_chip structure. Matching calls to disable_irq_wake() in gpio_keys_resume() then print the warnings. Since we should never need to resume via the 3 ASIC3 GPIO buttons, nor 2 of the 3 builtin GPIO buttons - MAIL/CONTACTS, the simplest remedy for the warnings is to enable irq wakeup on the POWER button only.

Signed-off-by Paul Parsons <[email protected]>

---

arch/arm/mach-pxa/hx4700.c ? ? ? 2011-05-05 00:07:57.034493981 +0100 static struct pxaficp_platform_data ficp · line 197

Quoted 8 lines
arch/arm/mach-pxa/hx4700.c ? ? ? 2011-05-05 00:07:57.034493981 +0100 renamed from arch/arm/mach-pxa/hx4700.c ? ? ? 2011-05-04 12:09:06.298511475 +0100 +1 -1
@@ -191,7 +191,7 @@ static struct pxaficp_platform_data ficp
 ? ? ? ? ? ? ? .active_low = _active_low, ? ? ? ? ? ? ?\
 ? ? ? ? ? ? ? .desc ? ? ? = _desc, ? ? ? ? ? ? ? ? ? ?\
 ? ? ? ? ? ? ? .type ? ? ? = EV_KEY, ? ? ? ? ? ? ? ? ? \
- ? ? ? ? ? ? .wakeup ? ? = 1, ? ? ? ? ? ? ? ? ? ? ? ?\
+ ? ? ? ? ? ? .wakeup ? ? = KEY_##_code == KEY_POWER, \
 ? ? ? }
 
 ?static struct gpio_keys_button gpio_keys_buttons[] = {

On my hx4700 iPAQ running Windows Mobile 5, gpio button wakeup is configurable under "Start -> Settings -> Personal -> Buttons -> Lock". There is a checkbox "Disable all buttons except power button" which enables/disables the keypad and ASIC3 GPIO buttons as wakeup sources.

Quoted 8 lines

To achieve the same functionality on linux, we could split the power button and the other buttons into separate gpio_keys devices and let userspace disable the other buttons as wakeup sources via the device's pm_wakeup interface. For that we need

Given that ASIC3 doesn't support irq_set_wake for now,

Acked-by Philipp Zabel <[email protected]>

It seems that this patch is only enable powerup key as wakeup source. Other gpio keys could be configured as wakeup source in user space. So could you help to figure out the interface of configuring gpio key as wakeup source in user space?

Thanks
Haojian

#9 lore raw

PATCH 1/1 pxa/hx4700: Avoid unbalanced irq wakeup enables/disables

Am Montag, den 27.02.2012, 10:00 +0800 schrieb Haojian Zhuang:

Quoted 6 lines · Philipp Zabel
Quoted 5 lines

On my hx4700 iPAQ running Windows Mobile 5, gpio button wakeup is configurable under "Start -> Settings -> Personal -> Buttons -> Lock". There is a checkbox "Disable all buttons except power button" which enables/disables the keypad and ASIC3 GPIO buttons as wakeup sources.

Quoted 6 lines
Quoted 4 lines

To achieve the same functionality on linux, we could split the power button and the other buttons into separate gpio_keys devices and let userspace disable the other buttons as wakeup sources via the device's pm_wakeup interface.

It seems that this patch is only enable powerup key as wakeup source.

Yes. This is temporary until ASIC3 GPIO wakeup support is in place.

Quoted 3 lines

Other gpio keys could be configured as wakeup source in user space. So could you help to figure out the interface of configuring gpio key as wakeup source in user space?

I was thinking of the /sys/devices/.../power/wakeup interface, as documented in Documentation/ABI/testing/sysfs-devices-power.

Put the power button into its own gpio-keys device:

arch/arm/mach-pxa/hx4700.c +19 -3
index 06ec926..21d26a3 100644
@@ -166,11 +166,26 @@ static struct pxaficp_platform_data ficp_info = {
 		.active_low = _active_low,		\
 		.desc       = _desc,			\
 		.type       = EV_KEY,			\
-		.wakeup     = KEY_##_code == KEY_POWER,	\
+		.wakeup     = 1,			\
 	}
 
+static struct gpio_keys_button gpio_keys_power_button =
+	INIT_KEY(POWER,       GPIO0_HX4700_nKEY_POWER,   1, "Power button");
+
+static struct gpio_keys_platform_data gpio_keys_power_data = {
+	.buttons = gpio_keys_power_button,
+	.nbuttons = 1,
+};
+
+static struct platform_device gpio_keys_power = {
+	.name = "gpio-keys",
+	.dev  = {
+		.platform_data = &gpio_keys_power_data,
+	},
+	.id   = 0,
+};
+
 static struct gpio_keys_button gpio_keys_buttons[] = {
-	INIT_KEY(POWER,       GPIO0_HX4700_nKEY_POWER,   1, "Power button"),
 	INIT_KEY(MAIL,        GPIO94_HX4700_KEY_MAIL,    0, "Mail button"),
 	INIT_KEY(ADDRESSBOOK, GPIO99_HX4700_KEY_CONTACTS,0, "Contacts button"),
 	INIT_KEY(RECORD,      GPIOD6_nKEY_RECORD,        1, "Record button"),

@@ -188,7 +203,7 @@ static struct platform_device gpio_keys = {
 	.dev  = {
 		.platform_data = &gpio_keys_data,
 	},
-	.id   = -1,
+	.id   = 1,
 };
 
 /*

@@ -836,6 +851,7 @@ static struct platform_device audio = {
 
 static struct platform_device *devices[] __initdata = {
 	&asic3,
+	&gpio_keys_power,
 	&gpio_keys,
 	&navpoint,
 	&backlight,

And then let userspace disable wakeup for the other keys via echo disabled > /sys/devices/platform/gpio-keys.1/power/wakeup

The gpio-keys driver will check this setting with device_may_wakeup() during gpio_keys_suspend() and decide whether to call enable_irq_wake() for its button GPIOs.

regards
Philipp

maintainer
#10 lore raw

PATCH 1/1 pxa/hx4700: Avoid unbalanced irq wakeup enables/disables

Quoted 79 lines · Philipp Zabel

Am Montag, den 27.02.2012, 10:00 +0800 schrieb Haojian Zhuang:

Quoted 6 lines · Philipp Zabel
Quoted 5 lines

On my hx4700 iPAQ running Windows Mobile 5, gpio button wakeup is configurable under "Start -> Settings -> Personal -> Buttons -> Lock". There is a checkbox "Disable all buttons except power button" which enables/disables the keypad and ASIC3 GPIO buttons as wakeup sources.

Quoted 6 lines
Quoted 4 lines

To achieve the same functionality on linux, we could split the power button and the other buttons into separate gpio_keys devices and let userspace disable the other buttons as wakeup sources via the device's pm_wakeup interface.

It seems that this patch is only enable powerup key as wakeup source.

Yes. This is temporary until ASIC3 GPIO wakeup support is in place.

Quoted 3 lines

Other gpio keys could be configured as wakeup source in user space. So could you help to figure out the interface of configuring gpio key as wakeup source in user space?

I was thinking of the /sys/devices/.../power/wakeup interface, as documented in Documentation/ABI/testing/sysfs-devices-power.

Put the power button into its own gpio-keys device:

arch/arm/mach-pxa/hx4700.c +18 -3
index 06ec926..21d26a3 100644
@@ -166,11 +166,26 @@ static struct pxaficp_platform_data ficp_info = {
 ? ? ? ? ? ? ? ?.active_low = _active_low, ? ? ? ? ? ? ?\
 ? ? ? ? ? ? ? ?.desc ? ? ? = _desc, ? ? ? ? ? ? ? ? ? ?\
 ? ? ? ? ? ? ? ?.type ? ? ? = EV_KEY, ? ? ? ? ? ? ? ? ? \
- ? ? ? ? ? ? ? .wakeup ? ? = KEY_##_code == KEY_POWER, \
+ ? ? ? ? ? ? ? .wakeup ? ? = 1, ? ? ? ? ? ? ? ? ? ? ? ?\
 ? ? ? ?}
 
+static struct gpio_keys_button gpio_keys_power_button =
+ ? ? ? INIT_KEY(POWER, ? ? ? GPIO0_HX4700_nKEY_POWER, ? 1, "Power button");
+
+static struct gpio_keys_platform_data gpio_keys_power_data = {
+ ? ? ? .buttons = gpio_keys_power_button,
+ ? ? ? .nbuttons = 1,
+};
+
+static struct platform_device gpio_keys_power = {
+ ? ? ? .name = "gpio-keys",
+ ? ? ? .dev ?= {
+ ? ? ? ? ? ? ? .platform_data = &gpio_keys_power_data,
+ ? ? ? },
+ ? ? ? .id ? = 0,
+};
+
 ?static struct gpio_keys_button gpio_keys_buttons[] = {
- ? ? ? INIT_KEY(POWER, ? ? ? GPIO0_HX4700_nKEY_POWER, ? 1, "Power button"),
 ? ? ? ?INIT_KEY(MAIL, ? ? ? ?GPIO94_HX4700_KEY_MAIL, ? ?0, "Mail button"),
 ? ? ? ?INIT_KEY(ADDRESSBOOK, GPIO99_HX4700_KEY_CONTACTS,0, "Contacts button"),
 ? ? ? ?INIT_KEY(RECORD, ? ? ?GPIOD6_nKEY_RECORD, ? ? ? ?1, "Record button"),

@@ -188,7 +203,7 @@ static struct platform_device gpio_keys = {
 ? ? ? ?.dev ?= {
 ? ? ? ? ? ? ? ?.platform_data = &gpio_keys_data,
 ? ? ? ?},
- ? ? ? .id ? = -1,
+ ? ? ? .id ? = 1,
 ?};

?/*

arch/arm/mach-pxa/hx4700.c +1 -0
@@ -836,6 +851,7 @@ static struct platform_device audio = {
 
 ?static struct platform_device *devices[] __initdata = {
 ? ? ? ?&asic3,
+ ? ? ? &gpio_keys_power,
 ? ? ? ?&gpio_keys,
 ? ? ? ?&navpoint,
 ? ? ? ?&backlight,

And then let userspace disable wakeup for the other keys via echo disabled > /sys/devices/platform/gpio-keys.1/power/wakeup

Quoted 8 lines

The gpio-keys driver will check this setting with device_may_wakeup() during gpio_keys_suspend() and decide whether to call enable_irq_wake() for its button GPIOs.

regards
Philipp

If one of gpio keys wakeup is enabled, every gpio keys in this device can wakeup system. So your patch is better than original one that only keeping power key as wakeup source.

You overwrite Paul's patch. I think that you can format a new patch that is based on current code base not Paul's patch.

Thanks
Haojian

author
#11 lore raw

PATCH 1/1 pxa/hx4700: Avoid unbalanced irq wakeup enables/disables

Quoted 128 lines
Quoted 1 line
Quoted 1 line
Quoted 1 line

On my hx4700 iPAQ running Windows Mobile 5,

gpio button wakeup is

Quoted 1 line
Quoted 1 line
Quoted 1 line

configurable under "Start -> Settings ->

Personal -> Buttons -> Lock".

Quoted 1 line
Quoted 1 line
Quoted 1 line

There is a checkbox "Disable all buttons

except power button" which

Quoted 1 line
Quoted 1 line
Quoted 1 line

enables/disables the keypad and ASIC3 GPIO

buttons as wakeup sources.

Quoted 2 lines
Quoted 2 lines
Quoted 2 lines

To achieve the same functionality on linux, we

could split the power

Quoted 1 line
Quoted 1 line
Quoted 1 line

button and the other buttons into separate

gpio_keys devices and let

Quoted 1 line
Quoted 1 line
Quoted 1 line

userspace disable the other buttons as wakeup

sources via the device's

Quoted 3 lines
Quoted 3 lines
Quoted 1 line

pm_wakeup interface.

It seems that this patch is only enable powerup key

as wakeup source.

Quoted 2 lines

Yes. This is temporary until ASIC3 GPIO wakeup support

is in place.

Quoted 2 lines
Quoted 2 lines

Other gpio keys could be configured as wakeup

source in user space. So could

Quoted 1 line
Quoted 1 line

you help to figure out the interface of configuring

gpio key as wakeup source

Quoted 3 lines
Quoted 1 line

in user space?

I was thinking of the /sys/devices/.../power/wakeup

interface, as

Quoted 1 line

documented in

Documentation/ABI/testing/sysfs-devices-power.

Review

Quoted 4 lines

Put the power button into its own gpio-keys device:

(quoted diff) +0 -0

b/arch/arm/mach-pxa/hx4700.c

arch/arm/mach-pxa/hx4700.c static struct

Quoted 4 lines

index 06ec926..21d26a3 100644

arch/arm/mach-pxa/hx4700.c +0 -0
@@ -166,11 +166,26 @@ static struct

pxaficp_platform_data ficp_info = {

Quoted 1 line

? ? ? ? ? ? ? ?.active_low = _active_low, ? ?

? ? ? ? ?\

Quoted 1 line

? ? ? ? ? ? ? ?.desc ? ? ? = _desc, ? ? ?

? ? ? ? ? ? ?\

Quoted 1 line

? ? ? ? ? ? ? ?.type ? ? ? = EV_KEY, ? ?

? ? ? ? ? ? ? \

Quoted 1 line

- ? ? ? ? ? ? ? .wakeup ? ? = KEY_##_code ==

KEY_POWER, \

Quoted 1 line

+ ? ? ? ? ? ? ? .wakeup ? ? = 1, ? ? ? ?

? ? ? ? ? ? ? ?\

Quoted 3 lines

? ? ? ?}

+static struct gpio_keys_button gpio_keys_power_button

=

Quoted 1 line

+ ? ? ? INIT_KEY(POWER, ? ? ?

GPIO0_HX4700_nKEY_POWER, ? 1, "Power button");

arch/arm/mach-pxa/hx4700.c

Quoted 2 lines
arch/arm/mach-pxa/hx4700.c +2 -0
+
+static struct gpio_keys_platform_data

gpio_keys_power_data = {

arch/arm/mach-pxa/hx4700.c static struct pxaficp_platform_data ficp_info = {

Quoted 8 lines

+ ? ? ? .buttons = gpio_keys_power_button,
+ ? ? ? .nbuttons = 1,

arch/arm/mach-pxa/hx4700.c +6 -0
+};
+
+static struct platform_device gpio_keys_power = {
+ ? ? ? .name = "gpio-keys",
+ ? ? ? .dev ?= {
+ ? ? ? ? ? ? ? .platform_data =

&gpio_keys_power_data,

Quoted 5 lines

+ ? ? ? },
+ ? ? ? .id ? = 0,

arch/arm/mach-pxa/hx4700.c +2 -0
+};
+

?static struct gpio_keys_button gpio_keys_buttons[] =

{

Quoted 1 line

- ? ? ? INIT_KEY(POWER, ? ? ?

GPIO0_HX4700_nKEY_POWER, ? 1, "Power button"),

Quoted 1 line

? ? ? ?INIT_KEY(MAIL, ? ? ?

?GPIO94_HX4700_KEY_MAIL, ? ?0, "Mail button"),

Quoted 1 line

? ? ? ?INIT_KEY(ADDRESSBOOK,

GPIO99_HX4700_KEY_CONTACTS,0, "Contacts button"),

Quoted 1 line

? ? ? ?INIT_KEY(RECORD, ? ? ?GPIOD6_nKEY_RECORD,

? ? ? ?1, "Record button"),

arch/arm/mach-pxa/hx4700.c static struct platform_device

Quoted 1 line
arch/arm/mach-pxa/hx4700.c +0 -0
@@ -188,7 +203,7 @@ static struct platform_device

gpio_keys = {

Quoted 2 lines
arch/arm/mach-pxa/hx4700.c +0 -0
 ? ? ? ?.dev ?= {

? ? ? ? ? ? ? ?.platform_data =

&gpio_keys_data,

arch/arm/mach-pxa/hx4700.c static struct platform_device audio

Quoted 7 lines

? ? ? ?},
- ? ? ? .id ? = -1,
+ ? ? ? .id ? = 1,
?};

?/*

arch/arm/mach-pxa/hx4700.c +0 -0
@@ -836,6 +851,7 @@ static struct platform_device audio

= {

Quoted 2 lines

?static struct platform_device *devices[] __initdata =

{

Quoted 7 lines

? ? ? ?&asic3,
+ ? ? ? &gpio_keys_power,
? ? ? ?&gpio_keys,
? ? ? ?&navpoint,
? ? ? ?&backlight,

And then let userspace disable wakeup for the other

keys via

Quoted 1 line

echo disabled >

/sys/devices/platform/gpio-keys.1/power/wakeup

Quoted 2 lines

The gpio-keys driver will check this setting with

device_may_wakeup()

Quoted 1 line

during gpio_keys_suspend() and decide whether to call

enable_irq_wake()

Quoted 6 lines

for its button GPIOs.

regards
Philipp

If one of gpio keys wakeup is enabled, every gpio keys in this device can
wakeup system. So your patch is better than original one that only keeping
power key as wakeup source.

Quoted 3 lines

You overwrite Paul's patch. I think that you can format a new patch that is
based on current code base not Paul's patch.

I'm not sure if allowing user space software to define which buttons can be used as wakeup sources is a really problem which needs to be solved.

Windows Mobile 5 may provide this functionality, but is there any Linux mobile software which attempts to do this?