[PATCH] net: dwc_eth_qos: Pad descriptors to cacheline size

[Stephen Warren]

On 4/29/20 1:14 PM, Marek Vasut wrote:
> The DWMAC4 IP has the possibility to skip up to 7 64bit words after
> the descriptor, use this to pad the descriptors to cacheline size.

> diff --git a/drivers/net/dwc_eth_qos.c b/drivers/net/dwc_eth_qos.c

>  /* Descriptors */
> +/*

Nit: Blank line between those two comments to match the existing style.

> + * #if EQOS_DESCRIPTOR_SIZE < ARCH_DMA_MINALIGN fails if we use sizeof()
> + * or offsetof() to calculate descriptor size, since neither is allowed
> + * in C preprocessor macros, so just hard-code this to 16=4*sizeof(u32).
> + */
> +#define EQOS_DESCRIPTOR_SIZE	16
> +#define EQOS_DESCRIPTOR_PAD	((ARCH_DMA_MINALIGN - EQOS_DESCRIPTOR_SIZE) / 4)
> +struct eqos_desc {
> +	u32 des0;
> +	u32 des1;
> +	u32 des2;
> +	u32 des3;
> +	u32 __pad[EQOS_DESCRIPTOR_PAD];
> +};

I think the padding needs to be optional based on the same condition in
the following existing warning logic, and the warning logic may need to
be removed/updated to account for the fact that padding is now being
used to avoid the issue:

(quoting this from existing code)
> /*
>  * Warn if the cache-line size is larger than the descriptor size. In such
>  * cases the driver will likely fail because the CPU needs to flush the cache
>  * when requeuing RX buffers, therefore descriptors written by the hardware
>  * may be discarded. Architectures with full IO coherence, such as x86, do not
>  * experience this issue, and hence are excluded from this condition.
>  *
>  * This can be fixed by defining CONFIG_SYS_NONCACHED_MEMORY which will cause
>  * the driver to allocate descriptors from a pool of non-cached memory.
>  */
> #if EQOS_DESCRIPTOR_SIZE < ARCH_DMA_MINALIGN
> #if !defined(CONFIG_SYS_NONCACHED_MEMORY) && \
>         !CONFIG_IS_ENABLED(SYS_DCACHE_OFF) && !defined(CONFIG_X86)
> #warning Cache line size is larger than descriptor size
> #endif
> #endif

(back to quoting from the patch)
> -#define EQOS_DESCRIPTOR_WORDS	4
> -#define EQOS_DESCRIPTOR_SIZE	(EQOS_DESCRIPTOR_WORDS * 4)
...
> -struct eqos_desc {
> -	u32 des0;
> -	u32 des1;
> -	u32 des2;
> -	u32 des3;
> -};

This patch would be a lot smaller if those definitions were kept in
their existing location instead of moving them...