Re: [patch]GPIO button is supposed to wake the system up if the wakeup attribute is set

[Dmitry Torokhov <dmitry.torokhov@gmail.com>]

Hi Aubrey,

On Fri, Apr 18, 2014 at 12:42:24AM +0800, Li, Aubrey wrote:
> On 2014/4/16 20:35, Laxman Dewangan wrote:
> > On Tuesday 15 April 2014 09:48 PM, Li, Aubrey wrote:
> >> On 2014/4/15 20:38, Laxman Dewangan wrote:
> >>> On Monday 14 April 2014 09:12 PM, Li, Aubrey wrote:
> >>>> ping...
> >>>>
> >>>> On 2014/4/10 18:48, One Thousand Gnomes wrote:
> >>>>>
> >>> I think when we say irq_wake_enable() then based on underlying HW, it
> >>> should not turn off the irq if it is require for the wakeup. I mean it
> >>> need to be handle in the hw specific callbacks to keep enabling the
> >>> wakeup irq on suspend also.
> >> I failed to see why this can't be generic to all of the GPIO buttons for
> >> suspend wakeup. Do you see any cases broken by this proposal?
> > 
> > My point here is that if underlying HW needs to have irq enabled for
> > wakup then it need to handle in centralized location i.e. the driver
> > which is implementing it for the irq callbacks.
> > Otherwise, we need to change this on multiple places who needs wakeups
> > which is vast in nature like sd driver for sdcard insert/remove etc.
> > almost all drivers which need wakeups through GPIOs.
> 
> I think we have to handle this driver by driver. I didn't see how can we
> make it in a centralized location. Looking forward to see your proposal.
> 
> > 
> >>> For me, I have key which is interrupt based from PMIC, not based on GPIO
> >>> and on that if I set it to IRQF_EARLY_RESUME then it works fine.
> >>>
> >> IRQF_NO_SUSPEND - Do not disable this IRQ during suspend
> >> IRQF_EARLY_RESUME - Resume IRQ early during syscore instead of at device
> >> resume time.
> >>
> >> IRQF_NO_SUSPEND is exactly what I want, instead of IRQF_EARLY_RESUME.
> >> Can you please send your proposal/code to help me understand why this
> >> has to hw specific and why IRQF_EARLY_RESUME is better than
> >> IRQF_NO_SUSPEND?
> > 
> > IRQF_EARLY_RESUME helps to re-enable mask or irq before parent interrupt
> > resume and so parent isr handler sees the irq flag enabled when it try
> > to scan source of interrupt. Otherwise parent isr handler treat this as
> > spurious interrupt and does not call handler as irq flag disabled for that.
> > 
> > This only happen when on resume, parent inettrupt enabled before the
> > child interrupt on irq resume. Because as soon as parent isr re-enabled
> > on resume, its hadnler get called before actually child interrupt
> > enabled. This is what I observed mainly on PMIC and its sub irq. Not
> > observed on SoC level of interrupts.
> > 
> 
> This is expected behavior. I think I still need IRQF_NO_SUSPEND here.
> What I want is, this IRQ is able to generate pm wakeup event to wake the
> system up. It's enough for my case.

The driver does call enable_irq_wake() in its suspend routine to prepare
the interrupt in question to be used as a wakeup source. Why isn't it
enough? It seems to me that your platform code should properly handle
this case instead of relying on the driver to modify IRQ flags.

Thanks.
 
-- 
Dmitry