* [PATCH 1/3] wcn36xx: add debug prints for sw_scan start/complete
2021-10-23 0:39 [PATCH 0/3] wcn36xx: software scanning improvements Benjamin Li
@ 2021-10-23 0:39 ` Benjamin Li
2021-10-27 7:53 ` Kalle Valo
2021-10-23 0:39 ` [PATCH 2/3] wcn36xx: implement flush op to speed up connected scan Benjamin Li
2021-10-23 0:39 ` [PATCH 3/3] wcn36xx: ensure pairing of init_scan/finish_scan and start_scan/end_scan Benjamin Li
2 siblings, 1 reply; 5+ messages in thread
From: Benjamin Li @ 2021-10-23 0:39 UTC (permalink / raw)
To: Kalle Valo
Cc: Joseph Gates, Bryan O'Donoghue, Loic Poulain, linux-arm-msm,
Benjamin Li, David S. Miller, Jakub Kicinski, Eugene Krasnikov,
John W. Linville, wcn36xx, linux-wireless, netdev, linux-kernel
Add some MAC debug prints for more easily demarcating a software scan
when parsing logs.
Signed-off-by: Benjamin Li <benl@squareup.com>
---
drivers/net/wireless/ath/wcn36xx/main.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/drivers/net/wireless/ath/wcn36xx/main.c b/drivers/net/wireless/ath/wcn36xx/main.c
index 263af65a889ab..81ac86eeaf60b 100644
--- a/drivers/net/wireless/ath/wcn36xx/main.c
+++ b/drivers/net/wireless/ath/wcn36xx/main.c
@@ -711,6 +711,8 @@ static void wcn36xx_sw_scan_start(struct ieee80211_hw *hw,
struct wcn36xx *wcn = hw->priv;
struct wcn36xx_vif *vif_priv = wcn36xx_vif_to_priv(vif);
+ wcn36xx_dbg(WCN36XX_DBG_MAC, "sw_scan_start");
+
wcn->sw_scan = true;
wcn->sw_scan_vif = vif;
wcn->sw_scan_channel = 0;
@@ -725,6 +727,8 @@ static void wcn36xx_sw_scan_complete(struct ieee80211_hw *hw,
{
struct wcn36xx *wcn = hw->priv;
+ wcn36xx_dbg(WCN36XX_DBG_MAC, "sw_scan_complete");
+
/* ensure that any scan session is finished */
wcn36xx_smd_finish_scan(wcn, HAL_SYS_MODE_SCAN, wcn->sw_scan_vif);
wcn->sw_scan = false;
--
2.25.1
^ permalink raw reply related [flat|nested] 5+ messages in thread* [PATCH 2/3] wcn36xx: implement flush op to speed up connected scan
2021-10-23 0:39 [PATCH 0/3] wcn36xx: software scanning improvements Benjamin Li
2021-10-23 0:39 ` [PATCH 1/3] wcn36xx: add debug prints for sw_scan start/complete Benjamin Li
@ 2021-10-23 0:39 ` Benjamin Li
2021-10-23 0:39 ` [PATCH 3/3] wcn36xx: ensure pairing of init_scan/finish_scan and start_scan/end_scan Benjamin Li
2 siblings, 0 replies; 5+ messages in thread
From: Benjamin Li @ 2021-10-23 0:39 UTC (permalink / raw)
To: Kalle Valo
Cc: Joseph Gates, Bryan O'Donoghue, Loic Poulain, linux-arm-msm,
Benjamin Li, David S. Miller, Jakub Kicinski, Eugene Krasnikov,
John W. Linville, wcn36xx, linux-wireless, netdev, linux-kernel
Without ieee80211_ops->flush implemented to empty HW queues, mac80211 will
do a 100ms dead wait after stopping SW queues, before leaving the operating
channel to resume a software connected scan[1].
(see ieee80211_scan_state_resume)
This wait is correctly included in the calculation for whether or not
we've exceeded max off-channel time, as it occurs after sending the null
frame with PS bit set. Thus, with 125 ms max off-channel time we only
have 25 ms of scan time, which technically isn't even enough to scan one
channel (although mac80211 always scans at least one channel per off-
channel window).
Moreover, for passive probes we end up spending at least 100 ms + 111 ms
(IEEE80211_PASSIVE_CHANNEL_TIME) "off-channel"[2], which exceeds the listen
interval of 200 ms that we provide in our association request frame. That's
technically out-of-spec.
[1]: Until recently, wcn36xx performed software (rather than FW-offloaded)
scanning when 5GHz channels are requested. This apparent limitation is now
resolved -- see commit 1395f8a6a4d5 ("wcn36xx: Enable hardware scan offload
for 5Ghz band").
[2]: in quotes because about 100 ms of it is still on-channel but with PS
set
Signed-off-by: Benjamin Li <benl@squareup.com>
---
drivers/net/wireless/ath/wcn36xx/dxe.c | 47 +++++++++++++++++++++++++
drivers/net/wireless/ath/wcn36xx/dxe.h | 1 +
drivers/net/wireless/ath/wcn36xx/main.c | 11 ++++++
3 files changed, 59 insertions(+)
diff --git a/drivers/net/wireless/ath/wcn36xx/dxe.c b/drivers/net/wireless/ath/wcn36xx/dxe.c
index 8e1dbfda65386..c4e9e939d7d6d 100644
--- a/drivers/net/wireless/ath/wcn36xx/dxe.c
+++ b/drivers/net/wireless/ath/wcn36xx/dxe.c
@@ -831,6 +831,53 @@ int wcn36xx_dxe_tx_frame(struct wcn36xx *wcn,
return ret;
}
+static bool _wcn36xx_dxe_tx_channel_is_empty(struct wcn36xx_dxe_ch *ch)
+{
+ int flags;
+ struct wcn36xx_dxe_ctl *ctl_bd_start, *ctl_skb_start;
+ struct wcn36xx_dxe_ctl *ctl_bd, *ctl_skb;
+ bool ret = true;
+
+ spin_lock_irqsave(&ch->lock, flags);
+
+ /* Loop through ring buffer looking for nonempty entries. */
+ ctl_bd_start = ch->head_blk_ctl;
+ ctl_bd = ctl_bd_start;
+ ctl_skb_start = ctl_bd_start->next;
+ ctl_skb = ctl_skb_start;
+ do {
+ if (ctl_skb->skb) {
+ ret = false;
+ goto unlock;
+ }
+ ctl_bd = ctl_skb->next;
+ ctl_skb = ctl_bd->next;
+ } while (ctl_skb != ctl_skb_start);
+
+unlock:
+ spin_unlock_irqrestore(&ch->lock, flags);
+ return ret;
+}
+
+int wcn36xx_dxe_tx_flush(struct wcn36xx *wcn)
+{
+ int i = 0;
+
+ /* Called with mac80211 queues stopped. Wait for empty HW queues. */
+ do {
+ if (_wcn36xx_dxe_tx_channel_is_empty(&wcn->dxe_tx_l_ch) &&
+ _wcn36xx_dxe_tx_channel_is_empty(&wcn->dxe_tx_h_ch)) {
+ return 0;
+ }
+ /* This ieee80211_ops callback is specifically allowed to
+ * sleep.
+ */
+ usleep_range(1000, 1100);
+ } while (++i < 100);
+
+ return -EBUSY;
+}
+
int wcn36xx_dxe_init(struct wcn36xx *wcn)
{
int reg_data = 0, ret;
diff --git a/drivers/net/wireless/ath/wcn36xx/dxe.h b/drivers/net/wireless/ath/wcn36xx/dxe.h
index 31b81b7547a32..26a31edf52e99 100644
--- a/drivers/net/wireless/ath/wcn36xx/dxe.h
+++ b/drivers/net/wireless/ath/wcn36xx/dxe.h
@@ -466,5 +466,6 @@ int wcn36xx_dxe_tx_frame(struct wcn36xx *wcn,
struct wcn36xx_tx_bd *bd,
struct sk_buff *skb,
bool is_low);
+int wcn36xx_dxe_tx_flush(struct wcn36xx *wcn);
void wcn36xx_dxe_tx_ack_ind(struct wcn36xx *wcn, u32 status);
#endif /* _DXE_H_ */
diff --git a/drivers/net/wireless/ath/wcn36xx/main.c b/drivers/net/wireless/ath/wcn36xx/main.c
index 81ac86eeaf60b..24bfb30a30f31 100644
--- a/drivers/net/wireless/ath/wcn36xx/main.c
+++ b/drivers/net/wireless/ath/wcn36xx/main.c
@@ -1275,6 +1275,16 @@ static void wcn36xx_ipv6_addr_change(struct ieee80211_hw *hw,
}
#endif
+static void wcn36xx_flush(struct ieee80211_hw *hw, struct ieee80211_vif *vif,
+ u32 queues, bool drop)
+{
+ struct wcn36xx *wcn = hw->priv;
+
+ if (wcn36xx_dxe_tx_flush(wcn)) {
+ wcn36xx_err("Failed to flush hardware tx queues\n");
+ }
+}
+
static const struct ieee80211_ops wcn36xx_ops = {
.start = wcn36xx_start,
.stop = wcn36xx_stop,
@@ -1302,6 +1312,7 @@ static const struct ieee80211_ops wcn36xx_ops = {
#if IS_ENABLED(CONFIG_IPV6)
.ipv6_addr_change = wcn36xx_ipv6_addr_change,
#endif
+ .flush = wcn36xx_flush,
CFG80211_TESTMODE_CMD(wcn36xx_tm_cmd)
};
--
2.25.1
^ permalink raw reply related [flat|nested] 5+ messages in thread* [PATCH 3/3] wcn36xx: ensure pairing of init_scan/finish_scan and start_scan/end_scan
2021-10-23 0:39 [PATCH 0/3] wcn36xx: software scanning improvements Benjamin Li
2021-10-23 0:39 ` [PATCH 1/3] wcn36xx: add debug prints for sw_scan start/complete Benjamin Li
2021-10-23 0:39 ` [PATCH 2/3] wcn36xx: implement flush op to speed up connected scan Benjamin Li
@ 2021-10-23 0:39 ` Benjamin Li
2 siblings, 0 replies; 5+ messages in thread
From: Benjamin Li @ 2021-10-23 0:39 UTC (permalink / raw)
To: Kalle Valo
Cc: Joseph Gates, Bryan O'Donoghue, Loic Poulain, linux-arm-msm,
Benjamin Li, David S. Miller, Jakub Kicinski, Eugene Krasnikov,
John W. Linville, wcn36xx, linux-wireless, netdev, linux-kernel
An SMD capture from the downstream prima driver on WCN3680B shows the
following command sequence for connected scans:
- init_scan_req
- start_scan_req, channel 1
- end_scan_req, channel 1
- start_scan_req, channel 2
- ...
- end_scan_req, channel 3
- finish_scan_req
- init_scan_req
- start_scan_req, channel 4
- ...
- end_scan_req, channel 6
- finish_scan_req
- ...
- end_scan_req, channel 165
- finish_scan_req
Upstream currently never calls wcn36xx_smd_end_scan, and in some cases[1]
still sends finish_scan_req twice in a row or before init_scan_req. A
typical connected scan looks like this:
- init_scan_req
- start_scan_req, channel 1
- finish_scan_req
- init_scan_req
- start_scan_req, channel 2
- ...
- start_scan_req, channel 165
- finish_scan_req
- finish_scan_req
This patch cleans up scanning so that init/finish and start/end are always
paired together and correctly nested.
- init_scan_req
- start_scan_req, channel 1
- end_scan_req, channel 1
- finish_scan_req
- init_scan_req
- start_scan_req, channel 2
- end_scan_req, channel 2
- ...
- start_scan_req, channel 165
- end_scan_req, channel 165
- finish_scan_req
Note that upstream will not do batching of 3 active-probe scans before
returning to the operating channel, and this patch does not change that.
To match downstream in this aspect, adjust IEEE80211_PROBE_DELAY and/or
the 125ms max off-channel time in ieee80211_scan_state_decision.
[1]: commit d195d7aac09b ("wcn36xx: Ensure finish scan is not requested
before start scan") addressed one case of finish_scan_req being sent
without a preceding init_scan_req (the case of the operating channel
coinciding with the first scan channel); two other cases are:
1) if SW scan is started and aborted immediately, without scanning any
channels, we send a finish_scan_req without ever sending init_scan_req,
and
2) as SW scan logic always returns us to the operating channel before
calling wcn36xx_sw_scan_complete, finish_scan_req is always sent twice
at the end of a SW scan
Fixes: 8e84c2582169 ("wcn36xx: mac80211 driver for Qualcomm WCN3660/WCN3680 hardware")
Signed-off-by: Benjamin Li <benl@squareup.com>
---
drivers/net/wireless/ath/wcn36xx/main.c | 34 +++++++++++++++++-----
drivers/net/wireless/ath/wcn36xx/smd.c | 4 +++
drivers/net/wireless/ath/wcn36xx/wcn36xx.h | 1 +
3 files changed, 32 insertions(+), 7 deletions(-)
diff --git a/drivers/net/wireless/ath/wcn36xx/main.c b/drivers/net/wireless/ath/wcn36xx/main.c
index 24bfb30a30f31..a6f4a22417b36 100644
--- a/drivers/net/wireless/ath/wcn36xx/main.c
+++ b/drivers/net/wireless/ath/wcn36xx/main.c
@@ -398,6 +398,7 @@ static void wcn36xx_change_opchannel(struct wcn36xx *wcn, int ch)
static int wcn36xx_config(struct ieee80211_hw *hw, u32 changed)
{
struct wcn36xx *wcn = hw->priv;
+ int ret;
wcn36xx_dbg(WCN36XX_DBG_MAC, "mac config changed 0x%08x\n", changed);
@@ -413,17 +414,31 @@ static int wcn36xx_config(struct ieee80211_hw *hw, u32 changed)
* want to receive/transmit regular data packets, then
* simply stop the scan session and exit PS mode.
*/
- wcn36xx_smd_finish_scan(wcn, HAL_SYS_MODE_SCAN,
- wcn->sw_scan_vif);
- wcn->sw_scan_channel = 0;
+ if (wcn->sw_scan_channel)
+ wcn36xx_smd_end_scan(wcn, wcn->sw_scan_channel);
+ if (wcn->sw_scan_init) {
+ wcn36xx_smd_finish_scan(wcn, HAL_SYS_MODE_SCAN,
+ wcn->sw_scan_vif);
+ }
} else if (wcn->sw_scan) {
/* A scan is ongoing, do not change the operating
* channel, but start a scan session on the channel.
*/
- wcn36xx_smd_init_scan(wcn, HAL_SYS_MODE_SCAN,
- wcn->sw_scan_vif);
+ if (wcn->sw_scan_channel)
+ wcn36xx_smd_end_scan(wcn, wcn->sw_scan_channel);
+ if (!wcn->sw_scan_init) {
+ /* This can fail if we are unable to notify the
+ * operating channel.
+ */
+ ret = wcn36xx_smd_init_scan(wcn,
+ HAL_SYS_MODE_SCAN,
+ wcn->sw_scan_vif);
+ if (ret) {
+ mutex_unlock(&wcn->conf_mutex);
+ return -EIO;
+ }
+ }
wcn36xx_smd_start_scan(wcn, ch);
- wcn->sw_scan_channel = ch;
} else {
wcn36xx_change_opchannel(wcn, ch);
}
@@ -730,7 +745,12 @@ static void wcn36xx_sw_scan_complete(struct ieee80211_hw *hw,
wcn36xx_dbg(WCN36XX_DBG_MAC, "sw_scan_complete");
/* ensure that any scan session is finished */
- wcn36xx_smd_finish_scan(wcn, HAL_SYS_MODE_SCAN, wcn->sw_scan_vif);
+ if (wcn->sw_scan_channel)
+ wcn36xx_smd_end_scan(wcn, wcn->sw_scan_channel);
+ if (wcn->sw_scan_init) {
+ wcn36xx_smd_finish_scan(wcn, HAL_SYS_MODE_SCAN,
+ wcn->sw_scan_vif);
+ }
wcn->sw_scan = false;
wcn->sw_scan_opchannel = 0;
}
diff --git a/drivers/net/wireless/ath/wcn36xx/smd.c b/drivers/net/wireless/ath/wcn36xx/smd.c
index 3979171c92dd2..e0de5da31158c 100644
--- a/drivers/net/wireless/ath/wcn36xx/smd.c
+++ b/drivers/net/wireless/ath/wcn36xx/smd.c
@@ -720,6 +720,7 @@ int wcn36xx_smd_init_scan(struct wcn36xx *wcn, enum wcn36xx_hal_sys_mode mode,
wcn36xx_err("hal_init_scan response failed err=%d\n", ret);
goto out;
}
+ wcn->sw_scan_init = true;
out:
mutex_unlock(&wcn->hal_mutex);
return ret;
@@ -750,6 +751,7 @@ int wcn36xx_smd_start_scan(struct wcn36xx *wcn, u8 scan_channel)
wcn36xx_err("hal_start_scan response failed err=%d\n", ret);
goto out;
}
+ wcn->sw_scan_channel = scan_channel;
out:
mutex_unlock(&wcn->hal_mutex);
return ret;
@@ -780,6 +782,7 @@ int wcn36xx_smd_end_scan(struct wcn36xx *wcn, u8 scan_channel)
wcn36xx_err("hal_end_scan response failed err=%d\n", ret);
goto out;
}
+ wcn->sw_scan_channel = 0;
out:
mutex_unlock(&wcn->hal_mutex);
return ret;
@@ -821,6 +824,7 @@ int wcn36xx_smd_finish_scan(struct wcn36xx *wcn,
wcn36xx_err("hal_finish_scan response failed err=%d\n", ret);
goto out;
}
+ wcn->sw_scan_init = false;
out:
mutex_unlock(&wcn->hal_mutex);
return ret;
diff --git a/drivers/net/wireless/ath/wcn36xx/wcn36xx.h b/drivers/net/wireless/ath/wcn36xx/wcn36xx.h
index add6e527e8330..d2a2769673716 100644
--- a/drivers/net/wireless/ath/wcn36xx/wcn36xx.h
+++ b/drivers/net/wireless/ath/wcn36xx/wcn36xx.h
@@ -247,6 +247,7 @@ struct wcn36xx {
struct cfg80211_scan_request *scan_req;
bool sw_scan;
u8 sw_scan_opchannel;
+ bool sw_scan_init;
u8 sw_scan_channel;
struct ieee80211_vif *sw_scan_vif;
struct mutex scan_lock;
--
2.25.1
^ permalink raw reply related [flat|nested] 5+ messages in thread