[PATCH] net: stmmac: enable MAC rx/tx after DMA start

[PATCH] net: stmmac: enable MAC rx/tx after DMA start

[Abid Ali via B4 Relay]

From: Abid Ali <dev.taqnialabs@gmail.com>

When the MAC receiver is enabled before the DMA channels are started,
incoming frames fill the MTL FIFO with no DMA engine to drain it.
If the PHY/Switch is already up at this point, and there is inflow
of packets towards EMAC, the per-queue FIFO overflows within this window.

Once the FIFO is full, the MTL read controller goes idle and never
retries delivery even if the DMA comes alive later on.
This behaviour happens due to enabling the MAC RX path before
the DMA setup is ready, and any delay between these events increases
the chances of blocking the rx path permanently.

The EMAC should only accept packets after the DMAs are enabled.

Signed-off-by: Abid Ali <dev.taqnialabs@gmail.com>
---
 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index 3591755ea..1213cb216 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -3692,9 +3692,6 @@ static int stmmac_hw_setup(struct net_device *dev)
 		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);
 
@@ -3750,6 +3747,9 @@ static int stmmac_hw_setup(struct net_device *dev)
 	/* Start the ball rolling... */
 	stmmac_start_all_dma(priv);
 
+	/* Enable the MAC Rx/Tx */
+	stmmac_mac_set(priv, priv->ioaddr, true);
+
 	phylink_rx_clk_stop_block(priv->phylink);
 	stmmac_set_hw_vlan_mode(priv, priv->hw);
 	phylink_rx_clk_stop_unblock(priv->phylink);
@@ -7189,12 +7189,12 @@ int stmmac_xdp_open(struct net_device *dev)
 		hrtimer_setup(&tx_q->txtimer, stmmac_tx_timer, CLOCK_MONOTONIC, HRTIMER_MODE_REL);
 	}
 
-	/* Enable the MAC Rx/Tx */
-	stmmac_mac_set(priv, priv->ioaddr, true);
-
 	/* Start Rx & Tx DMA Channels */
 	stmmac_start_all_dma(priv);
 
+	/* Enable the MAC Rx/Tx */
+	stmmac_mac_set(priv, priv->ioaddr, true);
+
 	ret = stmmac_request_irq(dev);
 	if (ret)
 		goto irq_error;

---
base-commit: 8cd9520d35a6c38db6567e97dd93b1f11f185dc6
change-id: 20260726-stmmac-rx-fifo-block-ef589c38f96b

Best regards,
-- 
Abid Ali <dev.taqnialabs@gmail.com>

[PATCH] net: stmmac: enable MAC rx/tx after DMA start

[Abid Ali]

When the MAC receiver is enabled before the DMA channels are started,
incoming frames fill the MTL FIFO with no DMA engine to drain it.
If the PHY/Switch is already up at this point, and there is inflow
of packets towards EMAC, the per-queue FIFO overflows within this window.

Once the FIFO is full, the MTL read controller goes idle and never
retries delivery even if the DMA comes alive later on.
This behaviour happens due to enabling the MAC RX path before
the DMA setup is ready, and any delay between these events increases
the chances of blocking the rx path permanently.

The EMAC should only accept packets after the DMAs are enabled.

Signed-off-by: Abid Ali <dev.taqnialabs@gmail.com>
---
 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index 3591755ea..1213cb216 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -3692,9 +3692,6 @@ static int stmmac_hw_setup(struct net_device *dev)
 		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);
 
@@ -3750,6 +3747,9 @@ static int stmmac_hw_setup(struct net_device *dev)
 	/* Start the ball rolling... */
 	stmmac_start_all_dma(priv);
 
+	/* Enable the MAC Rx/Tx */
+	stmmac_mac_set(priv, priv->ioaddr, true);
+
 	phylink_rx_clk_stop_block(priv->phylink);
 	stmmac_set_hw_vlan_mode(priv, priv->hw);
 	phylink_rx_clk_stop_unblock(priv->phylink);
@@ -7189,12 +7189,12 @@ int stmmac_xdp_open(struct net_device *dev)
 		hrtimer_setup(&tx_q->txtimer, stmmac_tx_timer, CLOCK_MONOTONIC, HRTIMER_MODE_REL);
 	}
 
-	/* Enable the MAC Rx/Tx */
-	stmmac_mac_set(priv, priv->ioaddr, true);
-
 	/* Start Rx & Tx DMA Channels */
 	stmmac_start_all_dma(priv);
 
+	/* Enable the MAC Rx/Tx */
+	stmmac_mac_set(priv, priv->ioaddr, true);
+
 	ret = stmmac_request_irq(dev);
 	if (ret)
 		goto irq_error;

---
base-commit: 8cd9520d35a6c38db6567e97dd93b1f11f185dc6
change-id: 20260726-stmmac-rx-fifo-block-ef589c38f96b

Best regards,
-- 
Abid Ali <dev.taqnialabs@gmail.com>

Re: [PATCH] net: stmmac: enable MAC rx/tx after DMA start

[Andrew Lunn]

> @@ -3692,9 +3692,6 @@ static int stmmac_hw_setup(struct net_device *dev)
>  		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);
>  
> @@ -3750,6 +3747,9 @@ static int stmmac_hw_setup(struct net_device *dev)
>  	/* Start the ball rolling... */
>  	stmmac_start_all_dma(priv);
>  
> +	/* Enable the MAC Rx/Tx */
> +	stmmac_mac_set(priv, priv->ioaddr, true);
> +

There should be a function which is the opposite of stmmac_hw_setup(),
which tears it down. What order are things done there? tear down is
generally done in the opposite order to setup, so does the "Disable
the MAC Rx/Tx" also need to move?

> @@ -7189,12 +7189,12 @@ int stmmac_xdp_open(struct net_device *dev)
>  		hrtimer_setup(&tx_q->txtimer, stmmac_tx_timer, CLOCK_MONOTONIC, HRTIMER_MODE_REL);
>  	}
>  
> -	/* Enable the MAC Rx/Tx */
> -	stmmac_mac_set(priv, priv->ioaddr, true);
> -
>  	/* Start Rx & Tx DMA Channels */
>  	stmmac_start_all_dma(priv);
>  
> +	/* Enable the MAC Rx/Tx */
> +	stmmac_mac_set(priv, priv->ioaddr, true);
> +
>  	ret = stmmac_request_irq(dev);
>  	if (ret)
>  		goto irq_error;

Same comment for the opposite of stmmac_xdp_open().

     Andrew

Re: [PATCH] net: stmmac: enable MAC rx/tx after DMA start

[Abid Ali]

On Sun, Jul 26, 2026 at 18:15:20 +0000, Andrew Lunn wrote:
> There should be a function which is the opposite of stmmac_hw_setup(),
> which tears it down. What order are things done there? tear down is
> generally done in the opposite order to setup, so does the "Disable
> the MAC Rx/Tx" also need to move?

I have not seen the FIFO stall in the teardown in my testing,
but the MAC Rx/Tx should be disabled before the DMA engine is stopped.
For the normal path (__stmmac_release), phylink_stop() is called first
which triggers stmmac_mac_link_down() -> stmmac_mac_set(false), so the
MAC is already disabled before stmmac_stop_all_dma(). The teardown
order is already correct there.

> Same comment for the opposite of stmmac_xdp_open().

Currently, stmmac_xdp_release() does:

    stmmac_stop_all_dma(priv);
    ...
    stmmac_mac_set(priv, priv->ioaddr, false);

The ordering is not followed here,  I'll make it follow the similar
ordering to (__stmmac_release) in v2.

Thanks,
Abid

Re: [PATCH] net: stmmac: enable MAC rx/tx after DMA start

[Andrew Lunn]

On Mon, Jul 27, 2026 at 05:57:15AM +0000, Abid Ali wrote:
> On Sun, Jul 26, 2026 at 18:15:20 +0000, Andrew Lunn wrote:
> > There should be a function which is the opposite of stmmac_hw_setup(),
> > which tears it down. What order are things done there? tear down is
> > generally done in the opposite order to setup, so does the "Disable
> > the MAC Rx/Tx" also need to move?
> 
> I have not seen the FIFO stall in the teardown in my testing,
> but the MAC Rx/Tx should be disabled before the DMA engine is stopped.
> For the normal path (__stmmac_release), phylink_stop() is called first
> which triggers stmmac_mac_link_down() -> stmmac_mac_set(false), so the
> MAC is already disabled before stmmac_stop_all_dma(). The teardown
> order is already correct there.

Thanks for checking. Please add a comment to the commit message about
this.

> 
> > Same comment for the opposite of stmmac_xdp_open().
> 
> Currently, stmmac_xdp_release() does:
> 
>     stmmac_stop_all_dma(priv);
>     ...
>     stmmac_mac_set(priv, priv->ioaddr, false);
> 
> The ordering is not followed here,  I'll make it follow the similar
> ordering to (__stmmac_release) in v2.

Great.


    Andrew

---
pw-bot: cr