RE: [PATCH v3 net 2/2] net: enetc: add graceful stop to safely reinitialize the TX Ring

[Sai Krishna Gajula]

> -----Original Message-----
> From: Wei Fang <wei.fang@xxxxxxx>
> Sent: Wednesday, March 11, 2026 2:11 PM
> To: claudiu.manoil@xxxxxxx; vladimir.oltean@xxxxxxx;
> xiaoning.wang@xxxxxxx; andrew+netdev@xxxxxxx; davem@xxxxxxxxxxxxx;
> edumazet@xxxxxxxxxx; kuba@xxxxxxxxxx; pabeni@xxxxxxxxxx;
> linux@xxxxxxxxxxxxxxx; Frank.Li@xxxxxxx; horms@xxxxxxxxxx
> Cc: netdev@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx; imx@xxxxxxxxxxxxxxx
> Subject: [PATCH v3 net 2/2] net: enetc: add graceful stop to safely
> reinitialize the TX Ring
> 
> For ENETC v4, the PIR and CIR will be reset if they are not equal when
> reinitializing the TX BD ring. However, resetting the PIR and CIR alone is
> insufficient. When a link-down event occurs while the TX BD ring is
> transmitting frames, 
> For ENETC v4, the PIR and CIR will be reset if they are not equal when
> reinitializing the TX BD ring. However, resetting the PIR and CIR alone is
> insufficient. When a link-down event occurs while the TX BD ring is
> transmitting frames, subsequent reinitialization of the TX BD ring may cause it
> to malfunction. For example, the below steps can reproduce the problem.
> 
> 1. Unplug the cable when the TX BD ring is busy transmitting frames.
> 2. Disable the network interface (ifconfig eth0 down).
> 3. Re-enable the network interface (ifconfig eth0 up).
> 4. Plug in the cable, the TX BD ring may fail to transmit packets.
> 
> When the link-down event occurs, enetc4_pl_mac_link_down() only clears
> PMa_COMMAND_CONFIG[TX_EN] to disable MAC transmit data path. It
> doesn't set PORT[TXDIS] to 1 to flush the TX BD ring. Therefore, reinitializing
> the TX BD ring at this point is unsafe. To safely reinitialize the TX BD ring after a
> link-down event, we checked with the NETC IP team, a proper Ethernet MAC
> graceful stop is necessary. Therefore, add the Ethernet MAC graceful stop to
> the link-down event handler enetc4_pl_mac_link_down().
> 
> Fixes: 99100d0d9922 ("net: enetc: add preliminary support for i.MX95 ENETC
> PF")
> Signed-off-by: Wei Fang <wei.fang@xxxxxxx>
> ---
>  .../net/ethernet/freescale/enetc/enetc4_hw.h  |  11 ++
> .../net/ethernet/freescale/enetc/enetc4_pf.c  | 111 +++++++++++++++---
>  2 files changed, 108 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/net/ethernet/freescale/enetc/enetc4_hw.h
> b/drivers/net/ethernet/freescale/enetc/enetc4_hw.h
> index 3ed0f7a02767..1ce6551e186c 100644
> --- a/drivers/net/ethernet/freescale/enetc/enetc4_hw.h
> +++ b/drivers/net/ethernet/freescale/enetc/enetc4_hw.h
> @@ -134,6 +134,12 @@
> 
>  /* Port operational register */
>  #define ENETC4_POR			0x4100
> +#define  POR_TXDIS			BIT(0)
> +#define  POR_RXDIS			BIT(1)
> +
> +/* Port status register */
> +#define ENETC4_PSR			0x4100
> +#define  PSR_RX_BUSY			BIT(1)

ENETC4_POR = 0x4100
ENETC4_PSR = 0x4100
Minor nit, are both macros point to the same register offset? 

> 
>  /* Port traffic class a transmit maximum SDU register */
>  #define ENETC4_PTCTMSDUR(a)		((a) * 0x20 + 0x4208)
> @@ -173,6 +179,11 @@
>  /* Port internal MDIO base address, use to access PCS */
>  #define ENETC4_PM_IMDIO_BASE		0x5030
> 
> +/* Port MAC 0/1 Interrupt Event Register */
> +#define ENETC4_PM_IEVENT(mac)		(0x5040 + (mac) * 0x400)
> +#define  PM_IEVENT_TX_EMPTY		BIT(5)
> +#define  PM_IEVENT_RX_EMPTY		BIT(6)
> +
>  /* Port MAC 0/1 Pause Quanta Register */
>  #define ENETC4_PM_PAUSE_QUANTA(mac)	(0x5054 + (mac) * 0x400)
> 
> diff --git a/drivers/net/ethernet/freescale/enetc/enetc4_pf.c
> b/drivers/net/ethernet/freescale/enetc/enetc4_pf.c
> index 689b9f13c5eb..53cecbb23a97 100644
> --- a/drivers/net/ethernet/freescale/enetc/enetc4_pf.c
> +++ b/drivers/net/ethernet/freescale/enetc/enetc4_pf.c
> @@ -444,20 +444,11 @@ static void enetc4_set_trx_frame_size(struct
> enetc_pf *pf)
>  	enetc4_pf_reset_tc_msdu(&si->hw);
>  }
> 
> -static void enetc4_enable_trx(struct enetc_pf *pf) -{
> -	struct enetc_hw *hw = &pf->si->hw;
> -
> -	/* Enable port transmit/receive */
> -	enetc_port_wr(hw, ENETC4_POR, 0);
> -}
> -
>  static void enetc4_configure_port(struct enetc_pf *pf)  {
>  	enetc4_configure_port_si(pf);
>  	enetc4_set_trx_frame_size(pf);
>  	enetc_set_default_rss_key(pf);
> -	enetc4_enable_trx(pf);
>  }
> 
>  static int enetc4_init_ntmp_user(struct enetc_si *si) @@ -801,15 +792,105
> @@ static void enetc4_set_tx_pause(struct enetc_pf *pf, int num_rxbdr, bool
> tx_paus
>  	enetc_port_wr(hw, ENETC4_PPAUOFFTR, pause_off_thresh);  }
> 
> -static void enetc4_enable_mac(struct enetc_pf *pf, bool en)
> +static void enetc4_mac_wait_tx_empty(struct enetc_si *si, int mac) {
> +	u32 val;
> +
> +	if (read_poll_timeout(enetc_port_rd, val,
> +			      val & PM_IEVENT_TX_EMPTY,
> +			      100, 10000, false, &si->hw,
> +			      ENETC4_PM_IEVENT(mac)))
> +		dev_warn(&si->pdev->dev,
> +			 "MAC %d TX is not empty\n", mac);
> +}
> +
> +static void enetc4_mac_tx_graceful_stop(struct enetc_pf *pf) {
> +	struct enetc_hw *hw = &pf->si->hw;
> +	struct enetc_si *si = pf->si;
> +	u32 val;
> +
> +	val = enetc_port_rd(hw, ENETC4_POR);
> +	val |= POR_TXDIS;
> +	enetc_port_wr(hw, ENETC4_POR, val);
> +
> +	enetc4_mac_wait_tx_empty(si, 0);
> +	if (si->hw_features & ENETC_SI_F_QBU)
> +		enetc4_mac_wait_tx_empty(si, 1);
> +
> +	val = enetc_port_mac_rd(si, ENETC4_PM_CMD_CFG(0));
> +	val &= ~PM_CMD_CFG_TX_EN;
> +	enetc_port_mac_wr(si, ENETC4_PM_CMD_CFG(0), val); }
> +
> +static void enetc4_mac_tx_enable(struct enetc_pf *pf)
>  {
> +	struct enetc_hw *hw = &pf->si->hw;
>  	struct enetc_si *si = pf->si;
>  	u32 val;
> 
>  	val = enetc_port_mac_rd(si, ENETC4_PM_CMD_CFG(0));
> -	val &= ~(PM_CMD_CFG_TX_EN | PM_CMD_CFG_RX_EN);
> -	val |= en ? (PM_CMD_CFG_TX_EN | PM_CMD_CFG_RX_EN) : 0;
> +	val |= PM_CMD_CFG_TX_EN;
> +	enetc_port_mac_wr(si, ENETC4_PM_CMD_CFG(0), val);
> +
> +	val = enetc_port_rd(hw, ENETC4_POR);
> +	val &= ~POR_TXDIS;
> +	enetc_port_wr(hw, ENETC4_POR, val);
> +}
> +
> +static void enetc4_mac_wait_rx_empty(struct enetc_si *si, int mac) {
> +	u32 val;
> +
> +	if (read_poll_timeout(enetc_port_rd, val,
> +			      val & PM_IEVENT_RX_EMPTY,
> +			      100, 10000, false, &si->hw,
> +			      ENETC4_PM_IEVENT(mac)))
> +		dev_warn(&si->pdev->dev,
> +			 "MAC %d RX is not empty\n", mac);
> +}
> +
> +static void enetc4_mac_rx_graceful_stop(struct enetc_pf *pf) {
> +	struct enetc_hw *hw = &pf->si->hw;
> +	struct enetc_si *si = pf->si;
> +	u32 val;
> +
> +	if (si->hw_features & ENETC_SI_F_QBU) {
> +		val = enetc_port_rd(hw, ENETC4_PM_CMD_CFG(1));
> +		val &= ~PM_CMD_CFG_RX_EN;
> +		enetc_port_wr(hw, ENETC4_PM_CMD_CFG(1), val);
> +		enetc4_mac_wait_rx_empty(si, 1);
> +	}
> +
> +	val = enetc_port_rd(hw, ENETC4_PM_CMD_CFG(0));
> +	val &= ~PM_CMD_CFG_RX_EN;
> +	enetc_port_wr(hw, ENETC4_PM_CMD_CFG(0), val);
> +	enetc4_mac_wait_rx_empty(si, 0);
> +
> +	if (read_poll_timeout(enetc_port_rd, val,
> +			      !(val & PSR_RX_BUSY),
> +			      100, 10000, false, hw,
> +			      ENETC4_PSR))
> +		dev_warn(&si->pdev->dev, "Port RX busy\n");
> +
> +	val = enetc_port_rd(hw, ENETC4_POR);
> +	val |= POR_RXDIS;
> +	enetc_port_wr(hw, ENETC4_POR, val);
> +}
> +
> +static void enetc4_mac_rx_enable(struct enetc_pf *pf) {
> +	struct enetc_hw *hw = &pf->si->hw;
> +	struct enetc_si *si = pf->si;
> +	u32 val;
> +
> +	val = enetc_port_rd(hw, ENETC4_POR);
> +	val &= ~POR_RXDIS;
> +	enetc_port_wr(hw, ENETC4_POR, val);
> 
> +	val = enetc_port_mac_rd(si, ENETC4_PM_CMD_CFG(0));
> +	val |= PM_CMD_CFG_RX_EN;
>  	enetc_port_mac_wr(si, ENETC4_PM_CMD_CFG(0), val);  }
> 
> @@ -853,7 +934,8 @@ static void enetc4_pl_mac_link_up(struct
> phylink_config *config,
>  	enetc4_set_hd_flow_control(pf, hd_fc);
>  	enetc4_set_tx_pause(pf, priv->num_rx_rings, tx_pause);
>  	enetc4_set_rx_pause(pf, rx_pause);
> -	enetc4_enable_mac(pf, true);
> +	enetc4_mac_tx_enable(pf);
> +	enetc4_mac_rx_enable(pf);
>  }
> 
>  static void enetc4_pl_mac_link_down(struct phylink_config *config, @@ -
> 862,7 +944,8 @@ static void enetc4_pl_mac_link_down(struct phylink_config
> *config,  {
>  	struct enetc_pf *pf = phylink_to_enetc_pf(config);
> 
> -	enetc4_enable_mac(pf, false);
> +	enetc4_mac_rx_graceful_stop(pf);
> +	enetc4_mac_tx_graceful_stop(pf);
>  }
> 
>  static const struct phylink_mac_ops enetc_pl_mac_ops = {
> --
> 2.34.1
>