[PATCH 0/7] ath10k: fix WMI atomic usage

[PATCH 0/7] ath10k: fix WMI atomic usage

[Kalle Valo]

Some WMI calls were made in atomic contex, these patches fix
those. Please review.

---

Kalle Valo (2):
      ath10k: fix ath10k_bss_assoc() to not sleep in atomic context
      ath10k: add might_sleep() to ath10k_wmi_cmd_send()

Michal Kazior (5):
      ath10k: use workqueue to set WEP TX key
      ath10k: fix add_interface failure handling
      ath10k: track vif list internally
      ath10k: fix scheduling while atomic config bug
      ath10k: remove unnecessary checks


 drivers/net/wireless/ath/ath10k/core.c |    2 
 drivers/net/wireless/ath/ath10k/core.h |    7 +
 drivers/net/wireless/ath/ath10k/mac.c  |  312 +++++++++++++++++---------------
 drivers/net/wireless/ath/ath10k/wmi.c  |    2 
 4 files changed, 178 insertions(+), 145 deletions(-)

[PATCH 1/7] ath10k: use workqueue to set WEP TX key

[Kalle Valo]

From: Michal Kazior <michal.kazior@tieto.com>

Recent WMI/HTC changes made it possible for WMI
commands to sleep (if there's not enough HTC TX
credits to submit a command). TX path is in an
atomic context so calling WMI commands in it is
wrong.

This simply moves WEP key index update to a worker
and fixes the 'scheduling while atomic' bug.

This still leaves multiple WEP key handling laggy,
i.e. some frames may be TXed with an old/different
key (although recipient should still be able to RX
them).

kvalo: changed the title

Signed-off-by: Michal Kazior <michal.kazior@tieto.com>
Signed-off-by: Kalle Valo <kvalo@qca.qualcomm.com>
---
 drivers/net/wireless/ath/ath10k/core.h |    4 ++
 drivers/net/wireless/ath/ath10k/mac.c  |   53 ++++++++++++++++++++++----------
 2 files changed, 39 insertions(+), 18 deletions(-)

diff --git a/drivers/net/wireless/ath/ath10k/core.h b/drivers/net/wireless/ath/ath10k/core.h
index ce36daa..cef5455 100644
--- a/drivers/net/wireless/ath/ath10k/core.h
+++ b/drivers/net/wireless/ath/ath10k/core.h
@@ -215,8 +215,10 @@ struct ath10k_vif {
 	struct ath10k *ar;
 	struct ieee80211_vif *vif;
 
+	struct work_struct wep_key_work;
 	struct ieee80211_key_conf *wep_keys[WMI_MAX_KEY_INDEX + 1];
-	u8 def_wep_key_index;
+	u8 def_wep_key_idx;
+	u8 def_wep_key_newidx;
 
 	u16 tx_seq_no;
 
diff --git a/drivers/net/wireless/ath/ath10k/mac.c b/drivers/net/wireless/ath/ath10k/mac.c
index 97d7111..bc0e17b 100644
--- a/drivers/net/wireless/ath/ath10k/mac.c
+++ b/drivers/net/wireless/ath/ath10k/mac.c
@@ -1239,7 +1239,7 @@ static void ath10k_bss_disassoc(struct ieee80211_hw *hw,
 	/* FIXME: why don't we print error if wmi call fails? */
 	ret = ath10k_wmi_vdev_down(ar, arvif->vdev_id);
 
-	arvif->def_wep_key_index = 0;
+	arvif->def_wep_key_idx = 0;
 }
 
 static int ath10k_station_assoc(struct ath10k *ar, struct ath10k_vif *arvif,
@@ -1467,6 +1467,30 @@ static void ath10k_tx_h_qos_workaround(struct ieee80211_hw *hw,
 	skb_pull(skb, IEEE80211_QOS_CTL_LEN);
 }
 
+static void ath10k_tx_wep_key_work(struct work_struct *work)
+{
+	struct ath10k_vif *arvif = container_of(work, struct ath10k_vif,
+						wep_key_work);
+	int ret, keyidx = arvif->def_wep_key_newidx;
+
+	if (arvif->def_wep_key_idx == keyidx)
+		return;
+
+	ath10k_dbg(ATH10K_DBG_MAC, "mac vdev %d set keyidx %d\n",
+		   arvif->vdev_id, keyidx);
+
+	ret = ath10k_wmi_vdev_set_param(arvif->ar,
+					arvif->vdev_id,
+					arvif->ar->wmi.vdev_param->def_keyid,
+					keyidx);
+	if (ret) {
+		ath10k_warn("could not update wep keyidx (%d)\n", ret);
+		return;
+	}
+
+	arvif->def_wep_key_idx = keyidx;
+}
+
 static void ath10k_tx_h_update_wep_key(struct sk_buff *skb)
 {
 	struct ieee80211_tx_info *info = IEEE80211_SKB_CB(skb);
@@ -1475,8 +1499,6 @@ static void ath10k_tx_h_update_wep_key(struct sk_buff *skb)
 	struct ath10k *ar = arvif->ar;
 	struct ieee80211_hdr *hdr = (struct ieee80211_hdr *)skb->data;
 	struct ieee80211_key_conf *key = info->control.hw_key;
-	u32 vdev_param;
-	int ret;
 
 	if (!ieee80211_has_protected(hdr->frame_control))
 		return;
@@ -1488,21 +1510,14 @@ static void ath10k_tx_h_update_wep_key(struct sk_buff *skb)
 	    key->cipher != WLAN_CIPHER_SUITE_WEP104)
 		return;
 
-	if (key->keyidx == arvif->def_wep_key_index)
-		return;
-
-	ath10k_dbg(ATH10K_DBG_MAC, "mac vdev %d keyidx %d\n",
-		   arvif->vdev_id, key->keyidx);
-
-	vdev_param = ar->wmi.vdev_param->def_keyid;
-	ret = ath10k_wmi_vdev_set_param(ar, arvif->vdev_id, vdev_param,
-					key->keyidx);
-	if (ret) {
-		ath10k_warn("could not update wep keyidx (%d)\n", ret);
+	if (key->keyidx == arvif->def_wep_key_idx)
 		return;
-	}
 
-	arvif->def_wep_key_index = key->keyidx;
+	/* FIXME: Most likely a few frames will be TXed with an old key. Simply
+	 * queueing frames until key index is updated is not an option because
+	 * sk_buff may need more processing to be done, e.g. offchannel */
+	arvif->def_wep_key_newidx = key->keyidx;
+	ieee80211_queue_work(ar->hw, &arvif->wep_key_work);
 }
 
 static void ath10k_tx_h_add_p2p_noa_ie(struct ath10k *ar, struct sk_buff *skb)
@@ -2023,6 +2038,8 @@ static int ath10k_add_interface(struct ieee80211_hw *hw,
 	arvif->ar = ar;
 	arvif->vif = vif;
 
+	INIT_WORK(&arvif->wep_key_work, ath10k_tx_wep_key_work);
+
 	if ((vif->type == NL80211_IFTYPE_MONITOR) && ar->monitor_present) {
 		ath10k_warn("Only one monitor interface allowed\n");
 		ret = -EBUSY;
@@ -2078,7 +2095,7 @@ static int ath10k_add_interface(struct ieee80211_hw *hw,
 
 	vdev_param = ar->wmi.vdev_param->def_keyid;
 	ret = ath10k_wmi_vdev_set_param(ar, 0, vdev_param,
-					arvif->def_wep_key_index);
+					arvif->def_wep_key_idx);
 	if (ret)
 		ath10k_warn("Failed to set default keyid: %d\n", ret);
 
@@ -2147,6 +2164,8 @@ static void ath10k_remove_interface(struct ieee80211_hw *hw,
 
 	mutex_lock(&ar->conf_mutex);
 
+	cancel_work_sync(&arvif->wep_key_work);
+
 	spin_lock_bh(&ar->data_lock);
 	if (arvif->beacon) {
 		dev_kfree_skb_any(arvif->beacon);

[PATCH 2/7] ath10k: fix add_interface failure handling

[Kalle Valo]

From: Michal Kazior <michal.kazior@tieto.com>

If something failed along add_interface() setup it
was possible to leak a vdev id, vdev and peer.

This could end up with leaked FW state or FW crash
(assuming add_interface() failure wasn't a result of
a crash).

kvalo: rebased, whitespace fixes

Signed-off-by: Michal Kazior <michal.kazior@tieto.com>
Signed-off-by: Kalle Valo <kvalo@qca.qualcomm.com>
---
 drivers/net/wireless/ath/ath10k/mac.c |   53 +++++++++++++++++++++++++--------
 1 file changed, 40 insertions(+), 13 deletions(-)

diff --git a/drivers/net/wireless/ath/ath10k/mac.c b/drivers/net/wireless/ath/ath10k/mac.c
index bc0e17b..16e55bd 100644
--- a/drivers/net/wireless/ath/ath10k/mac.c
+++ b/drivers/net/wireless/ath/ath10k/mac.c
@@ -2043,18 +2043,17 @@ static int ath10k_add_interface(struct ieee80211_hw *hw,
 	if ((vif->type == NL80211_IFTYPE_MONITOR) && ar->monitor_present) {
 		ath10k_warn("Only one monitor interface allowed\n");
 		ret = -EBUSY;
-		goto exit;
+		goto err;
 	}
 
 	bit = ffs(ar->free_vdev_map);
 	if (bit == 0) {
 		ret = -EBUSY;
-		goto exit;
+		goto err;
 	}
 
 	arvif->vdev_id = bit - 1;
 	arvif->vdev_subtype = WMI_VDEV_SUBTYPE_NONE;
-	ar->free_vdev_map &= ~(1 << arvif->vdev_id);
 
 	if (ar->p2p)
 		arvif->vdev_subtype = WMI_VDEV_SUBTYPE_P2P_DEVICE;
@@ -2090,27 +2089,33 @@ static int ath10k_add_interface(struct ieee80211_hw *hw,
 				     arvif->vdev_subtype, vif->addr);
 	if (ret) {
 		ath10k_warn("WMI vdev create failed: ret %d\n", ret);
-		goto exit;
+		goto err;
 	}
 
+	ar->free_vdev_map &= ~BIT(arvif->vdev_id);
+
 	vdev_param = ar->wmi.vdev_param->def_keyid;
 	ret = ath10k_wmi_vdev_set_param(ar, 0, vdev_param,
 					arvif->def_wep_key_idx);
-	if (ret)
+	if (ret) {
 		ath10k_warn("Failed to set default keyid: %d\n", ret);
+		goto err_vdev_delete;
+	}
 
 	vdev_param = ar->wmi.vdev_param->tx_encap_type;
 	ret = ath10k_wmi_vdev_set_param(ar, arvif->vdev_id, vdev_param,
 					ATH10K_HW_TXRX_NATIVE_WIFI);
 	/* 10.X firmware does not support this VDEV parameter. Do not warn */
-	if (ret && ret != -EOPNOTSUPP)
+	if (ret && ret != -EOPNOTSUPP) {
 		ath10k_warn("Failed to set TX encap: %d\n", ret);
+		goto err_vdev_delete;
+	}
 
 	if (arvif->vdev_type == WMI_VDEV_TYPE_AP) {
 		ret = ath10k_peer_create(ar, arvif->vdev_id, vif->addr);
 		if (ret) {
 			ath10k_warn("Failed to create peer for AP: %d\n", ret);
-			goto exit;
+			goto err_vdev_delete;
 		}
 	}
 
@@ -2119,39 +2124,61 @@ static int ath10k_add_interface(struct ieee80211_hw *hw,
 		value = WMI_STA_PS_RX_WAKE_POLICY_WAKE;
 		ret = ath10k_wmi_set_sta_ps_param(ar, arvif->vdev_id,
 						  param, value);
-		if (ret)
+		if (ret) {
 			ath10k_warn("Failed to set RX wake policy: %d\n", ret);
+			goto err_peer_delete;
+		}
 
 		param = WMI_STA_PS_PARAM_TX_WAKE_THRESHOLD;
 		value = WMI_STA_PS_TX_WAKE_THRESHOLD_ALWAYS;
 		ret = ath10k_wmi_set_sta_ps_param(ar, arvif->vdev_id,
 						  param, value);
-		if (ret)
+		if (ret) {
 			ath10k_warn("Failed to set TX wake thresh: %d\n", ret);
+			goto err_peer_delete;
+		}
 
 		param = WMI_STA_PS_PARAM_PSPOLL_COUNT;
 		value = WMI_STA_PS_PSPOLL_COUNT_NO_MAX;
 		ret = ath10k_wmi_set_sta_ps_param(ar, arvif->vdev_id,
 						  param, value);
-		if (ret)
+		if (ret) {
 			ath10k_warn("Failed to set PSPOLL count: %d\n", ret);
+			goto err_peer_delete;
+		}
 	}
 
 	ret = ath10k_mac_set_rts(arvif, ar->hw->wiphy->rts_threshold);
-	if (ret)
+	if (ret) {
 		ath10k_warn("failed to set rts threshold for vdev %d (%d)\n",
 			    arvif->vdev_id, ret);
+		goto err_peer_delete;
+	}
 
 	ret = ath10k_mac_set_frag(arvif, ar->hw->wiphy->frag_threshold);
-	if (ret)
+	if (ret) {
 		ath10k_warn("failed to set frag threshold for vdev %d (%d)\n",
 			    arvif->vdev_id, ret);
+		goto err_peer_delete;
+	}
 
 	if (arvif->vdev_type == WMI_VDEV_TYPE_MONITOR)
 		ar->monitor_present = true;
 
-exit:
 	mutex_unlock(&ar->conf_mutex);
+	return 0;
+
+err_peer_delete:
+	if (arvif->vdev_type == WMI_VDEV_TYPE_AP)
+		ath10k_wmi_peer_delete(ar, arvif->vdev_id, vif->addr);
+
+err_vdev_delete:
+	ath10k_wmi_vdev_delete(ar, arvif->vdev_id);
+	ar->free_vdev_map &= ~BIT(arvif->vdev_id);
+
+err:
+	mutex_unlock(&ar->conf_mutex);
+
 	return ret;
 }
 

[PATCH 3/7] ath10k: track vif list internally

[Kalle Valo]

From: Michal Kazior <michal.kazior@tieto.com>

mac80211 interface interations functions have
peculiar locking issues. This patch introduces
internal (to ath10k) vif list that will be used
for vif iteration purposes.

Signed-off-by: Michal Kazior <michal.kazior@tieto.com>
Signed-off-by: Kalle Valo <kvalo@qca.qualcomm.com>
---
 drivers/net/wireless/ath/ath10k/core.c |    2 ++
 drivers/net/wireless/ath/ath10k/core.h |    3 +++
 drivers/net/wireless/ath/ath10k/mac.c  |    3 +++
 3 files changed, 8 insertions(+)

diff --git a/drivers/net/wireless/ath/ath10k/core.c b/drivers/net/wireless/ath/ath10k/core.c
index c5561a9..ed51b51 100644
--- a/drivers/net/wireless/ath/ath10k/core.c
+++ b/drivers/net/wireless/ath/ath10k/core.c
@@ -713,6 +713,7 @@ struct ath10k *ath10k_core_create(void *hif_priv, struct device *dev,
 	mutex_init(&ar->conf_mutex);
 	spin_lock_init(&ar->data_lock);
 
+	INIT_LIST_HEAD(&ar->arvifs);
 	INIT_LIST_HEAD(&ar->peers);
 	init_waitqueue_head(&ar->peer_mapping_wq);
 
@@ -824,6 +825,7 @@ int ath10k_core_start(struct ath10k *ar)
 		goto err_disconnect_htc;
 
 	ar->free_vdev_map = (1 << TARGET_NUM_VDEVS) - 1;
+	INIT_LIST_HEAD(&ar->arvifs);
 
 	return 0;
 
diff --git a/drivers/net/wireless/ath/ath10k/core.h b/drivers/net/wireless/ath/ath10k/core.h
index cef5455..0934f76 100644
--- a/drivers/net/wireless/ath/ath10k/core.h
+++ b/drivers/net/wireless/ath/ath10k/core.h
@@ -205,6 +205,8 @@ struct ath10k_peer {
 #define ATH10K_VDEV_SETUP_TIMEOUT_HZ (5*HZ)
 
 struct ath10k_vif {
+	struct list_head list;
+
 	u32 vdev_id;
 	enum wmi_vdev_type vdev_type;
 	enum wmi_vdev_subtype vdev_subtype;
@@ -404,6 +406,7 @@ struct ath10k {
 	/* protects shared structure data */
 	spinlock_t data_lock;
 
+	struct list_head arvifs;
 	struct list_head peers;
 	wait_queue_head_t peer_mapping_wq;
 
diff --git a/drivers/net/wireless/ath/ath10k/mac.c b/drivers/net/wireless/ath/ath10k/mac.c
index 16e55bd..c8e4180 100644
--- a/drivers/net/wireless/ath/ath10k/mac.c
+++ b/drivers/net/wireless/ath/ath10k/mac.c
@@ -2093,6 +2093,7 @@ static int ath10k_add_interface(struct ieee80211_hw *hw,
 	}
 
 	ar->free_vdev_map &= ~BIT(arvif->vdev_id);
+	list_add(&arvif->list, &ar->arvifs);
 
 	vdev_param = ar->wmi.vdev_param->def_keyid;
 	ret = ath10k_wmi_vdev_set_param(ar, 0, vdev_param,
@@ -2175,6 +2176,7 @@ err_peer_delete:
 err_vdev_delete:
 	ath10k_wmi_vdev_delete(ar, arvif->vdev_id);
 	ar->free_vdev_map &= ~BIT(arvif->vdev_id);
+	list_del(&arvif->list);
 
 err:
 	mutex_unlock(&ar->conf_mutex);
@@ -2201,6 +2203,7 @@ static void ath10k_remove_interface(struct ieee80211_hw *hw,
 	spin_unlock_bh(&ar->data_lock);
 
 	ar->free_vdev_map |= 1 << (arvif->vdev_id);
+	list_del(&arvif->list);
 
 	if (arvif->vdev_type == WMI_VDEV_TYPE_AP) {
 		ret = ath10k_peer_delete(arvif->ar, arvif->vdev_id, vif->addr);

[PATCH 4/7] ath10k: fix scheduling while atomic config bug

[Kalle Valo]

From: Michal Kazior <michal.kazior@tieto.com>

Recent HTC/WMI changes introduced the bug. ath10k
was using _atomic iteration function with
sleepable functions.

mac80211 provides another iteration function but
it cannot be safely called in hw_config() callback
due to local->iflist_mtx being possibly acquired
already.

The patch uses internal vif list for iteration
purposes and removes/refactors no longer necessary
_iter functions.

Reported-By: Kalle Valo <kvalo@qca.qualcomm.com>
Signed-off-by: Michal Kazior <michal.kazior@tieto.com>
Signed-off-by: Kalle Valo <kvalo@qca.qualcomm.com>
---
 drivers/net/wireless/ath/ath10k/mac.c |  146 ++++++++++++++-------------------
 1 file changed, 62 insertions(+), 84 deletions(-)

diff --git a/drivers/net/wireless/ath/ath10k/mac.c b/drivers/net/wireless/ath/ath10k/mac.c
index c8e4180..bd42a14 100644
--- a/drivers/net/wireless/ath/ath10k/mac.c
+++ b/drivers/net/wireless/ath/ath10k/mac.c
@@ -723,35 +723,30 @@ static void ath10k_control_ibss(struct ath10k_vif *arvif,
 /*
  * Review this when mac80211 gains per-interface powersave support.
  */
-static void ath10k_ps_iter(void *data, u8 *mac, struct ieee80211_vif *vif)
+static int ath10k_mac_vif_setup_ps(struct ath10k_vif *arvif)
 {
-	struct ath10k_generic_iter *ar_iter = data;
-	struct ieee80211_conf *conf = &ar_iter->ar->hw->conf;
-	struct ath10k_vif *arvif = ath10k_vif_to_arvif(vif);
+	struct ath10k *ar = arvif->ar;
+	struct ieee80211_conf *conf = &ar->hw->conf;
 	enum wmi_sta_powersave_param param;
 	enum wmi_sta_ps_mode psmode;
 	int ret;
 
 	lockdep_assert_held(&arvif->ar->conf_mutex);
 
-	if (vif->type != NL80211_IFTYPE_STATION)
-		return;
+	if (arvif->vif->type != NL80211_IFTYPE_STATION)
+		return 0;
 
 	if (conf->flags & IEEE80211_CONF_PS) {
 		psmode = WMI_STA_PS_MODE_ENABLED;
 		param = WMI_STA_PS_PARAM_INACTIVITY_TIME;
 
-		ret = ath10k_wmi_set_sta_ps_param(ar_iter->ar,
-						  arvif->vdev_id,
-						  param,
+		ret = ath10k_wmi_set_sta_ps_param(ar, arvif->vdev_id, param,
 						  conf->dynamic_ps_timeout);
 		if (ret) {
 			ath10k_warn("Failed to set inactivity time for VDEV: %d\n",
 				    arvif->vdev_id);
-			return;
+			return ret;
 		}
-
-		ar_iter->ret = ret;
 	} else {
 		psmode = WMI_STA_PS_MODE_DISABLED;
 	}
@@ -759,11 +754,14 @@ static void ath10k_ps_iter(void *data, u8 *mac, struct ieee80211_vif *vif)
 	ath10k_dbg(ATH10K_DBG_MAC, "mac vdev %d psmode %s\n",
 		   arvif->vdev_id, psmode ? "enable" : "disable");
 
-	ar_iter->ret = ath10k_wmi_set_psmode(ar_iter->ar, arvif->vdev_id,
-					     psmode);
-	if (ar_iter->ret)
+	ret = ath10k_wmi_set_psmode(ar, arvif->vdev_id, psmode);
+	if (ret) {
 		ath10k_warn("Failed to set PS Mode: %d for VDEV: %d\n",
 			    psmode, arvif->vdev_id);
+		return ret;
+	}
+
+	return 0;
 }
 
 /**********************/
@@ -1959,9 +1957,10 @@ static void ath10k_stop(struct ieee80211_hw *hw)
 	cancel_work_sync(&ar->restart_work);
 }
 
-static void ath10k_config_ps(struct ath10k *ar)
+static int ath10k_config_ps(struct ath10k *ar)
 {
-	struct ath10k_generic_iter ar_iter;
+	struct ath10k_vif *arvif;
+	int ret = 0;
 
 	lockdep_assert_held(&ar->conf_mutex);
 
@@ -1970,17 +1969,17 @@ static void ath10k_config_ps(struct ath10k *ar)
 	 * vdevs at this point we must not iterate over this interface list.
 	 * This setting will be updated upon add_interface(). */
 	if (ar->state == ATH10K_STATE_RESTARTED)
-		return;
-
-	memset(&ar_iter, 0, sizeof(struct ath10k_generic_iter));
-	ar_iter.ar = ar;
+		return 0;
 
-	ieee80211_iterate_active_interfaces_atomic(
-		ar->hw, IEEE80211_IFACE_ITER_NORMAL,
-		ath10k_ps_iter, &ar_iter);
+	list_for_each_entry(arvif, &ar->arvifs, list) {
+		ret = ath10k_mac_vif_setup_ps(arvif);
+		if (ret) {
+			ath10k_warn("could not setup powersave (%d)\n", ret);
+			break;
+		}
+	}
 
-	if (ar_iter.ret)
-		ath10k_warn("failed to set ps config (%d)\n", ar_iter.ret);
+	return ret;
 }
 
 static int ath10k_config(struct ieee80211_hw *hw, u32 changed)
@@ -2882,86 +2881,65 @@ static int ath10k_cancel_remain_on_channel(struct ieee80211_hw *hw)
  * Both RTS and Fragmentation threshold are interface-specific
  * in ath10k, but device-specific in mac80211.
  */
-static void ath10k_set_rts_iter(void *data, u8 *mac, struct ieee80211_vif *vif)
-{
-	struct ath10k_generic_iter *ar_iter = data;
-	struct ath10k_vif *arvif = ath10k_vif_to_arvif(vif);
-	u32 rts = ar_iter->ar->hw->wiphy->rts_threshold;
 
-	lockdep_assert_held(&arvif->ar->conf_mutex);
+static int ath10k_set_rts_threshold(struct ieee80211_hw *hw, u32 value)
+{
+	struct ath10k *ar = hw->priv;
+	struct ath10k_vif *arvif;
+	int ret = 0;
 
 	/* During HW reconfiguration mac80211 reports all interfaces that were
 	 * running until reconfiguration was started. Since FW doesn't have any
 	 * vdevs at this point we must not iterate over this interface list.
 	 * This setting will be updated upon add_interface(). */
-	if (ar_iter->ar->state == ATH10K_STATE_RESTARTED)
-		return;
-
-	ath10k_dbg(ATH10K_DBG_MAC, "mac vdev %d rts_threshold %d\n",
-		   arvif->vdev_id, rts);
-
-	ar_iter->ret = ath10k_mac_set_rts(arvif, rts);
-	if (ar_iter->ret)
-		ath10k_warn("Failed to set RTS threshold for VDEV: %d\n",
-			    arvif->vdev_id);
-}
-
-static int ath10k_set_rts_threshold(struct ieee80211_hw *hw, u32 value)
-{
-	struct ath10k_generic_iter ar_iter;
-	struct ath10k *ar = hw->priv;
-
-	memset(&ar_iter, 0, sizeof(struct ath10k_generic_iter));
-	ar_iter.ar = ar;
+	if (ar->state == ATH10K_STATE_RESTARTED)
+		return 0;
 
 	mutex_lock(&ar->conf_mutex);
-	ieee80211_iterate_active_interfaces_atomic(
-		hw, IEEE80211_IFACE_ITER_NORMAL,
-		ath10k_set_rts_iter, &ar_iter);
+	list_for_each_entry(arvif, &ar->arvifs, list) {
+		ath10k_dbg(ATH10K_DBG_MAC, "mac vdev %d rts threshold %d\n",
+			   arvif->vdev_id, value);
+
+		ret = ath10k_mac_set_rts(arvif, value);
+		if (ret) {
+			ath10k_warn("could not set rts threshold for vdev %d (%d)\n",
+				    arvif->vdev_id, ret);
+			break;
+		}
+	}
 	mutex_unlock(&ar->conf_mutex);
 
-	return ar_iter.ret;
+	return ret;
 }
 
-static void ath10k_set_frag_iter(void *data, u8 *mac, struct ieee80211_vif *vif)
+static int ath10k_set_frag_threshold(struct ieee80211_hw *hw, u32 value)
 {
-	struct ath10k_generic_iter *ar_iter = data;
-	struct ath10k_vif *arvif = ath10k_vif_to_arvif(vif);
-	u32 frag = ar_iter->ar->hw->wiphy->frag_threshold;
-
-	lockdep_assert_held(&arvif->ar->conf_mutex);
+	struct ath10k *ar = hw->priv;
+	struct ath10k_vif *arvif;
+	int ret = 0;
 
 	/* During HW reconfiguration mac80211 reports all interfaces that were
 	 * running until reconfiguration was started. Since FW doesn't have any
 	 * vdevs at this point we must not iterate over this interface list.
 	 * This setting will be updated upon add_interface(). */
-	if (ar_iter->ar->state == ATH10K_STATE_RESTARTED)
-		return;
-
-	ath10k_dbg(ATH10K_DBG_MAC, "mac vdev %d fragmentation_threshold %d\n",
-		   arvif->vdev_id, frag);
-
-	ar_iter->ret = ath10k_mac_set_frag(arvif, frag);
-	if (ar_iter->ret)
-		ath10k_warn("Failed to set frag threshold for VDEV: %d\n",
-			    arvif->vdev_id);
-}
-
-static int ath10k_set_frag_threshold(struct ieee80211_hw *hw, u32 value)
-{
-	struct ath10k_generic_iter ar_iter;
-	struct ath10k *ar = hw->priv;
-
-	memset(&ar_iter, 0, sizeof(struct ath10k_generic_iter));
-	ar_iter.ar = ar;
+	if (ar->state == ATH10K_STATE_RESTARTED)
+		return 0;
 
 	mutex_lock(&ar->conf_mutex);
-	ieee80211_iterate_active_interfaces_atomic(
-		hw, IEEE80211_IFACE_ITER_NORMAL,
-		ath10k_set_frag_iter, &ar_iter);
+	list_for_each_entry(arvif, &ar->arvifs, list) {
+		ath10k_dbg(ATH10K_DBG_MAC, "mac vdev %d fragmentation threshold %d\n",
+			   arvif->vdev_id, value);
+
+		ret = ath10k_mac_set_rts(arvif, value);
+		if (ret) {
+			ath10k_warn("could not set fragmentation threshold for vdev %d (%d)\n",
+				    arvif->vdev_id, ret);
+			break;
+		}
+	}
 	mutex_unlock(&ar->conf_mutex);
 
-	return ar_iter.ret;
+	return ret;
 }
 
 static void ath10k_flush(struct ieee80211_hw *hw, u32 queues, bool drop)

[PATCH 5/7] ath10k: remove unnecessary checks

[Kalle Valo]

From: Michal Kazior <michal.kazior@tieto.com>

mac80211 interface iteration functions that were
used originally iterated over interfaces that
weren't re-added to the driver during recovery.

Since internal vif list is now used it's safe to
remove the safe-guard as internal vif list is
based on add/remove_interface function which
guarantees that vdev is created in FW before it is
iterated over.

Signed-off-by: Michal Kazior <michal.kazior@tieto.com>
Signed-off-by: Kalle Valo <kvalo@qca.qualcomm.com>
---
 drivers/net/wireless/ath/ath10k/mac.c |   21 ---------------------
 1 file changed, 21 deletions(-)

diff --git a/drivers/net/wireless/ath/ath10k/mac.c b/drivers/net/wireless/ath/ath10k/mac.c
index bd42a14..4273eef 100644
--- a/drivers/net/wireless/ath/ath10k/mac.c
+++ b/drivers/net/wireless/ath/ath10k/mac.c
@@ -1964,13 +1964,6 @@ static int ath10k_config_ps(struct ath10k *ar)
 
 	lockdep_assert_held(&ar->conf_mutex);
 
-	/* During HW reconfiguration mac80211 reports all interfaces that were
-	 * running until reconfiguration was started. Since FW doesn't have any
-	 * vdevs at this point we must not iterate over this interface list.
-	 * This setting will be updated upon add_interface(). */
-	if (ar->state == ATH10K_STATE_RESTARTED)
-		return 0;
-
 	list_for_each_entry(arvif, &ar->arvifs, list) {
 		ret = ath10k_mac_vif_setup_ps(arvif);
 		if (ret) {
@@ -2888,13 +2881,6 @@ static int ath10k_set_rts_threshold(struct ieee80211_hw *hw, u32 value)
 	struct ath10k_vif *arvif;
 	int ret = 0;
 
-	/* During HW reconfiguration mac80211 reports all interfaces that were
-	 * running until reconfiguration was started. Since FW doesn't have any
-	 * vdevs at this point we must not iterate over this interface list.
-	 * This setting will be updated upon add_interface(). */
-	if (ar->state == ATH10K_STATE_RESTARTED)
-		return 0;
-
 	mutex_lock(&ar->conf_mutex);
 	list_for_each_entry(arvif, &ar->arvifs, list) {
 		ath10k_dbg(ATH10K_DBG_MAC, "mac vdev %d rts threshold %d\n",
@@ -2918,13 +2904,6 @@ static int ath10k_set_frag_threshold(struct ieee80211_hw *hw, u32 value)
 	struct ath10k_vif *arvif;
 	int ret = 0;
 
-	/* During HW reconfiguration mac80211 reports all interfaces that were
-	 * running until reconfiguration was started. Since FW doesn't have any
-	 * vdevs at this point we must not iterate over this interface list.
-	 * This setting will be updated upon add_interface(). */
-	if (ar->state == ATH10K_STATE_RESTARTED)
-		return 0;
-
 	mutex_lock(&ar->conf_mutex);
 	list_for_each_entry(arvif, &ar->arvifs, list) {
 		ath10k_dbg(ATH10K_DBG_MAC, "mac vdev %d fragmentation threshold %d\n",

[PATCH 6/7] ath10k: fix ath10k_bss_assoc() to not sleep in atomic context

[Kalle Valo]

ath10k_bss_assoc() was calling ath10k_peer_assoc(), which can sleep, under
atomic rcu_read_lock() and causing scheduing while atomic errors. Workaround
that by delaying the call to ath10k_wmi_peer_assoc().

Signed-off-by: Kalle Valo <kvalo@qca.qualcomm.com>
---
 drivers/net/wireless/ath/ath10k/mac.c |   56 ++++++++++++++++++++++-----------
 1 file changed, 37 insertions(+), 19 deletions(-)

diff --git a/drivers/net/wireless/ath/ath10k/mac.c b/drivers/net/wireless/ath/ath10k/mac.c
index 4273eef..0b1cc51 100644
--- a/drivers/net/wireless/ath/ath10k/mac.c
+++ b/drivers/net/wireless/ath/ath10k/mac.c
@@ -1137,26 +1137,25 @@ static void ath10k_peer_assoc_h_phymode(struct ath10k *ar,
 	WARN_ON(phymode == MODE_UNKNOWN);
 }
 
-static int ath10k_peer_assoc(struct ath10k *ar,
-			     struct ath10k_vif *arvif,
-			     struct ieee80211_sta *sta,
-			     struct ieee80211_bss_conf *bss_conf)
+static int ath10k_peer_assoc_prepare(struct ath10k *ar,
+				     struct ath10k_vif *arvif,
+				     struct ieee80211_sta *sta,
+				     struct ieee80211_bss_conf *bss_conf,
+				     struct wmi_peer_assoc_complete_arg *arg)
 {
-	struct wmi_peer_assoc_complete_arg arg;
-
 	lockdep_assert_held(&ar->conf_mutex);
 
-	memset(&arg, 0, sizeof(struct wmi_peer_assoc_complete_arg));
+	memset(arg, 0, sizeof(*arg));
 
-	ath10k_peer_assoc_h_basic(ar, arvif, sta, bss_conf, &arg);
-	ath10k_peer_assoc_h_crypto(ar, arvif, &arg);
-	ath10k_peer_assoc_h_rates(ar, sta, &arg);
-	ath10k_peer_assoc_h_ht(ar, sta, &arg);
-	ath10k_peer_assoc_h_vht(ar, sta, &arg);
-	ath10k_peer_assoc_h_qos(ar, arvif, sta, bss_conf, &arg);
-	ath10k_peer_assoc_h_phymode(ar, arvif, sta, &arg);
+	ath10k_peer_assoc_h_basic(ar, arvif, sta, bss_conf, arg);
+	ath10k_peer_assoc_h_crypto(ar, arvif, arg);
+	ath10k_peer_assoc_h_rates(ar, sta, arg);
+	ath10k_peer_assoc_h_ht(ar, sta, arg);
+	ath10k_peer_assoc_h_vht(ar, sta, arg);
+	ath10k_peer_assoc_h_qos(ar, arvif, sta, bss_conf, arg);
+	ath10k_peer_assoc_h_phymode(ar, arvif, sta, arg);
 
-	return ath10k_wmi_peer_assoc(ar, &arg);
+	return 0;
 }
 
 /* can be called only in mac80211 callbacks due to `key_count` usage */
@@ -1166,6 +1165,7 @@ static void ath10k_bss_assoc(struct ieee80211_hw *hw,
 {
 	struct ath10k *ar = hw->priv;
 	struct ath10k_vif *arvif = ath10k_vif_to_arvif(vif);
+	struct wmi_peer_assoc_complete_arg peer_arg;
 	struct ieee80211_sta *ap_sta;
 	int ret;
 
@@ -1181,15 +1181,24 @@ static void ath10k_bss_assoc(struct ieee80211_hw *hw,
 		return;
 	}
 
-	ret = ath10k_peer_assoc(ar, arvif, ap_sta, bss_conf);
+	ret = ath10k_peer_assoc_prepare(ar, arvif, ap_sta,
+					bss_conf, &peer_arg);
 	if (ret) {
-		ath10k_warn("Peer assoc failed for %pM\n", bss_conf->bssid);
+		ath10k_warn("Peer assoc prepare failed for %pM\n: %d",
+			    bss_conf->bssid, ret);
 		rcu_read_unlock();
 		return;
 	}
 
 	rcu_read_unlock();
 
+	ret = ath10k_wmi_peer_assoc(ar, &peer_arg);
+	if (ret) {
+		ath10k_warn("Peer assoc failed for %pM\n: %d",
+			    bss_conf->bssid, ret);
+		return;
+	}
+
 	ath10k_dbg(ATH10K_DBG_MAC,
 		   "mac vdev %d up (associated) bssid %pM aid %d\n",
 		   arvif->vdev_id, bss_conf->bssid, bss_conf->aid);
@@ -1243,13 +1252,22 @@ static void ath10k_bss_disassoc(struct ieee80211_hw *hw,
 static int ath10k_station_assoc(struct ath10k *ar, struct ath10k_vif *arvif,
 				struct ieee80211_sta *sta)
 {
+	struct wmi_peer_assoc_complete_arg peer_arg;
 	int ret = 0;
 
 	lockdep_assert_held(&ar->conf_mutex);
 
-	ret = ath10k_peer_assoc(ar, arvif, sta, NULL);
+	ret = ath10k_peer_assoc_prepare(ar, arvif, sta, NULL, &peer_arg);
+	if (ret) {
+		ath10k_warn("WMI peer assoc prepare failed for %pM\n",
+			    sta->addr);
+		return ret;
+	}
+
+	ret = ath10k_wmi_peer_assoc(ar, &peer_arg);
 	if (ret) {
-		ath10k_warn("WMI peer assoc failed for %pM\n", sta->addr);
+		ath10k_warn("Peer assoc failed for STA %pM\n: %d",
+			    sta->addr, ret);
 		return ret;
 	}
 

[PATCH 7/7] ath10k: add might_sleep() to ath10k_wmi_cmd_send()

[Kalle Valo]

ath10k_wmi_cmd_send() will now sleep if there are no credits available.
To make it easier to catch callers in atomic context add might_sleep()
to the function.

Signed-off-by: Kalle Valo <kvalo@qca.qualcomm.com>
---
 drivers/net/wireless/ath/ath10k/wmi.c |    2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/net/wireless/ath/ath10k/wmi.c b/drivers/net/wireless/ath/ath10k/wmi.c
index d1e513e..77238af 100644
--- a/drivers/net/wireless/ath/ath10k/wmi.c
+++ b/drivers/net/wireless/ath/ath10k/wmi.c
@@ -613,6 +613,8 @@ static int ath10k_wmi_cmd_send(struct ath10k *ar, struct sk_buff *skb,
 {
 	int ret = -EOPNOTSUPP;
 
+	might_sleep();
+
 	if (cmd_id == WMI_CMD_UNSUPPORTED) {
 		ath10k_warn("wmi command %d is not supported by firmware\n",
 			    cmd_id);

Re: [PATCH 3/7] ath10k: track vif list internally

[Kalle Valo]

Kalle Valo <kvalo@qca.qualcomm.com> writes:

> From: Michal Kazior <michal.kazior@tieto.com>
>
> mac80211 interface interations functions have
> peculiar locking issues. This patch introduces
> internal (to ath10k) vif list that will be used
> for vif iteration purposes.
>
> Signed-off-by: Michal Kazior <michal.kazior@tieto.com>
> Signed-off-by: Kalle Valo <kvalo@qca.qualcomm.com>

[...]

> --- a/drivers/net/wireless/ath/ath10k/core.c
> +++ b/drivers/net/wireless/ath/ath10k/core.c
> @@ -713,6 +713,7 @@ struct ath10k *ath10k_core_create(void *hif_priv, struct device *dev,
>  	mutex_init(&ar->conf_mutex);
>  	spin_lock_init(&ar->data_lock);
>  
> +	INIT_LIST_HEAD(&ar->arvifs);
>  	INIT_LIST_HEAD(&ar->peers);
>  	init_waitqueue_head(&ar->peer_mapping_wq);
>  
> @@ -824,6 +825,7 @@ int ath10k_core_start(struct ath10k *ar)
>  		goto err_disconnect_htc;
>  
>  	ar->free_vdev_map = (1 << TARGET_NUM_VDEVS) - 1;
> +	INIT_LIST_HEAD(&ar->arvifs);
>  
>  	return 0;

Michal, why do the INIT_LIST_HEAD() twice? Isn't it enough to do it
core_start()?

-- 
Kalle Valo

Re: [PATCH 3/7] ath10k: track vif list internally

[Michal Kazior]

On 15 October 2013 11:49, Kalle Valo <kvalo@qca.qualcomm.com> wrote:
> Kalle Valo <kvalo@qca.qualcomm.com> writes:
>
>> From: Michal Kazior <michal.kazior@tieto.com>
>>
>> mac80211 interface interations functions have
>> peculiar locking issues. This patch introduces
>> internal (to ath10k) vif list that will be used
>> for vif iteration purposes.
>>
>> Signed-off-by: Michal Kazior <michal.kazior@tieto.com>
>> Signed-off-by: Kalle Valo <kvalo@qca.qualcomm.com>
>
> [...]
>
>> --- a/drivers/net/wireless/ath/ath10k/core.c
>> +++ b/drivers/net/wireless/ath/ath10k/core.c
>> @@ -713,6 +713,7 @@ struct ath10k *ath10k_core_create(void *hif_priv, struct device *dev,
>>       mutex_init(&ar->conf_mutex);
>>       spin_lock_init(&ar->data_lock);
>>
>> +     INIT_LIST_HEAD(&ar->arvifs);
>>       INIT_LIST_HEAD(&ar->peers);
>>       init_waitqueue_head(&ar->peer_mapping_wq);
>>
>> @@ -824,6 +825,7 @@ int ath10k_core_start(struct ath10k *ar)
>>               goto err_disconnect_htc;
>>
>>       ar->free_vdev_map = (1 << TARGET_NUM_VDEVS) - 1;
>> +     INIT_LIST_HEAD(&ar->arvifs);
>>
>>       return 0;
>
> Michal, why do the INIT_LIST_HEAD() twice? Isn't it enough to do it
> core_start()?

Ah, good point. It's most likely okay to just do it in
ath10k_core_start(). The one in ath10k_core_create() isn't required.


Michał

Re: [PATCH 3/7] ath10k: track vif list internally

[Kalle Valo]

Michal Kazior <michal.kazior@tieto.com> writes:

> On 15 October 2013 11:49, Kalle Valo <kvalo@qca.qualcomm.com> wrote:
>> Kalle Valo <kvalo@qca.qualcomm.com> writes:
>>
>>> From: Michal Kazior <michal.kazior@tieto.com>
>>>
>>> mac80211 interface interations functions have
>>> peculiar locking issues. This patch introduces
>>> internal (to ath10k) vif list that will be used
>>> for vif iteration purposes.
>>>
>>> Signed-off-by: Michal Kazior <michal.kazior@tieto.com>
>>> Signed-off-by: Kalle Valo <kvalo@qca.qualcomm.com>
>>
>> [...]
>>
>> Michal, why do the INIT_LIST_HEAD() twice? Isn't it enough to do it
>> core_start()?
>
> Ah, good point. It's most likely okay to just do it in
> ath10k_core_start(). The one in ath10k_core_create() isn't required.

Ok, I removed the one from ath10k_core_create() before I commited this.

-- 
Kalle Valo

Re: [PATCH 0/7] ath10k: fix WMI atomic usage

[Kalle Valo]

Kalle Valo <kvalo@qca.qualcomm.com> writes:

> Some WMI calls were made in atomic contex, these patches fix
> those. Please review.
>
> ---
>
> Kalle Valo (2):
>       ath10k: fix ath10k_bss_assoc() to not sleep in atomic context
>       ath10k: add might_sleep() to ath10k_wmi_cmd_send()
>
> Michal Kazior (5):
>       ath10k: use workqueue to set WEP TX key
>       ath10k: fix add_interface failure handling
>       ath10k: track vif list internally
>       ath10k: fix scheduling while atomic config bug
>       ath10k: remove unnecessary checks

All patches applied.

-- 
Kalle Valo