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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox