[PATCH 0/2] Two wakeirq fixes

[PATCH 0/2] Two wakeirq fixes

[Tony Lindgren]

Hi all,

Here are two wakeirq fixes from Grygorii that have been pending for
a while.

Earlier before commit bed570307ed7 ("PM / wakeirq: Fix dedicated
wakeirq for drivers not using autosuspend") I did not see changes
with my test cases with these two changes, but now I have test cases
that get fixed with these two patches.

Regards,

Tony

Grygorii Strashko (2):
  PM / wakeirq: Enable dedicated wakeirq for suspend
  PM / wakeirq: Fix spurious wake-up events for dedicated wakeirqs

 drivers/base/power/wakeirq.c | 15 +++++++++++++--
 1 file changed, 13 insertions(+), 2 deletions(-)

-- 
2.11.1

[PATCH 1/2] PM / wakeirq: Enable dedicated wakeirq for suspend

[Tony Lindgren]

From: Grygorii Strashko <grygorii.strashko@ti.com>

We currently rely on runtime PM to enable dedicated wakeirq for suspend.
This assumption fails in the following two cases:

1. If the consumer driver does not have runtime PM implemented, the
   dedicated wakeirq never gets enabled for suspend

2. If the consumer driver has runtime PM implemented, but does not idle
   in suspend

Let's fix the issue by always enabling the dedicated wakeirq during
suspend.

Depends-on: bed570307ed7 ("PM / wakeirq: Fix dedicated wakeirq for
drivers not using autosuspend")
Fixes: 4990d4fe327b ("PM / Wakeirq: Add automated device wake IRQ
handling")
Cc: Brian Norris <briannorris@chromium.org>
Cc: Tero Kristo <t-kristo@ti.com>
Reported-by: Keerthy <j-keerthy@ti.com>
Tested-by: Keerthy <j-keerthy@ti.com>
Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
[tony@atomide.com: updated based on bed570307ed7, added description]
Tested-by: Tony Lindgren <tony@atomide.com>
Signed-off-by: Tony Lindgren <tony@atomide.com>
---
 drivers/base/power/wakeirq.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/drivers/base/power/wakeirq.c b/drivers/base/power/wakeirq.c
--- a/drivers/base/power/wakeirq.c
+++ b/drivers/base/power/wakeirq.c
@@ -312,8 +312,12 @@ void dev_pm_arm_wake_irq(struct wake_irq *wirq)
 	if (!wirq)
 		return;
 
-	if (device_may_wakeup(wirq->dev))
+	if (device_may_wakeup(wirq->dev)) {
+		if (wirq->status & WAKE_IRQ_DEDICATED_ALLOCATED)
+			enable_irq(wirq->irq);
+
 		enable_irq_wake(wirq->irq);
+	}
 }
 
 /**
@@ -328,6 +332,10 @@ void dev_pm_disarm_wake_irq(struct wake_irq *wirq)
 	if (!wirq)
 		return;
 
-	if (device_may_wakeup(wirq->dev))
+	if (device_may_wakeup(wirq->dev)) {
 		disable_irq_wake(wirq->irq);
+
+		if (wirq->status & WAKE_IRQ_DEDICATED_ALLOCATED)
+			disable_irq_nosync(wirq->irq);
+	}
 }
-- 
2.11.1

[PATCH 2/2] PM / wakeirq: Fix spurious wake-up events for dedicated wakeirqs

[Tony Lindgren]

From: Grygorii Strashko <grygorii.strashko@ti.com>

Dedicated wakeirq is a one time event to wake-up the system from
low-power state and then call pm_runtime_resume() on the device wired
with the dedicated wakeirq.

Sometimes dedicated wakeirqs can get deferred if they trigger after we
call disable_irq_nosync() in dev_pm_disable_wake_irq(). This can happen
if pm_runtime_get() is called around the same time a wakeirq fires.

If an interrupt fires after disable_irq_nosync(), by default it will get
tagged with IRQS_PENDING and will run later on when the interrupt is
enabled again.

Deferred wakeirqs usually just produce pointless wake-up events. But they
can also cause suspend to fail if the deferred wakeirq fires during
dpm_suspend_noirq() for example. So we really don't want to see the
deferred wakeirqs triggering after the device has resumed.

Let's fix the issue by setting IRQ_DISABLE_UNLAZY flag for the dedicated
wakeirqs. The other option would be to implement irq_disable() in the
dedicated wakeirq controller, but that's not a generic solution.

For reference below is what happens with a IRQ_TYPE_EDGE_BOTH IRQ
type wakeirq:

- resume by dedicated IRQ (EDGE_FALLING)
 - suspend_enter()
  ....
 - arch_suspend_enable_irqs()
   |- dedicated IRQ armed and fired
   |- irq_pm_check_wakeup()
      |- disarm, disable IRQ and mark as IRQS_PENDING
  ....
 - dpm_resume_noirq()
   |- resume_device_irqs()
      |- __enable_irq()
         |- check_irq_resend()
            |- handle_threaded_wake_irq()
 	       |- dedicated IRQ processed
   |- device_wakeup_disarm_wake_irqs()
      |- disable_irq_wake()
  ....
 !-> dedicated IRQ (EDGE_RISING)
     -| handle_edge_irq()
        |- IRQ disabled: mask_ack_irq and mark as IRQS_PENDING
  ....
- subsequent suspend
  ....
  |- dpm_suspend_noirq()
     |- device_wakeup_arm_wake_irqs()
        |- __enable_irq()
           |- check_irq_resend()
(a)           |- handle_threaded_wake_irq()
                 |- pm_wakeup_event() --> abort suspend
  ....
     |- suspend_device_irqs()
        |- suspend_device_irq()
           |-  dedicated IRQ armed
  ....
(b)  |- resend_irqs
        |- irq_pm_check_wakeup()
           |- IRQ armed -> abort suspend

because of pending IRQ System suspend can be aborted at points
(a)-not armed or (b)-armed.

Fixes: 4990d4fe327b ("PM / Wakeirq: Add automated device wake IRQ
handling")
Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
[tony@atomide.com: added a comment, updated the description]
Tested-by: Tony Lindgren <tony@atomide.com>
Signed-off-by: Tony Lindgren <tony@atomide.com>
---
 drivers/base/power/wakeirq.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/base/power/wakeirq.c b/drivers/base/power/wakeirq.c
--- a/drivers/base/power/wakeirq.c
+++ b/drivers/base/power/wakeirq.c
@@ -183,6 +183,9 @@ int dev_pm_set_dedicated_wake_irq(struct device *dev, int irq)
 	wirq->irq = irq;
 	irq_set_status_flags(irq, IRQ_NOAUTOEN);
 
+	/* Prevent deferred spurious wakeirqs with disable_irq_nosync() */
+	irq_set_status_flags(irq, IRQ_DISABLE_UNLAZY);
+
 	/*
 	 * Consumer device may need to power up and restore state
 	 * so we use a threaded irq.
-- 
2.11.1

Re: [PATCH 0/2] Two wakeirq fixes

[Rafael J. Wysocki]

On Fri, Feb 10, 2017 at 11:24 PM, Tony Lindgren <tony@atomide.com> wrote:
> Hi all,
>
> Here are two wakeirq fixes from Grygorii that have been pending for
> a while.
>
> Earlier before commit bed570307ed7 ("PM / wakeirq: Fix dedicated
> wakeirq for drivers not using autosuspend") I did not see changes
> with my test cases with these two changes, but now I have test cases
> that get fixed with these two patches.

OK, I'll queue them up.

If a new merge window starts on Sunday, however, they will not make it
to my first pull request.  I'll push them in the second half of the
merge window in that case.

Thanks,
Rafael

Re: [PATCH 0/2] Two wakeirq fixes

[Tony Lindgren]

* Rafael J. Wysocki <rafael@kernel.org> [170210 14:35]:
> On Fri, Feb 10, 2017 at 11:24 PM, Tony Lindgren <tony@atomide.com> wrote:
> > Hi all,
> >
> > Here are two wakeirq fixes from Grygorii that have been pending for
> > a while.
> >
> > Earlier before commit bed570307ed7 ("PM / wakeirq: Fix dedicated
> > wakeirq for drivers not using autosuspend") I did not see changes
> > with my test cases with these two changes, but now I have test cases
> > that get fixed with these two patches.
> 
> OK, I'll queue them up.
> 
> If a new merge window starts on Sunday, however, they will not make it
> to my first pull request.  I'll push them in the second half of the
> merge window in that case.

OK thanks that's fine.

Regards,

Tony