Linux-ARM-Kernel Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2] net: stmmac: enable MAC rx/tx after DMA start
@ 2026-07-27  6:14 Abid Ali
  2026-07-27  8:23 ` Maxime Chevallier
  2026-07-31  2:11 ` Qingfang Deng
  0 siblings, 2 replies; 4+ messages in thread
From: Abid Ali @ 2026-07-27  6:14 UTC (permalink / raw)
  To: Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, Maxime Coquelin, Alexandre Torgue
  Cc: netdev, linux-stm32, linux-arm-kernel, linux-kernel, 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>



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

* Re: [PATCH v2] net: stmmac: enable MAC rx/tx after DMA start
  2026-07-27  6:14 [PATCH v2] net: stmmac: enable MAC rx/tx after DMA start Abid Ali
@ 2026-07-27  8:23 ` Maxime Chevallier
  2026-07-30 23:07   ` Jakub Kicinski
  2026-07-31  2:11 ` Qingfang Deng
  1 sibling, 1 reply; 4+ messages in thread
From: Maxime Chevallier @ 2026-07-27  8:23 UTC (permalink / raw)
  To: Abid Ali, Andrew Lunn, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, Maxime Coquelin, Alexandre Torgue
  Cc: netdev, linux-stm32, linux-arm-kernel, linux-kernel

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



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

* Re: [PATCH v2] net: stmmac: enable MAC rx/tx after DMA start
  2026-07-27  8:23 ` Maxime Chevallier
@ 2026-07-30 23:07   ` Jakub Kicinski
  0 siblings, 0 replies; 4+ messages in thread
From: Jakub Kicinski @ 2026-07-30 23:07 UTC (permalink / raw)
  To: Maxime Chevallier
  Cc: Abid Ali, Andrew Lunn, David S. Miller, Eric Dumazet, Paolo Abeni,
	Maxime Coquelin, Alexandre Torgue, netdev, linux-stm32,
	linux-arm-kernel, linux-kernel

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


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

* Re: [PATCH v2] net: stmmac: enable MAC rx/tx after DMA start
  2026-07-27  6:14 [PATCH v2] net: stmmac: enable MAC rx/tx after DMA start Abid Ali
  2026-07-27  8:23 ` Maxime Chevallier
@ 2026-07-31  2:11 ` Qingfang Deng
  1 sibling, 0 replies; 4+ messages in thread
From: Qingfang Deng @ 2026-07-31  2:11 UTC (permalink / raw)
  To: Abid Ali, Andrew Lunn, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, Maxime Coquelin, Alexandre Torgue
  Cc: netdev, linux-stm32, linux-arm-kernel, linux-kernel

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



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

end of thread, other threads:[~2026-07-31  2:12 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-27  6:14 [PATCH v2] net: stmmac: enable MAC rx/tx after DMA start Abid Ali
2026-07-27  8:23 ` Maxime Chevallier
2026-07-30 23:07   ` Jakub Kicinski
2026-07-31  2:11 ` Qingfang Deng

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox