[PATCH rtw-next v4 0/2] wifi: rtw88: channel switch in AP mode

[PATCH rtw-next v4 0/2] wifi: rtw88: channel switch in AP mode

[Mehmet Fide]

From: Mehmet Fide <mehmet.fide@screeningeagle.com>

An rtw88 AP cannot change its channel: the driver does not announce
channel switch support, so hostapd's CHAN_SWITCH is refused and the
only way is to tear the AP down. The series fixes the beacon download
the reserved page build makes twice, then implements
channel_switch_beacon on top of the firmware's beacon page.

v4:
- 1/2: Reviewed-by from Luka
- 2/2: the countdown work is back in the device, initialised once,
  with csa_vif naming the interface; no interface iteration and no
  lookup through the reserved page any more (Ping-Ke)
- 2/2: the countdown is cancelled before a firmware recovery restarts
  the hardware, instead of guarding the work with flags (Ping-Ke)
- 2/2: the page download on association skips a running countdown,
  like the TIM update and the PG backup (Luka)
- 2/2: the scan refusal comes before the scan offload test, so that
  firmware without scan offload does not fall back to a software scan
  during a countdown (Luka)
- tested on an RTL8821CU and an RTL8822CU as AP (counts 1 to 5,
  switches aborted and restarted, 50 switches with a station attached)
  and as IBSS

v3:
- 1/2: the beacon out-pointer is initialised in the caller (Luka)
- 2/2: the countdown work lives in the interface, remove_interface
  cancels it unconditionally, the queue wrapper and the comments are
  gone (Ping-Ke)
- 2/2: the TIM update and the PG backup on set_key skip the page
  download while a switch is announced; every beacon fetch advances
  the countdown (Luka)
- 2/2: the commit message names the refused hardware scan (Ping-Ke)
- the hardware scan path ran on an RTL8822CU, whose firmware has scan
  offload: a scan with the AP up keeps the beacon and the station, a
  scan during a countdown is refused

v2:
- 2/2: a hardware scan is refused while a switch is announced
- the rtw89 mention is gone from the cover

Mehmet Fide (2):
  wifi: rtw88: download the beacon the reserved page was built with
  wifi: rtw88: support channel switch in AP mode

 drivers/net/wireless/realtek/rtw88/fw.c       | 96 ++++++++++++++++---
 drivers/net/wireless/realtek/rtw88/fw.h       |  4 +
 drivers/net/wireless/realtek/rtw88/mac80211.c | 47 +++++++--
 drivers/net/wireless/realtek/rtw88/main.c     |  9 +-
 drivers/net/wireless/realtek/rtw88/main.h     |  2 +
 5 files changed, 136 insertions(+), 22 deletions(-)

-- 
2.55.0

[PATCH rtw-next v4 1/2] wifi: rtw88: download the beacon the reserved page was built with

[Mehmet Fide]

From: Mehmet Fide <mehmet.fide@screeningeagle.com>

rtw_fw_download_rsvd_page() downloads the beacon twice: once as part of
the reserved page and once more on its own, so that the firmware ends up
with a TX descriptor that describes the beacon rather than the whole
page. The second download fetched a new beacon from mac80211 instead of
downloading the one the page already holds.

Besides the extra work, every beacon fetch advances the DTIM count and,
while a channel switch is announced, the CSA countdown; doing it twice
per update lets a countdown that starts at 2 reach 0, which mac80211
warns about. Hand the beacon skb out of the page build and download
that. The hw scan, which downloads the beacon without a page build,
keeps fetching its own.

Signed-off-by: Mehmet Fide <mehmet.fide@screeningeagle.com>
Reviewed-by: Luka Gejak <luka.gejak@linux.dev>
Acked-by: Ping-Ke Shih <pkshih@realtek.com>
---
 drivers/net/wireless/realtek/rtw88/fw.c | 34 +++++++++++++++----------
 1 file changed, 21 insertions(+), 13 deletions(-)

diff --git a/drivers/net/wireless/realtek/rtw88/fw.c b/drivers/net/wireless/realtek/rtw88/fw.c
index 945fedcd375b..3cd17a3bb494 100644
--- a/drivers/net/wireless/realtek/rtw88/fw.c
+++ b/drivers/net/wireless/realtek/rtw88/fw.c
@@ -1622,7 +1622,8 @@ static int  __rtw_build_rsvd_page_from_vifs(struct rtw_dev *rtwdev)
 	return 0;
 }
 
-static u8 *rtw_build_rsvd_page(struct rtw_dev *rtwdev, u32 *size)
+static u8 *rtw_build_rsvd_page(struct rtw_dev *rtwdev, u32 *size,
+			       struct sk_buff **beacon)
 {
 	const struct rtw_chip_info *chip = rtwdev->chip;
 	struct ieee80211_hw *hw = rtwdev->hw;
@@ -1702,13 +1703,15 @@ static u8 *rtw_build_rsvd_page(struct rtw_dev *rtwdev, u32 *size)
 	list_for_each_entry(rsvd_pkt, &rtwdev->rsvd_page_list, build_list) {
 		rtw_rsvd_page_list_to_buf(rtwdev, page_size, page_margin,
 					  page, buf, rsvd_pkt);
-		if (page == 0)
+		if (page == 0) {
 			page += rtw_len_to_page(rsvd_pkt->skb->len +
 						tx_desc_sz, page_size);
-		else
+			/* the caller downloads it once more on its own */
+			*beacon = rsvd_pkt->skb;
+		} else {
 			page += rtw_len_to_page(rsvd_pkt->skb->len, page_size);
-
-		kfree_skb(rsvd_pkt->skb);
+			kfree_skb(rsvd_pkt->skb);
+		}
 		rsvd_pkt->skb = NULL;
 	}
 
@@ -1723,11 +1726,12 @@ static u8 *rtw_build_rsvd_page(struct rtw_dev *rtwdev, u32 *size)
 	return NULL;
 }
 
-static int rtw_download_beacon(struct rtw_dev *rtwdev)
+/* the beacon the page was built with, or a fresh one for the hw scan */
+static int rtw_download_beacon(struct rtw_dev *rtwdev, struct sk_buff *beacon)
 {
 	struct ieee80211_hw *hw = rtwdev->hw;
 	struct rtw_rsvd_page *rsvd_pkt;
-	struct sk_buff *skb;
+	struct sk_buff *skb = beacon;
 	int ret = 0;
 
 	rsvd_pkt = list_first_entry_or_null(&rtwdev->rsvd_page_list,
@@ -1744,7 +1748,8 @@ static int rtw_download_beacon(struct rtw_dev *rtwdev)
 		return -EINVAL;
 	}
 
-	skb = rtw_get_rsvd_page_skb(hw, rsvd_pkt);
+	if (!skb)
+		skb = rtw_get_rsvd_page_skb(hw, rsvd_pkt);
 	if (!skb) {
 		rtw_err(rtwdev, "failed to get beacon skb\n");
 		return -ENOMEM;
@@ -1754,18 +1759,20 @@ static int rtw_download_beacon(struct rtw_dev *rtwdev)
 	if (ret)
 		rtw_err(rtwdev, "failed to download drv rsvd page\n");
 
-	dev_kfree_skb(skb);
+	if (!beacon)
+		dev_kfree_skb(skb);
 
 	return ret;
 }
 
 int rtw_fw_download_rsvd_page(struct rtw_dev *rtwdev)
 {
-	u8 *buf;
+	struct sk_buff *beacon = NULL;
 	u32 size;
+	u8 *buf;
 	int ret;
 
-	buf = rtw_build_rsvd_page(rtwdev, &size);
+	buf = rtw_build_rsvd_page(rtwdev, &size, &beacon);
 	if (!buf) {
 		rtw_err(rtwdev, "failed to build rsvd page pkt\n");
 		return -ENOMEM;
@@ -1782,13 +1789,14 @@ int rtw_fw_download_rsvd_page(struct rtw_dev *rtwdev)
 	 * the beacon again to replace the TX desc header, and we will get
 	 * a correct tx_desc for the beacon in the rsvd page.
 	 */
-	ret = rtw_download_beacon(rtwdev);
+	ret = rtw_download_beacon(rtwdev, beacon);
 	if (ret) {
 		rtw_err(rtwdev, "failed to download beacon\n");
 		goto free;
 	}
 
 free:
+	dev_kfree_skb(beacon);
 	kfree(buf);
 
 	return ret;
@@ -2345,7 +2353,7 @@ int rtw_hw_scan_offload(struct rtw_dev *rtwdev, struct ieee80211_vif *vif,
 	rtw_fw_set_scan_offload(rtwdev, &cs_option, rtwvif, &chan_list);
 out:
 	if (rtwdev->ap_active) {
-		ret = rtw_download_beacon(rtwdev);
+		ret = rtw_download_beacon(rtwdev, NULL);
 		if (ret)
 			rtw_err(rtwdev, "HW scan download beacon failed\n");
 	}
-- 
2.55.0

[PATCH rtw-next v4 2/2] wifi: rtw88: support channel switch in AP mode

[Mehmet Fide]

From: Mehmet Fide <mehmet.fide@screeningeagle.com>

hostapd's CHAN_SWITCH is refused because the driver does not announce
channel switch support, so an AP on rtw88 can only change its channel
by being torn down and started again.

Declare WIPHY_FLAG_HAS_CHANNEL_SWITCH and implement
ieee80211_ops::channel_switch_beacon: the firmware repeats the beacon
held in the first reserved page, so while a switch is announced the
page is downloaded again every beacon interval to renew the countdown,
and ieee80211_csa_finish() is called once it completes. IBSS, which
the flag enables too, shares the page and the work.

The work is a wiphy delayed work of the device and csa_vif names the
interface it counts down for; the wiphy lock serializes the work with
the mac80211 state it reads and the driver mutex with the page build.
The countdown is cancelled when the AP stops, when the interface goes
away, on WoWLAN suspend and before a firmware recovery restarts the
hardware. The other page downloads that can run
during a countdown, the TIM update, the PG backup on set_key and the
one on association, are skipped then, since every beacon fetch
advances the countdown; the next countdown download, at most one
beacon interval later, carries their changes. A scan is refused while
a switch is announced, before the fallback to a software scan: it
would take the AP off the channel its stations count down to.

Signed-off-by: Mehmet Fide <mehmet.fide@screeningeagle.com>
---
 drivers/net/wireless/realtek/rtw88/fw.c       | 62 +++++++++++++++++++
 drivers/net/wireless/realtek/rtw88/fw.h       |  4 ++
 drivers/net/wireless/realtek/rtw88/mac80211.c | 47 +++++++++++---
 drivers/net/wireless/realtek/rtw88/main.c     |  9 ++-
 drivers/net/wireless/realtek/rtw88/main.h     |  2 +
 5 files changed, 115 insertions(+), 9 deletions(-)

diff --git a/drivers/net/wireless/realtek/rtw88/fw.c b/drivers/net/wireless/realtek/rtw88/fw.c
index 3cd17a3bb494..5d51d0122e6e 100644
--- a/drivers/net/wireless/realtek/rtw88/fw.c
+++ b/drivers/net/wireless/realtek/rtw88/fw.c
@@ -1802,14 +1802,76 @@ int rtw_fw_download_rsvd_page(struct rtw_dev *rtwdev)
 	return ret;
 }
 
+bool rtw_fw_csa_active(struct rtw_dev *rtwdev)
+{
+	return rtwdev->csa_vif && rtwdev->csa_vif->bss_conf.csa_active;
+}
+
+void rtw_fw_csa_start(struct rtw_dev *rtwdev, struct ieee80211_vif *vif)
+{
+	u32 interval = ieee80211_tu_to_usec(vif->bss_conf.beacon_int);
+
+	rtwdev->csa_vif = vif;
+	wiphy_delayed_work_queue(rtwdev->hw->wiphy, &rtwdev->csa_beacon_work,
+				 usecs_to_jiffies(interval));
+}
+
+void rtw_fw_csa_stop(struct rtw_dev *rtwdev, struct ieee80211_vif *vif)
+{
+	if (!vif || rtwdev->csa_vif != vif)
+		return;
+
+	wiphy_delayed_work_cancel(rtwdev->hw->wiphy, &rtwdev->csa_beacon_work);
+	rtwdev->csa_vif = NULL;
+}
+
 void rtw_fw_update_beacon_work(struct work_struct *work)
 {
 	struct rtw_dev *rtwdev = container_of(work, struct rtw_dev,
 					      update_beacon_work);
 
 	mutex_lock(&rtwdev->mutex);
+
+	if (rtw_fw_csa_active(rtwdev))
+		goto out;
+
 	rtw_fw_download_rsvd_page(rtwdev);
 	rtw_send_rsvd_page_h2c(rtwdev);
+
+out:
+	mutex_unlock(&rtwdev->mutex);
+}
+
+void rtw_fw_csa_beacon_work(struct wiphy *wiphy, struct wiphy_work *work)
+{
+	struct rtw_dev *rtwdev = container_of(work, struct rtw_dev,
+					      csa_beacon_work.work);
+	struct ieee80211_vif *vif = rtwdev->csa_vif;
+	unsigned int delay;
+
+	lockdep_assert_wiphy(wiphy);
+
+	mutex_lock(&rtwdev->mutex);
+
+	if (!vif->bss_conf.csa_active) {
+		rtwdev->csa_vif = NULL;
+		goto out;
+	}
+
+	delay = ieee80211_tu_to_usec(vif->bss_conf.beacon_int);
+
+	if (!ieee80211_beacon_cntdwn_is_complete(vif, 0)) {
+		rtw_fw_download_rsvd_page(rtwdev);
+		rtw_send_rsvd_page_h2c(rtwdev);
+
+		wiphy_delayed_work_queue(wiphy, &rtwdev->csa_beacon_work,
+					 usecs_to_jiffies(delay));
+	} else {
+		rtwdev->csa_vif = NULL;
+		ieee80211_csa_finish(vif, 0);
+	}
+
+out:
 	mutex_unlock(&rtwdev->mutex);
 }
 
diff --git a/drivers/net/wireless/realtek/rtw88/fw.h b/drivers/net/wireless/realtek/rtw88/fw.h
index 48ad9ceab6ea..dba9d5ce7e05 100644
--- a/drivers/net/wireless/realtek/rtw88/fw.h
+++ b/drivers/net/wireless/realtek/rtw88/fw.h
@@ -863,7 +863,11 @@ void rtw_add_rsvd_page_pno(struct rtw_dev *rtwdev,
 void rtw_add_rsvd_page_sta(struct rtw_dev *rtwdev,
 			   struct rtw_vif *rtwvif);
 int rtw_fw_download_rsvd_page(struct rtw_dev *rtwdev);
+bool rtw_fw_csa_active(struct rtw_dev *rtwdev);
+void rtw_fw_csa_start(struct rtw_dev *rtwdev, struct ieee80211_vif *vif);
+void rtw_fw_csa_stop(struct rtw_dev *rtwdev, struct ieee80211_vif *vif);
 void rtw_fw_update_beacon_work(struct work_struct *work);
+void rtw_fw_csa_beacon_work(struct wiphy *wiphy, struct wiphy_work *work);
 void rtw_send_rsvd_page_h2c(struct rtw_dev *rtwdev);
 int rtw_dump_drv_rsvd_page(struct rtw_dev *rtwdev,
 			   u32 offset, u32 size, u32 *buf);
diff --git a/drivers/net/wireless/realtek/rtw88/mac80211.c b/drivers/net/wireless/realtek/rtw88/mac80211.c
index 2a9b09fa76e7..087268909f5f 100644
--- a/drivers/net/wireless/realtek/rtw88/mac80211.c
+++ b/drivers/net/wireless/realtek/rtw88/mac80211.c
@@ -235,6 +235,8 @@ static void rtw_ops_remove_interface(struct ieee80211_hw *hw,
 	rtw_dbg(rtwdev, RTW_DBG_STATE, "stop vif %pM mac_id %d on port %d\n",
 		vif->addr, rtwvif->mac_id, rtwvif->port);
 
+	rtw_fw_csa_stop(rtwdev, vif);
+
 	mutex_lock(&rtwdev->mutex);
 
 	rtw_leave_lps_deep(rtwdev);
@@ -395,8 +397,10 @@ static void rtw_ops_bss_info_changed(struct ieee80211_hw *hw,
 		if (vif->cfg.assoc) {
 			rtw_coex_connect_notify(rtwdev, COEX_ASSOCIATE_FINISH);
 
-			rtw_fw_download_rsvd_page(rtwdev);
-			rtw_send_rsvd_page_h2c(rtwdev);
+			if (!rtw_fw_csa_active(rtwdev)) {
+				rtw_fw_download_rsvd_page(rtwdev);
+				rtw_send_rsvd_page_h2c(rtwdev);
+			}
 			rtw_fw_default_port(rtwdev, rtwvif);
 			rtw_coex_media_status_notify(rtwdev, vif->cfg.assoc);
 			if (rtw_bf_support)
@@ -438,6 +442,8 @@ static void rtw_ops_bss_info_changed(struct ieee80211_hw *hw,
 		rtw_set_dtim_period(rtwdev, conf->dtim_period);
 		rtw_fw_download_rsvd_page(rtwdev);
 		rtw_send_rsvd_page_h2c(rtwdev);
+		if (conf->csa_active)
+			rtw_fw_csa_start(rtwdev, vif);
 	}
 
 	if (changed & BSS_CHANGED_BEACON_ENABLED) {
@@ -489,6 +495,8 @@ static void rtw_ops_stop_ap(struct ieee80211_hw *hw,
 {
 	struct rtw_dev *rtwdev = hw->priv;
 
+	rtw_fw_csa_stop(rtwdev, vif);
+
 	mutex_lock(&rtwdev->mutex);
 	rtw_write32_clr(rtwdev, REG_TCR, BIT_TCR_UPDATE_HGQMD);
 	rtw_write16(rtwdev, REG_ATIMWND, ATIMWND_DEFAULT);
@@ -556,6 +564,15 @@ static int rtw_ops_set_tim(struct ieee80211_hw *hw, struct ieee80211_sta *sta,
 	return 0;
 }
 
+static void rtw_ops_channel_switch_beacon(struct ieee80211_hw *hw,
+					  struct ieee80211_vif *vif,
+					  struct cfg80211_chan_def *chandef)
+{
+	struct rtw_dev *rtwdev = hw->priv;
+
+	rtw_fw_csa_start(rtwdev, vif);
+}
+
 static int rtw_ops_set_key(struct ieee80211_hw *hw, enum set_key_cmd cmd,
 			   struct ieee80211_vif *vif, struct ieee80211_sta *sta,
 			   struct ieee80211_key_conf *key)
@@ -626,7 +643,8 @@ static int rtw_ops_set_key(struct ieee80211_hw *hw, enum set_key_cmd cmd,
 	}
 
 	/* download new cam settings for PG to backup */
-	if (rtw_get_lps_deep_mode(rtwdev) == LPS_DEEP_MODE_PG)
+	if (rtw_get_lps_deep_mode(rtwdev) == LPS_DEEP_MODE_PG &&
+	    !rtw_fw_csa_active(rtwdev))
 		rtw_fw_download_rsvd_page(rtwdev);
 
 out:
@@ -846,6 +864,7 @@ static int rtw_ops_suspend(struct ieee80211_hw *hw,
 	int ret;
 
 	mutex_lock(&rtwdev->mutex);
+	rtw_fw_csa_stop(rtwdev, rtwdev->csa_vif);
 	ret = rtw_wow_suspend(rtwdev, wowlan);
 	if (ret)
 		rtw_err(rtwdev, "failed to suspend for wow %d\n", ret);
@@ -893,19 +912,30 @@ static int rtw_ops_hw_scan(struct ieee80211_hw *hw, struct ieee80211_vif *vif,
 	struct rtw_dev *rtwdev = hw->priv;
 	int ret;
 
-	if (!rtw_fw_feature_check(&rtwdev->fw, FW_FEATURE_SCAN_OFFLOAD))
-		return 1;
+	mutex_lock(&rtwdev->mutex);
 
-	if (test_bit(RTW_FLAG_SCANNING, rtwdev->flags))
-		return -EBUSY;
+	if (rtw_fw_csa_active(rtwdev)) {
+		ret = -EBUSY;
+		goto out;
+	}
+
+	if (!rtw_fw_feature_check(&rtwdev->fw, FW_FEATURE_SCAN_OFFLOAD)) {
+		ret = 1;
+		goto out;
+	}
+
+	if (test_bit(RTW_FLAG_SCANNING, rtwdev->flags)) {
+		ret = -EBUSY;
+		goto out;
+	}
 
-	mutex_lock(&rtwdev->mutex);
 	rtw_hw_scan_start(rtwdev, vif, req);
 	ret = rtw_hw_scan_offload(rtwdev, vif, true);
 	if (ret) {
 		rtw_hw_scan_abort(rtwdev);
 		rtw_err(rtwdev, "HW scan failed with status: %d\n", ret);
 	}
+out:
 	mutex_unlock(&rtwdev->mutex);
 
 	return ret;
@@ -973,6 +1003,7 @@ const struct ieee80211_ops rtw_ops = {
 	.sta_add		= rtw_ops_sta_add,
 	.sta_remove		= rtw_ops_sta_remove,
 	.set_tim		= rtw_ops_set_tim,
+	.channel_switch_beacon	= rtw_ops_channel_switch_beacon,
 	.set_key		= rtw_ops_set_key,
 	.ampdu_action		= rtw_ops_ampdu_action,
 	.can_aggregate_in_amsdu	= rtw_ops_can_aggregate_in_amsdu,
diff --git a/drivers/net/wireless/realtek/rtw88/main.c b/drivers/net/wireless/realtek/rtw88/main.c
index 0f23498b5c96..3a140fdd5d98 100644
--- a/drivers/net/wireless/realtek/rtw88/main.c
+++ b/drivers/net/wireless/realtek/rtw88/main.c
@@ -676,6 +676,10 @@ static void rtw_fw_recovery_work(struct work_struct *work)
 	struct rtw_dev *rtwdev = container_of(work, struct rtw_dev,
 					      fw_recovery_work);
 
+	wiphy_lock(rtwdev->hw->wiphy);
+	rtw_fw_csa_stop(rtwdev, rtwdev->csa_vif);
+	wiphy_unlock(rtwdev->hw->wiphy);
+
 	mutex_lock(&rtwdev->mutex);
 	__fw_recovery_work(rtwdev);
 	mutex_unlock(&rtwdev->mutex);
@@ -2170,6 +2174,8 @@ int rtw_core_init(struct rtw_dev *rtwdev)
 	INIT_WORK(&rtwdev->ips_work, rtw_ips_work);
 	INIT_WORK(&rtwdev->fw_recovery_work, rtw_fw_recovery_work);
 	INIT_WORK(&rtwdev->update_beacon_work, rtw_fw_update_beacon_work);
+	wiphy_delayed_work_init(&rtwdev->csa_beacon_work,
+				rtw_fw_csa_beacon_work);
 	INIT_WORK(&rtwdev->ba_work, rtw_txq_ba_work);
 	skb_queue_head_init(&rtwdev->c2h_queue);
 	skb_queue_head_init(&rtwdev->coex.queue);
@@ -2293,7 +2299,8 @@ int rtw_register_hw(struct rtw_dev *rtwdev, struct ieee80211_hw *hw)
 	hw->wiphy->available_antennas_rx = hal->antenna_rx;
 
 	hw->wiphy->flags |= WIPHY_FLAG_SUPPORTS_TDLS |
-			    WIPHY_FLAG_TDLS_EXTERNAL_SETUP;
+			    WIPHY_FLAG_TDLS_EXTERNAL_SETUP |
+			    WIPHY_FLAG_HAS_CHANNEL_SWITCH;
 
 	hw->wiphy->features |= NL80211_FEATURE_SCAN_RANDOM_MAC_ADDR;
 	hw->wiphy->max_scan_ssids = RTW_SCAN_MAX_SSIDS;
diff --git a/drivers/net/wireless/realtek/rtw88/main.h b/drivers/net/wireless/realtek/rtw88/main.h
index d59f6e323adf..0c82ad5a3650 100644
--- a/drivers/net/wireless/realtek/rtw88/main.h
+++ b/drivers/net/wireless/realtek/rtw88/main.h
@@ -2094,6 +2094,8 @@ struct rtw_dev {
 	struct work_struct ips_work;
 	struct work_struct fw_recovery_work;
 	struct work_struct update_beacon_work;
+	struct wiphy_delayed_work csa_beacon_work;
+	struct ieee80211_vif *csa_vif;
 
 	/* used to protect txqs list */
 	spinlock_t txq_lock;
-- 
2.55.0

RE: [PATCH rtw-next v4 2/2] wifi: rtw88: support channel switch in AP mode

[Ping-Ke Shih]

Mehmet Fide <mehmet.fide@gmail.com> wrote:

[...]

> --- a/drivers/net/wireless/realtek/rtw88/fw.c
> +++ b/drivers/net/wireless/realtek/rtw88/fw.c
> @@ -1802,14 +1802,76 @@ int rtw_fw_download_rsvd_page(struct rtw_dev *rtwdev)
>         return ret;
>  }
> 
> +bool rtw_fw_csa_active(struct rtw_dev *rtwdev)
> +{

add lockdep_assert_wiphy(wiphy) to rtw_fw_csa_{active,start,stop}?
Since they access rtwdev->csa_vif.

> +       return rtwdev->csa_vif && rtwdev->csa_vif->bss_conf.csa_active;
> +}
> +
> +void rtw_fw_csa_start(struct rtw_dev *rtwdev, struct ieee80211_vif *vif)
> +{
> +       u32 interval = ieee80211_tu_to_usec(vif->bss_conf.beacon_int);
> +
> +       rtwdev->csa_vif = vif;
> +       wiphy_delayed_work_queue(rtwdev->hw->wiphy, &rtwdev->csa_beacon_work,
> +                                usecs_to_jiffies(interval));
> +}
> +
> +void rtw_fw_csa_stop(struct rtw_dev *rtwdev, struct ieee80211_vif *vif)
> +{
> +       if (!vif || rtwdev->csa_vif != vif)
> +               return;
> +
> +       wiphy_delayed_work_cancel(rtwdev->hw->wiphy, &rtwdev->csa_beacon_work);
> +       rtwdev->csa_vif = NULL;
> +}
> +

[...]

> diff --git a/drivers/net/wireless/realtek/rtw88/mac80211.c
> b/drivers/net/wireless/realtek/rtw88/mac80211.c
> index 2a9b09fa76e7..087268909f5f 100644
> --- a/drivers/net/wireless/realtek/rtw88/mac80211.c
> +++ b/drivers/net/wireless/realtek/rtw88/mac80211.c
> @@ -235,6 +235,8 @@ static void rtw_ops_remove_interface(struct ieee80211_hw *hw,
>         rtw_dbg(rtwdev, RTW_DBG_STATE, "stop vif %pM mac_id %d on port %d\n",
>                 vif->addr, rtwvif->mac_id, rtwvif->port);
> 
> +       rtw_fw_csa_stop(rtwdev, vif);
> +
>         mutex_lock(&rtwdev->mutex);
> 
>         rtw_leave_lps_deep(rtwdev);
> @@ -395,8 +397,10 @@ static void rtw_ops_bss_info_changed(struct ieee80211_hw *hw,
>                 if (vif->cfg.assoc) {
>                         rtw_coex_connect_notify(rtwdev, COEX_ASSOCIATE_FINISH);
> 
> -                       rtw_fw_download_rsvd_page(rtwdev);
> -                       rtw_send_rsvd_page_h2c(rtwdev);
> +                       if (!rtw_fw_csa_active(rtwdev)) {

Does it actually happen to AP mode?

If it could happen, check the condition by conf->csa_active (like below)?

> +                               rtw_fw_download_rsvd_page(rtwdev);
> +                               rtw_send_rsvd_page_h2c(rtwdev);
> +                       }
>                         rtw_fw_default_port(rtwdev, rtwvif);
>                         rtw_coex_media_status_notify(rtwdev, vif->cfg.assoc);
>                         if (rtw_bf_support)
> @@ -438,6 +442,8 @@ static void rtw_ops_bss_info_changed(struct ieee80211_hw *hw,
>                 rtw_set_dtim_period(rtwdev, conf->dtim_period);
>                 rtw_fw_download_rsvd_page(rtwdev);
>                 rtw_send_rsvd_page_h2c(rtwdev);
> +               if (conf->csa_active)
> +                       rtw_fw_csa_start(rtwdev, vif);
>         }
> 
>         if (changed & BSS_CHANGED_BEACON_ENABLED) {
> @@ -489,6 +495,8 @@ static void rtw_ops_stop_ap(struct ieee80211_hw *hw,
>  {
>         struct rtw_dev *rtwdev = hw->priv;
> 
> +       rtw_fw_csa_stop(rtwdev, vif);
> +
>         mutex_lock(&rtwdev->mutex);
>         rtw_write32_clr(rtwdev, REG_TCR, BIT_TCR_UPDATE_HGQMD);
>         rtw_write16(rtwdev, REG_ATIMWND, ATIMWND_DEFAULT);
> @@ -556,6 +564,15 @@ static int rtw_ops_set_tim(struct ieee80211_hw *hw, struct ieee80211_sta *sta,
>         return 0;
>  }
> 
> +static void rtw_ops_channel_switch_beacon(struct ieee80211_hw *hw,
> +                                         struct ieee80211_vif *vif,
> +                                         struct cfg80211_chan_def *chandef)
> +{
> +       struct rtw_dev *rtwdev = hw->priv;
> +
> +       rtw_fw_csa_start(rtwdev, vif);
> +}
> +
>  static int rtw_ops_set_key(struct ieee80211_hw *hw, enum set_key_cmd cmd,
>                            struct ieee80211_vif *vif, struct ieee80211_sta *sta,
>                            struct ieee80211_key_conf *key)
> @@ -626,7 +643,8 @@ static int rtw_ops_set_key(struct ieee80211_hw *hw, enum set_key_cmd cmd,
>         }
> 
>         /* download new cam settings for PG to backup */
> -       if (rtw_get_lps_deep_mode(rtwdev) == LPS_DEEP_MODE_PG)
> +       if (rtw_get_lps_deep_mode(rtwdev) == LPS_DEEP_MODE_PG &&
> +           !rtw_fw_csa_active(rtwdev))

Think of this deeper.... Is it existing race between set_key and !csa->active?

>                 rtw_fw_download_rsvd_page(rtwdev);
> 
>  out:
> @@ -846,6 +864,7 @@ static int rtw_ops_suspend(struct ieee80211_hw *hw,
>         int ret;
> 
>         mutex_lock(&rtwdev->mutex);
> +       rtw_fw_csa_stop(rtwdev, rtwdev->csa_vif);

Since you access rtwdev->csa_vif, should this take wiphy_lock?

>         ret = rtw_wow_suspend(rtwdev, wowlan);
>         if (ret)
>                 rtw_err(rtwdev, "failed to suspend for wow %d\n", ret);
> @@ -893,19 +912,30 @@ static int rtw_ops_hw_scan(struct ieee80211_hw *hw, struct ieee80211_vif *vif,
>         struct rtw_dev *rtwdev = hw->priv;
>         int ret;
> 
> -       if (!rtw_fw_feature_check(&rtwdev->fw, FW_FEATURE_SCAN_OFFLOAD))
> -               return 1;
> +       mutex_lock(&rtwdev->mutex);

guard(mutex)( &rtwdev->mutex); ?

> 
> -       if (test_bit(RTW_FLAG_SCANNING, rtwdev->flags))
> -               return -EBUSY;
> +       if (rtw_fw_csa_active(rtwdev)) {
> +               ret = -EBUSY;
> +               goto out;
> +       }
> +
> +       if (!rtw_fw_feature_check(&rtwdev->fw, FW_FEATURE_SCAN_OFFLOAD)) {
> +               ret = 1;
> +               goto out;
> +       }
> +
> +       if (test_bit(RTW_FLAG_SCANNING, rtwdev->flags)) {
> +               ret = -EBUSY;
> +               goto out;
> +       }
> 
> -       mutex_lock(&rtwdev->mutex);
>         rtw_hw_scan_start(rtwdev, vif, req);
>         ret = rtw_hw_scan_offload(rtwdev, vif, true);
>         if (ret) {
>                 rtw_hw_scan_abort(rtwdev);
>                 rtw_err(rtwdev, "HW scan failed with status: %d\n", ret);
>         }
> +out:
>         mutex_unlock(&rtwdev->mutex);
> 
>         return ret;

Re: [PATCH rtw-next v4 2/2] wifi: rtw88: support channel switch in AP mode

[Mehmet Fide]

Hi Ping-Ke,

On 2026-10-07 Ping-Ke Shih wrote:
> add lockdep_assert_wiphy(wiphy) to rtw_fw_csa_{active,start,stop}?
> Since they access rtwdev->csa_vif.

Yes. Adding them showed that update_beacon_work, the TIM download,
reads csa_vif outside the wiphy lock: it is an ordinary work queued
from set_tim(). v5 makes it a wiphy work first, in a patch of its own,
the way rtw89 runs its update_beacon_work, and the three helpers then
assert the lock. The series then went through the whole AP test set
on a PROVE_LOCKING kernel (8821CU and 8822CU, including a firmware
crash during a countdown and the IPS path) without a lockdep report.

> > -                       rtw_fw_download_rsvd_page(rtwdev);
> > -                       rtw_send_rsvd_page_h2c(rtwdev);
> > +                       if (!rtw_fw_csa_active(rtwdev)) {
>
> Does it actually happen to AP mode?
>
> If it could happen, check the condition by conf->csa_active (like below)?

Not for the AP interface: mac80211 raises BSS_CHANGED_ASSOC for a
station interface only. The hunk is for the STA+AP combination the
driver registers for the 8822C, and it does happen there: with an AP
on an RTL8822CU counting down from channel 6 to 11, a station interface
on the same device that had a fresh scan result associated with an AP
on channel 11 while the countdown ran, so BSS_CHANGED_ASSOC arrived in
the middle of it. That download would rebuild the reserved page with a
fresh beacon. conf belongs to the station there, so conf->csa_active
would say nothing about the AP; that is why the check is on
rtwdev->csa_vif.

> > -       if (rtw_get_lps_deep_mode(rtwdev) == LPS_DEEP_MODE_PG)
> > +       if (rtw_get_lps_deep_mode(rtwdev) == LPS_DEEP_MODE_PG &&
> > +           !rtw_fw_csa_active(rtwdev))
>
> Think of this deeper.... Is it existing race between set_key and !csa->active?

I do not see one. set_key(), the countdown work and the places that
set and clear csa_active all run under the wiphy lock, so the flag
cannot change under the check. The download set_key() skips is not
lost either: ieee80211_csa_finish() ends in ieee80211_csa_finalize(),
which assigns the next beacon with BSS_CHANGED_BEACON, and that path
rebuilds the whole page, CAM backup included, once the countdown is
over. Did you have another race in mind?

> > +       rtw_fw_csa_stop(rtwdev, rtwdev->csa_vif);
>
> Since you access rtwdev->csa_vif, should this take wiphy_lock?

It is held already: cfg80211's wiphy_suspend() calls rdev_suspend()
inside scoped_guard(wiphy, ...), and drv_suspend() asserts the lock.
The assert in rtw_fw_csa_stop() will make that visible.

> > +       mutex_lock(&rtwdev->mutex);
>
> guard(mutex)( &rtwdev->mutex); ?

Done in v5, which follows in a few days.

Thanks,
Mehmet

RE: [PATCH rtw-next v4 2/2] wifi: rtw88: support channel switch in AP mode

[Ping-Ke Shih]

Mehmet Fide <mehmet.fide@gmail.com> wrote:
> On 2026-10-07 Ping-Ke Shih wrote:
> > > -                       rtw_fw_download_rsvd_page(rtwdev);
> > > -                       rtw_send_rsvd_page_h2c(rtwdev);
> > > +                       if (!rtw_fw_csa_active(rtwdev)) {
> >
> > Does it actually happen to AP mode?
> >
> > If it could happen, check the condition by conf->csa_active (like below)?
> 
> Not for the AP interface: mac80211 raises BSS_CHANGED_ASSOC for a
> station interface only. The hunk is for the STA+AP combination the
> driver registers for the 8822C, and it does happen there: with an AP
> on an RTL8822CU counting down from channel 6 to 11, a station interface
> on the same device that had a fresh scan result associated with an AP
> on channel 11 while the countdown ran, so BSS_CHANGED_ASSOC arrived in
> the middle of it. That download would rebuild the reserved page with a
> fresh beacon. conf belongs to the station there, so conf->csa_active
> would say nothing about the AP; that is why the check is on
> rtwdev->csa_vif.

Got it. Add a comment to point out "STA+AP combination" then.

And, agree with your other reply. 

Ping-Ke