All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 0/9] wfx: implement Remain On Channel
@ 2023-09-27 16:32 Jérôme Pouiller
  2023-09-27 16:32 ` [PATCH v2 1/9] wifi: wfx: fix power_save setting when AP is stopped Jérôme Pouiller
                   ` (8 more replies)
  0 siblings, 9 replies; 16+ messages in thread
From: Jérôme Pouiller @ 2023-09-27 16:32 UTC (permalink / raw)
  To: Kalle Valo; +Cc: linux-wireless, linux-kernel, Jérôme Pouiller

Hello,

Apart from the 3 first patch, this series implements Remain On Channel for
WF200 chips. The implementation is a bit twisted (I hijack the scan feature
to implements RoC). However, it has been extensively tested with
DPP/EasyConnect and I have not noticed any issue.

v2:
  - Rebase on last stable tree


Jérôme Pouiller (9):
  wifi: wfx: fix power_save setting when AP is stopped
  wifi: wfx: relocate wfx_rate_mask_to_hw()
  wifi: wfx: move wfx_skb_*() out of the header file
  wifi: wfx: introduce hif_scan_uniq()
  wifi: wfx: add placeholders for remain_on_channel feature
  wifi: wfx: implement wfx_remain_on_channel()
  wifi: wfx: allow to send frames during ROC
  wifi: wfx: scan_lock is global to the device
  wifi: wfx: fix possible lock-up between scan and Rx filters

 drivers/net/wireless/silabs/wfx/data_tx.c | 54 ++++++++++++++++---
 drivers/net/wireless/silabs/wfx/data_tx.h | 21 ++------
 drivers/net/wireless/silabs/wfx/hif_tx.c  | 43 +++++++++++++++
 drivers/net/wireless/silabs/wfx/hif_tx.h  |  1 +
 drivers/net/wireless/silabs/wfx/main.c    |  5 ++
 drivers/net/wireless/silabs/wfx/queue.c   | 38 ++++++++++---
 drivers/net/wireless/silabs/wfx/queue.h   |  1 +
 drivers/net/wireless/silabs/wfx/scan.c    | 66 ++++++++++++++++++++++-
 drivers/net/wireless/silabs/wfx/scan.h    |  6 +++
 drivers/net/wireless/silabs/wfx/sta.c     | 41 +++++---------
 drivers/net/wireless/silabs/wfx/sta.h     |  1 -
 drivers/net/wireless/silabs/wfx/wfx.h     |  8 +--
 12 files changed, 218 insertions(+), 67 deletions(-)

-- 
2.39.2


^ permalink raw reply	[flat|nested] 16+ messages in thread
* [PATCH v2 1/9] wifi: wfx: fix power_save setting when AP is stopped
  2023-09-27 16:32 [PATCH v2 0/9] wfx: implement Remain On Channel Jérôme Pouiller
@ 2023-09-27 16:32 ` Jérôme Pouiller
  2023-10-04 10:20   ` Kalle Valo
  2023-09-27 16:32 ` [PATCH v2 2/9] wifi: wfx: relocate wfx_rate_mask_to_hw() Jérôme Pouiller
                   ` (7 subsequent siblings)
  8 siblings, 1 reply; 16+ messages in thread
From: Jérôme Pouiller @ 2023-09-27 16:32 UTC (permalink / raw)
  To: Kalle Valo; +Cc: linux-wireless, linux-kernel, Jérôme Pouiller

The WF200 allow to start two network interfaces (one AP, one station) on
two different channels. Since magic does not exist, it only works if the
station interface enables power save.

Thus, the driver detects this case and enforce power save as necessary.

This patch fixes the case where the AP interface is stopped and it is no
more necessary to enforce power saving on the station interface.

Signed-off-by: Jérôme Pouiller <jerome.pouiller@silabs.com>
---
 drivers/net/wireless/silabs/wfx/sta.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/net/wireless/silabs/wfx/sta.c b/drivers/net/wireless/silabs/wfx/sta.c
index 626dfb4b7a55d..9c0a11c277e97 100644
--- a/drivers/net/wireless/silabs/wfx/sta.c
+++ b/drivers/net/wireless/silabs/wfx/sta.c
@@ -402,7 +402,12 @@ void wfx_stop_ap(struct ieee80211_hw *hw, struct ieee80211_vif *vif,
 		 struct ieee80211_bss_conf *link_conf)
 {
 	struct wfx_vif *wvif = (struct wfx_vif *)vif->drv_priv;
+	struct wfx_dev *wdev = wvif->wdev;
 
+	wvif =  NULL;
+	while ((wvif = wvif_iterate(wdev, wvif)) != NULL)
+		wfx_update_pm(wvif);
+	wvif = (struct wfx_vif *)vif->drv_priv;
 	wfx_reset(wvif);
 }
 
-- 
2.39.2


^ permalink raw reply related	[flat|nested] 16+ messages in thread
* [PATCH v2 2/9] wifi: wfx: relocate wfx_rate_mask_to_hw()
  2023-09-27 16:32 [PATCH v2 0/9] wfx: implement Remain On Channel Jérôme Pouiller
  2023-09-27 16:32 ` [PATCH v2 1/9] wifi: wfx: fix power_save setting when AP is stopped Jérôme Pouiller
@ 2023-09-27 16:32 ` Jérôme Pouiller
  2023-09-27 16:32 ` [PATCH v2 3/9] wifi: wfx: move wfx_skb_*() out of the header file Jérôme Pouiller
                   ` (6 subsequent siblings)
  8 siblings, 0 replies; 16+ messages in thread
From: Jérôme Pouiller @ 2023-09-27 16:32 UTC (permalink / raw)
  To: Kalle Valo; +Cc: linux-wireless, linux-kernel, Jérôme Pouiller

wfx_rate_mask_to_hw() is only used in hif_tx.c. So relocate it into
hif_tx.c and mark it static.

Signed-off-by: Jérôme Pouiller <jerome.pouiller@silabs.com>
---
 drivers/net/wireless/silabs/wfx/hif_tx.c | 18 ++++++++++++++++++
 drivers/net/wireless/silabs/wfx/sta.c    | 18 ------------------
 drivers/net/wireless/silabs/wfx/sta.h    |  1 -
 3 files changed, 18 insertions(+), 19 deletions(-)

diff --git a/drivers/net/wireless/silabs/wfx/hif_tx.c b/drivers/net/wireless/silabs/wfx/hif_tx.c
index 9402503fbde3c..de5a31482df38 100644
--- a/drivers/net/wireless/silabs/wfx/hif_tx.c
+++ b/drivers/net/wireless/silabs/wfx/hif_tx.c
@@ -45,6 +45,24 @@ static void *wfx_alloc_hif(size_t body_len, struct wfx_hif_msg **hif)
 		return NULL;
 }
 
+static u32 wfx_rate_mask_to_hw(struct wfx_dev *wdev, u32 rates)
+{
+	int i;
+	u32 ret = 0;
+	/* The device only supports 2GHz */
+	struct ieee80211_supported_band *sband = wdev->hw->wiphy->bands[NL80211_BAND_2GHZ];
+
+	for (i = 0; i < sband->n_bitrates; i++) {
+		if (rates & BIT(i)) {
+			if (i >= sband->n_bitrates)
+				dev_warn(wdev->dev, "unsupported basic rate\n");
+			else
+				ret |= BIT(sband->bitrates[i].hw_value);
+		}
+	}
+	return ret;
+}
+
 int wfx_cmd_send(struct wfx_dev *wdev, struct wfx_hif_msg *request,
 		 void *reply, size_t reply_len, bool no_reply)
 {
diff --git a/drivers/net/wireless/silabs/wfx/sta.c b/drivers/net/wireless/silabs/wfx/sta.c
index 9c0a11c277e97..c58db2bcea87b 100644
--- a/drivers/net/wireless/silabs/wfx/sta.c
+++ b/drivers/net/wireless/silabs/wfx/sta.c
@@ -20,24 +20,6 @@
 
 #define HIF_MAX_ARP_IP_ADDRTABLE_ENTRIES 2
 
-u32 wfx_rate_mask_to_hw(struct wfx_dev *wdev, u32 rates)
-{
-	int i;
-	u32 ret = 0;
-	/* The device only supports 2GHz */
-	struct ieee80211_supported_band *sband = wdev->hw->wiphy->bands[NL80211_BAND_2GHZ];
-
-	for (i = 0; i < sband->n_bitrates; i++) {
-		if (rates & BIT(i)) {
-			if (i >= sband->n_bitrates)
-				dev_warn(wdev->dev, "unsupported basic rate\n");
-			else
-				ret |= BIT(sband->bitrates[i].hw_value);
-		}
-	}
-	return ret;
-}
-
 void wfx_cooling_timeout_work(struct work_struct *work)
 {
 	struct wfx_dev *wdev = container_of(to_delayed_work(work), struct wfx_dev,
diff --git a/drivers/net/wireless/silabs/wfx/sta.h b/drivers/net/wireless/silabs/wfx/sta.h
index 888db5cd3206b..c478ddcb934bd 100644
--- a/drivers/net/wireless/silabs/wfx/sta.h
+++ b/drivers/net/wireless/silabs/wfx/sta.h
@@ -66,6 +66,5 @@ int wfx_update_pm(struct wfx_vif *wvif);
 
 /* Other Helpers */
 void wfx_reset(struct wfx_vif *wvif);
-u32 wfx_rate_mask_to_hw(struct wfx_dev *wdev, u32 rates);
 
 #endif
-- 
2.39.2


^ permalink raw reply related	[flat|nested] 16+ messages in thread
* [PATCH v2 3/9] wifi: wfx: move wfx_skb_*() out of the header file
  2023-09-27 16:32 [PATCH v2 0/9] wfx: implement Remain On Channel Jérôme Pouiller
  2023-09-27 16:32 ` [PATCH v2 1/9] wifi: wfx: fix power_save setting when AP is stopped Jérôme Pouiller
  2023-09-27 16:32 ` [PATCH v2 2/9] wifi: wfx: relocate wfx_rate_mask_to_hw() Jérôme Pouiller
@ 2023-09-27 16:32 ` Jérôme Pouiller
  2023-09-27 16:32 ` [PATCH v2 4/9] wifi: wfx: introduce hif_scan_uniq() Jérôme Pouiller
                   ` (5 subsequent siblings)
  8 siblings, 0 replies; 16+ messages in thread
From: Jérôme Pouiller @ 2023-09-27 16:32 UTC (permalink / raw)
  To: Kalle Valo; +Cc: linux-wireless, linux-kernel, Jérôme Pouiller

There is no real reasons to keep these function in the header file.

Signed-off-by: Jérôme Pouiller <jerome.pouiller@silabs.com>
---
 drivers/net/wireless/silabs/wfx/data_tx.c | 18 ++++++++++++++++++
 drivers/net/wireless/silabs/wfx/data_tx.h | 19 ++-----------------
 2 files changed, 20 insertions(+), 17 deletions(-)

diff --git a/drivers/net/wireless/silabs/wfx/data_tx.c b/drivers/net/wireless/silabs/wfx/data_tx.c
index 6a5e52a96d183..ce2b5dcfd8d89 100644
--- a/drivers/net/wireless/silabs/wfx/data_tx.c
+++ b/drivers/net/wireless/silabs/wfx/data_tx.c
@@ -208,6 +208,24 @@ static bool wfx_is_action_back(struct ieee80211_hdr *hdr)
 	return true;
 }
 
+struct wfx_tx_priv *wfx_skb_tx_priv(struct sk_buff *skb)
+{
+	struct ieee80211_tx_info *tx_info;
+
+	if (!skb)
+		return NULL;
+	tx_info = IEEE80211_SKB_CB(skb);
+	return (struct wfx_tx_priv *)tx_info->rate_driver_data;
+}
+
+struct wfx_hif_req_tx *wfx_skb_txreq(struct sk_buff *skb)
+{
+	struct wfx_hif_msg *hif = (struct wfx_hif_msg *)skb->data;
+	struct wfx_hif_req_tx *req = (struct wfx_hif_req_tx *)hif->body;
+
+	return req;
+}
+
 static u8 wfx_tx_get_link_id(struct wfx_vif *wvif, struct ieee80211_sta *sta,
 			     struct ieee80211_hdr *hdr)
 {
diff --git a/drivers/net/wireless/silabs/wfx/data_tx.h b/drivers/net/wireless/silabs/wfx/data_tx.h
index 983470705e4bb..a5b80eacce39a 100644
--- a/drivers/net/wireless/silabs/wfx/data_tx.h
+++ b/drivers/net/wireless/silabs/wfx/data_tx.h
@@ -45,22 +45,7 @@ void wfx_tx(struct ieee80211_hw *hw, struct ieee80211_tx_control *control, struc
 void wfx_tx_confirm_cb(struct wfx_dev *wdev, const struct wfx_hif_cnf_tx *arg);
 void wfx_flush(struct ieee80211_hw *hw, struct ieee80211_vif *vif, u32 queues, bool drop);
 
-static inline struct wfx_tx_priv *wfx_skb_tx_priv(struct sk_buff *skb)
-{
-	struct ieee80211_tx_info *tx_info;
-
-	if (!skb)
-		return NULL;
-	tx_info = IEEE80211_SKB_CB(skb);
-	return (struct wfx_tx_priv *)tx_info->rate_driver_data;
-}
-
-static inline struct wfx_hif_req_tx *wfx_skb_txreq(struct sk_buff *skb)
-{
-	struct wfx_hif_msg *hif = (struct wfx_hif_msg *)skb->data;
-	struct wfx_hif_req_tx *req = (struct wfx_hif_req_tx *)hif->body;
-
-	return req;
-}
+struct wfx_tx_priv *wfx_skb_tx_priv(struct sk_buff *skb);
+struct wfx_hif_req_tx *wfx_skb_txreq(struct sk_buff *skb);
 
 #endif
-- 
2.39.2


^ permalink raw reply related	[flat|nested] 16+ messages in thread
* [PATCH v2 4/9] wifi: wfx: introduce hif_scan_uniq()
  2023-09-27 16:32 [PATCH v2 0/9] wfx: implement Remain On Channel Jérôme Pouiller
                   ` (2 preceding siblings ...)
  2023-09-27 16:32 ` [PATCH v2 3/9] wifi: wfx: move wfx_skb_*() out of the header file Jérôme Pouiller
@ 2023-09-27 16:32 ` Jérôme Pouiller
  2023-09-27 16:32 ` [PATCH v2 5/9] wifi: wfx: add placeholders for remain_on_channel feature Jérôme Pouiller
                   ` (4 subsequent siblings)
  8 siblings, 0 replies; 16+ messages in thread
From: Jérôme Pouiller @ 2023-09-27 16:32 UTC (permalink / raw)
  To: Kalle Valo; +Cc: linux-wireless, linux-kernel, Jérôme Pouiller

Like hof_scan(), hif_scan_uniq() invoke HIF_SCAN. However, it only
allows to probe one channel and disable probe requests. It works very
well to implement Remain-On-Channel.

Signed-off-by: Jérôme Pouiller <jerome.pouiller@silabs.com>
---
 drivers/net/wireless/silabs/wfx/hif_tx.c | 25 ++++++++++++++++++++++++
 drivers/net/wireless/silabs/wfx/hif_tx.h |  1 +
 2 files changed, 26 insertions(+)

diff --git a/drivers/net/wireless/silabs/wfx/hif_tx.c b/drivers/net/wireless/silabs/wfx/hif_tx.c
index de5a31482df38..9f403d275cb13 100644
--- a/drivers/net/wireless/silabs/wfx/hif_tx.c
+++ b/drivers/net/wireless/silabs/wfx/hif_tx.c
@@ -238,6 +238,31 @@ int wfx_hif_write_mib(struct wfx_dev *wdev, int vif_id, u16 mib_id, void *val, s
 	return ret;
 }
 
+/* Hijack scan request to implement Remain-On-Channel */
+int wfx_hif_scan_uniq(struct wfx_vif *wvif, struct ieee80211_channel *chan, int duration)
+{
+	int ret;
+	struct wfx_hif_msg *hif;
+	size_t buf_len = sizeof(struct wfx_hif_req_start_scan_alt) + sizeof(u8);
+	struct wfx_hif_req_start_scan_alt *body = wfx_alloc_hif(buf_len, &hif);
+
+	if (!hif)
+		return -ENOMEM;
+	body->num_of_ssids = HIF_API_MAX_NB_SSIDS;
+	body->maintain_current_bss = 1;
+	body->disallow_ps = 1;
+	body->tx_power_level = cpu_to_le32(chan->max_power);
+	body->num_of_channels = 1;
+	body->channel_list[0] = chan->hw_value;
+	body->max_transmit_rate = API_RATE_INDEX_B_1MBPS;
+	body->min_channel_time = cpu_to_le32(duration);
+	body->max_channel_time = cpu_to_le32(duration * 110 / 100);
+	wfx_fill_header(hif, wvif->id, HIF_REQ_ID_START_SCAN, buf_len);
+	ret = wfx_cmd_send(wvif->wdev, hif, NULL, 0, false);
+	kfree(hif);
+	return ret;
+}
+
 int wfx_hif_scan(struct wfx_vif *wvif, struct cfg80211_scan_request *req,
 		 int chan_start_idx, int chan_num)
 {
diff --git a/drivers/net/wireless/silabs/wfx/hif_tx.h b/drivers/net/wireless/silabs/wfx/hif_tx.h
index 71817a6571f0b..aab54df6aafa6 100644
--- a/drivers/net/wireless/silabs/wfx/hif_tx.h
+++ b/drivers/net/wireless/silabs/wfx/hif_tx.h
@@ -54,6 +54,7 @@ int wfx_hif_beacon_transmit(struct wfx_vif *wvif, bool enable);
 int wfx_hif_update_ie_beacon(struct wfx_vif *wvif, const u8 *ies, size_t ies_len);
 int wfx_hif_scan(struct wfx_vif *wvif, struct cfg80211_scan_request *req80211,
 		 int chan_start, int chan_num);
+int wfx_hif_scan_uniq(struct wfx_vif *wvif, struct ieee80211_channel *chan, int duration);
 int wfx_hif_stop_scan(struct wfx_vif *wvif);
 int wfx_hif_configuration(struct wfx_dev *wdev, const u8 *conf, size_t len);
 int wfx_hif_shutdown(struct wfx_dev *wdev);
-- 
2.39.2


^ permalink raw reply related	[flat|nested] 16+ messages in thread
* [PATCH v2 5/9] wifi: wfx: add placeholders for remain_on_channel feature
  2023-09-27 16:32 [PATCH v2 0/9] wfx: implement Remain On Channel Jérôme Pouiller
                   ` (3 preceding siblings ...)
  2023-09-27 16:32 ` [PATCH v2 4/9] wifi: wfx: introduce hif_scan_uniq() Jérôme Pouiller
@ 2023-09-27 16:32 ` Jérôme Pouiller
  2023-10-04 10:23   ` Kalle Valo
  2023-09-27 16:32 ` [PATCH v2 6/9] wifi: wfx: implement wfx_remain_on_channel() Jérôme Pouiller
                   ` (3 subsequent siblings)
  8 siblings, 1 reply; 16+ messages in thread
From: Jérôme Pouiller @ 2023-09-27 16:32 UTC (permalink / raw)
  To: Kalle Valo; +Cc: linux-wireless, linux-kernel, Jérôme Pouiller

First step to implement remain_on_channel.

Signed-off-by: Jérôme Pouiller <jerome.pouiller@silabs.com>
---
 drivers/net/wireless/silabs/wfx/main.c |  3 +++
 drivers/net/wireless/silabs/wfx/scan.c | 12 ++++++++++++
 drivers/net/wireless/silabs/wfx/scan.h |  5 +++++
 3 files changed, 20 insertions(+)

diff --git a/drivers/net/wireless/silabs/wfx/main.c b/drivers/net/wireless/silabs/wfx/main.c
index ede822d771aaf..31f6e0d3dc089 100644
--- a/drivers/net/wireless/silabs/wfx/main.c
+++ b/drivers/net/wireless/silabs/wfx/main.c
@@ -151,6 +151,8 @@ static const struct ieee80211_ops wfx_ops = {
 	.change_chanctx          = wfx_change_chanctx,
 	.assign_vif_chanctx      = wfx_assign_vif_chanctx,
 	.unassign_vif_chanctx    = wfx_unassign_vif_chanctx,
+	.remain_on_channel       = wfx_remain_on_channel,
+	.cancel_remain_on_channel = wfx_cancel_remain_on_channel,
 };
 
 bool wfx_api_older_than(struct wfx_dev *wdev, int major, int minor)
@@ -288,6 +290,7 @@ struct wfx_dev *wfx_init_common(struct device *dev, const struct wfx_platform_da
 	hw->wiphy->features |= NL80211_FEATURE_AP_SCAN;
 	hw->wiphy->flags |= WIPHY_FLAG_AP_PROBE_RESP_OFFLOAD;
 	hw->wiphy->flags |= WIPHY_FLAG_AP_UAPSD;
+	hw->wiphy->max_remain_on_channel_duration = 5000;
 	hw->wiphy->max_ap_assoc_sta = HIF_LINK_ID_MAX;
 	hw->wiphy->max_scan_ssids = 2;
 	hw->wiphy->max_scan_ie_len = IEEE80211_MAX_DATA_LEN;
diff --git a/drivers/net/wireless/silabs/wfx/scan.c b/drivers/net/wireless/silabs/wfx/scan.c
index 16f619ed22e00..51338fd43ae4f 100644
--- a/drivers/net/wireless/silabs/wfx/scan.c
+++ b/drivers/net/wireless/silabs/wfx/scan.c
@@ -145,3 +145,15 @@ void wfx_scan_complete(struct wfx_vif *wvif, int nb_chan_done)
 	wvif->scan_nb_chan_done = nb_chan_done;
 	complete(&wvif->scan_complete);
 }
+
+int wfx_remain_on_channel(struct ieee80211_hw *hw, struct ieee80211_vif *vif,
+			  struct ieee80211_channel *chan, int duration,
+			  enum ieee80211_roc_type type)
+{
+	return 0;
+}
+
+int wfx_cancel_remain_on_channel(struct ieee80211_hw *hw, struct ieee80211_vif *vif)
+{
+	return 0;
+}
diff --git a/drivers/net/wireless/silabs/wfx/scan.h b/drivers/net/wireless/silabs/wfx/scan.h
index 78e3b984f375c..2f8361769303e 100644
--- a/drivers/net/wireless/silabs/wfx/scan.h
+++ b/drivers/net/wireless/silabs/wfx/scan.h
@@ -19,4 +19,9 @@ int wfx_hw_scan(struct ieee80211_hw *hw, struct ieee80211_vif *vif,
 void wfx_cancel_hw_scan(struct ieee80211_hw *hw, struct ieee80211_vif *vif);
 void wfx_scan_complete(struct wfx_vif *wvif, int nb_chan_done);
 
+int wfx_remain_on_channel(struct ieee80211_hw *hw, struct ieee80211_vif *vif,
+			  struct ieee80211_channel *chan, int duration,
+			  enum ieee80211_roc_type type);
+int wfx_cancel_remain_on_channel(struct ieee80211_hw *hw, struct ieee80211_vif *vif);
+
 #endif
-- 
2.39.2


^ permalink raw reply related	[flat|nested] 16+ messages in thread
* [PATCH v2 6/9] wifi: wfx: implement wfx_remain_on_channel()
  2023-09-27 16:32 [PATCH v2 0/9] wfx: implement Remain On Channel Jérôme Pouiller
                   ` (4 preceding siblings ...)
  2023-09-27 16:32 ` [PATCH v2 5/9] wifi: wfx: add placeholders for remain_on_channel feature Jérôme Pouiller
@ 2023-09-27 16:32 ` Jérôme Pouiller
  2023-09-27 16:32 ` [PATCH v2 7/9] wifi: wfx: allow to send frames during ROC Jérôme Pouiller
                   ` (2 subsequent siblings)
  8 siblings, 0 replies; 16+ messages in thread
From: Jérôme Pouiller @ 2023-09-27 16:32 UTC (permalink / raw)
  To: Kalle Valo; +Cc: linux-wireless, linux-kernel, Jérôme Pouiller

With some conditions, the device is able to send/receive frames during
scan operation. So, it is possible to use it implement the "remain on
channel" feature. We just ask for a passive scan (without sending any
probe request) on one channel.

This architecture allows to leverage some interesting features:
  - if the device is AP, the device switches channel just after the next
    beacon and the beacons are stopped during the off-channel interval.
  - if the device is connected, it advertises it is asleep before to
    switch channel (so the AP should stop to try to send data)

Signed-off-by: Jérôme Pouiller <jerome.pouiller@silabs.com>
---
 drivers/net/wireless/silabs/wfx/scan.c | 50 ++++++++++++++++++++++++++
 drivers/net/wireless/silabs/wfx/scan.h |  1 +
 drivers/net/wireless/silabs/wfx/sta.c  |  1 +
 drivers/net/wireless/silabs/wfx/wfx.h  |  5 ++-
 4 files changed, 56 insertions(+), 1 deletion(-)

diff --git a/drivers/net/wireless/silabs/wfx/scan.c b/drivers/net/wireless/silabs/wfx/scan.c
index 51338fd43ae4f..57a2d63dd2a62 100644
--- a/drivers/net/wireless/silabs/wfx/scan.c
+++ b/drivers/net/wireless/silabs/wfx/scan.c
@@ -146,14 +146,64 @@ void wfx_scan_complete(struct wfx_vif *wvif, int nb_chan_done)
 	complete(&wvif->scan_complete);
 }
 
+void wfx_remain_on_channel_work(struct work_struct *work)
+{
+	struct wfx_vif *wvif = container_of(work, struct wfx_vif, remain_on_channel_work);
+	struct ieee80211_channel *chan = wvif->remain_on_channel_chan;
+	int duration = wvif->remain_on_channel_duration;
+	int ret;
+
+	/* Hijack scan request to implement Remain-On-Channel */
+	mutex_lock(&wvif->wdev->conf_mutex);
+	mutex_lock(&wvif->scan_lock);
+	if (wvif->join_in_progress) {
+		dev_info(wvif->wdev->dev, "abort in-progress REQ_JOIN");
+		wfx_reset(wvif);
+	}
+	wfx_tx_lock_flush(wvif->wdev);
+
+	reinit_completion(&wvif->scan_complete);
+	ret = wfx_hif_scan_uniq(wvif, chan, duration);
+	if (ret)
+		goto end;
+	ieee80211_ready_on_channel(wvif->wdev->hw);
+	ret = wait_for_completion_timeout(&wvif->scan_complete,
+					  msecs_to_jiffies(duration * 120 / 100));
+	if (!ret) {
+		wfx_hif_stop_scan(wvif);
+		ret = wait_for_completion_timeout(&wvif->scan_complete, 1 * HZ);
+		dev_dbg(wvif->wdev->dev, "roc timeout\n");
+	}
+	if (!ret)
+		dev_err(wvif->wdev->dev, "roc didn't stop\n");
+	ieee80211_remain_on_channel_expired(wvif->wdev->hw);
+end:
+	wfx_tx_unlock(wvif->wdev);
+	mutex_unlock(&wvif->scan_lock);
+	mutex_unlock(&wvif->wdev->conf_mutex);
+}
+
 int wfx_remain_on_channel(struct ieee80211_hw *hw, struct ieee80211_vif *vif,
 			  struct ieee80211_channel *chan, int duration,
 			  enum ieee80211_roc_type type)
 {
+	struct wfx_dev *wdev = hw->priv;
+	struct wfx_vif *wvif = (struct wfx_vif *)vif->drv_priv;
+
+	if (wfx_api_older_than(wdev, 3, 10))
+		return -EOPNOTSUPP;
+
+	wvif->remain_on_channel_duration = duration;
+	wvif->remain_on_channel_chan = chan;
+	schedule_work(&wvif->remain_on_channel_work);
 	return 0;
 }
 
 int wfx_cancel_remain_on_channel(struct ieee80211_hw *hw, struct ieee80211_vif *vif)
 {
+	struct wfx_vif *wvif = (struct wfx_vif *)vif->drv_priv;
+
+	wfx_hif_stop_scan(wvif);
+	flush_work(&wvif->remain_on_channel_work);
 	return 0;
 }
diff --git a/drivers/net/wireless/silabs/wfx/scan.h b/drivers/net/wireless/silabs/wfx/scan.h
index 2f8361769303e..995ab8c6cb5ef 100644
--- a/drivers/net/wireless/silabs/wfx/scan.h
+++ b/drivers/net/wireless/silabs/wfx/scan.h
@@ -19,6 +19,7 @@ int wfx_hw_scan(struct ieee80211_hw *hw, struct ieee80211_vif *vif,
 void wfx_cancel_hw_scan(struct ieee80211_hw *hw, struct ieee80211_vif *vif);
 void wfx_scan_complete(struct wfx_vif *wvif, int nb_chan_done);
 
+void wfx_remain_on_channel_work(struct work_struct *work);
 int wfx_remain_on_channel(struct ieee80211_hw *hw, struct ieee80211_vif *vif,
 			  struct ieee80211_channel *chan, int duration,
 			  enum ieee80211_roc_type type);
diff --git a/drivers/net/wireless/silabs/wfx/sta.c b/drivers/net/wireless/silabs/wfx/sta.c
index c58db2bcea87b..f42341c2baffb 100644
--- a/drivers/net/wireless/silabs/wfx/sta.c
+++ b/drivers/net/wireless/silabs/wfx/sta.c
@@ -733,6 +733,7 @@ int wfx_add_interface(struct ieee80211_hw *hw, struct ieee80211_vif *vif)
 	mutex_init(&wvif->scan_lock);
 	init_completion(&wvif->scan_complete);
 	INIT_WORK(&wvif->scan_work, wfx_hw_scan_work);
+	INIT_WORK(&wvif->remain_on_channel_work, wfx_remain_on_channel_work);
 
 	wfx_tx_queues_init(wvif);
 	wfx_tx_policy_init(wvif);
diff --git a/drivers/net/wireless/silabs/wfx/wfx.h b/drivers/net/wireless/silabs/wfx/wfx.h
index 13ba84b3b2c33..5fd80c423d6f6 100644
--- a/drivers/net/wireless/silabs/wfx/wfx.h
+++ b/drivers/net/wireless/silabs/wfx/wfx.h
@@ -69,6 +69,7 @@ struct wfx_vif {
 
 	bool                       after_dtim_tx_allowed;
 	bool                       join_in_progress;
+	struct completion          set_pm_mode_complete;
 
 	struct delayed_work        beacon_loss_work;
 
@@ -88,7 +89,9 @@ struct wfx_vif {
 	bool                       scan_abort;
 	struct ieee80211_scan_request *scan_req;
 
-	struct completion          set_pm_mode_complete;
+	struct ieee80211_channel   *remain_on_channel_chan;
+	int                        remain_on_channel_duration;
+	struct work_struct         remain_on_channel_work;
 };
 
 static inline struct ieee80211_vif *wvif_to_vif(struct wfx_vif *wvif)
-- 
2.39.2


^ permalink raw reply related	[flat|nested] 16+ messages in thread
* [PATCH v2 7/9] wifi: wfx: allow to send frames during ROC
  2023-09-27 16:32 [PATCH v2 0/9] wfx: implement Remain On Channel Jérôme Pouiller
                   ` (5 preceding siblings ...)
  2023-09-27 16:32 ` [PATCH v2 6/9] wifi: wfx: implement wfx_remain_on_channel() Jérôme Pouiller
@ 2023-09-27 16:32 ` Jérôme Pouiller
  2023-09-27 16:32 ` [PATCH v2 8/9] wifi: wfx: scan_lock is global to the device Jérôme Pouiller
  2023-09-27 16:32 ` [PATCH v2 9/9] wifi: wfx: fix possible lock-up between scan and Rx filters Jérôme Pouiller
  8 siblings, 0 replies; 16+ messages in thread
From: Jérôme Pouiller @ 2023-09-27 16:32 UTC (permalink / raw)
  To: Kalle Valo; +Cc: linux-wireless, linux-kernel, Jérôme Pouiller

Until now, all the traffic was blocked during Remain On Channel (ROC).
This patch allow to handle IEEE80211_TX_CTL_TX_OFFCHAN frames.

These frames need to be sent on the virtual interface #2. Until now,
this interface was only used by the device for internal purpose. But
since API 3.9, it can be used to send data during scan operation (we
hijack the scan process to implement ROC).

Thus, we need to change a bit the way we match the frames with the
interface.

Fortunately, the frames received during the scan are marked with the
correct interface number. So there is no change to do on this part.

Signed-off-by: Jérôme Pouiller <jerome.pouiller@silabs.com>
---
 drivers/net/wireless/silabs/wfx/data_tx.c | 36 +++++++++++++++-----
 drivers/net/wireless/silabs/wfx/data_tx.h |  2 ++
 drivers/net/wireless/silabs/wfx/queue.c   | 40 +++++++++++++++++++----
 drivers/net/wireless/silabs/wfx/queue.h   |  1 +
 drivers/net/wireless/silabs/wfx/scan.c    |  4 +--
 5 files changed, 66 insertions(+), 17 deletions(-)

diff --git a/drivers/net/wireless/silabs/wfx/data_tx.c b/drivers/net/wireless/silabs/wfx/data_tx.c
index ce2b5dcfd8d89..e8b6d41f55196 100644
--- a/drivers/net/wireless/silabs/wfx/data_tx.c
+++ b/drivers/net/wireless/silabs/wfx/data_tx.c
@@ -226,6 +226,18 @@ struct wfx_hif_req_tx *wfx_skb_txreq(struct sk_buff *skb)
 	return req;
 }
 
+struct wfx_vif *wfx_skb_wvif(struct wfx_dev *wdev, struct sk_buff *skb)
+{
+	struct wfx_tx_priv *tx_priv = wfx_skb_tx_priv(skb);
+	struct wfx_hif_msg *hif = (struct wfx_hif_msg *)skb->data;
+
+	if (tx_priv->vif_id != hif->interface && hif->interface != 2) {
+		dev_err(wdev->dev, "corrupted skb");
+		return wdev_to_wvif(wdev, hif->interface);
+	}
+	return wdev_to_wvif(wdev, tx_priv->vif_id);
+}
+
 static u8 wfx_tx_get_link_id(struct wfx_vif *wvif, struct ieee80211_sta *sta,
 			     struct ieee80211_hdr *hdr)
 {
@@ -352,6 +364,7 @@ static int wfx_tx_inner(struct wfx_vif *wvif, struct ieee80211_sta *sta, struct
 	/* Fill tx_priv */
 	tx_priv = (struct wfx_tx_priv *)tx_info->rate_driver_data;
 	tx_priv->icv_size = wfx_tx_get_icv_len(hw_key);
+	tx_priv->vif_id = wvif->id;
 
 	/* Fill hif_msg */
 	WARN(skb_headroom(skb) < wmsg_len, "not enough space in skb");
@@ -362,7 +375,10 @@ static int wfx_tx_inner(struct wfx_vif *wvif, struct ieee80211_sta *sta, struct
 	hif_msg = (struct wfx_hif_msg *)skb->data;
 	hif_msg->len = cpu_to_le16(skb->len);
 	hif_msg->id = HIF_REQ_ID_TX;
-	hif_msg->interface = wvif->id;
+	if (tx_info->flags & IEEE80211_TX_CTL_TX_OFFCHAN)
+		hif_msg->interface = 2;
+	else
+		hif_msg->interface = wvif->id;
 	if (skb->len > le16_to_cpu(wvif->wdev->hw_caps.size_inp_ch_buf)) {
 		dev_warn(wvif->wdev->dev,
 			 "requested frame size (%d) is larger than maximum supported (%d)\n",
@@ -383,9 +399,15 @@ static int wfx_tx_inner(struct wfx_vif *wvif, struct ieee80211_sta *sta, struct
 	req->fc_offset = offset;
 	/* Queue index are inverted between firmware and Linux */
 	req->queue_id = 3 - queue_id;
-	req->peer_sta_id = wfx_tx_get_link_id(wvif, sta, hdr);
-	req->retry_policy_index = wfx_tx_get_retry_policy_id(wvif, tx_info);
-	req->frame_format = wfx_tx_get_frame_format(tx_info);
+	if (tx_info->flags & IEEE80211_TX_CTL_TX_OFFCHAN) {
+		req->peer_sta_id = HIF_LINK_ID_NOT_ASSOCIATED;
+		req->retry_policy_index = HIF_TX_RETRY_POLICY_INVALID;
+		req->frame_format = HIF_FRAME_FORMAT_NON_HT;
+	} else {
+		req->peer_sta_id = wfx_tx_get_link_id(wvif, sta, hdr);
+		req->retry_policy_index = wfx_tx_get_retry_policy_id(wvif, tx_info);
+		req->frame_format = wfx_tx_get_frame_format(tx_info);
+	}
 	if (tx_info->driver_rates[0].flags & IEEE80211_TX_RC_SHORT_GI)
 		req->short_gi = 1;
 	if (tx_info->flags & IEEE80211_TX_CTL_SEND_AFTER_DTIM)
@@ -501,7 +523,7 @@ void wfx_tx_confirm_cb(struct wfx_dev *wdev, const struct wfx_hif_cnf_tx *arg)
 	}
 	tx_info = IEEE80211_SKB_CB(skb);
 	tx_priv = wfx_skb_tx_priv(skb);
-	wvif = wdev_to_wvif(wdev, ((struct wfx_hif_msg *)skb->data)->interface);
+	wvif = wfx_skb_wvif(wdev, skb);
 	WARN_ON(!wvif);
 	if (!wvif)
 		return;
@@ -563,7 +585,6 @@ void wfx_flush(struct ieee80211_hw *hw, struct ieee80211_vif *vif, u32 queues, b
 	struct wfx_dev *wdev = hw->priv;
 	struct sk_buff_head dropped;
 	struct wfx_vif *wvif;
-	struct wfx_hif_msg *hif;
 	struct sk_buff *skb;
 
 	skb_queue_head_init(&dropped);
@@ -579,8 +600,7 @@ void wfx_flush(struct ieee80211_hw *hw, struct ieee80211_vif *vif, u32 queues, b
 	if (wdev->chip_frozen)
 		wfx_pending_drop(wdev, &dropped);
 	while ((skb = skb_dequeue(&dropped)) != NULL) {
-		hif = (struct wfx_hif_msg *)skb->data;
-		wvif = wdev_to_wvif(wdev, hif->interface);
+		wvif = wfx_skb_wvif(wdev, skb);
 		ieee80211_tx_info_clear_status(IEEE80211_SKB_CB(skb));
 		wfx_skb_dtor(wvif, skb);
 	}
diff --git a/drivers/net/wireless/silabs/wfx/data_tx.h b/drivers/net/wireless/silabs/wfx/data_tx.h
index a5b80eacce39a..0621b82103bef 100644
--- a/drivers/net/wireless/silabs/wfx/data_tx.h
+++ b/drivers/net/wireless/silabs/wfx/data_tx.h
@@ -36,6 +36,7 @@ struct wfx_tx_policy_cache {
 struct wfx_tx_priv {
 	ktime_t xmit_timestamp;
 	unsigned char icv_size;
+	unsigned char vif_id;
 };
 
 void wfx_tx_policy_init(struct wfx_vif *wvif);
@@ -47,5 +48,6 @@ void wfx_flush(struct ieee80211_hw *hw, struct ieee80211_vif *vif, u32 queues, b
 
 struct wfx_tx_priv *wfx_skb_tx_priv(struct sk_buff *skb);
 struct wfx_hif_req_tx *wfx_skb_txreq(struct sk_buff *skb);
+struct wfx_vif *wfx_skb_wvif(struct wfx_dev *wdev, struct sk_buff *skb);
 
 #endif
diff --git a/drivers/net/wireless/silabs/wfx/queue.c b/drivers/net/wireless/silabs/wfx/queue.c
index 37f492e5d3bea..b5975d18f09b4 100644
--- a/drivers/net/wireless/silabs/wfx/queue.c
+++ b/drivers/net/wireless/silabs/wfx/queue.c
@@ -68,13 +68,16 @@ void wfx_tx_queues_init(struct wfx_vif *wvif)
 	for (i = 0; i < IEEE80211_NUM_ACS; ++i) {
 		skb_queue_head_init(&wvif->tx_queue[i].normal);
 		skb_queue_head_init(&wvif->tx_queue[i].cab);
+		skb_queue_head_init(&wvif->tx_queue[i].offchan);
 		wvif->tx_queue[i].priority = priorities[i];
 	}
 }
 
 bool wfx_tx_queue_empty(struct wfx_vif *wvif, struct wfx_queue *queue)
 {
-	return skb_queue_empty_lockless(&queue->normal) && skb_queue_empty_lockless(&queue->cab);
+	return skb_queue_empty_lockless(&queue->normal) &&
+	       skb_queue_empty_lockless(&queue->cab) &&
+	       skb_queue_empty_lockless(&queue->offchan);
 }
 
 void wfx_tx_queues_check_empty(struct wfx_vif *wvif)
@@ -103,8 +106,9 @@ static void __wfx_tx_queue_drop(struct wfx_vif *wvif,
 void wfx_tx_queue_drop(struct wfx_vif *wvif, struct wfx_queue *queue,
 		       struct sk_buff_head *dropped)
 {
-	__wfx_tx_queue_drop(wvif, &queue->cab, dropped);
 	__wfx_tx_queue_drop(wvif, &queue->normal, dropped);
+	__wfx_tx_queue_drop(wvif, &queue->cab, dropped);
+	__wfx_tx_queue_drop(wvif, &queue->offchan, dropped);
 	wake_up(&wvif->wdev->tx_dequeue);
 }
 
@@ -113,7 +117,9 @@ void wfx_tx_queues_put(struct wfx_vif *wvif, struct sk_buff *skb)
 	struct wfx_queue *queue = &wvif->tx_queue[skb_get_queue_mapping(skb)];
 	struct ieee80211_tx_info *tx_info = IEEE80211_SKB_CB(skb);
 
-	if (tx_info->flags & IEEE80211_TX_CTL_SEND_AFTER_DTIM)
+	if (tx_info->flags & IEEE80211_TX_CTL_TX_OFFCHAN)
+		skb_queue_tail(&queue->offchan, skb);
+	else if (tx_info->flags & IEEE80211_TX_CTL_SEND_AFTER_DTIM)
 		skb_queue_tail(&queue->cab, skb);
 	else
 		skb_queue_tail(&queue->normal, skb);
@@ -123,13 +129,11 @@ void wfx_pending_drop(struct wfx_dev *wdev, struct sk_buff_head *dropped)
 {
 	struct wfx_queue *queue;
 	struct wfx_vif *wvif;
-	struct wfx_hif_msg *hif;
 	struct sk_buff *skb;
 
 	WARN(!wdev->chip_frozen, "%s should only be used to recover a frozen device", __func__);
 	while ((skb = skb_dequeue(&wdev->tx_pending)) != NULL) {
-		hif = (struct wfx_hif_msg *)skb->data;
-		wvif = wdev_to_wvif(wdev, hif->interface);
+		wvif = wfx_skb_wvif(wdev, skb);
 		if (wvif) {
 			queue = &wvif->tx_queue[skb_get_queue_mapping(skb)];
 			WARN_ON(skb_get_queue_mapping(skb) > 3);
@@ -155,7 +159,7 @@ struct sk_buff *wfx_pending_get(struct wfx_dev *wdev, u32 packet_id)
 		if (req->packet_id != packet_id)
 			continue;
 		spin_unlock_bh(&wdev->tx_pending.lock);
-		wvif = wdev_to_wvif(wdev, hif->interface);
+		wvif = wfx_skb_wvif(wdev, skb);
 		if (wvif) {
 			queue = &wvif->tx_queue[skb_get_queue_mapping(skb)];
 			WARN_ON(skb_get_queue_mapping(skb) > 3);
@@ -246,6 +250,28 @@ static struct sk_buff *wfx_tx_queues_get_skb(struct wfx_dev *wdev)
 		}
 	}
 
+	wvif = NULL;
+	while ((wvif = wvif_iterate(wdev, wvif)) != NULL) {
+		for (i = 0; i < num_queues; i++) {
+			skb = skb_dequeue(&queues[i]->offchan);
+			if (!skb)
+				continue;
+			hif = (struct wfx_hif_msg *)skb->data;
+			/* Offchan frames are assigned to a special interface.
+			 * The only interface allowed to send data during scan.
+			 */
+			WARN_ON(hif->interface != 2);
+			atomic_inc(&queues[i]->pending_frames);
+			trace_queues_stats(wdev, queues[i]);
+			return skb;
+		}
+	}
+
+	wvif = NULL;
+	while ((wvif = wvif_iterate(wdev, wvif)) != NULL)
+		if (mutex_is_locked(&wvif->scan_lock))
+			return NULL;
+
 	wvif = NULL;
 	while ((wvif = wvif_iterate(wdev, wvif)) != NULL) {
 		if (!wvif->after_dtim_tx_allowed)
diff --git a/drivers/net/wireless/silabs/wfx/queue.h b/drivers/net/wireless/silabs/wfx/queue.h
index 4731debca93d2..6857fbd60fbad 100644
--- a/drivers/net/wireless/silabs/wfx/queue.h
+++ b/drivers/net/wireless/silabs/wfx/queue.h
@@ -17,6 +17,7 @@ struct wfx_vif;
 struct wfx_queue {
 	struct sk_buff_head normal;
 	struct sk_buff_head cab; /* Content After (DTIM) Beacon */
+	struct sk_buff_head offchan;
 	atomic_t            pending_frames;
 	int                 priority;
 };
diff --git a/drivers/net/wireless/silabs/wfx/scan.c b/drivers/net/wireless/silabs/wfx/scan.c
index 57a2d63dd2a62..817eeb3aa8bff 100644
--- a/drivers/net/wireless/silabs/wfx/scan.c
+++ b/drivers/net/wireless/silabs/wfx/scan.c
@@ -160,7 +160,7 @@ void wfx_remain_on_channel_work(struct work_struct *work)
 		dev_info(wvif->wdev->dev, "abort in-progress REQ_JOIN");
 		wfx_reset(wvif);
 	}
-	wfx_tx_lock_flush(wvif->wdev);
+	wfx_tx_flush(wvif->wdev);
 
 	reinit_completion(&wvif->scan_complete);
 	ret = wfx_hif_scan_uniq(wvif, chan, duration);
@@ -178,9 +178,9 @@ void wfx_remain_on_channel_work(struct work_struct *work)
 		dev_err(wvif->wdev->dev, "roc didn't stop\n");
 	ieee80211_remain_on_channel_expired(wvif->wdev->hw);
 end:
-	wfx_tx_unlock(wvif->wdev);
 	mutex_unlock(&wvif->scan_lock);
 	mutex_unlock(&wvif->wdev->conf_mutex);
+	wfx_bh_request_tx(wvif->wdev);
 }
 
 int wfx_remain_on_channel(struct ieee80211_hw *hw, struct ieee80211_vif *vif,
-- 
2.39.2


^ permalink raw reply related	[flat|nested] 16+ messages in thread
* [PATCH v2 8/9] wifi: wfx: scan_lock is global to the device
  2023-09-27 16:32 [PATCH v2 0/9] wfx: implement Remain On Channel Jérôme Pouiller
                   ` (6 preceding siblings ...)
  2023-09-27 16:32 ` [PATCH v2 7/9] wifi: wfx: allow to send frames during ROC Jérôme Pouiller
@ 2023-09-27 16:32 ` Jérôme Pouiller
  2023-09-27 16:32 ` [PATCH v2 9/9] wifi: wfx: fix possible lock-up between scan and Rx filters Jérôme Pouiller
  8 siblings, 0 replies; 16+ messages in thread
From: Jérôme Pouiller @ 2023-09-27 16:32 UTC (permalink / raw)
  To: Kalle Valo; +Cc: linux-wireless, linux-kernel, Jérôme Pouiller

Currently, one scan_lock is associated to each vif. However, concurrent
scan on vifs is explicitly prohitibed by the device. Currently,
scan_lock is associated with conf_mutex that ensure that concurrent scan
on vifs cannot happen.

In the only case where conf_mutex is not associated to scan_lock, the
scan_lock is tested on all interfaces.

So, this patch relocate scan_lock to the device and simplify the code.

Signed-off-by: Jérôme Pouiller <jerome.pouiller@silabs.com>
---
 drivers/net/wireless/silabs/wfx/main.c  |  2 ++
 drivers/net/wireless/silabs/wfx/queue.c |  6 ++----
 drivers/net/wireless/silabs/wfx/scan.c  |  8 ++++----
 drivers/net/wireless/silabs/wfx/sta.c   | 15 ++++-----------
 drivers/net/wireless/silabs/wfx/wfx.h   |  3 +--
 5 files changed, 13 insertions(+), 21 deletions(-)

diff --git a/drivers/net/wireless/silabs/wfx/main.c b/drivers/net/wireless/silabs/wfx/main.c
index 31f6e0d3dc089..e7198520bdffc 100644
--- a/drivers/net/wireless/silabs/wfx/main.c
+++ b/drivers/net/wireless/silabs/wfx/main.c
@@ -248,6 +248,7 @@ static void wfx_free_common(void *data)
 
 	mutex_destroy(&wdev->tx_power_loop_info_lock);
 	mutex_destroy(&wdev->rx_stats_lock);
+	mutex_destroy(&wdev->scan_lock);
 	mutex_destroy(&wdev->conf_mutex);
 	ieee80211_free_hw(wdev->hw);
 }
@@ -317,6 +318,7 @@ struct wfx_dev *wfx_init_common(struct device *dev, const struct wfx_platform_da
 		gpiod_set_consumer_name(wdev->pdata.gpio_wakeup, "wfx wakeup");
 
 	mutex_init(&wdev->conf_mutex);
+	mutex_init(&wdev->scan_lock);
 	mutex_init(&wdev->rx_stats_lock);
 	mutex_init(&wdev->tx_power_loop_info_lock);
 	init_completion(&wdev->firmware_ready);
diff --git a/drivers/net/wireless/silabs/wfx/queue.c b/drivers/net/wireless/silabs/wfx/queue.c
index b5975d18f09b4..e61b86f211e53 100644
--- a/drivers/net/wireless/silabs/wfx/queue.c
+++ b/drivers/net/wireless/silabs/wfx/queue.c
@@ -267,10 +267,8 @@ static struct sk_buff *wfx_tx_queues_get_skb(struct wfx_dev *wdev)
 		}
 	}
 
-	wvif = NULL;
-	while ((wvif = wvif_iterate(wdev, wvif)) != NULL)
-		if (mutex_is_locked(&wvif->scan_lock))
-			return NULL;
+	if (mutex_is_locked(&wdev->scan_lock))
+		return NULL;
 
 	wvif = NULL;
 	while ((wvif = wvif_iterate(wdev, wvif)) != NULL) {
diff --git a/drivers/net/wireless/silabs/wfx/scan.c b/drivers/net/wireless/silabs/wfx/scan.c
index 817eeb3aa8bff..c3c103ff88cce 100644
--- a/drivers/net/wireless/silabs/wfx/scan.c
+++ b/drivers/net/wireless/silabs/wfx/scan.c
@@ -95,7 +95,7 @@ void wfx_hw_scan_work(struct work_struct *work)
 	int chan_cur, ret, err;
 
 	mutex_lock(&wvif->wdev->conf_mutex);
-	mutex_lock(&wvif->scan_lock);
+	mutex_lock(&wvif->wdev->scan_lock);
 	if (wvif->join_in_progress) {
 		dev_info(wvif->wdev->dev, "abort in-progress REQ_JOIN");
 		wfx_reset(wvif);
@@ -116,7 +116,7 @@ void wfx_hw_scan_work(struct work_struct *work)
 			ret = -ETIMEDOUT;
 		}
 	} while (ret >= 0 && chan_cur < hw_req->req.n_channels);
-	mutex_unlock(&wvif->scan_lock);
+	mutex_unlock(&wvif->wdev->scan_lock);
 	mutex_unlock(&wvif->wdev->conf_mutex);
 	wfx_ieee80211_scan_completed_compat(wvif->wdev->hw, ret < 0);
 }
@@ -155,7 +155,7 @@ void wfx_remain_on_channel_work(struct work_struct *work)
 
 	/* Hijack scan request to implement Remain-On-Channel */
 	mutex_lock(&wvif->wdev->conf_mutex);
-	mutex_lock(&wvif->scan_lock);
+	mutex_lock(&wvif->wdev->scan_lock);
 	if (wvif->join_in_progress) {
 		dev_info(wvif->wdev->dev, "abort in-progress REQ_JOIN");
 		wfx_reset(wvif);
@@ -178,7 +178,7 @@ void wfx_remain_on_channel_work(struct work_struct *work)
 		dev_err(wvif->wdev->dev, "roc didn't stop\n");
 	ieee80211_remain_on_channel_expired(wvif->wdev->hw);
 end:
-	mutex_unlock(&wvif->scan_lock);
+	mutex_unlock(&wvif->wdev->scan_lock);
 	mutex_unlock(&wvif->wdev->conf_mutex);
 	wfx_bh_request_tx(wvif->wdev);
 }
diff --git a/drivers/net/wireless/silabs/wfx/sta.c b/drivers/net/wireless/silabs/wfx/sta.c
index f42341c2baffb..496b93de3ee58 100644
--- a/drivers/net/wireless/silabs/wfx/sta.c
+++ b/drivers/net/wireless/silabs/wfx/sta.c
@@ -97,9 +97,8 @@ void wfx_configure_filter(struct ieee80211_hw *hw, unsigned int changed_flags,
 			FIF_PROBE_REQ | FIF_PSPOLL;
 
 	mutex_lock(&wdev->conf_mutex);
+	mutex_lock(&wdev->scan_lock);
 	while ((wvif = wvif_iterate(wdev, wvif)) != NULL) {
-		mutex_lock(&wvif->scan_lock);
-
 		/* Note: FIF_BCN_PRBRESP_PROMISC covers probe response and
 		 * beacons from other BSS
 		 */
@@ -126,9 +125,8 @@ void wfx_configure_filter(struct ieee80211_hw *hw, unsigned int changed_flags,
 		else
 			filter_prbreq = true;
 		wfx_hif_set_rx_filter(wvif, filter_bssid, filter_prbreq);
-
-		mutex_unlock(&wvif->scan_lock);
 	}
+	mutex_unlock(&wdev->scan_lock);
 	mutex_unlock(&wdev->conf_mutex);
 }
 
@@ -621,18 +619,14 @@ int wfx_set_tim(struct ieee80211_hw *hw, struct ieee80211_sta *sta, bool set)
 
 void wfx_suspend_resume_mc(struct wfx_vif *wvif, enum sta_notify_cmd notify_cmd)
 {
-	struct wfx_vif *wvif_it;
-
 	if (notify_cmd != STA_NOTIFY_AWAKE)
 		return;
 
 	/* Device won't be able to honor CAB if a scan is in progress on any interface. Prefer to
 	 * skip this DTIM and wait for the next one.
 	 */
-	wvif_it = NULL;
-	while ((wvif_it = wvif_iterate(wvif->wdev, wvif_it)) != NULL)
-		if (mutex_is_locked(&wvif_it->scan_lock))
-			return;
+	if (mutex_is_locked(&wvif->wdev->scan_lock))
+		return;
 
 	if (!wfx_tx_queues_has_cab(wvif) || wvif->after_dtim_tx_allowed)
 		dev_warn(wvif->wdev->dev, "incorrect sequence (%d CAB in queue)",
@@ -730,7 +724,6 @@ int wfx_add_interface(struct ieee80211_hw *hw, struct ieee80211_vif *vif)
 	complete(&wvif->set_pm_mode_complete);
 	INIT_WORK(&wvif->tx_policy_upload_work, wfx_tx_policy_upload_work);
 
-	mutex_init(&wvif->scan_lock);
 	init_completion(&wvif->scan_complete);
 	INIT_WORK(&wvif->scan_work, wfx_hw_scan_work);
 	INIT_WORK(&wvif->remain_on_channel_work, wfx_remain_on_channel_work);
diff --git a/drivers/net/wireless/silabs/wfx/wfx.h b/drivers/net/wireless/silabs/wfx/wfx.h
index 5fd80c423d6f6..bd0df2e1ea990 100644
--- a/drivers/net/wireless/silabs/wfx/wfx.h
+++ b/drivers/net/wireless/silabs/wfx/wfx.h
@@ -43,6 +43,7 @@ struct wfx_dev {
 	struct delayed_work        cooling_timeout_work;
 	bool                       poll_irq;
 	bool                       chip_frozen;
+	struct mutex               scan_lock;
 	struct mutex               conf_mutex;
 
 	struct wfx_hif_cmd         hif_cmd;
@@ -81,8 +82,6 @@ struct wfx_vif {
 
 	unsigned long              uapsd_mask;
 
-	/* avoid some operations in parallel with scan */
-	struct mutex               scan_lock;
 	struct work_struct         scan_work;
 	struct completion          scan_complete;
 	int                        scan_nb_chan_done;
-- 
2.39.2


^ permalink raw reply related	[flat|nested] 16+ messages in thread
* [PATCH v2 9/9] wifi: wfx: fix possible lock-up between scan and Rx filters
  2023-09-27 16:32 [PATCH v2 0/9] wfx: implement Remain On Channel Jérôme Pouiller
                   ` (7 preceding siblings ...)
  2023-09-27 16:32 ` [PATCH v2 8/9] wifi: wfx: scan_lock is global to the device Jérôme Pouiller
@ 2023-09-27 16:32 ` Jérôme Pouiller
  8 siblings, 0 replies; 16+ messages in thread
From: Jérôme Pouiller @ 2023-09-27 16:32 UTC (permalink / raw)
  To: Kalle Valo; +Cc: linux-wireless, linux-kernel, Jérôme Pouiller

The device ignore the rx filters during the scan operation.
wfx_configure_filter() acquires scan_lock to reflect this restriction.
However, it is not really necessary since mac80211 don't try to
configure Rx filters during scan.

However, the things are changing. The scan operation is also used to
implement remain-on-channel. In this case, wfx_configure_filter() can be
called during the scan. Currently, this scenario generate a delay that
end with a timeout in the upper layers. For the final user, some
scenario of the EasyConnect specification end with a failure.

So, avoid acquiring the scan_lock and just return.

Signed-off-by: Jérôme Pouiller <jerome.pouiller@silabs.com>
---
 drivers/net/wireless/silabs/wfx/sta.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/net/wireless/silabs/wfx/sta.c b/drivers/net/wireless/silabs/wfx/sta.c
index 496b93de3ee58..1b6c158457b42 100644
--- a/drivers/net/wireless/silabs/wfx/sta.c
+++ b/drivers/net/wireless/silabs/wfx/sta.c
@@ -96,8 +96,11 @@ void wfx_configure_filter(struct ieee80211_hw *hw, unsigned int changed_flags,
 	*total_flags &= FIF_BCN_PRBRESP_PROMISC | FIF_ALLMULTI | FIF_OTHER_BSS |
 			FIF_PROBE_REQ | FIF_PSPOLL;
 
+	/* Filters are ignored during the scan. No frames are filtered. */
+	if (mutex_is_locked(&wdev->scan_lock))
+		return;
+
 	mutex_lock(&wdev->conf_mutex);
-	mutex_lock(&wdev->scan_lock);
 	while ((wvif = wvif_iterate(wdev, wvif)) != NULL) {
 		/* Note: FIF_BCN_PRBRESP_PROMISC covers probe response and
 		 * beacons from other BSS
@@ -126,7 +129,6 @@ void wfx_configure_filter(struct ieee80211_hw *hw, unsigned int changed_flags,
 			filter_prbreq = true;
 		wfx_hif_set_rx_filter(wvif, filter_bssid, filter_prbreq);
 	}
-	mutex_unlock(&wdev->scan_lock);
 	mutex_unlock(&wdev->conf_mutex);
 }
 
-- 
2.39.2


^ permalink raw reply related	[flat|nested] 16+ messages in thread
* Re: [PATCH v2 1/9] wifi: wfx: fix power_save setting when AP is stopped
  2023-09-27 16:32 ` [PATCH v2 1/9] wifi: wfx: fix power_save setting when AP is stopped Jérôme Pouiller
@ 2023-10-04 10:20   ` Kalle Valo
  2023-10-04 10:37     ` Jérôme Pouiller
  0 siblings, 1 reply; 16+ messages in thread
From: Kalle Valo @ 2023-10-04 10:20 UTC (permalink / raw)
  To: Jérôme Pouiller; +Cc: linux-wireless, linux-kernel

Jérôme Pouiller <jerome.pouiller@silabs.com> writes:

> The WF200 allow to start two network interfaces (one AP, one station) on
> two different channels. Since magic does not exist, it only works if the
> station interface enables power save.
>
> Thus, the driver detects this case and enforce power save as necessary.
>
> This patch fixes the case where the AP interface is stopped and it is no
> more necessary to enforce power saving on the station interface.
>
> Signed-off-by: Jérôme Pouiller <jerome.pouiller@silabs.com>
> ---
>  drivers/net/wireless/silabs/wfx/sta.c | 5 +++++
>  1 file changed, 5 insertions(+)
>
> diff --git a/drivers/net/wireless/silabs/wfx/sta.c b/drivers/net/wireless/silabs/wfx/sta.c
> index 626dfb4b7a55d..9c0a11c277e97 100644
> --- a/drivers/net/wireless/silabs/wfx/sta.c
> +++ b/drivers/net/wireless/silabs/wfx/sta.c
> @@ -402,7 +402,12 @@ void wfx_stop_ap(struct ieee80211_hw *hw, struct ieee80211_vif *vif,
>  		 struct ieee80211_bss_conf *link_conf)
>  {
>  	struct wfx_vif *wvif = (struct wfx_vif *)vif->drv_priv;
> +	struct wfx_dev *wdev = wvif->wdev;
>  
> +	wvif =  NULL;
> +	while ((wvif = wvif_iterate(wdev, wvif)) != NULL)
> +		wfx_update_pm(wvif);

Isn't the assignment of wvif to NULL unnecessary as in the next line we
assign it to again?

-- 
https://patchwork.kernel.org/project/linux-wireless/list/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

^ permalink raw reply	[flat|nested] 16+ messages in thread
* Re: [PATCH v2 5/9] wifi: wfx: add placeholders for remain_on_channel feature
  2023-09-27 16:32 ` [PATCH v2 5/9] wifi: wfx: add placeholders for remain_on_channel feature Jérôme Pouiller
@ 2023-10-04 10:23   ` Kalle Valo
  2023-10-04 10:40     ` Jérôme Pouiller
  0 siblings, 1 reply; 16+ messages in thread
From: Kalle Valo @ 2023-10-04 10:23 UTC (permalink / raw)
  To: Jérôme Pouiller; +Cc: linux-wireless, linux-kernel

Jérôme Pouiller <jerome.pouiller@silabs.com> writes:

> First step to implement remain_on_channel.
>
> Signed-off-by: Jérôme Pouiller <jerome.pouiller@silabs.com>
> ---
>  drivers/net/wireless/silabs/wfx/main.c |  3 +++
>  drivers/net/wireless/silabs/wfx/scan.c | 12 ++++++++++++
>  drivers/net/wireless/silabs/wfx/scan.h |  5 +++++
>  3 files changed, 20 insertions(+)
>
> diff --git a/drivers/net/wireless/silabs/wfx/main.c b/drivers/net/wireless/silabs/wfx/main.c
> index ede822d771aaf..31f6e0d3dc089 100644
> --- a/drivers/net/wireless/silabs/wfx/main.c
> +++ b/drivers/net/wireless/silabs/wfx/main.c
> @@ -151,6 +151,8 @@ static const struct ieee80211_ops wfx_ops = {
>  	.change_chanctx          = wfx_change_chanctx,
>  	.assign_vif_chanctx      = wfx_assign_vif_chanctx,
>  	.unassign_vif_chanctx    = wfx_unassign_vif_chanctx,
> +	.remain_on_channel       = wfx_remain_on_channel,
> +	.cancel_remain_on_channel = wfx_cancel_remain_on_channel,
>  };
>  
>  bool wfx_api_older_than(struct wfx_dev *wdev, int major, int minor)
> @@ -288,6 +290,7 @@ struct wfx_dev *wfx_init_common(struct device *dev, const struct wfx_platform_da
>  	hw->wiphy->features |= NL80211_FEATURE_AP_SCAN;
>  	hw->wiphy->flags |= WIPHY_FLAG_AP_PROBE_RESP_OFFLOAD;
>  	hw->wiphy->flags |= WIPHY_FLAG_AP_UAPSD;
> +	hw->wiphy->max_remain_on_channel_duration = 5000;
>  	hw->wiphy->max_ap_assoc_sta = HIF_LINK_ID_MAX;
>  	hw->wiphy->max_scan_ssids = 2;
>  	hw->wiphy->max_scan_ie_len = IEEE80211_MAX_DATA_LEN;
> diff --git a/drivers/net/wireless/silabs/wfx/scan.c b/drivers/net/wireless/silabs/wfx/scan.c
> index 16f619ed22e00..51338fd43ae4f 100644
> --- a/drivers/net/wireless/silabs/wfx/scan.c
> +++ b/drivers/net/wireless/silabs/wfx/scan.c
> @@ -145,3 +145,15 @@ void wfx_scan_complete(struct wfx_vif *wvif, int nb_chan_done)
>  	wvif->scan_nb_chan_done = nb_chan_done;
>  	complete(&wvif->scan_complete);
>  }
> +
> +int wfx_remain_on_channel(struct ieee80211_hw *hw, struct ieee80211_vif *vif,
> +			  struct ieee80211_channel *chan, int duration,
> +			  enum ieee80211_roc_type type)
> +{
> +	return 0;
> +}
> +
> +int wfx_cancel_remain_on_channel(struct ieee80211_hw *hw, struct ieee80211_vif *vif)
> +{
> +	return 0;
> +}
> diff --git a/drivers/net/wireless/silabs/wfx/scan.h b/drivers/net/wireless/silabs/wfx/scan.h
> index 78e3b984f375c..2f8361769303e 100644
> --- a/drivers/net/wireless/silabs/wfx/scan.h
> +++ b/drivers/net/wireless/silabs/wfx/scan.h
> @@ -19,4 +19,9 @@ int wfx_hw_scan(struct ieee80211_hw *hw, struct ieee80211_vif *vif,
>  void wfx_cancel_hw_scan(struct ieee80211_hw *hw, struct ieee80211_vif *vif);
>  void wfx_scan_complete(struct wfx_vif *wvif, int nb_chan_done);
>  
> +int wfx_remain_on_channel(struct ieee80211_hw *hw, struct ieee80211_vif *vif,
> +			  struct ieee80211_channel *chan, int duration,
> +			  enum ieee80211_roc_type type);
> +int wfx_cancel_remain_on_channel(struct ieee80211_hw *hw, struct ieee80211_vif *vif);
> +
>  #endif

I'm not really seeing the point of this patch. I would expect that once
.remain_on_channel is assign the feature will work without issues, for
example otherwise git bisect will not work correctly.

What about folding patches 5 and 6 into one patch? And then moving that
patch as the last to make sure that the feature is enabled on the driver
only after it works correctly?

-- 
https://patchwork.kernel.org/project/linux-wireless/list/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

^ permalink raw reply	[flat|nested] 16+ messages in thread
* Re: [PATCH v2 1/9] wifi: wfx: fix power_save setting when AP is stopped
  2023-10-04 10:20   ` Kalle Valo
@ 2023-10-04 10:37     ` Jérôme Pouiller
  2023-10-04 11:30       ` Kalle Valo
  0 siblings, 1 reply; 16+ messages in thread
From: Jérôme Pouiller @ 2023-10-04 10:37 UTC (permalink / raw)
  To: Kalle Valo; +Cc: linux-wireless, linux-kernel

Hi Kalle,

On Wednesday 4 October 2023 12:20:12 CEST Kalle Valo wrote:
> 
> Jérôme Pouiller <jerome.pouiller@silabs.com> writes:
> 
> > The WF200 allow to start two network interfaces (one AP, one station) on
> > two different channels. Since magic does not exist, it only works if the
> > station interface enables power save.
> >
> > Thus, the driver detects this case and enforce power save as necessary.
> >
> > This patch fixes the case where the AP interface is stopped and it is no
> > more necessary to enforce power saving on the station interface.
> >
> > Signed-off-by: Jérôme Pouiller <jerome.pouiller@silabs.com>
> > ---
> >  drivers/net/wireless/silabs/wfx/sta.c | 5 +++++
> >  1 file changed, 5 insertions(+)
> >
> > diff --git a/drivers/net/wireless/silabs/wfx/sta.c b/drivers/net/wireless/silabs/wfx/sta.c
> > index 626dfb4b7a55d..9c0a11c277e97 100644
> > --- a/drivers/net/wireless/silabs/wfx/sta.c
> > +++ b/drivers/net/wireless/silabs/wfx/sta.c
> > @@ -402,7 +402,12 @@ void wfx_stop_ap(struct ieee80211_hw *hw, struct ieee80211_vif *vif,
> >                struct ieee80211_bss_conf *link_conf)
> >  {
> >       struct wfx_vif *wvif = (struct wfx_vif *)vif->drv_priv;
> > +     struct wfx_dev *wdev = wvif->wdev;
> >
> > +     wvif =  NULL;
> > +     while ((wvif = wvif_iterate(wdev, wvif)) != NULL)
> > +             wfx_update_pm(wvif);
> 
> Isn't the assignment of wvif to NULL unnecessary as in the next line we
> assign it to again?

wvif is also passed as argument to wvif_iterate(). wvif_iterate() uses this
parameter to know where the iteration has stopped on previous call.

However, the assignation during the declaration of wvif is useless.

-- 
Jérôme Pouiller



^ permalink raw reply	[flat|nested] 16+ messages in thread
* Re: [PATCH v2 5/9] wifi: wfx: add placeholders for remain_on_channel feature
  2023-10-04 10:23   ` Kalle Valo
@ 2023-10-04 10:40     ` Jérôme Pouiller
  0 siblings, 0 replies; 16+ messages in thread
From: Jérôme Pouiller @ 2023-10-04 10:40 UTC (permalink / raw)
  To: Kalle Valo; +Cc: linux-wireless, linux-kernel

On Wednesday 4 October 2023 12:23:30 CEST Kalle Valo wrote:
> Jérôme Pouiller <jerome.pouiller@silabs.com> writes:
> 
> > First step to implement remain_on_channel.
> >
> > Signed-off-by: Jérôme Pouiller <jerome.pouiller@silabs.com>
> > ---
> >  drivers/net/wireless/silabs/wfx/main.c |  3 +++
> >  drivers/net/wireless/silabs/wfx/scan.c | 12 ++++++++++++
> >  drivers/net/wireless/silabs/wfx/scan.h |  5 +++++
> >  3 files changed, 20 insertions(+)
> >
> > diff --git a/drivers/net/wireless/silabs/wfx/main.c b/drivers/net/wireless/silabs/wfx/main.c
> > index ede822d771aaf..31f6e0d3dc089 100644
> > --- a/drivers/net/wireless/silabs/wfx/main.c
> > +++ b/drivers/net/wireless/silabs/wfx/main.c
> > @@ -151,6 +151,8 @@ static const struct ieee80211_ops wfx_ops = {
> >       .change_chanctx          = wfx_change_chanctx,
> >       .assign_vif_chanctx      = wfx_assign_vif_chanctx,
> >       .unassign_vif_chanctx    = wfx_unassign_vif_chanctx,
> > +     .remain_on_channel       = wfx_remain_on_channel,
> > +     .cancel_remain_on_channel = wfx_cancel_remain_on_channel,
> >  };
> >
> >  bool wfx_api_older_than(struct wfx_dev *wdev, int major, int minor)
> > @@ -288,6 +290,7 @@ struct wfx_dev *wfx_init_common(struct device *dev, const struct wfx_platform_da
> >       hw->wiphy->features |= NL80211_FEATURE_AP_SCAN;
> >       hw->wiphy->flags |= WIPHY_FLAG_AP_PROBE_RESP_OFFLOAD;
> >       hw->wiphy->flags |= WIPHY_FLAG_AP_UAPSD;
> > +     hw->wiphy->max_remain_on_channel_duration = 5000;
> >       hw->wiphy->max_ap_assoc_sta = HIF_LINK_ID_MAX;
> >       hw->wiphy->max_scan_ssids = 2;
> >       hw->wiphy->max_scan_ie_len = IEEE80211_MAX_DATA_LEN;
> > diff --git a/drivers/net/wireless/silabs/wfx/scan.c b/drivers/net/wireless/silabs/wfx/scan.c
> > index 16f619ed22e00..51338fd43ae4f 100644
> > --- a/drivers/net/wireless/silabs/wfx/scan.c
> > +++ b/drivers/net/wireless/silabs/wfx/scan.c
> > @@ -145,3 +145,15 @@ void wfx_scan_complete(struct wfx_vif *wvif, int nb_chan_done)
> >       wvif->scan_nb_chan_done = nb_chan_done;
> >       complete(&wvif->scan_complete);
> >  }
> > +
> > +int wfx_remain_on_channel(struct ieee80211_hw *hw, struct ieee80211_vif *vif,
> > +                       struct ieee80211_channel *chan, int duration,
> > +                       enum ieee80211_roc_type type)
> > +{
> > +     return 0;
> > +}
> > +
> > +int wfx_cancel_remain_on_channel(struct ieee80211_hw *hw, struct ieee80211_vif *vif)
> > +{
> > +     return 0;
> > +}
> > diff --git a/drivers/net/wireless/silabs/wfx/scan.h b/drivers/net/wireless/silabs/wfx/scan.h
> > index 78e3b984f375c..2f8361769303e 100644
> > --- a/drivers/net/wireless/silabs/wfx/scan.h
> > +++ b/drivers/net/wireless/silabs/wfx/scan.h
> > @@ -19,4 +19,9 @@ int wfx_hw_scan(struct ieee80211_hw *hw, struct ieee80211_vif *vif,
> >  void wfx_cancel_hw_scan(struct ieee80211_hw *hw, struct ieee80211_vif *vif);
> >  void wfx_scan_complete(struct wfx_vif *wvif, int nb_chan_done);
> >
> > +int wfx_remain_on_channel(struct ieee80211_hw *hw, struct ieee80211_vif *vif,
> > +                       struct ieee80211_channel *chan, int duration,
> > +                       enum ieee80211_roc_type type);
> > +int wfx_cancel_remain_on_channel(struct ieee80211_hw *hw, struct ieee80211_vif *vif);
> > +
> >  #endif
> 
> I'm not really seeing the point of this patch. I would expect that once
> .remain_on_channel is assign the feature will work without issues, for
> example otherwise git bisect will not work correctly.
> 
> What about folding patches 5 and 6 into one patch? And then moving that
> patch as the last to make sure that the feature is enabled on the driver
> only after it works correctly?

Ok.

I think I will have to reword a bit the commit logs but the reordering should
not be difficult.


-- 
Jérôme Pouiller



^ permalink raw reply	[flat|nested] 16+ messages in thread
* Re: [PATCH v2 1/9] wifi: wfx: fix power_save setting when AP is stopped
  2023-10-04 10:37     ` Jérôme Pouiller
@ 2023-10-04 11:30       ` Kalle Valo
  2023-10-04 17:26         ` Jérôme Pouiller
  0 siblings, 1 reply; 16+ messages in thread
From: Kalle Valo @ 2023-10-04 11:30 UTC (permalink / raw)
  To: Jérôme Pouiller; +Cc: linux-wireless, linux-kernel

Jérôme Pouiller <jerome.pouiller@silabs.com> writes:

> Hi Kalle,
>
> On Wednesday 4 October 2023 12:20:12 CEST Kalle Valo wrote:
>> 
>> Jérôme Pouiller <jerome.pouiller@silabs.com> writes:
>> 
>> > The WF200 allow to start two network interfaces (one AP, one station) on
>> > two different channels. Since magic does not exist, it only works if the
>> > station interface enables power save.
>> >
>> > Thus, the driver detects this case and enforce power save as necessary.
>> >
>> > This patch fixes the case where the AP interface is stopped and it is no
>> > more necessary to enforce power saving on the station interface.
>> >
>> > Signed-off-by: Jérôme Pouiller <jerome.pouiller@silabs.com>
>> > ---
>> >  drivers/net/wireless/silabs/wfx/sta.c | 5 +++++
>> >  1 file changed, 5 insertions(+)
>> >
>> > diff --git a/drivers/net/wireless/silabs/wfx/sta.c b/drivers/net/wireless/silabs/wfx/sta.c
>> > index 626dfb4b7a55d..9c0a11c277e97 100644
>> > --- a/drivers/net/wireless/silabs/wfx/sta.c
>> > +++ b/drivers/net/wireless/silabs/wfx/sta.c
>> > @@ -402,7 +402,12 @@ void wfx_stop_ap(struct ieee80211_hw *hw, struct ieee80211_vif *vif,
>> >                struct ieee80211_bss_conf *link_conf)
>> >  {
>> >       struct wfx_vif *wvif = (struct wfx_vif *)vif->drv_priv;
>> > +     struct wfx_dev *wdev = wvif->wdev;
>> >
>> > +     wvif =  NULL;
>> > +     while ((wvif = wvif_iterate(wdev, wvif)) != NULL)
>> > +             wfx_update_pm(wvif);
>> 
>> Isn't the assignment of wvif to NULL unnecessary as in the next line we
>> assign it to again?
>
> wvif is also passed as argument to wvif_iterate(). wvif_iterate() uses this
> parameter to know where the iteration has stopped on previous call.

Ah, I missed that.

> However, the assignation during the declaration of wvif is useless.

Indeed, missed that as well.

-- 
https://patchwork.kernel.org/project/linux-wireless/list/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

^ permalink raw reply	[flat|nested] 16+ messages in thread
* Re: [PATCH v2 1/9] wifi: wfx: fix power_save setting when AP is stopped
  2023-10-04 11:30       ` Kalle Valo
@ 2023-10-04 17:26         ` Jérôme Pouiller
  0 siblings, 0 replies; 16+ messages in thread
From: Jérôme Pouiller @ 2023-10-04 17:26 UTC (permalink / raw)
  To: Kalle Valo; +Cc: linux-wireless, linux-kernel

On Wednesday 4 October 2023 13:30:45 CEST Kalle Valo wrote:
> CAUTION: This email originated from outside of the organization. Do not click links or open attachments unless you recognize the sender and know the content is safe.
> 
> 
> Jérôme Pouiller <jerome.pouiller@silabs.com> writes:
> 
> > Hi Kalle,
> >
> > On Wednesday 4 October 2023 12:20:12 CEST Kalle Valo wrote:
> >>
> >> Jérôme Pouiller <jerome.pouiller@silabs.com> writes:
> >>
> >> > The WF200 allow to start two network interfaces (one AP, one station) on
> >> > two different channels. Since magic does not exist, it only works if the
> >> > station interface enables power save.
> >> >
> >> > Thus, the driver detects this case and enforce power save as necessary.
> >> >
> >> > This patch fixes the case where the AP interface is stopped and it is no
> >> > more necessary to enforce power saving on the station interface.
> >> >
> >> > Signed-off-by: Jérôme Pouiller <jerome.pouiller@silabs.com>
> >> > ---
> >> >  drivers/net/wireless/silabs/wfx/sta.c | 5 +++++
> >> >  1 file changed, 5 insertions(+)
> >> >
> >> > diff --git a/drivers/net/wireless/silabs/wfx/sta.c b/drivers/net/wireless/silabs/wfx/sta.c
> >> > index 626dfb4b7a55d..9c0a11c277e97 100644
> >> > --- a/drivers/net/wireless/silabs/wfx/sta.c
> >> > +++ b/drivers/net/wireless/silabs/wfx/sta.c
> >> > @@ -402,7 +402,12 @@ void wfx_stop_ap(struct ieee80211_hw *hw, struct ieee80211_vif *vif,
> >> >                struct ieee80211_bss_conf *link_conf)
> >> >  {
> >> >       struct wfx_vif *wvif = (struct wfx_vif *)vif->drv_priv;
> >> > +     struct wfx_dev *wdev = wvif->wdev;
> >> >
> >> > +     wvif =  NULL;
> >> > +     while ((wvif = wvif_iterate(wdev, wvif)) != NULL)
> >> > +             wfx_update_pm(wvif);
> >>
> >> Isn't the assignment of wvif to NULL unnecessary as in the next line we
> >> assign it to again?
> >
> > wvif is also passed as argument to wvif_iterate(). wvif_iterate() uses this
> > parameter to know where the iteration has stopped on previous call.
> 
> Ah, I missed that.
> 
> > However, the assignation during the declaration of wvif is useless.
> 
> Indeed, missed that as well.

In fact, this first assignation is used to fill wdev just below. So I am
keeping this code as-is.

-- 
Jérôme Pouiller



^ permalink raw reply	[flat|nested] 16+ messages in thread
end of thread, other threads:[~2023-10-04 17:26 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-09-27 16:32 [PATCH v2 0/9] wfx: implement Remain On Channel Jérôme Pouiller
2023-09-27 16:32 ` [PATCH v2 1/9] wifi: wfx: fix power_save setting when AP is stopped Jérôme Pouiller
2023-10-04 10:20   ` Kalle Valo
2023-10-04 10:37     ` Jérôme Pouiller
2023-10-04 11:30       ` Kalle Valo
2023-10-04 17:26         ` Jérôme Pouiller
2023-09-27 16:32 ` [PATCH v2 2/9] wifi: wfx: relocate wfx_rate_mask_to_hw() Jérôme Pouiller
2023-09-27 16:32 ` [PATCH v2 3/9] wifi: wfx: move wfx_skb_*() out of the header file Jérôme Pouiller
2023-09-27 16:32 ` [PATCH v2 4/9] wifi: wfx: introduce hif_scan_uniq() Jérôme Pouiller
2023-09-27 16:32 ` [PATCH v2 5/9] wifi: wfx: add placeholders for remain_on_channel feature Jérôme Pouiller
2023-10-04 10:23   ` Kalle Valo
2023-10-04 10:40     ` Jérôme Pouiller
2023-09-27 16:32 ` [PATCH v2 6/9] wifi: wfx: implement wfx_remain_on_channel() Jérôme Pouiller
2023-09-27 16:32 ` [PATCH v2 7/9] wifi: wfx: allow to send frames during ROC Jérôme Pouiller
2023-09-27 16:32 ` [PATCH v2 8/9] wifi: wfx: scan_lock is global to the device Jérôme Pouiller
2023-09-27 16:32 ` [PATCH v2 9/9] wifi: wfx: fix possible lock-up between scan and Rx filters Jérôme Pouiller

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.