Re: [PATCH mt76 08/12] wifi: mt76: mt7996: use correct link_id when filling TXD and TXP

[Lorenzo Bianconi <lorenzo@kernel.org>]

[-- Attachment #1: Type: text/plain, Size: 3855 bytes --]

> Obtain the correct link ID and, if needed, switch to the corresponding
> wcid before populating the TX descriptor and TX payload.
> 
> Rules for link id:
> - For QoS data from MLD peers (excluding EAPOL), select the primary or
>   secondary wcid based on whether the TID is odd or even to meet FW/HW
>   requirements
> - For other packets, use IEEE80211_TX_CTRL_MLO_LINK if specified
>   (such as multicast and broadcast packets)

Hi Shayne,

thx for the fix. I guess it is mostly fine, just few comments inline. Fixing
them:

Acked-by: Lorenzo Bianconi <lorenzo@kernel.org>

Regards,
Lorenzo

> 
> Signed-off-by: Shayne Chen <shayne.chen@mediatek.com>
> ---
>  .../net/wireless/mediatek/mt76/mt7996/mac.c   | 34 ++++++++++++++++---
>  .../net/wireless/mediatek/mt76/mt7996/main.c  |  6 ++++
>  .../net/wireless/mediatek/mt76/mt7996/mcu.c   |  4 +--
>  .../wireless/mediatek/mt76/mt7996/mt7996.h    |  1 +
>  4 files changed, 39 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/net/wireless/mediatek/mt76/mt7996/mac.c b/drivers/net/wireless/mediatek/mt76/mt7996/mac.c
> index 7d5f9d9b3b62..579084cbb30f 100644
> --- a/drivers/net/wireless/mediatek/mt76/mt7996/mac.c
> +++ b/drivers/net/wireless/mediatek/mt76/mt7996/mac.c
> @@ -1035,15 +1035,20 @@ int mt7996_tx_prepare_skb(struct mt76_dev *mdev, void *txwi_ptr,
>  			  struct ieee80211_sta *sta,
>  			  struct mt76_tx_info *tx_info)
>  {

[...]

> diff --git a/drivers/net/wireless/mediatek/mt76/mt7996/main.c b/drivers/net/wireless/mediatek/mt76/mt7996/main.c
> index c26f8f49ce44..3958688d622b 100644
> --- a/drivers/net/wireless/mediatek/mt76/mt7996/main.c
> +++ b/drivers/net/wireless/mediatek/mt76/mt7996/main.c
> @@ -963,6 +963,7 @@ mt7996_mac_sta_init_link(struct mt7996_dev *dev,
>  
>  		msta_link = &msta->deflink;
>  		msta->deflink_id = link_id;
> +		msta->seclink_id = msta->deflink_id;
>  
>  		for (i = 0; i < ARRAY_SIZE(sta->txq); i++) {
>  			struct mt76_txq *mtxq;
> @@ -977,6 +978,11 @@ mt7996_mac_sta_init_link(struct mt7996_dev *dev,
>  		msta_link = kzalloc(sizeof(*msta_link), GFP_KERNEL);
>  		if (!msta_link)
>  			return -ENOMEM;
> +
> +		if (msta->seclink_id == msta->deflink_id &&
> +		    (sta->valid_links & ~(BIT(msta->deflink_id))))
> +			msta->seclink_id = __ffs(sta->valid_links &
> +						 ~(BIT(msta->deflink_id)));
nit: you do need brackets around BIT().

I think we are missing seclink_id management when the corresponding link is
removed, right?

>  	}
>  
>  	INIT_LIST_HEAD(&msta_link->rc_list);
> diff --git a/drivers/net/wireless/mediatek/mt76/mt7996/mcu.c b/drivers/net/wireless/mediatek/mt76/mt7996/mcu.c
> index f1892aaf6a91..2bb98df317a6 100644
> --- a/drivers/net/wireless/mediatek/mt76/mt7996/mcu.c
> +++ b/drivers/net/wireless/mediatek/mt76/mt7996/mcu.c
> @@ -2394,8 +2394,8 @@ mt7996_mcu_sta_mld_setup_tlv(struct mt7996_dev *dev, struct sk_buff *skb,
>  	mld_setup->primary_id = cpu_to_le16(msta_link->wcid.idx);
>  
>  	if (nlinks > 1) {
> -		link_id = __ffs(sta->valid_links & ~BIT(msta->deflink_id));
> -		msta_link = mt76_dereference(msta->link[link_id], &dev->mt76);
> +		msta_link = mt76_dereference(msta->link[msta->seclink_id],
> +					     &dev->mt76);
>  		if (!msta_link)
>  			return;
>  	}
> diff --git a/drivers/net/wireless/mediatek/mt76/mt7996/mt7996.h b/drivers/net/wireless/mediatek/mt76/mt7996/mt7996.h
> index 65eeb37ab8c7..c47820699ec8 100644
> --- a/drivers/net/wireless/mediatek/mt76/mt7996/mt7996.h
> +++ b/drivers/net/wireless/mediatek/mt76/mt7996/mt7996.h
> @@ -243,6 +243,7 @@ struct mt7996_sta {
>  	struct mt7996_sta_link deflink; /* must be first */
>  	struct mt7996_sta_link __rcu *link[IEEE80211_MLD_MAX_NUM_LINKS];
>  	u8 deflink_id;
> +	u8 seclink_id;
>  
>  	struct mt7996_vif *vif;
>  };
> -- 
> 2.51.0
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]