On Thu, 01 May 2014 20:01:58 +0100 Ian Campbell ijc@hellion.org.uk wrote:
The {tx,rx}_mac_descrtable fields are aligned to ARCH_DMA_MINALIGN, which could be 256 or even larger. That means there is a potentially huge hole in the struct before those fields, so move them to the front where they are better packed.
Moving them to the front also helps ensure that so long as dw_eth_dev is properly aligned (which it is since "net/designware: ensure device private data is DMA aligned.") the {tx,rx}_mac_descrtable will be too, or at least avoids having to worry too much about compiler specifics.
Signed-off-by: Ian Campbell ijc@hellion.org.uk Cc: Alexey Brodkin abrodkin@synopsys.com
drivers/net/designware.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/net/designware.h b/drivers/net/designware.h index 382b0c7..6d94b3a 100644 --- a/drivers/net/designware.h +++ b/drivers/net/designware.h @@ -215,13 +215,13 @@ struct dmamacdescr { #endif
struct dw_eth_dev {
- struct dmamacdescr tx_mac_descrtable[CONFIG_TX_DESCR_NUM];
- struct dmamacdescr rx_mac_descrtable[CONFIG_RX_DESCR_NUM];
- u32 interface; u32 tx_currdescnum; u32 rx_currdescnum;
- struct dmamacdescr tx_mac_descrtable[CONFIG_TX_DESCR_NUM];
- struct dmamacdescr rx_mac_descrtable[CONFIG_RX_DESCR_NUM];
- char txbuffs[TX_TOTAL_BUFSIZE]; char rxbuffs[RX_TOTAL_BUFSIZE];
After this reordering, txbuffs and rxbuffs buffers become DMA unaligned. And they are also used with the cache flush/invalidate operations all over the place, causing all the same "v7_dcache_inval_range - start address is not aligned" failures.
The txbuffs/rxbuffs buffers probably should immediately follow dmamacdescr structs and also have their own alignment enforcement attribute.
As for the buffer sizes, we have the following defines:
#define CONFIG_TX_DESCR_NUM 16 #define CONFIG_RX_DESCR_NUM 16 #define CONFIG_ETH_BUFSIZE 2048 #define TX_TOTAL_BUFSIZE (CONFIG_ETH_BUFSIZE * CONFIG_TX_DESCR_NUM) #define RX_TOTAL_BUFSIZE (CONFIG_ETH_BUFSIZE * CONFIG_RX_DESCR_NUM)
CONFIG_ETH_BUFSIZE is a power of two, which is good. Still maybe an extra assertion check to verify/confirm that it is divisible by the cache line size would make the code cleaner? But that's just a nitpick, because the "v7_dcache_inval_range" function is noisy enough if anything is wrong :-)