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

[Zhi-Jun You]

On Mon, Jul 6, 2026 at 6:10 PM Paolo Abeni <pabeni at redhat.com> wrote:
>
> From: AI Reviewer <ai at example.com>
>
> This is an AI-generated review of your patch. The human sending this
> email has considered the AI review valid, or at least plausible.
> Full review at: https://sashiko.dev/
> ---
> net:ethernet: mtk_wed: setup WDMA_RING_TX(0) for non-DBDC MT7986
>
> This commit updates mtk_wed_start() to configure WDMA_RING_TX(0) on
> non-DBDC MT7986 devices. It adds a call to mtk_wed_wdma_tx_ring_setup()
> for v2 hardware if the descriptor is not yet set up.
>
> > diff --git a/drivers/net/ethernet/mediatek/mtk_wed.c b/drivers/net/ethernet/mediatek/mtk_wed.c
> > index 10d9beaae3721..2420557f6c966 100644
> > --- a/drivers/net/ethernet/mediatek/mtk_wed.c
> > +++ b/drivers/net/ethernet/mediatek/mtk_wed.c
> > @@ -2334,6 +2334,10 @@ 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);
>
> [Severity: High]
> This isn't a bug introduced by this patch, but does this loop need to check
> the return value of mtk_wed_wdma_rx_ring_setup()?
>
> If mtk_wed_wdma_rx_ring_setup() returns -ENOMEM, the hardware registers are
> not programmed, but mtk_wed_start() continues execution and enables DMA.
>
> Could this lead to hardware DMA using an uninitialized ring base address?
Hi,

I am not sure how to proceed with this one.
mtk_wed_start is a void function and both mt7915 and mt7996's dma
function that calls it don't have return value handling either.

>
> >
> > +     if (mtk_wed_is_v2(dev->hw))
> > +             if (!dev->tx_wdma[0].desc)
> > +                     mtk_wed_wdma_tx_ring_setup(dev, 0, MTK_WED_WDMA_RING_SIZE, false);
>
> [Severity: High]
> Are there similar error handling concerns with this new call to
> mtk_wed_wdma_tx_ring_setup()?
>
> Like the RX path, if dma_alloc_coherent() fails inside this setup function,
> it returns -ENOMEM without programming the hardware registers.
>
> Since mtk_wed_start() doesn't check this return value, it will proceed to
> enable hardware DMA, which might attempt to use an uninitialized DMA address.
>
> [Severity: High]
> Does the check for !dev->tx_wdma[0].desc prevent necessary hardware
> initialization after a warm reset?
>
> When the WLAN driver triggers a hardware reset, the WED hardware registers
> are cleared, and mtk_wed_start() is called again.
>
> Because dev->tx_wdma[0].desc was allocated during the initial start, the
> guard will evaluate to false, skipping the hardware register reprogramming
> for tx_wdma[0].
>
> Will the hardware DMA end up using a zeroed ring base address after a reset?

During warm reset, MTK_WDMA_RING_TX(i) + MTK_WED_RING_OFS_CPU_IDX is set to 0.
Other parts are left untouched.
After the warm reset, mtk_wed_rx_ring_setup will be called with
reset=true, which also calls
mtk_wed_wdma_tx_ring_setup with reset=true.

reset=true will skip ring allocation and call an extra mtk_wed_ring_reset
but for non-DBDC mt7986, this path won't happen for the tx_wdma[0].

Usually rx_ring and tx_wdma are paired together but this is a special
case that rx_ring[1] and
tx_wdma[1] are allocated but tx_wdma[0] is still required.
Maybe something like this would work?
Runs only if wed is v2 and rx ring 0 is not allocated, reset only when
tx_wdma[0] was already allocated.

if (mtk_wed_is_v2 && !dev->rx_ring[0].desc)
        mtk_wed_wdma_tx_ring_setup(dev, 0, MTK_WED_WDMA_RING_SIZE,
!!dev->tx_wdma[0].desc);

Best regards,
Zhi-Jun

>
> > +
> >       if (dev->wlan.hw_rro) {
> >               for (i = 0; i < MTK_WED_RX_PAGE_QUEUES; i++) {
> >                       u32 addr = MTK_WED_RRO_MSDU_PG_CTRL0(i) +
> --
> This is an AI-generated review.
>