diff mbox series

[v2] wifi: mt76: mt7915: add wds support when wed is enabled

Message ID e603722d58079af98c57a3dc117274d824d1d832.1669798063.git.sujuan.chen@mediatek.com (mailing list archive)
State Superseded
Delegated to: Felix Fietkau
Headers show
Series [v2] wifi: mt76: mt7915: add wds support when wed is enabled | expand

Commit Message

Sujuan Chen Nov. 30, 2022, 9:18 a.m. UTC 
The current WED only supports 256 wcid, whereas mt7986 can support up to 512 entries,
so firmware provides a rule to get sta_info by DA when wcid is set to 0x3ff by txd.
Also, WED provides a register to overwrite txd wcid, that is, wcid[9:8] can
be overwritten by 0x3 and wcid[7:0] is set to 0xff by host driver.

However, firmware is unable to get sta_info from DA as DA != RA for 4addr cases,
so firmware and wifi host driver both use wcid (256 - 271) and (768 ~ 783)
for sync up to get correct sta_info

Tested-by: Sujuan Chen <sujuan.chen@mediatek.com>
Co-developed-by: Bo Jiao <bo.jiao@mediatek.com>
Signed-off-by: Bo Jiao <bo.jiao@mediatek.com>
Signed-off-by: Sujuan Chen <sujuan.chen@mediatek.com>
---
v2: 
 - drop duplicate settings
 - reduce the patch size by redefining mt76_wcid_alloc
---
 drivers/net/wireless/mediatek/mt76/mt76.h     |  6 +++
 .../net/wireless/mediatek/mt76/mt7915/main.c  | 24 +++++++++--
 .../net/wireless/mediatek/mt76/mt7915/mcu.c   | 13 +++++-
 .../net/wireless/mediatek/mt76/mt7915/mcu.h   |  1 +
 drivers/net/wireless/mediatek/mt76/util.c     | 40 +++++++++++++++++--
 drivers/net/wireless/mediatek/mt76/util.h     |  7 +++-
 6 files changed, 82 insertions(+), 9 deletions(-)

Comments

Felix Fietkau Dec. 9, 2022, 12:32 p.m. UTC | #1 
On 30.11.22 10:18, Sujuan Chen wrote:
> The current WED only supports 256 wcid, whereas mt7986 can support up to 512 entries,
> so firmware provides a rule to get sta_info by DA when wcid is set to 0x3ff by txd.
> Also, WED provides a register to overwrite txd wcid, that is, wcid[9:8] can
> be overwritten by 0x3 and wcid[7:0] is set to 0xff by host driver.
> 
> However, firmware is unable to get sta_info from DA as DA != RA for 4addr cases,
> so firmware and wifi host driver both use wcid (256 - 271) and (768 ~ 783)
> for sync up to get correct sta_info
> 
> Tested-by: Sujuan Chen <sujuan.chen@mediatek.com>
> Co-developed-by: Bo Jiao <bo.jiao@mediatek.com>
> Signed-off-by: Bo Jiao <bo.jiao@mediatek.com>
> Signed-off-by: Sujuan Chen <sujuan.chen@mediatek.com>
> ---
> v2:
>   - drop duplicate settings
>   - reduce the patch size by redefining mt76_wcid_alloc
> ---
>   drivers/net/wireless/mediatek/mt76/mt76.h     |  6 +++
>   .../net/wireless/mediatek/mt76/mt7915/main.c  | 24 +++++++++--
>   .../net/wireless/mediatek/mt76/mt7915/mcu.c   | 13 +++++-
>   .../net/wireless/mediatek/mt76/mt7915/mcu.h   |  1 +
>   drivers/net/wireless/mediatek/mt76/util.c     | 40 +++++++++++++++++--
>   drivers/net/wireless/mediatek/mt76/util.h     |  7 +++-
>   6 files changed, 82 insertions(+), 9 deletions(-)
> 

> diff --git a/drivers/net/wireless/mediatek/mt76/mt7915/main.c b/drivers/net/wireless/mediatek/mt76/mt7915/main.c
> index c40b6098f19a..46a9e4f0396e 100644
> --- a/drivers/net/wireless/mediatek/mt76/mt7915/main.c
> +++ b/drivers/net/wireless/mediatek/mt76/mt7915/main.c
> @@ -1115,6 +1122,13 @@ static void mt7915_sta_set_4addr(struct ieee80211_hw *hw,
>   	else
>   		clear_bit(MT_WCID_FLAG_4ADDR, &msta->wcid.flags);
>   
> +	if (mtk_wed_device_active(&dev->mt76.mmio.wed) &&
> +	    !is_mt7915(&dev->mt76)) {
> +		mt7915_sta_remove(hw, vif, sta);
> +		mt76_sta_pre_rcu_remove(hw, vif, sta);
> +		mt7915_sta_add(hw, vif, sta);
> +	}
> +
>   	mt76_connac_mcu_wtbl_update_hdr_trans(&dev->mt76, vif, sta);
>   }
>   
I suspect that this may a bit racy if there is concurrent tx activity 
(e.g. for EAP auth). Not sure if this could cause problems for the 
firmware or other kinds of bugs.

While my idea may need some rework of the existing functions, I think a 
better flow would be:

1. mt76_sta_pre_rcu_remove
2. save old wcid
3. mt7915_sta_add
4. synchronize_rcu()
5. remove firmware state for old wcid entry

- Felix
Sujuan Chen Dec. 13, 2022, 1:57 a.m. UTC | #2 
On Fri, 2022-12-09 at 13:32 +0100, Felix Fietkau wrote:
> On 30.11.22 10:18, Sujuan Chen wrote:
> > The current WED only supports 256 wcid, whereas mt7986 can support
> > up to 512 entries,
> > so firmware provides a rule to get sta_info by DA when wcid is set
> > to 0x3ff by txd.
> > Also, WED provides a register to overwrite txd wcid, that is,
> > wcid[9:8] can
> > be overwritten by 0x3 and wcid[7:0] is set to 0xff by host driver.
> > 
> > However, firmware is unable to get sta_info from DA as DA != RA for
> > 4addr cases,
> > so firmware and wifi host driver both use wcid (256 - 271) and (768
> > ~ 783)
> > for sync up to get correct sta_info
> > 
> > Tested-by: Sujuan Chen <sujuan.chen@mediatek.com>
> > Co-developed-by: Bo Jiao <bo.jiao@mediatek.com>
> > Signed-off-by: Bo Jiao <bo.jiao@mediatek.com>
> > Signed-off-by: Sujuan Chen <sujuan.chen@mediatek.com>
> > ---
> > v2:
> >   - drop duplicate settings
> >   - reduce the patch size by redefining mt76_wcid_alloc
> > ---
> >   drivers/net/wireless/mediatek/mt76/mt76.h     |  6 +++
> >   .../net/wireless/mediatek/mt76/mt7915/main.c  | 24 +++++++++--
> >   .../net/wireless/mediatek/mt76/mt7915/mcu.c   | 13 +++++-
> >   .../net/wireless/mediatek/mt76/mt7915/mcu.h   |  1 +
> >   drivers/net/wireless/mediatek/mt76/util.c     | 40
> > +++++++++++++++++--
> >   drivers/net/wireless/mediatek/mt76/util.h     |  7 +++-
> >   6 files changed, 82 insertions(+), 9 deletions(-)
> > 
> > diff --git a/drivers/net/wireless/mediatek/mt76/mt7915/main.c
> > b/drivers/net/wireless/mediatek/mt76/mt7915/main.c
> > index c40b6098f19a..46a9e4f0396e 100644
> > --- a/drivers/net/wireless/mediatek/mt76/mt7915/main.c
> > +++ b/drivers/net/wireless/mediatek/mt76/mt7915/main.c
> > @@ -1115,6 +1122,13 @@ static void mt7915_sta_set_4addr(struct
> > ieee80211_hw *hw,
> >   	else
> >   		clear_bit(MT_WCID_FLAG_4ADDR, &msta->wcid.flags);
> >   
> > +	if (mtk_wed_device_active(&dev->mt76.mmio.wed) &&
> > +	    !is_mt7915(&dev->mt76)) {
> > +		mt7915_sta_remove(hw, vif, sta);
> > +		mt76_sta_pre_rcu_remove(hw, vif, sta);
> > +		mt7915_sta_add(hw, vif, sta);
> > +	}
> > +
> >   	mt76_connac_mcu_wtbl_update_hdr_trans(&dev->mt76, vif, sta);
> >   }
> >   
> 
> I suspect that this may a bit racy if there is concurrent tx
> activity 
> (e.g. for EAP auth). Not sure if this could cause problems for the 
> firmware or other kinds of bugs.
> 
> While my idea may need some rework of the existing functions, I think
> a 
> better flow would be:
> 
> 1. mt76_sta_pre_rcu_remove
> 2. save old wcid
> 3. mt7915_sta_add
> 4. synchronize_rcu()
> 5. remove firmware state for old wcid entry
> 
> - Felix

Hi Felix,

Can we modify it like this:

1. pre_sta = kzalloc(sizeof(*sta) + sizeof(*msta), GFP_KERNEL);
2. mt76_sta_pre_rcu_remove
3. memmove(old_sta, sta, sizeof(*sta) + sizeof(*msta))
4. mt7915_sta_add
5. synchronize_rcu()
6. mt7915_sta_remove(hw, vif, pre_sta)
7. kfree(pre_sta)

we can reuse mt7915_sta_remove and keep the sta info for 
mt7915_mcu_add_sta.

if save old wcid only, we need to rework __mt76_sta_remove and
mt7915_mcu_add_sta functions by add wcid as parameter.
__mt76_sta_remove is required to delete a wcid, right?

-sujuan
Sujuan Chen Dec. 20, 2022, 4:26 a.m. UTC | #3 
On Fri, 2022-12-09 at 13:32 +0100, Felix Fietkau wrote:
> On 30.11.22 10:18, Sujuan Chen wrote:
> > The current WED only supports 256 wcid, whereas mt7986 can support
> > up to 512 entries,
> > so firmware provides a rule to get sta_info by DA when wcid is set
> > to 0x3ff by txd.
> > Also, WED provides a register to overwrite txd wcid, that is,
> > wcid[9:8] can
> > be overwritten by 0x3 and wcid[7:0] is set to 0xff by host driver.
> > 
> > However, firmware is unable to get sta_info from DA as DA != RA for
> > 4addr cases,
> > so firmware and wifi host driver both use wcid (256 - 271) and (768
> > ~ 783)
> > for sync up to get correct sta_info
> > 
> > Tested-by: Sujuan Chen <sujuan.chen@mediatek.com>
> > Co-developed-by: Bo Jiao <bo.jiao@mediatek.com>
> > Signed-off-by: Bo Jiao <bo.jiao@mediatek.com>
> > Signed-off-by: Sujuan Chen <sujuan.chen@mediatek.com>
> > ---
> > v2:
> >   - drop duplicate settings
> >   - reduce the patch size by redefining mt76_wcid_alloc
> > ---
> >   drivers/net/wireless/mediatek/mt76/mt76.h     |  6 +++
> >   .../net/wireless/mediatek/mt76/mt7915/main.c  | 24 +++++++++--
> >   .../net/wireless/mediatek/mt76/mt7915/mcu.c   | 13 +++++-
> >   .../net/wireless/mediatek/mt76/mt7915/mcu.h   |  1 +
> >   drivers/net/wireless/mediatek/mt76/util.c     | 40
> > +++++++++++++++++--
> >   drivers/net/wireless/mediatek/mt76/util.h     |  7 +++-
> >   6 files changed, 82 insertions(+), 9 deletions(-)
> > 
> > diff --git a/drivers/net/wireless/mediatek/mt76/mt7915/main.c
> > b/drivers/net/wireless/mediatek/mt76/mt7915/main.c
> > index c40b6098f19a..46a9e4f0396e 100644
> > --- a/drivers/net/wireless/mediatek/mt76/mt7915/main.c
> > +++ b/drivers/net/wireless/mediatek/mt76/mt7915/main.c
> > @@ -1115,6 +1122,13 @@ static void mt7915_sta_set_4addr(struct
> > ieee80211_hw *hw,
> >   	else
> >   		clear_bit(MT_WCID_FLAG_4ADDR, &msta->wcid.flags);
> >   
> > +	if (mtk_wed_device_active(&dev->mt76.mmio.wed) &&
> > +	    !is_mt7915(&dev->mt76)) {
> > +		mt7915_sta_remove(hw, vif, sta);
> > +		mt76_sta_pre_rcu_remove(hw, vif, sta);
> > +		mt7915_sta_add(hw, vif, sta);
> > +	}
> > +
> >   	mt76_connac_mcu_wtbl_update_hdr_trans(&dev->mt76, vif, sta);
> >   }
> >   
> 
> I suspect that this may a bit racy if there is concurrent tx
> activity 
> (e.g. for EAP auth). Not sure if this could cause problems for the 
> firmware or other kinds of bugs.
> 
> While my idea may need some rework of the existing functions, I think
> a 
> better flow would be:
> 
> 1. mt76_sta_pre_rcu_remove
> 2. save old wcid
> 3. mt7915_sta_add
> 4. synchronize_rcu()
> 5. remove firmware state for old wcid entry
> 
Hi Felix,

Can we modify it like this:

1. pre_sta = kzalloc(sizeof(*sta) + sizeof(*msta), GFP_KERNEL);
2. mt76_sta_pre_rcu_remove
3. memmove(old_sta, sta, sizeof(*sta) + sizeof(*msta))
4. mt7915_sta_add
5. synchronize_rcu()
6. mt7915_sta_remove(hw, vif, pre_sta)
7. kfree(pre_sta)

we can reuse mt7915_sta_remove and keep the sta info for 
mt7915_mcu_add_sta.

if save old wcid only, we need to rework __mt76_sta_remove and
mt7915_mcu_add_sta functions by add wcid as parameter.
__mt76_sta_remove is required to delete a wcid, right?

-sujuan

> - Felix
diff mbox series

Patch

diff --git a/drivers/net/wireless/mediatek/mt76/mt76.h b/drivers/net/wireless/mediatek/mt76/mt76.h
index 32a77a0ae9da..4726378fd0ad 100644
--- a/drivers/net/wireless/mediatek/mt76/mt76.h
+++ b/drivers/net/wireless/mediatek/mt76/mt76.h
@@ -60,6 +60,12 @@  enum mt76_wed_type {
 	MT76_WED_Q_RX,
 };
 
+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);
diff --git a/drivers/net/wireless/mediatek/mt76/mt7915/main.c b/drivers/net/wireless/mediatek/mt76/mt7915/main.c
index c40b6098f19a..46a9e4f0396e 100644
--- a/drivers/net/wireless/mediatek/mt76/mt7915/main.c
+++ b/drivers/net/wireless/mediatek/mt76/mt7915/main.c
@@ -660,8 +660,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 ret, idx;
+	u8 flags = MT76_WED_DEFAULT;
 
-	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;
 
@@ -1115,6 +1122,13 @@  static void mt7915_sta_set_4addr(struct ieee80211_hw *hw,
 	else
 		clear_bit(MT_WCID_FLAG_4ADDR, &msta->wcid.flags);
 
+	if (mtk_wed_device_active(&dev->mt76.mmio.wed) &&
+	    !is_mt7915(&dev->mt76)) {
+		mt7915_sta_remove(hw, vif, sta);
+		mt76_sta_pre_rcu_remove(hw, vif, sta);
+		mt7915_sta_add(hw, vif, sta);
+	}
+
 	mt76_connac_mcu_wtbl_update_hdr_trans(&dev->mt76, vif, sta);
 }
 
@@ -1479,15 +1493,19 @@  mt7915_net_fill_forward_path(struct ieee80211_hw *hw,
 	if (!mtk_wed_device_active(wed))
 		return -ENODEV;
 
-	if (msta->wcid.idx > 0xff)
+	if (msta->wcid.idx > MT7915_WTBL_STA)
 		return -EIO;
 
 	path->type = DEV_PATH_MTK_WDMA;
 	path->dev = ctx->dev;
 	path->mtk_wdma.wdma_idx = wed->wdma_idx;
 	path->mtk_wdma.bss = mvif->mt76.idx;
-	path->mtk_wdma.wcid = is_mt7915(&dev->mt76) ? msta->wcid.idx : 0x3ff;
 	path->mtk_wdma.queue = phy != &dev->phy;
+	if (test_bit(MT_WCID_FLAG_4ADDR, &msta->wcid.flags) ||
+	    is_mt7915(&dev->mt76))
+		path->mtk_wdma.wcid = msta->wcid.idx;
+	else
+		path->mtk_wdma.wcid = 0x3ff;
 
 	ctx->dev = NULL;
 
diff --git a/drivers/net/wireless/mediatek/mt76/mt7915/mcu.c b/drivers/net/wireless/mediatek/mt76/mt7915/mcu.c
index 9e479d41eab5..3e73a95ff029 100644
--- a/drivers/net/wireless/mediatek/mt76/mt7915/mcu.c
+++ b/drivers/net/wireless/mediatek/mt76/mt7915/mcu.c
@@ -2307,8 +2307,17 @@  int mt7915_mcu_init_firmware(struct mt7915_dev *dev)
 	if (ret)
 		return ret;
 
-	if (mtk_wed_device_active(&dev->mt76.mmio.wed) && is_mt7915(&dev->mt76))
-		mt7915_mcu_wa_cmd(dev, MCU_WA_PARAM_CMD(CAPABILITY), 0, 0, 0);
+	if (mtk_wed_device_active(&dev->mt76.mmio.wed)) {
+		if (is_mt7915(&dev->mt76))
+			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;
+	}
 
 	ret = mt7915_mcu_set_mwds(dev, 1);
 	if (ret)
diff --git a/drivers/net/wireless/mediatek/mt76/mt7915/mcu.h b/drivers/net/wireless/mediatek/mt76/mt7915/mcu.h
index 46c517e50ae4..b0e5c0e9fa67 100644
--- a/drivers/net/wireless/mediatek/mt76/mt7915/mcu.h
+++ b/drivers/net/wireless/mediatek/mt76/mt7915/mcu.h
@@ -271,6 +271,7 @@  enum {
 	MCU_WA_PARAM_PDMA_RX = 0x04,
 	MCU_WA_PARAM_CPU_UTIL = 0x0b,
 	MCU_WA_PARAM_RED = 0x0e,
+	MCU_WA_PARAM_WED_VERSION = 0x32,
 };
 
 enum mcu_mmps_mode {
diff --git a/drivers/net/wireless/mediatek/mt76/util.c b/drivers/net/wireless/mediatek/mt76/util.c
index 581964425468..5cd5ede0438f 100644
--- a/drivers/net/wireless/mediatek/mt76/util.c
+++ b/drivers/net/wireless/mediatek/mt76/util.c
@@ -42,9 +42,14 @@  bool __mt76_poll_msec(struct mt76_dev *dev, u32 offset, u32 mask, u32 val,
 }
 EXPORT_SYMBOL_GPL(__mt76_poll_msec);
 
-int mt76_wcid_alloc(u32 *mask, int size)
+int __mt76_wcid_alloc(u32 *mask, int size, u8 flag)
 {
+#define MT76_WED_WDS_MIN    256
+#define MT76_WED_WDS_CNT    16
+
 	int i, idx = 0, cur;
+	int min = MT76_WED_WDS_MIN;
+	int max = min + MT76_WED_WDS_CNT;
 
 	for (i = 0; i < DIV_ROUND_UP(size, 32); i++) {
 		idx = ffs(~mask[i]);
@@ -53,16 +58,45 @@  int mt76_wcid_alloc(u32 *mask, int size)
 
 		idx--;
 		cur = i * 32 + idx;
-		if (cur >= size)
+
+		switch (flag) {
+		case MT76_WED_ACTIVE:
+			if (cur >= min && cur < max)
+				continue;
+
+			if (cur >= size) {
+				u32 end = MT76_WED_WDS_CNT - 1;
+
+				i = min / 32;
+				idx = ffs(~mask[i] & GENMASK(end, 0));
+				if (!idx)
+					goto error;
+				idx--;
+				cur = min + idx;
+			}
+
 			break;
+		case MT76_WED_WDS_ACTIVE:
+			if (cur < min)
+				continue;
+			if (cur >= max)
+				goto error;
+
+			break;
+		default:
+			if (cur >= size)
+				goto error;
+			break;
+		}
 
 		mask[i] |= BIT(idx);
 		return cur;
 	}
 
+error:
 	return -1;
 }
-EXPORT_SYMBOL_GPL(mt76_wcid_alloc);
+EXPORT_SYMBOL_GPL(__mt76_wcid_alloc);
 
 int mt76_get_min_avg_rssi(struct mt76_dev *dev, bool ext_phy)
 {
diff --git a/drivers/net/wireless/mediatek/mt76/util.h b/drivers/net/wireless/mediatek/mt76/util.h
index 260965dde94c..99b7263c0a20 100644
--- a/drivers/net/wireless/mediatek/mt76/util.h
+++ b/drivers/net/wireless/mediatek/mt76/util.h
@@ -27,7 +27,12 @@  enum {
 #define MT76_INCR(_var, _size) \
 	(_var = (((_var) + 1) % (_size)))
 
-int mt76_wcid_alloc(u32 *mask, int size);
+int __mt76_wcid_alloc(u32 *mask, int size, u8 flags);
+
+static inline int mt76_wcid_alloc(u32 *mask, int size)
+{
+	return __mt76_wcid_alloc(mask, size, 0);
+}
 
 static inline void
 mt76_wcid_mask_set(u32 *mask, int idx)