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

[Wei Fang]

> > 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?
> 

Oh, my bad, ENETC4_PSR should be 0x4104. I will fix it and verify
the solution again. Many thanks.