linux-kernel - RE: [RESEND] input: keyboard: imx: make sure keyboard can always wake up system

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 for Android: free password hash cracker in your pocket

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

Message-ID: <DB3PR0402MB39169646E73C1A78958EA325F5130@DB3PR0402MB3916.eurprd04.prod.outlook.com>
Date:   Mon, 10 Jun 2019 06:44:00 +0000
From:   Anson Huang <anson.huang@....com>
To:     "dmitry.torokhov@...il.com" <dmitry.torokhov@...il.com>
CC:     "shawnguo@...nel.org" <shawnguo@...nel.org>,
        "s.hauer@...gutronix.de" <s.hauer@...gutronix.de>,
        "kernel@...gutronix.de" <kernel@...gutronix.de>,
        "festevam@...il.com" <festevam@...il.com>,
        "linux-input@...r.kernel.org" <linux-input@...r.kernel.org>,
        "linux-arm-kernel@...ts.infradead.org" 
        <linux-arm-kernel@...ts.infradead.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        dl-linux-imx <linux-imx@....com>
Subject: RE: [RESEND] input: keyboard: imx: make sure keyboard can always wake
 up system

Hi, Dmitry
	Any feedback for this patch?

Thanks,
Anson

> -----Original Message-----
> From: Anson Huang
> Sent: Tuesday, May 21, 2019 2:36 PM
> To: dmitry.torokhov@...il.com
> Cc: shawnguo@...nel.org; s.hauer@...gutronix.de;
> kernel@...gutronix.de; festevam@...il.com; linux-input@...r.kernel.org;
> linux-arm-kernel@...ts.infradead.org; linux-kernel@...r.kernel.org; dl-linux-
> imx <linux-imx@....com>
> Subject: RE: [RESEND] input: keyboard: imx: make sure keyboard can always
> wake up system
> 
> Hi, Dmitry
> 
> > -----Original Message-----
> > From: dmitry.torokhov@...il.com [mailto:dmitry.torokhov@...il.com]
> > Sent: Tuesday, May 21, 2019 1:31 PM
> > To: Anson Huang <anson.huang@....com>
> > Cc: shawnguo@...nel.org; s.hauer@...gutronix.de;
> > kernel@...gutronix.de; festevam@...il.com;
> > linux-input@...r.kernel.org; linux-arm-kernel@...ts.infradead.org;
> > linux-kernel@...r.kernel.org; dl-linux- imx <linux-imx@....com>
> > Subject: Re: [RESEND] input: keyboard: imx: make sure keyboard can
> > always wake up system
> >
> > Hi Anson,
> > On Thu, Apr 04, 2019 at 01:40:16AM +0000, Anson Huang wrote:
> > > There are several scenarios that keyboard can NOT wake up system
> > > from suspend, e.g., if a keyboard is depressed between system device
> > > suspend phase and device noirq suspend phase, the keyboard ISR will
> > > be called and both keyboard depress and release interrupts will be
> > > disabled, then keyboard will no longer be able to wake up system.
> > > Another scenario would be, if a keyboard is kept depressed, and then
> > > system goes into suspend, the expected behavior would be when
> > > keyboard is released, system will be waked up, but current
> > > implementation can NOT achieve that, because both depress and
> > > release interrupts are disabled in ISR, and the event check is still in
> progress.
> > >
> > > To fix these issues, need to make sure keyboard's depress or release
> > > interrupt is enabled after noirq device suspend phase, this patch
> > > moves the suspend/resume callback to noirq suspend/resume phase,
> and
> > > enable the corresponding interrupt according to current keyboard status.
> >
> > I believe it is possible for IRQ to be disabled and still  being
> > enabled as wakeup source. What happens if you call disable_irq()
> > before disabling the clock?
> 
> Doing below does NOT fix the scenario/issue 100%, if the keypad's IRQ
> arrived during suspend phase but before disabling its IRQ in its suspend
> callback, then issue is still there, as the issue is that when system suspend,
> keypad's irq arrived during suspend and noirq suspend phase, then keypad's
> hardware interrupt detection will be disabled in the ISR handler, and the
> timer event setup by ISR handler is NOT fired,
> imx_keypad_check_for_events() is NOT executed and hardware keypad's
> depress/release interrupt is NOT re-enabled yet, so it can NOT wake up
> system anymore...
> 
> So I think the solid fix is to make sure keypad can generate IRQ (either
> depress or release) at any time during system suspend flow.
> 
> +++ b/drivers/input/keyboard/imx_keypad.c
> @@ -533,6 +533,8 @@ static int __maybe_unused imx_kbd_suspend(struct
> device *dev)
>         /* imx kbd can wake up system even clock is disabled */
>         mutex_lock(&input_dev->mutex);
> 
> +       disable_irq(kbd->irq);
> +
>         if (input_dev->users)
>                 clk_disable_unprepare(kbd->clk);
> 
> 
> @@ -562,6 +569,8 @@ static int __maybe_unused imx_kbd_resume(struct
> device *dev)
>                         goto err_clk;
>         }
> 
> +       enable_irq(kbd->irq);
> +
>  err_clk:
> 
> Anson.
> 
> >
> > Thanks.
> >
> > --
> > Dmitry