diff mbox series

[04/12] wifi: mt76: mt7996: fix incorrect interpretation of EHT MCS caps

Message ID 20240119085708.23592-4-shayne.chen@mediatek.com (mailing list archive)
State Superseded
Delegated to: Felix Fietkau
Headers show
Series [01/12] wifi: mt76: mt7996: check txs format before getting skb by pid | expand

Commit Message

Shayne Chen Jan. 19, 2024, 8:57 a.m. UTC
From: Benjamin Lin <benjamin-jw.lin@mediatek.com>

The EHT MCS map subfield of 20 MHz-Only is not present in the EHT
capability of AP, so STA does not need to parse the subfield.
Moreover, AP should parse the subfield only if STA is 20 MHz-Only, which
can be confirmed by checking supported channel width in HE capability.

Fixes: 92aa2da9fa49 ("wifi: mt76: mt7996: enable EHT support in firmware")
Co-developed-by: Shayne Chen <shayne.chen@mediatek.com>
Signed-off-by: Shayne Chen <shayne.chen@mediatek.com>
Signed-off-by: Benjamin Lin <benjamin-jw.lin@mediatek.com>
---
 drivers/net/wireless/mediatek/mt76/mt7996/mcu.c | 16 ++++++++++++++--
 1 file changed, 14 insertions(+), 2 deletions(-)

Comments

Kalle Valo Jan. 23, 2024, 11:16 a.m. UTC | #1
Shayne Chen <shayne.chen@mediatek.com> writes:

> From: Benjamin Lin <benjamin-jw.lin@mediatek.com>
>
> The EHT MCS map subfield of 20 MHz-Only is not present in the EHT
> capability of AP, so STA does not need to parse the subfield.
> Moreover, AP should parse the subfield only if STA is 20 MHz-Only, which
> can be confirmed by checking supported channel width in HE capability.
>
> Fixes: 92aa2da9fa49 ("wifi: mt76: mt7996: enable EHT support in firmware")
> Co-developed-by: Shayne Chen <shayne.chen@mediatek.com>
> Signed-off-by: Shayne Chen <shayne.chen@mediatek.com>
> Signed-off-by: Benjamin Lin <benjamin-jw.lin@mediatek.com>
> ---
>  drivers/net/wireless/mediatek/mt76/mt7996/mcu.c | 16 ++++++++++++++--
>  1 file changed, 14 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/wireless/mediatek/mt76/mt7996/mcu.c b/drivers/net/wireless/mediatek/mt76/mt7996/mcu.c
> index 3c729b563edc..02d858fdc9fe 100644
> --- a/drivers/net/wireless/mediatek/mt76/mt7996/mcu.c
> +++ b/drivers/net/wireless/mediatek/mt76/mt7996/mcu.c
> @@ -1240,6 +1240,9 @@ mt7996_mcu_sta_he_6g_tlv(struct sk_buff *skb, struct ieee80211_sta *sta)
>  static void
>  mt7996_mcu_sta_eht_tlv(struct sk_buff *skb, struct ieee80211_sta *sta)
>  {
> +	struct mt7996_sta *msta = (struct mt7996_sta *)sta->drv_priv;
> +	struct ieee80211_vif *vif = container_of((void *)msta->vif,
> +						 struct ieee80211_vif, drv_priv);

The void pointer cast looks to be unnecessary. This is nitpicking but I
really hate casts.
Felix Fietkau Jan. 23, 2024, 2:47 p.m. UTC | #2
On 23.01.24 12:16, Kalle Valo wrote:
> Shayne Chen <shayne.chen@mediatek.com> writes:
> 
>> From: Benjamin Lin <benjamin-jw.lin@mediatek.com>
>>
>> The EHT MCS map subfield of 20 MHz-Only is not present in the EHT
>> capability of AP, so STA does not need to parse the subfield.
>> Moreover, AP should parse the subfield only if STA is 20 MHz-Only, which
>> can be confirmed by checking supported channel width in HE capability.
>>
>> Fixes: 92aa2da9fa49 ("wifi: mt76: mt7996: enable EHT support in firmware")
>> Co-developed-by: Shayne Chen <shayne.chen@mediatek.com>
>> Signed-off-by: Shayne Chen <shayne.chen@mediatek.com>
>> Signed-off-by: Benjamin Lin <benjamin-jw.lin@mediatek.com>
>> ---
>>  drivers/net/wireless/mediatek/mt76/mt7996/mcu.c | 16 ++++++++++++++--
>>  1 file changed, 14 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/net/wireless/mediatek/mt76/mt7996/mcu.c b/drivers/net/wireless/mediatek/mt76/mt7996/mcu.c
>> index 3c729b563edc..02d858fdc9fe 100644
>> --- a/drivers/net/wireless/mediatek/mt76/mt7996/mcu.c
>> +++ b/drivers/net/wireless/mediatek/mt76/mt7996/mcu.c
>> @@ -1240,6 +1240,9 @@ mt7996_mcu_sta_he_6g_tlv(struct sk_buff *skb, struct ieee80211_sta *sta)
>>  static void
>>  mt7996_mcu_sta_eht_tlv(struct sk_buff *skb, struct ieee80211_sta *sta)
>>  {
>> +	struct mt7996_sta *msta = (struct mt7996_sta *)sta->drv_priv;
>> +	struct ieee80211_vif *vif = container_of((void *)msta->vif,
>> +						 struct ieee80211_vif, drv_priv);
> 
> The void pointer cast looks to be unnecessary. This is nitpicking but I
> really hate casts.

It is not unnecessary - removing it results in a compile error.

- Felix
diff mbox series

Patch

diff --git a/drivers/net/wireless/mediatek/mt76/mt7996/mcu.c b/drivers/net/wireless/mediatek/mt76/mt7996/mcu.c
index 3c729b563edc..02d858fdc9fe 100644
--- a/drivers/net/wireless/mediatek/mt76/mt7996/mcu.c
+++ b/drivers/net/wireless/mediatek/mt76/mt7996/mcu.c
@@ -1240,6 +1240,9 @@  mt7996_mcu_sta_he_6g_tlv(struct sk_buff *skb, struct ieee80211_sta *sta)
 static void
 mt7996_mcu_sta_eht_tlv(struct sk_buff *skb, struct ieee80211_sta *sta)
 {
+	struct mt7996_sta *msta = (struct mt7996_sta *)sta->drv_priv;
+	struct ieee80211_vif *vif = container_of((void *)msta->vif,
+						 struct ieee80211_vif, drv_priv);
 	struct ieee80211_eht_mcs_nss_supp *mcs_map;
 	struct ieee80211_eht_cap_elem_fixed *elem;
 	struct sta_rec_eht *eht;
@@ -1259,8 +1262,17 @@  mt7996_mcu_sta_eht_tlv(struct sk_buff *skb, struct ieee80211_sta *sta)
 	eht->phy_cap = cpu_to_le64(*(u64 *)elem->phy_cap_info);
 	eht->phy_cap_ext = cpu_to_le64(elem->phy_cap_info[8]);
 
-	if (sta->deflink.bandwidth == IEEE80211_STA_RX_BW_20)
-		memcpy(eht->mcs_map_bw20, &mcs_map->only_20mhz, sizeof(eht->mcs_map_bw20));
+	if (vif->type != NL80211_IFTYPE_STATION &&
+	    (sta->deflink.he_cap.he_cap_elem.phy_cap_info[0] &
+	     (IEEE80211_HE_PHY_CAP0_CHANNEL_WIDTH_SET_40MHZ_IN_2G |
+	      IEEE80211_HE_PHY_CAP0_CHANNEL_WIDTH_SET_40MHZ_80MHZ_IN_5G |
+	      IEEE80211_HE_PHY_CAP0_CHANNEL_WIDTH_SET_160MHZ_IN_5G |
+	      IEEE80211_HE_PHY_CAP0_CHANNEL_WIDTH_SET_80PLUS80_MHZ_IN_5G)) == 0) {
+		memcpy(eht->mcs_map_bw20, &mcs_map->only_20mhz,
+		       sizeof(eht->mcs_map_bw20));
+		return;
+	}
+
 	memcpy(eht->mcs_map_bw80, &mcs_map->bw._80, sizeof(eht->mcs_map_bw80));
 	memcpy(eht->mcs_map_bw160, &mcs_map->bw._160, sizeof(eht->mcs_map_bw160));
 	memcpy(eht->mcs_map_bw320, &mcs_map->bw._320, sizeof(eht->mcs_map_bw320));