RE: [PATCH v2 net] net: enetc: safely reinitialize TX BD ring when it has unsent frames

[Wei Fang]

> This is a not a small patch for a fix.
> Could you consider splitting it up a bit?
> Say one patch for the ENETC_TBPIR/ENETC_TBCIR
> portion and another for the graceful stop?

Sure, I will, thanks.

> 
> >
> > diff --git a/drivers/net/ethernet/freescale/enetc/enetc.c
> b/drivers/net/ethernet/freescale/enetc/enetc.c
> > index 70768392912c..825a5d1f2965 100644
> > --- a/drivers/net/ethernet/freescale/enetc/enetc.c
> > +++ b/drivers/net/ethernet/freescale/enetc/enetc.c
> > @@ -2578,6 +2578,7 @@ EXPORT_SYMBOL_GPL(enetc_free_si_resources);
> >
> >  static void enetc_setup_txbdr(struct enetc_hw *hw, struct enetc_bdr
> *tx_ring)
> >  {
> > +	struct enetc_si *si = container_of(hw, struct enetc_si, hw);
> >  	int idx = tx_ring->index;
> >  	u32 tbmr;
> >
> > @@ -2595,6 +2596,14 @@ static void enetc_setup_txbdr(struct enetc_hw
> *hw, struct enetc_bdr *tx_ring)
> 
> The line above this hunk is:
> 
> 	/* clearing PI/CI registers for Tx not supported, adjust sw indexes */
> 
> And the AI generated review reports:
> 
>   This isn't a bug, but the comment now contradicts the code. The comment
>   says "clearing PI/CI registers for Tx not supported" but the code
>   immediately below clears TBPIR and TBCIR for ENETC v4 hardware.
> 
>   Should the comment be updated to clarify that clearing is only
>   unsupported on ENETC rev1?

I will update the comments, only ENETC v1 does not support setting
PIR and CIR.

> 
> >  	tx_ring->next_to_use = enetc_txbdr_rd(hw, idx, ENETC_TBPIR);
> >  	tx_ring->next_to_clean = enetc_txbdr_rd(hw, idx, ENETC_TBCIR);
> >
> > +	if (tx_ring->next_to_use != tx_ring->next_to_clean &&
> > +	    !is_enetc_rev1(si)) {
> > +		tx_ring->next_to_use = 0;
> > +		tx_ring->next_to_clean = 0;
> > +		enetc_txbdr_wr(hw, idx, ENETC_TBPIR, 0);
> > +		enetc_txbdr_wr(hw, idx, ENETC_TBCIR, 0);
> > +	}
> > +
> >  	/* enable Tx ints by setting pkt thr to 1 */
> >  	enetc_txbdr_wr(hw, idx, ENETC_TBICR0, ENETC_TBICR0_ICEN | 0x1);
> >
> 
> ...
> 
> > diff --git a/drivers/net/ethernet/freescale/enetc/enetc4_pf.c
> b/drivers/net/ethernet/freescale/enetc/enetc4_pf.c
> 
> ...
> 
> > @@ -801,15 +792,120 @@ 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 timeout = 0;
> > +
> > +	while (!(enetc_port_rd(&si->hw, ENETC4_PM_IEVENT(mac)) &
> > +		 PM_IEVENT_TX_EMPTY)) {
> > +		if (timeout >= 100) {
> > +			dev_warn(&si->pdev->dev,
> > +				 "MAC %d TX is not empty\n", mac);
> > +			break;
> > +		}
> > +
> > +		usleep_range(100, 110);
> > +		timeout++;
> > +	}
> > +}
> 
> Can this be implemented using poll_timeout_us() ?

Sure, thanks.