2024-03-01 09:28:21

by Qingliang Li

[permalink] [raw]
Subject: [PATCH v2] PM: sleep: wakeirq: fix wake irq warning in system suspend

When driver uses pm_runtime_force_suspend() as the system suspend callback
function and registers the wake irq with reverse enable ordering, the wake
irq will be re-enabled when entering system suspend, triggering an
'Unbalanced enable for IRQ xxx' warning. In this scenario, the call
sequence during system suspend is as follows:
suspend_devices_and_enter()
-> dpm_suspend_start()
-> dpm_run_callback()
-> pm_runtime_force_suspend()
-> dev_pm_enable_wake_irq_check()
-> dev_pm_enable_wake_irq_complete()

-> suspend_enter()
-> dpm_suspend_noirq()
-> device_wakeup_arm_wake_irqs()
-> dev_pm_arm_wake_irq()

To fix this issue, complete the setting of WAKE_IRQ_DEDICATED_ENABLED flag
in dev_pm_enable_wake_irq_complete() to avoid redundant irq enablement.

Reviewed-by: Dhruva Gole <[email protected]>
Signed-off-by: Qingliang Li <[email protected]>
---
drivers/base/power/wakeirq.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/base/power/wakeirq.c b/drivers/base/power/wakeirq.c
index 42171f766dcb..5a5a9e978e85 100644
--- a/drivers/base/power/wakeirq.c
+++ b/drivers/base/power/wakeirq.c
@@ -313,8 +313,10 @@ void dev_pm_enable_wake_irq_complete(struct device *dev)
return;

if (wirq->status & WAKE_IRQ_DEDICATED_MANAGED &&
- wirq->status & WAKE_IRQ_DEDICATED_REVERSE)
+ wirq->status & WAKE_IRQ_DEDICATED_REVERSE) {
enable_irq(wirq->irq);
+ wirq->status |= WAKE_IRQ_DEDICATED_ENABLED;
+ }
}

/**
--
2.25.1



2024-03-04 15:21:57

by Johan Hovold

[permalink] [raw]
Subject: Re: [PATCH v2] PM: sleep: wakeirq: fix wake irq warning in system suspend

On Fri, Mar 01, 2024 at 05:26:57PM +0800, Qingliang Li wrote:
> When driver uses pm_runtime_force_suspend() as the system suspend callback
> function and registers the wake irq with reverse enable ordering, the wake
> irq will be re-enabled when entering system suspend, triggering an
> 'Unbalanced enable for IRQ xxx' warning. In this scenario, the call
> sequence during system suspend is as follows:
> suspend_devices_and_enter()
> -> dpm_suspend_start()
> -> dpm_run_callback()
> -> pm_runtime_force_suspend()
> -> dev_pm_enable_wake_irq_check()
> -> dev_pm_enable_wake_irq_complete()
>
> -> suspend_enter()
> -> dpm_suspend_noirq()
> -> device_wakeup_arm_wake_irqs()
> -> dev_pm_arm_wake_irq()
>
> To fix this issue, complete the setting of WAKE_IRQ_DEDICATED_ENABLED flag
> in dev_pm_enable_wake_irq_complete() to avoid redundant irq enablement.
>
> Reviewed-by: Dhruva Gole <[email protected]>
> Signed-off-by: Qingliang Li <[email protected]>

Thanks for the fix. Looks correct to me:

Reviewed-by: Johan Hovold <[email protected]>

I think you should add back the Fixes tag from v1 and CC stable as well:

Fixes: 8527beb12087 ("PM: sleep: wakeirq: fix wake irq arming")
Cc: [email protected] # 5.16

Note that WAKE_IRQ_DEDICATED_REVERSE was added in 5.16 by commit
259714100d98 ("PM / wakeirq: support enabling wake-up irq after
runtime_suspend called") so no need to try to backport any further than
that.

Johan

2024-03-05 11:42:31

by Rafael J. Wysocki

[permalink] [raw]
Subject: Re: [PATCH v2] PM: sleep: wakeirq: fix wake irq warning in system suspend

On Mon, Mar 4, 2024 at 4:19 PM Johan Hovold <[email protected]> wrote:
>
> On Fri, Mar 01, 2024 at 05:26:57PM +0800, Qingliang Li wrote:
> > When driver uses pm_runtime_force_suspend() as the system suspend callback
> > function and registers the wake irq with reverse enable ordering, the wake
> > irq will be re-enabled when entering system suspend, triggering an
> > 'Unbalanced enable for IRQ xxx' warning. In this scenario, the call
> > sequence during system suspend is as follows:
> > suspend_devices_and_enter()
> > -> dpm_suspend_start()
> > -> dpm_run_callback()
> > -> pm_runtime_force_suspend()
> > -> dev_pm_enable_wake_irq_check()
> > -> dev_pm_enable_wake_irq_complete()
> >
> > -> suspend_enter()
> > -> dpm_suspend_noirq()
> > -> device_wakeup_arm_wake_irqs()
> > -> dev_pm_arm_wake_irq()
> >
> > To fix this issue, complete the setting of WAKE_IRQ_DEDICATED_ENABLED flag
> > in dev_pm_enable_wake_irq_complete() to avoid redundant irq enablement.
> >
> > Reviewed-by: Dhruva Gole <[email protected]>
> > Signed-off-by: Qingliang Li <[email protected]>
>
> Thanks for the fix. Looks correct to me:
>
> Reviewed-by: Johan Hovold <[email protected]>
>
> I think you should add back the Fixes tag from v1 and CC stable as well:
>
> Fixes: 8527beb12087 ("PM: sleep: wakeirq: fix wake irq arming")
> Cc: [email protected] # 5.16
>
> Note that WAKE_IRQ_DEDICATED_REVERSE was added in 5.16 by commit
> 259714100d98 ("PM / wakeirq: support enabling wake-up irq after
> runtime_suspend called") so no need to try to backport any further than
> that.

Applied as 6.9 material, tags added as suggested above.

Thanks!