* Re: [PATCH net v2] net:ethernet: mtk_wed: setup WDMA_RING_TX(0) for non-DBDC MT7986
2026-07-11 11:21 [PATCH net v2] net:ethernet: mtk_wed: setup WDMA_RING_TX(0) for non-DBDC MT7986 Zhi-Jun You
@ 2026-07-17 10:55 ` Simon Horman
0 siblings, 0 replies; 2+ messages in thread
From: Simon Horman @ 2026-07-17 10:55 UTC (permalink / raw)
To: hujy652
Cc: Simon Horman, nbd, lorenzo, andrew+netdev, davem, edumazet, kuba,
pabeni, matthias.bgg, angelogioacchino.delregno, sujuan.chen,
netdev, linux-kernel, linux-arm-kernel, linux-mediatek
This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
https://sashiko.dev/#/patchset/20260711112128.971-1-hujy652@gmail.com?part=1
---
--- Patch [1]: [PATCH net v2] net:ethernet: mtk_wed: setup WDMA_RING_TX(0) for non-DBDC MT7986 ---
commit d478e0562d6cafb9be212cb1356ac8344230386e
Author: Zhi-Jun You <hujy652@gmail.com>
net:ethernet: mtk_wed: setup WDMA_RING_TX(0) for non-DBDC MT7986
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. Fix this by calling
mtk_wed_wdma_tx_ring_setup if wed is v2 and rx_ring[0] is not allocated.
Link: https://lore.kernel.org/netdev/20260630144831.1109-1-hujy652@gmail.com/
> 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
> @@ -2334,6 +2334,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);
[Severity: High]
Is it safe to ignore the return value of mtk_wed_wdma_tx_ring_setup() here?
If mtk_wed_ring_alloc() fails under memory pressure, it returns -ENOMEM:
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;
...
}
Could this result in the hardware being enabled with uninitialized or
zeroed base addresses, potentially leading to DMA faults or system crashes?
Since mtk_wed_start() has the ability to return early on allocation
failure (as seen with mtk_wed_rx_buffer_alloc()), should it abort the
start sequence when this ring setup fails?
[Severity: High]
This isn't a bug introduced by this patch, but when this setup function
is called with reset evaluating to true on v3 hardware, does
mtk_wed_ring_reset() corrupt the v3-specific descriptor initialization?
In mtk_wed_wdma_tx_ring_setup(), v3 hardware initializes desc->info:
mtk_wed_wdma_tx_ring_setup() {
...
if (mtk_wed_is_v3_or_greater(dev->hw)) {
for (i = 0; i < MTK_WED_WDMA_RING_SIZE; i++) {
...
desc->info = cpu_to_le32(MTK_WDMA_TXD0_DESC_INFO_DMA_DONE);
...
}
}
...
if (reset)
mtk_wed_ring_reset(wdma, MTK_WED_WDMA_RING_SIZE, true);
...
}
However, mtk_wed_ring_reset() loops over the ring and unconditionally
clears desc->info:
mtk_wed_ring_reset() {
...
desc->info = 0;
...
}
Could this wipe out the configured v3-specific state and lead to a DMA
stall or faults on v3 hardware?
> +
> if (dev->wlan.hw_rro) {
> for (i = 0; i < MTK_WED_RX_PAGE_QUEUES; i++) {
> u32 addr = MTK_WED_RRO_MSDU_PG_CTRL0(i) +
^ permalink raw reply [flat|nested] 2+ messages in thread