RE: [PATCH v9 RESEND] wifi: mt76: mt7915: add wds support when wed is enabled

[Ping-Ke Shih <pkshih@realtek.com>]

Shengyu Qu <wiagn233@outlook.com> wrote:
> Hello,
> 
> I modified the patch as you two adviced. If you think it's ok, please
> tell me and I'll send a v10 patch.
> 
> Modified patch is attached below.

Since you want people review whole patch, why not just sending v10?

> 
> Best regards,
> Shengyu
> 
> ---
>   drivers/net/wireless/mediatek/mt76/mt76.h     | 15 ++++++
>   .../net/wireless/mediatek/mt76/mt7915/main.c  | 53 +++++++++++++++++--
>   .../net/wireless/mediatek/mt76/mt7915/mcu.c   | 18 +++++--
>   .../net/wireless/mediatek/mt76/mt7915/mcu.h   |  1 +
>   drivers/net/wireless/mediatek/mt76/util.c     | 37 +++++++++++--
>   drivers/net/wireless/mediatek/mt76/util.h     |  2 +-
>   6 files changed, 115 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/net/wireless/mediatek/mt76/mt76.h b/drivers/net/wireless/mediatek/mt76/mt76.h
> index 132148f7b107..bd943b8b20bc 100644
> --- a/drivers/net/wireless/mediatek/mt76/mt76.h
> +++ b/drivers/net/wireless/mediatek/mt76/mt76.h
> @@ -28,6 +28,9 @@
> 
>   #define MT76_TOKEN_FREE_THR	64
> 
> +#define MT76_WED_WDS_MIN    256
> +#define MT76_WED_WDS_MAX    272
> +
>   #define MT_QFLAG_WED_RING	GENMASK(1, 0)
>   #define MT_QFLAG_WED_TYPE	GENMASK(4, 2)
>   #define MT_QFLAG_WED		BIT(5)
> @@ -73,6 +76,12 @@ enum mt76_wed_type {
>   	MT76_WED_RRO_Q_IND,
>   };
> 
> +enum mt76_wed_state {
> +	MT76_WED_DEFAULT,
> +	MT76_WED_ACTIVE,
> +	MT76_WED_WDS_ACTIVE,
> +};
> +
>   struct mt76_bus_ops {
>   	u32 (*rr)(struct mt76_dev *dev, u32 offset);
>   	void (*wr)(struct mt76_dev *dev, u32 offset, u32 val);
> @@ -1165,6 +1174,12 @@ void mt76_wed_dma_reset(struct mt76_dev *dev);
>   int mt76_wed_net_setup_tc(struct ieee80211_hw *hw, struct ieee80211_vif *vif,
>   			  struct net_device *netdev, enum tc_setup_type type,
>   			  void *type_data);
> +
> +static inline int mt76_wcid_alloc(u32 *mask, int size)
> +{
> +	return __mt76_wcid_alloc(mask, size, MT76_WED_DEFAULT);
> +}
> +
>   #ifdef CONFIG_NET_MEDIATEK_SOC_WED
>   u32 mt76_wed_init_rx_buf(struct mtk_wed_device *wed, int size);
>   int mt76_wed_offload_enable(struct mtk_wed_device *wed);
> diff --git a/drivers/net/wireless/mediatek/mt76/mt7915/main.c
> b/drivers/net/wireless/mediatek/mt76/mt7915/main.c
> index 3aa31c5cefa6..07c2ce518d97 100644
> --- a/drivers/net/wireless/mediatek/mt76/mt7915/main.c
> +++ b/drivers/net/wireless/mediatek/mt76/mt7915/main.c
> @@ -745,8 +745,15 @@ int mt7915_mac_sta_add(struct mt76_dev *mdev, struct ieee80211_vif *vif,
>   	struct mt7915_vif *mvif = (struct mt7915_vif *)vif->drv_priv;
>   	bool ext_phy = mvif->phy != &dev->phy;
>   	int idx;
> +	u8 flags = MT76_WED_DEFAULT;

In reverse X'mas tree order.

> 
> -	idx = mt76_wcid_alloc(dev->mt76.wcid_mask, MT7915_WTBL_STA);
> +	if (mtk_wed_device_active(&dev->mt76.mmio.wed) &&
> +	    !is_mt7915(&dev->mt76)) {
> +		flags = test_bit(MT_WCID_FLAG_4ADDR, &msta->wcid.flags) ?
> +		       MT76_WED_WDS_ACTIVE : MT76_WED_ACTIVE;
> +	}
> +
> +	idx = __mt76_wcid_alloc(mdev->wcid_mask, MT7915_WTBL_STA, flags);
>   	if (idx < 0)
>   		return -ENOSPC;
> 

[...]

> diff --git a/drivers/net/wireless/mediatek/mt76/mt7915/mcu.c
> b/drivers/net/wireless/mediatek/mt76/mt7915/mcu.c
> index 9d790f234e82..191f966c3f32 100644
> --- a/drivers/net/wireless/mediatek/mt76/mt7915/mcu.c
> +++ b/drivers/net/wireless/mediatek/mt76/mt7915/mcu.c
> @@ -2385,10 +2385,20 @@ int mt7915_mcu_init_firmware(struct mt7915_dev *dev)
> 
>   	mt76_connac_mcu_del_wtbl_all(&dev->mt76);
> 
> -	if ((mtk_wed_device_active(&dev->mt76.mmio.wed) &&
> -	     is_mt7915(&dev->mt76)) ||
> -	    !mtk_wed_get_rx_capa(&dev->mt76.mmio.wed))
> -		mt7915_mcu_wa_cmd(dev, MCU_WA_PARAM_CMD(CAPABILITY), 0, 0, 0);
> +#if IS_ENABLED(CONFIG_NET_MEDIATEK_SOC_WED)

In MT76, there are two styles:
  #if IS_ENABLED(CONFIG_NET_MEDIATEK_SOC_WED)
  #ifdef CONFIG_NET_MEDIATEK_SOC_WED

I think both are correct. Just wonder why not making them consistent. 

> +	if (mtk_wed_device_active(&dev->mt76.mmio.wed)) {
> +		if (is_mt7915(&dev->mt76) ||
> +		    !mtk_wed_get_rx_capa(&dev->mt76.mmio.wed))
> +			ret = mt7915_mcu_wa_cmd(dev, MCU_WA_PARAM_CMD(CAPABILITY),
> +						0, 0, 0);
> +		else
> +			ret = mt7915_mcu_wa_cmd(dev, MCU_WA_PARAM_CMD(SET),
> +						MCU_WA_PARAM_WED_VERSION,
> +						dev->mt76.mmio.wed.rev_id, 0);
> +		if (ret)
> +			return ret;
> +	}
> +#endif
> 
>   	ret = mt7915_mcu_set_mwds(dev, 1);
>   	if (ret)