[U-Boot] [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.
Signed-off-by: Ian Campbell ijc@hellion.org.uk Cc: Alexey Brodkin abrodkin@synopsys.com --- 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(-)
diff --git a/drivers/net/designware.c b/drivers/net/designware.c index b70df82..02ceb91 100644 --- a/drivers/net/designware.c +++ b/drivers/net/designware.c @@ -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);
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];
On Thursday, May 01, 2014 at 09:01:58 PM, Ian Campbell 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
Acked-by: Marek Vasut marex@denx.de
Best regards, Marek Vasut
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 :-)
On Wed, 2014-05-14 at 10:44 +0300, Siarhei Siamashka wrote:
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.
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.
On Wed, 14 May 2014 08:52:50 +0100 Ian Campbell ijc@hellion.org.uk wrote:
On Wed, 2014-05-14 at 10:44 +0300, Siarhei Siamashka wrote:
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.
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.
On Wed, 14 May 2014 11:01:59 +0300 Siarhei Siamashka siarhei.siamashka@gmail.com wrote:
On Wed, 14 May 2014 08:52:50 +0100 Ian Campbell ijc@hellion.org.uk wrote:
On Wed, 2014-05-14 at 10:44 +0300, Siarhei Siamashka wrote:
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.
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.
On Wed, 2014-05-14 at 11:32 +0300, Siarhei Siamashka wrote:
On Wed, 14 May 2014 11:01:59 +0300 Siarhei Siamashka siarhei.siamashka@gmail.com wrote:
On Wed, 14 May 2014 08:52:50 +0100 Ian Campbell ijc@hellion.org.uk wrote:
On Wed, 2014-05-14 at 10:44 +0300, Siarhei Siamashka wrote:
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.
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.
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.
On Wednesday, May 14, 2014 at 10:01:59 AM, Siarhei Siamashka wrote:
On Wed, 14 May 2014 08:52:50 +0100
Ian Campbell ijc@hellion.org.uk wrote:
On Wed, 2014-05-14 at 10:44 +0300, Siarhei Siamashka wrote:
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.
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
participants (3)
-
Ian Campbell -
Marek Vasut -
Siarhei Siamashka