[PATCH net-next v4 0/3] fix the MTL fifo and DMA race condition

[PATCH net-next v4 0/3] fix the MTL fifo and DMA race condition

[Abid Ali]

To: Andrew Lunn <andrew+netdev@lunn.ch>
To: David S. Miller <davem@davemloft.net>
To: Eric Dumazet <edumazet@google.com>
To: Jakub Kicinski <kuba@kernel.org>
To: Paolo Abeni <pabeni@redhat.com>
To: Maxime Coquelin <mcoquelin.stm32@gmail.com>
To: Alexandre Torgue <alexandre.torgue@foss.st.com>
To: Russell King <linux@armlinux.org.uk>
To: Qingfang Deng <qingfang.deng@linux.dev>
To: Maxime Chevallier <maxime.chevallier@bootlin.com>
Cc: netdev@vger.kernel.org
Cc: linux-stm32@st-md-mailman.stormreply.com
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-kernel@vger.kernel.org
Signed-off-by: Abid Ali <dev.taqnialabs@gmail.com>

Changes in v4:
- Rebased on net-next.
- Link to v3: https://lore.kernel.org/r/20260813-stmmac-rx-fifo-block-v3-0-738acbee55d9@gmail.com

Changes in v3:
- v2 tried to fix the ordering in teardown path, but it still would
  not follow the ideal flow mentioned in the databook. So have
  seperated the patches to deal with the teardown and some findings
  from sashiko.

- "net: stmmac: enable MAC rx/tx after DMA start": reduced in scope,
  now only moves the MAC enable after stmmac_start_all_dma() in
  stmmac_hw_setup() and stmmac_xdp_open().
  This is to handle the overflow scenario during bootup causing hang.
  The teardown side in __stmmac_release() is no longer touched here.
  This change could go independently fix the overflow hang at bootup.

- Added "net: stmmac: xgmac: tear the datapath down in the documented
  order": for ensuring ordered sequence is followed for teardown.

- Added "net: stmmac: xgmac: decouple the MAC Rx/Tx enables from the
  DMA ops": drops the global TE/RE writes from the per-channel XGMAC
  DMA start/stop ops.

- Link to v2: https://lore.kernel.org/r/20260727-stmmac-rx-fifo-block-v2-1-4dc3457d31d0@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

---
Abid Ali (3):
      net: stmmac: enable MAC rx/tx after DMA start
      net: stmmac: xgmac: tear the datapath down in the documented order
      net: stmmac: xgmac: decouple the MAC Rx/Tx enables from the DMA ops

 drivers/net/ethernet/stmicro/stmmac/dwxgmac2.h     |  8 ++
 drivers/net/ethernet/stmicro/stmmac/dwxgmac2_dma.c | 94 +++++++++++++++++++---
 drivers/net/ethernet/stmicro/stmmac/hwif.h         | 17 ++++
 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c  | 85 ++++++++++++++-----
 4 files changed, 170 insertions(+), 34 deletions(-)
---
base-commit: 4f93b12cf7b25fbf8e73d222722805b049f0a6d3
change-id: 20260726-stmmac-rx-fifo-block-ef589c38f96b

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

[PATCH net-next v4 1/3] 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 b2b7d0242dd3..b9d5d4aabf42 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -3685,9 +3685,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);
 
@@ -3743,6 +3740,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);
@@ -7184,12 +7184,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;

-- 
2.43.0

Re: [PATCH net-next v4 1/3] net: stmmac: enable MAC rx/tx after DMA start

[Paolo Abeni]

This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
Full review at: https://netdev-ai.bots.linux.dev/sashiko/
---
net: stmmac: enable MAC rx/tx after DMA start

This moves the stmmac_mac_set(priv, priv->ioaddr, true) call in
stmmac_hw_setup() and stmmac_xdp_open() so that it runs after
stmmac_start_all_dma() instead of before it. The stated goal is that the
MAC only starts accepting frames once the DMA channels can drain the MTL
FIFOs.

A few questions below.

The commit body describes a concrete hardware failure ("the per-queue FIFO
overflows within this window", "never retries delivery even if the DMA
comes alive later on"), so this reads as a bug fix rather than a cleanup,
but the only trailer present is:

> Signed-off-by: Abid Ali <dev.taqnialabs@gmail.com>

[Severity: Low]
Could a Fixes: tag be added for each of the two sequences being corrected,
one for the ordering in stmmac_hw_setup() and one for the same ordering in
stmmac_xdp_open() which arrived with the AF_XDP path?

Without a Fixes: trailer, how do stable/backport tools determine the
affected release range, and is this intended for net or net-next?

Would it also help to mention the core/platform on which the stall was
reproduced and how the fix was validated, plus any Reported-by:/Closes:
reference if one exists?

> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> index b2b7d0242dd3c..b9d5d4aabf423 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> @@ -3685,9 +3685,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);
>  
> @@ -3743,6 +3740,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);
> +

[Severity: Medium]
Does this reordering actually close the window on the dwmac4/dwmac5
(GMAC4+) cores?

There the MAC receiver and transmitter enables are set from inside the
per-channel DMA start callbacks:

drivers/net/ethernet/stmicro/stmmac/dwmac4_lib.c:dwmac4_dma_start_rx() {
	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);
}

and dwmac4_dma_start_tx() does the same for GMAC_CONFIG_TE.

stmmac_start_all_dma() starts the RX channels one at a time:

	for (chan = 0; chan < rx_channels_count; chan++)
		stmmac_start_rx_dma(priv, chan);

so the global MAC receiver is already enabled by the first iteration while
channels 1..n-1 still have SR == 0. On those cores, isn't the relocated
stmmac_mac_set(priv, priv->ioaddr, true) redundant, and doesn't the
"MAC classifies frames into an MTL queue whose DMA is not running"
condition still exist whenever rx_queues_to_use > 1?

The RE-in-DMA-op coupling itself isn't introduced by this patch, but should
the commit message claim "The EMAC should only accept packets after the DMAs
are enabled" be qualified, or should the dwmac4 coupling be decoupled too?

[Severity: Low]
This isn't a bug introduced by this patch, but the mirror-image teardown
ordering used to be the reverse of what this patch establishes, i.e.
stopping the DMA before disabling the MAC Rx in __stmmac_release(),
stmmac_xdp_release() and stmmac_suspend(). The follow-on commit "net:
stmmac: xgmac: tear the datapath down in the documented order" introduces
stmmac_datapath_teardown() and converts those callers, so the MAC is
disabled before stmmac_stop_rx_dma() runs:

	/* Disable the MAC TX and RX */
	stmmac_mac_set(priv, priv->ioaddr, false);

	/* Wait for the MTL RX queues to drain into the RX DMA */
	...
	for (chan = 0; chan < rx_channels_count; chan++)
		stmmac_stop_rx_dma(priv, chan);

Noting it here only for completeness, since the remaining "MAC Rx on, DMA
off" case is the WoL branch of stmmac_suspend(), which that commit
documents as intentional.

>  	phylink_rx_clk_stop_block(priv->phylink);
>  	stmmac_set_hw_vlan_mode(priv, priv->hw);
>  	phylink_rx_clk_stop_unblock(priv->phylink);
> @@ -7184,12 +7184,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;

[Severity: High]
This isn't a bug introduced by this patch, since the DMA was already started
here before the reorder, but the MAC enable now sits inside a region whose
error path never unwinds either the DMA or the MAC.

If stmmac_request_irq() fails, the unwind in stmmac_xdp_open() is:

irq_error:
	for (chan = 0; chan < priv->plat->tx_queues_to_use; chan++)
		hrtimer_cancel(&priv->dma_conf.tx_queue[chan].txtimer);

init_error:
	free_dma_desc_resources(priv, &priv->dma_conf);

There is no stmmac_stop_all_dma() and no stmmac_mac_set(priv, priv->ioaddr,
false), while free_dma_desc_resources() reaches:

__free_dma_rx_desc_resources() {
	dma_free_coherent(priv->device, size, addr, rx_q->dma_rx_phy);
	...
	if (rx_q->page_pool)
		page_pool_destroy(rx_q->page_pool);
}

Can the RX DMA then write received frames into the freed coherent
descriptor ring and the freed page-pool pages? stmmac_xdp_set_prog() also
discards the stmmac_xdp_open() return value:

	if (if_running && need_update)
		stmmac_xdp_open(dev);
	...
	return 0;

so does the device stay in that state indefinitely rather than being
reported and cleaned up?

The same gap looks present on the ifup path: __stmmac_open()'s irq_error
label only does phylink_stop(), the per-queue hrtimer_cancel() and
stmmac_release_ptp(), and stmmac_open() then falls through to
err_dma_resources: free_dma_desc_resources(priv, dma_conf), which frees the
rings that were programmed into the live DMA engine.

Should these error paths be made symmetric with the release path, which
does:

	/* Stop the MAC and the TX/RX DMA channels */
	stmmac_datapath_teardown(priv);

	/* Release and free the Rx/Tx resources */
	free_dma_desc_resources(priv, &priv->dma_conf);

[Severity: Medium]
This is a pre-existing race rather than something the patch adds, but it
bears on the invariant the patch is establishing: the ordering here is only
statement order inside stmmac_xdp_open(), and nothing serializes it against
phylink's resolve worker.

phylink_resolve() runs from a work item holding only pl->phydev_mutex,
phy->lock and pl->state_mutex, with no rtnl:

drivers/net/phy/phylink.c:phylink_resolve() {
	mutex_lock(&pl->phydev_mutex);
	phy = pl->phydev;
	if (phy)
		mutex_lock(&phy->lock);
	mutex_lock(&pl->state_mutex);
	...
		else
			phylink_link_up(pl, link_state);
}

and that reaches stmmac_mac_link_up() -> stmmac_mac_set(priv, priv->ioaddr,
true). The AF_XDP reconfiguration never quiesces phylink:

drivers/net/ethernet/stmicro/stmmac/stmmac_xdp.c:stmmac_xdp_set_prog() {
	if (if_running && need_update)
		stmmac_xdp_release(dev);
	...
	if (if_running && need_update)
		stmmac_xdp_open(dev);
}

So can a link-up resolved on another CPU re-enable MAC Rx after
stmmac_xdp_release() has stopped the DMA and before stmmac_xdp_open()
reaches stmmac_start_all_dma(), which is the same "MAC accepting frames with
no DMA to drain the MTL FIFO" condition described in the commit message?

Separately, stmmac_mac_link_up() and stmmac_mac_set() both do unlocked
read-modify-write on MAC_CTRL_REG:

	old_ctrl = readl(priv->ioaddr + MAC_CTRL_REG);
	ctrl = old_ctrl & ~priv->hw->link.speed_mask;
	...
	if (ctrl != old_ctrl)
		writel(ctrl, priv->ioaddr + MAC_CTRL_REG);
	...
	stmmac_mac_set(priv, priv->ioaddr, true);

Can concurrent speed/duplex and RE/TE updates lose each other's bits?
-- 
This is an AI-generated review.

[PATCH net-next v4 2/3] net: stmmac: xgmac: tear the datapath down in the documented order

[Abid Ali]

The driver stops all DMA channels at once and only some paths disable
the MAC afterwards. The databook requires the reverse order with a wait
at each stage, otherwise a frame can be left stranded in the MTL FIFOs
and reappear as corruption on the next bring-up.

Add four optional DMA callbacks and drive them from a new
stmmac_datapath_teardown():

  1) stop the Tx DMA channels, wait for stopped
  2) wait for the MTL Tx queues to drain into the MAC
  3) disable the MAC Tx and Rx
  4) wait for the MTL Rx queues to drain into the Rx DMA
  5) stop the Rx DMA channels, wait for stopped

These are implemented for the XGMAC variant.

TPS and RPS are latched, so clear them in the matching start op.
__stmmac_release() calls phylink_stop() after the teardown rather than
before it, as mac_link_down() clears TE and RE and stages 2 and 4
cannot progress once the MAC is disabled.

The Wake-on-LAN path in stmmac_suspend() keeps the old sequence, as it
has to leave the receiver enabled.

Signed-off-by: Abid Ali <dev.taqnialabs@gmail.com>
---
 drivers/net/ethernet/stmicro/stmmac/dwxgmac2.h     |  8 +++
 drivers/net/ethernet/stmicro/stmmac/dwxgmac2_dma.c | 82 ++++++++++++++++++++++
 drivers/net/ethernet/stmicro/stmmac/hwif.h         | 17 +++++
 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c  | 73 ++++++++++++++-----
 4 files changed, 164 insertions(+), 16 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2.h b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2.h
index f8ab347f7b5b..50f9ccd4a9c4 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2.h
+++ b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2.h
@@ -291,6 +291,10 @@
 #define XGMAC_TTC			GENMASK(6, 4)
 #define XGMAC_TXQEN			GENMASK(3, 2)
 #define XGMAC_TSF			BIT(1)
+#define XGMAC_MTL_TXQ_DEBUG(x)		(0x00001108 + (0x80 * (x)))
+#define XGMAC_TXQSTS			BIT(4)
+#define XGMAC_TRCSTS			GENMASK(2, 1)
+#define XGMAC_TRCSTS_READ		0x1
 #define XGMAC_MTL_TCx_ETS_CONTROL(x)	(0x00001110 + (0x80 * (x)))
 #define XGMAC_MTL_TCx_QUANTUM_WEIGHT(x)	(0x00001118 + (0x80 * (x)))
 #define XGMAC_MTL_TCx_SENDSLOPE(x)	(0x0000111c + (0x80 * (x)))
@@ -306,6 +310,9 @@
 #define XGMAC_EHFC			BIT(7)
 #define XGMAC_RSF			BIT(5)
 #define XGMAC_RTC			GENMASK(1, 0)
+#define XGMAC_MTL_RXQ_DEBUG(x)		(0x00001148 + (0x80 * (x)))
+#define XGMAC_PRXQ			GENMASK(29, 16)
+#define XGMAC_RXQSTS			GENMASK(5, 4)
 #define XGMAC_MTL_RXQ_FLOW_CONTROL(x)	(0x00001150 + (0x80 * (x)))
 #define XGMAC_RFD			GENMASK(31, 17)
 #define XGMAC_RFA			GENMASK(15, 1)
@@ -391,6 +398,7 @@
 #define XGMAC_NIS			BIT(15)
 #define XGMAC_AIS			BIT(14)
 #define XGMAC_FBE			BIT(12)
+#define XGMAC_RPS			BIT(8)
 #define XGMAC_RBU			BIT(7)
 #define XGMAC_RI			BIT(6)
 #define XGMAC_TBU			BIT(2)
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_dma.c b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_dma.c
index ff83858ebc1f..df366b2e6bd7 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_dma.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_dma.c
@@ -4,6 +4,7 @@
  * stmmac XGMAC support.
  */
 
+#include <linux/bitfield.h>
 #include <linux/iopoll.h>
 #include "stmmac.h"
 #include "dwxgmac2.h"
@@ -261,6 +262,9 @@ static void dwxgmac2_dma_start_tx(struct stmmac_priv *priv,
 {
 	u32 value;
 
+	/* TPS is latched once set, so clear it on every Tx DMA start. */
+	writel(XGMAC_TPS, ioaddr + XGMAC_DMA_CH_STATUS(chan));
+
 	value = readl(ioaddr + XGMAC_DMA_CH_TX_CONTROL(chan));
 	value |= XGMAC_TXST;
 	writel(value, ioaddr + XGMAC_DMA_CH_TX_CONTROL(chan));
@@ -289,6 +293,9 @@ static void dwxgmac2_dma_start_rx(struct stmmac_priv *priv,
 {
 	u32 value;
 
+	/* RPS is latched once set, so clear it on every Rx DMA start. */
+	writel(XGMAC_RPS, ioaddr + XGMAC_DMA_CH_STATUS(chan));
+
 	value = readl(ioaddr + XGMAC_DMA_CH_RX_CONTROL(chan));
 	value |= XGMAC_RXST;
 	writel(value, ioaddr + XGMAC_DMA_CH_RX_CONTROL(chan));
@@ -592,6 +599,77 @@ static int dwxgmac2_enable_tbs(struct stmmac_priv *priv, void __iomem *ioaddr,
 	return 0;
 }
 
+static int dwxgmac2_tx_dma_stopped(struct stmmac_priv *priv,
+				   void __iomem *ioaddr, u32 chan)
+{
+	u32 value;
+	int ret;
+
+	ret = readl_poll_timeout(ioaddr + XGMAC_DMA_CH_STATUS(chan), value,
+				 value & XGMAC_TPS, 100, 10000);
+	if (ret)
+		netdev_warn(priv->dev, "Tx DMA channel %u stop timeout\n",
+			    chan);
+
+	return ret;
+}
+
+static int dwxgmac2_tx_mtl_drain(struct stmmac_priv *priv,
+				 void __iomem *ioaddr, u32 queue)
+{
+	u32 value;
+	int ret;
+
+	/* Wait until the queue is empty and its read controller is no longer
+	 * pulling a frame out towards the MAC.
+	 */
+	ret = readl_poll_timeout(ioaddr + XGMAC_MTL_TXQ_DEBUG(queue), value,
+				 !(value & XGMAC_TXQSTS) &&
+				 FIELD_GET(XGMAC_TRCSTS, value) !=
+				 XGMAC_TRCSTS_READ,
+				 100, 10000);
+	if (ret)
+		netdev_warn(priv->dev, "MTL Tx queue %u drain timeout\n",
+			    queue);
+
+	return ret;
+}
+
+static int dwxgmac2_rx_mtl_drain(struct stmmac_priv *priv,
+				 void __iomem *ioaddr, u32 queue)
+{
+	u32 value;
+	int ret;
+
+	/* Wait until no packet is left in the queue and the queue reports
+	 * itself empty.
+	 */
+	ret = readl_poll_timeout(ioaddr + XGMAC_MTL_RXQ_DEBUG(queue), value,
+				 !FIELD_GET(XGMAC_PRXQ, value) &&
+				 !FIELD_GET(XGMAC_RXQSTS, value),
+				 100, 10000);
+	if (ret)
+		netdev_warn(priv->dev, "MTL Rx queue %u drain timeout\n",
+			    queue);
+
+	return ret;
+}
+
+static int dwxgmac2_rx_dma_stopped(struct stmmac_priv *priv,
+				   void __iomem *ioaddr, u32 chan)
+{
+	u32 value;
+	int ret;
+
+	ret = readl_poll_timeout(ioaddr + XGMAC_DMA_CH_STATUS(chan), value,
+				 value & XGMAC_RPS, 100, 10000);
+	if (ret)
+		netdev_warn(priv->dev, "Rx DMA channel %u stop timeout\n",
+			    chan);
+
+	return ret;
+}
+
 const struct stmmac_dma_ops dwxgmac210_dma_ops = {
 	.reset = dwxgmac2_dma_reset,
 	.init = dwxgmac2_dma_init,
@@ -620,4 +698,8 @@ const struct stmmac_dma_ops dwxgmac210_dma_ops = {
 	.set_bfsize = dwxgmac2_set_bfsize,
 	.enable_sph = dwxgmac2_enable_sph,
 	.enable_tbs = dwxgmac2_enable_tbs,
+	.tx_dma_stopped = dwxgmac2_tx_dma_stopped,
+	.tx_mtl_drain = dwxgmac2_tx_mtl_drain,
+	.rx_mtl_drain = dwxgmac2_rx_mtl_drain,
+	.rx_dma_stopped = dwxgmac2_rx_dma_stopped,
 };
diff --git a/drivers/net/ethernet/stmicro/stmmac/hwif.h b/drivers/net/ethernet/stmicro/stmmac/hwif.h
index 04dafec021b4..86f4aa88961c 100644
--- a/drivers/net/ethernet/stmicro/stmmac/hwif.h
+++ b/drivers/net/ethernet/stmicro/stmmac/hwif.h
@@ -231,6 +231,15 @@ struct stmmac_dma_ops {
 			   bool en, u32 chan);
 	int (*enable_tbs)(struct stmmac_priv *priv, void __iomem *ioaddr,
 			  bool en, u32 chan);
+	/* Ordered datapath teardown */
+	int (*tx_dma_stopped)(struct stmmac_priv *priv, void __iomem *ioaddr,
+			      u32 chan);
+	int (*tx_mtl_drain)(struct stmmac_priv *priv, void __iomem *ioaddr,
+			    u32 queue);
+	int (*rx_mtl_drain)(struct stmmac_priv *priv, void __iomem *ioaddr,
+			    u32 queue);
+	int (*rx_dma_stopped)(struct stmmac_priv *priv, void __iomem *ioaddr,
+			      u32 chan);
 };
 
 #define stmmac_dma_init(__priv, __args...) \
@@ -293,6 +302,14 @@ struct stmmac_dma_ops {
 	stmmac_do_void_callback(__priv, dma, enable_sph, __priv, __args)
 #define stmmac_enable_tbs(__priv, __args...) \
 	stmmac_do_callback(__priv, dma, enable_tbs, __priv, __args)
+#define stmmac_tx_dma_stopped(__priv, __args...) \
+	stmmac_do_callback(__priv, dma, tx_dma_stopped, __priv, __args)
+#define stmmac_tx_mtl_drain(__priv, __args...) \
+	stmmac_do_callback(__priv, dma, tx_mtl_drain, __priv, __args)
+#define stmmac_rx_mtl_drain(__priv, __args...) \
+	stmmac_do_callback(__priv, dma, rx_mtl_drain, __priv, __args)
+#define stmmac_rx_dma_stopped(__priv, __args...) \
+	stmmac_do_callback(__priv, dma, rx_dma_stopped, __priv, __args)
 
 struct mac_device_info;
 struct net_device;
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index b9d5d4aabf42..9c3a8f122b94 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -2560,6 +2560,52 @@ static void stmmac_stop_all_dma(struct stmmac_priv *priv)
 		stmmac_deinit_chan(priv, priv->ioaddr, chan);
 }
 
+/**
+ * stmmac_datapath_teardown - ordered datapath teardown as per IP specification
+ * @priv: driver private structure
+ * Description:
+ * When teardown ops are available, follow the databook ordered teardown
+ * sequence.
+ * Follows the legacy stop + MAC disable for variants without the feature.
+ */
+static void stmmac_datapath_teardown(struct stmmac_priv *priv)
+{
+	u32 rx_channels_count = priv->plat->rx_queues_to_use;
+	u32 tx_channels_count = priv->plat->tx_queues_to_use;
+	u32 chan;
+
+	/* Stop all TX DMA channels */
+	for (chan = 0; chan < tx_channels_count; chan++)
+		stmmac_stop_tx_dma(priv, chan);
+
+	/* Wait for every TX DMA channel to report itself stopped */
+	if (priv->hw->dma->tx_dma_stopped)
+		for (chan = 0; chan < tx_channels_count; chan++)
+			stmmac_tx_dma_stopped(priv, priv->ioaddr, chan);
+
+	/* Wait for the MTL TX queues to finish pushing into the MAC */
+	if (priv->hw->dma->tx_mtl_drain)
+		for (chan = 0; chan < tx_channels_count; chan++)
+			stmmac_tx_mtl_drain(priv, priv->ioaddr, chan);
+
+	/* Disable the MAC TX and RX */
+	stmmac_mac_set(priv, priv->ioaddr, false);
+
+	/* Wait for the MTL RX queues to drain into the RX DMA */
+	if (priv->hw->dma->rx_mtl_drain)
+		for (chan = 0; chan < rx_channels_count; chan++)
+			stmmac_rx_mtl_drain(priv, priv->ioaddr, chan);
+
+	/* Stop all RX DMA channels */
+	for (chan = 0; chan < rx_channels_count; chan++)
+		stmmac_stop_rx_dma(priv, chan);
+
+	/* Wait for every RX DMA channel to report itself stopped */
+	if (priv->hw->dma->rx_dma_stopped)
+		for (chan = 0; chan < rx_channels_count; chan++)
+			stmmac_rx_dma_stopped(priv, priv->ioaddr, chan);
+}
+
 /**
  *  stmmac_dma_operation_mode - HW DMA operation mode
  *  @priv: driver private structure
@@ -4240,9 +4286,6 @@ static void __stmmac_release(struct net_device *dev)
 	struct stmmac_priv *priv = netdev_priv(dev);
 	u8 chan;
 
-	/* Stop and disconnect the PHY */
-	phylink_stop(priv->phylink);
-
 	stmmac_disable_all_queues(priv);
 
 	for (chan = 0; chan < priv->plat->tx_queues_to_use; chan++)
@@ -4253,8 +4296,11 @@ static void __stmmac_release(struct net_device *dev)
 	/* Free the IRQ lines */
 	stmmac_free_irq(dev, REQ_IRQ_ERR_ALL, 0);
 
-	/* Stop TX/RX DMA and clear the descriptors */
-	stmmac_stop_all_dma(priv);
+	/* Has to run before mac_link_down() disables the MAC. */
+	stmmac_datapath_teardown(priv);
+
+	/* Stop and disconnect the PHY */
+	phylink_stop(priv->phylink);
 
 	/* Release and free the Rx/Tx resources */
 	free_dma_desc_resources(priv, &priv->dma_conf);
@@ -7104,15 +7150,12 @@ void stmmac_xdp_release(struct net_device *dev)
 	/* Free the IRQ lines */
 	stmmac_free_irq(dev, REQ_IRQ_ERR_ALL, 0);
 
-	/* Stop TX/RX DMA channels */
-	stmmac_stop_all_dma(priv);
+	/* Stop the MAC and the TX/RX DMA channels */
+	stmmac_datapath_teardown(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
 	 */
@@ -8190,17 +8233,15 @@ int stmmac_suspend(struct device *dev)
 		timer_delete_sync(&priv->eee_ctrl_timer);
 	}
 
-	/* Stop TX/RX DMA */
-	stmmac_stop_all_dma(priv);
-
-	stmmac_legacy_serdes_power_down(priv);
-
 	/* Enable Power down mode by programming the PMT regs */
 	if (priv->wolopts) {
+		stmmac_stop_all_dma(priv);
+		stmmac_legacy_serdes_power_down(priv);
 		stmmac_pmt(priv, priv->hw, priv->wolopts);
 		priv->irq_wake = 1;
 	} else {
-		stmmac_mac_set(priv, priv->ioaddr, false);
+		stmmac_datapath_teardown(priv);
+		stmmac_legacy_serdes_power_down(priv);
 		pinctrl_pm_select_sleep_state(priv->device);
 	}
 

-- 
2.43.0

Re: [PATCH net-next v4 2/3] net: stmmac: xgmac: tear the datapath down in the documented order

[Paolo Abeni]

This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
Full review at: https://netdev-ai.bots.linux.dev/sashiko/
---
net: stmmac: xgmac: tear the datapath down in the documented order

This adds four optional stmmac_dma_ops callbacks (tx_dma_stopped,
tx_mtl_drain, rx_mtl_drain, rx_dma_stopped), implements them for XGMAC, and
drives them from a new stmmac_datapath_teardown() used by __stmmac_release(),
stmmac_xdp_release() and the non-WoL branch of stmmac_suspend().  It also
clears the latched TPS/RPS status bits in the Tx/Rx DMA start ops and moves
phylink_stop() after the teardown in __stmmac_release().

A few questions below.

>     The driver stops all DMA channels at once and only some paths disable
>     the MAC afterwards. The databook requires the reverse order with a wait
>     at each stage, otherwise a frame can be left stranded in the MTL FIFOs
>     and reappear as corruption on the next bring-up.

[Severity: Low]
The changelog describes a user-visible failure ("reappear as corruption on
the next bring-up") but the patch carries no Fixes: tag and no statement
about stable backporting, while also adding new hwif callbacks and
reordering phylink_stop().

Is this meant as a fix for stable trees, and if so which commit does it fix?
If it is hardening for a hardware ordering requirement rather than a
regression fix, could the changelog say so explicitly?

> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_dma.c b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_dma.c
> index ff83858ebc1f3..df366b2e6bd72 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_dma.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_dma.c

[ ... ]

> @@ -592,6 +599,77 @@ static int dwxgmac2_enable_tbs(struct stmmac_priv *priv, void __iomem *ioaddr,
>  	return 0;
>  }
>  
> +static int dwxgmac2_tx_mtl_drain(struct stmmac_priv *priv,
> +				 void __iomem *ioaddr, u32 queue)
> +{
> +	u32 value;
> +	int ret;
> +
> +	/* Wait until the queue is empty and its read controller is no longer
> +	 * pulling a frame out towards the MAC.
> +	 */
> +	ret = readl_poll_timeout(ioaddr + XGMAC_MTL_TXQ_DEBUG(queue), value,
> +				 !(value & XGMAC_TXQSTS) &&
> +				 FIELD_GET(XGMAC_TRCSTS, value) !=
> +				 XGMAC_TRCSTS_READ,
> +				 100, 10000);
> +	if (ret)
> +		netdev_warn(priv->dev, "MTL Tx queue %u drain timeout\n",
> +			    queue);
> +
> +	return ret;
> +}
> +
> +static int dwxgmac2_rx_mtl_drain(struct stmmac_priv *priv,
> +				 void __iomem *ioaddr, u32 queue)
> +{
> +	u32 value;
> +	int ret;
> +
> +	/* Wait until no packet is left in the queue and the queue reports
> +	 * itself empty.
> +	 */
> +	ret = readl_poll_timeout(ioaddr + XGMAC_MTL_RXQ_DEBUG(queue), value,
> +				 !FIELD_GET(XGMAC_PRXQ, value) &&
> +				 !FIELD_GET(XGMAC_RXQSTS, value),
> +				 100, 10000);
> +	if (ret)
> +		netdev_warn(priv->dev, "MTL Rx queue %u drain timeout\n",
> +			    queue);
> +
> +	return ret;
> +}

[ ... ]

> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> index b9d5d4aabf423..9c3a8f122b94f 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> @@ -2560,6 +2560,52 @@ static void stmmac_stop_all_dma(struct stmmac_priv *priv)
>  		stmmac_deinit_chan(priv, priv->ioaddr, chan);
>  }
>  
> +/**
> + * stmmac_datapath_teardown - ordered datapath teardown as per IP specification
> + * @priv: driver private structure
> + * Description:
> + * When teardown ops are available, follow the databook ordered teardown
> + * sequence.
> + * Follows the legacy stop + MAC disable for variants without the feature.
> + */
> +static void stmmac_datapath_teardown(struct stmmac_priv *priv)
> +{
> +	u32 rx_channels_count = priv->plat->rx_queues_to_use;
> +	u32 tx_channels_count = priv->plat->tx_queues_to_use;
> +	u32 chan;
> +
> +	/* Stop all TX DMA channels */
> +	for (chan = 0; chan < tx_channels_count; chan++)
> +		stmmac_stop_tx_dma(priv, chan);

[Severity: Low]
This isn't a bug in the final tree, but at this commit stage 1 dispatches to
dwxgmac2_dma_stop_tx(), which still clears the global MAC transmit enable:

  dwxgmac2_dma_stop_tx()
      value = readl(ioaddr + XGMAC_TX_CONFIG);
      value &= ~XGMAC_CONFIG_TE;
      writel(value, ioaddr + XGMAC_TX_CONFIG);

Stage 2 then waits for MTL_TXQ_DEBUG.TXQSTS to clear, which the new helper's
own comment says depends on the read controller still pulling frames towards
the MAC, and the changelog says "stages 2 and 4 cannot progress once the MAC
is disabled".

Does this commit on its own make every XGMAC close/suspend/XDP reconfigure
burn the full 10 ms poll plus one "MTL Tx queue %u drain timeout" per Tx
queue?  The later patch "net: stmmac: xgmac: decouple the MAC Rx/Tx enables
from the DMA ops" removes the XGMAC_CONFIG_TE clear from
dwxgmac2_dma_stop_tx(), so this is only a bisect hazard within the series -
could the ordering of the two patches be swapped?

> +
> +	/* Wait for every TX DMA channel to report itself stopped */
> +	if (priv->hw->dma->tx_dma_stopped)
> +		for (chan = 0; chan < tx_channels_count; chan++)
> +			stmmac_tx_dma_stopped(priv, priv->ioaddr, chan);

[Severity: Medium]
This isn't a problem introduced by this patch - the old stmmac_stop_all_dma()
path did not wait for the hardware at all - but now that the new ops do
return a status, is it worth acting on it?

All four calls here discard the return value, so a -ETIMEDOUT only produces
a netdev_warn() and the sequence continues:

  - a Tx MTL drain timeout is followed by disabling the MAC
  - an Rx MTL drain timeout is followed by stopping the Rx DMA, leaving the
    frame the wait was supposed to drain
  - an unconfirmed Tx/Rx DMA stop is followed by free_dma_desc_resources()
    in __stmmac_release() and stmmac_xdp_release(), or by the SerDes and pin
    power-down and a success return in stmmac_suspend()

If a channel really did not stop, can it still fetch descriptors or write
packet data and completion status into memory that has already been unmapped
or freed?  Should a timeout trigger a DMA software reset, or at least
propagate out of the suspend path?

> +
> +	/* Wait for the MTL TX queues to finish pushing into the MAC */
> +	if (priv->hw->dma->tx_mtl_drain)
> +		for (chan = 0; chan < tx_channels_count; chan++)
> +			stmmac_tx_mtl_drain(priv, priv->ioaddr, chan);
> +
> +	/* Disable the MAC TX and RX */
> +	stmmac_mac_set(priv, priv->ioaddr, false);

[Severity: Medium]
Stage 2 needs MAC TE still set so the MTL Tx FIFO can push into the MAC, but
the MAC transmitter is cleared on every carrier-down event, not only by the
phylink_stop() call this patch reordered:

  stmmac_mac_link_down()
      stmmac_mac_set(priv, priv->ioaddr, false);

So on ifdown or suspend while the link is already down - cable unplugged,
autoneg incomplete, link dropped while a frame was in flight, which is
exactly when a frame is most likely to sit in the MTL Tx FIFO - TE is
already clear.

Does dwxgmac2_tx_mtl_drain() then poll for the full 10000 us per Tx queue,
warn, and return -ETIMEDOUT while the stranded frame stays where it was?
Would it make sense to check the carrier state or read back MAC_CONFIG
TE/RE before entering stage 2, and to fall back to the MTL flush-Tx-queue
path when the MAC transmitter is off?

> +
> +	/* Wait for the MTL RX queues to drain into the RX DMA */
> +	if (priv->hw->dma->rx_mtl_drain)
> +		for (chan = 0; chan < rx_channels_count; chan++)
> +			stmmac_rx_mtl_drain(priv, priv->ioaddr, chan);

[Severity: Medium]
Stage 4 waits for the Rx DMA to move frames out of the MTL Rx FIFO, but all
three callers have already stopped NAPI before reaching here:

  __stmmac_release() / stmmac_xdp_release() / stmmac_suspend()
      stmmac_disable_all_queues()
          __stmmac_disable_all_queues()   /* napi_disable() */
      ...
      stmmac_datapath_teardown()

With no NAPI to recycle descriptors and inbound traffic still arriving, can
the Rx ring run out of free descriptors so that the Rx DMA suspends on
descriptor unavailability and stops draining the FIFO?  In that case
dwxgmac2_rx_mtl_drain() cannot see PRXQ == 0 && RXQSTS == 0 and spends the
full 10000 us per Rx queue plus a warning, and the frame stays in the MTL Rx
FIFO anyway.

Stages 1 and 2 can already sleep tx_queues_to_use * 10 ms before this point,
which widens the window for the ring to be exhausted.

> +
> +	/* Stop all RX DMA channels */
> +	for (chan = 0; chan < rx_channels_count; chan++)
> +		stmmac_stop_rx_dma(priv, chan);
> +
> +	/* Wait for every RX DMA channel to report itself stopped */
> +	if (priv->hw->dma->rx_dma_stopped)
> +		for (chan = 0; chan < rx_channels_count; chan++)
> +			stmmac_rx_dma_stopped(priv, priv->ioaddr, chan);
> +}

[Severity: High]
The new helper replaces stmmac_stop_all_dma() in __stmmac_release(),
stmmac_xdp_release() and the non-WoL branch of stmmac_suspend(), but it does
not end with the loop stmmac_stop_all_dma() finishes with:

  stmmac_stop_all_dma()
      ...
      for (chan = 0; chan < dma_csr_ch; chan++)
              stmmac_deinit_chan(priv, priv->ioaddr, chan);

deinit_chan is implemented only by dwmac4_dma_ops and dwmac410_dma_ops
(dwmac4_dma_deinit_channel() and dwmac410_dma_deinit_channel()), and its only
job is to clear DMA_CHAN_INTR_DEFAULT_MASK in DMA_CHAN_INTR_ENA, i.e. it is
the matching disable for the enables written by dwmac4_dma_init_channel().
That loop was added by commit 59a57128ae5231 ("net: stmmac: dwmac4: mask
interrupts when stopping DMA in suspend", Fixes: 1b9707e6f1a9) precisely
because suspend otherwise produces an RPS interrupt storm.

DWMAC4 and DWMAC4.10 provide none of the new ordered-teardown ops, so on
those parts the new helper is plain stop plus MAC disable with the masking
gone.  Nothing else covers it: dwmac4_disable_dma_irq() only touches RIE and
TIE, and stmmac_disable_all_queues() only does napi_disable().

Does routing the non-WoL suspend path through the new helper reintroduce the
storm that 59a57128ae5231 fixed, with the IRQ handler still registered?  And
since RPS/TPS clearing moved into the start ops, do those abnormal status
bits now stay asserted for the whole time the interface is down or suspended
while the corresponding enables remain set?

> @@ -4240,9 +4286,6 @@ static void __stmmac_release(struct net_device *dev)
>  	struct stmmac_priv *priv = netdev_priv(dev);
>  	u8 chan;
>  
> -	/* Stop and disconnect the PHY */
> -	phylink_stop(priv->phylink);
> -
>  	stmmac_disable_all_queues(priv);
>  
>  	for (chan = 0; chan < priv->plat->tx_queues_to_use; chan++)
> @@ -4253,8 +4296,11 @@ static void __stmmac_release(struct net_device *dev)
>  	/* Free the IRQ lines */
>  	stmmac_free_irq(dev, REQ_IRQ_ERR_ALL, 0);
>  
> -	/* Stop TX/RX DMA and clear the descriptors */
> -	stmmac_stop_all_dma(priv);
> +	/* Has to run before mac_link_down() disables the MAC. */
> +	stmmac_datapath_teardown(priv);
> +
> +	/* Stop and disconnect the PHY */
> +	phylink_stop(priv->phylink);

[Severity: Medium]
phylink_stop() is the only fence against the phylink resolve worker - it
calls phylink_run_resolve_and_disable(), which sets PHYLINK_DISABLE_STOPPED
and flushes the work.  Moving it after the teardown means the whole ordered
sequence now runs concurrently with that worker, which takes only
pl->phydev_mutex, phy->lock and pl->state_mutex and never RTNL:

  phylink_resolve()            /* system_power_efficient_wq, no RTNL */
      phylink_link_up()
          stmmac_mac_link_up()
              stmmac_mac_set(priv, priv->ioaddr, true);
  or
      phylink_link_down()
          stmmac_mac_link_down()
              stmmac_mac_set(priv, priv->ioaddr, false);

stmmac_datapath_teardown() holds neither priv->lock nor any lock shared with
those callbacks, and both sides do an unsynchronized read-modify-write of
MAC_CTRL_REG.

Can a link event arriving during ifdown set TE|RE again after stage 3, so
that the MAC keeps filling the MTL Rx FIFO after stage 4's drain wait and
after stage 5 has stopped the Rx DMA?  Conversely, if the worker clears
TE/RE just after stage 1, do stages 2 and 4 each burn the full 10 ms poll and
warn per queue?

> @@ -7104,15 +7150,12 @@ void stmmac_xdp_release(struct net_device *dev)
>  	/* Free the IRQ lines */
>  	stmmac_free_irq(dev, REQ_IRQ_ERR_ALL, 0);
>  
> -	/* Stop TX/RX DMA channels */
> -	stmmac_stop_all_dma(priv);
> +	/* Stop the MAC and the TX/RX DMA channels */
> +	stmmac_datapath_teardown(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
>  	 */
> @@ -8190,17 +8233,15 @@ int stmmac_suspend(struct device *dev)
>  		timer_delete_sync(&priv->eee_ctrl_timer);
>  	}
>  
> -	/* Stop TX/RX DMA */
> -	stmmac_stop_all_dma(priv);
> -
> -	stmmac_legacy_serdes_power_down(priv);
> -
>  	/* Enable Power down mode by programming the PMT regs */
>  	if (priv->wolopts) {
> +		stmmac_stop_all_dma(priv);
> +		stmmac_legacy_serdes_power_down(priv);
>  		stmmac_pmt(priv, priv->hw, priv->wolopts);
>  		priv->irq_wake = 1;
>  	} else {
> -		stmmac_mac_set(priv, priv->ioaddr, false);
> +		stmmac_datapath_teardown(priv);
> +		stmmac_legacy_serdes_power_down(priv);
>  		pinctrl_pm_select_sleep_state(priv->device);
>  	}

Only the WoL branch keeps stmmac_stop_all_dma() and therefore the
stmmac_deinit_chan() interrupt masking; the plain suspend case is the one
that loses it, as noted above.
-- 
This is an AI-generated review.

[PATCH net-next v4 3/3] net: stmmac: xgmac: decouple the MAC Rx/Tx enables from the DMA ops

[Abid Ali]

The XGMAC per-channel DMA start and stop ops also raise and lower
MAC_Tx_Configuration.TE and MAC_Rx_Configuration.RE. Those bits are
global, not per channel, so an op meant to touch one channel
reconfigures the whole MAC. stmmac_tx_err() restarting a single Tx
channel clears TE and stops transmit on every other channel too.

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

diff --git a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_dma.c b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_dma.c
index df366b2e6bd7..ba4f535fb9eb 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_dma.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_dma.c
@@ -268,10 +268,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,
@@ -282,10 +278,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,
@@ -299,10 +291,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,

-- 
2.43.0