[PATCH] Input: gpio_keys: support wakeup system from freeze mode

For "freeze" mode of suspend, cpu will go into idle and
those wakeup sources' irq should NOT be disabled during
devices suspend, so we need to add IRQF_NO_SUSPEND flag
for those wakeup sources.

Steps to test this patch:

1. echo freeze > /sys/power/state;
2. press gpio key which has wakeup function, then system
   will resume.

Signed-off-by: Anson Huang <redacted>
---
 drivers/input/keyboard/gpio_keys.c |    2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/input/keyboard/gpio_keys.c b/drivers/input/keyboard/gpio_keys.c
index 2db1324..aadb1db 100644
--- a/drivers/input/keyboard/gpio_keys.c
+++ b/drivers/input/keyboard/gpio_keys.c

@@ -473,6 +473,8 @@ static int gpio_keys_setup_key(struct platform_device *pdev,
 
 		isr = gpio_keys_gpio_isr;
 		irqflags = IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING;
+		if (bdata->button->wakeup)
+			irqflags |= IRQF_NO_SUSPEND;
 
 	} else {
 		if (!button->irq) {

-- 
1.7.9.5

Hi Anson,

On Mon, Dec 23, 2013 at 01:21:20PM -0500, Anson Huang wrote:

For "freeze" mode of suspend, cpu will go into idle and
those wakeup sources' irq should NOT be disabled during
devices suspend, so we need to add IRQF_NO_SUSPEND flag
for those wakeup sources.

Steps to test this patch:

1. echo freeze > /sys/power/state;
2. press gpio key which has wakeup function, then system
   will resume.

I do no think this is correct approach, otheriwse every driver that can
be a wakeup source would have to use IRQF_NO_SUSPEND flag. The driver
does use enable_irq_wake() in its suspend path, and platform code should
perform all necessary work for this IRQ to be usable as a wakeup source.

Thanks.

-- 
Dmitry

Hi, Dmitry

	See below:

Best Regards.
Anson huang 黄勇才
 
Freescale Semiconductor Shanghai
上海浦东新区亮景路192号A座2楼
201203
Tel:021-28937058

-----Original Message-----
From: Dmitry Torokhov [mailto:dmitry.torokhov@gmail.com]
Sent: Tuesday, December 24, 2013 8:31 AM
To: Huang Yongcai-B20788
Cc: sachin.kamat@linaro.org; linux-input@vger.kernel.org
Subject: Re: [PATCH] Input: gpio_keys: support wakeup system from freeze mode

Hi Anson,

On Mon, Dec 23, 2013 at 01:21:20PM -0500, Anson Huang wrote:

quoted

For "freeze" mode of suspend, cpu will go into idle and those wakeup
sources' irq should NOT be disabled during devices suspend, so we need
to add IRQF_NO_SUSPEND flag for those wakeup sources.

Steps to test this patch:

1. echo freeze > /sys/power/state;
2. press gpio key which has wakeup function, then system
   will resume.

I do no think this is correct approach, otheriwse every driver that can be a
wakeup source would have to use IRQF_NO_SUSPEND flag. The driver does use
enable_irq_wake() in its suspend path, and platform code should perform all
necessary work for this IRQ to be usable as a wakeup source.

Thanks.

From the suspend flow, we can see that kernel will disable all devices' irq unless
its IRQF_NO_SUSPEND flag is set. For freeze mode, as cpu will be in idle, SOC is
not in low power mode, that means freeze mode equals cpu in idle and devices
in suspend. The wakeup source must come from GIC. Yes, for normal standby/mem mode's
suspend, the enable_irq_wake will make everything done for waking up a system, in
gic_set_wake routine, it will call extern irq chip's wakeup function, but for freeze
mode, the wakeup process is same as normal wakeup from cpu idle, so the wakeup source
must be not masked in gic. 

	Take our i.MX6 SOC for example, there is GPC module which
is used to wake up SOC from STOP mode(low power mode of SOC), normal standby/mem mode
suspend, the wakeup source's enable_irq_wake will set GPC to monitor these irq source and
wakeup system from STOP mode when there is an wakeup event. But for freeze mode, kernel's
suspend flow will not finish the SOC's low level power management, the secondary CPUs are
even not disabled, kernel just suspend the devices and put CPU into idle and waiting for
wakeup even, in GIC's gic_set_wake routine, it only calls our GPC's imx_gpc_irq_set_wake,
here comes the problem, as our SOC is not entering STOP mode, GPC has no use to set this
wakeup source enabled. So either we should enable GIC's wakeup in GIC driver, or just set
this flag to make kernel do NOT disable this wakeup source's irq when executing a freeze
mode suspend.

	Otherwise, we need to modify the GIC driver?

quoted

Signed-off-by: Anson Huang <redacted>
---
 drivers/input/keyboard/gpio_keys.c |    2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/input/keyboard/gpio_keys.c

b/drivers/input/keyboard/gpio_keys.c
index 2db1324..aadb1db 100644

--- a/drivers/input/keyboard/gpio_keys.c
+++ b/drivers/input/keyboard/gpio_keys.c

@@ -473,6 +473,8 @@ static int gpio_keys_setup_key(struct

platform_device *pdev,

 		isr = gpio_keys_gpio_isr;
 		irqflags = IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING;
+		if (bdata->button->wakeup)
+			irqflags |= IRQF_NO_SUSPEND;

 	} else {
 		if (!button->irq) {
--
1.7.9.5

--
Dmitry

Hi Anson,

On Tue, Dec 24, 2013 at 02:29:54AM +0000, Anson.Huang@freescale.com
wrote:

Hi, Dmitry

	See below:

Best Regards.  Anson huang 黄勇才   Freescale Semiconductor Shanghai
上海浦东新区亮景路192号A座2楼 201203 Tel:021-28937058

quoted

-----Original Message----- From: Dmitry Torokhov
[mailto:dmitry.torokhov@gmail.com] Sent: Tuesday, December 24, 2013
8:31 AM To: Huang Yongcai-B20788 Cc: sachin.kamat@linaro.org;
linux-input@vger.kernel.org Subject: Re: [PATCH] Input: gpio_keys:
support wakeup system from freeze mode

Hi Anson,

On Mon, Dec 23, 2013 at 01:21:20PM -0500, Anson Huang wrote:

quoted

For "freeze" mode of suspend, cpu will go into idle and those
wakeup sources' irq should NOT be disabled during devices suspend,
so we need to add IRQF_NO_SUSPEND flag for those wakeup sources.

Steps to test this patch:

1. echo freeze > /sys/power/state; 2. press gpio key which has
wakeup function, then system will resume.

I do no think this is correct approach, otheriwse every driver that
can be a wakeup source would have to use IRQF_NO_SUSPEND flag. The
driver does use enable_irq_wake() in its suspend path, and platform
code should perform all necessary work for this IRQ to be usable as a
wakeup source.

Thanks.

From the suspend flow, we can see that kernel will disable all
devices' irq unless its IRQF_NO_SUSPEND flag is set. For freeze mode,
as cpu will be in idle, SOC is not in low power mode, that means
freeze mode equals cpu in idle and devices in suspend. The wakeup
source must come from GIC. Yes, for normal standby/mem mode's suspend,
the enable_irq_wake will make everything done for waking up a system,
in gic_set_wake routine, it will call extern irq chip's wakeup
function, but for freeze mode, the wakeup process is same as normal
wakeup from cpu idle, so the wakeup source must be not masked in gic. 

Take our i.MX6 SOC for example, there is GPC module which is used to
wake up SOC from STOP mode(low power mode of SOC), normal standby/mem
mode suspend, the wakeup source's enable_irq_wake will set GPC to
monitor these irq source and wakeup system from STOP mode when there
is an wakeup event. But for freeze mode, kernel's suspend flow will
not finish the SOC's low level power management, the secondary CPUs
are even not disabled, kernel just suspend the devices and put CPU
into idle and waiting for wakeup even, in GIC's gic_set_wake routine,
it only calls our GPC's imx_gpc_irq_set_wake, here comes the problem,
as our SOC is not entering STOP mode, GPC has no use to set this
wakeup source enabled. So either we should enable GIC's wakeup in GIC
driver, or just set this flag to make kernel do NOT disable this
wakeup source's irq when executing a freeze mode suspend.

Otherwise, we need to modify the GIC driver?

Yes, I believe it is the task of platform code to make sure that
enable_irq_wake() enables irq wakeup properly, be it suspend or
hibernate.

Thanks.

-- 
Dmitry
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html