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

[PATCH v2] 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.
Similarly, the teardown should ensure the MAC rx/tx is stopped before
DMA engine is stopped.

Signed-off-by: Abid Ali <dev.taqnialabs@gmail.com>
---
Changes in v2:
- Corrected the ordering in teardown for xdp at stmmac_xdp_release().
- Link to v1: https://lore.kernel.org/r/20260726-stmmac-rx-fifo-block-v1-1-45ab094b6c02@gmail.com
---
 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index 3591755ea..92864d7df 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);
@@ -7109,15 +7109,15 @@ void stmmac_xdp_release(struct net_device *dev)
 	/* Free the IRQ lines */
 	stmmac_free_irq(dev, REQ_IRQ_ERR_ALL, 0);
 
+	/* Disable the MAC Rx/Tx */
+	stmmac_mac_set(priv, priv->ioaddr, false);
+
 	/* Stop TX/RX DMA channels */
 	stmmac_stop_all_dma(priv);
 
 	/* Release and free the Rx/Tx resources */
 	free_dma_desc_resources(priv, &priv->dma_conf);
 
-	/* Disable the MAC Rx/Tx */
-	stmmac_mac_set(priv, priv->ioaddr, false);
-
 	/* set trans_start so we don't get spurious
 	 * watchdogs during reset
 	 */
@@ -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 v2] net: stmmac: enable MAC rx/tx after DMA start

[Maxime Chevallier]

Hi,

On 7/27/26 08:14, Abid Ali wrote:
> 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.
> Similarly, the teardown should ensure the MAC rx/tx is stopped before
> DMA engine is stopped.
> 
> Signed-off-by: Abid Ali <dev.taqnialabs@gmail.com>

Ah great ! This matches the programming model for the few ref manuals I
have, e.g. cycloneV manual does state that the MAC must be enabled after the
DMA has been configured.

Reviewed-by: Maxime Chevallier <maxime.chevallier@bootlin.com>

Maxime

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

[Jakub Kicinski]

On Mon, 27 Jul 2026 10:23:13 +0200 Maxime Chevallier wrote:
> On 7/27/26 08:14, Abid Ali wrote:
> > 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.
> > Similarly, the teardown should ensure the MAC rx/tx is stopped before
> > DMA engine is stopped.
> > 
> > Signed-off-by: Abid Ali <dev.taqnialabs@gmail.com>  
> 
> Ah great ! This matches the programming model for the few ref manuals I
> have, e.g. cycloneV manual does state that the MAC must be enabled after the
> DMA has been configured.

Sorry, me+Sashiko again :S

Sashiko wrote a damn essay, IDK if it makes sense.

Looking at the patch - isn't it a bit awkward to handle Rx and Tx
together? We want to keep Rx blocked to prevent incoming frames,
but Tx should be allowed so that Tx can egress?

Grepping around I'm not seeing any flushing of Tx. Aren't there
issues with this driver if we stop the interface while it was
paused by link partner?
-- 
pw-bot: cr

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

[Qingfang Deng]

Hi,

On 2026/7/27 14:14, Abid Ali wrote:
> The EMAC should only accept packets after the DMAs are enabled.
This is correct.
> Similarly, the teardown should ensure the MAC rx/tx is stopped before
> DMA engine is stopped.

This is _not_ correct. Instead it's a bit more complicated, according to 
their programming guidelines:

Stopping and Restarting Transmission

- Disable the Transmit DMA (if applicable) by clearing Bit 0 (ST) of the 
`DMA_CH(#i)_Tx_Control` register.
- Wait until the Transmit DMA generates the stopped interrupt (TPS field 
of the `DMA_CH(#i)_Status` register).
- Wait for any previous frame transmissions to complete. You can check 
this by reading the appropriate fields of the `MTL_TxQ0_Debug` register 
(`TRCSTS != 1` and `TXQSTS = 0`).
- Disable the MAC transmitter and MAC receiver by clearing Bit 0 (RE) of 
the `MAC_Rx_Configuration` and Bit 0 (TE) of the `MAC_Tx_Configuration` 
register.
- Disable the Receive DMA (if applicable), after ensuring the data in 
the Rx FIFO is transferred to the system memory (by reading the 
appropriate fields of the `MTL_RxQ0_Debug` register; `PRXQ = 0` and 
`RXQSTS = 0`).
- Wait until the Receive DMA generates the stopped interrupt, which is 
available in the RPS field of the `DMA_CH(#i)_Status` register.
- Restart the DMAs.
- Enable the MAC Transmitter and Receiver.

So the correct order is: stop TX DMA -> stop MAC RX/TX -> stop RX DMA.

Best regards,

Qingfang

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

[Abid Ali]

On Thu, 30 Jul 2026 16:07:15 -0700 Jakub Kicinski wrote:
> Looking at the patch - isn't it a bit awkward to handle Rx and Tx
> together? We want to keep Rx blocked to prevent incoming frames,
> but Tx should be allowed so that Tx can egress?

netif_tx_start_all_queues() runs after stmmac_mac_set() in both
__stmmac_open() and stmmac_xdp_open(), so no frame can be queued
in the window between DMA start and MAC enable. So I expect this
reorder to be fine and see no issue of coupling them together.

> Grepping around I'm not seeing any flushing of Tx. Aren't there
> issues with this driver if we stop the interface while it was
> paused by link partner?

I haven't run into these issues myself, but as Qingfang noted in
this thread the proper release sequence per the programming guide
needs more steps:

  stop TX DMA -> wait TPS (TX DMA stop confirmation) ->
  wait TX drain -> disable MAC RX/TX -> drain RX FIFO ->
  stop RX DMA -> wait RPS

This is a pre-existing gap in the driver, currently it just clears
the SR/ST bits for each channel and returns. Implementing the
proper sequence would need readl_poll_timeout on TPS/RPS status
bits per channel (up to 16 TX + 16 RX in my case), plus handling
timeouts if the status never updates in teardown.

The startup reorder is tested and verified to fix the RX hang I
have seen. Would it make sense to take this on its own (only the
startup reorder, with the teardown changes removed from v2) and
address the teardown at a later point as I see multiple open points
on what would be the right implementation to have there.

Thanks,
Abid

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

[Abid Ali]

On Fri, 31 Jul 2026 10:11:40 +0800 Qingfang Deng wrote:
> This is _not_ correct. Instead it's a bit more complicated, according to
> their programming guidelines:
>
> So the correct order is: stop TX DMA -> stop MAC RX/TX -> stop RX DMA.

You're right, the XGMAC TRM with me documents the same
sequence.

IMO this looks like a fix on its own as the current teardown logic
doesn't take care of any of this. The proper shutdown sequence is
a larger change that needs per-channel status polling and timeout
handling.

I'll remove the teardown change from v3 if that makes sense and keep
only the startup reorder addressing the RX hang.