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

[Conor Dooley <conor.dooley@microchip.com>]

[-- 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 --]