diff mbox

[EXTERNAL,PATCHv2,0/5] Runtime PM support for wlcore

Message ID 38ddca4c73bb4dbf835a84133035dbfb@ti.com (mailing list archive)
State RFC
Delegated to: Kalle Valo
Headers show

Commit Message

Reizer, Eyal May 22, 2018, 1:26 p.m. UTC
> >
> > 8< ------------------------
> > diff --git a/drivers/net/wireless/ti/wlcore/main.c
> > b/drivers/net/wireless/ti/wlcore/main.c
> > --- a/drivers/net/wireless/ti/wlcore/main.c
> > +++ b/drivers/net/wireless/ti/wlcore/main.c
> > @@ -1867,8 +1867,6 @@ static int __maybe_unused
> > wl1271_op_resume(struct ieee80211_hw *hw)
> >  			if (ret)
> >  				wl12xx_queue_recovery_work(wl);
> >  		}
> > -
> > -		wlcore_enable_interrupts(wl);
> >  	}
> >
> >  	if (pending_recovery) {
> > @@ -1877,6 +1875,8 @@ static int __maybe_unused
> > wl1271_op_resume(struct ieee80211_hw *hw)
> >  		goto out_sleep;
> >  	}
> >
> > +	wlcore_enable_interrupts(wl);
> > +
> >  	ret = pm_runtime_get_sync(wl->dev);
> >  	if (ret < 0) {
> >  		pm_runtime_put_noidle(wl->dev);
> 
> It still crash.
> The crash is different now.
> It also complains about:
> [   60.544224] Unbalanced enable for IRQ 65
> Need down/up of the interface to recover after it.
> Log below:
> 

Actually the below patch removing the call to wlcore_fw_sleep() avoids this error.
The downside is that the wl8 firmware remains fully active during supend, so we
Would need to find the root cause why the last call allowing the wilink8 firmware 
To go into ELP mode during suspend is only completing on resume and not during
Suspend enter.

With this wowlan seems to work ok and suspend/resume is not crashing when 
enabling wowlan. See below:

root@am335x-evm:/usr/share/wl18xx#
root@am335x-evm:/usr/share/wl18xx#
root@am335x-evm:/usr/share/wl18xx#
root@am335x-evm:/usr/share/wl18xx# iw phy0 wowlan enable any
root@am335x-evm:/usr/share/wl18xx#
root@am335x-evm:/usr/share/wl18xx# echo mem > /sys/power/state
[   63.794805] PM: suspend entry (deep)
[   63.798455] PM: Syncing filesystems ... done.
[   65.779673] Freezing user space processes ... (elapsed 0.001 seconds) done.
[   65.788878] OOM killer disabled.
[   65.792117] Freezing remaining freezable tasks ... (elapsed 0.001 seconds) done.
[   65.801196] Suspending console(s) (use no_console_suspend to debug)
[   65.952459] pm33xx pm33xx: PM: Successfully put all powerdomains to target state
[   65.952459] PM: Wakeup source GPIO0
[   65.977028] net eth0: initializing cpsw version 1.12 (0)
[   66.074419] SMSC LAN8710/LAN8720 4a101000.mdio:00: attached PHY driver [SMSC LAN8710/LAN8720] (mii_bus:phy_addr=4a101000.mdio:00, irq=POLL)
[   66.236312] OOM killer enabled.
[   66.239604] Restarting tasks ... done.
[   66.282501] PM: suspend exit
root@am335x-evm:/usr/share/wl18xx#

BR,
Eyal

Comments

Tony Lindgren May 22, 2018, 1:31 p.m. UTC | #1
* Reizer, Eyal <eyalr@ti.com> [180522 13:28]:
> Actually the below patch removing the call to wlcore_fw_sleep() avoids this error.
> The downside is that the wl8 firmware remains fully active during supend, so we
> Would need to find the root cause why the last call allowing the wilink8 firmware 
> To go into ELP mode during suspend is only completing on resume and not during
> Suspend enter.
> 
> diff --git a/drivers/net/wireless/ti/wlcore/main.c
> b/drivers/net/wireless/ti/wlcore/main.c
> index 4c297aa..8df1ae6 100644
> --- a/drivers/net/wireless/ti/wlcore/main.c
> +++ b/drivers/net/wireless/ti/wlcore/main.c
> @@ -1789,7 +1789,6 @@ static int wl1271_op_suspend(struct ieee80211_hw *hw,
>                 goto out_sleep;
> 
>  out_sleep:
> -       pm_runtime_put_noidle(wl->dev);
>         mutex_unlock(&wl->mutex);
> 
>         if (ret < 0) {
> @@ -1821,15 +1820,7 @@ 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.
> -        */
> -       wlcore_fw_sleep(wl);
> -
> +       pm_runtime_put_noidle(wl->dev);
>         return 0;
>  }

OK try replacing the pm_runtime_put_noidle() above with just
pm_runtime_put_sync(). The reason why I put noidle there was the
wlcore_fw_sleep() call, with that gone put_sync should do the trick.

Regards,

Tony
Reizer, Eyal May 22, 2018, 1:48 p.m. UTC | #2
> 
> * Reizer, Eyal <eyalr@ti.com> [180522 13:28]:
> > Actually the below patch removing the call to wlcore_fw_sleep() avoids this
> error.
> > The downside is that the wl8 firmware remains fully active during supend, so
> we
> > Would need to find the root cause why the last call allowing the wilink8
> firmware
> > To go into ELP mode during suspend is only completing on resume and not
> during
> > Suspend enter.
> >
> > diff --git a/drivers/net/wireless/ti/wlcore/main.c
> > b/drivers/net/wireless/ti/wlcore/main.c
> > index 4c297aa..8df1ae6 100644
> > --- a/drivers/net/wireless/ti/wlcore/main.c
> > +++ b/drivers/net/wireless/ti/wlcore/main.c
> > @@ -1789,7 +1789,6 @@ static int wl1271_op_suspend(struct
> ieee80211_hw *hw,
> >                 goto out_sleep;
> >
> >  out_sleep:
> > -       pm_runtime_put_noidle(wl->dev);
> >         mutex_unlock(&wl->mutex);
> >
> >         if (ret < 0) {
> > @@ -1821,15 +1820,7 @@ 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.
> > -        */
> > -       wlcore_fw_sleep(wl);
> > -
> > +       pm_runtime_put_noidle(wl->dev);
> >         return 0;
> >  }
> 
> OK try replacing the pm_runtime_put_noidle() above with just
> pm_runtime_put_sync(). The reason why I put noidle there was the
> wlcore_fw_sleep() call, with that gone put_sync should do the trick.
> 

I have tried that already. Same problem. The last call to:
ret = wlcore_raw_write32(wl, HW_ACCESS_ELP_CTRL_REG, ELPCTRL_SLEEP)

which allows the firmware to get into ELP state during wowlan suspend is 
only completing after system resume for some unknown reason...

Best Regards,
Eyal
Tony Lindgren May 22, 2018, 1:55 p.m. UTC | #3
* Reizer, Eyal <eyalr@ti.com> [180522 13:50]:
> > 
> > OK try replacing the pm_runtime_put_noidle() above with just
> > pm_runtime_put_sync(). The reason why I put noidle there was the
> > wlcore_fw_sleep() call, with that gone put_sync should do the trick.
> > 
> 
> I have tried that already. Same problem. The last call to:
> ret = wlcore_raw_write32(wl, HW_ACCESS_ELP_CTRL_REG, ELPCTRL_SLEEP)
> 
> which allows the firmware to get into ELP state during wowlan suspend is 
> only completing after system resume for some unknown reason...

Hmm maybe try also adding wl1271_power_off(wl) after put_sync()?

Regards,

Tony
Reizer, Eyal May 22, 2018, 2:05 p.m. UTC | #4
> > >
> > > OK try replacing the pm_runtime_put_noidle() above with just
> > > pm_runtime_put_sync(). The reason why I put noidle there was the
> > > wlcore_fw_sleep() call, with that gone put_sync should do the trick.
> > >
> >
> > I have tried that already. Same problem. The last call to:
> > ret = wlcore_raw_write32(wl, HW_ACCESS_ELP_CTRL_REG, ELPCTRL_SLEEP)
> >
> > which allows the firmware to get into ELP state during wowlan suspend is
> > only completing after system resume for some unknown reason...
> 
> Hmm maybe try also adding wl1271_power_off(wl) after put_sync()?
> 

No, we don't want to power off the chip in wowlan mode.
We power it of only during standard suspend.

The trick is that it stays on during suspend and can be used 
As a wakeup source to the host on specific packets received by
The firmware over the air.

BR,
Eyal
Tony Lindgren May 22, 2018, 3:01 p.m. UTC | #5
* Reizer, Eyal <eyalr@ti.com> [180522 14:07]:
> > > >
> > > > OK try replacing the pm_runtime_put_noidle() above with just
> > > > pm_runtime_put_sync(). The reason why I put noidle there was the
> > > > wlcore_fw_sleep() call, with that gone put_sync should do the trick.
> > > >
> > >
> > > I have tried that already. Same problem. The last call to:
> > > ret = wlcore_raw_write32(wl, HW_ACCESS_ELP_CTRL_REG, ELPCTRL_SLEEP)
> > >
> > > which allows the firmware to get into ELP state during wowlan suspend is
> > > only completing after system resume for some unknown reason...
> > 
> > Hmm maybe try also adding wl1271_power_off(wl) after put_sync()?
> > 
> 
> No, we don't want to power off the chip in wowlan mode.
> We power it of only during standard suspend.
> 
> The trick is that it stays on during suspend and can be used 
> As a wakeup source to the host on specific packets received by
> The firmware over the air.

Oh right, then in theory pm_runtime_put_sync() should do the
here.

Regards,

Tony
diff mbox

Patch

diff --git a/drivers/net/wireless/ti/wlcore/main.c
b/drivers/net/wireless/ti/wlcore/main.c
index 4c297aa..8df1ae6 100644
--- a/drivers/net/wireless/ti/wlcore/main.c
+++ b/drivers/net/wireless/ti/wlcore/main.c
@@ -1789,7 +1789,6 @@  static int wl1271_op_suspend(struct ieee80211_hw *hw,
                goto out_sleep;

 out_sleep:
-       pm_runtime_put_noidle(wl->dev);
        mutex_unlock(&wl->mutex);

        if (ret < 0) {
@@ -1821,15 +1820,7 @@  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.
-        */
-       wlcore_fw_sleep(wl);
-
+       pm_runtime_put_noidle(wl->dev);
        return 0;
 }