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...
