| Message ID | 1433178885-8787-1-git-send-email-chaitanya.mgit@gmail.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Johannes Berg |
| Headers | show |
On Mon, 2015-06-01 at 22:44 +0530, Chaitanya T K wrote: > From: Chaitanya T K <chaitanya.mgit@gmail.com> > > If suspended during TX in progress, there can be > race where the driver is put of of power-save due > to TX and during suspend dynamic_ps_time is cancelled > and TX packet is flushed, leaving the driver in ACTIVE > even after resuming until dynamic_ps_time puts > driver back in DOZE. (Which only happens if there > is another TX). > > This can lead high power consumption of chipset during (or) > after resuming also. This isn't what your patch is actually doing though. You need to mention WoWLAN at the very least in your commit log; or are you just randomly changing the code until it fixes your bug? :) johannes -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Jun 2, 2015 at 2:06 AM, Johannes Berg <johannes@sipsolutions.net> wrote: > On Mon, 2015-06-01 at 22:44 +0530, Chaitanya T K wrote: >> From: Chaitanya T K <chaitanya.mgit@gmail.com> >> >> If suspended during TX in progress, there can be >> race where the driver is put of of power-save due >> to TX and during suspend dynamic_ps_time is cancelled >> and TX packet is flushed, leaving the driver in ACTIVE >> even after resuming until dynamic_ps_time puts >> driver back in DOZE. (Which only happens if there >> is another TX). >> >> This can lead high power consumption of chipset during (or) >> after resuming also. > > This isn't what your patch is actually doing though. You need to mention > WoWLAN at the very least in your commit log; Yes, WoWLAN is enabled in our testing. Without wowlan the connection will not be intact. So i assumed the check "is_associated" would imply wowlan. > or are you just randomly > changing the code until it fixes your bug? :) I am not that lucky to find a proper fix that way :-). -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, 2015-06-02 at 03:02 +0530, Krishna Chaitanya wrote: > > This isn't what your patch is actually doing though. You need to mention > > WoWLAN at the very least in your commit log; > Yes, WoWLAN is enabled in our testing. Without wowlan the connection > will not be intact. So i assumed the check "is_associated" would > imply wowlan. I meant the commit log. If I read your commit log without the patch, it doesn't make sense at all since regular suspend/resume will turn off the device anyway. You really need to mention a) the fact that wowlan is used, and b) the fact that you care about powersave state *while in wowlan* johannes -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Jun 10, 2015 at 1:40 AM, Johannes Berg <johannes@sipsolutions.net> wrote: > On Tue, 2015-06-02 at 03:02 +0530, Krishna Chaitanya wrote: > >> > This isn't what your patch is actually doing though. You need to mention >> > WoWLAN at the very least in your commit log; > >> Yes, WoWLAN is enabled in our testing. Without wowlan the connection >> will not be intact. So i assumed the check "is_associated" would >> imply wowlan. > > I meant the commit log. > > If I read your commit log without the patch, it doesn't make sense at > all since regular suspend/resume will turn off the device anyway. You > really need to mention > a) the fact that wowlan is used, and > b) the fact that you care about powersave state *while in wowlan* Hmm...ok, i will rephrase the commit log and re-submit V3, Thanks. -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/net/mac80211/pm.c b/net/mac80211/pm.c index ac6ad62..cc311be 100644 --- a/net/mac80211/pm.c +++ b/net/mac80211/pm.c @@ -76,6 +76,21 @@ int __ieee80211_suspend(struct ieee80211_hw *hw, struct cfg80211_wowlan *wowlan) if (sdata->vif.type != NL80211_IFTYPE_STATION) continue; ieee80211_mgd_quiesce(sdata); + /* If suspended during TX in progress, there + * can be a race where the driver is put out + * of power-save due to TX and during suspend + * dynamic_ps_timer is cancelled and TX packet + * is flushed, leaving the driver in ACTIVE even + * after resuming until dynamic_ps_timer puts + * driver back in DOZE. + */ + if (sdata->u.mgd.associated && + sdata->u.mgd.powersave && + !(local->hw.conf.flags & IEEE80211_CONF_PS)) { + local->hw.conf.flags |= IEEE80211_CONF_PS; + ieee80211_hw_config(local, + IEEE80211_CONF_CHANGE_PS); + } } err = drv_suspend(local, wowlan);