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

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

[2694439648]

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

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

Signed-off-by: hailong.fan <hailong.fan@siengine.com>
---
 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

Re: [PATCH] net: stmmac: enable MAC after MTL configuring

[Simon Horman]

On Mon, Oct 14, 2024 at 01:44:03PM +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
> 
> Signed-off-by: hailong.fan <hailong.fan@siengine.com>

Hi,

I think that some more explanation of this is required.
Including if a problem has been observed, and if so under what
conditions. Or, if not, some background information on why
this adjustment is correct.

I also think some explanation is required of the relationship
between the changes this patch makes to setup, and the
changes it makes to start and stop.

...

回复: [PATCH] net: stmmac: enable MAC after MTL configuring

[Fan Hailong/范海龙]

Hi

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

stmmac_start_rx/tx and stmmac_stop_rx/tx should only handle the DMA module to avoid confusion



-----邮件原件-----
发件人: Simon Horman <horms@kernel.org> 
发送时间: 2024年10月17日 18:19
收件人: 2694439648@qq.com
抄送: alexandre.torgue@foss.st.com; joabreu@synopsys.com; davem@davemloft.net; edumazet@google.com; kuba@kernel.org; pabeni@redhat.com; mcoquelin.stm32@gmail.com; netdev@vger.kernel.org; linux-stm32@st-md-mailman.stormreply.com; linux-arm-kernel@lists.infradead.org; linux-kernel@vger.kernel.org; Fan Hailong/范海龙 <hailong.fan@siengine.com>
主题: Re: [PATCH] net: stmmac: enable MAC after MTL configuring

On Mon, Oct 14, 2024 at 01:44:03PM +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
> 
> Signed-off-by: hailong.fan <hailong.fan@siengine.com>

Hi,

I think that some more explanation of this is required.
Including if a problem has been observed, and if so under what conditions. Or, if not, some background information on why this adjustment is correct.

I also think some explanation is required of the relationship between the changes this patch makes to setup, and the changes it makes to start and stop.

...

Re: 回复: [PATCH] net: stmmac: enable MAC after MTL configuring

[Andrew Lunn]

On Fri, Oct 18, 2024 at 01:15:30AM +0000, Fan Hailong/范海龙 wrote:
> Hi
> 
> 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

What Simon is asking for is that this is part of the commit message.

Please also don't top post.

    Andrew

---
pw-bot: cr

回复: 回复: [PATCH] net: stmmac: enable MAC after MTL configuring

[Fan Hailong/范海龙]

[-- Attachment #1: Type: text/plain, Size: 1451 bytes --]

Hi 

Please find new patch in attachments, thanks.


-----邮件原件-----
发件人: Andrew Lunn <andrew@lunn.ch> 
发送时间: 2024年10月19日 1:57
收件人: Fan Hailong/范海龙 <hailong.fan@siengine.com>
抄送: Simon Horman <horms@kernel.org>; 2694439648@qq.com; alexandre.torgue@foss.st.com; joabreu@synopsys.com; davem@davemloft.net; edumazet@google.com; kuba@kernel.org; pabeni@redhat.com; mcoquelin.stm32@gmail.com; netdev@vger.kernel.org; linux-stm32@st-md-mailman.stormreply.com; linux-arm-kernel@lists.infradead.org; linux-kernel@vger.kernel.org
主题: Re: 回复: [PATCH] net: stmmac: enable MAC after MTL configuring

On Fri, Oct 18, 2024 at 01:15:30AM +0000, Fan Hailong/范海龙 wrote:
> Hi
> 
> 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

What Simon is asking for is that this is part of the commit message.

Please also don't top post.

    Andrew

---
pw-bot: cr

[-- Attachment #2: 0001-net-stmmac-enable-MAC-after-MTL-configuring.patch --]
[-- Type: application/octet-stream, Size: 4554 bytes --]

From 57c93463cf6be3fd030918d846ef3d65c754d201 Mon Sep 17 00:00:00 2001
From: "hailong.fan" <hailong.fan@siengine.com>
Date: Mon, 14 Oct 2024 11:19:16 +0800
Subject: [PATCH] net: stmmac: enable MAC after MTL configuring

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

Re: 回复: 回复: [PATCH] net: stmmac: enable MAC after MTL configuring

[Andrew Lunn]

On Sun, Oct 20, 2024 at 01:45:41AM +0000, Fan Hailong/范海龙 wrote:
> Hi 
> 
> Please find new patch in attachments, thanks.

Please read

https://docs.kernel.org/process/submitting-patches.html

https://www.kernel.org/doc/html/latest/process/maintainer-netdev.html

Attachments are not accepted.

	Andrew