Re: [PATCH] PM: wakup: export wakeup irq when system aborts suspend

[Zhang Rui <rui.zhang@intel.com>]

On 三, 2019-04-24 at 10:32 +0200, Rafael J. Wysocki wrote:
> On Wed, Apr 24, 2019 at 9:46 AM Zhang Rui <rui.zhang@intel.com>
> wrote:
> > 
> > 
> > On 三, 2019-04-24 at 09:29 +0200, Rafael J. Wysocki wrote:
> > > 
> > > On Wed, Apr 24, 2019 at 8:59 AM Zhang Rui <rui.zhang@intel.com>
> > > wrote:
> > > > 
> > > > 
> > > > 
> > > > When suspend is aborted because of an ARMED irq, we can not get
> > > > any
> > > > useful information about which irq is triggered.
> > > That should be provided via the /sys/power/pm_wakeup_irq
> > > attribute.
> > > 
> > another case is that,
> > say we got spurious wakeup interrupts in the idle loop, and then
> > waked
> > by a real wakeup interrupt, say, rtc.
> > /sys/power/pm_wakeup_irq only shows the latest one,
> No it doesn't, it shows the *first* one.
> 
> Please look at the code: pm_wakeup_irq is only set when it is zero
> which is unless it has been set before.
> 

hmmm, by reading the code, pm_wakeup_clear() clears pm_wakeup_irq, and it is invoked inside the s2idle loop. So I think pm_wakeup_irq is cleared and set again if there is a second spurious interrupt.

> > and we can not find out the spurious wakeup interrupts.
> The wakeup interrupts that have occurred after the first one are not
> recorded, but your patch doesn't change that.
> 
> Your patch causes pm_wakeup_irq to be added to the trace only when it
> is set, but that is the /sys/power/pm_wakeup_irq which is my point.

yes, the first case (suspend aborted) can be covered
by /sys/power/pm_wakeup_irq.
For the spurious wakeup case, I think I observed the behavior before,
e.g, pm_wakeup_irq keeps on showing 9 because of spurious GPE, and then
 returns 8 because of RTC wakeup event. Anyway, let me do some
experimental to confirm and get back to you then.

thanks,
rui