On Fri Apr 18, 2025 at 7:48 PM CEST, Pradeep Kumar Chitrapu wrote:
> Add support to configure maximum NSS in 160 MHz bandwidth.
> Firmware advertises support for handling NSS ratio information
> as a part of service ready ext event using nss_ratio_enabled
> flag. Save this information in ath12k_pdev_cap to calculate
> NSS ratio.
>
> Additionally, reorder the code by moving
> ath12k_peer_assoc_h_phymode() before ath12k_peer_assoc_h_vht()
> to ensure that arg->peer_phymode correctly reflects the bandwidth
> in the max NSS calculation.
>
> Tested-on: QCN9274 hw2.0 PCI WLAN.WBE.1.0.1-00029-QCAHKSWPL_SILICONZ-1
>
> Co-developed-by: P Praneesh <quic_ppranees@xxxxxxxxxxx>
> Signed-off-by: P Praneesh <quic_ppranees@xxxxxxxxxxx>
> Signed-off-by: Pradeep Kumar Chitrapu <quic_pradeepc@xxxxxxxxxxx>
> Acked-by: Jeff Johnson <quic_jjohnson@xxxxxxxxxxx>
[...]
Hello,
With this patch set we sometimes see the following warning shortly followed by a
firmware crash on QCN9274 running WLAN.WBE.1.3.1-00218-QCAHKSWPL_SILICONZ-1
[ 242.416516] ------------[ cut here ]------------
[ 242.423608] WARNING: CPU: 2 PID: 1427 at drivers/net/wireless/ath/ath12k/mac.c:3224 ath12k_peer_assoc_h_phymode+0xd4/0x800 [ath12k]
[ 242.428475] Modules linked in: dvb_usb_af9035 dvb_usb_dib0700 dib0070 dib7000m dibx000_common at24 ath12k qmi_helpers nf_nat_sip nf_conntrack_sip dwc3_qcom nf_nat_pptp nf_conntrack_pptp ipq_lpass
[ 242.444176] CPU: 2 UID: 0 PID: 1427 Comm: memcheck-arm64- Not tainted 6.13.0-02013-gd424b7b9a373-dirty #140
[ 242.466407] Hardware name: XXX (DT)
[ 242.476124] pstate: 60000005 (nZCv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
[ 242.480639] pc : ath12k_peer_assoc_h_phymode+0xd4/0x800 [ath12k]
[ 242.487584] lr : ath12k_peer_assoc_h_phymode+0x540/0x800 [ath12k]
[ 242.493834] sp : ffffffc08588b4b0
[ 242.499820] x29: ffffffc08588b4b0 x28: 0000000000000008 x27: ffffff802781bb60
[ 242.503123] x26: ffffff802cfa9af0 x25: ffffff802781bb60 x24: ffffff8016f38be0
[ 242.510241] x23: ffffff802cfa9fb0 x22: ffffff802781bce0 x21: ffffff802d50dc00
[ 242.517360] x20: ffffff8016f38be0 x19: ffffff802781bce0 x18: ffffffffffffffff
[ 242.524477] x17: 30363120726f6620 x16: 6f666e6920797469 x15: 6c69626170616320
[ 242.531595] x14: 5948502054485620 x13: 0000000000000001 x12: 0000000000000060
[ 242.538714] x11: ffffffc08126c6b0 x10: 0000000000000274 x9 : ffffffc08010468c
[ 242.545832] x8 : 00000000ffffdfff x7 : 00000000ffffe000 x6 : 0000000000000001
[ 242.552949] x5 : 0000000000000000 x4 : 0000000000000000 x3 : 0000000000000000
[ 242.560067] x2 : ffffff80bff6de70 x1 : ffffff80bff640c8 x0 : 0000000000000021
[ 242.567186] Call trace:
[ 242.574299] ath12k_peer_assoc_h_phymode+0xd4/0x800 [ath12k] (P)
[ 242.576560] ath12k_peer_assoc_prepare+0x5d4/0x1608 [ath12k]
[ 242.582809] ath12k_mac_op_sta_state+0x1f4/0x1100 [ath12k]
[ 242.588451] drv_sta_state+0xa0/0x5e0
[ 242.593745] _sta_info_move_state+0x20c/0x4b8
[ 242.597477] sta_info_move_state+0x18/0x28
[ 242.601817] sta_apply_auth_flags.isra.0+0x190/0x1c0
[ 242.605812] sta_apply_parameters+0x240/0x5e0
[ 242.610933] ieee80211_change_station+0x164/0x200
[ 242.615187] nl80211_set_station+0x3bc/0x520
[ 242.619874] genl_family_rcv_msg_doit+0xbc/0x128
[ 242.624215] genl_rcv_msg+0x1c0/0x260
[ 242.628815] netlink_rcv_skb+0x40/0x100
[ 242.632373] genl_rcv+0x3c/0x60
[ 242.636018] netlink_unicast+0x2d8/0x338
[ 242.639144] netlink_sendmsg+0x158/0x398
[ 242.643311] ____sys_sendmsg+0x104/0x290
[ 242.647217] ___sys_sendmsg+0x70/0xa0
[ 242.651123] __sys_sendmsg+0x64/0xb0
[ 242.654682] __arm64_sys_sendmsg+0x28/0x38
[ 242.658329] do_el0_svc+0x70/0x100
[ 242.662234] el0_svc+0x18/0x60
[ 242.665619] el0t_64_sync_handler+0x104/0x130
[ 242.668659] el0t_64_sync+0x154/0x158
[ 242.673086] ---[ end trace 0000000000000000 ]---
It's probably there since v1 of thies series but I just finally got to the
bottom of this now and I've tracked it down to the following hunk:
> @@ -2822,11 +2883,12 @@ static enum wmi_phy_mode ath12k_mac_get_phymode_vht(struct ath12k *ar,
> struct ieee80211_link_sta *link_sta)
> {
> if (link_sta->bandwidth == IEEE80211_STA_RX_BW_160) {
> - if (link_sta->vht_cap.cap & IEEE80211_VHT_CAP_SUPP_CHAN_WIDTH_160MHZ)
> + if (link_sta->vht_cap.cap & (IEEE80211_VHT_CAP_SUPP_CHAN_WIDTH_160MHZ |
> + IEEE80211_VHT_CAP_EXT_NSS_BW_MASK))
> return MODE_11AC_VHT160;
>
> /* not sure if this is a valid case? */
> - return MODE_11AC_VHT160;
> + return MODE_UNKNOWN;
> }
>
> if (link_sta->bandwidth == IEEE80211_STA_RX_BW_80)
Which led me to understand that a sta that supports both VHT 160 and 80+80 ie
IEEE80211_VHT_CAP_SUPP_CHAN_WIDTH_160_80PLUS80MHZ will end up with a phy mode
MODE_UNKNOWN send to the firmware. Which I guess is the reason of the crash.
I understand that VHT 80+80 is not supported by this chipset, but shouldn't we
just allow the sta to connect using VHT 160 right ? Something like this maybe ?
diff --git a/drivers/net/wireless/ath/ath12k/mac.c b/drivers/net/wireless/ath/ath12k/mac.c
index 048d62a48c88..d3a87af076e7 100644
--- a/drivers/net/wireless/ath/ath12k/mac.c
+++ b/drivers/net/wireless/ath/ath12k/mac.c
@@ -3041,7 +3041,7 @@ static enum wmi_phy_mode ath12k_mac_get_phymode_vht(struct ath12k *ar,
struct ieee80211_link_sta *link_sta)
{
if (link_sta->bandwidth == IEEE80211_STA_RX_BW_160) {
- if (link_sta->vht_cap.cap & (IEEE80211_VHT_CAP_SUPP_CHAN_WIDTH_160MHZ |
+ if (link_sta->vht_cap.cap & (IEEE80211_VHT_CAP_SUPP_CHAN_WIDTH_MASK |
IEEE80211_VHT_CAP_EXT_NSS_BW_MASK))
return MODE_11AC_VHT160;
