Re: [PATCH V13 8/9] wifi: ath12k: add support for 160 MHz bandwidth

["Nicolas Escande" <nico.escande@gmail.com>]

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@quicinc.com>
> Signed-off-by: P Praneesh <quic_ppranees@quicinc.com>
> Signed-off-by: Pradeep Kumar Chitrapu <quic_pradeepc@quicinc.com>
> Acked-by: Jeff Johnson <quic_jjohnson@quicinc.com>
[...]

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;