RE: [PATCHv2 0/5] Runtime PM support for wlcore

RE: [PATCHv2 0/5] Runtime PM support for wlcore

[Reizer, Eyal]

> 
> >
> > Here's a modified version of your patch, does that put wlcore to
> > idle with wowlan during suspend for you?
> >
> 
> Still no joy.
> It suspends/resumes ok but leaves the firmware disabled from entering ELP.

Spent some time on it today and looks like adding calls to pm_generic_runtime_suspend ()
And pm_generic_runtime_resume is helping and all seems to work well.
See the modified version of your patch below.
Let me know what you think.

Best Regards,
Eyal

8< ------------------------------
diff --git a/drivers/net/wireless/ti/wlcore/main.c
b/drivers/net/wireless/ti/wlcore/main.c
index 4c297aa..9859e5a 100644
--- a/drivers/net/wireless/ti/wlcore/main.c
+++ b/drivers/net/wireless/ti/wlcore/main.c
@@ -1001,24 +1001,6 @@ static int wlcore_fw_wakeup(struct wl1271 *wl)
     return wlcore_raw_write32(wl, HW_ACCESS_ELP_CTRL_REG, ELPCTRL_WAKE_UP);
 }

-static int wlcore_fw_sleep(struct wl1271 *wl)
-{
-    int ret;
-
-    mutex_lock(&wl->mutex);
-    ret = wlcore_raw_write32(wl, HW_ACCESS_ELP_CTRL_REG, ELPCTRL_SLEEP);
-    if (ret < 0) {
-        wl12xx_queue_recovery_work(wl);
-        goto out;
-    }
-    set_bit(WL1271_FLAG_IN_ELP, &wl->flags);
-out:
-    mutex_unlock(&wl->mutex);
-    mdelay(WL1271_SUSPEND_SLEEP);
-
-    return 0;
-}
-
 static int wl1271_setup(struct wl1271 *wl)
 {
     wl->raw_fw_status = kzalloc(wl->fw_status_len, GFP_KERNEL);
@@ -1742,6 +1724,7 @@ static int wl1271_op_suspend(struct ieee80211_hw *hw,
 {
     struct wl1271 *wl = hw->priv;
     struct wl12xx_vif *wlvif;
+    unsigned long flags;
     int ret;

     wl1271_debug(DEBUG_MAC80211, "mac80211 suspend wow=%d", !!wow);
@@ -1800,19 +1783,6 @@ static int wl1271_op_suspend(struct ieee80211_hw *hw,
     /* flush any remaining work */
     wl1271_debug(DEBUG_MAC80211, "flushing remaining works");

-    /*
-     * disable and re-enable interrupts in order to flush
-     * the threaded_irq
-     */
-    wlcore_disable_interrupts(wl);
-
-    /*
-     * set suspended flag to avoid triggering a new threaded_irq
-     * work. no need for spinlock as interrupts are disabled.
-     */
-    set_bit(WL1271_FLAG_SUSPENDED, &wl->flags);
-
-    wlcore_enable_interrupts(wl);
     flush_work(&wl->tx_work);

     /*
@@ -1822,13 +1792,14 @@ static int wl1271_op_suspend(struct ieee80211_hw *hw,
     cancel_delayed_work(&wl->tx_watchdog_work);

     /*
-     * Use an immediate call for allowing the firmware to go into power
-     * save during suspend.
-     * Using a workque for this last write was only hapenning on resume
-     * leaving the firmware with power save disabled during suspend,
-     * while consuming full power during wowlan suspend.
+     * set suspended flag to avoid triggering a new threaded_irq
+     * work.
      */
-    wlcore_fw_sleep(wl);
+    spin_lock_irqsave(&wl->wl_lock, flags);
+    set_bit(WL1271_FLAG_SUSPENDED, &wl->flags);
+    spin_unlock_irqrestore(&wl->wl_lock, flags);
+
+    pm_generic_runtime_suspend(wl->dev);

     return 0;
 }
@@ -1845,6 +1816,8 @@ static int wl1271_op_resume(struct ieee80211_hw *hw)
              wl->wow_enabled);
     WARN_ON(!wl->wow_enabled);

+    pm_generic_runtime_resume(wl->dev);
+
     /*
      * re-enable irq_work enqueuing, and call irq_work directly if
      * there is a pending work.

Re: [PATCHv2 0/5] Runtime PM support for wlcore

[Tony Lindgren]

* Reizer, Eyal <eyalr-l0cyMroinI0@public.gmane.org> [180523 12:45]:
> > 
> > >
> > > Here's a modified version of your patch, does that put wlcore to
> > > idle with wowlan during suspend for you?
> > >
> > 
> > Still no joy.
> > It suspends/resumes ok but leaves the firmware disabled from entering ELP.
> 
> Spent some time on it today and looks like adding calls to pm_generic_runtime_suspend ()
> And pm_generic_runtime_resume is helping and all seems to work well.

Oh right yeah you got it, I totally forgot about those.

> See the modified version of your patch below.
> Let me know what you think.

Looks good to me, one comment below..

> -    wlcore_fw_sleep(wl);
> +    spin_lock_irqsave(&wl->wl_lock, flags);
> +    set_bit(WL1271_FLAG_SUSPENDED, &wl->flags);
> +    spin_unlock_irqrestore(&wl->wl_lock, flags);
> +
> +    pm_generic_runtime_suspend(wl->dev);
> 
>      return 0;

Maybe just return pm_generic_runtime_suspend(wl->dev)
here?

> @@ -1845,6 +1816,8 @@ static int wl1271_op_resume(struct ieee80211_hw *hw)
>               wl->wow_enabled);
>      WARN_ON(!wl->wow_enabled);
> 
> +    pm_generic_runtime_resume(wl->dev);
> +

And add error handling to pm_generic_runtime_resume(wl->dev)
here?

Then if you can please post your patch with proper Signed-off-by
I'll add it to my set and repost v3 of all the patches :)

Regards,

Tony

[PATCHv2 0/5] Runtime PM support for wlcore

[Tony Lindgren]

Hi all,

Here's a series of patches to add runtime PM support for wlcore. It does not
yet implement autosuspend support, but let's get this tested first as the
autosuspend can mask enable/disable issues easily.

Regards,

Tony

Changes since v1:

- Fix issues reported by Eyal for recovery

- Add few patches for enable/disable issues found when using runtime PM

- Remove unused ps.h includes

Tony Lindgren (5):
  wlcore: Add missing PM call for wlcore_cmd_wait_for_event_or_timeout()
  wlcore: Make sure PM calls are paired
  wlcore: Add support for runtime PM
  wlcore: Fix misplaced PM call for scan_complete_work()
  wlcore: sdio: Warn about runtime PM suspend errors

 drivers/net/wireless/ti/wl18xx/debugfs.c    |  26 +-
 drivers/net/wireless/ti/wlcore/acx.c        |   1 -
 drivers/net/wireless/ti/wlcore/cmd.c        |   9 +
 drivers/net/wireless/ti/wlcore/debugfs.c    |  79 ++--
 drivers/net/wireless/ti/wlcore/main.c       | 404 ++++++++++++++------
 drivers/net/wireless/ti/wlcore/ps.c         | 146 -------
 drivers/net/wireless/ti/wlcore/ps.h         |   3 -
 drivers/net/wireless/ti/wlcore/scan.c       |  12 +-
 drivers/net/wireless/ti/wlcore/sdio.c       |   7 +
 drivers/net/wireless/ti/wlcore/sysfs.c      |  12 +-
 drivers/net/wireless/ti/wlcore/testmode.c   |  18 +-
 drivers/net/wireless/ti/wlcore/tx.c         |   9 +-
 drivers/net/wireless/ti/wlcore/vendor_cmd.c |  27 +-
 drivers/net/wireless/ti/wlcore/wlcore.h     |   1 -
 drivers/net/wireless/ti/wlcore/wlcore_i.h   |   1 -
 15 files changed, 421 insertions(+), 334 deletions(-)

-- 
2.17.0

Re: [PATCHv2 0/5] Runtime PM support for wlcore

[Tony Lindgren]

* Tony Lindgren <tony-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org> [180517 18:52]:
> Hi all,
> 
> Here's a series of patches to add runtime PM support for wlcore. It does not
> yet implement autosuspend support, but let's get this tested first as the
> autosuspend can mask enable/disable issues easily.

Sorry forgot to mention that you will want to have patch
"[PATCHv2] wlcore: sdio: Fix flakey SDIO runtime PM handling"
applied when testing to avoid bogus errors.

Regards,

Tony