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

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

[Anson Huang]

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 <b20788@freescale.com>
---
 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

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

[Dmitry Torokhov]

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.

> 
> Signed-off-by: Anson Huang <b20788@freescale.com>
> ---
>  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

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

[Anson.Huang]

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:
>> 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?


>
>>
>> Signed-off-by: Anson Huang <b20788@freescale.com>
>> ---
>>  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
>

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

[Dmitry Torokhov]

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
> 
> 
> >-----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:
> >> 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