diff mbox series

[v1] net: stmmac: enable MAC after MTL configuring

Message ID tencent_CCC29C4F562F2DEFE48289DB52F4D91BDE05@qq.com (mailing list archive)
State New, archived
Headers show
Series [v1] net: stmmac: enable MAC after MTL configuring | expand

Commit Message

2694439648@qq.com Oct. 21, 2024, 1:03 a.m. UTC
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(-)

Comments

Furong Xu Oct. 21, 2024, 5:05 a.m. UTC | #1
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()
Simon Horman Oct. 23, 2024, 8:49 a.m. UTC | #2
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.
diff mbox series

Patch

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;