[PATCH net v5 0/5] net: ethernet: mtk_wed: misc fixes

[PATCH net v5 0/5] net: ethernet: mtk_wed: misc fixes

[Zhi-Jun You]

Ths patch series includes some fixes for issues that were discovered
by Sashiko and one original patch fixing issue I encountered.

Patch 1,2,3,5 fix issues that were found by Sashiko
Patch 1 fixes tx wdma ring reset for WED v3
Patch 2 fixes the reset condition for rx wdma.
Patch 3 fixes rx ring and tx wdma ring not being freed up
Patch 4 fixes WED on non-DBDC mt7986.
Patch 5 adds error messages and bail out when wed_start fail

---
Changes in v5:
- Add cover letter.
- Add patch 3 to address Sashiko review on rx ring and tx wdma ring not
  being freed up.
- Add patch 5 to address Sashiko review on error handling.
- Link to review: https://lore.kernel.org/all/179000700034.2160803.10594281691384015089@kernel.org/

Changes in v4:
- Add patch 2 to address Sashiko review on rx wdma reset condition.
- Link to review: https://lore.kernel.org/all/20260730090030.201100-1-pabeni@redhat.com/

Changes in v3:
- Add patch 1 to address Sashiko review on WED v3 tx wdma ring reset.
- Link to review: https://lore.kernel.org/netdev/20260706101044.161059-1-pabeni@redhat.com/

Changes in v2:
- Address issues raisied by Sashiko in patch 4
- Link to review: https://lore.kernel.org/all/20260706101044.161059-1-pabeni@redhat.com/
- Add comment to explain the special case

---
Zhi-Jun You (5):
      net: ethernet: mtk_wed: skip ring reset in wdma_tx_ring_setup for WED v3
      net: ethernet: mtk_wed: fix reset condition for rx wdma
      net: ethernet: mtk_wed: free up rings during detach
      net: ethernet: mtk_wed: setup WDMA_RING_TX(0) for non-DBDC MT7986
      net: ethernet: mtk_wed: bail out when failed to set up rings in wed_start

 drivers/net/ethernet/mediatek/mtk_wed.c | 40 ++++++++++++++++++++++++++++-----
 1 file changed, 35 insertions(+), 5 deletions(-)
---
base-commit: 191aa41548b2d2b6af534a6d5cf84a552435c74c
change-id: 20261016-wedfix-8079f58dbf06

Best regards,
--  
Zhi-Jun You <hujy652@gmail.com>

[PATCH net v5 1/5] net: ethernet: mtk_wed: skip ring reset in wdma_tx_ring_setup for WED v3

[Zhi-Jun You]

Currently WED v3 tx wdma ring, ring reset happens in a separate code
block unconditionally.

Moreover, v3 has double the size of wdma_desc and has different init
values. mtk_wed_ring_reset doens't deal with that and would corrupt the
wdma_desc.

Fix this by skipping mtk_wed_ring_reset in mtk_wdma_tx_ring_setup for
WED v3.

Fixes: e2f64db13aa1 ("net: ethernet: mtk_wed: introduce WED support for MT7988")
Signed-off-by: Zhi-Jun You <hujy652@gmail.com>
---
 drivers/net/ethernet/mediatek/mtk_wed.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/mediatek/mtk_wed.c b/drivers/net/ethernet/mediatek/mtk_wed.c
index 53d3b7e00e1c..d55042ccaff1 100644
--- a/drivers/net/ethernet/mediatek/mtk_wed.c
+++ b/drivers/net/ethernet/mediatek/mtk_wed.c
@@ -1938,7 +1938,7 @@ mtk_wed_wdma_tx_ring_setup(struct mtk_wed_device *dev, int idx, int size,
 	wdma_w32(dev, MTK_WDMA_RING_TX(idx) + MTK_WED_RING_OFS_CPU_IDX, 0);
 	wdma_w32(dev, MTK_WDMA_RING_TX(idx) + MTK_WED_RING_OFS_DMA_IDX, 0);
 
-	if (reset)
+	if (!mtk_wed_is_v3_or_greater(dev->hw) && reset)
 		mtk_wed_ring_reset(wdma, MTK_WED_WDMA_RING_SIZE, true);
 
 	if (!idx)  {

-- 
2.47.3

[PATCH net v5 2/5] net: ethernet: mtk_wed: fix reset condition for rx wdma

[Zhi-Jun You]

In mtk_wdma_rx_reset, only the wdma rings that weren't allocated are
reset which is wrong.

Fix this by inverting the reset condition so allocated rings will be
reset.

Fixes: 4c5de09eb0d0 ("net: ethernet: mtk_wed: add configure wed wo support")
Signed-off-by: Zhi-Jun You <hujy652@gmail.com>
---
 drivers/net/ethernet/mediatek/mtk_wed.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/mediatek/mtk_wed.c b/drivers/net/ethernet/mediatek/mtk_wed.c
index d55042ccaff1..3f0be70c3f14 100644
--- a/drivers/net/ethernet/mediatek/mtk_wed.c
+++ b/drivers/net/ethernet/mediatek/mtk_wed.c
@@ -257,7 +257,7 @@ mtk_wdma_rx_reset(struct mtk_wed_device *dev)
 	wdma_w32(dev, MTK_WDMA_RESET_IDX, 0);
 
 	for (i = 0; i < ARRAY_SIZE(dev->rx_wdma); i++) {
-		if (dev->rx_wdma[i].desc)
+		if (!dev->rx_wdma[i].desc)
 			continue;
 
 		wdma_w32(dev,

-- 
2.47.3

[PATCH net v5 3/5] net: ethernet: mtk_wed: free up rings during detach

[Zhi-Jun You]

During __mtk_wed_detach, rx_ring and tx_wdma aren't freed up.

Fix this by add them to mtk_wed_free_rx_rings, also set
MTK_WED_RING_CONFIGURED flag in tx/rx ring and wdma tx/rx ring setup.
Only free rings that has the flag set.

This patch is pulled from mtk-openwrt-feeds to address Sashiko's review.

Link: https://github.com/mediatek/mtk-openwrt-feeds/blob/main/25.12/files/target/linux/mediatek/patches-6.12/999-wed-04-Fix-reinsert-wifi-module-cause-memory-leak-issue.patch
Fixes: 4c5de09eb0d0 ("net: ethernet: mtk_wed: add configure wed wo support")
Signed-off-by: Zhi-Jun You <hujy652@gmail.com>
---
 drivers/net/ethernet/mediatek/mtk_wed.c | 21 +++++++++++++++++++--
 1 file changed, 19 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/mediatek/mtk_wed.c b/drivers/net/ethernet/mediatek/mtk_wed.c
index 3f0be70c3f14..b8264435acbe 100644
--- a/drivers/net/ethernet/mediatek/mtk_wed.c
+++ b/drivers/net/ethernet/mediatek/mtk_wed.c
@@ -949,6 +949,16 @@ mtk_wed_free_ring(struct mtk_wed_device *dev, struct mtk_wed_ring *ring)
 static void
 mtk_wed_free_rx_rings(struct mtk_wed_device *dev)
 {
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(dev->rx_ring); i++)
+		if ((dev->tx_ring[i].flags & MTK_WED_RING_CONFIGURED))
+			mtk_wed_free_ring(dev, &dev->rx_ring[i]);
+
+	for (i = 0; i < ARRAY_SIZE(dev->tx_wdma); i++)
+		if ((dev->tx_wdma[i].flags & MTK_WED_RING_CONFIGURED))
+			mtk_wed_free_ring(dev, &dev->tx_wdma[i]);
+
 	mtk_wed_free_rx_buffer(dev);
 	mtk_wed_free_ring(dev, &dev->rro.ring);
 }
@@ -959,9 +969,11 @@ mtk_wed_free_tx_rings(struct mtk_wed_device *dev)
 	int i;
 
 	for (i = 0; i < ARRAY_SIZE(dev->tx_ring); i++)
-		mtk_wed_free_ring(dev, &dev->tx_ring[i]);
+		if ((dev->tx_ring[i].flags & MTK_WED_RING_CONFIGURED))
+			mtk_wed_free_ring(dev, &dev->tx_ring[i]);
 	for (i = 0; i < ARRAY_SIZE(dev->rx_wdma); i++)
-		mtk_wed_free_ring(dev, &dev->rx_wdma[i]);
+		if ((dev->rx_wdma[i].flags & MTK_WED_RING_CONFIGURED))
+			mtk_wed_free_ring(dev, &dev->rx_wdma[i]);
 }
 
 static void
@@ -1885,6 +1897,8 @@ mtk_wed_wdma_rx_ring_setup(struct mtk_wed_device *dev, int idx, int size,
 					 dev->hw->soc->wdma_desc_size, true))
 		return -ENOMEM;
 
+	wdma->flags |= MTK_WED_RING_CONFIGURED;
+
 	wdma_w32(dev, MTK_WDMA_RING_RX(idx) + MTK_WED_RING_OFS_BASE,
 		 wdma->desc_phys);
 	wdma_w32(dev, MTK_WDMA_RING_RX(idx) + MTK_WED_RING_OFS_COUNT,
@@ -1931,6 +1945,8 @@ mtk_wed_wdma_tx_ring_setup(struct mtk_wed_device *dev, int idx, int size,
 		}
 	}
 
+	wdma->flags |= MTK_WED_RING_CONFIGURED;
+
 	wdma_w32(dev, MTK_WDMA_RING_TX(idx) + MTK_WED_RING_OFS_BASE,
 		 wdma->desc_phys);
 	wdma_w32(dev, MTK_WDMA_RING_TX(idx) + MTK_WED_RING_OFS_COUNT,
@@ -2504,6 +2520,7 @@ mtk_wed_tx_ring_setup(struct mtk_wed_device *dev, int idx, void __iomem *regs,
 
 	ring->reg_base = MTK_WED_RING_TX(idx);
 	ring->wpdma = regs;
+	ring->flags |= MTK_WED_RING_CONFIGURED;
 
 	if (mtk_wed_is_v3_or_greater(dev->hw) && idx == 1) {
 		/* reset prefetch index */

-- 
2.47.3

[PATCH net v5 4/5] net: ethernet: mtk_wed: setup WDMA_RING_TX(0) for non-DBDC MT7986

[Zhi-Jun You]

WDMA_RING_TX(0) is required to set MTK_WED_WDMA_RING_TX for WED RX
but on a non-DBDC MT7986 it is never setup because idx is 1.

Setting MTK_WED_WDMA_RING_TX with WDMA_RING_TX(1) is not feasible because
WED still tries to send through WDMA_RING_TX(0). This is verified with
register dump.

Fix this by calling mtk_wed_wdma_tx_ring_setup if wed is v2 and
rx_ring[0] is not allocated and reset tx_wdma[0] if it's already
allocated.

Fixes: 4c5de09eb0d0 ("net: ethernet: mtk_wed: add configure wed wo support")
Signed-off-by: Zhi-Jun You <hujy652@gmail.com>
---
 drivers/net/ethernet/mediatek/mtk_wed.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/net/ethernet/mediatek/mtk_wed.c b/drivers/net/ethernet/mediatek/mtk_wed.c
index b8264435acbe..3a7c2bb33691 100644
--- a/drivers/net/ethernet/mediatek/mtk_wed.c
+++ b/drivers/net/ethernet/mediatek/mtk_wed.c
@@ -2355,6 +2355,13 @@ mtk_wed_start(struct mtk_wed_device *dev, u32 irq_mask)
 		if (!dev->rx_wdma[i].desc)
 			mtk_wed_wdma_rx_ring_setup(dev, i, 16, false);
 
+	/*
+	 * non-DBDC MT7986 allocates only rx_ring[1] and tx_wdma[1] during setup
+	 * but tx_wdma[0] is also needed for WED to function.
+	 */
+	if (mtk_wed_is_v2(dev->hw) && !dev->rx_ring[0].desc)
+		mtk_wed_wdma_tx_ring_setup(dev, 0, MTK_WED_WDMA_RING_SIZE, !!dev->tx_wdma[0].desc);
+
 	if (dev->wlan.hw_rro) {
 		for (i = 0; i < MTK_WED_RX_PAGE_QUEUES; i++) {
 			u32 addr = MTK_WED_RRO_MSDU_PG_CTRL0(i) +

-- 
2.47.3

[PATCH net v5 5/5] net: ethernet: mtk_wed: bail out when failed to set up rings in wed_start

[Zhi-Jun You]

Add error message and return when fail to set up tx/rx wdma rings.

This is suggested by Sashiko.

Link: https://lore.kernel.org/all/179000700034.2160803.10594281691384015089@kernel.org/
Signed-off-by: Zhi-Jun You <hujy652@gmail.com>
---
 drivers/net/ethernet/mediatek/mtk_wed.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/mediatek/mtk_wed.c b/drivers/net/ethernet/mediatek/mtk_wed.c
index 3a7c2bb33691..86845893ae20 100644
--- a/drivers/net/ethernet/mediatek/mtk_wed.c
+++ b/drivers/net/ethernet/mediatek/mtk_wed.c
@@ -2353,14 +2353,20 @@ mtk_wed_start(struct mtk_wed_device *dev, u32 irq_mask)
 
 	for (i = 0; i < ARRAY_SIZE(dev->rx_wdma); i++)
 		if (!dev->rx_wdma[i].desc)
-			mtk_wed_wdma_rx_ring_setup(dev, i, 16, false);
+			if (mtk_wed_wdma_rx_ring_setup(dev, i, 16, false)) {
+				dev_err(dev->hw->dev, "wdma rx ring %d setup failed\n", i);
+				return;
+			}
 
 	/*
 	 * non-DBDC MT7986 allocates only rx_ring[1] and tx_wdma[1] during setup
 	 * but tx_wdma[0] is also needed for WED to function.
 	 */
 	if (mtk_wed_is_v2(dev->hw) && !dev->rx_ring[0].desc)
-		mtk_wed_wdma_tx_ring_setup(dev, 0, MTK_WED_WDMA_RING_SIZE, !!dev->tx_wdma[0].desc);
+		if (mtk_wed_wdma_tx_ring_setup(dev, 0, MTK_WED_WDMA_RING_SIZE, !!dev->tx_wdma[0].desc)) {
+			dev_err(dev->hw->dev, "wdma tx ring 0 setup failed\n");
+			return;
+		}
 
 	if (dev->wlan.hw_rro) {
 		for (i = 0; i < MTK_WED_RX_PAGE_QUEUES; i++) {

-- 
2.47.3

Re: [PATCH net v5 0/5] net: ethernet: mtk_wed: misc fixes

[netdev-bot+sinfo]

Hi!

This is an automated message. This series looks like a fix, but its
commit messages seem to be missing some information:

 - Whether the issue was actually triggered, or is only theoretical
   (e.g. found by code inspection). If it was triggered please include
   the symptoms, like the stack trace or error messages.

 - What hardware the change was tested on. For driver fixes please
   mention the device (and if relevant firmware version) used for
   testing, or say that the change was not tested on real hardware.

Please do not repost the series just to address the above. Instead,
reply to this email with the missing information, so that reviewers
can take it into account. If the series needs another revision for
other reasons, please include the information in the commit messages
then.

The evaluation is done by an LLM so it may be wrong, if you think
that is the case please reply and explain.

Re: [PATCH net v5 0/5] net: ethernet: mtk_wed: misc fixes

[Zhi-Jun You]

On Fri, Oct 16, 2026 at 5:19 PM <netdev-bot+sinfo@kernel.org> wrote:
>
> Hi!
>
> This is an automated message. This series looks like a fix, but its
> commit messages seem to be missing some information:
>
>  - Whether the issue was actually triggered, or is only theoretical
>    (e.g. found by code inspection). If it was triggered please include
>    the symptoms, like the stack trace or error messages.

Patch 1,2,3,5 were found by Sashiko

>  - What hardware the change was tested on. For driver fixes please
>    mention the device (and if relevant firmware version) used for
>    testing, or say that the change was not tested on real hardware.

Patch 1 compiled test only since I don't have a WEDv3 board.

The rest were tested on a mt7986 + mt7916 board.

> Please do not repost the series just to address the above. Instead,
> reply to this email with the missing information, so that reviewers
> can take it into account. If the series needs another revision for
> other reasons, please include the information in the commit messages
> then.
>
> The evaluation is done by an LLM so it may be wrong, if you think
> that is the case please reply and explain.

I will send a new revision addressing issues found by review.

Best regards,
Zhi-Jun

pw-bot: cr

Re: [PATCH net v5 1/5] net: ethernet: mtk_wed: skip ring reset in wdma_tx_ring_setup for WED v3

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 1 potential
issue(s) to consider.

Critical: 0 · High: 1 · Medium: 0 · Low: 0

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [High] net: mtk_wed: memory leak of rx_ring and tx_ring on setup
  failure

--

> diff --git a/drivers/net/ethernet/mediatek/mtk_wed.c b/drivers/net/ethernet/mediatek/mtk_wed.c
> --- a/drivers/net/ethernet/mediatek/mtk_wed.c
> +++ b/drivers/net/ethernet/mediatek/mtk_wed.c
> @@ -1938,7 +1938,7 @@ mtk_wed_wdma_tx_ring_setup(struct mtk_wed_device *dev, int idx, int size,
>  	wdma_w32(dev, MTK_WDMA_RING_TX(idx) + MTK_WED_RING_OFS_CPU_IDX, 0);
>  	wdma_w32(dev, MTK_WDMA_RING_TX(idx) + MTK_WED_RING_OFS_DMA_IDX, 0);
>  
> -	if (reset)
> +	if (!mtk_wed_is_v3_or_greater(dev->hw) && reset)
>  		mtk_wed_ring_reset(wdma, MTK_WED_WDMA_RING_SIZE, true);
>  
>  	if (!idx)  {
>

[Severity: High]
The change here looks fine to me. Since mtk_wed_wdma_tx_ring_setup()
is one of the failure points involved, I'm raising the ring teardown
changes later in this series here. That is the patch that gates
mtk_wed_free_ring() on MTK_WED_RING_CONFIGURED.

First, mtk_wed_free_rx_rings() walks dev->rx_ring[] but tests
dev->tx_ring[i].flags before calling mtk_wed_free_ring() on
dev->rx_ring[i]. That looks like a copy/paste slip. If tx_ring[i] was
never configured, an allocated rx_ring[i] is never freed. Should that
be dev->rx_ring[i].flags?

Second, mtk_wed_tx_ring_setup() and mtk_wed_rx_ring_setup() call
mtk_wed_ring_alloc() first. They then call
mtk_wed_wdma_rx_ring_setup() or mtk_wed_wdma_tx_ring_setup(). If the
wdma step fails, they return -ENOMEM before MTK_WED_RING_CONFIGURED
is set on the ring that was already allocated.

Once the free paths only call mtk_wed_free_ring() for rings with
MTK_WED_RING_CONFIGURED set, __mtk_wed_detach() skips those partially
set up rings. Their coherent DMA descriptor memory is then leaked.
Before this series, mtk_wed_free_ring() keyed off ring->desc, which
handled this case.

I see two options:

 - Keep relying on the ring->desc check in mtk_wed_free_ring() for
   freeing, so the flag gating is not needed in the free paths.
 - Free the just-allocated ring on the error path in the setup
   functions.

The right choice depends on what the flag gating is meant to protect
against, so could you explain the intent there?

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261016-wedfix-v5-0-8b02e78fd34f%40gmail.com

Re: [PATCH net v5 2/5] net: ethernet: mtk_wed: fix reset condition for rx wdma

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found more severe
issue(s) in other patches of this series. This mail is only an FYI, in case
you want to address the nit picks below when you respin.

Low priority / nit picks for your consideration:

- [Low] The commit message for "net: ethernet: mtk_wed: fix reset
  condition for rx wdma" only says the condition in mtk_wdma_rx_reset()…

--

net: ethernet: mtk_wed: fix reset condition for rx wdma

This commit inverts the check in the rx_wdma loop of mtk_wdma_rx_reset().
Allocated WDMA RX rings now have their CPU index cleared. Unallocated
rings are skipped.

> In mtk_wdma_rx_reset, only the wdma rings that weren't allocated are
> reset which is wrong.
>
> Fix this by inverting the reset condition so allocated rings will be
> reset.

[Severity: Low]
This isn't a bug, and the code change looks right. Could the commit
message say what goes wrong for users, and which paths are affected?

mtk_wed_start() sets up every rx_wdma ring that doesn't have a
descriptor yet:

drivers/net/ethernet/mediatek/mtk_wed.c:mtk_wed_start() {
    ...
	for (i = 0; i < ARRAY_SIZE(dev->rx_wdma); i++)
		if (!dev->rx_wdma[i].desc)
			mtk_wed_wdma_rx_ring_setup(dev, i, 16, false);
    ...
}

So once the device has started, all rx_wdma rings normally have a
descriptor. The exception is when buffer or ring allocation fails.

With the old condition, the loop in mtk_wdma_rx_reset() would then skip
every ring. MTK_WDMA_RING_RX(i) + MTK_WED_RING_OFS_CPU_IDX would never be
cleared for any live ring.

mtk_wdma_rx_reset() has two callers:

mtk_wed_reset_dma()
    /* 2. reset WDMA rx DMA */
    busy = !!mtk_wdma_rx_reset(dev);

__mtk_wed_detach()
    mtk_wdma_rx_reset(dev);

Could the message add a sentence on the symptom? For example: stale WDMA
RX CPU indices after a WED/SER reset, or after detach and re-attach.

That would help reviewers and stable maintainers judge how urgent this
Fixes: patch is.

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261016-wedfix-v5-0-8b02e78fd34f%40gmail.com

Re: [PATCH net v5 3/5] net: ethernet: mtk_wed: free up rings during detach

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 3 potential
issue(s) to consider.

Critical: 0 · High: 1 · Medium: 2 · Low: 0

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [High] Copy-paste bug in the new rx_ring cleanup loop in
  mtk_wed_free_rx_rings()…
- [Medium] The new flag gating in mtk_wed_free_tx_rings() causes a leak
  on partial setup failure.
- [Medium] The patch now frees the rx_ring descriptor memory in
  __mtk_wed_detach() → mtk_wed_free_rx_rings(), but WED never disables…

--

net: ethernet: mtk_wed: free up rings during detach

This commit adds frees for rx_ring and tx_wdma in mtk_wed_free_rx_rings()
during __mtk_wed_detach(). It also sets MTK_WED_RING_CONFIGURED in the
tx/rx ring and wdma tx/rx ring setup paths, and only frees rings that have
that flag set.

Link: https://github.com/mediatek/mtk-openwrt-feeds/blob/main/25.12/files/target/linux/mediatek/patches-6.12/999-wed-04-Fix-reinsert-wifi-module-cause-memory-leak-issue.patch

> diff --git a/drivers/net/ethernet/mediatek/mtk_wed.c b/drivers/net/ethernet/mediatek/mtk_wed.c
> index 3f0be70c3f145..b8264435acbe5 100644
> --- a/drivers/net/ethernet/mediatek/mtk_wed.c
> +++ b/drivers/net/ethernet/mediatek/mtk_wed.c
> @@ -949,6 +949,16 @@ mtk_wed_free_ring(struct mtk_wed_device *dev, struct mtk_wed_ring *ring)
>  static void
>  mtk_wed_free_rx_rings(struct mtk_wed_device *dev)
>  {
> +	int i;
> +
> +	for (i = 0; i < ARRAY_SIZE(dev->rx_ring); i++)
> +		if ((dev->tx_ring[i].flags & MTK_WED_RING_CONFIGURED))
> +			mtk_wed_free_ring(dev, &dev->rx_ring[i]);

[Severity: High]
Should this check dev->rx_ring[i].flags instead of dev->tx_ring[i].flags?

The flag on rx_ring[idx] is set in mtk_wed_rx_ring_setup(), and that
function never touches tx_ring[idx]. The existing reader in
mtk_wed_dma_enable() also tests the flag on the rx ring itself:

	for (i = 0; i < MTK_WED_RX_QUEUES; i++) {
		struct mtk_wed_ring *ring = &dev->rx_ring[i];
		...
		if (!(ring->flags & MTK_WED_RING_CONFIGURED))
			continue; /* queue is not configured by mt76 */

TX and RX rings are set up separately, so their flags can differ. Take
MT7992 with a second HIF:

- mt7996_dma_init() puts band1 RX on the primary WED's rx_ring[1].
- mt7996_register_phy() sends band1 TX to wed_hif2.
- mt7996_init_tx_queues() then uses MT_WED_Q_TX(0) on wed_hif2.

So the primary WED's tx_ring[1] is never configured.

In that case, is the rx_ring[1] descriptor memory still leaked on every
detach? This loop skips it, and the pointer is then lost to the
memset(dev, 0, sizeof(*dev)) at the end of __mtk_wed_detach(). That looks
like the same leak the commit message says it fixes.

mtk_wed_free_ring() already returns early when ring->desc is NULL. Would
dropping the flag check here be enough?

> +
> +	for (i = 0; i < ARRAY_SIZE(dev->tx_wdma); i++)
> +		if ((dev->tx_wdma[i].flags & MTK_WED_RING_CONFIGURED))
> +			mtk_wed_free_ring(dev, &dev->tx_wdma[i]);
> +
>  	mtk_wed_free_rx_buffer(dev);
>  	mtk_wed_free_ring(dev, &dev->rro.ring);
>  }

[Severity: Medium]
Can rx_ring[i] be freed here while the WLAN chip's WFDMA RX engine still
points at it?

mtk_wed_rx_ring_setup() writes ring->desc_phys into the WLAN chip's PDMA
RX ring registers through wpdma_rx_w32(). mtk_wed_dma_enable() then turns
on the chip's RX DMA:

		val = wifi_r32(dev,
			       dev->wlan.wpdma_rx_glo -
			       dev->wlan.phy_base) | MTK_WFMDA_RX_DMA_EN;
		wifi_w32(dev,
			 dev->wlan.wpdma_rx_glo - dev->wlan.phy_base,
			 val);

mtk_wed_dma_disable() never clears MTK_WFMDA_RX_DMA_EN. On normal teardown
mt7915/mt7996 disable DMA before mt76_dma_cleanup() detaches WED. The
mt7915 probe error path does not seem to do that:

mt7915_init_hardware()
  mt7915_dma_init()
    mt7915_dma_enable()
      mt7915_dma_start()
        mtk_wed_device_start()     <- WFDMA RX enabled
  mt7915_mcu_init()                <- fails, returns without DMA disable

The probe error label in mt7915/pci.c or mt7915/soc.c then runs:

mtk_wed_device_detach()
  __mtk_wed_detach()
    mtk_wed_free_rx_rings()
      mtk_wed_free_ring(dev, &dev->rx_ring[i])
        dma_free_coherent()

If the chip writes back an RX descriptor after this point, would that
corrupt memory that has since been reused? Before this patch the ring
leaked, so such writes landed in memory that was still allocated.

The error path leaving DMA armed predates this patch, and the RX packet
buffers were already freed in this window. Whether the chip really writes
descriptors after MCU init fails depends on hardware behaviour. Freeing
the descriptor ring here is the new part.

> @@ -959,9 +969,11 @@ mtk_wed_free_tx_rings(struct mtk_wed_device *dev)
>  	int i;
>  
>  	for (i = 0; i < ARRAY_SIZE(dev->tx_ring); i++)
> -		mtk_wed_free_ring(dev, &dev->tx_ring[i]);
> +		if ((dev->tx_ring[i].flags & MTK_WED_RING_CONFIGURED))
> +			mtk_wed_free_ring(dev, &dev->tx_ring[i]);

[Severity: Medium]
Does this leak the tx_ring[idx] descriptor buffer when
mtk_wed_tx_ring_setup() fails partway?

mtk_wed_tx_ring_setup() allocates the descriptor before it sets the flag:

	if (!reset && mtk_wed_ring_alloc(dev, ring, MTK_WED_TX_RING_SIZE,
					 sizeof(*ring->desc), true))
		return -ENOMEM;

	if (mtk_wed_wdma_rx_ring_setup(dev, idx, MTK_WED_WDMA_RING_SIZE,
				       reset))
		return -ENOMEM;

	ring->reg_base = MTK_WED_RING_TX(idx);
	ring->wpdma = regs;
	ring->flags |= MTK_WED_RING_CONFIGURED;

If the rx_wdma dma_alloc_coherent() fails, tx_ring[idx].desc stays
allocated but the flag is never set. The -ENOMEM travels up:

mt76_wed_dma_setup()
  mt76_dma_alloc_queue()
    mt7915/mt7996 dma init
      probe error path
        mtk_wed_device_detach()

Before this patch, mtk_wed_free_tx_rings() freed the ring because
mtk_wed_free_ring() only checks ring->desc. With the new check the ring
is skipped, and the memset in __mtk_wed_detach() then drops the pointer.

mtk_wed_rx_ring_setup() uses the same order. A failure in
mtk_wed_wdma_tx_ring_setup() returns before rx_ring[idx].flags is set, so
the flag-gated rx_ring free in mtk_wed_free_rx_rings() cannot reclaim
that ring either.

Since mtk_wed_free_ring() already guards on ring->desc, what does the
MTK_WED_RING_CONFIGURED check add in these free loops? The commit message
does not explain why the frees are gated.

>  	for (i = 0; i < ARRAY_SIZE(dev->rx_wdma); i++)
> -		mtk_wed_free_ring(dev, &dev->rx_wdma[i]);
> +		if ((dev->rx_wdma[i].flags & MTK_WED_RING_CONFIGURED))
> +			mtk_wed_free_ring(dev, &dev->rx_wdma[i]);
>  }

[ ... ]

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261016-wedfix-v5-0-8b02e78fd34f%40gmail.com

Re: [PATCH net v5 4/5] net: ethernet: mtk_wed: setup WDMA_RING_TX(0) for non-DBDC MT7986

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 3 potential
issue(s) to consider.

Critical: 0 · High: 0 · Medium: 2 · Low: 1

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [Medium] In mtk_wed_start()
  (drivers/net/ethernet/mediatek/mtk_wed.c:2362-2363), this commit adds…
- [Medium] mtk_wed_free_rx_rings()
  (drivers/net/ethernet/mediatek/mtk_wed.c:954-956) decides whether to…
- [Low] The new comment in mtk_wed_start() describes the motivating case,
  a non-DBDC MT7986 that allocates only rx_ring[1]/tx_wdma[1].

--

net: ethernet: mtk_wed: setup WDMA_RING_TX(0) for non-DBDC MT7986

This patch makes mtk_wed_start() set up WDMA_RING_TX(0) on WED v2 devices
when rx_ring[0] was not allocated. This is the case on a non-DBDC MT7986,
which only sets up ring index 1. If tx_wdma[0] is already allocated, it is
reset instead of being allocated again.

> diff --git a/drivers/net/ethernet/mediatek/mtk_wed.c b/drivers/net/ethernet/mediatek/mtk_wed.c
> index b8264435acbe5..3a7c2bb33691f 100644
> --- a/drivers/net/ethernet/mediatek/mtk_wed.c
> +++ b/drivers/net/ethernet/mediatek/mtk_wed.c
> @@ -2355,6 +2355,13 @@ mtk_wed_start(struct mtk_wed_device *dev, u32 irq_mask)
>  		if (!dev->rx_wdma[i].desc)
>  			mtk_wed_wdma_rx_ring_setup(dev, i, 16, false);
>  
> +	/*
> +	 * non-DBDC MT7986 allocates only rx_ring[1] and tx_wdma[1] during setup
> +	 * but tx_wdma[0] is also needed for WED to function.
> +	 */
> +	if (mtk_wed_is_v2(dev->hw) && !dev->rx_ring[0].desc)

[Severity: Low]
This isn't a bug, but the comment covers less than the condition does. The
guard matches any WED v2 device where rx_ring[0] was not set up, and MT7981
also has .version = 2. It is not limited to a non-DBDC MT7986.

Could the comment describe the condition as it is implemented, the way the
commit message does?

> +		mtk_wed_wdma_tx_ring_setup(dev, 0, MTK_WED_WDMA_RING_SIZE, !!dev->tx_wdma[0].desc);

[Severity: Medium]
Should the return value of mtk_wed_wdma_tx_ring_setup() be checked here?

On the first start, tx_wdma[0].desc is NULL, so reset is false and the
helper allocates the ring. If the allocation fails, the helper returns
before it programs any WDMA or WED ring register:

mtk_wed_wdma_tx_ring_setup() {
	...
	if (!reset && mtk_wed_ring_alloc(dev, wdma, MTK_WED_WDMA_RING_SIZE,
					 dev->hw->soc->wdma_desc_size, true))
		return -ENOMEM;
	...
}

mtk_wed_start() keeps going anyway. It calls mtk_wed_hw_init() and then
mtk_wed_dma_enable(), which does:

	wed_set(dev, MTK_WED_WDMA_GLO_CFG,
		MTK_WED_WDMA_GLO_CFG_TX_DRV_EN |
		MTK_WED_WDMA_GLO_CFG_TX_DDONE_CHK);

It then sets dev->running = true. At that point WDMA TX is enabled but the
ring 0 base was never programmed.

The other caller of this helper, mtk_wed_rx_ring_setup(), does pass the
failure back:

	if (mtk_wed_wdma_tx_ring_setup(dev, idx, MTK_WED_WDMA_RING_SIZE,
				       reset))
		return -ENOMEM;

A later patch in this series, "net: ethernet: mtk_wed: bail out when
failed to set up rings in wed_start", wraps this call with a dev_err()
and a return. With that patch applied, the end of the series is fine. Can
the check be added in this patch so it works on its own?

[ ... ]

[Severity: Medium]
This code is not part of this hunk. It comes from the earlier patch in
this series, "net: ethernet: mtk_wed: free up rings during detach". It is
mentioned here because mtk_wed_free_rx_rings() is also what frees the
tx_wdma[0] ring added above. Does the flag check test the right array?

mtk_wed_free_rx_rings() {
	...
	for (i = 0; i < ARRAY_SIZE(dev->rx_ring); i++)
		if ((dev->tx_ring[i].flags & MTK_WED_RING_CONFIGURED))
			mtk_wed_free_ring(dev, &dev->rx_ring[i]);
	...
}

mtk_wed_rx_ring_setup() sets MTK_WED_RING_CONFIGURED on
dev->rx_ring[idx].flags, not on tx_ring[idx]. So if rx_ring[i] is
configured and tx_ring[i] is not, the coherent descriptor buffer for
rx_ring[i] is not freed. After that, __mtk_wed_detach() calls
memset(dev, 0, sizeof(*dev)) and the pointer is lost.

One setup where this appears to happen is MT7988 with an MT7992 using
hif2. mt7996_dma_init() registers the band1 rx queue on the primary WED
as MT_WED_Q_RX(1). mt7996_register_phy() sends band1 tx to wed_hif2. The
primary WED therefore has rx_ring[1] configured but not tx_ring[1].

Does primary rx_ring[1] then leak on every detach, for example on a
module reload? Should this check dev->rx_ring[i].flags instead?

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261016-wedfix-v5-0-8b02e78fd34f%40gmail.com

Re: [PATCH net v5 5/5] net: ethernet: mtk_wed: bail out when failed to set up rings in wed_start

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 3 potential
issue(s) to consider.

Critical: 0 (1) · High: 0 · Medium: 2 · Low: 1

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [Medium] The new bail-outs in mtk_wed_start()
  (drivers/net/ethernet/mediatek/mtk_wed.c) log a dev_err() and return,…
- [Medium] net: ethernet: mtk_wed: memory leak on ring setup error paths
- [Low] The commit fixes error handling for two calls in mtk_wed_start()
  but has no Fixes: tag.

Pre-existing issues:
- [Critical] This problem wasn't introduced by this patch, but it is a
  double free in the same reset -> start lifecycle.

--

net: ethernet: mtk_wed: bail out when failed to set up rings in wed_start

This commit checks the return values of mtk_wed_wdma_rx_ring_setup() and
mtk_wed_wdma_tx_ring_setup() in mtk_wed_start(). When either one fails, it
prints an error and returns early.

Link: https://lore.kernel.org/all/179000700034.2160803.10594281691384015089@kernel.org/

> Add error message and return when fail to set up tx/rx wdma rings.
>
> This is suggested by Sashiko.

[Severity: Low]
Should this patch have a Fixes: tag?

The previous patch in this series, "net: ethernet: mtk_wed: setup
WDMA_RING_TX(0) for non-DBDC MT7986", added the
mtk_wed_wdma_tx_ring_setup(dev, 0, ...) call checked here, and that patch
carries Fixes: 4c5de09eb0d0. Without a tag on this patch, a stable
backport of the earlier one could land without its error check.

The unchecked mtk_wed_wdma_rx_ring_setup() call comes from commit
23dca7a90017 ("net: ethernet: mtk_wed: add reset to tx_ring_setup
callback").

Every other patch in this series has a Fixes: tag. The line "This is
suggested by Sashiko." is free text and does not say which bug is being
fixed.

> diff --git a/drivers/net/ethernet/mediatek/mtk_wed.c b/drivers/net/ethernet/mediatek/mtk_wed.c
> index 3a7c2bb33691f..86845893ae20f 100644
> --- a/drivers/net/ethernet/mediatek/mtk_wed.c
> +++ b/drivers/net/ethernet/mediatek/mtk_wed.c
> @@ -2353,14 +2353,20 @@ mtk_wed_start(struct mtk_wed_device *dev, u32 irq_mask)
>  
>  	for (i = 0; i < ARRAY_SIZE(dev->rx_wdma); i++)
>  		if (!dev->rx_wdma[i].desc)
> -			mtk_wed_wdma_rx_ring_setup(dev, i, 16, false);
> +			if (mtk_wed_wdma_rx_ring_setup(dev, i, 16, false)) {
> +				dev_err(dev->hw->dev, "wdma rx ring %d setup failed\n", i);
> +				return;
> +			}

[Severity: Medium]
Does the caller ever find out that start failed here? The same question
applies to the tx_wdma[0] return below.

mtk_wed_start() is static void and is installed as the .start op. In
include/linux/soc/mediatek/mtk_wed.h the op is declared as:

	void (*start)(struct mtk_wed_device *dev, u32 irq_mask);

The early return therefore skips mtk_wed_hw_init(),
mtk_wed_configure_irq(), mtk_wed_set_ext_int(), the v1 PCIe mirror map
write, mtk_wed_rro_cfg(), mtk_wed_amsdu_init(), mtk_wed_dma_enable() and
dev->running = true, and nothing is reported back.

mtk_wed_device_active() only checks the ops pointer:

#define mtk_wed_device_active(_dev) !!(_dev)->ops

so mt76 still treats WED as active. mt7915_dma_start() has already put
WFDMA into WED mode and written MT_INT_WED_MASK_CSR before it gets here:

		mtk_wed_device_start(&dev->mt76.mmio.wed, wed_irq_mask);
	}

	irq_mask = reset ? MT_INT_MCU_CMD : irq_mask;

	mt7915_irq_enable(dev, irq_mask);

It then returns 0, even though it returns int and could pass an error up.
mt7996_dma_start() does the same after writing MT_INT_MASK_CSR.

On WED v3 with hw_rro, mt76 later calls mtk_wed_start_hw_rro(). That
function has no init or running check. It writes MTK_WED_WPDMA_INT_MASK
and MTK_WED_INT_MASK and sets MTK_WED_RRO_MSDU_PG_DRV_EN. Because of the
early return, mtk_wed_hw_init() never programmed MTK_WED_RX_BM_BASE or
MTK_WED_RRO_PG_BM_BASE.

Can this leave a half-initialized WED that the wlan driver keeps using as
if it had started, with only the dev_err() to show anything went wrong?

Would it be better to have ->start return an int, so that mtk_wed_start()
and the mt76 callers can propagate the error and unwind?

>  
>  	/*
>  	 * non-DBDC MT7986 allocates only rx_ring[1] and tx_wdma[1] during setup
>  	 * but tx_wdma[0] is also needed for WED to function.
>  	 */
>  	if (mtk_wed_is_v2(dev->hw) && !dev->rx_ring[0].desc)
> -		mtk_wed_wdma_tx_ring_setup(dev, 0, MTK_WED_WDMA_RING_SIZE, !!dev->tx_wdma[0].desc);
> +		if (mtk_wed_wdma_tx_ring_setup(dev, 0, MTK_WED_WDMA_RING_SIZE, !!dev->tx_wdma[0].desc)) {
> +			dev_err(dev->hw->dev, "wdma tx ring 0 setup failed\n");
> +			return;
> +		}

[Severity: Medium]
This is outside the lines changed here. It is the same ring setup
error handling this patch is tightening, though, so it seems worth
raising.

In mtk_wed_rx_ring_setup(), mtk_wed_ring_alloc() first allocates the
descriptor memory for dev->rx_ring[idx]. If the following
mtk_wed_wdma_tx_ring_setup() call fails, the function returns -ENOMEM
without setting MTK_WED_RING_CONFIGURED on that ring.

mtk_wed_tx_ring_setup() has the same shape. dev->tx_ring[idx] is
allocated, and a failure from mtk_wed_wdma_rx_ring_setup() returns
without setting the flag.

The teardown helpers only free rings that have
MTK_WED_RING_CONFIGURED set. In both cases the coherent DMA buffer
for the wlan-facing ring is therefore never released by
mtk_wed_free_tx_rings() or mtk_wed_free_rx_rings(). If setup is
later retried with reset == false, mtk_wed_ring_alloc() overwrites
ring->desc and the old buffer is lost for good.

mtk_wed_free_rx_rings() also looks like it tests the wrong array. It
frees dev->rx_ring[i] based on dev->tx_ring[i].flags. Whether an rx
ring is freed or skipped then depends on the tx ring with the same
index, not on the rx ring itself. That makes the rx side leak more
easily still.

Should the error paths in mtk_wed_rx_ring_setup() and
mtk_wed_tx_ring_setup() free the ring they just allocated? Or should
the free helpers key off ring->desc instead of the flag? And should
the check in mtk_wed_free_rx_rings() test dev->rx_ring[i].flags?

[ ... ]

[Severity: Critical]
This isn't a bug introduced by this patch, but it sits in the same reset
and start lifecycle. mtk_wed_rx_reset() does:

	mtk_wed_free_rx_buffer(dev);
	mtk_wed_hwrro_free_buffer(dev);

When dev->rx_buf_ring.desc is non-NULL, mtk_wed_free_rx_buffer() already
ends with its own call:

	dev->wlan.release_rx_buf(dev);
	dma_free_coherent(dev->hw->dev, dev->rx_buf_ring.size * sizeof(*desc),
			  desc, dev->rx_buf_ring.desc_phys);

	mtk_wed_hwrro_free_buffer(dev);

mtk_wed_hwrro_free_buffer() frees everything but never clears
dev->hw_rro.pages or dev->hw_rro.desc:

	dma_free_coherent(dev->hw->dev, dev->hw_rro.size * sizeof(*desc),
			  desc, dev->hw_rro.desc_phys);

free_pagelist:
	kfree(page_list);

Does the second call in mtk_wed_rx_reset() then read page_list[] from
freed memory? It would also __free_page() the hw_rro pages,
dma_free_coherent() hw_rro.desc and kfree() the page list a second time.

This looks reachable on every SER/DMA reset on WED v3 with wlan.hw_rro
set, which mt7996 does for the primary WED:

mt7996_dma_reset()
  mtk_wed_device_dma_reset()
    mtk_wed_reset_dma()
      mtk_wed_rx_reset()
        mtk_wed_free_rx_buffer()
          mtk_wed_hwrro_free_buffer()
        mtk_wed_hwrro_free_buffer()

There is a second, separate issue: mtk_wed_free_rx_buffer() leaves
dev->rx_buf_ring.desc pointing at freed memory. Suppose the next
mtk_wed_start()->mtk_wed_rx_buffer_alloc() fails at dma_alloc_coherent()
before that pointer is reassigned. Could a later
__mtk_wed_detach()->mtk_wed_free_rx_rings()->mtk_wed_free_rx_buffer() then
free it again?

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261016-wedfix-v5-0-8b02e78fd34f%40gmail.com

Re: [PATCH net v5 3/5] net: ethernet: mtk_wed: free up rings during detach

[Zhi-Jun You]

On Tue, Oct 20, 2026 at 6:10 PM <netdev-bot+sashiko@kernel.org> wrote:
>
> Thank you for your contribution! Sashiko AI review found 3 potential
> issue(s) to consider.
>
> Critical: 0 · High: 1 · Medium: 2 · Low: 0
>
> If you believe none of the issues are legitimate, please reply to each
> issue of Medium severity or higher and explain why the AI is wrong.
> If any of them are legitimate and you plan to respin, please let patchwork
> know by including "pw-bot: cr" as a separate line at the end of your reply
> (one such reply per series is enough).
>
> - [High] Copy-paste bug in the new rx_ring cleanup loop in
>   mtk_wed_free_rx_rings()…
> - [Medium] The new flag gating in mtk_wed_free_tx_rings() causes a leak
>   on partial setup failure.
> - [Medium] The patch now frees the rx_ring descriptor memory in
>   __mtk_wed_detach() → mtk_wed_free_rx_rings(), but WED never disables…
>
> --
>
> net: ethernet: mtk_wed: free up rings during detach
>
> This commit adds frees for rx_ring and tx_wdma in mtk_wed_free_rx_rings()
> during __mtk_wed_detach(). It also sets MTK_WED_RING_CONFIGURED in the
> tx/rx ring and wdma tx/rx ring setup paths, and only frees rings that have
> that flag set.
>
> Link: https://github.com/mediatek/mtk-openwrt-feeds/blob/main/25.12/files/target/linux/mediatek/patches-6.12/999-wed-04-Fix-reinsert-wifi-module-cause-memory-leak-issue.patch
>
> > diff --git a/drivers/net/ethernet/mediatek/mtk_wed.c b/drivers/net/ethernet/mediatek/mtk_wed.c
> > index 3f0be70c3f145..b8264435acbe5 100644
> > --- a/drivers/net/ethernet/mediatek/mtk_wed.c
> > +++ b/drivers/net/ethernet/mediatek/mtk_wed.c
> > @@ -949,6 +949,16 @@ mtk_wed_free_ring(struct mtk_wed_device *dev, struct mtk_wed_ring *ring)
> >  static void
> >  mtk_wed_free_rx_rings(struct mtk_wed_device *dev)
> >  {
> > +     int i;
> > +
> > +     for (i = 0; i < ARRAY_SIZE(dev->rx_ring); i++)
> > +             if ((dev->tx_ring[i].flags & MTK_WED_RING_CONFIGURED))
> > +                     mtk_wed_free_ring(dev, &dev->rx_ring[i]);
>
> [Severity: High]
> Should this check dev->rx_ring[i].flags instead of dev->tx_ring[i].flags?
>
> The flag on rx_ring[idx] is set in mtk_wed_rx_ring_setup(), and that
> function never touches tx_ring[idx]. The existing reader in
> mtk_wed_dma_enable() also tests the flag on the rx ring itself:

ACK will fix it.

>
>         for (i = 0; i < MTK_WED_RX_QUEUES; i++) {
>                 struct mtk_wed_ring *ring = &dev->rx_ring[i];
>                 ...
>                 if (!(ring->flags & MTK_WED_RING_CONFIGURED))
>                         continue; /* queue is not configured by mt76 */
>
> TX and RX rings are set up separately, so their flags can differ. Take
> MT7992 with a second HIF:
>
> - mt7996_dma_init() puts band1 RX on the primary WED's rx_ring[1].
> - mt7996_register_phy() sends band1 TX to wed_hif2.
> - mt7996_init_tx_queues() then uses MT_WED_Q_TX(0) on wed_hif2.
>
> So the primary WED's tx_ring[1] is never configured.
>
> In that case, is the rx_ring[1] descriptor memory still leaked on every
> detach? This loop skips it, and the pointer is then lost to the
> memset(dev, 0, sizeof(*dev)) at the end of __mtk_wed_detach(). That looks
> like the same leak the commit message says it fixes.
>
> mtk_wed_free_ring() already returns early when ring->desc is NULL. Would
> dropping the flag check here be enough?
>
> > +
> > +     for (i = 0; i < ARRAY_SIZE(dev->tx_wdma); i++)
> > +             if ((dev->tx_wdma[i].flags & MTK_WED_RING_CONFIGURED))
> > +                     mtk_wed_free_ring(dev, &dev->tx_wdma[i]);
> > +
> >       mtk_wed_free_rx_buffer(dev);
> >       mtk_wed_free_ring(dev, &dev->rro.ring);
> >  }
>
> [Severity: Medium]
> Can rx_ring[i] be freed here while the WLAN chip's WFDMA RX engine still
> points at it?
>
> mtk_wed_rx_ring_setup() writes ring->desc_phys into the WLAN chip's PDMA
> RX ring registers through wpdma_rx_w32(). mtk_wed_dma_enable() then turns
> on the chip's RX DMA:
>
>                 val = wifi_r32(dev,
>                                dev->wlan.wpdma_rx_glo -
>                                dev->wlan.phy_base) | MTK_WFMDA_RX_DMA_EN;
>                 wifi_w32(dev,
>                          dev->wlan.wpdma_rx_glo - dev->wlan.phy_base,
>                          val);
>
> mtk_wed_dma_disable() never clears MTK_WFMDA_RX_DMA_EN. On normal teardown
> mt7915/mt7996 disable DMA before mt76_dma_cleanup() detaches WED. The
> mt7915 probe error path does not seem to do that:
>
> mt7915_init_hardware()
>   mt7915_dma_init()
>     mt7915_dma_enable()
>       mt7915_dma_start()
>         mtk_wed_device_start()     <- WFDMA RX enabled
>   mt7915_mcu_init()                <- fails, returns without DMA disable
>
> The probe error label in mt7915/pci.c or mt7915/soc.c then runs:
>
> mtk_wed_device_detach()
>   __mtk_wed_detach()
>     mtk_wed_free_rx_rings()
>       mtk_wed_free_ring(dev, &dev->rx_ring[i])
>         dma_free_coherent()
>
> If the chip writes back an RX descriptor after this point, would that
> corrupt memory that has since been reused? Before this patch the ring
> leaked, so such writes landed in memory that was still allocated.
>
> The error path leaving DMA armed predates this patch, and the RX packet
> buffers were already freed in this window. Whether the chip really writes
> descriptors after MCU init fails depends on hardware behaviour. Freeing
> the descriptor ring here is the new part.
>
> > @@ -959,9 +969,11 @@ mtk_wed_free_tx_rings(struct mtk_wed_device *dev)
> >       int i;
> >
> >       for (i = 0; i < ARRAY_SIZE(dev->tx_ring); i++)
> > -             mtk_wed_free_ring(dev, &dev->tx_ring[i]);
> > +             if ((dev->tx_ring[i].flags & MTK_WED_RING_CONFIGURED))
> > +                     mtk_wed_free_ring(dev, &dev->tx_ring[i]);
>
> [Severity: Medium]
> Does this leak the tx_ring[idx] descriptor buffer when
> mtk_wed_tx_ring_setup() fails partway?
>
> mtk_wed_tx_ring_setup() allocates the descriptor before it sets the flag:
>
>         if (!reset && mtk_wed_ring_alloc(dev, ring, MTK_WED_TX_RING_SIZE,
>                                          sizeof(*ring->desc), true))
>                 return -ENOMEM;
>
>         if (mtk_wed_wdma_rx_ring_setup(dev, idx, MTK_WED_WDMA_RING_SIZE,
>                                        reset))
>                 return -ENOMEM;
>
>         ring->reg_base = MTK_WED_RING_TX(idx);
>         ring->wpdma = regs;
>         ring->flags |= MTK_WED_RING_CONFIGURED;
>
> If the rx_wdma dma_alloc_coherent() fails, tx_ring[idx].desc stays
> allocated but the flag is never set. The -ENOMEM travels up:
>
> mt76_wed_dma_setup()
>   mt76_dma_alloc_queue()
>     mt7915/mt7996 dma init
>       probe error path
>         mtk_wed_device_detach()
>
> Before this patch, mtk_wed_free_tx_rings() freed the ring because
> mtk_wed_free_ring() only checks ring->desc. With the new check the ring
> is skipped, and the memset in __mtk_wed_detach() then drops the pointer.
>
> mtk_wed_rx_ring_setup() uses the same order. A failure in
> mtk_wed_wdma_tx_ring_setup() returns before rx_ring[idx].flags is set, so
> the flag-gated rx_ring free in mtk_wed_free_rx_rings() cannot reclaim
> that ring either.
>
> Since mtk_wed_free_ring() already guards on ring->desc, what does the
> MTK_WED_RING_CONFIGURED check add in these free loops? The commit message
> does not explain why the frees are gated.

The flag was from the original vendor patch.
I will change to rely on ring->desc.

Best regards,
Zhi-Jun


>
> >       for (i = 0; i < ARRAY_SIZE(dev->rx_wdma); i++)
> > -             mtk_wed_free_ring(dev, &dev->rx_wdma[i]);
> > +             if ((dev->rx_wdma[i].flags & MTK_WED_RING_CONFIGURED))
> > +                     mtk_wed_free_ring(dev, &dev->rx_wdma[i]);
> >  }
>
> [ ... ]
>
> --
> Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261016-wedfix-v5-0-8b02e78fd34f%40gmail.com

Re: [PATCH net v5 4/5] net: ethernet: mtk_wed: setup WDMA_RING_TX(0) for non-DBDC MT7986

[Zhi-Jun You]

On Tue, Oct 20, 2026 at 6:10 PM <netdev-bot+sashiko@kernel.org> wrote:
>
> Thank you for your contribution! Sashiko AI review found 3 potential
> issue(s) to consider.
>
> Critical: 0 · High: 0 · Medium: 2 · Low: 1
>
> If you believe none of the issues are legitimate, please reply to each
> issue of Medium severity or higher and explain why the AI is wrong.
> If any of them are legitimate and you plan to respin, please let patchwork
> know by including "pw-bot: cr" as a separate line at the end of your reply
> (one such reply per series is enough).
>
> - [Medium] In mtk_wed_start()
>   (drivers/net/ethernet/mediatek/mtk_wed.c:2362-2363), this commit adds…
> - [Medium] mtk_wed_free_rx_rings()
>   (drivers/net/ethernet/mediatek/mtk_wed.c:954-956) decides whether to…
> - [Low] The new comment in mtk_wed_start() describes the motivating case,
>   a non-DBDC MT7986 that allocates only rx_ring[1]/tx_wdma[1].
>
> --
>
> net: ethernet: mtk_wed: setup WDMA_RING_TX(0) for non-DBDC MT7986
>
> This patch makes mtk_wed_start() set up WDMA_RING_TX(0) on WED v2 devices
> when rx_ring[0] was not allocated. This is the case on a non-DBDC MT7986,
> which only sets up ring index 1. If tx_wdma[0] is already allocated, it is
> reset instead of being allocated again.
>
> > diff --git a/drivers/net/ethernet/mediatek/mtk_wed.c b/drivers/net/ethernet/mediatek/mtk_wed.c
> > index b8264435acbe5..3a7c2bb33691f 100644
> > --- a/drivers/net/ethernet/mediatek/mtk_wed.c
> > +++ b/drivers/net/ethernet/mediatek/mtk_wed.c
> > @@ -2355,6 +2355,13 @@ mtk_wed_start(struct mtk_wed_device *dev, u32 irq_mask)
> >               if (!dev->rx_wdma[i].desc)
> >                       mtk_wed_wdma_rx_ring_setup(dev, i, 16, false);
> >
> > +     /*
> > +      * non-DBDC MT7986 allocates only rx_ring[1] and tx_wdma[1] during setup
> > +      * but tx_wdma[0] is also needed for WED to function.
> > +      */
> > +     if (mtk_wed_is_v2(dev->hw) && !dev->rx_ring[0].desc)
>
> [Severity: Low]
> This isn't a bug, but the comment covers less than the condition does. The
> guard matches any WED v2 device where rx_ring[0] was not set up, and MT7981
> also has .version = 2. It is not limited to a non-DBDC MT7986.
>

This is incorrect. MT7981 is DBDC which means two rx_rings.
My condition check is correct.

> Could the comment describe the condition as it is implemented, the way the
> commit message does?
>
> > +             mtk_wed_wdma_tx_ring_setup(dev, 0, MTK_WED_WDMA_RING_SIZE, !!dev->tx_wdma[0].desc);
>
> [Severity: Medium]
> Should the return value of mtk_wed_wdma_tx_ring_setup() be checked here?
>
> On the first start, tx_wdma[0].desc is NULL, so reset is false and the
> helper allocates the ring. If the allocation fails, the helper returns
> before it programs any WDMA or WED ring register:
>
> mtk_wed_wdma_tx_ring_setup() {
>         ...
>         if (!reset && mtk_wed_ring_alloc(dev, wdma, MTK_WED_WDMA_RING_SIZE,
>                                          dev->hw->soc->wdma_desc_size, true))
>                 return -ENOMEM;
>         ...
> }
>
> mtk_wed_start() keeps going anyway. It calls mtk_wed_hw_init() and then
> mtk_wed_dma_enable(), which does:
>
>         wed_set(dev, MTK_WED_WDMA_GLO_CFG,
>                 MTK_WED_WDMA_GLO_CFG_TX_DRV_EN |
>                 MTK_WED_WDMA_GLO_CFG_TX_DDONE_CHK);
>
> It then sets dev->running = true. At that point WDMA TX is enabled but the
> ring 0 base was never programmed.
>
> The other caller of this helper, mtk_wed_rx_ring_setup(), does pass the
> failure back:
>
>         if (mtk_wed_wdma_tx_ring_setup(dev, idx, MTK_WED_WDMA_RING_SIZE,
>                                        reset))
>                 return -ENOMEM;
>
> A later patch in this series, "net: ethernet: mtk_wed: bail out when
> failed to set up rings in wed_start", wraps this call with a dev_err()
> and a return. With that patch applied, the end of the series is fine. Can
> the check be added in this patch so it works on its own?

ACK. Will do.

Best regards,
Zhi-Jun
>
> [ ... ]
>
> [Severity: Medium]
> This code is not part of this hunk. It comes from the earlier patch in
> this series, "net: ethernet: mtk_wed: free up rings during detach". It is
> mentioned here because mtk_wed_free_rx_rings() is also what frees the
> tx_wdma[0] ring added above. Does the flag check test the right array?
>
> mtk_wed_free_rx_rings() {
>         ...
>         for (i = 0; i < ARRAY_SIZE(dev->rx_ring); i++)
>                 if ((dev->tx_ring[i].flags & MTK_WED_RING_CONFIGURED))
>                         mtk_wed_free_ring(dev, &dev->rx_ring[i]);
>         ...
> }
>
> mtk_wed_rx_ring_setup() sets MTK_WED_RING_CONFIGURED on
> dev->rx_ring[idx].flags, not on tx_ring[idx]. So if rx_ring[i] is
> configured and tx_ring[i] is not, the coherent descriptor buffer for
> rx_ring[i] is not freed. After that, __mtk_wed_detach() calls
> memset(dev, 0, sizeof(*dev)) and the pointer is lost.
>
> One setup where this appears to happen is MT7988 with an MT7992 using
> hif2. mt7996_dma_init() registers the band1 rx queue on the primary WED
> as MT_WED_Q_RX(1). mt7996_register_phy() sends band1 tx to wed_hif2. The
> primary WED therefore has rx_ring[1] configured but not tx_ring[1].
>
> Does primary rx_ring[1] then leak on every detach, for example on a
> module reload? Should this check dev->rx_ring[i].flags instead?
>
> --
> Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261016-wedfix-v5-0-8b02e78fd34f%40gmail.com

Re: [PATCH net v5 5/5] net: ethernet: mtk_wed: bail out when failed to set up rings in wed_start

[Zhi-Jun You]

On Tue, Oct 20, 2026 at 6:10 PM <netdev-bot+sashiko@kernel.org> wrote:
>
> Thank you for your contribution! Sashiko AI review found 3 potential
> issue(s) to consider.
>
> Critical: 0 (1) · High: 0 · Medium: 2 · Low: 1
>
> If you believe none of the issues are legitimate, please reply to each
> issue of Medium severity or higher and explain why the AI is wrong.
> If any of them are legitimate and you plan to respin, please let patchwork
> know by including "pw-bot: cr" as a separate line at the end of your reply
> (one such reply per series is enough).
>
> - [Medium] The new bail-outs in mtk_wed_start()
>   (drivers/net/ethernet/mediatek/mtk_wed.c) log a dev_err() and return,…
> - [Medium] net: ethernet: mtk_wed: memory leak on ring setup error paths
> - [Low] The commit fixes error handling for two calls in mtk_wed_start()
>   but has no Fixes: tag.
>
> Pre-existing issues:
> - [Critical] This problem wasn't introduced by this patch, but it is a
>   double free in the same reset -> start lifecycle.
>
> --
>
> net: ethernet: mtk_wed: bail out when failed to set up rings in wed_start
>
> This commit checks the return values of mtk_wed_wdma_rx_ring_setup() and
> mtk_wed_wdma_tx_ring_setup() in mtk_wed_start(). When either one fails, it
> prints an error and returns early.
>
> Link: https://lore.kernel.org/all/179000700034.2160803.10594281691384015089@kernel.org/
>
> > Add error message and return when fail to set up tx/rx wdma rings.
> >
> > This is suggested by Sashiko.
>
> [Severity: Low]
> Should this patch have a Fixes: tag?
>
> The previous patch in this series, "net: ethernet: mtk_wed: setup
> WDMA_RING_TX(0) for non-DBDC MT7986", added the
> mtk_wed_wdma_tx_ring_setup(dev, 0, ...) call checked here, and that patch
> carries Fixes: 4c5de09eb0d0. Without a tag on this patch, a stable
> backport of the earlier one could land without its error check.
>
> The unchecked mtk_wed_wdma_rx_ring_setup() call comes from commit
> 23dca7a90017 ("net: ethernet: mtk_wed: add reset to tx_ring_setup
> callback").
>
> Every other patch in this series has a Fixes: tag. The line "This is
> suggested by Sashiko." is free text and does not say which bug is being
> fixed.
>
> > diff --git a/drivers/net/ethernet/mediatek/mtk_wed.c b/drivers/net/ethernet/mediatek/mtk_wed.c
> > index 3a7c2bb33691f..86845893ae20f 100644
> > --- a/drivers/net/ethernet/mediatek/mtk_wed.c
> > +++ b/drivers/net/ethernet/mediatek/mtk_wed.c
> > @@ -2353,14 +2353,20 @@ mtk_wed_start(struct mtk_wed_device *dev, u32 irq_mask)
> >
> >       for (i = 0; i < ARRAY_SIZE(dev->rx_wdma); i++)
> >               if (!dev->rx_wdma[i].desc)
> > -                     mtk_wed_wdma_rx_ring_setup(dev, i, 16, false);
> > +                     if (mtk_wed_wdma_rx_ring_setup(dev, i, 16, false)) {
> > +                             dev_err(dev->hw->dev, "wdma rx ring %d setup failed\n", i);
> > +                             return;
> > +                     }
>
> [Severity: Medium]
> Does the caller ever find out that start failed here? The same question
> applies to the tx_wdma[0] return below.
>
> mtk_wed_start() is static void and is installed as the .start op. In
> include/linux/soc/mediatek/mtk_wed.h the op is declared as:
>
>         void (*start)(struct mtk_wed_device *dev, u32 irq_mask);
>
> The early return therefore skips mtk_wed_hw_init(),
> mtk_wed_configure_irq(), mtk_wed_set_ext_int(), the v1 PCIe mirror map
> write, mtk_wed_rro_cfg(), mtk_wed_amsdu_init(), mtk_wed_dma_enable() and
> dev->running = true, and nothing is reported back.
>
> mtk_wed_device_active() only checks the ops pointer:
>
> #define mtk_wed_device_active(_dev) !!(_dev)->ops
>
> so mt76 still treats WED as active. mt7915_dma_start() has already put
> WFDMA into WED mode and written MT_INT_WED_MASK_CSR before it gets here:
>
>                 mtk_wed_device_start(&dev->mt76.mmio.wed, wed_irq_mask);
>         }
>
>         irq_mask = reset ? MT_INT_MCU_CMD : irq_mask;
>
>         mt7915_irq_enable(dev, irq_mask);
>
> It then returns 0, even though it returns int and could pass an error up.
> mt7996_dma_start() does the same after writing MT_INT_MASK_CSR.
>
> On WED v3 with hw_rro, mt76 later calls mtk_wed_start_hw_rro(). That
> function has no init or running check. It writes MTK_WED_WPDMA_INT_MASK
> and MTK_WED_INT_MASK and sets MTK_WED_RRO_MSDU_PG_DRV_EN. Because of the
> early return, mtk_wed_hw_init() never programmed MTK_WED_RX_BM_BASE or
> MTK_WED_RRO_PG_BM_BASE.
>
> Can this leave a half-initialized WED that the wlan driver keeps using as
> if it had started, with only the dev_err() to show anything went wrong?
>
> Would it be better to have ->start return an int, so that mtk_wed_start()
> and the mt76 callers can propagate the error and unwind?

I will let the maintainer decide what to do there.

Best regards,
Zhi-Jun

>
> >
> >       /*
> >        * non-DBDC MT7986 allocates only rx_ring[1] and tx_wdma[1] during setup
> >        * but tx_wdma[0] is also needed for WED to function.
> >        */
> >       if (mtk_wed_is_v2(dev->hw) && !dev->rx_ring[0].desc)
> > -             mtk_wed_wdma_tx_ring_setup(dev, 0, MTK_WED_WDMA_RING_SIZE, !!dev->tx_wdma[0].desc);
> > +             if (mtk_wed_wdma_tx_ring_setup(dev, 0, MTK_WED_WDMA_RING_SIZE, !!dev->tx_wdma[0].desc)) {
> > +                     dev_err(dev->hw->dev, "wdma tx ring 0 setup failed\n");
> > +                     return;
> > +             }
>
> [Severity: Medium]
> This is outside the lines changed here. It is the same ring setup
> error handling this patch is tightening, though, so it seems worth
> raising.
>
> In mtk_wed_rx_ring_setup(), mtk_wed_ring_alloc() first allocates the
> descriptor memory for dev->rx_ring[idx]. If the following
> mtk_wed_wdma_tx_ring_setup() call fails, the function returns -ENOMEM
> without setting MTK_WED_RING_CONFIGURED on that ring.
>
> mtk_wed_tx_ring_setup() has the same shape. dev->tx_ring[idx] is
> allocated, and a failure from mtk_wed_wdma_rx_ring_setup() returns
> without setting the flag.
>
> The teardown helpers only free rings that have
> MTK_WED_RING_CONFIGURED set. In both cases the coherent DMA buffer
> for the wlan-facing ring is therefore never released by
> mtk_wed_free_tx_rings() or mtk_wed_free_rx_rings(). If setup is
> later retried with reset == false, mtk_wed_ring_alloc() overwrites
> ring->desc and the old buffer is lost for good.
>
> mtk_wed_free_rx_rings() also looks like it tests the wrong array. It
> frees dev->rx_ring[i] based on dev->tx_ring[i].flags. Whether an rx
> ring is freed or skipped then depends on the tx ring with the same
> index, not on the rx ring itself. That makes the rx side leak more
> easily still.
>
> Should the error paths in mtk_wed_rx_ring_setup() and
> mtk_wed_tx_ring_setup() free the ring they just allocated? Or should
> the free helpers key off ring->desc instead of the flag? And should
> the check in mtk_wed_free_rx_rings() test dev->rx_ring[i].flags?
>
> [ ... ]
>
> [Severity: Critical]
> This isn't a bug introduced by this patch, but it sits in the same reset
> and start lifecycle. mtk_wed_rx_reset() does:
>
>         mtk_wed_free_rx_buffer(dev);
>         mtk_wed_hwrro_free_buffer(dev);
>
> When dev->rx_buf_ring.desc is non-NULL, mtk_wed_free_rx_buffer() already
> ends with its own call:
>
>         dev->wlan.release_rx_buf(dev);
>         dma_free_coherent(dev->hw->dev, dev->rx_buf_ring.size * sizeof(*desc),
>                           desc, dev->rx_buf_ring.desc_phys);
>
>         mtk_wed_hwrro_free_buffer(dev);
>
> mtk_wed_hwrro_free_buffer() frees everything but never clears
> dev->hw_rro.pages or dev->hw_rro.desc:
>
>         dma_free_coherent(dev->hw->dev, dev->hw_rro.size * sizeof(*desc),
>                           desc, dev->hw_rro.desc_phys);
>
> free_pagelist:
>         kfree(page_list);
>
> Does the second call in mtk_wed_rx_reset() then read page_list[] from
> freed memory? It would also __free_page() the hw_rro pages,
> dma_free_coherent() hw_rro.desc and kfree() the page list a second time.
>
> This looks reachable on every SER/DMA reset on WED v3 with wlan.hw_rro
> set, which mt7996 does for the primary WED:
>
> mt7996_dma_reset()
>   mtk_wed_device_dma_reset()
>     mtk_wed_reset_dma()
>       mtk_wed_rx_reset()
>         mtk_wed_free_rx_buffer()
>           mtk_wed_hwrro_free_buffer()
>         mtk_wed_hwrro_free_buffer()
>
> There is a second, separate issue: mtk_wed_free_rx_buffer() leaves
> dev->rx_buf_ring.desc pointing at freed memory. Suppose the next
> mtk_wed_start()->mtk_wed_rx_buffer_alloc() fails at dma_alloc_coherent()
> before that pointer is reassigned. Could a later
> __mtk_wed_detach()->mtk_wed_free_rx_rings()->mtk_wed_free_rx_buffer() then
> free it again?
>
> --
> Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261016-wedfix-v5-0-8b02e78fd34f%40gmail.com