* [PATCH] PM: wakup: export wakeup irq when system aborts suspend @ 2019-04-24 6:59 Zhang Rui 2019-04-24 7:29 ` Rafael J. Wysocki 0 siblings, 1 reply; 6+ messages in thread From: Zhang Rui @ 2019-04-24 6:59 UTC (permalink / raw) To: rjw; +Cc: linux-pm When suspend is aborted because of an ARMED irq, we can not get any useful information about which irq is triggered. Export the wakeup irq via suspend_resume trace event in this case. Signed-off-by: Brandt, Todd E <todd.e.brandt@intel.com> Signed-off-by: Zhang Rui <rui.zhang@intel.com> --- drivers/base/power/wakeup.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/base/power/wakeup.c b/drivers/base/power/wakeup.c index bb1ae17..d32bd91 100644 --- a/drivers/base/power/wakeup.c +++ b/drivers/base/power/wakeup.c @@ -875,6 +875,7 @@ void pm_system_irq_wakeup(unsigned int irq_number) { if (pm_wakeup_irq == 0) { pm_wakeup_irq = irq_number; + trace_suspend_resume(TPS("irq_wakeup"), irq_number, true); pm_system_wakeup(); } } -- 2.7.4 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] PM: wakup: export wakeup irq when system aborts suspend 2019-04-24 6:59 [PATCH] PM: wakup: export wakeup irq when system aborts suspend Zhang Rui @ 2019-04-24 7:29 ` Rafael J. Wysocki 2019-04-24 7:46 ` Zhang Rui 0 siblings, 1 reply; 6+ messages in thread From: Rafael J. Wysocki @ 2019-04-24 7:29 UTC (permalink / raw) To: Zhang Rui; +Cc: Rafael J. Wysocki, Linux PM 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. > Export the wakeup irq via suspend_resume trace event in this case. > > Signed-off-by: Brandt, Todd E <todd.e.brandt@intel.com> > Signed-off-by: Zhang Rui <rui.zhang@intel.com> > --- > drivers/base/power/wakeup.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/base/power/wakeup.c b/drivers/base/power/wakeup.c > index bb1ae17..d32bd91 100644 > --- a/drivers/base/power/wakeup.c > +++ b/drivers/base/power/wakeup.c > @@ -875,6 +875,7 @@ void pm_system_irq_wakeup(unsigned int irq_number) > { > if (pm_wakeup_irq == 0) { > pm_wakeup_irq = irq_number; > + trace_suspend_resume(TPS("irq_wakeup"), irq_number, true); pm_wakeup_irq can be read from /sys/power/pm_wakeup_irq, so why is it necessary to trace it additionally? > pm_system_wakeup(); > } > } > -- ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] PM: wakup: export wakeup irq when system aborts suspend 2019-04-24 7:29 ` Rafael J. Wysocki @ 2019-04-24 7:46 ` Zhang Rui 2019-04-24 8:32 ` Rafael J. Wysocki 0 siblings, 1 reply; 6+ messages in thread From: Zhang Rui @ 2019-04-24 7:46 UTC (permalink / raw) To: Rafael J. Wysocki; +Cc: Rafael J. Wysocki, Linux PM 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, and we can not find out the spurious wakeup interrupts. I should add this in the changelog as well. thanks, rui > > > > Export the wakeup irq via suspend_resume trace event in this case. > > > > Signed-off-by: Brandt, Todd E <todd.e.brandt@intel.com> > > Signed-off-by: Zhang Rui <rui.zhang@intel.com> > > --- > > drivers/base/power/wakeup.c | 1 + > > 1 file changed, 1 insertion(+) > > > > diff --git a/drivers/base/power/wakeup.c > > b/drivers/base/power/wakeup.c > > index bb1ae17..d32bd91 100644 > > --- a/drivers/base/power/wakeup.c > > +++ b/drivers/base/power/wakeup.c > > @@ -875,6 +875,7 @@ void pm_system_irq_wakeup(unsigned int > > irq_number) > > { > > if (pm_wakeup_irq == 0) { > > pm_wakeup_irq = irq_number; > > + trace_suspend_resume(TPS("irq_wakeup"), irq_number, > > true); > pm_wakeup_irq can be read from /sys/power/pm_wakeup_irq, so why is it > necessary to trace it additionally? > > > > > pm_system_wakeup(); > > } > > } > > -- ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] PM: wakup: export wakeup irq when system aborts suspend 2019-04-24 7:46 ` Zhang Rui @ 2019-04-24 8:32 ` Rafael J. Wysocki 2019-04-24 8:57 ` Zhang Rui 0 siblings, 1 reply; 6+ messages in thread From: Rafael J. Wysocki @ 2019-04-24 8:32 UTC (permalink / raw) To: Zhang Rui; +Cc: Rafael J. Wysocki, Rafael J. Wysocki, Linux PM 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. > 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. ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] PM: wakup: export wakeup irq when system aborts suspend 2019-04-24 8:32 ` Rafael J. Wysocki @ 2019-04-24 8:57 ` Zhang Rui 2019-04-25 10:12 ` Rafael J. Wysocki 0 siblings, 1 reply; 6+ messages in thread From: Zhang Rui @ 2019-04-24 8:57 UTC (permalink / raw) To: Rafael J. Wysocki; +Cc: Rafael J. Wysocki, Linux PM 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 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] PM: wakup: export wakeup irq when system aborts suspend 2019-04-24 8:57 ` Zhang Rui @ 2019-04-25 10:12 ` Rafael J. Wysocki 0 siblings, 0 replies; 6+ messages in thread From: Rafael J. Wysocki @ 2019-04-25 10:12 UTC (permalink / raw) To: Zhang Rui; +Cc: Rafael J. Wysocki, Rafael J. Wysocki, Linux PM On Wed, Apr 24, 2019 at 10:58 AM Zhang Rui <rui.zhang@intel.com> wrote: > > 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. If that is a concern, I would suggest adding a debug printout to pm_wakeup_clear() to print the current value of pm_wakeup_irq before clearing it (if not zero already). ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2019-04-25 10:12 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2019-04-24 6:59 [PATCH] PM: wakup: export wakeup irq when system aborts suspend Zhang Rui 2019-04-24 7:29 ` Rafael J. Wysocki 2019-04-24 7:46 ` Zhang Rui 2019-04-24 8:32 ` Rafael J. Wysocki 2019-04-24 8:57 ` Zhang Rui 2019-04-25 10:12 ` Rafael J. Wysocki
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).