diff mbox series

[net-next,1/5] net: ethernet: mtk_wed: return status value in mtk_wdma_rx_reset

Message ID 8917d87eded7142a3a792c3ba64434a983278247.1669020847.git.lorenzo@kernel.org (mailing list archive)
State Superseded
Delegated to: Netdev Maintainers
Headers show
Series refactor mtk_wed code to introduce SER support | expand

Checks

Context Check Description
netdev/tree_selection success Clearly marked for net-next
netdev/fixes_present success Fixes tag not required for -next series
netdev/subject_prefix success Link
netdev/cover_letter success Series has a cover letter
netdev/patch_count success Link
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 0 this patch: 0
netdev/cc_maintainers warning 2 maintainers not CCed: linux-arm-kernel@lists.infradead.org matthias.bgg@gmail.com
netdev/build_clang success Errors and warnings before: 0 this patch: 0
netdev/module_param success Was 0 now: 0
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 0 this patch: 0
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 62 lines checked
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

Lorenzo Bianconi Nov. 21, 2022, 8:59 a.m. UTC 
Move MTK_WDMA_RESET_IDX configuration in mtk_wdma_rx_reset routine.
This is a preliminary patch to add Wireless Ethernet Dispatcher reset
support.

Co-developed-by: Sujuan Chen <sujuan.chen@mediatek.com>
Signed-off-by: Sujuan Chen <sujuan.chen@mediatek.com>
Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
---
 drivers/net/ethernet/mediatek/mtk_wed.c | 25 ++++++++++++-------------
 1 file changed, 12 insertions(+), 13 deletions(-)

Comments

Alexander Lobakin Nov. 21, 2022, 4:55 p.m. UTC | #1 
From: Lorenzo Bianconi <lorenzo@kernel.org>
Date: Mon, 21 Nov 2022 09:59:21 +0100

> Move MTK_WDMA_RESET_IDX configuration in mtk_wdma_rx_reset routine.
> This is a preliminary patch to add Wireless Ethernet Dispatcher reset
> support.
> 
> Co-developed-by: Sujuan Chen <sujuan.chen@mediatek.com>
> Signed-off-by: Sujuan Chen <sujuan.chen@mediatek.com>
> Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
> ---
>  drivers/net/ethernet/mediatek/mtk_wed.c | 25 ++++++++++++-------------
>  1 file changed, 12 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/net/ethernet/mediatek/mtk_wed.c b/drivers/net/ethernet/mediatek/mtk_wed.c
> index 7d8842378c2b..dc898ded2f05 100644
> --- a/drivers/net/ethernet/mediatek/mtk_wed.c
> +++ b/drivers/net/ethernet/mediatek/mtk_wed.c
> @@ -101,17 +101,21 @@ mtk_wdma_read_reset(struct mtk_wed_device *dev)
>  	return wdma_r32(dev, MTK_WDMA_GLO_CFG);
>  }
>  
> -static void
> +static int
>  mtk_wdma_rx_reset(struct mtk_wed_device *dev)
>  {
>  	u32 status, mask = MTK_WDMA_GLO_CFG_RX_DMA_BUSY;
> -	int i;
> +	int i, ret;
>  
>  	wdma_clr(dev, MTK_WDMA_GLO_CFG, MTK_WDMA_GLO_CFG_RX_DMA_EN);
> -	if (readx_poll_timeout(mtk_wdma_read_reset, dev, status,
> -			       !(status & mask), 0, 1000))
> +	ret = readx_poll_timeout(mtk_wdma_read_reset, dev, status,
> +				 !(status & mask), 0, 10000);

You didn't mention anywhere this change of the timeout from 1000 to
10000, and for example for me it's not clear from the code why you
did this. Maybe leave a comment in the commitmsg?
Same in 2/5 for Tx, also 1000 -> 10000.

> +	if (ret)
>  		dev_err(dev->hw->dev, "rx reset failed\n");
>  
> +	wdma_w32(dev, MTK_WDMA_RESET_IDX, MTK_WDMA_RESET_IDX_RX);
> +	wdma_w32(dev, MTK_WDMA_RESET_IDX, 0);
> +
>  	for (i = 0; i < ARRAY_SIZE(dev->rx_wdma); i++) {
>  		if (dev->rx_wdma[i].desc)
>  			continue;

[...]

> -- 
> 2.38.1

Thanks,
Olek
Lorenzo Bianconi Nov. 21, 2022, 5:34 p.m. UTC | #2 
> 
> From: Lorenzo Bianconi <lorenzo@kernel.org>
> Date: Mon, 21 Nov 2022 09:59:21 +0100
> 
> > Move MTK_WDMA_RESET_IDX configuration in mtk_wdma_rx_reset routine.
> > This is a preliminary patch to add Wireless Ethernet Dispatcher reset
> > support.
> > 
> > Co-developed-by: Sujuan Chen <sujuan.chen@mediatek.com>
> > Signed-off-by: Sujuan Chen <sujuan.chen@mediatek.com>
> > Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
> > ---
> >  drivers/net/ethernet/mediatek/mtk_wed.c | 25 ++++++++++++-------------
> >  1 file changed, 12 insertions(+), 13 deletions(-)
> > 
> > diff --git a/drivers/net/ethernet/mediatek/mtk_wed.c b/drivers/net/ethernet/mediatek/mtk_wed.c
> > index 7d8842378c2b..dc898ded2f05 100644
> > --- a/drivers/net/ethernet/mediatek/mtk_wed.c
> > +++ b/drivers/net/ethernet/mediatek/mtk_wed.c
> > @@ -101,17 +101,21 @@ mtk_wdma_read_reset(struct mtk_wed_device *dev)
> >  	return wdma_r32(dev, MTK_WDMA_GLO_CFG);
> >  }
> >  
> > -static void
> > +static int
> >  mtk_wdma_rx_reset(struct mtk_wed_device *dev)
> >  {
> >  	u32 status, mask = MTK_WDMA_GLO_CFG_RX_DMA_BUSY;
> > -	int i;
> > +	int i, ret;
> >  
> >  	wdma_clr(dev, MTK_WDMA_GLO_CFG, MTK_WDMA_GLO_CFG_RX_DMA_EN);
> > -	if (readx_poll_timeout(mtk_wdma_read_reset, dev, status,
> > -			       !(status & mask), 0, 1000))
> > +	ret = readx_poll_timeout(mtk_wdma_read_reset, dev, status,
> > +				 !(status & mask), 0, 10000);
> 
> You didn't mention anywhere this change of the timeout from 1000 to
> 10000, and for example for me it's not clear from the code why you
> did this. Maybe leave a comment in the commitmsg?
> Same in 2/5 for Tx, also 1000 -> 10000.

ops I forgot them, sorry. I actually aligned the these values to the vendor sdk.

Regards,
Lorenzo

> 
> > +	if (ret)
> >  		dev_err(dev->hw->dev, "rx reset failed\n");
> >  
> > +	wdma_w32(dev, MTK_WDMA_RESET_IDX, MTK_WDMA_RESET_IDX_RX);
> > +	wdma_w32(dev, MTK_WDMA_RESET_IDX, 0);
> > +
> >  	for (i = 0; i < ARRAY_SIZE(dev->rx_wdma); i++) {
> >  		if (dev->rx_wdma[i].desc)
> >  			continue;
> 
> [...]
> 
> > -- 
> > 2.38.1
> 
> Thanks,
> Olek
diff mbox series

Patch

diff --git a/drivers/net/ethernet/mediatek/mtk_wed.c b/drivers/net/ethernet/mediatek/mtk_wed.c
index 7d8842378c2b..dc898ded2f05 100644
--- a/drivers/net/ethernet/mediatek/mtk_wed.c
+++ b/drivers/net/ethernet/mediatek/mtk_wed.c
@@ -101,17 +101,21 @@  mtk_wdma_read_reset(struct mtk_wed_device *dev)
 	return wdma_r32(dev, MTK_WDMA_GLO_CFG);
 }
 
-static void
+static int
 mtk_wdma_rx_reset(struct mtk_wed_device *dev)
 {
 	u32 status, mask = MTK_WDMA_GLO_CFG_RX_DMA_BUSY;
-	int i;
+	int i, ret;
 
 	wdma_clr(dev, MTK_WDMA_GLO_CFG, MTK_WDMA_GLO_CFG_RX_DMA_EN);
-	if (readx_poll_timeout(mtk_wdma_read_reset, dev, status,
-			       !(status & mask), 0, 1000))
+	ret = readx_poll_timeout(mtk_wdma_read_reset, dev, status,
+				 !(status & mask), 0, 10000);
+	if (ret)
 		dev_err(dev->hw->dev, "rx reset failed\n");
 
+	wdma_w32(dev, MTK_WDMA_RESET_IDX, MTK_WDMA_RESET_IDX_RX);
+	wdma_w32(dev, MTK_WDMA_RESET_IDX, 0);
+
 	for (i = 0; i < ARRAY_SIZE(dev->rx_wdma); i++) {
 		if (dev->rx_wdma[i].desc)
 			continue;
@@ -119,6 +123,8 @@  mtk_wdma_rx_reset(struct mtk_wed_device *dev)
 		wdma_w32(dev,
 			 MTK_WDMA_RING_RX(i) + MTK_WED_RING_OFS_CPU_IDX, 0);
 	}
+
+	return ret;
 }
 
 static void
@@ -565,9 +571,7 @@  mtk_wed_detach(struct mtk_wed_device *dev)
 
 	mtk_wed_stop(dev);
 
-	wdma_w32(dev, MTK_WDMA_RESET_IDX, MTK_WDMA_RESET_IDX_RX);
-	wdma_w32(dev, MTK_WDMA_RESET_IDX, 0);
-
+	mtk_wdma_rx_reset(dev);
 	mtk_wed_reset(dev, MTK_WED_RESET_WED);
 	if (mtk_wed_get_rx_capa(dev)) {
 		wdma_clr(dev, MTK_WDMA_GLO_CFG, MTK_WDMA_GLO_CFG_TX_DMA_EN);
@@ -582,7 +586,6 @@  mtk_wed_detach(struct mtk_wed_device *dev)
 		mtk_wed_wo_reset(dev);
 		mtk_wed_free_rx_rings(dev);
 		mtk_wed_wo_deinit(hw);
-		mtk_wdma_rx_reset(dev);
 	}
 
 	if (dev->wlan.bus_type == MTK_WED_BUS_PCIE) {
@@ -999,11 +1002,7 @@  mtk_wed_reset_dma(struct mtk_wed_device *dev)
 		wed_w32(dev, MTK_WED_RESET_IDX, 0);
 	}
 
-	wdma_w32(dev, MTK_WDMA_RESET_IDX, MTK_WDMA_RESET_IDX_RX);
-	wdma_w32(dev, MTK_WDMA_RESET_IDX, 0);
-
-	if (mtk_wed_get_rx_capa(dev))
-		mtk_wdma_rx_reset(dev);
+	mtk_wdma_rx_reset(dev);
 
 	if (busy) {
 		mtk_wed_reset(dev, MTK_WED_RESET_WDMA_INT_AGENT);