| Message ID | 20240223133930.582041-2-elder@linaro.org |
|---|---|
| State | Accepted |
| Commit | 4b2274d3811a25831068025ce20bf2adbd48c101 |
| Headers | show |
| Series | net: ipa: don't abort system suspend | expand |
On Fri, 2024-02-23 at 07:39 -0600, Alex Elder wrote: > The IPA interrupt can fire if there is data to be delivered to a GSI > channel that is suspended. This condition occurs in three scenarios. > > First, runtime power management automatically suspends the IPA > hardware after half a second of inactivity. This has nothing > to do with system suspend, so a SYSTEM IPA power flag is used to > avoid calling pm_wakeup_dev_event() when runtime suspended. > > Second, if the system is suspended, the receipt of an IPA interrupt > should trigger a system resume. Configuring the IPA interrupt for > wakeup accomplishes this. > > Finally, if system suspend is underway and the IPA interrupt fires, > we currently call pm_wakeup_dev_event() to abort the system suspend. > > The IPA driver correctly handles quiescing the hardware before > suspending it, so there's really no need to abort a suspend in > progress in the third case. We can simply quiesce and suspend > things, and be done. > > Incoming data can still wake the system after it's suspended. > The IPA interrupt has wakeup mode enabled, so if it fires *after* > we've suspended, it will trigger a wakeup (if not disabled via > sysfs). > > Stop calling pm_wakeup_dev_event() to abort a system suspend in > progress in ipa_power_suspend_handler(). > > Signed-off-by: Alex Elder <elder@linaro.org> > --- > Note: checkpatch warns: braces {} are not necessary... > > drivers/net/ipa/ipa_power.c | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > diff --git a/drivers/net/ipa/ipa_power.c b/drivers/net/ipa/ipa_power.c > index 128a816f65237..694bc71e0a170 100644 > --- a/drivers/net/ipa/ipa_power.c > +++ b/drivers/net/ipa/ipa_power.c > @@ -220,8 +220,9 @@ void ipa_power_suspend_handler(struct ipa *ipa, enum ipa_irq_id irq_id) > * system suspend, trigger a system resume. > */ > if (!__test_and_set_bit(IPA_POWER_FLAG_RESUMED, ipa->power->flags)) > - if (test_bit(IPA_POWER_FLAG_SYSTEM, ipa->power->flags)) > - pm_wakeup_dev_event(&ipa->pdev->dev, 0, true); > + if (test_bit(IPA_POWER_FLAG_SYSTEM, ipa->power->flags)) { > + ; > + } FTR, I would have dropped the whole 'if' statement above and the related comment in this patch, saving a few checkpatch warnings. Not a big deal since the the chunk is removed a few patches later. Cheers, Paolo
diff --git a/drivers/net/ipa/ipa_power.c b/drivers/net/ipa/ipa_power.c index 128a816f65237..694bc71e0a170 100644 --- a/drivers/net/ipa/ipa_power.c +++ b/drivers/net/ipa/ipa_power.c @@ -220,8 +220,9 @@ void ipa_power_suspend_handler(struct ipa *ipa, enum ipa_irq_id irq_id) * system suspend, trigger a system resume. */ if (!__test_and_set_bit(IPA_POWER_FLAG_RESUMED, ipa->power->flags)) - if (test_bit(IPA_POWER_FLAG_SYSTEM, ipa->power->flags)) - pm_wakeup_dev_event(&ipa->pdev->dev, 0, true); + if (test_bit(IPA_POWER_FLAG_SYSTEM, ipa->power->flags)) { + ; + } /* Acknowledge/clear the suspend interrupt on all endpoints */ ipa_interrupt_suspend_clear_all(ipa->interrupt);
The IPA interrupt can fire if there is data to be delivered to a GSI channel that is suspended. This condition occurs in three scenarios. First, runtime power management automatically suspends the IPA hardware after half a second of inactivity. This has nothing to do with system suspend, so a SYSTEM IPA power flag is used to avoid calling pm_wakeup_dev_event() when runtime suspended. Second, if the system is suspended, the receipt of an IPA interrupt should trigger a system resume. Configuring the IPA interrupt for wakeup accomplishes this. Finally, if system suspend is underway and the IPA interrupt fires, we currently call pm_wakeup_dev_event() to abort the system suspend. The IPA driver correctly handles quiescing the hardware before suspending it, so there's really no need to abort a suspend in progress in the third case. We can simply quiesce and suspend things, and be done. Incoming data can still wake the system after it's suspended. The IPA interrupt has wakeup mode enabled, so if it fires *after* we've suspended, it will trigger a wakeup (if not disabled via sysfs). Stop calling pm_wakeup_dev_event() to abort a system suspend in progress in ipa_power_suspend_handler(). Signed-off-by: Alex Elder <elder@linaro.org> --- Note: checkpatch warns: braces {} are not necessary... drivers/net/ipa/ipa_power.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)