wcn36xx: RFC self-limit sw_scan to 166ms to avoid link stallsmainline-qcom64-basis-test-2015-02-04-1mainline-qcom64-basis

On wcn3620 with wpa_supplicant set to defaults, it can asks for a
10s sw_scan as often as every 40s.

During the 10s scan, the link is basically stopped for the whole
time, leading to behaviours like this on ping

...
64 bytes from 198.41.208.143: icmp_seq=242 ttl=53 time=40.8 ms
64 bytes from 198.41.208.143: icmp_seq=243 ttl=53 time=31.6 ms
64 bytes from 198.41.208.143: icmp_seq=244 ttl=53 time=32.2 ms
64 bytes from 198.41.208.143: icmp_seq=245 ttl=53 time=8968 ms
64 bytes from 198.41.208.143: icmp_seq=246 ttl=53 time=7977 ms
64 bytes from 198.41.208.143: icmp_seq=247 ttl=53 time=6977 ms
64 bytes from 198.41.208.143: icmp_seq=248 ttl=53 time=6018 ms
64 bytes from 198.41.208.143: icmp_seq=249 ttl=53 time=5018 ms
64 bytes from 198.41.208.143: icmp_seq=250 ttl=53 time=4019 ms
64 bytes from 198.41.208.143: icmp_seq=251 ttl=53 time=3019 ms
64 bytes from 198.41.208.143: icmp_seq=252 ttl=53 time=2019 ms
64 bytes from 198.41.208.143: icmp_seq=253 ttl=53 time=1019 ms
64 bytes from 198.41.208.143: icmp_seq=254 ttl=53 time=34.3 ms
64 bytes from 198.41.208.143: icmp_seq=255 ttl=53 time=30.9 ms
64 bytes from 198.41.208.143: icmp_seq=256 ttl=53 time=32.9 ms
64 bytes from 198.41.208.143: icmp_seq=257 ttl=53 time=30.8 ms
64 bytes from 198.41.208.143: icmp_seq=258 ttl=53 time=30.9 ms
64 bytes from 198.41.208.143: icmp_seq=259 ttl=53 time=33.7 ms
...

As was mentioned on the wcn36xx list, the prima driver does not
use this kind of scan behaviour, as captured here in the
wcn36xx-dissector example dumps

https://raw.githubusercontent.com/kanstrup/wcn36xx-dissector/master/examples/prima-3660-sta-connect-11g-wpa2-wmm-screen-off.txt

Instead he scans for 150ms channel by channel, with explicit
scan start and stop SMD commands for each channel.

This patch decouples the scan action of the driver from the
sw_scan start and stop ops, in this initial version of the
patch the driver now does one 166ms scan and stops / finishes
the scan action itself and waits until the sw_scan stop.

This removes the 10s blocking behaviour, while still
introducing problems with the link, they are far more
tolerable than 10s

...
64 bytes from 198.41.209.142: icmp_seq=57 ttl=54 time=31.2 ms
64 bytes from 198.41.209.142: icmp_seq=58 ttl=54 time=31.9 ms
64 bytes from 198.41.209.142: icmp_seq=59 ttl=54 time=31.2 ms
64 bytes from 198.41.209.142: icmp_seq=60 ttl=54 time=31.1 ms
64 bytes from 198.41.209.142: icmp_seq=61 ttl=54 time=37.9 ms
64 bytes from 198.41.209.142: icmp_seq=62 ttl=54 time=31.5 ms
64 bytes from 198.41.209.142: icmp_seq=63 ttl=54 time=198 ms
64 bytes from 198.41.209.142: icmp_seq=64 ttl=54 time=31.7 ms
64 bytes from 198.41.209.142: icmp_seq=65 ttl=54 time=33.0 ms
64 bytes from 198.41.209.142: icmp_seq=66 ttl=54 time=80.2 ms
64 bytes from 198.41.209.142: icmp_seq=67 ttl=54 time=159 ms
64 bytes from 198.41.209.142: icmp_seq=68 ttl=54 time=34.3 ms
64 bytes from 198.41.209.142: icmp_seq=69 ttl=54 time=32.7 ms
64 bytes from 198.41.209.142: icmp_seq=70 ttl=54 time=39.3 ms
64 bytes from 198.41.209.142: icmp_seq=71 ttl=54 time=118 ms
64 bytes from 198.41.209.142: icmp_seq=72 ttl=54 time=31.7 ms
64 bytes from 198.41.209.142: icmp_seq=73 ttl=54 time=35.5 ms
64 bytes from 198.41.209.142: icmp_seq=74 ttl=54 time=32.0 ms
64 bytes from 198.41.209.142: icmp_seq=75 ttl=54 time=33.0 ms
64 bytes from 198.41.209.142: icmp_seq=76 ttl=54 time=35.1 ms
64 bytes from 198.41.209.142: icmp_seq=77 ttl=54 time=31.8 ms
64 bytes from 198.41.209.142: icmp_seq=78 ttl=54 time=32.0 ms
64 bytes from 198.41.209.142: icmp_seq=79 ttl=54 time=31.9 ms
64 bytes from 198.41.209.142: icmp_seq=80 ttl=54 time=46.0 ms
64 bytes from 198.41.209.142: icmp_seq=81 ttl=54 time=38.3 ms
...

It takes a bit longer to associate probably because it takes
longer to find the AP beacon.

For bulk transfer, a download that was interrupted regularly and
averaged 150KBps before is able to complete at average 1.0MBps

# wget -Obbb.mp4 http://download.blender.org/peach/bigbuckbunny_movies/BigBuckBunny_320x180.mp4
--1970-01-01 00:03:13--  http://download.blender.org/peach/bigbuckbunny_movies/BigBuckBunny_320x180.mp4
Resolving download.blender.org (download.blender.org)... 82.94.213.221
Connecting to download.blender.org (download.blender.org)|82.94.213.221|:80... connected.
HTTP request sent, awaiting response... 200 OK
Length: 64657027 (62M) [application/octet-stream]
Saving to: ‘bbb.mp4’

100%[======================================>] 64,657,027  1.02MB/s   in 61s

2015-01-24 13:50:11 (1.00 MB/s) - ‘bbb.mp4’ saved [64657027/64657027]

It seems wpa_supplicant is asking for the scans, according to dynamic
average rssi.  That makes the scan requests unpredictable, eg

[  261.144852] wcn36xx wcn36xx: wcn36xx_scan_work: ch 11
[  300.940232] wcn36xx wcn36xx: wcn36xx_scan_work: ch 1
[  340.726914] wcn36xx wcn36xx: wcn36xx_scan_work: ch 11
[  380.529928] wcn36xx wcn36xx: wcn36xx_scan_work: ch 1
[  420.314379] wcn36xx wcn36xx: wcn36xx_scan_work: ch 11
[  460.102097] wcn36xx wcn36xx: wcn36xx_scan_work: ch 11
[  499.892589] wcn36xx wcn36xx: wcn36xx_scan_work: ch 11
[  809.681723] wcn36xx wcn36xx: wcn36xx_scan_work: ch 11
[ 1119.474430] wcn36xx wcn36xx: wcn36xx_scan_work: ch 11
[ 1429.264266] wcn36xx wcn36xx: wcn36xx_scan_work: ch 11
[ 1739.055172] wcn36xx wcn36xx: wcn36xx_scan_work: ch 11

the channel it wants to scan is also unpredictable (this is while
associated to the only AP with the BSSID on ch11).

Any advice about if this is a legitimate way to cover the
problem, if there's a better way and if the other wcn36xx
variants also have this problem (and could use the solution)
are welcome.

Signed-off-by: Andy Green <andy.green@linaro.org>

4 files changed, 100 insertions, 52 deletions

diff --git a/drivers/net/wireless/ath/wcn36xx/main.c b/drivers/net/wireless/ath/wcn36xx/main.c
index aff1ea0b2b2e..9f67c9896042 100644
--- a/drivers/net/wireless/ath/wcn36xx/main.c
+++ b/drivers/net/wireless/ath/wcn36xx/main.c
@@ -19,6 +19,7 @@
 #include <linux/module.h>
 #include <linux/firmware.h>
 #include <linux/platform_device.h>
+#include <linux/workqueue.h>
 #include "wcn36xx.h"
 
 unsigned int wcn36xx_dbg_mask;
@@ -501,8 +502,7 @@ static void wcn36xx_sw_scan_start(struct ieee80211_hw *hw,
 {
 	struct wcn36xx *wcn = hw->priv;
 
-	wcn36xx_smd_init_scan(wcn, HAL_SYS_MODE_SCAN);
-	wcn36xx_smd_start_scan(wcn);
+	wcn36xx_smd_init_scan(wcn);
 }
 
 static void wcn36xx_sw_scan_complete(struct ieee80211_hw *hw,
@@ -510,8 +510,7 @@ static void wcn36xx_sw_scan_complete(struct ieee80211_hw *hw,
 {
 	struct wcn36xx *wcn = hw->priv;
 
-	wcn36xx_smd_end_scan(wcn);
-	wcn36xx_smd_finish_scan(wcn, HAL_SYS_MODE_SCAN);
+	wcn36xx_smd_finish_scan(wcn);
 }
 
 static void wcn36xx_update_allowed_rates(struct ieee80211_sta *sta,
@@ -1035,7 +1034,7 @@ static int wcn36xx_probe(struct platform_device *pdev)
 	wcn->chip_version = wcn->ctrl_ops->get_chip_type();
 
 	mutex_init(&wcn->hal_mutex);
-
+	INIT_DELAYED_WORK(&wcn->scan_work, wcn36xx_scan_work);
 	if (!wcn->ctrl_ops->get_hw_mac(addr)) {
 		wcn36xx_info("mac address: %pM\n", addr);
 		SET_IEEE80211_PERM_ADDR(wcn->hw, addr);
diff --git a/drivers/net/wireless/ath/wcn36xx/smd.c b/drivers/net/wireless/ath/wcn36xx/smd.c
index e9bf8bd32ec5..91b7a1eaad81 100644
--- a/drivers/net/wireless/ath/wcn36xx/smd.c
+++ b/drivers/net/wireless/ath/wcn36xx/smd.c
@@ -19,6 +19,7 @@
 #include <linux/etherdevice.h>
 #include <linux/firmware.h>
 #include <linux/bitops.h>
+#include <linux/workqueue.h>
 #include "smd.h"
 
 static int put_cfg_tlv_u32(struct wcn36xx *wcn, size_t *len, u32 id, u32 value)
@@ -432,126 +433,168 @@ out:
 	return ret;
 }
 
-int wcn36xx_smd_init_scan(struct wcn36xx *wcn, enum wcn36xx_hal_sys_mode mode)
+int _wcn36xx_smd_finish_scan(struct wcn36xx *wcn)
 {
-	struct wcn36xx_hal_init_scan_req_msg msg_body;
-	int ret = 0;
+	struct wcn36xx_hal_finish_scan_req_msg msg_body;
+        int ret = 0;
 
-	mutex_lock(&wcn->hal_mutex);
-	INIT_HAL_MSG(msg_body, WCN36XX_HAL_INIT_SCAN_REQ);
+	INIT_HAL_MSG(msg_body, WCN36XX_HAL_FINISH_SCAN_REQ);
 
-	msg_body.mode = mode;
+	msg_body.mode = HAL_SYS_MODE_SCAN;
 
 	PREPARE_HAL_BUF(wcn->hal_buf, msg_body);
 
-	wcn36xx_dbg(WCN36XX_DBG_HAL, "hal init scan mode %d\n", msg_body.mode);
+	wcn36xx_dbg(WCN36XX_DBG_HAL, "hal finish scan mode %d\n",
+		    msg_body.mode);
 
 	ret = wcn36xx_smd_send_and_wait(wcn, msg_body.header.len);
 	if (ret) {
-		wcn36xx_err("Sending hal_init_scan failed\n");
-		goto out;
+		wcn36xx_err("Sending hal_finish_scan failed\n");
+		return ret;
 	}
 	ret = wcn36xx_smd_rsp_status_check(wcn->hal_buf, wcn->hal_rsp_len);
 	if (ret) {
-		wcn36xx_err("hal_init_scan response failed err=%d\n", ret);
-		goto out;
+		wcn36xx_err("hal_finish_scan response failed err=%d\n", ret);
+		return ret;
 	}
-out:
-	mutex_unlock(&wcn->hal_mutex);
-	return ret;
+
+	return 0;
 }
 
-int wcn36xx_smd_start_scan(struct wcn36xx *wcn)
+int _wcn36xx_smd_end_scan(struct wcn36xx *wcn)
 {
-	struct wcn36xx_hal_start_scan_req_msg msg_body;
+	struct wcn36xx_hal_end_scan_req_msg msg_body;
 	int ret = 0;
 
-	mutex_lock(&wcn->hal_mutex);
-	INIT_HAL_MSG(msg_body, WCN36XX_HAL_START_SCAN_REQ);
+	INIT_HAL_MSG(msg_body, WCN36XX_HAL_END_SCAN_REQ);
 
-	msg_body.scan_channel = WCN36XX_HW_CHANNEL(wcn);
+	msg_body.scan_channel = wcn->scan_channel;
 
 	PREPARE_HAL_BUF(wcn->hal_buf, msg_body);
 
-	wcn36xx_dbg(WCN36XX_DBG_HAL, "hal start scan channel %d\n",
+	wcn36xx_dbg(WCN36XX_DBG_HAL, "hal end scan channel %d\n",
 		    msg_body.scan_channel);
 
 	ret = wcn36xx_smd_send_and_wait(wcn, msg_body.header.len);
 	if (ret) {
-		wcn36xx_err("Sending hal_start_scan failed\n");
-		goto out;
+		wcn36xx_err("Sending hal_end_scan failed\n");
+		return ret;
 	}
 	ret = wcn36xx_smd_rsp_status_check(wcn->hal_buf, wcn->hal_rsp_len);
 	if (ret) {
-		wcn36xx_err("hal_start_scan response failed err=%d\n", ret);
-		goto out;
+		wcn36xx_err("hal_end_scan response failed err=%d\n", ret);
+		return ret;
 	}
-out:
-	mutex_unlock(&wcn->hal_mutex);
-	return ret;
+
+	wcn->scan_started = false;
+
+	return 0;
 }
 
-int wcn36xx_smd_end_scan(struct wcn36xx *wcn)
+void wcn36xx_scan_work(struct work_struct *work)
 {
-	struct wcn36xx_hal_end_scan_req_msg msg_body;
+	struct wcn36xx *wcn = container_of(work, struct wcn36xx, scan_work.work);
+	struct wcn36xx_hal_start_scan_req_msg msg_body;
 	int ret = 0;
 
 	mutex_lock(&wcn->hal_mutex);
-	INIT_HAL_MSG(msg_body, WCN36XX_HAL_END_SCAN_REQ);
 
-	msg_body.scan_channel = WCN36XX_HW_CHANNEL(wcn);
+	if (wcn->scan_started) {
+		ret = _wcn36xx_smd_end_scan(wcn);
+		if (ret)
+			goto out;
+
+		/* we just do only the one start -> stop then finish scan */
+		wcn->scan_finishing = true;
+	}
+	if (wcn->scan_finishing) {
+		ret = _wcn36xx_smd_finish_scan(wcn);
+		goto out;
+	}
+
+	INIT_HAL_MSG(msg_body, WCN36XX_HAL_START_SCAN_REQ);
+
+	wcn->scan_channel = WCN36XX_HW_CHANNEL(wcn);
+	msg_body.scan_channel = wcn->scan_channel;
+	dev_dbg(wcn->dev, "%s: ch %d\n", __func__, wcn->scan_channel);
 
 	PREPARE_HAL_BUF(wcn->hal_buf, msg_body);
 
-	wcn36xx_dbg(WCN36XX_DBG_HAL, "hal end scan channel %d\n",
+	wcn36xx_dbg(WCN36XX_DBG_HAL, "hal start scan channel %d\n",
 		    msg_body.scan_channel);
 
 	ret = wcn36xx_smd_send_and_wait(wcn, msg_body.header.len);
 	if (ret) {
-		wcn36xx_err("Sending hal_end_scan failed\n");
+		wcn36xx_err("Sending hal_start_scan failed\n");
 		goto out;
 	}
 	ret = wcn36xx_smd_rsp_status_check(wcn->hal_buf, wcn->hal_rsp_len);
 	if (ret) {
-		wcn36xx_err("hal_end_scan response failed err=%d\n", ret);
+		wcn36xx_err("hal_start_scan response failed err=%d\n", ret);
 		goto out;
 	}
+
+	wcn->scan_started = true;
+	/* we will come back in around 166ms and stop the scan */
+	schedule_delayed_work(&wcn->scan_work, HZ / 6);
+
 out:
 	mutex_unlock(&wcn->hal_mutex);
-	return ret;
 }
 
-int wcn36xx_smd_finish_scan(struct wcn36xx *wcn,
-			    enum wcn36xx_hal_sys_mode mode)
+int wcn36xx_smd_init_scan(struct wcn36xx *wcn)
 {
-	struct wcn36xx_hal_finish_scan_req_msg msg_body;
+	struct wcn36xx_hal_init_scan_req_msg msg_body;
 	int ret = 0;
 
 	mutex_lock(&wcn->hal_mutex);
-	INIT_HAL_MSG(msg_body, WCN36XX_HAL_FINISH_SCAN_REQ);
 
-	msg_body.mode = mode;
+	wcn->scan_started = false;
+	wcn->scan_finishing = false;
+
+	INIT_HAL_MSG(msg_body, WCN36XX_HAL_INIT_SCAN_REQ);
+
+	msg_body.mode = HAL_SYS_MODE_SCAN;
 
 	PREPARE_HAL_BUF(wcn->hal_buf, msg_body);
 
-	wcn36xx_dbg(WCN36XX_DBG_HAL, "hal finish scan mode %d\n",
-		    msg_body.mode);
+	wcn36xx_dbg(WCN36XX_DBG_HAL, "hal init scan mode %d\n", msg_body.mode);
 
 	ret = wcn36xx_smd_send_and_wait(wcn, msg_body.header.len);
 	if (ret) {
-		wcn36xx_err("Sending hal_finish_scan failed\n");
+		wcn36xx_err("Sending hal_init_scan failed\n");
 		goto out;
 	}
 	ret = wcn36xx_smd_rsp_status_check(wcn->hal_buf, wcn->hal_rsp_len);
 	if (ret) {
-		wcn36xx_err("hal_finish_scan response failed err=%d\n", ret);
+		wcn36xx_err("hal_init_scan response failed err=%d\n", ret);
 		goto out;
 	}
+
+	schedule_delayed_work(&wcn->scan_work, 0);
+
 out:
 	mutex_unlock(&wcn->hal_mutex);
 	return ret;
 }
 
+int wcn36xx_smd_finish_scan(struct wcn36xx *wcn)
+{
+	int ret = 0;
+
+	mutex_lock(&wcn->hal_mutex);
+
+	cancel_delayed_work(&wcn->scan_work);
+
+	if (wcn->scan_started) {
+		wcn->scan_finishing = true;
+		ret = _wcn36xx_smd_end_scan(wcn);
+	}
+
+	mutex_unlock(&wcn->hal_mutex);
+	return ret;
+}
+
 static int wcn36xx_smd_switch_channel_rsp(void *buf, size_t len)
 {
 	struct wcn36xx_hal_switch_channel_rsp_msg *rsp;
diff --git a/drivers/net/wireless/ath/wcn36xx/smd.h b/drivers/net/wireless/ath/wcn36xx/smd.h
index 8361f9e3995b..299d60ef7410 100644
--- a/drivers/net/wireless/ath/wcn36xx/smd.h
+++ b/drivers/net/wireless/ath/wcn36xx/smd.h
@@ -67,11 +67,10 @@ void wcn36xx_smd_close(struct wcn36xx *wcn);
 int wcn36xx_smd_load_nv(struct wcn36xx *wcn);
 int wcn36xx_smd_start(struct wcn36xx *wcn);
 int wcn36xx_smd_stop(struct wcn36xx *wcn);
-int wcn36xx_smd_init_scan(struct wcn36xx *wcn, enum wcn36xx_hal_sys_mode mode);
+int wcn36xx_smd_init_scan(struct wcn36xx *wcn);
 int wcn36xx_smd_start_scan(struct wcn36xx *wcn);
 int wcn36xx_smd_end_scan(struct wcn36xx *wcn);
-int wcn36xx_smd_finish_scan(struct wcn36xx *wcn,
-			    enum wcn36xx_hal_sys_mode mode);
+int wcn36xx_smd_finish_scan(struct wcn36xx *wcn);
 int wcn36xx_smd_update_scan_params(struct wcn36xx *wcn);
 int wcn36xx_smd_add_sta_self(struct wcn36xx *wcn, struct ieee80211_vif *vif);
 int wcn36xx_smd_delete_sta_self(struct wcn36xx *wcn, u8 *addr);
diff --git a/drivers/net/wireless/ath/wcn36xx/wcn36xx.h b/drivers/net/wireless/ath/wcn36xx/wcn36xx.h
index 04793c687682..82d96bb7642e 100644
--- a/drivers/net/wireless/ath/wcn36xx/wcn36xx.h
+++ b/drivers/net/wireless/ath/wcn36xx/wcn36xx.h
@@ -228,6 +228,12 @@ struct wcn36xx {
 
 	struct sk_buff		*tx_ack_skb;
 
+	/* we restrict the amount of time scan can block normal operation */
+	struct delayed_work 	scan_work;
+	bool			scan_started;
+	bool			scan_finishing;
+	u8			scan_channel;
+
 #ifdef CONFIG_WCN36XX_DEBUGFS
 	/* Debug file system entry */
 	struct wcn36xx_dfs_entry    dfs;
@@ -247,5 +253,6 @@ static inline bool wcn36xx_is_fw_version(struct wcn36xx *wcn,
 		wcn->fw_revision == revision);
 }
 void wcn36xx_set_default_rates(struct wcn36xx_hal_supported_rates *rates);
+void wcn36xx_scan_work(struct work_struct *work);
 
 #endif	/* _WCN36XX_H_ */