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