From: Christopher Griffin A station vdev is not stopped when mac80211 unassigns its channel context. Since commit c9e4e41e71ff ("wifi: ath12k: move peer delete after vdev stop of station for WCN7850") it is stopped when the BSS peer is removed instead, so that firmware gets the vdev stop before the peer delete. ath12k_mac_op_unassign_vif_chanctx() still clears is_started for it, and nothing else records that the vdev is running in firmware. mac80211 can move a station to another channel context while the BSS peer exists. It does so between authentication and association when it associates with a narrower width than it authenticated with, for example when a station with EHT disabled (wpa_supplicant disable_eht=1) connects to an AP operating EHT at 320 MHz: authentication runs on a 320 MHz channel context, association on a 160 MHz one. The vdev was started for the first context (by ath12k_mac_op_assign_vif_chanctx(), or when the peer was created on hardware with vdev_start_delay), and ath12k_mac_op_assign_vif_chanctx() starts it again for the second one. Firmware gets a second vdev start with no stop in between, and ar->num_started_vdevs counts the vdev twice. The stop on disconnect takes only one of the two back. Since commit 18ae7d0cdd76 ("wifi: ath12k: Avoid CPU busy-wait by handling VDEV_STAT and BCN_STAT"), ath12k_wmi_fw_stats_process() completes a WMI_REQUEST_VDEV_STAT request only after one stats event for each started vdev. With the count too high that never happens: from then on every station statistics request that needs the beacon RSSI waits the full 3 seconds in ath12k_mac_get_fw_stats() with the wiphy lock held, logs "time out while waiting for fw stats done" and reports no signal, until the driver is reloaded. Record in each link vif whether its vdev is running in firmware. ath12k_mac_vdev_start() now stops a vdev that is still running before it starts it, as ath12k_mac_update_vif_chan() already does for a vdev that is not up, and ath12k_mac_vdev_stop() takes a vdev out of num_started_vdevs only if it was running. Firmware then sees start, stop and start for such a connection, and the vdev is counted once. AP vdevs are still stopped on unassign, so in practice only station vdevs take the new stop. This was found by tracing ath12k_mac_vdev_start_restart(), ath12k_mac_vdev_stop() and the mac80211 channel context and station state trace events on a WCN7850 station: the first HE connection (disable_eht=1) to an EHT 320 MHz AP after the driver was loaded started the vdev twice, and the timeouts began right after it. The change and this message were written with Claude, an AI coding assistant. Tested-on: WCN7850 hw2.0 PCI WLAN.HMT.1.1.c7-00108-QCAHMTSWPL_V1.0_V2.0_SILICONZ_UPSTREAM-3 Fixes: c9e4e41e71ff ("wifi: ath12k: move peer delete after vdev stop of station for WCN7850") Signed-off-by: Christopher Griffin --- There are two ways ar->num_started_vdevs ends up too high. The other one, counting a vdev restart as a start, is the one Baochen diagnosed in Mike Garuccio's report [1] and that Birk Skyum's pending patch [2] fixes. This patch fixes only the station case described above and does not touch the line [2] changes, so it applies to ath-next with or without [2]. How it was tested. The station was a Quectel NCM865 (WCN7850) on Ubuntu's 7.3.0-1004-oem kernel, with ath12k built from that kernel's source, connecting on 6 GHz to a RUCKUS R670 that runs EHT at 320 MHz. Without the change, the first connection with disable_eht=1 started the timeouts, one every 3 seconds, until the module was reloaded. With the change, six cycles of EHT, HE and EHT connections (13 in all) and 20 more EHT reconnects gave no timeout, and every station query returned a signal value in about 10 ms. Tracing showed start, stop and start for each HE connection, one start for each EHT connection, and one stop for each disconnect. The build tested on hardware was one change for both cases: this station part, written slightly differently, plus a restart part with the same effect as [2]. With [2] and this patch applied, the code behaves the same way. On ath-next this exact patch is compile-tested only (x86_64, W=1, with and without [2]). [1] https://lore.kernel.org/all/f9d0a06d-b7b0-4e51-8173-c53104c2a096@oss.qualcomm.com/ [2] https://lore.kernel.org/all/20261020213505.31624-1-birk.skyum@pm.me/ The change and this message were written, and the tests run, with Claude, an AI coding assistant. --- drivers/net/wireless/ath/ath12k/core.h | 2 ++ drivers/net/wireless/ath/ath12k/mac.c | 34 +++++++++++++++++++++++++++++++--- 2 files changed, 33 insertions(+), 3 deletions(-) diff --git a/drivers/net/wireless/ath/ath12k/core.h b/drivers/net/wireless/ath/ath12k/core.h index 3a92a4b033599..1c0cdf75a22bb 100644 --- a/drivers/net/wireless/ath/ath12k/core.h +++ b/drivers/net/wireless/ath/ath12k/core.h @@ -330,6 +330,8 @@ struct ath12k_link_vif { bool is_created; bool is_started; + /* vdev is running in firmware and counted in ar->num_started_vdevs */ + bool vdev_started; bool is_up; u8 bssid[ETH_ALEN]; struct cfg80211_bitrate_mask bitrate_mask; diff --git a/drivers/net/wireless/ath/ath12k/mac.c b/drivers/net/wireless/ath/ath12k/mac.c index 4cb41b8b1d3be..65486383ea9f1 100644 --- a/drivers/net/wireless/ath/ath12k/mac.c +++ b/drivers/net/wireless/ath/ath12k/mac.c @@ -1565,9 +1565,12 @@ int ath12k_mac_vdev_stop(struct ath12k_link_vif *arvif) goto err; } - WARN_ON(ar->num_started_vdevs == 0); + if (arvif->vdev_started) { + arvif->vdev_started = false; + WARN_ON(ar->num_started_vdevs == 0); + ar->num_started_vdevs--; + } - ar->num_started_vdevs--; ath12k_dbg(ar->ab, ATH12K_DBG_MAC, "vdev %pM stopped, vdev_id %d\n", ahvif->vif->addr, arvif->vdev_id); @@ -10722,6 +10725,7 @@ int ath12k_mac_vdev_create(struct ath12k *ar, struct ath12k_link_vif *arvif) err: arvif->is_created = false; + arvif->vdev_started = false; arvif->ar = NULL; return ret; } @@ -11032,6 +11036,7 @@ static int ath12k_mac_vdev_delete(struct ath12k *ar, struct ath12k_link_vif *arv /* TODO: recal traffic pause state based on the available vdevs */ arvif->is_created = false; + arvif->vdev_started = false; arvif->ar = NULL; return ret; @@ -11544,7 +11549,30 @@ ath12k_mac_vdev_start_restart(struct ath12k_link_vif *arvif, static int ath12k_mac_vdev_start(struct ath12k_link_vif *arvif, struct ieee80211_chanctx_conf *ctx) { - return ath12k_mac_vdev_start_restart(arvif, ctx, false); + int ret; + + /* A station vdev is stopped when its BSS peer is removed, not when + * its channel context is unassigned. If mac80211 moves the station + * to another channel context while the peer exists, the vdev is + * still running here: stop it before starting it again, so that + * firmware does not get a second start and the vdev is counted once. + */ + if (arvif->vdev_started) { + ath12k_dbg(arvif->ar->ab, ATH12K_DBG_MAC, + "vdev %d still running, stopping it before start\n", + arvif->vdev_id); + ret = ath12k_mac_vdev_stop(arvif); + if (ret) + return ret; + } + + ret = ath12k_mac_vdev_start_restart(arvif, ctx, false); + if (ret) + return ret; + + arvif->vdev_started = true; + + return 0; } static int ath12k_mac_vdev_restart(struct ath12k_link_vif *arvif, --- base-commit: ff4c7359e310744a7abe53cf66d921fa2969eb19 change-id: 20261111-ath12k-sta-vdev-start-once-5680f05f548f Best regards, -- Christopher Griffin