b43-dev.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [RFC/RFT] mac80211 powersave rework
@ 2013-12-16 22:00 Seth Forshee
  2013-12-16 22:00 ` [RFC PATCH 1/8] mac80211: Move dynamic PS data out of common code Seth Forshee
                   ` (7 more replies)
  0 siblings, 8 replies; 17+ messages in thread
From: Seth Forshee @ 2013-12-16 22:00 UTC (permalink / raw)
  To: Johannes Berg
  Cc: linux-wireless, b43-dev, brcm80211-dev-list, John W. Linville,
	Stefano Brivio, Arend van Spriel, Seth Forshee

Hi Johannes,

This is a set of patches that I've been working on on-and-off for
several months, in response to your feedback to some less ambitious
changes I submitted previously to fix off-channel problems with Broadcom
hardware [1] (though I haven't taken things so far as what you suggest
there). The patches fix the Broadcom problems and should also make
powersave capable of supporting multi-interface devices.

The patches add a new powersave sub-module which handles the direct
hardware control of PS. The managed mode code is changed to instead
control only the PS state of individual interfaces. The hardware PS
state of a device is changed only from the PS module by aggregating the
PS states of that device's interfaces.

The patches also add a new driver callback named change_ps to inform
drivers of changes to interface PS states. My expectation is that
multi-interface drivers will require something like this, but this is
one area in particular where I'd like to get some feedback. I have
already put this interface to use though in the Broadcom drivers to make
them aware of when the HWPS field should be enabled for transmitting
nullfuncs with the PM bit set.

Along with all of this I did a little bit of general clean-up of the
managed mode PS code as well as some necessary changes to the scan and
off-channel code.

So far I've tested this with iwlwifi, ath9k, b43, and brcmsmac, and
everything seems to be working well. I've also been using the code for a
couple of weeks on two machines with no sign of ill effects, so I think
the code is fairly stable, at least for managed mode. I do need to test
a few more specific code paths and double check that no additional
locking is needed. I also haven't done testing with anything other than
managed mode yet, though other modes should be largely unaffected by the
changes.

Please take a look and let me know what you think. I would also love to
receive testing feedback from anyone who'd like to try out these
changes.

Thanks,
Seth

[1] http://marc.info/?l=linux-wireless&m=135838252227053


Seth Forshee (8):
  mac80211: Move dynamic PS data out of common code
  mac80211: Add per-interface powersave states and parameters
  mac80211: Add powersave module
  mac80211: Use PS module for managed mode powersave
  mac80211: Don't start dynamic PS timer when leaving off-channel if
    still scanning
  brcmsmac: Set MCTL_HPS when PM should be set
  b43: Allow HWPS state to be changed
  b43: Set B43_MACCTL_HWPS when PM should be set

 drivers/net/wireless/b43/main.c                    |  24 ++-
 .../net/wireless/brcm80211/brcmsmac/mac80211_if.c  |   7 +
 drivers/net/wireless/brcm80211/brcmsmac/main.c     |  17 +-
 drivers/net/wireless/brcm80211/brcmsmac/main.h     |   1 +
 include/net/mac80211.h                             |  90 ++++++--
 net/mac80211/Makefile                              |   3 +-
 net/mac80211/cfg.c                                 |   9 +-
 net/mac80211/driver-ops.h                          |  13 ++
 net/mac80211/ieee80211_i.h                         |  29 +--
 net/mac80211/iface.c                               |  11 +-
 net/mac80211/main.c                                |   7 -
 net/mac80211/mlme.c                                | 240 ++++++++++-----------
 net/mac80211/offchannel.c                          |  61 ++----
 net/mac80211/pm.c                                  |  17 +-
 net/mac80211/ps.c                                  | 162 ++++++++++++++
 net/mac80211/rx.c                                  |  10 +-
 net/mac80211/scan.c                                |  14 ++
 net/mac80211/status.c                              |  15 +-
 net/mac80211/trace.h                               |  27 +++
 net/mac80211/tx.c                                  |  16 +-
 net/mac80211/util.c                                |  58 ++++-
 21 files changed, 562 insertions(+), 269 deletions(-)
 create mode 100644 net/mac80211/ps.c

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [RFC PATCH 1/8] mac80211: Move dynamic PS data out of common code
  2013-12-16 22:00 [RFC/RFT] mac80211 powersave rework Seth Forshee
@ 2013-12-16 22:00 ` Seth Forshee
  2013-12-17  8:08   ` Johannes Berg
  2013-12-16 22:00 ` [RFC PATCH 2/8] mac80211: Add per-interface powersave states and parameters Seth Forshee
                   ` (6 subsequent siblings)
  7 siblings, 1 reply; 17+ messages in thread
From: Seth Forshee @ 2013-12-16 22:00 UTC (permalink / raw)
  To: Johannes Berg
  Cc: linux-wireless, b43-dev, brcm80211-dev-list, John W. Linville,
	Stefano Brivio, Arend van Spriel, Seth Forshee

The timers and work structs for dynamic powersave are used only
for managed interfaces, so move them to ieee80211_if_managed and
deal with them only in sections of code specifically dealing with
managed mode interfaces.

Signed-off-by: Seth Forshee <seth.forshee@canonical.com>
---
 net/mac80211/ieee80211_i.h | 11 +++---
 net/mac80211/iface.c       |  3 --
 net/mac80211/main.c        |  7 ----
 net/mac80211/mlme.c        | 89 +++++++++++++++++++++++++++++++---------------
 net/mac80211/offchannel.c  |  6 ++--
 net/mac80211/pm.c          | 17 +++++----
 net/mac80211/rx.c          | 10 +-----
 net/mac80211/status.c      |  4 +--
 net/mac80211/tx.c          |  4 +--
 net/mac80211/util.c        | 11 ++++++
 10 files changed, 94 insertions(+), 68 deletions(-)

diff --git a/net/mac80211/ieee80211_i.h b/net/mac80211/ieee80211_i.h
index ed5bf8b..785d1b8 100644
--- a/net/mac80211/ieee80211_i.h
+++ b/net/mac80211/ieee80211_i.h
@@ -387,10 +387,13 @@ struct ieee80211_if_managed {
 	struct timer_list conn_mon_timer;
 	struct timer_list bcn_mon_timer;
 	struct timer_list chswitch_timer;
+	struct timer_list dynamic_ps_timer;
 	struct work_struct monitor_work;
 	struct work_struct chswitch_work;
 	struct work_struct beacon_connection_loss_work;
 	struct work_struct csa_connection_drop_work;
+	struct work_struct dynamic_ps_enable_work;
+	struct work_struct dynamic_ps_disable_work;
 
 	unsigned long beacon_timeout;
 	unsigned long probe_timeout;
@@ -1188,9 +1191,6 @@ struct ieee80211_local {
 	 * interface (and monitors) in PS, this then points there.
 	 */
 	struct ieee80211_sub_if_data *ps_sdata;
-	struct work_struct dynamic_ps_enable_work;
-	struct work_struct dynamic_ps_disable_work;
-	struct timer_list dynamic_ps_timer;
 	struct notifier_block network_latency_notifier;
 	struct notifier_block ifa_notifier;
 	struct notifier_block ifa6_notifier;
@@ -1363,6 +1363,7 @@ void ieee80211_send_pspoll(struct ieee80211_local *local,
 			   struct ieee80211_sub_if_data *sdata);
 void ieee80211_recalc_ps(struct ieee80211_local *local, s32 latency);
 void ieee80211_recalc_ps_vif(struct ieee80211_sub_if_data *sdata);
+void ieee80211_mgd_notify_rx(struct ieee80211_rx_data *rx);
 int ieee80211_max_network_latency(struct notifier_block *nb,
 				  unsigned long data, void *dummy);
 int ieee80211_set_arp_filter(struct ieee80211_sub_if_data *sdata);
@@ -1594,6 +1595,7 @@ static inline int __ieee80211_resume(struct ieee80211_hw *hw)
 extern void *mac80211_wiphy_privid; /* for wiphy privid */
 u8 *ieee80211_get_bssid(struct ieee80211_hdr *hdr, size_t len,
 			enum nl80211_iftype type);
+void ieee80211_notify_rx(struct ieee80211_rx_data *rx);
 int ieee80211_frame_duration(enum ieee80211_band band, size_t len,
 			     int rate, int erp, int short_preamble,
 			     int shift);
@@ -1654,9 +1656,6 @@ static inline void ieee802_11_parse_elems(const u8 *start, size_t len,
 	ieee802_11_parse_elems_crc(start, len, action, elems, 0, 0);
 }
 
-void ieee80211_dynamic_ps_enable_work(struct work_struct *work);
-void ieee80211_dynamic_ps_disable_work(struct work_struct *work);
-void ieee80211_dynamic_ps_timer(unsigned long data);
 void ieee80211_send_nullfunc(struct ieee80211_local *local,
 			     struct ieee80211_sub_if_data *sdata,
 			     int powersave);
diff --git a/net/mac80211/iface.c b/net/mac80211/iface.c
index 7aa9f9d..784b651 100644
--- a/net/mac80211/iface.c
+++ b/net/mac80211/iface.c
@@ -816,9 +816,6 @@ static void ieee80211_do_stop(struct ieee80211_sub_if_data *sdata,
 		netif_addr_unlock_bh(sdata->dev);
 	}
 
-	del_timer_sync(&local->dynamic_ps_timer);
-	cancel_work_sync(&local->dynamic_ps_enable_work);
-
 	cancel_work_sync(&sdata->recalc_smps);
 	sdata->vif.csa_active = false;
 	cancel_work_sync(&sdata->csa_finalize_work);
diff --git a/net/mac80211/main.c b/net/mac80211/main.c
index fa34cd2..4ab607c 100644
--- a/net/mac80211/main.c
+++ b/net/mac80211/main.c
@@ -614,13 +614,6 @@ struct ieee80211_hw *ieee80211_alloc_hw(size_t priv_data_len,
 	INIT_WORK(&local->reconfig_filter, ieee80211_reconfig_filter);
 	local->smps_mode = IEEE80211_SMPS_OFF;
 
-	INIT_WORK(&local->dynamic_ps_enable_work,
-		  ieee80211_dynamic_ps_enable_work);
-	INIT_WORK(&local->dynamic_ps_disable_work,
-		  ieee80211_dynamic_ps_disable_work);
-	setup_timer(&local->dynamic_ps_timer,
-		    ieee80211_dynamic_ps_timer, (unsigned long) local);
-
 	INIT_WORK(&local->sched_scan_stopped_work,
 		  ieee80211_sched_scan_stopped_work);
 
diff --git a/net/mac80211/mlme.c b/net/mac80211/mlme.c
index 900ead3..4ec8c0a 100644
--- a/net/mac80211/mlme.c
+++ b/net/mac80211/mlme.c
@@ -1139,6 +1139,7 @@ static void ieee80211_enable_ps(struct ieee80211_local *local,
 				struct ieee80211_sub_if_data *sdata)
 {
 	struct ieee80211_conf *conf = &local->hw.conf;
+	struct ieee80211_if_managed *ifmgd = &sdata->u.mgd;
 
 	/*
 	 * If we are scanning right now then the parameters will
@@ -1149,7 +1150,7 @@ static void ieee80211_enable_ps(struct ieee80211_local *local,
 
 	if (conf->dynamic_ps_timeout > 0 &&
 	    !(local->hw.flags & IEEE80211_HW_SUPPORTS_DYNAMIC_PS)) {
-		mod_timer(&local->dynamic_ps_timer, jiffies +
+		mod_timer(&ifmgd->dynamic_ps_timer, jiffies +
 			  msecs_to_jiffies(conf->dynamic_ps_timeout));
 	} else {
 		if (local->hw.flags & IEEE80211_HW_PS_NULLFUNC_STACK)
@@ -1164,17 +1165,19 @@ static void ieee80211_enable_ps(struct ieee80211_local *local,
 	}
 }
 
-static void ieee80211_change_ps(struct ieee80211_local *local)
+static void ieee80211_change_ps(struct ieee80211_sub_if_data *sdata, bool ps_enable)
 {
+	struct ieee80211_local *local = sdata->local;
 	struct ieee80211_conf *conf = &local->hw.conf;
+	struct ieee80211_if_managed *ifmgd = &sdata->u.mgd;
 
-	if (local->ps_sdata) {
-		ieee80211_enable_ps(local, local->ps_sdata);
+	if (ps_enable) {
+		ieee80211_enable_ps(local, sdata);
 	} else if (conf->flags & IEEE80211_CONF_PS) {
 		conf->flags &= ~IEEE80211_CONF_PS;
 		ieee80211_hw_config(local, IEEE80211_CONF_CHANGE_PS);
-		del_timer_sync(&local->dynamic_ps_timer);
-		cancel_work_sync(&local->dynamic_ps_enable_work);
+		del_timer_sync(&ifmgd->dynamic_ps_timer);
+		cancel_work_sync(&ifmgd->dynamic_ps_enable_work);
 	}
 }
 
@@ -1211,7 +1214,7 @@ static bool ieee80211_powersave_allowed(struct ieee80211_sub_if_data *sdata)
 /* need to hold RTNL or interface lock */
 void ieee80211_recalc_ps(struct ieee80211_local *local, s32 latency)
 {
-	struct ieee80211_sub_if_data *sdata, *found = NULL;
+	struct ieee80211_sub_if_data *sdata, *old_ps_sdata, *found = NULL;
 	int count = 0;
 	int timeout;
 
@@ -1220,6 +1223,8 @@ void ieee80211_recalc_ps(struct ieee80211_local *local, s32 latency)
 		return;
 	}
 
+	old_ps_sdata = local->ps_sdata;
+
 	list_for_each_entry(sdata, &local->interfaces, list) {
 		if (!ieee80211_sdata_running(sdata))
 			continue;
@@ -1284,7 +1289,10 @@ void ieee80211_recalc_ps(struct ieee80211_local *local, s32 latency)
 		local->ps_sdata = NULL;
 	}
 
-	ieee80211_change_ps(local);
+	if (local->ps_sdata)
+		ieee80211_change_ps(local->ps_sdata, true);
+	else if (old_ps_sdata)
+		ieee80211_change_ps(old_ps_sdata, false);
 }
 
 void ieee80211_recalc_ps_vif(struct ieee80211_sub_if_data *sdata)
@@ -1297,11 +1305,12 @@ void ieee80211_recalc_ps_vif(struct ieee80211_sub_if_data *sdata)
 	}
 }
 
-void ieee80211_dynamic_ps_disable_work(struct work_struct *work)
+static void ieee80211_dynamic_ps_disable_work(struct work_struct *work)
 {
-	struct ieee80211_local *local =
-		container_of(work, struct ieee80211_local,
-			     dynamic_ps_disable_work);
+	struct ieee80211_sub_if_data *sdata =
+		container_of(work, struct ieee80211_sub_if_data,
+			     u.mgd.dynamic_ps_disable_work);
+	struct ieee80211_local *local = sdata->local;
 
 	if (local->hw.conf.flags & IEEE80211_CONF_PS) {
 		local->hw.conf.flags &= ~IEEE80211_CONF_PS;
@@ -1313,29 +1322,27 @@ void ieee80211_dynamic_ps_disable_work(struct work_struct *work)
 					IEEE80211_QUEUE_STOP_REASON_PS);
 }
 
-void ieee80211_dynamic_ps_enable_work(struct work_struct *work)
+static void ieee80211_dynamic_ps_enable_work(struct work_struct *work)
 {
-	struct ieee80211_local *local =
-		container_of(work, struct ieee80211_local,
-			     dynamic_ps_enable_work);
-	struct ieee80211_sub_if_data *sdata = local->ps_sdata;
-	struct ieee80211_if_managed *ifmgd;
+	struct ieee80211_sub_if_data *sdata =
+		container_of(work, struct ieee80211_sub_if_data,
+			     u.mgd.dynamic_ps_enable_work);
+	struct ieee80211_local *local = sdata->local;
+	struct ieee80211_if_managed *ifmgd = &sdata->u.mgd;
 	unsigned long flags;
 	int q;
 
 	/* can only happen when PS was just disabled anyway */
-	if (!sdata)
+	if (!local->ps_sdata)
 		return;
 
-	ifmgd = &sdata->u.mgd;
-
 	if (local->hw.conf.flags & IEEE80211_CONF_PS)
 		return;
 
 	if (local->hw.conf.dynamic_ps_timeout > 0) {
 		/* don't enter PS if TX frames are pending */
 		if (drv_tx_frames_pending(local)) {
-			mod_timer(&local->dynamic_ps_timer, jiffies +
+			mod_timer(&ifmgd->dynamic_ps_timer, jiffies +
 				  msecs_to_jiffies(
 				  local->hw.conf.dynamic_ps_timeout));
 			return;
@@ -1351,7 +1358,7 @@ void ieee80211_dynamic_ps_enable_work(struct work_struct *work)
 			if (local->queue_stop_reasons[q]) {
 				spin_unlock_irqrestore(&local->queue_stop_reason_lock,
 						       flags);
-				mod_timer(&local->dynamic_ps_timer, jiffies +
+				mod_timer(&ifmgd->dynamic_ps_timer, jiffies +
 					  msecs_to_jiffies(
 					  local->hw.conf.dynamic_ps_timeout));
 				return;
@@ -1363,7 +1370,7 @@ void ieee80211_dynamic_ps_enable_work(struct work_struct *work)
 	if ((local->hw.flags & IEEE80211_HW_PS_NULLFUNC_STACK) &&
 	    !(ifmgd->flags & IEEE80211_STA_NULLFUNC_ACKED)) {
 		if (drv_tx_frames_pending(local)) {
-			mod_timer(&local->dynamic_ps_timer, jiffies +
+			mod_timer(&ifmgd->dynamic_ps_timer, jiffies +
 				  msecs_to_jiffies(
 				  local->hw.conf.dynamic_ps_timeout));
 		} else {
@@ -1382,14 +1389,30 @@ void ieee80211_dynamic_ps_enable_work(struct work_struct *work)
 	}
 }
 
-void ieee80211_dynamic_ps_timer(unsigned long data)
+static void ieee80211_dynamic_ps_timer(unsigned long data)
 {
-	struct ieee80211_local *local = (void *) data;
+	struct ieee80211_sub_if_data *sdata = (void *)data;
+	struct ieee80211_local *local = sdata->local;
 
 	if (local->quiescing || local->suspended)
 		return;
 
-	ieee80211_queue_work(&local->hw, &local->dynamic_ps_enable_work);
+	ieee80211_queue_work(&local->hw, &sdata->u.mgd.dynamic_ps_enable_work);
+}
+
+void ieee80211_mgd_notify_rx(struct ieee80211_rx_data *rx)
+{
+	struct ieee80211_sub_if_data *sdata = rx->sdata;
+	struct ieee80211_local *local = rx->local;
+
+	if (local->ps_sdata && local->hw.conf.dynamic_ps_timeout > 0 &&
+	    !is_multicast_ether_addr(
+		    ((struct ethhdr *)rx->skb->data)->h_dest) &&
+	    (!local->scanning &&
+	     !test_bit(SDATA_STATE_OFFCHANNEL, &sdata->state))) {
+			mod_timer(&sdata->u.mgd.dynamic_ps_timer, jiffies +
+			 msecs_to_jiffies(local->hw.conf.dynamic_ps_timeout));
+	}
 }
 
 void ieee80211_dfs_cac_timer_work(struct work_struct *work)
@@ -1719,8 +1742,8 @@ static void ieee80211_set_disassoc(struct ieee80211_sub_if_data *sdata,
 
 	sdata->ap_power_level = IEEE80211_UNSET_POWER_LEVEL;
 
-	del_timer_sync(&local->dynamic_ps_timer);
-	cancel_work_sync(&local->dynamic_ps_enable_work);
+	del_timer_sync(&ifmgd->dynamic_ps_timer);
+	cancel_work_sync(&ifmgd->dynamic_ps_enable_work);
 
 	/* Disable ARP filtering */
 	if (sdata->vif.bss_conf.arp_addr_cnt)
@@ -3506,6 +3529,10 @@ void ieee80211_sta_setup_sdata(struct ieee80211_sub_if_data *sdata)
 	INIT_WORK(&ifmgd->csa_connection_drop_work,
 		  ieee80211_csa_connection_drop_work);
 	INIT_WORK(&ifmgd->request_smps_work, ieee80211_request_smps_mgd_work);
+	INIT_WORK(&ifmgd->dynamic_ps_enable_work,
+		  ieee80211_dynamic_ps_enable_work);
+	INIT_WORK(&ifmgd->dynamic_ps_disable_work,
+		  ieee80211_dynamic_ps_disable_work);
 	setup_timer(&ifmgd->timer, ieee80211_sta_timer,
 		    (unsigned long) sdata);
 	setup_timer(&ifmgd->bcn_mon_timer, ieee80211_sta_bcn_mon_timer,
@@ -3514,6 +3541,8 @@ void ieee80211_sta_setup_sdata(struct ieee80211_sub_if_data *sdata)
 		    (unsigned long) sdata);
 	setup_timer(&ifmgd->chswitch_timer, ieee80211_chswitch_timer,
 		    (unsigned long) sdata);
+	setup_timer(&ifmgd->dynamic_ps_timer, ieee80211_dynamic_ps_timer,
+		    (unsigned long) sdata);
 
 	ifmgd->flags = 0;
 	ifmgd->powersave = sdata->wdev.ps;
@@ -4361,6 +4390,7 @@ void ieee80211_mgd_stop(struct ieee80211_sub_if_data *sdata)
 	cancel_work_sync(&ifmgd->request_smps_work);
 	cancel_work_sync(&ifmgd->csa_connection_drop_work);
 	cancel_work_sync(&ifmgd->chswitch_work);
+	cancel_work_sync(&ifmgd->dynamic_ps_enable_work);
 
 	sdata_lock(sdata);
 	if (ifmgd->assoc_data) {
@@ -4371,6 +4401,7 @@ void ieee80211_mgd_stop(struct ieee80211_sub_if_data *sdata)
 	if (ifmgd->auth_data)
 		ieee80211_destroy_auth_data(sdata, false);
 	del_timer_sync(&ifmgd->timer);
+	del_timer_sync(&ifmgd->dynamic_ps_timer);
 	sdata_unlock(sdata);
 }
 
diff --git a/net/mac80211/offchannel.c b/net/mac80211/offchannel.c
index 0c2a294..2049a0a 100644
--- a/net/mac80211/offchannel.c
+++ b/net/mac80211/offchannel.c
@@ -33,11 +33,11 @@ static void ieee80211_offchannel_ps_enable(struct ieee80211_sub_if_data *sdata)
 
 	/* FIXME: what to do when local->pspolling is true? */
 
-	del_timer_sync(&local->dynamic_ps_timer);
+	del_timer_sync(&ifmgd->dynamic_ps_timer);
 	del_timer_sync(&ifmgd->bcn_mon_timer);
 	del_timer_sync(&ifmgd->conn_mon_timer);
 
-	cancel_work_sync(&local->dynamic_ps_enable_work);
+	cancel_work_sync(&ifmgd->dynamic_ps_enable_work);
 
 	if (local->hw.conf.flags & IEEE80211_CONF_PS) {
 		local->offchannel_ps_enabled = true;
@@ -94,7 +94,7 @@ static void ieee80211_offchannel_ps_disable(struct ieee80211_sub_if_data *sdata)
 		 * the AP that we are awake.
 		 */
 		ieee80211_send_nullfunc(local, sdata, 0);
-		mod_timer(&local->dynamic_ps_timer, jiffies +
+		mod_timer(&sdata->u.mgd.dynamic_ps_timer, jiffies +
 			  msecs_to_jiffies(local->hw.conf.dynamic_ps_timeout));
 	}
 
diff --git a/net/mac80211/pm.c b/net/mac80211/pm.c
index 3401262..894c497 100644
--- a/net/mac80211/pm.c
+++ b/net/mac80211/pm.c
@@ -52,13 +52,6 @@ int __ieee80211_suspend(struct ieee80211_hw *hw, struct cfg80211_wowlan *wowlan)
 	/* Don't try to run timers while suspended. */
 	del_timer_sync(&local->sta_cleanup);
 
-	 /*
-	 * Note that this particular timer doesn't need to be
-	 * restarted at resume.
-	 */
-	cancel_work_sync(&local->dynamic_ps_enable_work);
-	del_timer_sync(&local->dynamic_ps_timer);
-
 	local->wowlan = wowlan && local->open_count;
 	if (local->wowlan) {
 		int err = drv_suspend(local, wowlan);
@@ -106,6 +99,16 @@ int __ieee80211_suspend(struct ieee80211_hw *hw, struct cfg80211_wowlan *wowlan)
 		    sdata->vif.type == NL80211_IFTYPE_MONITOR)
 			continue;
 
+		if (sdata->vif.type == NL80211_IFTYPE_STATION) {
+			struct ieee80211_if_managed *ifmgd = &sdata->u.mgd;
+			 /*
+			 * Note that this particular timer doesn't need to be
+			 * restarted at resume.
+			 */
+			cancel_work_sync(&ifmgd->dynamic_ps_enable_work);
+			del_timer_sync(&ifmgd->dynamic_ps_timer);
+		}
+
 		drv_remove_interface(local, sdata);
 	}
 
diff --git a/net/mac80211/rx.c b/net/mac80211/rx.c
index 2dfa755..5707566 100644
--- a/net/mac80211/rx.c
+++ b/net/mac80211/rx.c
@@ -2212,7 +2212,6 @@ static ieee80211_rx_result debug_noinline
 ieee80211_rx_h_data(struct ieee80211_rx_data *rx)
 {
 	struct ieee80211_sub_if_data *sdata = rx->sdata;
-	struct ieee80211_local *local = rx->local;
 	struct net_device *dev = sdata->dev;
 	struct ieee80211_hdr *hdr = (struct ieee80211_hdr *)rx->skb->data;
 	__le16 fc = hdr->frame_control;
@@ -2258,14 +2257,7 @@ ieee80211_rx_h_data(struct ieee80211_rx_data *rx)
 	dev->stats.rx_packets++;
 	dev->stats.rx_bytes += rx->skb->len;
 
-	if (local->ps_sdata && local->hw.conf.dynamic_ps_timeout > 0 &&
-	    !is_multicast_ether_addr(
-		    ((struct ethhdr *)rx->skb->data)->h_dest) &&
-	    (!local->scanning &&
-	     !test_bit(SDATA_STATE_OFFCHANNEL, &sdata->state))) {
-			mod_timer(&local->dynamic_ps_timer, jiffies +
-			 msecs_to_jiffies(local->hw.conf.dynamic_ps_timeout));
-	}
+	ieee80211_notify_rx(rx);
 
 	ieee80211_deliver_skb(rx);
 
diff --git a/net/mac80211/status.c b/net/mac80211/status.c
index 1ee85c4..3298fe9 100644
--- a/net/mac80211/status.c
+++ b/net/mac80211/status.c
@@ -740,8 +740,8 @@ void ieee80211_tx_status(struct ieee80211_hw *hw, struct sk_buff *skb)
 			local->ps_sdata->u.mgd.flags |=
 					IEEE80211_STA_NULLFUNC_ACKED;
 		} else
-			mod_timer(&local->dynamic_ps_timer, jiffies +
-					msecs_to_jiffies(10));
+			mod_timer(&local->ps_sdata->u.mgd.dynamic_ps_timer,
+				  jiffies + msecs_to_jiffies(10));
 	}
 
 	ieee80211_report_used_skb(local, skb, false);
diff --git a/net/mac80211/tx.c b/net/mac80211/tx.c
index 6d59e21..0ffc2066 100644
--- a/net/mac80211/tx.c
+++ b/net/mac80211/tx.c
@@ -253,14 +253,14 @@ ieee80211_tx_h_dynamic_ps(struct ieee80211_tx_data *tx)
 						IEEE80211_QUEUE_STOP_REASON_PS);
 		ifmgd->flags &= ~IEEE80211_STA_NULLFUNC_ACKED;
 		ieee80211_queue_work(&local->hw,
-				     &local->dynamic_ps_disable_work);
+				     &ifmgd->dynamic_ps_disable_work);
 	}
 
 	/* Don't restart the timer if we're not disassociated */
 	if (!ifmgd->associated)
 		return TX_CONTINUE;
 
-	mod_timer(&local->dynamic_ps_timer, jiffies +
+	mod_timer(&ifmgd->dynamic_ps_timer, jiffies +
 		  msecs_to_jiffies(local->hw.conf.dynamic_ps_timeout));
 
 	return TX_CONTINUE;
diff --git a/net/mac80211/util.c b/net/mac80211/util.c
index 875e172..010cd2c 100644
--- a/net/mac80211/util.c
+++ b/net/mac80211/util.c
@@ -95,6 +95,17 @@ u8 *ieee80211_get_bssid(struct ieee80211_hdr *hdr, size_t len,
 	return NULL;
 }
 
+void ieee80211_notify_rx(struct ieee80211_rx_data *rx)
+{
+	switch(rx->sdata->vif.type) {
+	case NL80211_IFTYPE_MONITOR:
+		ieee80211_mgd_notify_rx(rx);
+		break;
+	default:
+		break;
+	}
+}
+
 void ieee80211_tx_set_protected(struct ieee80211_tx_data *tx)
 {
 	struct sk_buff *skb;
-- 
1.8.3.2

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [RFC PATCH 2/8] mac80211: Add per-interface powersave states and parameters
  2013-12-16 22:00 [RFC/RFT] mac80211 powersave rework Seth Forshee
  2013-12-16 22:00 ` [RFC PATCH 1/8] mac80211: Move dynamic PS data out of common code Seth Forshee
@ 2013-12-16 22:00 ` Seth Forshee
  2013-12-17  8:11   ` Johannes Berg
  2013-12-16 22:00 ` [RFC PATCH 3/8] mac80211: Add powersave module Seth Forshee
                   ` (5 subsequent siblings)
  7 siblings, 1 reply; 17+ messages in thread
From: Seth Forshee @ 2013-12-16 22:00 UTC (permalink / raw)
  To: Johannes Berg
  Cc: linux-wireless, b43-dev, brcm80211-dev-list, John W. Linville,
	Stefano Brivio, Arend van Spriel, Seth Forshee

In preparation for managing powersave states on a per-interface
basis, add powersave states and parameters to the interface-
specific data structures. Also add a change_ps driver callback
to notify drivers about changes to interface powersave states.

The new members and callback are unused here but will be
utilized by subsequent commits.

Signed-off-by: Seth Forshee <seth.forshee@canonical.com>
---
 include/net/mac80211.h    | 43 +++++++++++++++++++++++++++++++++++++++++++
 net/mac80211/driver-ops.h | 13 +++++++++++++
 net/mac80211/trace.h      | 27 +++++++++++++++++++++++++++
 3 files changed, 83 insertions(+)

diff --git a/include/net/mac80211.h b/include/net/mac80211.h
index 3cd408b..03e4a63 100644
--- a/include/net/mac80211.h
+++ b/include/net/mac80211.h
@@ -1103,6 +1103,26 @@ enum ieee80211_vif_flags {
 };
 
 /**
+ * enum ieee80211_vif_ps_mode - virtual interface power save mode
+ *
+ * Ordered in terms of increasing wakefulness.
+ *
+ * @IEEE80211_VIF_PS_INACTIVE: the interface is not currently open
+ * @IEEE80211_VIF_PS_DOZE: the interface is in a low-power mode and may
+ *	not be able to transmit or receive frames
+ * @IEEE80211_VIF_PS_AWAKE_PM: the interface is awake and able to transmit
+ *	and receive frames but PM may be set in frame control
+ * @IEEE80211_VIF_PS_AWAKE: the interface is fully awake and able to
+ *	transmit and receive frames
+ */
+enum ieee80211_vif_ps_mode {
+	IEEE80211_VIF_PS_INACTIVE,
+	IEEE80211_VIF_PS_DOZE,
+	IEEE80211_VIF_PS_AWAKE_PM,
+	IEEE80211_VIF_PS_AWAKE,
+};
+
+/**
  * struct ieee80211_vif - per-interface data
  *
  * Data in this structure is continually present for driver
@@ -1129,6 +1149,19 @@ enum ieee80211_vif_flags {
  * @debugfs_dir: debugfs dentry, can be used by drivers to create own per
  *	interface debug files. Note that it will be NULL for the virtual
  *	monitor interface (if that is requested.)
+ * @ps_mode: power save mode of this vif
+ * @dynamic_ps_active: indicates whether dynamic PS is active for this vif
+ * @dynamic_ps_timeout: The dynamic powersave timeout (in ms), see the
+ *	powersave documentation below. This variable is valid only when
+ *	the interface is in the doze state.
+ * @max_sleep_period: the maximum number of beacon intervals to sleep for
+ *	before checking the beacon for a TIM bit (managed mode only); this
+ *	value will be only achievable between DTIM frames, the hardware
+ *	needs to check for the multicast traffic bit in DTIM beacons.
+ *	This variable is valid only when the interface is in the doze state.
+ * @ps_dtim_period: The DTIM period of the AP we're connected to, for use
+ *	in power saving. Power saving will not be enabled until a beacon
+ *	has been received and the DTIM period is known.
  * @drv_priv: data area for driver use, will always be aligned to
  *	sizeof(void *).
  */
@@ -1144,6 +1177,11 @@ struct ieee80211_vif {
 
 	struct ieee80211_chanctx_conf __rcu *chanctx_conf;
 
+	enum ieee80211_vif_ps_mode ps_mode;
+	bool dynamic_ps_active;
+	int dynamic_ps_timeout, max_sleep_period;
+	u8 ps_dtim_period;
+
 	u32 driver_flags;
 
 #ifdef CONFIG_MAC80211_DEBUGFS
@@ -2390,6 +2428,10 @@ enum ieee80211_roc_type {
  *	of the bss parameters has changed when a call is made. The callback
  *	can sleep.
  *
+ * @change_ps: Change the power save mode or parameters for the given virtual
+ *	interface. @next_tbtt is required for mesh powersave but is currently
+ *	unused. This callback is optional and may sleep.
+ *
  * @prepare_multicast: Prepare for multicast filter configuration.
  *	This callback is optional, and its return value is passed
  *	to configure_filter(). This callback must be atomic.
@@ -2754,6 +2796,7 @@ struct ieee80211_ops {
 				 struct ieee80211_vif *vif,
 				 struct ieee80211_bss_conf *info,
 				 u32 changed);
+	void (*change_ps)(struct ieee80211_hw *hw, struct ieee80211_vif *vif);
 
 	int (*start_ap)(struct ieee80211_hw *hw, struct ieee80211_vif *vif);
 	void (*stop_ap)(struct ieee80211_hw *hw, struct ieee80211_vif *vif);
diff --git a/net/mac80211/driver-ops.h b/net/mac80211/driver-ops.h
index 5d03c47..8e708bd7 100644
--- a/net/mac80211/driver-ops.h
+++ b/net/mac80211/driver-ops.h
@@ -227,6 +227,19 @@ static inline void drv_bss_info_changed(struct ieee80211_local *local,
 	trace_drv_return_void(local);
 }
 
+static inline void drv_change_ps(struct ieee80211_local *local,
+				 struct ieee80211_sub_if_data *sdata)
+{
+	might_sleep();
+
+	check_sdata_in_driver(sdata);
+
+	trace_drv_change_ps(local, sdata);
+	if (local->ops->change_ps)
+		local->ops->change_ps(&local->hw, &sdata->vif);
+	trace_drv_return_void(local);
+}
+
 static inline u64 drv_prepare_multicast(struct ieee80211_local *local,
 					struct netdev_hw_addr_list *mc_list)
 {
diff --git a/net/mac80211/trace.h b/net/mac80211/trace.h
index e9ccf22..fcc9ac5 100644
--- a/net/mac80211/trace.h
+++ b/net/mac80211/trace.h
@@ -422,6 +422,33 @@ TRACE_EVENT(drv_bss_info_changed,
 	)
 );
 
+TRACE_EVENT(drv_change_ps,
+	TP_PROTO(struct ieee80211_local *local,
+		 struct ieee80211_sub_if_data *sdata),
+
+	TP_ARGS(local, sdata),
+
+	TP_STRUCT__entry(
+		LOCAL_ENTRY
+		VIF_ENTRY
+		__field(int, ps_mode)
+		__field(bool, dynamic_ps_active)
+	),
+
+	TP_fast_assign(
+		LOCAL_ASSIGN;
+		VIF_ASSIGN;
+		__entry->ps_mode = sdata->vif.ps_mode;
+		__entry->dynamic_ps_active = sdata->vif.dynamic_ps_active;
+	),
+
+	TP_printk(
+		LOCAL_PR_FMT  VIF_PR_FMT "mode:%d dynamic_ps:%sactive",
+		LOCAL_PR_ARG, VIF_PR_ARG, __entry->ps_mode,
+		__entry->dynamic_ps_active ? "" : "in"
+	)
+);
+
 TRACE_EVENT(drv_prepare_multicast,
 	TP_PROTO(struct ieee80211_local *local, int mc_count),
 
-- 
1.8.3.2

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [RFC PATCH 3/8] mac80211: Add powersave module
  2013-12-16 22:00 [RFC/RFT] mac80211 powersave rework Seth Forshee
  2013-12-16 22:00 ` [RFC PATCH 1/8] mac80211: Move dynamic PS data out of common code Seth Forshee
  2013-12-16 22:00 ` [RFC PATCH 2/8] mac80211: Add per-interface powersave states and parameters Seth Forshee
@ 2013-12-16 22:00 ` Seth Forshee
  2013-12-17  8:16   ` Johannes Berg
  2013-12-16 22:00 ` [RFC PATCH 4/8] mac80211: Use PS module for managed mode powersave Seth Forshee
                   ` (4 subsequent siblings)
  7 siblings, 1 reply; 17+ messages in thread
From: Seth Forshee @ 2013-12-16 22:00 UTC (permalink / raw)
  To: Johannes Berg
  Cc: linux-wireless, b43-dev, brcm80211-dev-list, John W. Linville,
	Stefano Brivio, Arend van Spriel, Seth Forshee

The current powersave implementation manages the hw PS state
directly based on the type of interface attached to the device,
making it unsuitable for supporting multiple vifs on a single
device. This commit adds a new powersave module which will take
over the direct control of the hw PS states. The power states
of interfaces will be managed based on the interface type, and
the PS module will aggregate the interface states for a device
and set the hw PS state accordingly.

This commit adds the PS module but leaves it unused. Later
commits will convert the rest of mac80211 to use this module.

Signed-off-by: Seth Forshee <seth.forshee@canonical.com>
---
 include/net/mac80211.h     |  47 +++++++------
 net/mac80211/Makefile      |   3 +-
 net/mac80211/ieee80211_i.h |   8 +++
 net/mac80211/ps.c          | 162 +++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 200 insertions(+), 20 deletions(-)
 create mode 100644 net/mac80211/ps.c

diff --git a/include/net/mac80211.h b/include/net/mac80211.h
index 03e4a63..3c9ce86 100644
--- a/include/net/mac80211.h
+++ b/include/net/mac80211.h
@@ -1870,22 +1870,29 @@ void ieee80211_free_txskb(struct ieee80211_hw *hw, struct sk_buff *skb);
 /**
  * DOC: Powersave support
  *
+ * There are two methods by which drivers can support powersave. Drivers
+ * which support only a single interface can use the information in
+ * &struct ieee80211_conf from the config() callback. Drivers supporting
+ * multiple interfaces must implement the change_ps() callback and use the
+ * per-interface powersave state in &struct ieee80211_vif. Details for using
+ * both techniques are described in subsequent sections.
+ *
  * mac80211 has support for various powersave implementations.
  *
  * First, it can support hardware that handles all powersaving by itself,
  * such hardware should simply set the %IEEE80211_HW_SUPPORTS_PS hardware
- * flag. In that case, it will be told about the desired powersave mode
- * with the %IEEE80211_CONF_PS flag depending on the association status.
- * The hardware must take care of sending nullfunc frames when necessary,
- * i.e. when entering and leaving powersave mode. The hardware is required
- * to look at the AID in beacons and signal to the AP that it woke up when
- * it finds traffic directed to it.
- *
- * %IEEE80211_CONF_PS flag enabled means that the powersave mode defined in
- * IEEE 802.11-2007 section 11.2 is enabled. This is not to be confused
- * with hardware wakeup and sleep states. Driver is responsible for waking
- * up the hardware before issuing commands to the hardware and putting it
- * back to sleep at appropriate times.
+ * flag. In that case, depending on association status, it will be told about
+ * the desired powersave mode with the @ps_mode field or the
+ * %IEEE80211_CONF_PS flag. The hardware must take care of sending nullfunc
+ * frames when necessary, i.e. when entering and leaving powersave mode. The
+ * hardware is required to look at the AID in beacons and signal to the AP
+ * that it woke up when it finds traffic directed to it.
+ *
+ * When @ps_mode == %IEEE80211_VIF_PS_DOZE or the %IEEE80211_CONF_PS flag is
+ * set it means that the powersave mode defined in IEEE 802.11-2007 section
+ * 11.2 is enabled. This is not to be confused with hardware wakeup and sleep
+ * states. The driver is responsible for waking up the hardware before issuing
+ * commands to the hardware and putting it back to sleep at appropriate times.
  *
  * When PS is enabled, hardware needs to wakeup for beacons and receive the
  * buffered multicast/broadcast frames after the beacon. Also it must be
@@ -1903,19 +1910,21 @@ void ieee80211_free_txskb(struct ieee80211_hw *hw, struct sk_buff *skb);
  * hardware stays awake for a user-specified period of time after sending a
  * frame so that reply frames need not be buffered and therefore delayed to
  * the next wakeup. It's compromise of getting good enough latency when
- * there's data traffic and still saving significantly power in idle
- * periods.
+ * there's data traffic and still saving significant power in idle periods.
  *
  * Dynamic powersave is simply supported by mac80211 enabling and disabling
  * PS based on traffic. Driver needs to only set %IEEE80211_HW_SUPPORTS_PS
  * flag and mac80211 will handle everything automatically. Additionally,
  * hardware having support for the dynamic PS feature may set the
  * %IEEE80211_HW_SUPPORTS_DYNAMIC_PS flag to indicate that it can support
- * dynamic PS mode itself. The driver needs to look at the
- * @dynamic_ps_timeout hardware configuration value and use it that value
- * whenever %IEEE80211_CONF_PS is set. In this case mac80211 will disable
- * dynamic PS feature in stack and will just keep %IEEE80211_CONF_PS
- * enabled whenever user has enabled powersave.
+ * dynamic PS mode itself. Single-interface drivers need to look at the
+ * @dynamic_ps_timeout hardware configuration value and use that value
+ * whenever %IEEE80211_CONF_PS is set. Drivers supporting multiple interfaces
+ * should look at @dynamic_ps_active whenever @ps_mode is
+ * %IEEE80211_VIF_PS_DOZE to determine if dynamic powersave is enabled and
+ * use the @dynamic_ps_timeout value in &struct ieee80211_vif. In this case
+ * mac80211 will disable dynamic PS feature in stack and will just keep
+ * powersave enabled whenever the user has enabled powersave.
  *
  * Driver informs U-APSD client support by enabling
  * %IEEE80211_HW_SUPPORTS_UAPSD flag. The mode is configured through the
diff --git a/net/mac80211/Makefile b/net/mac80211/Makefile
index 9d7d840..9416198 100644
--- a/net/mac80211/Makefile
+++ b/net/mac80211/Makefile
@@ -25,7 +25,8 @@ mac80211-y := \
 	wme.o \
 	event.o \
 	chan.o \
-	trace.o mlme.o
+	trace.o mlme.o \
+	ps.o
 
 mac80211-$(CONFIG_MAC80211_LEDS) += led.o
 mac80211-$(CONFIG_MAC80211_DEBUGFS) += \
diff --git a/net/mac80211/ieee80211_i.h b/net/mac80211/ieee80211_i.h
index 785d1b8..a489676 100644
--- a/net/mac80211/ieee80211_i.h
+++ b/net/mac80211/ieee80211_i.h
@@ -1180,6 +1180,7 @@ struct ieee80211_local {
 #endif /* CONFIG_MAC80211_DEBUG_COUNTERS */
 
 
+	int awake_cnt;
 	int total_ps_buffered; /* total number of all buffered unicast and
 				* multicast packets for power saving stations
 				*/
@@ -1788,6 +1789,13 @@ int ieee80211_cs_headroom(struct ieee80211_local *local,
 			  struct cfg80211_crypto_settings *crypto,
 			  enum nl80211_iftype iftype);
 
+/* power save */
+void ieee80211_ps_init_vif(struct ieee80211_sub_if_data *sdata);
+void ieee80211_ps_vif_open(struct ieee80211_sub_if_data *sdata);
+void ieee80211_ps_vif_close(struct ieee80211_sub_if_data *sdata);
+void ieee80211_vif_set_ps_mode(struct ieee80211_sub_if_data *sdata,
+			       enum ieee80211_vif_ps_mode mode);
+
 #ifdef CONFIG_MAC80211_NOINLINE
 #define debug_noinline noinline
 #else
diff --git a/net/mac80211/ps.c b/net/mac80211/ps.c
new file mode 100644
index 0000000..b691e8d
--- /dev/null
+++ b/net/mac80211/ps.c
@@ -0,0 +1,162 @@
+/*
+ * Copyright (C) 2013 Canonical Ltd.
+ * Author: Seth Forshee <seth.forshee@canonical.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/nl80211.h>
+#include <linux/pm_qos.h>
+#include <net/mac80211.h>
+#include "ieee80211_i.h"
+#include "driver-ops.h"
+
+/*
+ * Sets the PS parameters in ieee80211_conf. Uses parameters from the first
+ * managed interface found which has power save enabled, or failing that
+ * the parameters from an arbitrary managed interface are used.
+ */
+static void ieee80211_set_ps_params(struct ieee80211_local *local) { struct
+	ieee80211_sub_if_data *sdata, *ps_sdata = NULL;
+
+	list_for_each_entry(sdata, &local->interfaces, list) {
+		if (sdata->vif.ps_mode == IEEE80211_VIF_PS_INACTIVE)
+			continue;
+		if (sdata->vif.type == NL80211_IFTYPE_STATION) {
+			ps_sdata = sdata;
+			if (sdata->vif.dynamic_ps_active ||
+			    sdata->vif.ps_mode == IEEE80211_VIF_PS_DOZE)
+				break;
+		}
+	}
+
+	if (ps_sdata) {
+		struct ieee80211_vif *vif = &sdata->vif;
+		local->hw.conf.dynamic_ps_timeout = vif->dynamic_ps_timeout;
+		local->hw.conf.max_sleep_period = vif->max_sleep_period;
+		local->hw.conf.ps_dtim_period = vif->ps_dtim_period;
+	}
+}
+
+static void ieee80211_recalc_hw_ps(struct ieee80211_local *local)
+{
+	struct ieee80211_conf *conf = &local->hw.conf;
+	bool ps_enabled, ps_enable;
+
+	ps_enable = (local->awake_cnt == 0 &&
+		     local->hw.flags & IEEE80211_HW_SUPPORTS_PS);
+	ps_enabled = !!(conf->flags & IEEE80211_CONF_PS);
+
+	if (ps_enable != ps_enabled) {
+		if (ps_enable) {
+			conf->flags |= IEEE80211_CONF_PS;
+			ieee80211_set_ps_params(local);
+		} else {
+			conf->flags &= ~IEEE80211_CONF_PS;
+		}
+		ieee80211_hw_config(local, IEEE80211_CONF_CHANGE_PS);
+	}
+}
+
+static void __ieee80211_vif_set_ps_mode(struct ieee80211_sub_if_data *sdata,
+					enum ieee80211_vif_ps_mode mode)
+{
+	struct ieee80211_local *local = sdata->local;
+
+	if (WARN_ON_ONCE(sdata->vif.type == NL80211_IFTYPE_MONITOR))
+		return;
+
+	switch (mode) {
+	case IEEE80211_VIF_PS_INACTIVE:
+	case IEEE80211_VIF_PS_DOZE:
+		if (sdata->vif.ps_mode > IEEE80211_VIF_PS_DOZE &&
+		    !WARN_ON(local->awake_cnt <= 0))
+			local->awake_cnt--;
+		break;
+	case IEEE80211_VIF_PS_AWAKE:
+	case IEEE80211_VIF_PS_AWAKE_PM:
+		if (sdata->vif.ps_mode <= IEEE80211_VIF_PS_DOZE)
+			local->awake_cnt++;
+		break;
+	default:
+		sdata_err(sdata, "Invalid PS mode %d\n", mode);
+		return;
+	}
+	sdata->vif.ps_mode = mode;
+}
+
+void ieee80211_ps_init_vif(struct ieee80211_sub_if_data *sdata)
+{
+	sdata->vif.dynamic_ps_active = false;
+	sdata->vif.ps_mode = IEEE80211_VIF_PS_INACTIVE;
+}
+
+void ieee80211_ps_vif_open(struct ieee80211_sub_if_data *sdata)
+{
+	if (sdata->vif.ps_mode != IEEE80211_VIF_PS_INACTIVE)
+		return;
+
+	/* Always start vifs in awake state */
+	sdata->vif.dynamic_ps_active = false;
+	if (sdata->vif.type == NL80211_IFTYPE_MONITOR) {
+		sdata->vif.ps_mode = IEEE80211_VIF_PS_AWAKE;
+		return;
+	}
+	__ieee80211_vif_set_ps_mode(sdata, IEEE80211_VIF_PS_AWAKE);
+	ieee80211_recalc_hw_ps(sdata->local);
+	drv_change_ps(sdata->local, sdata);
+}
+
+void ieee80211_ps_vif_close(struct ieee80211_sub_if_data *sdata)
+{
+	sdata->vif.dynamic_ps_active = false;
+	if (sdata->vif.type == NL80211_IFTYPE_MONITOR) {
+		sdata->vif.ps_mode = IEEE80211_VIF_PS_INACTIVE;
+		return;
+	}
+	__ieee80211_vif_set_ps_mode(sdata, IEEE80211_VIF_PS_INACTIVE);
+	drv_change_ps(sdata->local, sdata);
+	ieee80211_recalc_hw_ps(sdata->local);
+}
+
+void ieee80211_vif_set_ps_mode(struct ieee80211_sub_if_data *sdata,
+			       enum ieee80211_vif_ps_mode mode)
+{
+	/*
+	 * Only ieee80211_ps_vif_{close,open} should move interfaces in
+	 * and out of inactive mode
+	 */
+	if (WARN_ON_ONCE(mode == IEEE80211_VIF_PS_INACTIVE ||
+			 sdata->vif.ps_mode == IEEE80211_VIF_PS_INACTIVE))
+		return;
+
+	if (sdata->vif.type == NL80211_IFTYPE_MONITOR)
+		return;
+
+	/*
+	 * Skip updating the PS mode if already in the requested state,
+	 * except when setting HW dynamic PS active. In that case we
+	 * must always call down into the driver in case any of the
+	 * dynamic PS parameters have changed.
+	 */
+	if (!(sdata->local->hw.flags & IEEE80211_HW_SUPPORTS_DYNAMIC_PS &&
+	      sdata->vif.dynamic_ps_active && mode == IEEE80211_VIF_PS_DOZE) &&
+	    sdata->vif.ps_mode == mode)
+		return;
+
+	__ieee80211_vif_set_ps_mode(sdata, mode);
+
+	/*
+	 * If entering powersave the vif must be transitioned first, and
+	 * the other way around when leaving.
+	 */
+	if (mode <= IEEE80211_VIF_PS_DOZE) {
+		drv_change_ps(sdata->local, sdata);
+		ieee80211_recalc_hw_ps(sdata->local);
+	} else {
+		ieee80211_recalc_hw_ps(sdata->local);
+		drv_change_ps(sdata->local, sdata);
+	}
+}
-- 
1.8.3.2

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [RFC PATCH 4/8] mac80211: Use PS module for managed mode powersave
  2013-12-16 22:00 [RFC/RFT] mac80211 powersave rework Seth Forshee
                   ` (2 preceding siblings ...)
  2013-12-16 22:00 ` [RFC PATCH 3/8] mac80211: Add powersave module Seth Forshee
@ 2013-12-16 22:00 ` Seth Forshee
  2013-12-17  8:25   ` Johannes Berg
  2013-12-16 22:00 ` [RFC PATCH 5/8] mac80211: Don't start dynamic PS timer when leaving off-channel if still scanning Seth Forshee
                   ` (3 subsequent siblings)
  7 siblings, 1 reply; 17+ messages in thread
From: Seth Forshee @ 2013-12-16 22:00 UTC (permalink / raw)
  To: Johannes Berg
  Cc: linux-wireless, b43-dev, brcm80211-dev-list, John W. Linville,
	Stefano Brivio, Arend van Spriel, Seth Forshee

Convert the managed mode code to manipulate interface PS states
rather than changing the hw PS state directly. The off-channel
and scan code must be updated at the same time in order to avoid
conflicts with the PS module.

Signed-off-by: Seth Forshee <seth.forshee@canonical.com>
---
 net/mac80211/cfg.c         |   9 +--
 net/mac80211/ieee80211_i.h |  10 +--
 net/mac80211/iface.c       |   8 ++-
 net/mac80211/mlme.c        | 173 +++++++++++++++++----------------------------
 net/mac80211/offchannel.c  |  57 +++++----------
 net/mac80211/scan.c        |  14 ++++
 net/mac80211/status.c      |  13 ++--
 net/mac80211/tx.c          |  12 ++--
 net/mac80211/util.c        |  47 +++++++++---
 9 files changed, 159 insertions(+), 184 deletions(-)

diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c
index f80e8c4..40040ad 100644
--- a/net/mac80211/cfg.c
+++ b/net/mac80211/cfg.c
@@ -552,7 +552,7 @@ static void sta_set_sinfo(struct sta_info *sta, struct station_info *sinfo)
 		sinfo->bss_param.flags |= BSS_PARAM_FLAGS_SHORT_PREAMBLE;
 	if (sdata->vif.bss_conf.use_short_slot)
 		sinfo->bss_param.flags |= BSS_PARAM_FLAGS_SHORT_SLOT_TIME;
-	sinfo->bss_param.dtim_period = sdata->local->hw.conf.ps_dtim_period;
+	sinfo->bss_param.dtim_period = sdata->vif.ps_dtim_period;
 	sinfo->bss_param.beacon_interval = sdata->vif.bss_conf.beacon_int;
 
 	sinfo->sta_flags.set = 0;
@@ -1596,7 +1596,7 @@ static int ieee80211_change_station(struct wiphy *wiphy,
 
 	if (sdata->vif.type == NL80211_IFTYPE_STATION &&
 	    params->sta_flags_mask & BIT(NL80211_STA_FLAG_AUTHORIZED)) {
-		ieee80211_recalc_ps(local, -1);
+		ieee80211_recalc_ps(sdata);
 		ieee80211_recalc_ps_vif(sdata);
 	}
 
@@ -2532,10 +2532,7 @@ static int ieee80211_set_power_mgmt(struct wiphy *wiphy, struct net_device *dev,
 	__ieee80211_request_smps_mgd(sdata, sdata->u.mgd.req_smps);
 	sdata_unlock(sdata);
 
-	if (local->hw.flags & IEEE80211_HW_SUPPORTS_DYNAMIC_PS)
-		ieee80211_hw_config(local, IEEE80211_CONF_CHANGE_PS);
-
-	ieee80211_recalc_ps(local, -1);
+	ieee80211_recalc_ps(sdata);
 	ieee80211_recalc_ps_vif(sdata);
 
 	return 0;
diff --git a/net/mac80211/ieee80211_i.h b/net/mac80211/ieee80211_i.h
index a489676..4b0750f 100644
--- a/net/mac80211/ieee80211_i.h
+++ b/net/mac80211/ieee80211_i.h
@@ -394,6 +394,7 @@ struct ieee80211_if_managed {
 	struct work_struct csa_connection_drop_work;
 	struct work_struct dynamic_ps_enable_work;
 	struct work_struct dynamic_ps_disable_work;
+	enum ieee80211_vif_ps_mode offchannel_ps_mode;
 
 	unsigned long beacon_timeout;
 	unsigned long probe_timeout;
@@ -1186,12 +1187,6 @@ struct ieee80211_local {
 				*/
 
 	bool pspolling;
-	bool offchannel_ps_enabled;
-	/*
-	 * PS can only be enabled when we have exactly one managed
-	 * interface (and monitors) in PS, this then points there.
-	 */
-	struct ieee80211_sub_if_data *ps_sdata;
 	struct notifier_block network_latency_notifier;
 	struct notifier_block ifa_notifier;
 	struct notifier_block ifa6_notifier;
@@ -1362,7 +1357,8 @@ int ieee80211_mgd_disassoc(struct ieee80211_sub_if_data *sdata,
 			   struct cfg80211_disassoc_request *req);
 void ieee80211_send_pspoll(struct ieee80211_local *local,
 			   struct ieee80211_sub_if_data *sdata);
-void ieee80211_recalc_ps(struct ieee80211_local *local, s32 latency);
+void ieee80211_recalc_ps(struct ieee80211_sub_if_data *sdata);
+void ieee80211_mgd_recalc_ps(struct ieee80211_sub_if_data *sdata);
 void ieee80211_recalc_ps_vif(struct ieee80211_sub_if_data *sdata);
 void ieee80211_mgd_notify_rx(struct ieee80211_rx_data *rx);
 int ieee80211_max_network_latency(struct notifier_block *nb,
diff --git a/net/mac80211/iface.c b/net/mac80211/iface.c
index 784b651..6444a50 100644
--- a/net/mac80211/iface.c
+++ b/net/mac80211/iface.c
@@ -682,7 +682,7 @@ int ieee80211_do_open(struct wireless_dev *wdev, bool coming_up)
 	if (hw_reconf_flags)
 		ieee80211_hw_config(local, hw_reconf_flags);
 
-	ieee80211_recalc_ps(local, -1);
+	ieee80211_ps_vif_open(sdata);
 
 	if (sdata->vif.type == NL80211_IFTYPE_MONITOR ||
 	    sdata->vif.type == NL80211_IFTYPE_AP_VLAN) {
@@ -943,6 +943,8 @@ static void ieee80211_do_stop(struct ieee80211_sub_if_data *sdata,
 		return;
 	}
 
+	ieee80211_ps_vif_close(sdata);
+
 	switch (sdata->vif.type) {
 	case NL80211_IFTYPE_AP_VLAN:
 		break;
@@ -963,8 +965,6 @@ static void ieee80211_do_stop(struct ieee80211_sub_if_data *sdata,
 			drv_remove_interface(local, sdata);
 	}
 
-	ieee80211_recalc_ps(local, -1);
-
 	if (local->open_count == 0) {
 		ieee80211_stop_device(local);
 
@@ -1609,6 +1609,7 @@ int ieee80211_if_add(struct ieee80211_local *local, const char *name,
 		strlcpy(sdata->name, name, IFNAMSIZ);
 		ieee80211_assign_perm_addr(local, wdev->address, type);
 		memcpy(sdata->vif.addr, wdev->address, ETH_ALEN);
+		ieee80211_ps_init_vif(sdata);
 	} else {
 		if (local->hw.queues >= IEEE80211_NUM_ACS)
 			txqs = IEEE80211_NUM_ACS;
@@ -1644,6 +1645,7 @@ int ieee80211_if_add(struct ieee80211_local *local, const char *name,
 		ndev->ieee80211_ptr = &sdata->wdev;
 		memcpy(sdata->vif.addr, ndev->dev_addr, ETH_ALEN);
 		memcpy(sdata->name, ndev->name, IFNAMSIZ);
+		ieee80211_ps_init_vif(sdata);
 
 		sdata->dev = ndev;
 	}
diff --git a/net/mac80211/mlme.c b/net/mac80211/mlme.c
index 4ec8c0a..6deb080 100644
--- a/net/mac80211/mlme.c
+++ b/net/mac80211/mlme.c
@@ -1138,7 +1138,7 @@ static u32 ieee80211_handle_pwr_constr(struct ieee80211_sub_if_data *sdata,
 static void ieee80211_enable_ps(struct ieee80211_local *local,
 				struct ieee80211_sub_if_data *sdata)
 {
-	struct ieee80211_conf *conf = &local->hw.conf;
+	struct ieee80211_vif *vif = &sdata->vif;
 	struct ieee80211_if_managed *ifmgd = &sdata->u.mgd;
 
 	/*
@@ -1148,36 +1148,38 @@ static void ieee80211_enable_ps(struct ieee80211_local *local,
 	if (local->scanning)
 		return;
 
-	if (conf->dynamic_ps_timeout > 0 &&
+	vif->dynamic_ps_active = vif->dynamic_ps_timeout > 0;
+	if (vif->dynamic_ps_active &&
 	    !(local->hw.flags & IEEE80211_HW_SUPPORTS_DYNAMIC_PS)) {
 		mod_timer(&ifmgd->dynamic_ps_timer, jiffies +
-			  msecs_to_jiffies(conf->dynamic_ps_timeout));
+			  msecs_to_jiffies(vif->dynamic_ps_timeout));
 	} else {
-		if (local->hw.flags & IEEE80211_HW_PS_NULLFUNC_STACK)
+		if (local->hw.flags & IEEE80211_HW_PS_NULLFUNC_STACK) {
+			ieee80211_vif_set_ps_mode(sdata,
+						  IEEE80211_VIF_PS_AWAKE_PM);
 			ieee80211_send_nullfunc(local, sdata, 1);
+		}
 
 		if ((local->hw.flags & IEEE80211_HW_PS_NULLFUNC_STACK) &&
 		    (local->hw.flags & IEEE80211_HW_REPORTS_TX_ACK_STATUS))
 			return;
 
-		conf->flags |= IEEE80211_CONF_PS;
-		ieee80211_hw_config(local, IEEE80211_CONF_CHANGE_PS);
+		ieee80211_vif_set_ps_mode(sdata, IEEE80211_VIF_PS_DOZE);
 	}
 }
 
 static void ieee80211_change_ps(struct ieee80211_sub_if_data *sdata, bool ps_enable)
 {
 	struct ieee80211_local *local = sdata->local;
-	struct ieee80211_conf *conf = &local->hw.conf;
 	struct ieee80211_if_managed *ifmgd = &sdata->u.mgd;
 
 	if (ps_enable) {
 		ieee80211_enable_ps(local, sdata);
-	} else if (conf->flags & IEEE80211_CONF_PS) {
-		conf->flags &= ~IEEE80211_CONF_PS;
-		ieee80211_hw_config(local, IEEE80211_CONF_CHANGE_PS);
+	} else if (sdata->vif.ps_mode < IEEE80211_VIF_PS_AWAKE) {
 		del_timer_sync(&ifmgd->dynamic_ps_timer);
 		cancel_work_sync(&ifmgd->dynamic_ps_enable_work);
+		sdata->vif.dynamic_ps_active = false;
+		ieee80211_vif_set_ps_mode(sdata, IEEE80211_VIF_PS_AWAKE);
 	}
 }
 
@@ -1212,44 +1214,20 @@ static bool ieee80211_powersave_allowed(struct ieee80211_sub_if_data *sdata)
 }
 
 /* need to hold RTNL or interface lock */
-void ieee80211_recalc_ps(struct ieee80211_local *local, s32 latency)
+void ieee80211_mgd_recalc_ps(struct ieee80211_sub_if_data *sdata)
 {
-	struct ieee80211_sub_if_data *sdata, *old_ps_sdata, *found = NULL;
-	int count = 0;
-	int timeout;
-
-	if (!(local->hw.flags & IEEE80211_HW_SUPPORTS_PS)) {
-		local->ps_sdata = NULL;
-		return;
-	}
-
-	old_ps_sdata = local->ps_sdata;
-
-	list_for_each_entry(sdata, &local->interfaces, list) {
-		if (!ieee80211_sdata_running(sdata))
-			continue;
-		if (sdata->vif.type == NL80211_IFTYPE_AP) {
-			/* If an AP vif is found, then disable PS
-			 * by setting the count to zero thereby setting
-			 * ps_sdata to NULL.
-			 */
-			count = 0;
-			break;
-		}
-		if (sdata->vif.type != NL80211_IFTYPE_STATION)
-			continue;
-		found = sdata;
-		count++;
-	}
+	struct ieee80211_local *local = sdata->local;
+	bool ps_enable = false;
 
-	if (count == 1 && ieee80211_powersave_allowed(found)) {
+	if (local->hw.flags & IEEE80211_HW_SUPPORTS_PS &&
+	    ieee80211_powersave_allowed(sdata)) {
+		struct ieee80211_vif *vif = &sdata->vif;
+		int latency, timeout;
 		s32 beaconint_us;
 
-		if (latency < 0)
-			latency = pm_qos_request(PM_QOS_NETWORK_LATENCY);
-
+		latency = pm_qos_request(PM_QOS_NETWORK_LATENCY);
 		beaconint_us = ieee80211_tu_to_usec(
-					found->vif.bss_conf.beacon_int);
+					sdata->vif.bss_conf.beacon_int);
 
 		timeout = local->dynamic_ps_forced_timeout;
 		if (timeout < 0) {
@@ -1266,13 +1244,11 @@ void ieee80211_recalc_ps(struct ieee80211_local *local, s32 latency)
 			else
 				timeout = 100;
 		}
-		local->hw.conf.dynamic_ps_timeout = timeout;
+		vif->dynamic_ps_timeout = timeout;
 
-		if (beaconint_us > latency) {
-			local->ps_sdata = NULL;
-		} else {
+		if (latency >= beaconint_us) {
 			int maxslp = 1;
-			u8 dtimper = found->u.mgd.dtim_period;
+			u8 dtimper = sdata->u.mgd.dtim_period;
 
 			/* If the TIM IE is invalid, pretend the value is 1 */
 			if (!dtimper)
@@ -1281,18 +1257,13 @@ void ieee80211_recalc_ps(struct ieee80211_local *local, s32 latency)
 				maxslp = min_t(int, dtimper,
 						    latency / beaconint_us);
 
-			local->hw.conf.max_sleep_period = maxslp;
-			local->hw.conf.ps_dtim_period = dtimper;
-			local->ps_sdata = found;
+			vif->max_sleep_period = maxslp;
+			vif->ps_dtim_period = dtimper;
+			ps_enable = true;
 		}
-	} else {
-		local->ps_sdata = NULL;
 	}
 
-	if (local->ps_sdata)
-		ieee80211_change_ps(local->ps_sdata, true);
-	else if (old_ps_sdata)
-		ieee80211_change_ps(old_ps_sdata, false);
+	ieee80211_change_ps(sdata, ps_enable);
 }
 
 void ieee80211_recalc_ps_vif(struct ieee80211_sub_if_data *sdata)
@@ -1310,14 +1281,11 @@ static void ieee80211_dynamic_ps_disable_work(struct work_struct *work)
 	struct ieee80211_sub_if_data *sdata =
 		container_of(work, struct ieee80211_sub_if_data,
 			     u.mgd.dynamic_ps_disable_work);
-	struct ieee80211_local *local = sdata->local;
 
-	if (local->hw.conf.flags & IEEE80211_CONF_PS) {
-		local->hw.conf.flags &= ~IEEE80211_CONF_PS;
-		ieee80211_hw_config(local, IEEE80211_CONF_CHANGE_PS);
-	}
+	if (sdata->vif.ps_mode < IEEE80211_VIF_PS_AWAKE)
+		ieee80211_vif_set_ps_mode(sdata, IEEE80211_VIF_PS_AWAKE);
 
-	ieee80211_wake_queues_by_reason(&local->hw,
+	ieee80211_wake_queues_by_reason(&sdata->local->hw,
 					IEEE80211_MAX_QUEUE_MAP,
 					IEEE80211_QUEUE_STOP_REASON_PS);
 }
@@ -1330,21 +1298,23 @@ static void ieee80211_dynamic_ps_enable_work(struct work_struct *work)
 	struct ieee80211_local *local = sdata->local;
 	struct ieee80211_if_managed *ifmgd = &sdata->u.mgd;
 	unsigned long flags;
-	int q;
+	int q, n_queues;
 
-	/* can only happen when PS was just disabled anyway */
-	if (!local->ps_sdata)
+	if (!sdata->vif.dynamic_ps_active ||
+	    sdata->vif.ps_mode == IEEE80211_VIF_PS_DOZE)
 		return;
 
-	if (local->hw.conf.flags & IEEE80211_CONF_PS)
-		return;
-
-	if (local->hw.conf.dynamic_ps_timeout > 0) {
-		/* don't enter PS if TX frames are pending */
+	if (sdata->vif.dynamic_ps_timeout > 0) {
+		/*
+		 * don't enter PS if TX frames are pending
+		 *
+		 * XXX: Ideally we should be checking only for frames on
+		 * this interface.
+		 */
 		if (drv_tx_frames_pending(local)) {
 			mod_timer(&ifmgd->dynamic_ps_timer, jiffies +
 				  msecs_to_jiffies(
-				  local->hw.conf.dynamic_ps_timeout));
+				  sdata->vif.dynamic_ps_timeout));
 			return;
 		}
 
@@ -1353,14 +1323,18 @@ static void ieee80211_dynamic_ps_enable_work(struct work_struct *work)
 		 * dynamic_ps_timer expiry. Postpone the ps timer if it
 		 * is not the actual idle state.
 		 */
+		n_queues = IEEE80211_NUM_ACS;
+		if (local->hw.queues < n_queues)
+			n_queues = 1;
+
 		spin_lock_irqsave(&local->queue_stop_reason_lock, flags);
-		for (q = 0; q < local->hw.queues; q++) {
+		for (q = 0; q < n_queues; q++) {
 			if (local->queue_stop_reasons[q]) {
 				spin_unlock_irqrestore(&local->queue_stop_reason_lock,
 						       flags);
 				mod_timer(&ifmgd->dynamic_ps_timer, jiffies +
 					  msecs_to_jiffies(
-					  local->hw.conf.dynamic_ps_timeout));
+					  sdata->vif.dynamic_ps_timeout));
 				return;
 			}
 		}
@@ -1372,8 +1346,10 @@ static void ieee80211_dynamic_ps_enable_work(struct work_struct *work)
 		if (drv_tx_frames_pending(local)) {
 			mod_timer(&ifmgd->dynamic_ps_timer, jiffies +
 				  msecs_to_jiffies(
-				  local->hw.conf.dynamic_ps_timeout));
+				  sdata->vif.dynamic_ps_timeout));
 		} else {
+			ieee80211_vif_set_ps_mode(sdata,
+						  IEEE80211_VIF_PS_AWAKE_PM);
 			ieee80211_send_nullfunc(local, sdata, 1);
 			/* Flush to get the tx status of nullfunc frame */
 			ieee80211_flush_queues(local, sdata);
@@ -1384,8 +1360,7 @@ static void ieee80211_dynamic_ps_enable_work(struct work_struct *work)
 	      (local->hw.flags & IEEE80211_HW_PS_NULLFUNC_STACK)) ||
 	    (ifmgd->flags & IEEE80211_STA_NULLFUNC_ACKED)) {
 		ifmgd->flags &= ~IEEE80211_STA_NULLFUNC_ACKED;
-		local->hw.conf.flags |= IEEE80211_CONF_PS;
-		ieee80211_hw_config(local, IEEE80211_CONF_CHANGE_PS);
+		ieee80211_vif_set_ps_mode(sdata, IEEE80211_VIF_PS_DOZE);
 	}
 }
 
@@ -1403,15 +1378,16 @@ static void ieee80211_dynamic_ps_timer(unsigned long data)
 void ieee80211_mgd_notify_rx(struct ieee80211_rx_data *rx)
 {
 	struct ieee80211_sub_if_data *sdata = rx->sdata;
+	struct ieee80211_vif *vif = &sdata->vif;
 	struct ieee80211_local *local = rx->local;
 
-	if (local->ps_sdata && local->hw.conf.dynamic_ps_timeout > 0 &&
+	if (vif->dynamic_ps_active &&
 	    !is_multicast_ether_addr(
 		    ((struct ethhdr *)rx->skb->data)->h_dest) &&
 	    (!local->scanning &&
 	     !test_bit(SDATA_STATE_OFFCHANNEL, &sdata->state))) {
 			mod_timer(&sdata->u.mgd.dynamic_ps_timer, jiffies +
-			 msecs_to_jiffies(local->hw.conf.dynamic_ps_timeout));
+				  msecs_to_jiffies(vif->dynamic_ps_timeout));
 	}
 }
 
@@ -1660,7 +1636,7 @@ static void ieee80211_set_associated(struct ieee80211_sub_if_data *sdata,
 	ieee80211_bss_info_change_notify(sdata, bss_info_changed);
 
 	mutex_lock(&local->iflist_mtx);
-	ieee80211_recalc_ps(local, -1);
+	ieee80211_mgd_recalc_ps(sdata);
 	mutex_unlock(&local->iflist_mtx);
 
 	ieee80211_recalc_smps(sdata);
@@ -1695,11 +1671,8 @@ static void ieee80211_set_disassoc(struct ieee80211_sub_if_data *sdata,
 	 * to do it before sending disassoc, as otherwise the null-packet
 	 * won't be valid.
 	 */
-	if (local->hw.conf.flags & IEEE80211_CONF_PS) {
-		local->hw.conf.flags &= ~IEEE80211_CONF_PS;
-		ieee80211_hw_config(local, IEEE80211_CONF_CHANGE_PS);
-	}
-	local->ps_sdata = NULL;
+	sdata->vif.dynamic_ps_active = false;
+	ieee80211_vif_set_ps_mode(sdata, IEEE80211_VIF_PS_AWAKE);
 
 	/* disable per-vif ps */
 	ieee80211_recalc_ps_vif(sdata);
@@ -1804,7 +1777,7 @@ static void ieee80211_reset_ap_probe(struct ieee80211_sub_if_data *sdata)
 	__ieee80211_stop_poll(sdata);
 
 	mutex_lock(&local->iflist_mtx);
-	ieee80211_recalc_ps(local, -1);
+	ieee80211_mgd_recalc_ps(sdata);
 	mutex_unlock(&local->iflist_mtx);
 
 	if (sdata->local->hw.flags & IEEE80211_HW_CONNECTION_MONITOR)
@@ -1946,7 +1919,7 @@ static void ieee80211_mgd_probe_ap(struct ieee80211_sub_if_data *sdata,
 		goto out;
 
 	mutex_lock(&sdata->local->iflist_mtx);
-	ieee80211_recalc_ps(sdata->local, -1);
+	ieee80211_mgd_recalc_ps(sdata);
 	mutex_unlock(&sdata->local->iflist_mtx);
 
 	ifmgd->probe_send_count = 0;
@@ -2921,12 +2894,9 @@ static void ieee80211_rx_mgmt_beacon(struct ieee80211_sub_if_data *sdata,
 							elems.tim_len,
 							ifmgd->aid);
 		if (directed_tim) {
-			if (local->hw.conf.dynamic_ps_timeout > 0) {
-				if (local->hw.conf.flags & IEEE80211_CONF_PS) {
-					local->hw.conf.flags &= ~IEEE80211_CONF_PS;
-					ieee80211_hw_config(local,
-							    IEEE80211_CONF_CHANGE_PS);
-				}
+			if (sdata->vif.dynamic_ps_active) {
+				ieee80211_vif_set_ps_mode(sdata,
+							  IEEE80211_VIF_PS_AWAKE);
 				ieee80211_send_nullfunc(local, sdata, 0);
 			} else if (!local->pspolling && sdata->u.mgd.powersave) {
 				local->pspolling = true;
@@ -3015,7 +2985,7 @@ static void ieee80211_rx_mgmt_beacon(struct ieee80211_sub_if_data *sdata,
 		ifmgd->have_beacon = true;
 
 		mutex_lock(&local->iflist_mtx);
-		ieee80211_recalc_ps(local, -1);
+		ieee80211_mgd_recalc_ps(sdata);
 		mutex_unlock(&local->iflist_mtx);
 
 		ieee80211_recalc_ps_vif(sdata);
@@ -3570,21 +3540,6 @@ void ieee80211_mlme_notify_scan_completed(struct ieee80211_local *local)
 	rcu_read_unlock();
 }
 
-int ieee80211_max_network_latency(struct notifier_block *nb,
-				  unsigned long data, void *dummy)
-{
-	s32 latency_usec = (s32) data;
-	struct ieee80211_local *local =
-		container_of(nb, struct ieee80211_local,
-			     network_latency_notifier);
-
-	mutex_lock(&local->iflist_mtx);
-	ieee80211_recalc_ps(local, latency_usec);
-	mutex_unlock(&local->iflist_mtx);
-
-	return 0;
-}
-
 static u8 ieee80211_ht_vht_rx_chains(struct ieee80211_sub_if_data *sdata,
 				     struct cfg80211_bss *cbss)
 {
diff --git a/net/mac80211/offchannel.c b/net/mac80211/offchannel.c
index 2049a0a..5802c00 100644
--- a/net/mac80211/offchannel.c
+++ b/net/mac80211/offchannel.c
@@ -29,8 +29,6 @@ static void ieee80211_offchannel_ps_enable(struct ieee80211_sub_if_data *sdata)
 	struct ieee80211_local *local = sdata->local;
 	struct ieee80211_if_managed *ifmgd = &sdata->u.mgd;
 
-	local->offchannel_ps_enabled = false;
-
 	/* FIXME: what to do when local->pspolling is true? */
 
 	del_timer_sync(&ifmgd->dynamic_ps_timer);
@@ -39,13 +37,9 @@ static void ieee80211_offchannel_ps_enable(struct ieee80211_sub_if_data *sdata)
 
 	cancel_work_sync(&ifmgd->dynamic_ps_enable_work);
 
-	if (local->hw.conf.flags & IEEE80211_CONF_PS) {
-		local->offchannel_ps_enabled = true;
-		local->hw.conf.flags &= ~IEEE80211_CONF_PS;
-		ieee80211_hw_config(local, IEEE80211_CONF_CHANGE_PS);
-	}
-
-	if (!local->offchannel_ps_enabled ||
+	ifmgd->offchannel_ps_mode = sdata->vif.ps_mode;
+	ieee80211_vif_set_ps_mode(sdata, IEEE80211_VIF_PS_AWAKE_PM);
+	if (ifmgd->offchannel_ps_mode != IEEE80211_VIF_PS_DOZE ||
 	    !(local->hw.flags & IEEE80211_HW_PS_NULLFUNC_STACK))
 		/*
 		 * If power save was enabled, no need to send a nullfunc
@@ -64,38 +58,23 @@ static void ieee80211_offchannel_ps_enable(struct ieee80211_sub_if_data *sdata)
 static void ieee80211_offchannel_ps_disable(struct ieee80211_sub_if_data *sdata)
 {
 	struct ieee80211_local *local = sdata->local;
+	struct ieee80211_if_managed *ifmgd = &sdata->u.mgd;
+	enum ieee80211_vif_ps_mode mode = ifmgd->offchannel_ps_mode;
 
-	if (!local->ps_sdata)
-		ieee80211_send_nullfunc(local, sdata, 0);
-	else if (local->offchannel_ps_enabled) {
-		/*
-		 * In !IEEE80211_HW_PS_NULLFUNC_STACK case the hardware
-		 * will send a nullfunc frame with the powersave bit set
-		 * even though the AP already knows that we are sleeping.
-		 * This could be avoided by sending a null frame with power
-		 * save bit disabled before enabling the power save, but
-		 * this doesn't gain anything.
-		 *
-		 * When IEEE80211_HW_PS_NULLFUNC_STACK is enabled, no need
-		 * to send a nullfunc frame because AP already knows that
-		 * we are sleeping, let's just enable power save mode in
-		 * hardware.
-		 */
-		/* TODO:  Only set hardware if CONF_PS changed?
-		 * TODO:  Should we set offchannel_ps_enabled to false?
-		 */
-		local->hw.conf.flags |= IEEE80211_CONF_PS;
-		ieee80211_hw_config(local, IEEE80211_CONF_CHANGE_PS);
-	} else if (local->hw.conf.dynamic_ps_timeout > 0) {
-		/*
-		 * If IEEE80211_CONF_PS was not set and the dynamic_ps_timer
-		 * had been running before leaving the operating channel,
-		 * restart the timer now and send a nullfunc frame to inform
-		 * the AP that we are awake.
-		 */
+	/*
+	 * If mode is AWAKE_PM we may have started off-channel during a
+	 * transition to powersave. The AP should already think we're in
+	 * powersave, so go straight to DOZE.
+	 */
+	if (mode == IEEE80211_VIF_PS_AWAKE_PM)
+		mode = IEEE80211_VIF_PS_DOZE;
+
+	ieee80211_vif_set_ps_mode(sdata, mode);
+	if (mode == IEEE80211_VIF_PS_AWAKE) {
 		ieee80211_send_nullfunc(local, sdata, 0);
-		mod_timer(&sdata->u.mgd.dynamic_ps_timer, jiffies +
-			  msecs_to_jiffies(local->hw.conf.dynamic_ps_timeout));
+		if (sdata->vif.dynamic_ps_active)
+			mod_timer(&ifmgd->dynamic_ps_timer, jiffies +
+				  msecs_to_jiffies(sdata->vif.dynamic_ps_timeout));
 	}
 
 	ieee80211_sta_reset_beacon_monitor(sdata);
diff --git a/net/mac80211/scan.c b/net/mac80211/scan.c
index 4d73c46..b0e538e 100644
--- a/net/mac80211/scan.c
+++ b/net/mac80211/scan.c
@@ -345,6 +345,8 @@ EXPORT_SYMBOL(ieee80211_scan_completed);
 
 static int ieee80211_start_sw_scan(struct ieee80211_local *local)
 {
+	struct ieee80211_sub_if_data *sdata;
+
 	/* Software scan is not supported in multi-channel cases */
 	if (local->use_chanctx)
 		return -EOPNOTSUPP;
@@ -378,6 +380,11 @@ static int ieee80211_start_sw_scan(struct ieee80211_local *local)
 	/* We need to set power level at maximum rate for scanning. */
 	ieee80211_hw_config(local, 0);
 
+	sdata = rcu_dereference_protected(local->scan_sdata,
+					  lockdep_is_held(&local->mtx));
+	if (sdata && sdata->vif.ps_mode != IEEE80211_VIF_PS_AWAKE)
+		ieee80211_vif_set_ps_mode(sdata, IEEE80211_VIF_PS_AWAKE);
+
 	ieee80211_queue_delayed_work(&local->hw,
 				     &local->scan_work, 0);
 
@@ -726,6 +733,8 @@ static void ieee80211_scan_state_suspend(struct ieee80211_local *local,
 static void ieee80211_scan_state_resume(struct ieee80211_local *local,
 					unsigned long *next_delay)
 {
+	struct ieee80211_sub_if_data *sdata;
+
 	ieee80211_offchannel_stop_vifs(local);
 
 	if (local->ops->flush) {
@@ -737,6 +746,11 @@ static void ieee80211_scan_state_resume(struct ieee80211_local *local,
 	/* remember when we left the operating channel */
 	local->leave_oper_channel_time = jiffies;
 
+	sdata = rcu_dereference_protected(local->scan_sdata,
+					  lockdep_is_held(&local->mtx));
+	if (sdata && sdata->vif.ps_mode != IEEE80211_VIF_PS_AWAKE)
+		ieee80211_vif_set_ps_mode(sdata, IEEE80211_VIF_PS_AWAKE);
+
 	/* advance to the next channel to be scanned */
 	local->next_scan_state = SCAN_SET_CHANNEL;
 }
diff --git a/net/mac80211/status.c b/net/mac80211/status.c
index 3298fe9..5e001e0 100644
--- a/net/mac80211/status.c
+++ b/net/mac80211/status.c
@@ -732,15 +732,16 @@ void ieee80211_tx_status(struct ieee80211_hw *hw, struct sk_buff *skb)
 			local->dot11FailedCount++;
 	}
 
+	sdata = IEEE80211_DEV_TO_SUB_IF(skb->dev);
 	if (ieee80211_is_nullfunc(fc) && ieee80211_has_pm(fc) &&
 	    (local->hw.flags & IEEE80211_HW_REPORTS_TX_ACK_STATUS) &&
 	    !(info->flags & IEEE80211_TX_CTL_INJECTED) &&
-	    local->ps_sdata && !(local->scanning)) {
-		if (info->flags & IEEE80211_TX_STAT_ACK) {
-			local->ps_sdata->u.mgd.flags |=
-					IEEE80211_STA_NULLFUNC_ACKED;
-		} else
-			mod_timer(&local->ps_sdata->u.mgd.dynamic_ps_timer,
+	    sdata->vif.ps_mode < IEEE80211_VIF_PS_AWAKE &&
+	    !(local->scanning)) {
+		if (info->flags & IEEE80211_TX_STAT_ACK)
+			sdata->u.mgd.flags |= IEEE80211_STA_NULLFUNC_ACKED;
+		else
+			mod_timer(&sdata->u.mgd.dynamic_ps_timer,
 				  jiffies + msecs_to_jiffies(10));
 	}
 
diff --git a/net/mac80211/tx.c b/net/mac80211/tx.c
index 0ffc2066..995fae4 100644
--- a/net/mac80211/tx.c
+++ b/net/mac80211/tx.c
@@ -198,6 +198,7 @@ static ieee80211_tx_result debug_noinline
 ieee80211_tx_h_dynamic_ps(struct ieee80211_tx_data *tx)
 {
 	struct ieee80211_local *local = tx->local;
+	struct ieee80211_sub_if_data *sdata = tx->sdata;
 	struct ieee80211_if_managed *ifmgd;
 
 	/* driver doesn't support power save */
@@ -209,14 +210,15 @@ ieee80211_tx_h_dynamic_ps(struct ieee80211_tx_data *tx)
 		return TX_CONTINUE;
 
 	/* dynamic power save disabled */
-	if (local->hw.conf.dynamic_ps_timeout <= 0)
+	if (sdata->vif.dynamic_ps_timeout <= 0)
 		return TX_CONTINUE;
 
 	/* we are scanning, don't enable power save */
 	if (local->scanning)
 		return TX_CONTINUE;
 
-	if (!local->ps_sdata)
+	if (!sdata->vif.dynamic_ps_active &&
+	    sdata->vif.ps_mode == IEEE80211_VIF_PS_AWAKE)
 		return TX_CONTINUE;
 
 	/* No point if we're going to suspend */
@@ -227,7 +229,7 @@ ieee80211_tx_h_dynamic_ps(struct ieee80211_tx_data *tx)
 	if (tx->sdata->vif.type != NL80211_IFTYPE_STATION)
 		return TX_CONTINUE;
 
-	ifmgd = &tx->sdata->u.mgd;
+	ifmgd = &sdata->u.mgd;
 
 	/*
 	 * Don't wakeup from power save if u-apsd is enabled, voip ac has
@@ -247,7 +249,7 @@ ieee80211_tx_h_dynamic_ps(struct ieee80211_tx_data *tx)
 	    skb_get_queue_mapping(tx->skb) == IEEE80211_AC_VO)
 		return TX_CONTINUE;
 
-	if (local->hw.conf.flags & IEEE80211_CONF_PS) {
+	if (sdata->vif.ps_mode == IEEE80211_VIF_PS_DOZE) {
 		ieee80211_stop_queues_by_reason(&local->hw,
 						IEEE80211_MAX_QUEUE_MAP,
 						IEEE80211_QUEUE_STOP_REASON_PS);
@@ -261,7 +263,7 @@ ieee80211_tx_h_dynamic_ps(struct ieee80211_tx_data *tx)
 		return TX_CONTINUE;
 
 	mod_timer(&ifmgd->dynamic_ps_timer, jiffies +
-		  msecs_to_jiffies(local->hw.conf.dynamic_ps_timeout));
+		  msecs_to_jiffies(sdata->vif.dynamic_ps_timeout));
 
 	return TX_CONTINUE;
 }
diff --git a/net/mac80211/util.c b/net/mac80211/util.c
index 010cd2c..9542295 100644
--- a/net/mac80211/util.c
+++ b/net/mac80211/util.c
@@ -1664,7 +1664,7 @@ int ieee80211_reconfig(struct ieee80211_local *local)
 		}
 	}
 
-	ieee80211_recalc_ps(local, -1);
+	ieee80211_recalc_ps(sdata);
 
 	/*
 	 * The sta might be in psm against the ap (e.g. because
@@ -1672,15 +1672,15 @@ int ieee80211_reconfig(struct ieee80211_local *local)
 	 * explicitly send a null packet in order to make sure
 	 * it'll sync against the ap (and get out of psm).
 	 */
-	if (!(local->hw.conf.flags & IEEE80211_CONF_PS)) {
-		list_for_each_entry(sdata, &local->interfaces, list) {
-			if (sdata->vif.type != NL80211_IFTYPE_STATION)
-				continue;
-			if (!sdata->u.mgd.associated)
-				continue;
+	list_for_each_entry(sdata, &local->interfaces, list) {
+		if (sdata->vif.type != NL80211_IFTYPE_STATION)
+			continue;
+		if (!sdata->u.mgd.associated)
+			continue;
+		if (sdata->vif.ps_mode != IEEE80211_VIF_PS_AWAKE)
+			continue;
 
-			ieee80211_send_nullfunc(local, sdata, 0);
-		}
+		ieee80211_send_nullfunc(local, sdata, 0);
 	}
 
 	/* APs are now beaconing, add back stations */
@@ -1795,6 +1795,17 @@ void ieee80211_resume_disconnect(struct ieee80211_vif *vif)
 }
 EXPORT_SYMBOL_GPL(ieee80211_resume_disconnect);
 
+void ieee80211_recalc_ps(struct ieee80211_sub_if_data *sdata)
+{
+	switch (sdata->vif.type) {
+	case NL80211_IFTYPE_STATION:
+		ieee80211_mgd_recalc_ps(sdata);
+		break;
+	default:
+		break;
+	}
+}
+
 void ieee80211_recalc_smps(struct ieee80211_sub_if_data *sdata)
 {
 	struct ieee80211_local *local = sdata->local;
@@ -1835,6 +1846,24 @@ void ieee80211_recalc_min_chandef(struct ieee80211_sub_if_data *sdata)
 	mutex_unlock(&local->chanctx_mtx);
 }
 
+int ieee80211_max_network_latency(struct notifier_block *nb,
+				  unsigned long data, void *dummy)
+{
+	struct ieee80211_local *local =
+		container_of(nb, struct ieee80211_local,
+			     network_latency_notifier);
+	struct ieee80211_sub_if_data *sdata;
+
+	mutex_lock(&local->iflist_mtx);
+	list_for_each_entry_rcu(sdata, &local->interfaces, list) {
+		if (ieee80211_sdata_running(sdata))
+			ieee80211_recalc_ps(sdata);
+	}
+	mutex_unlock(&local->iflist_mtx);
+
+	return 0;
+}
+
 static bool ieee80211_id_in_list(const u8 *ids, int n_ids, u8 id)
 {
 	int i;
-- 
1.8.3.2

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [RFC PATCH 5/8] mac80211: Don't start dynamic PS timer when leaving off-channel if still scanning
  2013-12-16 22:00 [RFC/RFT] mac80211 powersave rework Seth Forshee
                   ` (3 preceding siblings ...)
  2013-12-16 22:00 ` [RFC PATCH 4/8] mac80211: Use PS module for managed mode powersave Seth Forshee
@ 2013-12-16 22:00 ` Seth Forshee
  2013-12-16 22:00 ` [RFC PATCH 6/8] brcmsmac: Set MCTL_HPS when PM should be set Seth Forshee
                   ` (2 subsequent siblings)
  7 siblings, 0 replies; 17+ messages in thread
From: Seth Forshee @ 2013-12-16 22:00 UTC (permalink / raw)
  To: Johannes Berg
  Cc: linux-wireless, b43-dev, brcm80211-dev-list, John W. Linville,
	Stefano Brivio, Arend van Spriel, Seth Forshee

Most of mac80211 does not update the dynamic PS timer during
scans, but ieee80211_offchannel_ps_disable() will still restart
it if a scan is in progress. It doesn't make sense to do this if
no other code is updating the timer, so update this function to
only restart the timer if not scanning.

Signed-off-by: Seth Forshee <seth.forshee@canonical.com>
---
 net/mac80211/offchannel.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/mac80211/offchannel.c b/net/mac80211/offchannel.c
index 5802c00..1744886 100644
--- a/net/mac80211/offchannel.c
+++ b/net/mac80211/offchannel.c
@@ -72,7 +72,7 @@ static void ieee80211_offchannel_ps_disable(struct ieee80211_sub_if_data *sdata)
 	ieee80211_vif_set_ps_mode(sdata, mode);
 	if (mode == IEEE80211_VIF_PS_AWAKE) {
 		ieee80211_send_nullfunc(local, sdata, 0);
-		if (sdata->vif.dynamic_ps_active)
+		if (sdata->vif.dynamic_ps_active && !local->scanning)
 			mod_timer(&ifmgd->dynamic_ps_timer, jiffies +
 				  msecs_to_jiffies(sdata->vif.dynamic_ps_timeout));
 	}
-- 
1.8.3.2

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [RFC PATCH 6/8] brcmsmac: Set MCTL_HPS when PM should be set
  2013-12-16 22:00 [RFC/RFT] mac80211 powersave rework Seth Forshee
                   ` (4 preceding siblings ...)
  2013-12-16 22:00 ` [RFC PATCH 5/8] mac80211: Don't start dynamic PS timer when leaving off-channel if still scanning Seth Forshee
@ 2013-12-16 22:00 ` Seth Forshee
  2013-12-16 22:00 ` [RFC PATCH 7/8] b43: Allow HWPS state to be changed Seth Forshee
  2013-12-16 22:01 ` [RFC PATCH 8/8] b43: Set B43_MACCTL_HWPS when PM should be set Seth Forshee
  7 siblings, 0 replies; 17+ messages in thread
From: Seth Forshee @ 2013-12-16 22:00 UTC (permalink / raw)
  To: Johannes Berg
  Cc: linux-wireless, b43-dev, brcm80211-dev-list, John W. Linville,
	Stefano Brivio, Arend van Spriel, Seth Forshee

Even though powersave is not supported MCTL_HPS still needs to be
set in order to transmit nullfunc frames with PM set. Add support
for the change_ps callback to set this bit whenever the powersave
mode requires PM to be set.

Signed-off-by: Seth Forshee <seth.forshee@canonical.com>
---
 drivers/net/wireless/brcm80211/brcmsmac/mac80211_if.c |  7 +++++++
 drivers/net/wireless/brcm80211/brcmsmac/main.c        | 17 ++++++++++-------
 drivers/net/wireless/brcm80211/brcmsmac/main.h        |  1 +
 3 files changed, 18 insertions(+), 7 deletions(-)

diff --git a/drivers/net/wireless/brcm80211/brcmsmac/mac80211_if.c b/drivers/net/wireless/brcm80211/brcmsmac/mac80211_if.c
index e71ce8c..e6a8c72 100644
--- a/drivers/net/wireless/brcm80211/brcmsmac/mac80211_if.c
+++ b/drivers/net/wireless/brcm80211/brcmsmac/mac80211_if.c
@@ -734,6 +734,12 @@ brcms_ops_bss_info_changed(struct ieee80211_hw *hw,
 	return;
 }
 
+void brcms_ops_change_ps(struct ieee80211_hw *hw, struct ieee80211_vif *vif)
+{
+	struct brcms_info *wl = hw->priv;
+	brcms_c_set_ps_ctrl(wl->wlc, vif);
+}
+
 static void
 brcms_ops_configure_filter(struct ieee80211_hw *hw,
 			unsigned int changed_flags,
@@ -944,6 +950,7 @@ static const struct ieee80211_ops brcms_ops = {
 	.remove_interface = brcms_ops_remove_interface,
 	.config = brcms_ops_config,
 	.bss_info_changed = brcms_ops_bss_info_changed,
+	.change_ps = brcms_ops_change_ps,
 	.configure_filter = brcms_ops_configure_filter,
 	.sw_scan_start = brcms_ops_sw_scan_start,
 	.sw_scan_complete = brcms_ops_sw_scan_complete,
diff --git a/drivers/net/wireless/brcm80211/brcmsmac/main.c b/drivers/net/wireless/brcm80211/brcmsmac/main.c
index 8138f1c..5d4dabb 100644
--- a/drivers/net/wireless/brcm80211/brcmsmac/main.c
+++ b/drivers/net/wireless/brcm80211/brcmsmac/main.c
@@ -3072,10 +3072,13 @@ static void brcms_b_antsel_set(struct brcms_hardware *wlc_hw, u32 antsel_avail)
  * conditions under which the PM bit should be set in outgoing frames
  * and STAY_AWAKE is meaningful
  */
-static bool brcms_c_ps_allowed(struct brcms_c_info *wlc)
+static bool brcms_c_ps_allowed(struct brcms_c_info *wlc,
+			       struct ieee80211_vif *vif)
 {
-	/* not supporting PS so always return false for now */
-	return false;
+	if (!vif)
+		return false;
+	return vif->ps_mode == IEEE80211_VIF_PS_DOZE ||
+	       vif->ps_mode == IEEE80211_VIF_PS_AWAKE_PM;
 }
 
 static void brcms_c_statsupd(struct brcms_c_info *wlc)
@@ -3748,13 +3751,13 @@ brcms_c_duty_cycle_set(struct brcms_c_info *wlc, int duty_cycle, bool isOFDM,
 }
 
 /* push sw hps and wake state through hardware */
-static void brcms_c_set_ps_ctrl(struct brcms_c_info *wlc)
+void brcms_c_set_ps_ctrl(struct brcms_c_info *wlc, struct ieee80211_vif *vif)
 {
 	u32 v1, v2;
 	bool hps;
 	bool awake_before;
 
-	hps = brcms_c_ps_allowed(wlc);
+	hps = brcms_c_ps_allowed(wlc, vif);
 
 	brcms_dbg_mac80211(wlc->hw->d11core, "wl%d: hps %d\n", wlc->pub->unit,
 			   hps);
@@ -3901,7 +3904,7 @@ static void brcms_c_setband(struct brcms_c_info *wlc,
 		return;
 
 	/* wait for at least one beacon before entering sleeping state */
-	brcms_c_set_ps_ctrl(wlc);
+	brcms_c_set_ps_ctrl(wlc, NULL);
 
 	/* band-specific initializations */
 	brcms_c_bsinit(wlc);
@@ -7938,7 +7941,7 @@ void brcms_c_init(struct brcms_c_info *wlc, bool mute_tx)
 			     bi << CFPREP_CBI_SHIFT);
 
 		/* Update maccontrol PM related bits */
-		brcms_c_set_ps_ctrl(wlc);
+		brcms_c_set_ps_ctrl(wlc, NULL);
 	}
 
 	brcms_c_bandinit_ordered(wlc, chanspec);
diff --git a/drivers/net/wireless/brcm80211/brcmsmac/main.h b/drivers/net/wireless/brcm80211/brcmsmac/main.h
index c4d135c..58abbe3 100644
--- a/drivers/net/wireless/brcm80211/brcmsmac/main.h
+++ b/drivers/net/wireless/brcm80211/brcmsmac/main.h
@@ -622,6 +622,7 @@ int brcms_b_xmtfifo_sz_get(struct brcms_hardware *wlc_hw, uint fifo,
 
 int brcms_c_set_gmode(struct brcms_c_info *wlc, u8 gmode, bool config);
 void brcms_c_mac_promisc(struct brcms_c_info *wlc, uint filter_flags);
+void brcms_c_set_ps_ctrl(struct brcms_c_info *wlc, struct ieee80211_vif *vif);
 u16 brcms_c_calc_lsig_len(struct brcms_c_info *wlc, u32 ratespec, uint mac_len);
 u32 brcms_c_rspec_to_rts_rspec(struct brcms_c_info *wlc, u32 rspec,
 			       bool use_rspec, u16 mimo_ctlchbw);
-- 
1.8.3.2

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [RFC PATCH 7/8] b43: Allow HWPS state to be changed
  2013-12-16 22:00 [RFC/RFT] mac80211 powersave rework Seth Forshee
                   ` (5 preceding siblings ...)
  2013-12-16 22:00 ` [RFC PATCH 6/8] brcmsmac: Set MCTL_HPS when PM should be set Seth Forshee
@ 2013-12-16 22:00 ` Seth Forshee
  2013-12-16 22:01 ` [RFC PATCH 8/8] b43: Set B43_MACCTL_HWPS when PM should be set Seth Forshee
  7 siblings, 0 replies; 17+ messages in thread
From: Seth Forshee @ 2013-12-16 22:00 UTC (permalink / raw)
  To: Johannes Berg
  Cc: linux-wireless, b43-dev, brcm80211-dev-list, John W. Linville,
	Stefano Brivio, Arend van Spriel, Seth Forshee

During background scans mac80211 wants to set powersave at the AP
so that frames will be buffered while off-channel. In order to do
this on Broadcom chips B43_MACCTL_HWPS needs to be set when
transmitting the nullfunc frames to enable powersave, however
b43_power_saving_ctl_bits() doesn't allow it to be set.

Rework this function a bit to allow setting HWPS. First,
initialize the hwps and awake variables based off the macctl
register to avoid changing the unintentionally changing the
values. It's not really necessary to set awake since it is later
forced to true, but it does no harm and might help avoid future
problems.

Second, remove the force set of hwps to false. No code currently tries
to change the hwps state anyway, so this results in no functional
change.

Signed-off-by: Seth Forshee <seth.forshee@canonical.com>
---
 drivers/net/wireless/b43/main.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/net/wireless/b43/main.c b/drivers/net/wireless/b43/main.c
index ccd24f0a..bc6ba1c 100644
--- a/drivers/net/wireless/b43/main.c
+++ b/drivers/net/wireless/b43/main.c
@@ -1117,6 +1117,10 @@ void b43_power_saving_ctl_bits(struct b43_wldev *dev, unsigned int ps_flags)
 		    (ps_flags & B43_PS_DISABLED));
 	B43_WARN_ON((ps_flags & B43_PS_AWAKE) && (ps_flags & B43_PS_ASLEEP));
 
+	macctl = b43_read32(dev, B43_MMIO_MACCTL);
+	hwps = !!(macctl & B43_MACCTL_HWPS);
+	awake = !!(macctl & B43_MACCTL_AWAKE);
+
 	if (ps_flags & B43_PS_ENABLED) {
 		hwps = true;
 	} else if (ps_flags & B43_PS_DISABLED) {
@@ -1135,11 +1139,9 @@ void b43_power_saving_ctl_bits(struct b43_wldev *dev, unsigned int ps_flags)
 		//      successful, set bit26
 	}
 
-/* FIXME: For now we force awake-on and hwps-off */
-	hwps = false;
+/* FIXME: For now we force awake-on */
 	awake = true;
 
-	macctl = b43_read32(dev, B43_MMIO_MACCTL);
 	if (hwps)
 		macctl |= B43_MACCTL_HWPS;
 	else
-- 
1.8.3.2

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [RFC PATCH 8/8] b43: Set B43_MACCTL_HWPS when PM should be set
  2013-12-16 22:00 [RFC/RFT] mac80211 powersave rework Seth Forshee
                   ` (6 preceding siblings ...)
  2013-12-16 22:00 ` [RFC PATCH 7/8] b43: Allow HWPS state to be changed Seth Forshee
@ 2013-12-16 22:01 ` Seth Forshee
  7 siblings, 0 replies; 17+ messages in thread
From: Seth Forshee @ 2013-12-16 22:01 UTC (permalink / raw)
  To: Johannes Berg
  Cc: linux-wireless, b43-dev, brcm80211-dev-list, John W. Linville,
	Stefano Brivio, Arend van Spriel, Seth Forshee

Even though powersave is not supported the HWPS bit still needs
to be set in order to transmit nullfunc frames with PM set. Add
support for the change_ps callback to set this bit whenever the
powersave mode requires PM to be set.

Signed-off-by: Seth Forshee <seth.forshee@canonical.com>
---
 drivers/net/wireless/b43/main.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/drivers/net/wireless/b43/main.c b/drivers/net/wireless/b43/main.c
index bc6ba1c..d883160 100644
--- a/drivers/net/wireless/b43/main.c
+++ b/drivers/net/wireless/b43/main.c
@@ -4030,6 +4030,21 @@ out_unlock_mutex:
 	mutex_unlock(&wl->mutex);
 }
 
+static void b43_op_change_ps(struct ieee80211_hw *hw,
+			     struct ieee80211_vif *vif)
+{
+	struct b43_wl *wl = hw_to_b43_wl(hw);
+	unsigned int ps_flags;
+
+	if (vif->ps_mode == IEEE80211_VIF_PS_DOZE ||
+	    vif->ps_mode == IEEE80211_VIF_PS_AWAKE_PM)
+		ps_flags = B43_PS_ENABLED;
+	else
+		ps_flags = B43_PS_DISABLED;
+
+	b43_power_saving_ctl_bits(wl->current_dev, ps_flags);
+}
+
 static int b43_op_set_key(struct ieee80211_hw *hw, enum set_key_cmd cmd,
 			  struct ieee80211_vif *vif, struct ieee80211_sta *sta,
 			  struct ieee80211_key_conf *key)
@@ -5031,6 +5046,7 @@ static const struct ieee80211_ops b43_hw_ops = {
 	.remove_interface	= b43_op_remove_interface,
 	.config			= b43_op_config,
 	.bss_info_changed	= b43_op_bss_info_changed,
+	.change_ps		= b43_op_change_ps,
 	.configure_filter	= b43_op_configure_filter,
 	.set_key		= b43_op_set_key,
 	.update_tkip_key	= b43_op_update_tkip_key,
-- 
1.8.3.2

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [RFC PATCH 1/8] mac80211: Move dynamic PS data out of common code
  2013-12-16 22:00 ` [RFC PATCH 1/8] mac80211: Move dynamic PS data out of common code Seth Forshee
@ 2013-12-17  8:08   ` Johannes Berg
  2013-12-17 12:37     ` Seth Forshee
  0 siblings, 1 reply; 17+ messages in thread
From: Johannes Berg @ 2013-12-17  8:08 UTC (permalink / raw)
  To: Seth Forshee
  Cc: linux-wireless, b43-dev, brcm80211-dev-list, John W. Linville,
	Stefano Brivio, Arend van Spriel

On Mon, 2013-12-16 at 16:00 -0600, Seth Forshee wrote:

> +void ieee80211_notify_rx(struct ieee80211_rx_data *rx)
> +{
> +	switch(rx->sdata->vif.type) {
> +	case NL80211_IFTYPE_MONITOR:
> +		ieee80211_mgd_notify_rx(rx);

This ... seems wrong.

Also, we already have ieee80211_sta_rx_notify() which you want, I think?

With this change, do you even still need local->ps_sdata?

johannes

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [RFC PATCH 2/8] mac80211: Add per-interface powersave states and parameters
  2013-12-16 22:00 ` [RFC PATCH 2/8] mac80211: Add per-interface powersave states and parameters Seth Forshee
@ 2013-12-17  8:11   ` Johannes Berg
  2013-12-17 13:12     ` Seth Forshee
  0 siblings, 1 reply; 17+ messages in thread
From: Johannes Berg @ 2013-12-17  8:11 UTC (permalink / raw)
  To: Seth Forshee
  Cc: linux-wireless, b43-dev, brcm80211-dev-list, John W. Linville,
	Stefano Brivio, Arend van Spriel

On Mon, 2013-12-16 at 16:00 -0600, Seth Forshee wrote:
> In preparation for managing powersave states on a per-interface
> basis, add powersave states and parameters to the interface-
> specific data structures. Also add a change_ps driver callback
> to notify drivers about changes to interface powersave states.
> 
> The new members and callback are unused here but will be
> utilized by subsequent commits.

Can't say I like that part much, it just makes it harder to review.

>  /**
> + * enum ieee80211_vif_ps_mode - virtual interface power save mode
> + *
> + * Ordered in terms of increasing wakefulness.
> + *
> + * @IEEE80211_VIF_PS_INACTIVE: the interface is not currently open
> + * @IEEE80211_VIF_PS_DOZE: the interface is in a low-power mode and may
> + *	not be able to transmit or receive frames
> + * @IEEE80211_VIF_PS_AWAKE_PM: the interface is awake and able to transmit
> + *	and receive frames but PM may be set in frame control
> + * @IEEE80211_VIF_PS_AWAKE: the interface is fully awake and able to
> + *	transmit and receive frames
> + */
> +enum ieee80211_vif_ps_mode {
> +	IEEE80211_VIF_PS_INACTIVE,

Does this INACTIVE thing really make sense? It should just be undefined
if it's not associated, no? Doing this might make people want to rely on
this to indicate associated-ness or something...

> +	IEEE80211_VIF_PS_AWAKE_PM,
> +	IEEE80211_VIF_PS_AWAKE,

The distinction between these seems somewhat unclear? "PM may be set"?

> + * @ps_mode: power save mode of this vif
> + * @dynamic_ps_active: indicates whether dynamic PS is active for this vif
> + * @dynamic_ps_timeout: The dynamic powersave timeout (in ms), see the
> + *	powersave documentation below. This variable is valid only when
> + *	the interface is in the doze state.
> + * @max_sleep_period: the maximum number of beacon intervals to sleep for
> + *	before checking the beacon for a TIM bit (managed mode only); this
> + *	value will be only achievable between DTIM frames, the hardware
> + *	needs to check for the multicast traffic bit in DTIM beacons.
> + *	This variable is valid only when the interface is in the doze state.
> + * @ps_dtim_period: The DTIM period of the AP we're connected to, for use
> + *	in power saving. Power saving will not be enabled until a beacon
> + *	has been received and the DTIM period is known.

Should these really be in the vif struct? They still seem somewhat
internal to the implementation.

johannes

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [RFC PATCH 3/8] mac80211: Add powersave module
  2013-12-16 22:00 ` [RFC PATCH 3/8] mac80211: Add powersave module Seth Forshee
@ 2013-12-17  8:16   ` Johannes Berg
  2013-12-17 13:31     ` Seth Forshee
  0 siblings, 1 reply; 17+ messages in thread
From: Johannes Berg @ 2013-12-17  8:16 UTC (permalink / raw)
  To: Seth Forshee
  Cc: linux-wireless, b43-dev, brcm80211-dev-list, John W. Linville,
	Stefano Brivio, Arend van Spriel

On Mon, 2013-12-16 at 16:00 -0600, Seth Forshee wrote:

> + * When @ps_mode == %IEEE80211_VIF_PS_DOZE or the %IEEE80211_CONF_PS flag is
> + * set it means that the powersave mode defined in IEEE 802.11-2007 section
> + * 11.2 is enabled. 

We might consider updating the references to 802.11-2012 since
everything shuffled around.

> +++ b/net/mac80211/ieee80211_i.h
> @@ -1180,6 +1180,7 @@ struct ieee80211_local {
>  #endif /* CONFIG_MAC80211_DEBUG_COUNTERS */
>  
> 
> +	int awake_cnt;

How will the locking for this possibly work? :)

> +#include <linux/nl80211.h>
> +#include <linux/pm_qos.h>
> +#include <net/mac80211.h>
> +#include "ieee80211_i.h"
> +#include "driver-ops.h"
> +
> +/*
> + * Sets the PS parameters in ieee80211_conf. Uses parameters from the first
> + * managed interface found which has power save enabled, or failing that
> + * the parameters from an arbitrary managed interface are used.
> + */
> +static void ieee80211_set_ps_params(struct ieee80211_local *local) { struct
> +	ieee80211_sub_if_data *sdata, *ps_sdata = NULL;

what happened to indentation here??

> +	list_for_each_entry(sdata, &local->interfaces, list) {

locking?

> +	if (ps_sdata) {
> +		struct ieee80211_vif *vif = &sdata->vif;

add a newline here please

> +static void __ieee80211_vif_set_ps_mode(struct ieee80211_sub_if_data *sdata,
> +					enum ieee80211_vif_ps_mode mode)
> +{
> +	struct ieee80211_local *local = sdata->local;
> +
> +	if (WARN_ON_ONCE(sdata->vif.type == NL80211_IFTYPE_MONITOR))
> +		return;
> +
> +	switch (mode) {
> +	case IEEE80211_VIF_PS_INACTIVE:
> +	case IEEE80211_VIF_PS_DOZE:
> +		if (sdata->vif.ps_mode > IEEE80211_VIF_PS_DOZE &&
> +		    !WARN_ON(local->awake_cnt <= 0))
> +			local->awake_cnt--;

locking?

johannes

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [RFC PATCH 4/8] mac80211: Use PS module for managed mode powersave
  2013-12-16 22:00 ` [RFC PATCH 4/8] mac80211: Use PS module for managed mode powersave Seth Forshee
@ 2013-12-17  8:25   ` Johannes Berg
  2013-12-17 14:09     ` Seth Forshee
  0 siblings, 1 reply; 17+ messages in thread
From: Johannes Berg @ 2013-12-17  8:25 UTC (permalink / raw)
  To: Seth Forshee
  Cc: linux-wireless, b43-dev, brcm80211-dev-list, John W. Linville,
	Stefano Brivio, Arend van Spriel

On Mon, 2013-12-16 at 16:00 -0600, Seth Forshee wrote:

> --- a/net/mac80211/iface.c
> +++ b/net/mac80211/iface.c
> @@ -682,7 +682,7 @@ int ieee80211_do_open(struct wireless_dev *wdev, bool coming_up)
>  	if (hw_reconf_flags)
>  		ieee80211_hw_config(local, hw_reconf_flags);
>  
> -	ieee80211_recalc_ps(local, -1);
> +	ieee80211_ps_vif_open(sdata);

I have a feeling that now that we have very regular join/leave code, and
are guaranteed to go through disassoc when the interface is stopped, we
probably don't need all the hooks in this file at all any more.

> @@ -1609,6 +1609,7 @@ int ieee80211_if_add(struct ieee80211_local *local, const char *name,
>  		strlcpy(sdata->name, name, IFNAMSIZ);
>  		ieee80211_assign_perm_addr(local, wdev->address, type);
>  		memcpy(sdata->vif.addr, wdev->address, ETH_ALEN);
> +		ieee80211_ps_init_vif(sdata);

That doesn't really seem like the right place? Shouldn't it somehow be
specific for managed interfaces, and be reset when the interface changes
type, for example?

> --- a/net/mac80211/status.c
> +++ b/net/mac80211/status.c
> @@ -732,15 +732,16 @@ void ieee80211_tx_status(struct ieee80211_hw *hw, struct sk_buff *skb)
>  			local->dot11FailedCount++;
>  	}
>  
> +	sdata = IEEE80211_DEV_TO_SUB_IF(skb->dev);

This isn't safe, the interface can go away while the SKB is on some
hardware queue. Look at what we do in ieee80211_report_used_skb() and
maybe refactor that somehow.

> -	if (local->hw.conf.flags & IEEE80211_CONF_PS) {
> +	if (sdata->vif.ps_mode == IEEE80211_VIF_PS_DOZE) {
>  		ieee80211_stop_queues_by_reason(&local->hw,
>  						IEEE80211_MAX_QUEUE_MAP,
>  						IEEE80211_QUEUE_STOP_REASON_PS);

This is a bit odd now - shouldn't you only stop the queues for that
interface? Or is this still assuming only a single interface?

> +	list_for_each_entry(sdata, &local->interfaces, list) {
> +		if (sdata->vif.type != NL80211_IFTYPE_STATION)
> +			continue;
> +		if (!sdata->u.mgd.associated)
> +			continue;
> +		if (sdata->vif.ps_mode != IEEE80211_VIF_PS_AWAKE)
> +			continue;
>  
> -			ieee80211_send_nullfunc(local, sdata, 0);
> -		}
> +		ieee80211_send_nullfunc(local, sdata, 0);

Unrelated to your patch, but we should probably send a nullfunc(PM=1)
packet for the other cases to detect if we got disconnected. Or wake up
briefly, and go back to sleep later or something ...

> +void ieee80211_recalc_ps(struct ieee80211_sub_if_data *sdata)
> +{
> +	switch (sdata->vif.type) {
> +	case NL80211_IFTYPE_STATION:
> +		ieee80211_mgd_recalc_ps(sdata);
> +		break;
> +	default:
> +		break;
> +	}
> +}

Would that make more sense in ps.c now?

> +int ieee80211_max_network_latency(struct notifier_block *nb,
> +				  unsigned long data, void *dummy)

ditto?

> +	mutex_lock(&local->iflist_mtx);
> +	list_for_each_entry_rcu(sdata, &local->interfaces, list) {

locking mutex & then rcu seems weird.
johannes

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [RFC PATCH 1/8] mac80211: Move dynamic PS data out of common code
  2013-12-17  8:08   ` Johannes Berg
@ 2013-12-17 12:37     ` Seth Forshee
  0 siblings, 0 replies; 17+ messages in thread
From: Seth Forshee @ 2013-12-17 12:37 UTC (permalink / raw)
  To: Johannes Berg
  Cc: linux-wireless, b43-dev, brcm80211-dev-list, John W. Linville,
	Stefano Brivio, Arend van Spriel

On Tue, Dec 17, 2013 at 09:08:50AM +0100, Johannes Berg wrote:
> On Mon, 2013-12-16 at 16:00 -0600, Seth Forshee wrote:
> 
> > +void ieee80211_notify_rx(struct ieee80211_rx_data *rx)
> > +{
> > +	switch(rx->sdata->vif.type) {
> > +	case NL80211_IFTYPE_MONITOR:
> > +		ieee80211_mgd_notify_rx(rx);
> 
> This ... seems wrong.

Yeah, it is. I guess there's always been enough tx while I was testing
to keep the dynamic ps timer updated.

> Also, we already have ieee80211_sta_rx_notify() which you want, I think?

I'm not sure, that's called earlier in the rx handlers. I guess that
probably only matters if there's an rx error, and the only penalty is
coming out of PS briefly. So I could probably use that instead.

> With this change, do you even still need local->ps_sdata?

Some of the code still uses ps_sdata, but I do remove it in a later
patch. I might be able to remove it here, but it's easier to do it when
the managed code changes to use the PS module.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [RFC PATCH 2/8] mac80211: Add per-interface powersave states and parameters
  2013-12-17  8:11   ` Johannes Berg
@ 2013-12-17 13:12     ` Seth Forshee
  0 siblings, 0 replies; 17+ messages in thread
From: Seth Forshee @ 2013-12-17 13:12 UTC (permalink / raw)
  To: Johannes Berg
  Cc: linux-wireless, b43-dev, brcm80211-dev-list, John W. Linville,
	Stefano Brivio, Arend van Spriel

On Tue, Dec 17, 2013 at 09:11:38AM +0100, Johannes Berg wrote:
> On Mon, 2013-12-16 at 16:00 -0600, Seth Forshee wrote:
> > In preparation for managing powersave states on a per-interface
> > basis, add powersave states and parameters to the interface-
> > specific data structures. Also add a change_ps driver callback
> > to notify drivers about changes to interface powersave states.
> > 
> > The new members and callback are unused here but will be
> > utilized by subsequent commits.
> 
> Can't say I like that part much, it just makes it harder to review.

Okay, I can squash it into the other patch easily enough.

> >  /**
> > + * enum ieee80211_vif_ps_mode - virtual interface power save mode
> > + *
> > + * Ordered in terms of increasing wakefulness.
> > + *
> > + * @IEEE80211_VIF_PS_INACTIVE: the interface is not currently open
> > + * @IEEE80211_VIF_PS_DOZE: the interface is in a low-power mode and may
> > + *	not be able to transmit or receive frames
> > + * @IEEE80211_VIF_PS_AWAKE_PM: the interface is awake and able to transmit
> > + *	and receive frames but PM may be set in frame control
> > + * @IEEE80211_VIF_PS_AWAKE: the interface is fully awake and able to
> > + *	transmit and receive frames
> > + */
> > +enum ieee80211_vif_ps_mode {
> > +	IEEE80211_VIF_PS_INACTIVE,
> 
> Does this INACTIVE thing really make sense? It should just be undefined
> if it's not associated, no? Doing this might make people want to rely on
> this to indicate associated-ness or something...

I use it to detect attempts to set the PS mode on interface which aren't
opened, but perhaps that isn't necessary. I'll look into it.

> > +	IEEE80211_VIF_PS_AWAKE_PM,
> > +	IEEE80211_VIF_PS_AWAKE,
> 
> The distinction between these seems somewhat unclear? "PM may be set"?

This is for Broadcom. It needs to be told somehow that mac80211 wants to
transmit frames with PM set, otherwise it's clearing them in the HW. I
thought about using a flag too, so long as there's some way to tell it
to set PM without powering down the hw.

> > + * @ps_mode: power save mode of this vif
> > + * @dynamic_ps_active: indicates whether dynamic PS is active for this vif
> > + * @dynamic_ps_timeout: The dynamic powersave timeout (in ms), see the
> > + *	powersave documentation below. This variable is valid only when
> > + *	the interface is in the doze state.
> > + * @max_sleep_period: the maximum number of beacon intervals to sleep for
> > + *	before checking the beacon for a TIM bit (managed mode only); this
> > + *	value will be only achievable between DTIM frames, the hardware
> > + *	needs to check for the multicast traffic bit in DTIM beacons.
> > + *	This variable is valid only when the interface is in the doze state.
> > + * @ps_dtim_period: The DTIM period of the AP we're connected to, for use
> > + *	in power saving. Power saving will not be enabled until a beacon
> > + *	has been received and the DTIM period is known.
> 
> Should these really be in the vif struct? They still seem somewhat
> internal to the implementation.

I made an assumption that multi-interface drivers would want to be told
about PS states and parameters for individual interfaces. This
notification happens via the change_ps callback using these members from
the vif.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [RFC PATCH 3/8] mac80211: Add powersave module
  2013-12-17  8:16   ` Johannes Berg
@ 2013-12-17 13:31     ` Seth Forshee
  0 siblings, 0 replies; 17+ messages in thread
From: Seth Forshee @ 2013-12-17 13:31 UTC (permalink / raw)
  To: Johannes Berg
  Cc: linux-wireless, b43-dev, brcm80211-dev-list, John W. Linville,
	Stefano Brivio, Arend van Spriel

On Tue, Dec 17, 2013 at 09:16:20AM +0100, Johannes Berg wrote:
> On Mon, 2013-12-16 at 16:00 -0600, Seth Forshee wrote:
> 
> > + * When @ps_mode == %IEEE80211_VIF_PS_DOZE or the %IEEE80211_CONF_PS flag is
> > + * set it means that the powersave mode defined in IEEE 802.11-2007 section
> > + * 11.2 is enabled. 
> 
> We might consider updating the references to 802.11-2012 since
> everything shuffled around.

Okay.

> > +++ b/net/mac80211/ieee80211_i.h
> > @@ -1180,6 +1180,7 @@ struct ieee80211_local {
> >  #endif /* CONFIG_MAC80211_DEBUG_COUNTERS */
> >  
> > 
> > +	int awake_cnt;
> 
> How will the locking for this possibly work? :)

Well, I did warn that I still need to look at locking ;-)

Mostly the access is already serialized, either by nature of going
through ieee80211_recalc_ps which already has locking rules or by being
run on the device workqueue. I need to try and grok how all the current
mac80211 locking works and see if I need to add something new.
Suggestions are welcome :-)

> 
> > +#include <linux/nl80211.h>
> > +#include <linux/pm_qos.h>
> > +#include <net/mac80211.h>
> > +#include "ieee80211_i.h"
> > +#include "driver-ops.h"
> > +
> > +/*
> > + * Sets the PS parameters in ieee80211_conf. Uses parameters from the first
> > + * managed interface found which has power save enabled, or failing that
> > + * the parameters from an arbitrary managed interface are used.
> > + */
> > +static void ieee80211_set_ps_params(struct ieee80211_local *local) { struct
> > +	ieee80211_sub_if_data *sdata, *ps_sdata = NULL;
> 
> what happened to indentation here??

Hmm, I don't know. Probably I accidentally hit J in vim a couple of
times and didn't notice.

> > +	if (ps_sdata) {
> > +		struct ieee80211_vif *vif = &sdata->vif;
> 
> add a newline here please

Okay.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [RFC PATCH 4/8] mac80211: Use PS module for managed mode powersave
  2013-12-17  8:25   ` Johannes Berg
@ 2013-12-17 14:09     ` Seth Forshee
  0 siblings, 0 replies; 17+ messages in thread
From: Seth Forshee @ 2013-12-17 14:09 UTC (permalink / raw)
  To: Johannes Berg
  Cc: linux-wireless, b43-dev, brcm80211-dev-list, John W. Linville,
	Stefano Brivio, Arend van Spriel

On Tue, Dec 17, 2013 at 09:25:12AM +0100, Johannes Berg wrote:
> On Mon, 2013-12-16 at 16:00 -0600, Seth Forshee wrote:
> 
> > --- a/net/mac80211/iface.c
> > +++ b/net/mac80211/iface.c
> > @@ -682,7 +682,7 @@ int ieee80211_do_open(struct wireless_dev *wdev, bool coming_up)
> >  	if (hw_reconf_flags)
> >  		ieee80211_hw_config(local, hw_reconf_flags);
> >  
> > -	ieee80211_recalc_ps(local, -1);
> > +	ieee80211_ps_vif_open(sdata);
> 
> I have a feeling that now that we have very regular join/leave code, and
> are guaranteed to go through disassoc when the interface is stopped, we
> probably don't need all the hooks in this file at all any more.

Okay, I'll look at this.

> > @@ -1609,6 +1609,7 @@ int ieee80211_if_add(struct ieee80211_local *local, const char *name,
> >  		strlcpy(sdata->name, name, IFNAMSIZ);
> >  		ieee80211_assign_perm_addr(local, wdev->address, type);
> >  		memcpy(sdata->vif.addr, wdev->address, ETH_ALEN);
> > +		ieee80211_ps_init_vif(sdata);
> 
> That doesn't really seem like the right place? Shouldn't it somehow be
> specific for managed interfaces, and be reset when the interface changes
> type, for example?

I intended the PS code to be usable for other interface types and tried
to isolate the parts specific to managed mode. Right now at minimum
other interface types need to "open" the interface with the PS module to
reference awake_count.

Maybe this is the wrong place though. I'll look at it.

> > --- a/net/mac80211/status.c
> > +++ b/net/mac80211/status.c
> > @@ -732,15 +732,16 @@ void ieee80211_tx_status(struct ieee80211_hw *hw, struct sk_buff *skb)
> >  			local->dot11FailedCount++;
> >  	}
> >  
> > +	sdata = IEEE80211_DEV_TO_SUB_IF(skb->dev);
> 
> This isn't safe, the interface can go away while the SKB is on some
> hardware queue. Look at what we do in ieee80211_report_used_skb() and
> maybe refactor that somehow.

Okay. This was using ps_sdata, so I need some other way to get at sdata.
I'll look at that code.

> > -	if (local->hw.conf.flags & IEEE80211_CONF_PS) {
> > +	if (sdata->vif.ps_mode == IEEE80211_VIF_PS_DOZE) {
> >  		ieee80211_stop_queues_by_reason(&local->hw,
> >  						IEEE80211_MAX_QUEUE_MAP,
> >  						IEEE80211_QUEUE_STOP_REASON_PS);
> 
> This is a bit odd now - shouldn't you only stop the queues for that
> interface? Or is this still assuming only a single interface?

You're right, this probably should just stop the interface queues.

> > +	list_for_each_entry(sdata, &local->interfaces, list) {
> > +		if (sdata->vif.type != NL80211_IFTYPE_STATION)
> > +			continue;
> > +		if (!sdata->u.mgd.associated)
> > +			continue;
> > +		if (sdata->vif.ps_mode != IEEE80211_VIF_PS_AWAKE)
> > +			continue;
> >  
> > -			ieee80211_send_nullfunc(local, sdata, 0);
> > -		}
> > +		ieee80211_send_nullfunc(local, sdata, 0);
> 
> Unrelated to your patch, but we should probably send a nullfunc(PM=1)
> packet for the other cases to detect if we got disconnected. Or wake up
> briefly, and go back to sleep later or something ...
> 
> > +void ieee80211_recalc_ps(struct ieee80211_sub_if_data *sdata)
> > +{
> > +	switch (sdata->vif.type) {
> > +	case NL80211_IFTYPE_STATION:
> > +		ieee80211_mgd_recalc_ps(sdata);
> > +		break;
> > +	default:
> > +		break;
> > +	}
> > +}
> 
> Would that make more sense in ps.c now?
> 
> > +int ieee80211_max_network_latency(struct notifier_block *nb,
> > +				  unsigned long data, void *dummy)
> 
> ditto?

I guess so, either one seems okay to me.

> > +	mutex_lock(&local->iflist_mtx);
> > +	list_for_each_entry_rcu(sdata, &local->interfaces, list) {
> 
> locking mutex & then rcu seems weird.

Oops. I'll fix that.

^ permalink raw reply	[flat|nested] 17+ messages in thread

end of thread, other threads:[~2013-12-17 14:09 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-12-16 22:00 [RFC/RFT] mac80211 powersave rework Seth Forshee
2013-12-16 22:00 ` [RFC PATCH 1/8] mac80211: Move dynamic PS data out of common code Seth Forshee
2013-12-17  8:08   ` Johannes Berg
2013-12-17 12:37     ` Seth Forshee
2013-12-16 22:00 ` [RFC PATCH 2/8] mac80211: Add per-interface powersave states and parameters Seth Forshee
2013-12-17  8:11   ` Johannes Berg
2013-12-17 13:12     ` Seth Forshee
2013-12-16 22:00 ` [RFC PATCH 3/8] mac80211: Add powersave module Seth Forshee
2013-12-17  8:16   ` Johannes Berg
2013-12-17 13:31     ` Seth Forshee
2013-12-16 22:00 ` [RFC PATCH 4/8] mac80211: Use PS module for managed mode powersave Seth Forshee
2013-12-17  8:25   ` Johannes Berg
2013-12-17 14:09     ` Seth Forshee
2013-12-16 22:00 ` [RFC PATCH 5/8] mac80211: Don't start dynamic PS timer when leaving off-channel if still scanning Seth Forshee
2013-12-16 22:00 ` [RFC PATCH 6/8] brcmsmac: Set MCTL_HPS when PM should be set Seth Forshee
2013-12-16 22:00 ` [RFC PATCH 7/8] b43: Allow HWPS state to be changed Seth Forshee
2013-12-16 22:01 ` [RFC PATCH 8/8] b43: Set B43_MACCTL_HWPS when PM should be set Seth Forshee

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).