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

[PATCH net] 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 WDMA_RING_TX(0) is not
setup and guard it with mtk_wed_is_v2.

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 | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/net/ethernet/mediatek/mtk_wed.c b/drivers/net/ethernet/mediatek/mtk_wed.c
index 10d9beaae372..2420557f6c96 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);
 
+	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);
+
 	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

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

[Paolo Abeni]

From: AI Reviewer <ai@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?

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

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