net/designware: ensure cache invalidations are aligned to ARCH_DMA_MINALIGN
PATCH v2 2/2 net/designware: ensure cache invalidations are aligned to ARCH_DMA_MINALIGN
This is required at least on ARM.
When sending instead of simply invalidating the entire descriptor, flush as little as possible while still respecting ARCH_DMA_MINALIGN, as requested by Alexey.
---
v2: - collapsed "net/designware: align cache invalidation on rx" and
"net/designware: invalidate entire descriptor in dw_eth_send" into one.
- roundup sizeof(txrx_status) to ARCH_DMA_MINALIGN instead of just invalidating the entire descriptor.
--- drivers/net/designware.c | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-)
drivers/net/designware.c +13 -5
@@ -280,10 +280,18 @@ static int dw_eth_send(struct eth_device *dev, void *packet, int length)u32 desc_num = priv->tx_currdescnum;struct dmamacdescr *desc_p = &priv->tx_mac_descrtable[desc_num]; -/* Invalidate only "status" field for the following check */ -invalidate_dcache_range ((unsigned long )&desc_p->txrx_status, - (unsigned long )&desc_p->txrx_status + -sizeof (desc_p->txrx_status)); +/* +* Strictly we only need to invalidate the "txrx_status" field +* for the following check, but on some platforms we cannot +* invalidate only 4 bytes, so roundup to +* ARCH_DMA_MINALIGN. This is safe because the individual +* descriptors in the array are each aligned to +* ARCH_DMA_MINALIGN. +*/ +invalidate_dcache_range ( + (unsigned long )desc_p, + (unsigned long )desc_p + +roundup (sizeof (desc_p->txrx_status),ARCH_DMA_MINALIGN ));/* Check if the descriptor is owned by CPU */ if (desc_p->txrx_status &DESC_TXSTS_OWNBYDMA ) {
@@ -351,7 +359,7 @@ static int dw_eth_recv(struct eth_device *dev)/* Invalidate received data */ invalidate_dcache_range ((unsigned long )desc_p->dmamac_addr, (unsigned long )desc_p->dmamac_addr + - length); +roundup (length,ARCH_DMA_MINALIGN ));NetReceive (desc_p->dmamac_addr, length);
Signature · 1 line
1.9.0
[U-Boot] [PATCH 3/2] net/designware: reorder struct dw_eth_dev to pack more efficiently.
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.
--- drivers/net/designware.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
drivers/net/designware.h +3 -3
@@ -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 ];
Signature · 1 line
1.9.0
Marek Vasut
Acked-by[U-Boot] [PATCH 3/2] net/designware: reorder struct dw_eth_dev to pack more efficiently.
Quoted 5 lines · Ian Campbell
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.
Quoted 7 lines
Moving them to the front also helps ensure that so long as
dw_eth_devis 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 <[email protected]>Cc Alexey Brodkin <[email protected]>
Best regards,
Marek Vasut
[U-Boot] [linux-sunxi] [PATCH 3/2] net/designware: reorder struct dw_eth_dev to pack more efficiently.
In reply to Ian CampbellQuoted 26 lines · On Thu, 01 May 2014 20:01:58 +0100 Ian Campbell <[email protected]> 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_devis 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 <[email protected]>Cc Alexey Brodkin <[email protected]>--- drivers/net/designware.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)drivers/net/designware.h +2
-0@@ -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 ];
drivers/net/designware.h struct dmamacdescr { · line 226
Quoted 10 lines
drivers/net/designware.h +1
-3+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 :-)
Signature · 2 lines
Best regards, Siarhei Siamashka
[U-Boot] [linux-sunxi] [PATCH 3/2] net/designware: reorder struct dw_eth_dev to pack more efficiently.
Quoted 15 lines · Siarhei Siamashka
Quoted 15 lines
drivers/net/designware.h +3
-0@@ -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;
Quoted 7 lines
drivers/net/designware.h struct dmamacdescr { · line 226
Quoted 5 lines
drivers/net/designware.h +0
-3-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.
Right, this is fixed in the repost which is part of "[PATCH v3 0/5] net/designware: fixes for data cache, phylib and burst size".
Ian.
[U-Boot] [linux-sunxi] [PATCH 3/2] net/designware: reorder struct dw_eth_dev to pack more efficiently.
Quoted 24 lines · On Wed, 14 May 2014 08:52:50 +0100 Ian Campbell <[email protected]> wrote:
Quoted 15 lines · Siarhei Siamashka
Quoted 15 lines
drivers/net/designware.h +3
-0@@ -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;Quoted 7 lines
drivers/net/designware.h struct dmamacdescr { · line 226
Quoted 5 lines
drivers/net/designware.h +0
-3-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.
Quoted 2 lines
Right, this is fixed in the repost which is part of "[PATCH v3 0/5] net/designware: fixes for data cache, phylib and burst size".
Oh, you just forgot to add the linux-sunxi list to CC when sending the updated patches. This explains why I don't see them in my mailbox.
Signature · 2 lines
Best regards, Siarhei Siamashka
[U-Boot] [linux-sunxi] [PATCH 3/2] net/designware: reorder struct dw_eth_dev to pack more efficiently.
Quoted 27 lines · On Wed, 14 May 2014 11:01:59 +0300 Siarhei Siamashka…
Quoted 24 lines · On Wed, 14 May 2014 08:52:50 +0100 Ian Campbell <[email protected]> wrote:
Quoted 22 lines · Siarhei Siamashka
Quoted 10 lines
drivers/net/designware.h +2
-0@@ -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 ];drivers/net/designware.h struct dmamacdescr { · line 226
Quoted 10 lines
drivers/net/designware.h +1
-3+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.
Quoted 5 lines
Quoted 2 lines
Right, this is fixed in the repost which is part of "[PATCH v3 0/5] net/designware: fixes for data cache, phylib and burst size".
Oh, you just forgot to add the linux-sunxi list to CC when sending the updated patches. This explains why I don't see them in my mailbox.
However, as I can see at
http://lists.denx.de/pipermail/u-boot/2014-May/179218.html
you are still sandwiching interface, tx_currdescnum and rx_currdescnum between the DMA aligned stuff. This does not really improve packing.
Signature · 2 lines
Best regards, Siarhei Siamashka
[U-Boot] [linux-sunxi] [PATCH 3/2] net/designware: reorder struct dw_eth_dev to pack more efficiently.
Quoted 33 lines · Siarhei Siamashka
Quoted 30 lines · On Wed, 14 May 2014 11:01:59 +0300 Siarhei Siamashka…
Quoted 26 lines · On Wed, 14 May 2014 08:52:50 +0100 Ian Campbell <[email protected]> wrote:
Quoted 15 lines · Siarhei Siamashka
Quoted 15 lines
drivers/net/designware.h +3
-0@@ -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;Quoted 7 lines
drivers/net/designware.h struct dmamacdescr { · line 226
Quoted 5 lines
drivers/net/designware.h +0
-3-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.
Right, this is fixed in the repost which is part of "[PATCH v3 0/5] net/designware: fixes for data cache, phylib and burst size".
Quoted 7 lines
Quoted 2 lines
Oh, you just forgot to add the linux-sunxi list to CC when sending the updated patches. This explains why I don't see them in my mailbox.
However, as I can see at
http://lists.denx.de/pipermail/u-boot/2014-May/179218.htmlyou are still sandwiching interface,
tx_currdescnumandrx_currdescnumbetween the DMA aligned stuff. This does not really improve packing.
Yes, I thought I had also moved them, perhaps I forgot to refresh the patch before git send-email.
I'll take a look and resend (not today thought probably).
Ian.
[U-Boot] [linux-sunxi] [PATCH 3/2] net/designware: reorder struct dw_eth_dev to pack more efficiently.
In reply to Siarhei SiamashkaQuoted 32 lines · Siarhei Siamashka
On Wed, 14 May 2014 08:52:50 +0100
Quoted 29 lines · Ian Campbell
Quoted 27 lines · Siarhei Siamashka
Quoted 22 lines
drivers/net/designware.h +3
-3@@ -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 ]; -drivers/net/designware.h struct dmamacdescr { · line 226
Quoted 2 lines
drivers/net/designware.h +0
-0char txbuffs[TX_TOTAL_BUFSIZE ];char rxbuffs[RX_TOTAL_BUFSIZE ];After this reordering, txbuffs and rxbuffs buffers become DMA unaligned.
Quoted 5 lines
Quoted 2 lines
Right, this is fixed in the repost which is part of "[PATCH v3 0/5] net/designware: fixes for data cache, phylib and burst size".
Oh, you just forgot to add the linux-sunxi list to CC when sending the updated patches. This explains why I don't see them in my mailbox.
Well yes, U-Boot patches usually don't go into Linux MLs ;-)
Best regards,
Marek Vasut