From: Rameshkumar Sundaram <rameshkumar.sundaram@oss.qualcomm.com>
To: Matthew Leach <matthew.leach@collabora.com>,
Jeff Johnson <jjohnson@kernel.org>,
Vasanthakumar Thiagarajan <quic_vthiagar@quicinc.com>,
Baochen Qiang <quic_bqiang@quicinc.com>
Cc: Kalle Valo <kvalo@kernel.org>,
Pradeep Kumar Chitrapu <quic_pradeepc@quicinc.com>,
linux-wireless@vger.kernel.org, ath12k@lists.infradead.org,
linux-kernel@vger.kernel.org, kernel@collabora.com,
Rameshkumar Sundaram <quic_ramess@quicinc.com>,
Jeff Johnson <jeff.johnson@oss.qualcomm.com>
Subject: Re: [PATCH v2] wifi: ath12k: fix survey indexing across bands
Date: Thu, 2 Jul 2026 23:36:13 +0530 [thread overview]
Message-ID: <8dd4aceb-48e8-45cb-abe8-af7ece2285ae@oss.qualcomm.com> (raw)
In-Reply-To: <20260702-ath12-survey-band-fix-v2-1-75b5bdf72a08@collabora.com>
On 7/2/2026 4:20 PM, Matthew Leach wrote:
> When running 'iw dev wlan0 survey dump' the values for the channel busy
> time have the same sequence across bands. This is caused by indexing
> into the ath12k survey array using a band-local index rather than the
> global index passed by mac80211. This results in surveys for 5 GHz and 6
> GHz channels returning values from 2.4 GHz slots, making the survey
> unusable on those bands. Further, there are redundant survey slots for
> multi-radio/single-phy instances.
>
> Fix by moving the survey data into ath12k_hw so multiple radios under a
> single wiphy share one table, and index into it using the global
> mac80211 index. A new spinlock in ath12k_hw serialises access to the
> survey array, which is now shared across all radios under a single hw.
>
> Band busy-times Before this fix:
>
> 2.4 GHz: 9, 2, 2, 2, 4, 2, 10, 16, 4, 12, 5
> 5 GHz: 9, 2, 2, 2, 4, 2, 10, 16, 4, 12, 5
> 6 GHz: 9, 2, 2, 2, 4, 2, 10, 16, 4, 12, 5
>
> After this fix, times are independent:
>
> 2.4 GHz: 23, 5, 5, 12, 2, 12, 26, 5, 3, 1, 27
> 5 GHz: 30, 40, 29, 27, 118, 118, 112, 120, 11, 11, 11
> 6 GHz: 1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1
>
> Tested-on: wcn7850 hw2.0 PCI WLAN.IOE_HMT.1.1-00018-QCAHMTSWPL_V1.0_V2.0_SILICONZ-1
>
> Fixes: 4f242b1d6996 ("wifi: ath12k: support get_survey mac op for single wiphy")
> Signed-off-by: Matthew Leach <matthew.leach@collabora.com>
> ---
> Changes in v2:
> - Move survey[] from ath12k to ath12k_hw so multi-radio single-wiphy
> setups share one global table (suggested by Rameshkumar Sundaram).
> - Drop the ar->mac.sbands[] filter in freq_to_idx() so the WMI event
> handlers use the same global index
> - Add ah->survey_lock to serialise access to the shared survey table
> - Update Fixes: tag to the correct commit
> - Link to v1: https://patch.msgid.link/20260617-ath12-survey-band-fix-v1-1-e7d9555bb478@collabora.com
>
> To: Jeff Johnson <jjohnson@kernel.org>
> To: Sriram R <quic_srirrama@quicinc.com>
> To: Kalle Valo <kvalo@kernel.org>
> To: Rameshkumar Sundaram <quic_ramess@quicinc.com>
> Cc: Jeff Johnson <jeff.johnson@oss.qualcomm.com>
> Cc: linux-wireless@vger.kernel.org
> Cc: ath12k@lists.infradead.org
> Cc: linux-kernel@vger.kernel.org
> ---
> drivers/net/wireless/ath/ath12k/core.h | 6 ++++-
> drivers/net/wireless/ath/ath12k/mac.c | 33 ++++++++++++++-------------
> drivers/net/wireless/ath/ath12k/wmi.c | 41 ++++++++++++++++++----------------
> 3 files changed, 45 insertions(+), 35 deletions(-)
>
> diff --git a/drivers/net/wireless/ath/ath12k/core.h b/drivers/net/wireless/ath/ath12k/core.h
> index 8be435535a4e..6ce2f7b3fa50 100644
> --- a/drivers/net/wireless/ath/ath12k/core.h
> +++ b/drivers/net/wireless/ath/ath12k/core.h
> @@ -712,7 +712,6 @@ struct ath12k {
> * avoid reporting garbage data.
> */
> bool ch_info_can_report_survey;
> - struct survey_info survey[ATH12K_NUM_CHANS];
> struct completion bss_survey_done;
>
> struct work_struct regd_update_work;
> @@ -774,6 +773,11 @@ struct ath12k_hw {
> */
> struct mutex hw_mutex;
> enum ath12k_hw_state state;
> +
> + /* protects survey[] shared across radios of this hw. */
> + spinlock_t survey_lock;
> + struct survey_info survey[ATH12K_NUM_CHANS];
> +
> bool regd_updated;
> bool use_6ghz_regd;
>
> diff --git a/drivers/net/wireless/ath/ath12k/mac.c b/drivers/net/wireless/ath/ath12k/mac.c
> index 2cff9485c95a..daf9bc8722df 100644
> --- a/drivers/net/wireless/ath/ath12k/mac.c
> +++ b/drivers/net/wireless/ath/ath12k/mac.c
> @@ -13348,52 +13348,54 @@ ath12k_mac_update_bss_chan_survey(struct ath12k *ar,
> int ath12k_mac_op_get_survey(struct ieee80211_hw *hw, int idx,
> struct survey_info *survey)
> {
> + struct ath12k_hw *ah = hw->priv;
> struct ath12k *ar;
> struct ieee80211_supported_band *sband;
> - struct survey_info *ar_survey;
> + struct survey_info *ah_survey;
> + int sband_idx = idx;
>
> lockdep_assert_wiphy(hw->wiphy);
>
> - if (idx >= ATH12K_NUM_CHANS)
> + if (sband_idx >= ATH12K_NUM_CHANS)
> return -ENOENT;
>
> sband = hw->wiphy->bands[NL80211_BAND_2GHZ];
> - if (sband && idx >= sband->n_channels) {
> - idx -= sband->n_channels;
> + if (sband && sband_idx >= sband->n_channels) {
> + sband_idx -= sband->n_channels;
> sband = NULL;
> }
>
> if (!sband)
> sband = hw->wiphy->bands[NL80211_BAND_5GHZ];
> - if (sband && idx >= sband->n_channels) {
> - idx -= sband->n_channels;
> + if (sband && sband_idx >= sband->n_channels) {
> + sband_idx -= sband->n_channels;
> sband = NULL;
> }
>
> if (!sband)
> sband = hw->wiphy->bands[NL80211_BAND_6GHZ];
>
> - if (!sband || idx >= sband->n_channels)
> + if (!sband || sband_idx >= sband->n_channels)
> return -ENOENT;
>
> - ar = ath12k_mac_get_ar_by_chan(hw, &sband->channels[idx]);
> + ar = ath12k_mac_get_ar_by_chan(hw, &sband->channels[sband_idx]);
> if (!ar) {
> - if (sband->channels[idx].flags & IEEE80211_CHAN_DISABLED) {
> + if (sband->channels[sband_idx].flags & IEEE80211_CHAN_DISABLED) {
> memset(survey, 0, sizeof(*survey));
> return 0;
> }
> return -ENOENT;
> }
>
> - ar_survey = &ar->survey[idx];
> + ah_survey = &ah->survey[idx];
>
> - ath12k_mac_update_bss_chan_survey(ar, &sband->channels[idx]);
> + ath12k_mac_update_bss_chan_survey(ar, &sband->channels[sband_idx]);
>
> - spin_lock_bh(&ar->data_lock);
> - memcpy(survey, ar_survey, sizeof(*survey));
> - spin_unlock_bh(&ar->data_lock);
> + scoped_guard(spinlock_bh, &ah->survey_lock) {
> + memcpy(survey, ah_survey, sizeof(*survey));
> + }
>
> - survey->channel = &sband->channels[idx];
> + survey->channel = &sband->channels[sband_idx];
>
> if (ar->rx_channel == survey->channel)
> survey->filled |= SURVEY_INFO_IN_USE;
> @@ -15055,6 +15057,7 @@ static struct ath12k_hw *ath12k_mac_hw_allocate(struct ath12k_hw_group *ag,
>
> mutex_init(&ah->hw_mutex);
>
> + spin_lock_init(&ah->survey_lock);
> spin_lock_init(&ah->dp_hw.peer_lock);
> INIT_LIST_HEAD(&ah->dp_hw.dp_peers_list);
>
> diff --git a/drivers/net/wireless/ath/ath12k/wmi.c b/drivers/net/wireless/ath/ath12k/wmi.c
> index b5e904a55aea..aa70d2a61d38 100644
> --- a/drivers/net/wireless/ath/ath12k/wmi.c
> +++ b/drivers/net/wireless/ath/ath12k/wmi.c
> @@ -6617,16 +6617,12 @@ static int ath12k_pull_roam_ev(struct ath12k_base *ab, struct sk_buff *skb,
> return 0;
> }
>
> -static int freq_to_idx(struct ath12k *ar, int freq)
> +static int freq_to_idx(struct ieee80211_hw *hw, int freq)
> {
> struct ieee80211_supported_band *sband;
> - struct ieee80211_hw *hw = ath12k_ar_to_hw(ar);
> int band, ch, idx = 0;
>
> for (band = NL80211_BAND_2GHZ; band < NUM_NL80211_BANDS; band++) {
> - if (!ar->mac.sbands[band].channels)
> - continue;
> -
> sband = hw->wiphy->bands[band];
> if (!sband)
> continue;
> @@ -7507,6 +7503,7 @@ static void ath12k_chan_info_event(struct ath12k_base *ab, struct sk_buff *skb)
> {
> struct wmi_chan_info_event ch_info_ev = {};
> struct ath12k *ar;
> + struct ath12k_hw *ah;
> struct survey_info *survey;
> int idx;
> /* HW channel counters frequency value in hertz */
> @@ -7538,6 +7535,7 @@ static void ath12k_chan_info_event(struct ath12k_base *ab, struct sk_buff *skb)
> return;
> }
> spin_lock_bh(&ar->data_lock);
> + ah = ath12k_ar_to_ah(ar);
>
> switch (ar->scan.state) {
> case ATH12K_SCAN_IDLE:
> @@ -7549,8 +7547,8 @@ static void ath12k_chan_info_event(struct ath12k_base *ab, struct sk_buff *skb)
> break;
> }
>
> - idx = freq_to_idx(ar, le32_to_cpu(ch_info_ev.freq));
> - if (idx >= ARRAY_SIZE(ar->survey)) {
> + idx = freq_to_idx(ath12k_ar_to_hw(ar), le32_to_cpu(ch_info_ev.freq));
> + if (idx >= ARRAY_SIZE(ah->survey)) {
> ath12k_warn(ab, "chan info: invalid frequency %d (idx %d out of bounds)\n",
> ch_info_ev.freq, idx);
> goto exit;
> @@ -7563,14 +7561,16 @@ static void ath12k_chan_info_event(struct ath12k_base *ab, struct sk_buff *skb)
> cc_freq_hz = (le32_to_cpu(ch_info_ev.mac_clk_mhz) * 1000);
>
> if (ch_info_ev.cmd_flags == WMI_CHAN_INFO_START_RESP) {
> - survey = &ar->survey[idx];
> - memset(survey, 0, sizeof(*survey));
> - survey->noise = le32_to_cpu(ch_info_ev.noise_floor);
> - survey->filled = SURVEY_INFO_NOISE_DBM | SURVEY_INFO_TIME |
> + scoped_guard(spinlock_bh, &ah->survey_lock) {
> + survey = &ah->survey[idx];
> + memset(survey, 0, sizeof(*survey));
> + survey->noise = le32_to_cpu(ch_info_ev.noise_floor);
> + survey->filled = SURVEY_INFO_NOISE_DBM | SURVEY_INFO_TIME |
> SURVEY_INFO_TIME_BUSY;
> - survey->time = div_u64(le32_to_cpu(ch_info_ev.cycle_count), cc_freq_hz);
> - survey->time_busy = div_u64(le32_to_cpu(ch_info_ev.rx_clear_count),
> - cc_freq_hz);
> + survey->time = div_u64(le32_to_cpu(ch_info_ev.cycle_count), cc_freq_hz);
> + survey->time_busy = div_u64(le32_to_cpu(ch_info_ev.rx_clear_count),
> + cc_freq_hz);
> + }
> }
> exit:
> spin_unlock_bh(&ar->data_lock);
> @@ -7583,6 +7583,7 @@ ath12k_pdev_bss_chan_info_event(struct ath12k_base *ab, struct sk_buff *skb)
> struct wmi_pdev_bss_chan_info_event bss_ch_info_ev = {};
> struct survey_info *survey;
> struct ath12k *ar;
> + struct ath12k_hw *ah;
> u32 cc_freq_hz = ab->cc_freq_hz;
> u64 busy, total, tx, rx, rx_bss;
> int idx;
> @@ -7623,15 +7624,18 @@ ath12k_pdev_bss_chan_info_event(struct ath12k_base *ab, struct sk_buff *skb)
> return;
> }
>
> - spin_lock_bh(&ar->data_lock);
> - idx = freq_to_idx(ar, le32_to_cpu(bss_ch_info_ev.freq));
> - if (idx >= ARRAY_SIZE(ar->survey)) {
> + ah = ath12k_ar_to_ah(ar);
> +
> + guard(spinlock_bh)(&ah->survey_lock);
> +
> + idx = freq_to_idx(ath12k_ar_to_hw(ar), le32_to_cpu(bss_ch_info_ev.freq));
> + if (idx >= ARRAY_SIZE(ah->survey)) {
> ath12k_warn(ab, "bss chan info: invalid frequency %d (idx %d out of bounds)\n",
> bss_ch_info_ev.freq, idx);
> goto exit;
> }
>
the scope of survey_lock should start here and ..
> - survey = &ar->survey[idx];
> + survey = &ah->survey[idx];
>
> survey->noise = le32_to_cpu(bss_ch_info_ev.noise_floor);
> survey->time = div_u64(total, cc_freq_hz);
> @@ -7644,7 +7648,6 @@ ath12k_pdev_bss_chan_info_event(struct ath12k_base *ab, struct sk_buff *skb)
> SURVEY_INFO_TIME_RX |
> SURVEY_INFO_TIME_TX);
> exit:
> - spin_unlock_bh(&ar->data_lock);
end here, may be a scoped guard here as well ?
> complete(&ar->bss_survey_done);
>
> rcu_read_unlock();
>
> ---
> base-commit: 8cd9520d35a6c38db6567e97dd93b1f11f185dc6
> change-id: 20260617-ath12-survey-band-fix-4b5e78579379
>
> Best regards,
> --
> Matt
>
Rest of the changes looks fine to me.
next prev parent reply other threads:[~2026-07-02 18:06 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-07-02 10:50 [PATCH v2] wifi: ath12k: fix survey indexing across bands Matthew Leach
2026-07-02 18:06 ` Rameshkumar Sundaram [this message]
2026-07-02 21:14 ` Nicolas Escande
2026-07-03 7:06 ` Matthew Leach
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=8dd4aceb-48e8-45cb-abe8-af7ece2285ae@oss.qualcomm.com \
--to=rameshkumar.sundaram@oss.qualcomm.com \
--cc=ath12k@lists.infradead.org \
--cc=jeff.johnson@oss.qualcomm.com \
--cc=jjohnson@kernel.org \
--cc=kernel@collabora.com \
--cc=kvalo@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-wireless@vger.kernel.org \
--cc=matthew.leach@collabora.com \
--cc=quic_bqiang@quicinc.com \
--cc=quic_pradeepc@quicinc.com \
--cc=quic_ramess@quicinc.com \
--cc=quic_vthiagar@quicinc.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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.