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

Zhi-Jun You <[email protected]>
Newsgroups gmane.linux.network,gmane.linux.ports.arm.kernel,gmane.linux.kernel,gmane.linux.ports.arm.mediatek
Message-ID <[email protected]>
On Sat, Jul 25, 2026 at 01:20:39AM +0800, Zhi-Jun You wrote:
> 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.

Commenting on Sashiko's report:
https://sashiko.dev/#/patchset/20260724172040.1653-1-hujy652%40gmail.com

> 
> Fixes: e2f64db13aa1 ("net: ethernet: mtk_wed: introduce WED support for MT7988")
> Signed-off-by: Zhi-Jun You <[email protected]>
> ---
> Changes in v3:
> - new patch addressing WED v3 ring reset issue raised by Sashiko
> - Link to the review: https://lore.kernel.org/netdev/[email protected]/
> ---
>  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 10d9beaae372..7d2775a97fa0 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)  {

-This is a pre-existing issue, but while reviewing the WDMA ring reset logic,
 I noticed what looks like reversed logic in mtk_wdma_rx_reset():
 drivers/net/ethernet/mediatek/mtk_wed.c:mtk_wdma_rx_reset() {
 	...
 	for (i = 0; i < ARRAY_SIZE(dev->rx_wdma); i++) {
 		if (dev->rx_wdma[i].desc)
 			continue;
 		wdma_w32(dev,
 			 MTK_WDMA_RING_RX(i) + MTK_WED_RING_OFS_CPU_IDX, 0);
 	}
 	...
 }
 Does the if (dev->rx_wdma[i].desc) continue; check skip the CPU_IDX reset for
 active rings?
 When a WDMA RX reset occurs (e.g., during recovery from a hang), the hardware
 index is reset to 0, but the CPU_IDX register won't be reset for active rings
 since they have a non-NULL desc.
 Could this desynchronize the driver and hardware, leading to a permanent stall
 of the WDMA RX rings and network traffic failure?
 - ACK I will add a patch addressing this.

 Best regards,
 Zhi-Jun
> -- 
> 2.47.3
>
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.