All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Nicolas Escande" <nico.escande@gmail.com>
To: "Rameshkumar Sundaram" <rameshkumar.sundaram@oss.qualcomm.com>,
	"Nicolas Escande" <nico.escande@gmail.com>,
	<ath12k@lists.infradead.org>
Cc: <linux-wireless@vger.kernel.org>
Subject: Re: [PATCH ath-next] wifi: ath12k: avoid setting 320MHZ support on non 6GHz band
Date: Mon, 01 Jun 2026 19:08:48 +0200	[thread overview]
Message-ID: <DIXVMW7QYLL5.M5EQ7YQQ0DTM@gmail.com> (raw)
In-Reply-To: <a4ee39fd-f1d6-4b84-832c-c4eb0166476d@oss.qualcomm.com>

On Thu May 28, 2026 at 9:56 AM CEST, Rameshkumar Sundaram wrote:
> On 1/23/2026 8:12 PM, Nicolas Escande wrote:
>> On a split phy qcn9274 (2.4GHz + 5GHz low), "iw phy" reports 320MHz
>> realated features on the 5GHz band while it should not:
>> 
>>      Wiphy phy1
>>      [...]
>>          Band 2:
>>      [...]
>>              EHT Iftypes: managed
>>      [...]
>>                  EHT PHY Capabilities: (0xe2ffdbe018778000):
>>                      320MHz in 6GHz Supported
>>      [...]
>>                      Beamformee SS (320MHz): 7
>>      [...]
>>                      Number Of Sounding Dimensions (320MHz): 3
>>      [...]
>>                  EHT MCS/NSS: (0x22222222222222222200000000):
>> 
>> This is also reflected in the beacons sent by a mesh interface started on
>> that band. They erroneously advertise 320MHZ support too.
>> 
>> This should not happen as the spec at section 9.4.2.323.3 says we should
>> not set the 320MHz related fields when not operating on a 6GHz band.
>> For example it says about Bit 0 "Support For 320 MHz In 6 GHz"
>> 
>>    "Reserved if the EHT Capabilities element is indicating capabilities for
>>     the 2.4 GHz or 5 GHz bands."
>> 
>> Fix this by clearing the related bits when converting from WMI eht phy
>> capabilities to mac80211 phy capabilities, for bands other than 6GHz.
>> 
>> Tested-on: QCN9274 hw2.0 PCI WLAN.WBE.1.3.1-00218-QCAHKSWPL_SILICONZ-1
>> 
>> Signed-off-by: Nicolas Escande <nico.escande@gmail.com>
>> ---
>>   drivers/net/wireless/ath/ath12k/wmi.c | 9 ++++++++-
>>   1 file changed, 8 insertions(+), 1 deletion(-)
>> 
>> diff --git a/drivers/net/wireless/ath/ath12k/wmi.c b/drivers/net/wireless/ath/ath12k/wmi.c
>> index 84c29e4896a4..14947fdb9813 100644
>> --- a/drivers/net/wireless/ath/ath12k/wmi.c
>> +++ b/drivers/net/wireless/ath/ath12k/wmi.c
>> @@ -4888,6 +4888,7 @@ static void ath12k_wmi_eht_caps_parse(struct ath12k_pdev *pdev, u32 band,
>>   				       __le32 cap_info_internal)
>>   {
>>   	struct ath12k_band_cap *cap_band = &pdev->cap.band[band];
>> +	u8 *phy_cap = (u8 *)&cap_band->eht_cap_phy_info[0];
>>   	u32 support_320mhz;
>>   	u8 i;
>>   
>> @@ -4901,8 +4902,14 @@ static void ath12k_wmi_eht_caps_parse(struct ath12k_pdev *pdev, u32 band,
>>   	for (i = 0; i < WMI_MAX_EHTCAP_PHY_SIZE; i++)
>>   		cap_band->eht_cap_phy_info[i] = le32_to_cpu(cap_phy_info[i]);
>>   
>> -	if (band == NL80211_BAND_6GHZ)
>> +	if (band == NL80211_BAND_6GHZ) {
>>   		cap_band->eht_cap_phy_info[0] |= support_320mhz;
>> +	} else {
>> +		phy_cap[0] &= ~IEEE80211_EHT_PHY_CAP0_320MHZ_IN_6GHZ;
>> +		phy_cap[1] &= ~IEEE80211_EHT_PHY_CAP1_BEAMFORMEE_SS_320MHZ_MASK;
>> +		phy_cap[2] &= ~IEEE80211_EHT_PHY_CAP2_SOUNDING_DIM_320MHZ_MASK;
>
> This field is split across PHY capability byte 2 and byte 3, so should
> IEEE80211_EHT_PHY_CAP3_SOUNDING_DIM_320MHZ_MASK be cleared as well ?

Indeed

>
>
>> +		phy_cap[6] &= ~IEEE80211_EHT_PHY_CAP6_MCS15_SUPP_320MHZ;
>> +	}
>>   
>>   	cap_band->eht_mcs_20_only = le32_to_cpu(supp_mcs[0]);
>>   	cap_band->eht_mcs_80 = le32_to_cpu(supp_mcs[1]);
>
>
> Since you said "On a split phy qcn9274 (2.4GHz + 5GHz low)" i wonder how 
> firmware set 6GHz capability bits in this case. That said, the approach 

I suspect that the firmware sets the same features for mode 'a', regardless of
the actual frequency range supported by the device. I've also seen this on a
5G + 6G split device too.

> looks fine to me, although I would prefer to clear the remaining related 
> bits as well:
>
>    IEEE80211_EHT_PHY_CAP3_SOUNDING_DIM_320MHZ_MASK
>    IEEE80211_EHT_PHY_CAP6_MCS15_SUPP_320MHZ
>    IEEE80211_EHT_PHY_CAP6_EHT_DUP_6GHZ_SUPP
>    IEEE80211_EHT_PHY_CAP7_NON_OFDMA_UL_MU_MIMO_320MHZ
>    IEEE80211_EHT_PHY_CAP7_MU_BEAMFORMER_320MHZ

Yes, I cleared the ones I used in my middleware but I really don't mind
clearing all the 6GHz/320MHz related bits I can find.

Thanks for the review, I'll spin a v2


      reply	other threads:[~2026-06-01 17:08 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-01-23 14:42 [PATCH ath-next] wifi: ath12k: avoid setting 320MHZ support on non 6GHz band Nicolas Escande
2026-01-23 18:33 ` Pablo MARTIN-GOMEZ
2026-01-23 19:08   ` Johannes Berg
2026-01-23 19:21     ` Pablo MARTIN-GOMEZ
2026-01-23 19:29       ` Johannes Berg
2026-01-26 10:11         ` Nicolas Escande
2026-01-26 10:36           ` Johannes Berg
2026-05-27  8:21 ` Nicolas Escande
2026-05-28  7:56 ` Rameshkumar Sundaram
2026-06-01 17:08   ` Nicolas Escande [this message]

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=DIXVMW7QYLL5.M5EQ7YQQ0DTM@gmail.com \
    --to=nico.escande@gmail.com \
    --cc=ath12k@lists.infradead.org \
    --cc=linux-wireless@vger.kernel.org \
    --cc=rameshkumar.sundaram@oss.qualcomm.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.