[PATCH net-next] net: stmmac: bump tc when get underflow error from DMA descriptor

[PATCH net-next] net: stmmac: bump tc when get underflow error from DMA descriptor

[Xiaoliang Yang]

In DMA threshold mode, frame underflow errors may sometimes occur when
the TC(threshold control) value is not enough. The TC value need to be
bumped up in this case.

There is no underflow interrupt bit on DMA_CH(#i)_Status of dwmac4, so
the DMA threshold cannot be bumped up in stmmac_dma_interrupt(). The
i.mx8mp board observed an underflow error while running NFS boot, the
NFS rootfs could not be mounted.

The underflow error can be got from the DMA descriptor TDES3 on dwmac4.
This patch bump up tc value once underflow error is got from TDES3.

Signed-off-by: Xiaoliang Yang <xiaoliang.yang_1@nxp.com>
---
 drivers/net/ethernet/stmicro/stmmac/common.h  |  1 +
 .../ethernet/stmicro/stmmac/dwmac4_descs.c    |  8 +--
 .../net/ethernet/stmicro/stmmac/stmmac_main.c | 51 ++++++++-----------
 3 files changed, 27 insertions(+), 33 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/common.h b/drivers/net/ethernet/stmicro/stmmac/common.h
index 9160f9ed363a..6b5d96bced47 100644
--- a/drivers/net/ethernet/stmicro/stmmac/common.h
+++ b/drivers/net/ethernet/stmicro/stmmac/common.h
@@ -317,6 +317,7 @@ enum tx_frame_status {
 	tx_not_ls = 0x1,
 	tx_err = 0x2,
 	tx_dma_own = 0x4,
+	tx_err_bump_tc = 0x8,
 };
 
 enum dma_irq_status {
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac4_descs.c b/drivers/net/ethernet/stmicro/stmmac/dwmac4_descs.c
index cbf4429fb1d2..d3b4765c1a5b 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac4_descs.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac4_descs.c
@@ -32,6 +32,8 @@ static int dwmac4_wrback_get_tx_status(void *data, struct stmmac_extra_stats *x,
 		return tx_not_ls;
 
 	if (unlikely(tdes3 & TDES3_ERROR_SUMMARY)) {
+		ret = tx_err;
+
 		if (unlikely(tdes3 & TDES3_JABBER_TIMEOUT))
 			x->tx_jabber++;
 		if (unlikely(tdes3 & TDES3_PACKET_FLUSHED))
@@ -53,16 +55,16 @@ static int dwmac4_wrback_get_tx_status(void *data, struct stmmac_extra_stats *x,
 		if (unlikely(tdes3 & TDES3_EXCESSIVE_DEFERRAL))
 			x->tx_deferred++;
 
-		if (unlikely(tdes3 & TDES3_UNDERFLOW_ERROR))
+		if (unlikely(tdes3 & TDES3_UNDERFLOW_ERROR)) {
 			x->tx_underflow++;
+			ret |= tx_err_bump_tc;
+		}
 
 		if (unlikely(tdes3 & TDES3_IP_HDR_ERROR))
 			x->tx_ip_header_error++;
 
 		if (unlikely(tdes3 & TDES3_PAYLOAD_ERROR))
 			x->tx_payload_error++;
-
-		ret = tx_err;
 	}
 
 	if (unlikely(tdes3 & TDES3_DEFERRED))
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index 4e05c1d92935..7e3e1bc0f61d 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -132,6 +132,8 @@ static irqreturn_t stmmac_msi_intr_tx(int irq, void *data);
 static irqreturn_t stmmac_msi_intr_rx(int irq, void *data);
 static void stmmac_tx_timer_arm(struct stmmac_priv *priv, u32 queue);
 static void stmmac_flush_tx_descriptors(struct stmmac_priv *priv, int queue);
+static void stmmac_set_dma_operation_mode(struct stmmac_priv *priv, u32 txmode,
+					  u32 rxmode, u32 chan);
 
 #ifdef CONFIG_DEBUG_FS
 static const struct net_device_ops stmmac_netdev_ops;
@@ -2466,6 +2468,21 @@ static bool stmmac_xdp_xmit_zc(struct stmmac_priv *priv, u32 queue, u32 budget)
 	return !!budget && work_done;
 }
 
+static void stmmac_bump_dma_threshold(struct stmmac_priv *priv, u32 chan)
+{
+	if (unlikely(priv->xstats.threshold != SF_DMA_MODE) && tc <= 256) {
+		tc += 64;
+
+		if (priv->plat->force_thresh_dma_mode)
+			stmmac_set_dma_operation_mode(priv, tc, tc, chan);
+		else
+			stmmac_set_dma_operation_mode(priv, tc, SF_DMA_MODE,
+						      chan);
+
+		priv->xstats.threshold = tc;
+	}
+}
+
 /**
  * stmmac_tx_clean - to manage the transmission completion
  * @priv: driver private structure
@@ -2531,6 +2548,8 @@ static int stmmac_tx_clean(struct stmmac_priv *priv, int budget, u32 queue)
 			/* ... verify the status error condition */
 			if (unlikely(status & tx_err)) {
 				priv->dev->stats.tx_errors++;
+				if (unlikely(status & tx_err_bump_tc))
+					stmmac_bump_dma_threshold(priv, queue);
 			} else {
 				priv->dev->stats.tx_packets++;
 				priv->xstats.tx_pkt_n++;
@@ -2781,21 +2800,7 @@ static void stmmac_dma_interrupt(struct stmmac_priv *priv)
 	for (chan = 0; chan < tx_channel_count; chan++) {
 		if (unlikely(status[chan] & tx_hard_error_bump_tc)) {
 			/* Try to bump up the dma threshold on this failure */
-			if (unlikely(priv->xstats.threshold != SF_DMA_MODE) &&
-			    (tc <= 256)) {
-				tc += 64;
-				if (priv->plat->force_thresh_dma_mode)
-					stmmac_set_dma_operation_mode(priv,
-								      tc,
-								      tc,
-								      chan);
-				else
-					stmmac_set_dma_operation_mode(priv,
-								    tc,
-								    SF_DMA_MODE,
-								    chan);
-				priv->xstats.threshold = tc;
-			}
+			stmmac_bump_dma_threshold(priv, chan);
 		} else if (unlikely(status[chan] == tx_hard_error)) {
 			stmmac_tx_err(priv, chan);
 		}
@@ -5745,21 +5750,7 @@ static irqreturn_t stmmac_msi_intr_tx(int irq, void *data)
 
 	if (unlikely(status & tx_hard_error_bump_tc)) {
 		/* Try to bump up the dma threshold on this failure */
-		if (unlikely(priv->xstats.threshold != SF_DMA_MODE) &&
-		    tc <= 256) {
-			tc += 64;
-			if (priv->plat->force_thresh_dma_mode)
-				stmmac_set_dma_operation_mode(priv,
-							      tc,
-							      tc,
-							      chan);
-			else
-				stmmac_set_dma_operation_mode(priv,
-							      tc,
-							      SF_DMA_MODE,
-							      chan);
-			priv->xstats.threshold = tc;
-		}
+		stmmac_bump_dma_threshold(priv, chan);
 	} else if (unlikely(status == tx_hard_error)) {
 		stmmac_tx_err(priv, chan);
 	}
-- 
2.17.1

RE: [PATCH net-next] net: stmmac: bump tc when get underflow error from DMA descriptor

[Joakim Zhang]

> -----Original Message-----
> From: Xiaoliang Yang <xiaoliang.yang_1@nxp.com>
> Sent: 2021年12月8日 18:07
> To: davem@davemloft.net; netdev@vger.kernel.org;
> linux-kernel@vger.kernel.org
> Cc: kuba@kernel.org; Joakim Zhang <qiangqing.zhang@nxp.com>;
> peppe.cavallaro@st.com; alexandre.torgue@foss.st.com;
> joabreu@synopsys.com; Yannick Vignon <yannick.vignon@nxp.com>;
> boon.leong.ong@intel.com; Jose.Abreu@synopsys.com; mst@redhat.com;
> sonic.zhang@analog.com; Joao.Pinto@synopsys.com; Mingkai Hu
> <mingkai.hu@nxp.com>; Leo Li <leoyang.li@nxp.com>; Xiaoliang Yang
> <xiaoliang.yang_1@nxp.com>
> Subject: [PATCH net-next] net: stmmac: bump tc when get underflow error
> from DMA descriptor
> 
> In DMA threshold mode, frame underflow errors may sometimes occur
> when the TC(threshold control) value is not enough. The TC value need to be
> bumped up in this case.
> 
> There is no underflow interrupt bit on DMA_CH(#i)_Status of dwmac4, so
> the DMA threshold cannot be bumped up in stmmac_dma_interrupt(). The
> i.mx8mp board observed an underflow error while running NFS boot, the
> NFS rootfs could not be mounted.
> 
> The underflow error can be got from the DMA descriptor TDES3 on dwmac4.
> This patch bump up tc value once underflow error is got from TDES3.
> 
> Signed-off-by: Xiaoliang Yang <xiaoliang.yang_1@nxp.com>

5 queues with FIFO cut-through mode can work well after applying this patch.

Tested-by/Reviewed-by: Joakim Zhang <qiangqing.zhang@nxp.com>

Best Regards,
Joakim Zhang
> ---
>  drivers/net/ethernet/stmicro/stmmac/common.h  |  1 +
>  .../ethernet/stmicro/stmmac/dwmac4_descs.c    |  8 +--
>  .../net/ethernet/stmicro/stmmac/stmmac_main.c | 51 ++++++++-----------
>  3 files changed, 27 insertions(+), 33 deletions(-)
> 
> diff --git a/drivers/net/ethernet/stmicro/stmmac/common.h
> b/drivers/net/ethernet/stmicro/stmmac/common.h
> index 9160f9ed363a..6b5d96bced47 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/common.h
> +++ b/drivers/net/ethernet/stmicro/stmmac/common.h
> @@ -317,6 +317,7 @@ enum tx_frame_status {
>  	tx_not_ls = 0x1,
>  	tx_err = 0x2,
>  	tx_dma_own = 0x4,
> +	tx_err_bump_tc = 0x8,
>  };
> 
>  enum dma_irq_status {
> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac4_descs.c
> b/drivers/net/ethernet/stmicro/stmmac/dwmac4_descs.c
> index cbf4429fb1d2..d3b4765c1a5b 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac4_descs.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac4_descs.c
> @@ -32,6 +32,8 @@ static int dwmac4_wrback_get_tx_status(void *data,
> struct stmmac_extra_stats *x,
>  		return tx_not_ls;
> 
>  	if (unlikely(tdes3 & TDES3_ERROR_SUMMARY)) {
> +		ret = tx_err;
> +
>  		if (unlikely(tdes3 & TDES3_JABBER_TIMEOUT))
>  			x->tx_jabber++;
>  		if (unlikely(tdes3 & TDES3_PACKET_FLUSHED)) @@ -53,16 +55,16
> @@ static int dwmac4_wrback_get_tx_status(void *data, struct
> stmmac_extra_stats *x,
>  		if (unlikely(tdes3 & TDES3_EXCESSIVE_DEFERRAL))
>  			x->tx_deferred++;
> 
> -		if (unlikely(tdes3 & TDES3_UNDERFLOW_ERROR))
> +		if (unlikely(tdes3 & TDES3_UNDERFLOW_ERROR)) {
>  			x->tx_underflow++;
> +			ret |= tx_err_bump_tc;
> +		}
> 
>  		if (unlikely(tdes3 & TDES3_IP_HDR_ERROR))
>  			x->tx_ip_header_error++;
> 
>  		if (unlikely(tdes3 & TDES3_PAYLOAD_ERROR))
>  			x->tx_payload_error++;
> -
> -		ret = tx_err;
>  	}
> 
>  	if (unlikely(tdes3 & TDES3_DEFERRED))
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> index 4e05c1d92935..7e3e1bc0f61d 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> @@ -132,6 +132,8 @@ static irqreturn_t stmmac_msi_intr_tx(int irq, void
> *data);  static irqreturn_t stmmac_msi_intr_rx(int irq, void *data);  static
> void stmmac_tx_timer_arm(struct stmmac_priv *priv, u32 queue);  static
> void stmmac_flush_tx_descriptors(struct stmmac_priv *priv, int queue);
> +static void stmmac_set_dma_operation_mode(struct stmmac_priv *priv,
> u32 txmode,
> +					  u32 rxmode, u32 chan);
> 
>  #ifdef CONFIG_DEBUG_FS
>  static const struct net_device_ops stmmac_netdev_ops; @@ -2466,6
> +2468,21 @@ static bool stmmac_xdp_xmit_zc(struct stmmac_priv *priv, u32
> queue, u32 budget)
>  	return !!budget && work_done;
>  }
> 
> +static void stmmac_bump_dma_threshold(struct stmmac_priv *priv, u32
> +chan) {
> +	if (unlikely(priv->xstats.threshold != SF_DMA_MODE) && tc <= 256) {
> +		tc += 64;
> +
> +		if (priv->plat->force_thresh_dma_mode)
> +			stmmac_set_dma_operation_mode(priv, tc, tc, chan);
> +		else
> +			stmmac_set_dma_operation_mode(priv, tc, SF_DMA_MODE,
> +						      chan);
> +
> +		priv->xstats.threshold = tc;
> +	}
> +}
> +
>  /**
>   * stmmac_tx_clean - to manage the transmission completion
>   * @priv: driver private structure
> @@ -2531,6 +2548,8 @@ static int stmmac_tx_clean(struct stmmac_priv
> *priv, int budget, u32 queue)
>  			/* ... verify the status error condition */
>  			if (unlikely(status & tx_err)) {
>  				priv->dev->stats.tx_errors++;
> +				if (unlikely(status & tx_err_bump_tc))
> +					stmmac_bump_dma_threshold(priv, queue);
>  			} else {
>  				priv->dev->stats.tx_packets++;
>  				priv->xstats.tx_pkt_n++;
> @@ -2781,21 +2800,7 @@ static void stmmac_dma_interrupt(struct
> stmmac_priv *priv)
>  	for (chan = 0; chan < tx_channel_count; chan++) {
>  		if (unlikely(status[chan] & tx_hard_error_bump_tc)) {
>  			/* Try to bump up the dma threshold on this failure */
> -			if (unlikely(priv->xstats.threshold != SF_DMA_MODE) &&
> -			    (tc <= 256)) {
> -				tc += 64;
> -				if (priv->plat->force_thresh_dma_mode)
> -					stmmac_set_dma_operation_mode(priv,
> -								      tc,
> -								      tc,
> -								      chan);
> -				else
> -					stmmac_set_dma_operation_mode(priv,
> -								    tc,
> -								    SF_DMA_MODE,
> -								    chan);
> -				priv->xstats.threshold = tc;
> -			}
> +			stmmac_bump_dma_threshold(priv, chan);
>  		} else if (unlikely(status[chan] == tx_hard_error)) {
>  			stmmac_tx_err(priv, chan);
>  		}
> @@ -5745,21 +5750,7 @@ static irqreturn_t stmmac_msi_intr_tx(int irq, void
> *data)
> 
>  	if (unlikely(status & tx_hard_error_bump_tc)) {
>  		/* Try to bump up the dma threshold on this failure */
> -		if (unlikely(priv->xstats.threshold != SF_DMA_MODE) &&
> -		    tc <= 256) {
> -			tc += 64;
> -			if (priv->plat->force_thresh_dma_mode)
> -				stmmac_set_dma_operation_mode(priv,
> -							      tc,
> -							      tc,
> -							      chan);
> -			else
> -				stmmac_set_dma_operation_mode(priv,
> -							      tc,
> -							      SF_DMA_MODE,
> -							      chan);
> -			priv->xstats.threshold = tc;
> -		}
> +		stmmac_bump_dma_threshold(priv, chan);
>  	} else if (unlikely(status == tx_hard_error)) {
>  		stmmac_tx_err(priv, chan);
>  	}
> --
> 2.17.1

Re: [PATCH net-next] net: stmmac: bump tc when get underflow error from DMA descriptor

[Jakub Kicinski]

On Thu, 9 Dec 2021 01:31:52 +0000 Joakim Zhang wrote:
> > net: stmmac: bump tc when get underflow error from DMA descriptor
> > 
> > In DMA threshold mode, frame underflow errors may sometimes occur
> > when the TC(threshold control) value is not enough. The TC value need to be
> > bumped up in this case.
> > 
> > There is no underflow interrupt bit on DMA_CH(#i)_Status of dwmac4, so
> > the DMA threshold cannot be bumped up in stmmac_dma_interrupt(). The
> > i.mx8mp board observed an underflow error while running NFS boot, the
> > NFS rootfs could not be mounted.
> > 
> > The underflow error can be got from the DMA descriptor TDES3 on dwmac4.
> > This patch bump up tc value once underflow error is got from TDES3.
> > 
> > Signed-off-by: Xiaoliang Yang <xiaoliang.yang_1@nxp.com>  
> 
> 5 queues with FIFO cut-through mode can work well after applying this patch.

This never worked, correct? It's not a regression fix?

RE: [EXT] Re: [PATCH net-next] net: stmmac: bump tc when get underflow error from DMA descriptor

[Xiaoliang Yang]

Hi Jakub,

On Fri, 10 Dec 2021 10:41:00 +0800 Jakub Kicinski wrote:
> > > net: stmmac: bump tc when get underflow error from DMA descriptor
> > >
> > > In DMA threshold mode, frame underflow errors may sometimes occur
> > > when the TC(threshold control) value is not enough. The TC value
> > > need to be bumped up in this case.
> > >
> > > There is no underflow interrupt bit on DMA_CH(#i)_Status of dwmac4,
> > > so the DMA threshold cannot be bumped up in stmmac_dma_interrupt().
> > > The i.mx8mp board observed an underflow error while running NFS
> > > boot, the NFS rootfs could not be mounted.
> > >
> > > The underflow error can be got from the DMA descriptor TDES3 on
> dwmac4.
> > > This patch bump up tc value once underflow error is got from TDES3.
> > >
> > > Signed-off-by: Xiaoliang Yang <xiaoliang.yang_1@nxp.com>
> >
> > 5 queues with FIFO cut-through mode can work well after applying this
> patch.
> 
> This never worked, correct? It's not a regression fix?
Yes, it's never worked when the underflow error is observed in the case of NFS boot on i.mx8mp. I'm not sure if other SoC have same issue in this case, but I think it's necessary to increase the threshold value in case of underflow error.
Do you mean that I need to send the patch as a bug fix to net branch?

Regards,
Xiaoliang

Re: [EXT] Re: [PATCH net-next] net: stmmac: bump tc when get underflow error from DMA descriptor

[Jakub Kicinski]

On Fri, 10 Dec 2021 03:11:16 +0000 Xiaoliang Yang wrote:
> > > 5 queues with FIFO cut-through mode can work well after applying this patch.
> > 
> > This never worked, correct? It's not a regression fix?  
>
> Yes, it's never worked when the underflow error is observed in the
> case of NFS boot on i.mx8mp. I'm not sure if other SoC have same
> issue in this case, but I think it's necessary to increase the
> threshold value in case of underflow error.

Oh, so NFS boot works for the most part on i.mx8mp but under certain
conditions (or with certain configuration?) this error can be observed 
and the boot will fail?

> Do you mean that I need to send the patch as a bug fix to net branch?

Your call, if you would like for the patch to go to stable and LTS
releases -- then it need to be resent for net with a Fixes tag.

LMK if you prefer net or net-next.

RE: [EXT] Re: [PATCH net-next] net: stmmac: bump tc when get underflow error from DMA descriptor

[Xiaoliang Yang]

On Fri, 11 Dec 2021 00:07:13 Jakub Kicinski wrote:
> > > > 5 queues with FIFO cut-through mode can work well after applying this
> patch.
> > >
> > > This never worked, correct? It's not a regression fix?
> >
> > Yes, it's never worked when the underflow error is observed in the
> > case of NFS boot on i.mx8mp. I'm not sure if other SoC have same issue
> > in this case, but I think it's necessary to increase the threshold
> > value in case of underflow error.
> 
> Oh, so NFS boot works for the most part on i.mx8mp but under certain
> conditions (or with certain configuration?) this error can be observed and the
> boot will fail?
Yes, when configure the DMA as threshold mode, this error can be observed.

> 
> > Do you mean that I need to send the patch as a bug fix to net branch?
> 
> Your call, if you would like for the patch to go to stable and LTS releases -- then
> it need to be resent for net with a Fixes tag.
> 
> LMK if you prefer net or net-next.
Because this error requires a certain condition in DTS file, and I haven't update the DTS file. So I prefer to add the patch in net-next, thanks.

Re: [PATCH net-next] net: stmmac: bump tc when get underflow error from DMA descriptor

[patchwork-bot+netdevbpf]

Hello:

This patch was applied to netdev/net-next.git (master)
by David S. Miller <davem@davemloft.net>:

On Wed,  8 Dec 2021 18:06:51 +0800 you wrote:
> In DMA threshold mode, frame underflow errors may sometimes occur when
> the TC(threshold control) value is not enough. The TC value need to be
> bumped up in this case.
> 
> There is no underflow interrupt bit on DMA_CH(#i)_Status of dwmac4, so
> the DMA threshold cannot be bumped up in stmmac_dma_interrupt(). The
> i.mx8mp board observed an underflow error while running NFS boot, the
> NFS rootfs could not be mounted.
> 
> [...]

Here is the summary with links:
  - [net-next] net: stmmac: bump tc when get underflow error from DMA descriptor
    https://git.kernel.org/netdev/net-next/c/3a6c12a0c6c3

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html