[PATCH v1 0/1] Adjust macb max_tx_len for mpfs

[PATCH v1 0/1] Adjust macb max_tx_len for mpfs

[daire.mcnamara]

From: Daire McNamara <daire.mcnamara@microchip.com>

Several customers have reported unexpected ethernet issues whereby
the GEM stops transmitting and receiving. Performing an action such
as ifconfig <ethX> down; ifconfig <ethX> up clears this particular
condition.

The origin of the issue is a stream of AMBA_ERRORS (bit 6) from the
tx queues.

This patch sets the max_tx_length to SRAM size (16 KiB
in the case of mpfs) divided by num_queues (4 in the case of mpfs)
and then subtracts 56 bytes from that figure - resulting in max_tx_len
of 4040.  The max jumbo length is also set to 4040.  These figures
are derived from Cadence erratum 1686.

Daire McNamara (1):
  net: macb: Shorten max_tx_len to 4KiB - 56 on mpfs

 drivers/net/ethernet/cadence/macb.h      |  1 +
 drivers/net/ethernet/cadence/macb_main.c | 16 ++++++++++++----
 2 files changed, 13 insertions(+), 4 deletions(-)


base-commit: 7a124d3a4a346563967b8887882858e5ef46f030
prerequisite-patch-id: 08908afdfe3b919f92560dd4306178c5f3f588dd
-- 
2.25.1

[PATCH v1 1/1] net: macb: Shorten max_tx_len to 4KiB - 56 on mpfs

[daire.mcnamara]

From: Daire McNamara <daire.mcnamara@microchip.com>

On mpfs, with SRAM configured for 4 queues, setting max_tx_len
to GEM_TX_MAX_LEN=0x3f0 results multiple AMBA errors.
Setting max_tx_len to (4KiB - 56) removes those errors.

The details are described in erratum 1686 by Cadence

The max jumbo frame size is also reduced for mpfs to (4KiB - 56).

Signed-off-by: Daire McNamara <daire.mcnamara@microchip.com>
---
 drivers/net/ethernet/cadence/macb.h      |  1 +
 drivers/net/ethernet/cadence/macb_main.c | 16 ++++++++++++----
 2 files changed, 13 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/cadence/macb.h b/drivers/net/ethernet/cadence/macb.h
index 14dfec4db8f9..989e7c5db9b9 100644
--- a/drivers/net/ethernet/cadence/macb.h
+++ b/drivers/net/ethernet/cadence/macb.h
@@ -1175,6 +1175,7 @@ struct macb_config {
 			    struct clk **hclk, struct clk **tx_clk,
 			    struct clk **rx_clk, struct clk **tsu_clk);
 	int	(*init)(struct platform_device *pdev);
+	unsigned int		max_tx_length;
 	int	jumbo_max_len;
 	const struct macb_usrio_config *usrio;
 };
diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
index 66e30561569e..1f362bbc360f 100644
--- a/drivers/net/ethernet/cadence/macb_main.c
+++ b/drivers/net/ethernet/cadence/macb_main.c
@@ -4095,14 +4095,12 @@ static int macb_init(struct platform_device *pdev)
 
 	/* setup appropriated routines according to adapter type */
 	if (macb_is_gem(bp)) {
-		bp->max_tx_length = GEM_MAX_TX_LEN;
 		bp->macbgem_ops.mog_alloc_rx_buffers = gem_alloc_rx_buffers;
 		bp->macbgem_ops.mog_free_rx_buffers = gem_free_rx_buffers;
 		bp->macbgem_ops.mog_init_rings = gem_init_rings;
 		bp->macbgem_ops.mog_rx = gem_rx;
 		dev->ethtool_ops = &gem_ethtool_ops;
 	} else {
-		bp->max_tx_length = MACB_MAX_TX_LEN;
 		bp->macbgem_ops.mog_alloc_rx_buffers = macb_alloc_rx_buffers;
 		bp->macbgem_ops.mog_free_rx_buffers = macb_free_rx_buffers;
 		bp->macbgem_ops.mog_init_rings = macb_init_rings;
@@ -4839,7 +4837,8 @@ static const struct macb_config mpfs_config = {
 	.clk_init = macb_clk_init,
 	.init = init_reset_optional,
 	.usrio = &macb_default_usrio,
-	.jumbo_max_len = 10240,
+	.max_tx_length = 4040, /* Cadence Erratum 1686 */
+	.jumbo_max_len = 4040,
 };
 
 static const struct macb_config sama7g5_gem_config = {
@@ -4986,8 +4985,17 @@ static int macb_probe(struct platform_device *pdev)
 	bp->tx_clk = tx_clk;
 	bp->rx_clk = rx_clk;
 	bp->tsu_clk = tsu_clk;
-	if (macb_config)
+	if (macb_config) {
+		if (macb_is_gem(bp)) {
+			if (macb_config->max_tx_length)
+				bp->max_tx_length = macb_config->max_tx_length;
+			else
+				bp->max_tx_length = GEM_MAX_TX_LEN;
+		} else {
+			bp->max_tx_length = MACB_MAX_TX_LEN;
+		}
 		bp->jumbo_max_len = macb_config->jumbo_max_len;
+	}
 
 	bp->wol = 0;
 	if (of_property_read_bool(np, "magic-packet"))
-- 
2.25.1

Re: [PATCH v1 1/1] net: macb: Shorten max_tx_len to 4KiB - 56 on mpfs

[Samudrala, Sridhar]

On 4/13/2023 1:03 PM, daire.mcnamara@microchip.com wrote:
> From: Daire McNamara <daire.mcnamara@microchip.com>
> 
> On mpfs, with SRAM configured for 4 queues, setting max_tx_len
> to GEM_TX_MAX_LEN=0x3f0 results multiple AMBA errors.
> Setting max_tx_len to (4KiB - 56) removes those errors.
> 
> The details are described in erratum 1686 by Cadence
> 
> The max jumbo frame size is also reduced for mpfs to (4KiB - 56).
> 
> Signed-off-by: Daire McNamara <daire.mcnamara@microchip.com>

Reviewed-by: Sridhar Samudrala <sridhar.samudrala@intel.com>

Re: [PATCH v1 1/1] net: macb: Shorten max_tx_len to 4KiB - 56 on mpfs

[Nicolas.Ferre]

On 13/04/2023 at 20:03, daire.mcnamara@microchip.com wrote:
> From: Daire McNamara <daire.mcnamara@microchip.com>
> 
> On mpfs, with SRAM configured for 4 queues, setting max_tx_len
> to GEM_TX_MAX_LEN=0x3f0 results multiple AMBA errors.
> Setting max_tx_len to (4KiB - 56) removes those errors.
> 
> The details are described in erratum 1686 by Cadence
> 
> The max jumbo frame size is also reduced for mpfs to (4KiB - 56).
> 
> Signed-off-by: Daire McNamara <daire.mcnamara@microchip.com>

Looks good to me:
Acked-by: Nicolas Ferre <nicolas.ferre@microchip.com>

Best regards,
   Nicolas

> ---
>   drivers/net/ethernet/cadence/macb.h      |  1 +
>   drivers/net/ethernet/cadence/macb_main.c | 16 ++++++++++++----
>   2 files changed, 13 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/net/ethernet/cadence/macb.h b/drivers/net/ethernet/cadence/macb.h
> index 14dfec4db8f9..989e7c5db9b9 100644
> --- a/drivers/net/ethernet/cadence/macb.h
> +++ b/drivers/net/ethernet/cadence/macb.h
> @@ -1175,6 +1175,7 @@ struct macb_config {
>   			    struct clk **hclk, struct clk **tx_clk,
>   			    struct clk **rx_clk, struct clk **tsu_clk);
>   	int	(*init)(struct platform_device *pdev);
> +	unsigned int		max_tx_length;
>   	int	jumbo_max_len;
>   	const struct macb_usrio_config *usrio;
>   };
> diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
> index 66e30561569e..1f362bbc360f 100644
> --- a/drivers/net/ethernet/cadence/macb_main.c
> +++ b/drivers/net/ethernet/cadence/macb_main.c
> @@ -4095,14 +4095,12 @@ static int macb_init(struct platform_device *pdev)
>   
>   	/* setup appropriated routines according to adapter type */
>   	if (macb_is_gem(bp)) {
> -		bp->max_tx_length = GEM_MAX_TX_LEN;
>   		bp->macbgem_ops.mog_alloc_rx_buffers = gem_alloc_rx_buffers;
>   		bp->macbgem_ops.mog_free_rx_buffers = gem_free_rx_buffers;
>   		bp->macbgem_ops.mog_init_rings = gem_init_rings;
>   		bp->macbgem_ops.mog_rx = gem_rx;
>   		dev->ethtool_ops = &gem_ethtool_ops;
>   	} else {
> -		bp->max_tx_length = MACB_MAX_TX_LEN;
>   		bp->macbgem_ops.mog_alloc_rx_buffers = macb_alloc_rx_buffers;
>   		bp->macbgem_ops.mog_free_rx_buffers = macb_free_rx_buffers;
>   		bp->macbgem_ops.mog_init_rings = macb_init_rings;
> @@ -4839,7 +4837,8 @@ static const struct macb_config mpfs_config = {
>   	.clk_init = macb_clk_init,
>   	.init = init_reset_optional,
>   	.usrio = &macb_default_usrio,
> -	.jumbo_max_len = 10240,
> +	.max_tx_length = 4040, /* Cadence Erratum 1686 */
> +	.jumbo_max_len = 4040,
>   };
>   
>   static const struct macb_config sama7g5_gem_config = {
> @@ -4986,8 +4985,17 @@ static int macb_probe(struct platform_device *pdev)
>   	bp->tx_clk = tx_clk;
>   	bp->rx_clk = rx_clk;
>   	bp->tsu_clk = tsu_clk;
> -	if (macb_config)
> +	if (macb_config) {
> +		if (macb_is_gem(bp)) {
> +			if (macb_config->max_tx_length)
> +				bp->max_tx_length = macb_config->max_tx_length;
> +			else
> +				bp->max_tx_length = GEM_MAX_TX_LEN;
> +		} else {
> +			bp->max_tx_length = MACB_MAX_TX_LEN;
> +		}
>   		bp->jumbo_max_len = macb_config->jumbo_max_len;
> +	}
>   
>   	bp->wol = 0;
>   	if (of_property_read_bool(np, "magic-packet"))

-- 
Nicolas Ferre

RE: [PATCH v1 1/1] net: macb: Shorten max_tx_len to 4KiB - 56 on mpfs

[Katakam, Harini]

> -----Original Message-----
> From: Nicolas.Ferre@microchip.com <Nicolas.Ferre@microchip.com>
> Sent: Friday, April 14, 2023 2:30 PM
> To: Daire.McNamara@microchip.com; Claudiu.Beznea@microchip.com;
> davem@davemloft.net; edumazet@google.com; kuba@kernel.org;
> pabeni@redhat.com; netdev@vger.kernel.org;
> Conor.Dooley@microchip.com; Nicolas.Ferre@microchip.com
> Cc: Katakam, Harini <harini.katakam@amd.com>; Simek, Michal
> <michal.simek@amd.com>; roman.gushchin@linux.dev;
> jacob.e.keller@intel.com
> Subject: Re: [PATCH v1 1/1] net: macb: Shorten max_tx_len to 4KiB - 56 on
> mpfs
> 
> On 13/04/2023 at 20:03, daire.mcnamara@microchip.com wrote:
> > From: Daire McNamara <daire.mcnamara@microchip.com>
> >
> > On mpfs, with SRAM configured for 4 queues, setting max_tx_len to
> > GEM_TX_MAX_LEN=0x3f0 results multiple AMBA errors.
> > Setting max_tx_len to (4KiB - 56) removes those errors.
> >
> > The details are described in erratum 1686 by Cadence
> >
> > The max jumbo frame size is also reduced for mpfs to (4KiB - 56).
> >
> > Signed-off-by: Daire McNamara <daire.mcnamara@microchip.com>
> 
> Looks good to me:
> Acked-by: Nicolas Ferre <nicolas.ferre@microchip.com>
 
Thanks
Reviewed-by: Harini Katakam <harini.katakam@amd.com>

Regards,
Harini

Re: [PATCH v1 1/1] net: macb: Shorten max_tx_len to 4KiB - 56 on mpfs

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 1793 bytes --]

On Thu, Apr 13, 2023 at 07:03:37PM +0100, daire.mcnamara@microchip.com wrote:
> From: Daire McNamara <daire.mcnamara@microchip.com>
> 
> On mpfs, with SRAM configured for 4 queues, setting max_tx_len
> to GEM_TX_MAX_LEN=0x3f0 results multiple AMBA errors.
> Setting max_tx_len to (4KiB - 56) removes those errors.
> 
> The details are described in erratum 1686 by Cadence
> 
> The max jumbo frame size is also reduced for mpfs to (4KiB - 56).
> 
> Signed-off-by: Daire McNamara <daire.mcnamara@microchip.com>

I guess this also needs to be given a CC: stable, but I am not really
sure what to pick as a fix commit for this. It should go back as far as
the mpfs compatible existed (it was introduced in 6.0), but the commit
adding the compatible didn't introduce the problem.
The default GEM limit, which we used before I added reset support, is
also too large for us.

>  static const struct macb_config sama7g5_gem_config = {
> @@ -4986,8 +4985,17 @@ static int macb_probe(struct platform_device *pdev)
>  	bp->tx_clk = tx_clk;
>  	bp->rx_clk = rx_clk;
>  	bp->tsu_clk = tsu_clk;
> -	if (macb_config)
> +	if (macb_config) {
> +		if (macb_is_gem(bp)) {

I don't think this here is correct. This is being done before we have
configured the caps, so macb_is_gem() is always going to return false.

AFAICT, this should instead use hw_is_gem(bp->regs, bp->native_io).

Cheers,
Conor.

> +			if (macb_config->max_tx_length)
> +				bp->max_tx_length = macb_config->max_tx_length;
> +			else
> +				bp->max_tx_length = GEM_MAX_TX_LEN;
> +		} else {
> +			bp->max_tx_length = MACB_MAX_TX_LEN;
> +		}
>  		bp->jumbo_max_len = macb_config->jumbo_max_len;
> +	}
>  
>  	bp->wol = 0;
>  	if (of_property_read_bool(np, "magic-packet"))
> -- 
> 2.25.1
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]