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

[Simon Horman <horms@kernel.org>]

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) +