netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v1] net: stmmac: enable MAC after MTL configuring
@ 2024-10-21  1:03 2694439648
  2024-10-21  5:05 ` Furong Xu
  0 siblings, 1 reply; 3+ messages in thread
From: 2694439648 @ 2024-10-21  1:03 UTC (permalink / raw)
  To: alexandre.torgue, joabreu, davem, edumazet, kuba, pabeni,
	mcoquelin.stm32
  Cc: netdev, linux-stm32, linux-arm-kernel, linux-kernel, hailong.fan

From: "hailong.fan" <hailong.fan@siengine.com>

DMA maybe block while ETH is opening,
Adjust the enable sequence, put the MAC enable last

For example, ETH is directly connected to the switch,
which never power down and sends broadcast packets at regular intervals.
During the process of opening ETH, data may flow into the MTL FIFO,
once MAC RX is enabled. and then, MTL will be set, such as FIFO size.
Once enable DMA, There is a certain probability that DMA will read
incorrect data from MTL FIFO, causing DMA to hang up.
By read DMA_Debug_Status, you can be observed that the RPS remains at
a certain value forever. The correct process should be to configure
MAC/MTL/DMA before enabling DMA/MAC

Signed-off-by: hailong.fan <hailong.fan@siengine.com>

---
V0-V1:
   1. update commit messages
---
 drivers/net/ethernet/stmicro/stmmac/dwmac4_lib.c   |  8 --------
 drivers/net/ethernet/stmicro/stmmac/dwxgmac2_dma.c | 12 ------------
 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c  |  6 +++---
 3 files changed, 3 insertions(+), 23 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac4_lib.c b/drivers/net/ethernet/stmicro/stmmac/dwmac4_lib.c
index 0d185e54e..92448d858 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac4_lib.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac4_lib.c
@@ -50,10 +50,6 @@ void dwmac4_dma_start_tx(struct stmmac_priv *priv, void __iomem *ioaddr,
 
 	value |= DMA_CONTROL_ST;
 	writel(value, ioaddr + DMA_CHAN_TX_CONTROL(dwmac4_addrs, chan));
-
-	value = readl(ioaddr + GMAC_CONFIG);
-	value |= GMAC_CONFIG_TE;
-	writel(value, ioaddr + GMAC_CONFIG);
 }
 
 void dwmac4_dma_stop_tx(struct stmmac_priv *priv, void __iomem *ioaddr,
@@ -77,10 +73,6 @@ void dwmac4_dma_start_rx(struct stmmac_priv *priv, void __iomem *ioaddr,
 	value |= DMA_CONTROL_SR;
 
 	writel(value, ioaddr + DMA_CHAN_RX_CONTROL(dwmac4_addrs, chan));
-
-	value = readl(ioaddr + GMAC_CONFIG);
-	value |= GMAC_CONFIG_RE;
-	writel(value, ioaddr + GMAC_CONFIG);
 }
 
 void dwmac4_dma_stop_rx(struct stmmac_priv *priv, void __iomem *ioaddr,
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_dma.c b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_dma.c
index 7840bc403..cba12edc1 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_dma.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_dma.c
@@ -288,10 +288,6 @@ static void dwxgmac2_dma_start_tx(struct stmmac_priv *priv,
 	value = readl(ioaddr + XGMAC_DMA_CH_TX_CONTROL(chan));
 	value |= XGMAC_TXST;
 	writel(value, ioaddr + XGMAC_DMA_CH_TX_CONTROL(chan));
-
-	value = readl(ioaddr + XGMAC_TX_CONFIG);
-	value |= XGMAC_CONFIG_TE;
-	writel(value, ioaddr + XGMAC_TX_CONFIG);
 }
 
 static void dwxgmac2_dma_stop_tx(struct stmmac_priv *priv, void __iomem *ioaddr,
@@ -302,10 +298,6 @@ static void dwxgmac2_dma_stop_tx(struct stmmac_priv *priv, void __iomem *ioaddr,
 	value = readl(ioaddr + XGMAC_DMA_CH_TX_CONTROL(chan));
 	value &= ~XGMAC_TXST;
 	writel(value, ioaddr + XGMAC_DMA_CH_TX_CONTROL(chan));
-
-	value = readl(ioaddr + XGMAC_TX_CONFIG);
-	value &= ~XGMAC_CONFIG_TE;
-	writel(value, ioaddr + XGMAC_TX_CONFIG);
 }
 
 static void dwxgmac2_dma_start_rx(struct stmmac_priv *priv,
@@ -316,10 +308,6 @@ static void dwxgmac2_dma_start_rx(struct stmmac_priv *priv,
 	value = readl(ioaddr + XGMAC_DMA_CH_RX_CONTROL(chan));
 	value |= XGMAC_RXST;
 	writel(value, ioaddr + XGMAC_DMA_CH_RX_CONTROL(chan));
-
-	value = readl(ioaddr + XGMAC_RX_CONFIG);
-	value |= XGMAC_CONFIG_RE;
-	writel(value, ioaddr + XGMAC_RX_CONFIG);
 }
 
 static void dwxgmac2_dma_stop_rx(struct stmmac_priv *priv, void __iomem *ioaddr,
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index e21404822..c19ca62a4 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -3437,9 +3437,6 @@ static int stmmac_hw_setup(struct net_device *dev, bool ptp_register)
 		priv->hw->rx_csum = 0;
 	}
 
-	/* Enable the MAC Rx/Tx */
-	stmmac_mac_set(priv, priv->ioaddr, true);
-
 	/* Set the HW DMA mode and the COE */
 	stmmac_dma_operation_mode(priv);
 
@@ -3523,6 +3520,9 @@ static int stmmac_hw_setup(struct net_device *dev, bool ptp_register)
 	/* Start the ball rolling... */
 	stmmac_start_all_dma(priv);
 
+	/* Enable the MAC Rx/Tx */
+	stmmac_mac_set(priv, priv->ioaddr, true);
+
 	stmmac_set_hw_vlan_mode(priv, priv->hw);
 
 	return 0;
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH v1] net: stmmac: enable MAC after MTL configuring
  2024-10-21  1:03 [PATCH v1] net: stmmac: enable MAC after MTL configuring 2694439648
@ 2024-10-21  5:05 ` Furong Xu
  2024-10-23  8:49   ` Simon Horman
  0 siblings, 1 reply; 3+ messages in thread
From: Furong Xu @ 2024-10-21  5:05 UTC (permalink / raw)
  To: 2694439648
  Cc: alexandre.torgue, joabreu, davem, edumazet, kuba, pabeni,
	mcoquelin.stm32, netdev, linux-stm32, linux-arm-kernel,
	linux-kernel, hailong.fan

On Mon, 21 Oct 2024 09:03:05 +0800, 2694439648@qq.com wrote:

> From: "hailong.fan" <hailong.fan@siengine.com>
> 
> DMA maybe block while ETH is opening,
> Adjust the enable sequence, put the MAC enable last
> 
> For example, ETH is directly connected to the switch,
> which never power down and sends broadcast packets at regular intervals.
> During the process of opening ETH, data may flow into the MTL FIFO,
> once MAC RX is enabled. and then, MTL will be set, such as FIFO size.
> Once enable DMA, There is a certain probability that DMA will read
> incorrect data from MTL FIFO, causing DMA to hang up.
> By read DMA_Debug_Status, you can be observed that the RPS remains at
> a certain value forever. The correct process should be to configure
> MAC/MTL/DMA before enabling DMA/MAC
> 
> Signed-off-by: hailong.fan <hailong.fan@siengine.com>
> 

A Fixes: tag should be added.

>  static void dwxgmac2_dma_stop_rx(struct stmmac_priv *priv, void __iomem *ioaddr,
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> index e21404822..c19ca62a4 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> @@ -3437,9 +3437,6 @@ static int stmmac_hw_setup(struct net_device *dev, bool ptp_register)
>  		priv->hw->rx_csum = 0;
>  	}
>  
> -	/* Enable the MAC Rx/Tx */
> -	stmmac_mac_set(priv, priv->ioaddr, true);
> -
>  	/* Set the HW DMA mode and the COE */
>  	stmmac_dma_operation_mode(priv);
>  
> @@ -3523,6 +3520,9 @@ static int stmmac_hw_setup(struct net_device *dev, bool ptp_register)
>  	/* Start the ball rolling... */
>  	stmmac_start_all_dma(priv);
>  
> +	/* Enable the MAC Rx/Tx */
> +	stmmac_mac_set(priv, priv->ioaddr, true);
> +

This sequence fix should be applied to stmmac_xdp_open() too.

>  	stmmac_set_hw_vlan_mode(priv, priv->hw);
>  
>  	return 0;

It is better to split this patch into individual patches, since you are
trying to fix an issue related to several previous commits:
dwmac4, dwxgmac2, stmmac_hw_setup() and stmmac_xdp_open()

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH v1] net: stmmac: enable MAC after MTL configuring
  2024-10-21  5:05 ` Furong Xu
@ 2024-10-23  8:49   ` Simon Horman
  0 siblings, 0 replies; 3+ messages in thread
From: Simon Horman @ 2024-10-23  8:49 UTC (permalink / raw)
  To: Furong Xu
  Cc: 2694439648, alexandre.torgue, joabreu, davem, edumazet, kuba,
	pabeni, mcoquelin.stm32, netdev, linux-stm32, linux-arm-kernel,
	linux-kernel, hailong.fan

On Mon, Oct 21, 2024 at 01:05:54PM +0800, Furong Xu wrote:
> On Mon, 21 Oct 2024 09:03:05 +0800, 2694439648@qq.com wrote:
> 
> > From: "hailong.fan" <hailong.fan@siengine.com>
> > 
> > DMA maybe block while ETH is opening,
> > Adjust the enable sequence, put the MAC enable last
> > 
> > For example, ETH is directly connected to the switch,
> > which never power down and sends broadcast packets at regular intervals.
> > During the process of opening ETH, data may flow into the MTL FIFO,
> > once MAC RX is enabled. and then, MTL will be set, such as FIFO size.
> > Once enable DMA, There is a certain probability that DMA will read
> > incorrect data from MTL FIFO, causing DMA to hang up.
> > By read DMA_Debug_Status, you can be observed that the RPS remains at
> > a certain value forever. The correct process should be to configure
> > MAC/MTL/DMA before enabling DMA/MAC
> > 
> > Signed-off-by: hailong.fan <hailong.fan@siengine.com>
> > 
> 
> A Fixes: tag should be added.

Also, as this patch is a fix for net, that target should be noted in the
subject.

  Subject: [PATCH v2 net] ...

Please address this and the issues raised by Furong Xu and post a v3.

> >  static void dwxgmac2_dma_stop_rx(struct stmmac_priv *priv, void __iomem *ioaddr,
> > diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> > index e21404822..c19ca62a4 100644
> > --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> > +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> > @@ -3437,9 +3437,6 @@ static int stmmac_hw_setup(struct net_device *dev, bool ptp_register)
> >  		priv->hw->rx_csum = 0;
> >  	}
> >  
> > -	/* Enable the MAC Rx/Tx */
> > -	stmmac_mac_set(priv, priv->ioaddr, true);
> > -
> >  	/* Set the HW DMA mode and the COE */
> >  	stmmac_dma_operation_mode(priv);
> >  
> > @@ -3523,6 +3520,9 @@ static int stmmac_hw_setup(struct net_device *dev, bool ptp_register)
> >  	/* Start the ball rolling... */
> >  	stmmac_start_all_dma(priv);
> >  
> > +	/* Enable the MAC Rx/Tx */
> > +	stmmac_mac_set(priv, priv->ioaddr, true);
> > +
> 
> This sequence fix should be applied to stmmac_xdp_open() too.
> 
> >  	stmmac_set_hw_vlan_mode(priv, priv->hw);
> >  
> >  	return 0;
> 
> It is better to split this patch into individual patches, since you are
> trying to fix an issue related to several previous commits:
> dwmac4, dwxgmac2, stmmac_hw_setup() and stmmac_xdp_open()

And each patch should have an appropriate Fixes tag.

-- 
pw-bot: changes-requested

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2024-10-23  8:49 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-10-21  1:03 [PATCH v1] net: stmmac: enable MAC after MTL configuring 2694439648
2024-10-21  5:05 ` Furong Xu
2024-10-23  8:49   ` Simon Horman

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).