[U-Boot] [linux-sunxi] [PATCH 3/2] net/designware: reorder struct dw_eth_dev to pack more efficiently.

[Siarhei Siamashka]

On Thu, 01 May 2014 20:01:58 +0100
Ian Campbell <ijc at 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 at hellion.org.uk>
> Cc: Alexey Brodkin <abrodkin at 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 :-)

-- 
Best regards,
Siarhei Siamashka