From: "Sujuan Chen (陈素娟)" <Sujuan.Chen@mediatek.com>
To: "lorenzo@kernel.org" <lorenzo@kernel.org>
Cc: "linux-wireless@vger.kernel.org" <linux-wireless@vger.kernel.org>,
"linux-mediatek@lists.infradead.org"
<linux-mediatek@lists.infradead.org>,
"nbd@nbd.name" <nbd@nbd.name>,
"Evelyn Tsai (蔡珊鈺)" <Evelyn.Tsai@mediatek.com>,
"Ryder Lee" <Ryder.Lee@mediatek.com>,
"Bo Jiao (焦波)" <Bo.Jiao@mediatek.com>
Subject: Re: [PATCH] wifi: mt76: mt7915: add wds support when wed is enabled
Date: Tue, 22 Nov 2022 10:03:45 +0000 [thread overview]
Message-ID: <d7c7437ce145a2c42da01eca1269dbe660e37b8a.camel@mediatek.com> (raw)
In-Reply-To: <Y3t9NOKseNw2myzE@lore-desk>
On Mon, 2022-11-21 at 14:29 +0100, Lorenzo Bianconi 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>
> > ---
> > This patch is based on
> >
https://patchwork.kernel.org/project/linux-mediatek/list/?series=697444
> > ---
> > drivers/net/wireless/mediatek/mt76/mt76.h | 6 +++
> > .../net/wireless/mediatek/mt76/mt7603/main.c | 2 +-
> > .../net/wireless/mediatek/mt76/mt7615/main.c | 2 +-
> > .../wireless/mediatek/mt76/mt7615/pci_init.c | 2 +-
> > .../wireless/mediatek/mt76/mt7615/usb_sdio.c | 2 +-
> > .../net/wireless/mediatek/mt76/mt76x02_util.c | 2 +-
> > .../net/wireless/mediatek/mt76/mt7915/init.c | 2 +-
> > .../net/wireless/mediatek/mt76/mt7915/main.c | 24 +++++++++--
> > .../net/wireless/mediatek/mt76/mt7915/mcu.c | 11 ++++-
> > .../net/wireless/mediatek/mt76/mt7915/mcu.h | 1 +
> > .../net/wireless/mediatek/mt76/mt7915/mmio.c | 3 ++
> > .../net/wireless/mediatek/mt76/mt7921/init.c | 2 +-
> > .../net/wireless/mediatek/mt76/mt7921/main.c | 2 +-
> > .../net/wireless/mediatek/mt76/mt7996/init.c | 2 +-
> > .../net/wireless/mediatek/mt76/mt7996/main.c | 2 +-
> > drivers/net/wireless/mediatek/mt76/util.c | 42
> > ++++++++++++++++++-
> > drivers/net/wireless/mediatek/mt76/util.h | 2 +-
> > 17 files changed, 91 insertions(+), 18 deletions(-)
> >
>
> Hi Sujuan,
>
> I took just a brief look at the patch, but I think you can
> significantly
> reduce the patch size doing something like:
>
> int __mt76_wcid_alloc(u32 *mask, int size, u8 flag)
> {
> ...
> }
>
> static inline int mt76_wcid_alloc(u32 *mask, int size)
> {
> return __mt76_wcid_alloc(, 0);
> }
>
Hi Lore,
ack, thanks. I will do it in v2.
> > diff --git a/drivers/net/wireless/mediatek/mt76/mt76.h
> > b/drivers/net/wireless/mediatek/mt76/mt76.h
> > index 33f87e518d68..1763b582c020 100644
> > --- a/drivers/net/wireless/mediatek/mt76/mt76.h
> > +++ b/drivers/net/wireless/mediatek/mt76/mt76.h
> > @@ -38,6 +38,12 @@
> > #define MT_WED_Q_RX(_n) __MT_WED_Q(MT76_WED_Q_RX, _n)
> > #define MT_WED_Q_TXFREE __MT_WED_Q(MT76_WED_Q_TXFREE,
> > 0)
> >
> > +enum mt76_wed_state {
> > + MT76_WED_DISABLED,
> > + MT76_WED_ACTIVE,
> > + MT76_WED_WDS_ACTIVE,
> > +};
> > +
> > struct mt76_dev;
> > struct mt76_phy;
> > struct mt76_wcid;
> > diff --git a/drivers/net/wireless/mediatek/mt76/mt7603/main.c
> > b/drivers/net/wireless/mediatek/mt76/mt7603/main.c
> > index ca50feb0b3a9..d788bb59a113 100644
> > --- a/drivers/net/wireless/mediatek/mt76/mt7603/main.c
> > +++ b/drivers/net/wireless/mediatek/mt76/mt7603/main.c
> > @@ -347,7 +347,7 @@ mt7603_sta_add(struct mt76_dev *mdev, struct
> > ieee80211_vif *vif,
> > int idx;
> > int ret = 0;
> >
> > - idx = mt76_wcid_alloc(dev->mt76.wcid_mask, MT7603_WTBL_STA -
> > 1);
> > + idx = mt76_wcid_alloc(dev->mt76.wcid_mask, MT7603_WTBL_STA - 1,
> > 0);
> > if (idx < 0)
> > return -ENOSPC;
> >
>
> [...]
>
> > mt76_connac_mcu_wtbl_update_hdr_trans(&dev->mt76, vif, sta);
> > }
> >
> > @@ -1448,15 +1462,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 2769d6c897d9..aeeeff9b2143 100644
> > --- a/drivers/net/wireless/mediatek/mt76/mt7915/mcu.c
> > +++ b/drivers/net/wireless/mediatek/mt76/mt7915/mcu.c
> > @@ -2303,8 +2303,15 @@ 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))
> > + mt7915_mcu_wa_cmd(dev,
> > MCU_WA_PARAM_CMD(CAPABILITY),
> > + 0, 0, 0);
> > + else
> > + mt7915_mcu_wa_cmd(dev, MCU_WA_PARAM_CMD(SET),
> > + MCU_WA_PARAM_WED_VERSION,
> > + dev->mt76.mmio.wed.rev_id,
> > 0);
> > + }
>
> can you please honor mt7915_mcu_wa_cmd() returned value?
>
Ack.
> >
> > 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 c19b5d66c0e1..59e1ea35f77f 100644
> > --- a/drivers/net/wireless/mediatek/mt76/mt7915/mcu.h
> > +++ b/drivers/net/wireless/mediatek/mt76/mt7915/mcu.h
> > @@ -260,6 +260,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/mt7915/mmio.c
> > b/drivers/net/wireless/mediatek/mt76/mt7915/mmio.c
> > index 1fcf34f57a16..d90793d082b8 100644
> > --- a/drivers/net/wireless/mediatek/mt76/mt7915/mmio.c
> > +++ b/drivers/net/wireless/mediatek/mt76/mt7915/mmio.c
> > @@ -773,6 +773,9 @@ int mt7915_mmio_wed_init(struct mt7915_dev
> > *dev, void *pdev_ptr,
> >
> > dev->mt76.rx_token_size = wed->wlan.rx_npkt;
> >
> > + if (!is_mt7915(&dev->mt76))
> > + wed->wlan.wcid_512 = true;
> > +
> > if (mtk_wed_device_attach(wed))
> > return 0;
> >
> > diff --git a/drivers/net/wireless/mediatek/mt76/mt7921/init.c
> > b/drivers/net/wireless/mediatek/mt76/mt7921/init.c
> > index 79b8055ce4c4..702ff300f8f7 100644
> > --- a/drivers/net/wireless/mediatek/mt76/mt7921/init.c
> > +++ b/drivers/net/wireless/mediatek/mt76/mt7921/init.c
> > @@ -283,7 +283,7 @@ static int mt7921_init_wcid(struct mt7921_dev
> > *dev)
> > int idx;
> >
> > /* Beacon and mgmt frames should occupy wcid 0 */
> > - idx = mt76_wcid_alloc(dev->mt76.wcid_mask, MT7921_WTBL_STA -
> > 1);
> > + idx = mt76_wcid_alloc(dev->mt76.wcid_mask, MT7921_WTBL_STA - 1,
> > 0);
> > if (idx)
> > return -ENOSPC;
> >
> > diff --git a/drivers/net/wireless/mediatek/mt76/mt7921/main.c
> > b/drivers/net/wireless/mediatek/mt76/mt7921/main.c
> > index 41df17efdb3a..3d8771fcb847 100644
> > --- a/drivers/net/wireless/mediatek/mt76/mt7921/main.c
> > +++ b/drivers/net/wireless/mediatek/mt76/mt7921/main.c
> > @@ -814,7 +814,7 @@ int mt7921_mac_sta_add(struct mt76_dev *mdev,
> > struct ieee80211_vif *vif,
> > struct mt7921_vif *mvif = (struct mt7921_vif *)vif->drv_priv;
> > int ret, idx;
> >
> > - idx = mt76_wcid_alloc(dev->mt76.wcid_mask, MT7921_WTBL_STA -
> > 1);
> > + idx = mt76_wcid_alloc(dev->mt76.wcid_mask, MT7921_WTBL_STA - 1,
> > 0);
> > if (idx < 0)
> > return -ENOSPC;
> >
> > diff --git a/drivers/net/wireless/mediatek/mt76/mt7996/init.c
> > b/drivers/net/wireless/mediatek/mt76/mt7996/init.c
> > index cd1657e3585d..4cf055040519 100644
> > --- a/drivers/net/wireless/mediatek/mt76/mt7996/init.c
> > +++ b/drivers/net/wireless/mediatek/mt76/mt7996/init.c
> > @@ -433,7 +433,7 @@ static int mt7996_init_hardware(struct
> > mt7996_dev *dev)
> > return ret;
> >
> > /* Beacon and mgmt frames should occupy wcid 0 */
> > - idx = mt76_wcid_alloc(dev->mt76.wcid_mask, MT7996_WTBL_STA);
> > + idx = mt76_wcid_alloc(dev->mt76.wcid_mask, MT7996_WTBL_STA, 0);
> > if (idx)
> > return -ENOSPC;
> >
> > diff --git a/drivers/net/wireless/mediatek/mt76/mt7996/main.c
> > b/drivers/net/wireless/mediatek/mt76/mt7996/main.c
> > index 21dea3fa7dc1..fd40b515cc5b 100644
> > --- a/drivers/net/wireless/mediatek/mt76/mt7996/main.c
> > +++ b/drivers/net/wireless/mediatek/mt76/mt7996/main.c
> > @@ -579,7 +579,7 @@ int mt7996_mac_sta_add(struct mt76_dev *mdev,
> > struct ieee80211_vif *vif,
> > u8 band_idx = mvif->phy->mt76->band_idx;
> > int ret, idx;
> >
> > - idx = mt76_wcid_alloc(dev->mt76.wcid_mask, MT7996_WTBL_STA);
> > + idx = mt76_wcid_alloc(dev->mt76.wcid_mask, MT7996_WTBL_STA, 0);
> > if (idx < 0)
> > return -ENOSPC;
> >
> > diff --git a/drivers/net/wireless/mediatek/mt76/util.c
> > b/drivers/net/wireless/mediatek/mt76/util.c
> > index 581964425468..0850149f4200 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,13 +58,46 @@ int mt76_wcid_alloc(u32 *mask, int size)
> >
> > idx--;
> > cur = i * 32 + idx;
> > - if (cur >= size)
>
> I think it easier to understand the code if you run the code below
> just if wed
> is active, right?
>
> if (!mtk_wed_device_active())
> break;
>
mtk_wed_device_active() need mt76_dev to get wed.
!mtk_wed_device_active() is include in MT76_WED_DISABLED.
Would it be better to run if (!mtk_wed_device_active())?
Actually, the logic is a bit more complicated.
Using only mtk_wed_device_active() cannot cover all cases.
wcid(256~271) is special using for WDS when WED is enabled.
and wcid(256~271) will be used for normal sta when wcid(exclude
256~271) is run out.
I will change MT76_WED_DISABLED to MT76_WED_DEFAULT which includes
below situation:
(wed is disabled || beacon wcid(0))
is it ok?
> Regards,
> Lorenzo
>
> > +
> > + switch (flag) {
> > + case MT76_WED_DISABLED:
> > + if (cur >= size)
> > + goto error;
> > +
> > break;
> > + 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:
> > + WARN_ON(1);
> > + break;
> > + }
> >
> > mask[i] |= BIT(idx);
> > return cur;
> > }
> >
> > +error:
> > return -1;
> > }
> > EXPORT_SYMBOL_GPL(mt76_wcid_alloc);
> > diff --git a/drivers/net/wireless/mediatek/mt76/util.h
> > b/drivers/net/wireless/mediatek/mt76/util.h
> > index 260965dde94c..c72460e78389 100644
> > --- a/drivers/net/wireless/mediatek/mt76/util.h
> > +++ b/drivers/net/wireless/mediatek/mt76/util.h
> > @@ -27,7 +27,7 @@ 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 flag);
> >
> > static inline void
> > mt76_wcid_mask_set(u32 *mask, int idx)
> > --
> > 2.18.0
> >
next prev parent reply other threads:[~2022-11-22 10:04 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-11-21 11:37 [PATCH] wifi: mt76: mt7915: add wds support when wed is enabled Sujuan Chen
2022-11-21 13:29 ` Lorenzo Bianconi
2022-11-22 10:03 ` Sujuan Chen (陈素娟) [this message]
2022-12-06 3:17 ` Sujuan Chen (陈素娟)
2022-12-06 3:17 ` Sujuan Chen (陈素娟)
2022-11-21 13:42 ` Lorenzo Bianconi
2022-11-22 6:27 ` Sujuan Chen (陈素娟)
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=d7c7437ce145a2c42da01eca1269dbe660e37b8a.camel@mediatek.com \
--to=sujuan.chen@mediatek.com \
--cc=Bo.Jiao@mediatek.com \
--cc=Evelyn.Tsai@mediatek.com \
--cc=Ryder.Lee@mediatek.com \
--cc=linux-mediatek@lists.infradead.org \
--cc=linux-wireless@vger.kernel.org \
--cc=lorenzo@kernel.org \
--cc=nbd@nbd.name \
/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.