[ath9k-devel] [PATCH 0/2] ath10k: scan fixes

[ath9k-devel] [PATCH 0/2] ath10k: scan fixes

[Michal Kazior]

This is a result of a patch split.

Michal Kazior (2):
  ath10k: sync wmi before starting scan timeout
  ath10k: refactor scan locking

 drivers/net/wireless/ath/ath10k/core.c |    1 -
 drivers/net/wireless/ath/ath10k/core.h |    1 -
 drivers/net/wireless/ath/ath10k/mac.c  |   56 +++++++++++++++-----------------
 drivers/net/wireless/ath/ath10k/wmi.c  |   10 +++---
 4 files changed, 33 insertions(+), 35 deletions(-)

-- 
1.7.9.5

[ath9k-devel] [PATCH 1/2] ath10k: sync wmi before starting scan timeout

[Michal Kazior]

We may have some WMI commands pending. We
shouldn't start the timeout before we're sure we
have submitted the scan stop command.

Signed-off-by: Michal Kazior <michal.kazior@tieto.com>
---
 drivers/net/wireless/ath/ath10k/mac.c |    2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/net/wireless/ath/ath10k/mac.c b/drivers/net/wireless/ath/ath10k/mac.c
index 0f8869d..4589f6f 100644
--- a/drivers/net/wireless/ath/ath10k/mac.c
+++ b/drivers/net/wireless/ath/ath10k/mac.c
@@ -1418,6 +1418,8 @@ static void ath10k_abort_scan(struct ath10k *ar)
 	if (ret)
 		ath10k_warn("%s: ath10k_wmi_stop_scan failed (%d)\n", __func__, ret);
 
+	ath10k_wmi_flush_tx(ar);
+
 	ret = wait_for_completion_timeout(&ar->scan.completed, 3*HZ);
 	if (ret == 0)
 		ret = -ETIMEDOUT;
-- 
1.7.9.5

[ath9k-devel] [PATCH 2/2] ath10k: refactor scan locking

[Michal Kazior]

Since we have data_lock it is no longer necessary
to have scan.lock.

Signed-off-by: Michal Kazior <michal.kazior@tieto.com>
---
 drivers/net/wireless/ath/ath10k/core.c |    1 -
 drivers/net/wireless/ath/ath10k/core.h |    1 -
 drivers/net/wireless/ath/ath10k/mac.c  |   54 +++++++++++++++-----------------
 drivers/net/wireless/ath/ath10k/wmi.c  |   10 +++---
 4 files changed, 31 insertions(+), 35 deletions(-)

diff --git a/drivers/net/wireless/ath/ath10k/core.c b/drivers/net/wireless/ath/ath10k/core.c
index 7036695..c9068b0 100644
--- a/drivers/net/wireless/ath/ath10k/core.c
+++ b/drivers/net/wireless/ath/ath10k/core.c
@@ -482,7 +482,6 @@ struct ath10k *ath10k_core_create(void *hif_priv, struct device *dev,
 
 	init_completion(&ar->scan.started);
 	init_completion(&ar->scan.completed);
-	spin_lock_init(&ar->scan.lock);
 
 	init_completion(&ar->install_key_done);
 	init_completion(&ar->vdev_setup_done);
diff --git a/drivers/net/wireless/ath/ath10k/core.h b/drivers/net/wireless/ath/ath10k/core.h
index 4e19264..65b1d58 100644
--- a/drivers/net/wireless/ath/ath10k/core.h
+++ b/drivers/net/wireless/ath/ath10k/core.h
@@ -309,7 +309,6 @@ struct ath10k {
 	} hw_params;
 
 	struct {
-		spinlock_t lock;
 		struct completion started;
 		struct completion completed;
 		struct timer_list timeout;
diff --git a/drivers/net/wireless/ath/ath10k/mac.c b/drivers/net/wireless/ath/ath10k/mac.c
index 4589f6f..943c879 100644
--- a/drivers/net/wireless/ath/ath10k/mac.c
+++ b/drivers/net/wireless/ath/ath10k/mac.c
@@ -1372,11 +1372,9 @@ void ath10k_reset_scan(unsigned long ptr)
 {
 	struct ath10k *ar = (struct ath10k *)ptr;
 
-	spin_lock_bh(&ar->scan.lock);
-
+	spin_lock_bh(&ar->data_lock);
 	if (!ar->scan.in_progress) {
-		/* lucky! scan must've completed right before timeout */
-		spin_unlock_bh(&ar->scan.lock);
+		spin_unlock_bh(&ar->data_lock);
 		return;
 	}
 
@@ -1389,7 +1387,7 @@ void ath10k_reset_scan(unsigned long ptr)
 
 	ar->scan.in_progress = false;
 	complete_all(&ar->scan.completed);
-	spin_unlock_bh(&ar->scan.lock);
+	spin_unlock_bh(&ar->data_lock);
 }
 
 static void ath10k_abort_scan(struct ath10k *ar)
@@ -1405,14 +1403,14 @@ static void ath10k_abort_scan(struct ath10k *ar)
 
 	del_timer_sync(&ar->scan.timeout);
 
-	spin_lock_bh(&ar->scan.lock);
+	spin_lock_bh(&ar->data_lock);
 	if (!ar->scan.in_progress) {
-		spin_unlock_bh(&ar->scan.lock);
+		spin_unlock_bh(&ar->data_lock);
 		return;
 	}
 
 	ar->scan.aborting = true;
-	spin_unlock_bh(&ar->scan.lock);
+	spin_unlock_bh(&ar->data_lock);
 
 	ret = ath10k_wmi_stop_scan(ar, &arg);
 	if (ret)
@@ -1424,13 +1422,13 @@ static void ath10k_abort_scan(struct ath10k *ar)
 	if (ret == 0)
 		ret = -ETIMEDOUT;
 
-	spin_lock_bh(&ar->scan.lock);
+	spin_lock_bh(&ar->data_lock);
 	if (ar->scan.in_progress) {
 		ath10k_warn("%s: could not stop scan (%d)\n", __func__, ret);
 		ar->scan.in_progress = false;
 		ath10k_offchan_tx_purge(ar);
 	}
-	spin_unlock_bh(&ar->scan.lock);
+	spin_unlock_bh(&ar->data_lock);
 }
 
 static int ath10k_start_scan(struct ath10k *ar,
@@ -1438,9 +1436,11 @@ static int ath10k_start_scan(struct ath10k *ar,
 {
 	int ret;
 
+	lockdep_assert_held(&ar->conf_mutex);
+
 	ret = ath10k_wmi_start_scan(ar, arg);
 	if (ret)
-		goto abort;
+		return ret;
 
 	/* make sure we submit the command so the completion
 	* timeout makes sense */
@@ -1450,7 +1450,7 @@ static int ath10k_start_scan(struct ath10k *ar,
 	if (ret == 0)
 		ret = -ETIMEDOUT;
 	if (ret < 0)
-		goto abort;
+		return ret;
 
 	/* the scan can complete earlier, before we even
 	 * start the timer. in that case the timer handler
@@ -1458,11 +1458,6 @@ static int ath10k_start_scan(struct ath10k *ar,
 	 * false. */
 	mod_timer(&ar->scan.timeout, jiffies + (arg->max_scan_time*HZ)/1000);
 	return 0;
-abort:
-	spin_lock_bh(&ar->scan.lock);
-	ar->scan.in_progress = false;
-	spin_unlock_bh(&ar->scan.lock);
-	return ret;
 }
 
 /**********************/
@@ -1509,10 +1504,10 @@ static void ath10k_tx(struct ieee80211_hw *hw,
 	ATH10K_SKB_CB(skb)->htt.tid = tid;
 
 	if (info->flags & IEEE80211_TX_CTL_TX_OFFCHAN) {
-		spin_lock_bh(&ar->scan.lock);
+		spin_lock_bh(&ar->data_lock);
 		ATH10K_SKB_CB(skb)->htt.is_offchan = true;
 		ATH10K_SKB_CB(skb)->htt.vdev_id = ar->scan.vdev_id;
-		spin_unlock_bh(&ar->scan.lock);
+		spin_unlock_bh(&ar->data_lock);
 
 		ath10k_dbg(ATH10K_DBG_MAC, "queued offchannel skb %p\n", skb);
 
@@ -1951,9 +1946,9 @@ static int ath10k_hw_scan(struct ieee80211_hw *hw,
 
 	mutex_lock(&ar->conf_mutex);
 
-	spin_lock_bh(&ar->scan.lock);
+	spin_lock_bh(&ar->data_lock);
 	if (ar->scan.in_progress) {
-		spin_unlock_bh(&ar->scan.lock);
+		spin_unlock_bh(&ar->data_lock);
 		ret = -EBUSY;
 		goto exit;
 	}
@@ -1964,7 +1959,7 @@ static int ath10k_hw_scan(struct ieee80211_hw *hw,
 	ar->scan.aborting = false;
 	ar->scan.is_roc = false;
 	ar->scan.vdev_id = arvif->vdev_id;
-	spin_unlock_bh(&ar->scan.lock);
+	spin_unlock_bh(&ar->data_lock);
 
 	memset(&arg, 0, sizeof(arg));
 	ath10k_wmi_start_scan_init(ar, &arg);
@@ -1995,9 +1990,10 @@ static int ath10k_hw_scan(struct ieee80211_hw *hw,
 
 	ret = ath10k_start_scan(ar, &arg);
 	if (ret) {
-		spin_lock_bh(&ar->scan.lock);
+		ath10k_warn("could not start hw scan (%d)\n", ret);
+		spin_lock_bh(&ar->data_lock);
 		ar->scan.in_progress = false;
-		spin_unlock_bh(&ar->scan.lock);
+		spin_unlock_bh(&ar->data_lock);
 	}
 
 exit:
@@ -2228,9 +2224,9 @@ static int ath10k_remain_on_channel(struct ieee80211_hw *hw,
 
 	mutex_lock(&ar->conf_mutex);
 
-	spin_lock_bh(&ar->scan.lock);
+	spin_lock_bh(&ar->data_lock);
 	if (ar->scan.in_progress) {
-		spin_unlock_bh(&ar->scan.lock);
+		spin_unlock_bh(&ar->data_lock);
 		ret = -EBUSY;
 		goto exit;
 	}
@@ -2241,7 +2237,7 @@ static int ath10k_remain_on_channel(struct ieee80211_hw *hw,
 	ar->scan.aborting = false;
 	ar->scan.is_roc = true;
 	ar->scan.vdev_id = arvif->vdev_id;
-	spin_unlock_bh(&ar->scan.lock);
+	spin_unlock_bh(&ar->data_lock);
 
 	memset(&arg, 0, sizeof(arg));
 	ath10k_wmi_start_scan_init(ar, &arg);
@@ -2258,9 +2254,9 @@ static int ath10k_remain_on_channel(struct ieee80211_hw *hw,
 	ret = ath10k_start_scan(ar, &arg);
 	if (ret) {
 		ath10k_warn("could not start roc scan (%d)\n", ret);
-		spin_lock_bh(&ar->scan.lock);
+		spin_lock_bh(&ar->data_lock);
 		ar->scan.in_progress = false;
-		spin_unlock_bh(&ar->scan.lock);
+		spin_unlock_bh(&ar->data_lock);
 	}
 
 exit:
diff --git a/drivers/net/wireless/ath/ath10k/wmi.c b/drivers/net/wireless/ath/ath10k/wmi.c
index fa521b0..ebb212e 100644
--- a/drivers/net/wireless/ath/ath10k/wmi.c
+++ b/drivers/net/wireless/ath/ath10k/wmi.c
@@ -149,10 +149,12 @@ static int ath10k_wmi_event_scan(struct ath10k *ar, struct sk_buff *skb)
 	case WMI_SCAN_EVENT_STARTED:
 		ath10k_dbg(ATH10K_DBG_WMI, "SCAN_EVENT_STARTED\n");
 
-		if (ar->scan.is_roc)
+		spin_lock_bh(&ar->data_lock);
+		if (ar->scan.in_progress && ar->scan.is_roc)
 			ieee80211_ready_on_channel(ar->hw);
 
 		complete(&ar->scan.started);
+		spin_unlock_bh(&ar->data_lock);
 		break;
 	case WMI_SCAN_EVENT_COMPLETED:
 		ath10k_dbg(ATH10K_DBG_WMI, "SCAN_EVENT_COMPLETED\n");
@@ -175,10 +177,10 @@ static int ath10k_wmi_event_scan(struct ath10k *ar, struct sk_buff *skb)
 
 		rcu_assign_pointer(ar->scan_channel, NULL);
 
-		spin_lock_bh(&ar->scan.lock);
+		spin_lock_bh(&ar->data_lock);
 		if (!ar->scan.in_progress) {
+			spin_unlock_bh(&ar->data_lock);
 			ath10k_warn("no scan requested, ignoring\n");
-			spin_unlock_bh(&ar->scan.lock);
 			break;
 		}
 
@@ -194,7 +196,7 @@ static int ath10k_wmi_event_scan(struct ath10k *ar, struct sk_buff *skb)
 		del_timer(&ar->scan.timeout);
 		complete_all(&ar->scan.completed);
 		ar->scan.in_progress = false;
-		spin_unlock_bh(&ar->scan.lock);
+		spin_unlock_bh(&ar->data_lock);
 		break;
 	case WMI_SCAN_EVENT_BSS_CHANNEL:
 		ath10k_dbg(ATH10K_DBG_WMI, "SCAN_EVENT_BSS_CHANNEL\n");
-- 
1.7.9.5

[ath9k-devel] [PATCH 0/2] ath10k: scan fixes

[Kalle Valo]

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

> This is a result of a patch split.
>
> Michal Kazior (2):
>   ath10k: sync wmi before starting scan timeout
>   ath10k: refactor scan locking

Thanks, both patches applied.

-- 
Kalle Valo