linux-kernel - Re: [PATCH] Input: gpio_keys: Make sure wake-up buttons work.

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20141108110058.6c5217e3@notabene.brown>
Date:	Sat, 8 Nov 2014 11:00:58 +1100
From:	NeilBrown <neilb@...e.de>
To:	Dmitry Torokhov <dmitry.torokhov@...il.com>
Cc:	linux-input@...r.kernel.org, linux-kernel@...r.kernel.org,
	GTA04 owners <gta04-owner@...delico.com>,
	"Rafael J. Wysocki" <rjw@...ysocki.net>
Subject: Re: [PATCH] Input: gpio_keys: Make sure wake-up buttons work.

On Fri, 7 Nov 2014 15:45:07 -0800 Dmitry Torokhov <dmitry.torokhov@...il.com>
wrote:

> Hi Neil,
> 
> On Sat, Nov 08, 2014 at 10:37:07AM +1100, NeilBrown wrote:
> > 
> > 
> > If a key is to be used for wake-up, we must not disable
> > the interrupt during suspend.
> > 
> > Signed-off-by: NeilBrown <neilb@...e.de>
> > 
> > diff --git a/drivers/input/keyboard/gpio_keys.c b/drivers/input/keyboard/gpio_keys.c
> > index 8c98e97f8e41..0b5e54ae343e 100644
> > --- a/drivers/input/keyboard/gpio_keys.c
> > +++ b/drivers/input/keyboard/gpio_keys.c
> > @@ -526,6 +526,8 @@ static int gpio_keys_setup_key(struct platform_device *pdev,
> >  	 */
> >  	if (!button->can_disable)
> >  		irqflags |= IRQF_SHARED;
> > +	if (button->wakeup)
> > +		irqflags |= IRQF_NO_SUSPEND;
> 
> No, enable_irq_wake() should be enough. I believe Rafael has fixed that
> in the core, right?
> 

Interesting...  you seem to be right, but I was having wakeup problems until
I added that patch.
I didn't test exhaustively, but the first time my device entered suspend, the
gpio-key didn't wake it up.  Subsequent suspends did...

After I applied this patch, it would reliably wake up even on the first
suspend.

So there seems to be something wrong, but maybe it is more subtle.

Is there a good reason why enable_irq_wake() is only called just as the
device is being suspended, and why disable_irq_wake() is called on resume?
To me it would make more sense to just enable it once (if required) and leave
it enabled....

I'll see what I can find.

Thanks,
NeilBrown

Content of type "application/pgp-signature" skipped