[U-Boot] [PATCH v3] net/designware: make driver compatible with data cache
From: Alexey Brodkin alexey.brod...@synopsys.com
Up until now this driver only worked with data cache disabled.
To make it work with enabled data cache following changes were required:
* Flush Tx/Rx buffer descriptors their modification
* Invalidate Tx/Rx buffer descriptors before reading its values
* Flush cache for data passed from CPU to GMAC
* Invalidate cache for data passed from GMAC to CPU
Signed-off-by: Alexey Brodkin abrod...@synopsys.com
Cc: Joe Hershberger joe.hershber...@ni.com
Cc: Vipin Kumar vipin.ku...@st.com
Cc: Stefan Roese s...@denx.de
Cc: Mischa Jonker mjon...@synopsys.com
Cc: Shiraz Hashim shiraz.has...@st.com
Cc: Albert ARIBAUD albert.u.b...@aribaud.net
Cc: Amit Virdi amit.vi...@st.com
Cc: Sonic Zhang sonic.zh...@analog.com
Compared to v2:
1. Removed trailing white space
---
drivers/net/designware.c | 53 +---
1 file changed, 50 insertions(+), 3 deletions(-)
diff --git a/drivers/net/designware.c b/drivers/net/designware.c
index 22155b4..c0c8659 100644
--- a/drivers/net/designware.c
+++ b/drivers/net/designware.c
@@ -51,6 +51,11 @@ static void tx_descs_init(struct eth_device *dev)
/* Correcting the last pointer of the chain */
desc_p-dmamac_next = desc_table_p[0];
+ /* Flush all Tx buffer descriptors at once */
+ flush_dcache_range((unsigned int)priv-tx_mac_descrtable,
+ (unsigned int)priv-tx_mac_descrtable +
+ sizeof(priv-tx_mac_descrtable));
+
writel((ulong)desc_table_p[0], dma_p-txdesclistaddr);
}
@@ -63,6 +68,15 @@ static void rx_descs_init(struct eth_device *dev)
struct dmamacdescr *desc_p;
u32 idx;
+ /* Before passing buffers to GMAC we need to make sure zeros
+* written there right after priv structure allocation were
+* flushed into RAM.
+* Otherwise there's a chance to get some of them flushed in RAM when
+* GMAC is already pushing data to RAM via DMA. This way incoming from
+* GMAC data will be corrupted. */
+ flush_dcache_range((unsigned int)rxbuffs, (unsigned int)rxbuffs +
+ RX_TOTAL_BUFSIZE);
+
for (idx = 0; idx CONFIG_RX_DESCR_NUM; idx++) {
desc_p = desc_table_p[idx];
desc_p-dmamac_addr = rxbuffs[idx * CONFIG_ETH_BUFSIZE];
@@ -78,6 +92,11 @@ static void rx_descs_init(struct eth_device *dev)
/* Correcting the last pointer of the chain */
desc_p-dmamac_next = desc_table_p[0];
+ /* Flush all Rx buffer descriptors at once */
+ flush_dcache_range((unsigned int)priv-rx_mac_descrtable,
+ (unsigned int)priv-rx_mac_descrtable +
+ sizeof(priv-rx_mac_descrtable));
+
writel((ulong)desc_table_p[0], dma_p-rxdesclistaddr);
}
@@ -197,6 +216,11 @@ 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));
+
/* Check if the descriptor is owned by CPU */
if (desc_p-txrx_status DESC_TXSTS_OWNBYDMA) {
printf(CPU not owner of tx frame\n);
@@ -205,6 +229,10 @@ static int dw_eth_send(struct eth_device *dev, void
*packet, int length)
memcpy((void *)desc_p-dmamac_addr, packet, length);
+ /* Flush data to be sent */
+ flush_dcache_range((unsigned long)desc_p-dmamac_addr,
+ (unsigned long)desc_p-dmamac_addr + length);
+
#if defined(CONFIG_DW_ALTDESCRIPTOR)
desc_p-txrx_status |= DESC_TXSTS_TXFIRST | DESC_TXSTS_TXLAST;
desc_p-dmamac_cntl |= (length DESC_TXCTRL_SIZE1SHFT) \
@@ -220,6 +248,10 @@ static int dw_eth_send(struct eth_device *dev, void
*packet, int length)
desc_p-txrx_status = DESC_TXSTS_OWNBYDMA;
#endif
+ /* Flush modified buffer descriptor */
+ flush_dcache_range((unsigned long)desc_p,
+ (unsigned long)desc_p + sizeof(struct dmamacdescr));
+
/* Test the wrap-around condition. */
if (++desc_num = CONFIG_TX_DESCR_NUM)
desc_num = 0;
@@ -235,18 +267,28 @@ static int dw_eth_send(struct eth_device *dev, void
*packet, int length)
static int dw_eth_recv(struct eth_device *dev)
{
struct dw_eth_dev *priv = dev-priv;
- u32 desc_num = priv-rx_currdescnum;
+ u32 status, desc_num = priv-rx_currdescnum;
struct dmamacdescr *desc_p = priv-rx_mac_descrtable[desc_num];
-
- u32 status = desc_p-txrx_status;
int length = 0;
+ /* Invalidate entire buffer descriptor */
+ invalidate_dcache_range((unsigned long)desc_p,
+
Re: [U-Boot] [PATCH v3] net/designware: make driver compatible with data cache
Hi Alexey, On 22.01.2014 17:49, Alexey Brodkin wrote: From: Alexey Brodkin alexey.brod...@synopsys.com Up until now this driver only worked with data cache disabled. To make it work with enabled data cache following changes were required: * Flush Tx/Rx buffer descriptors their modification * Invalidate Tx/Rx buffer descriptors before reading its values * Flush cache for data passed from CPU to GMAC * Invalidate cache for data passed from GMAC to CPU Signed-off-by: Alexey Brodkin abrod...@synopsys.com A small nitpicking comment below. Cc: Joe Hershberger joe.hershber...@ni.com Cc: Vipin Kumar vipin.ku...@st.com Cc: Stefan Roese s...@denx.de Cc: Mischa Jonker mjon...@synopsys.com Cc: Shiraz Hashim shiraz.has...@st.com Cc: Albert ARIBAUD albert.u.b...@aribaud.net Cc: Amit Virdi amit.vi...@st.com Cc: Sonic Zhang sonic.zh...@analog.com Compared to v2: 1. Removed trailing white space --- drivers/net/designware.c | 53 +--- 1 file changed, 50 insertions(+), 3 deletions(-) diff --git a/drivers/net/designware.c b/drivers/net/designware.c index 22155b4..c0c8659 100644 --- a/drivers/net/designware.c +++ b/drivers/net/designware.c @@ -51,6 +51,11 @@ static void tx_descs_init(struct eth_device *dev) /* Correcting the last pointer of the chain */ desc_p-dmamac_next = desc_table_p[0]; + /* Flush all Tx buffer descriptors at once */ + flush_dcache_range((unsigned int)priv-tx_mac_descrtable, + (unsigned int)priv-tx_mac_descrtable + + sizeof(priv-tx_mac_descrtable)); + writel((ulong)desc_table_p[0], dma_p-txdesclistaddr); } @@ -63,6 +68,15 @@ static void rx_descs_init(struct eth_device *dev) struct dmamacdescr *desc_p; u32 idx; + /* Before passing buffers to GMAC we need to make sure zeros +* written there right after priv structure allocation were +* flushed into RAM. +* Otherwise there's a chance to get some of them flushed in RAM when +* GMAC is already pushing data to RAM via DMA. This way incoming from +* GMAC data will be corrupted. */ Please use this recommended multi-line comment style: /* * Before ... * ... * * ... will be corrupted. */ Thanks, Stefan ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v3] net/designware: make driver compatible with data cache
Hello Alexey, In general, a very nice, clean patch. + /* Flush modified buffer descriptor */ + flush_dcache_range((unsigned long)desc_p, +(unsigned long)desc_p + sizeof(struct dmamacdescr)); + If I remember correctly, there is some bit that tells you if the DMA descriptor is owned by CPU or by GMAC. What if the descriptor size is smaller than the cache line size of the CPU? How do you prevent the CPU from overwriting adiacent DMA descriptors that may be owned by the GMAC? As far as I can remember, in Linux they solve this by mapping the descriptors (not the packet buffers, these are always cacheline aligned) in uncached memory, but we cannot do that in u-boot as the MMU is still disabled. OTOH, as we may not need to have the performance benefits of the CPU and GMAC concurrently accessing the descriptor table, we may be able to work around it by handing off multiple descriptors at once from GMAC to CPU and vice versa (maybe depending on cache line size?). I remember that a similar patch (that looked a lot uglier BTW) solved it by doing uncached accesses to the descriptors, but that would require using arch-specific accessor macro's (and I'm not sure if all architectures support an 'uncached access' attribute/flag with load/store instructions). Mischa ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v3] net/designware: make driver compatible with data cache
Hi Mischa, On Wed, 2014-01-22 at 17:10 +, Mischa Jonker wrote: Hello Alexey, In general, a very nice, clean patch. + /* Flush modified buffer descriptor */ + flush_dcache_range((unsigned long)desc_p, + (unsigned long)desc_p + sizeof(struct dmamacdescr)); + If I remember correctly, there is some bit that tells you if the DMA descriptor is owned by CPU or by GMAC. What if the descriptor size is smaller than the cache line size of the CPU? How do you prevent the CPU from overwriting adiacent DMA descriptors that may be owned by the GMAC? Well, good point. It might happen with long cache line easily. As far as I can remember, in Linux they solve this by mapping the descriptors (not the packet buffers, these are always cacheline aligned) in uncached memory, but we cannot do that in u-boot as the MMU is still disabled. OTOH, as we may not need to have the performance benefits of the CPU and GMAC concurrently accessing the descriptor table, we may be able to work around it by handing off multiple descriptors at once from GMAC to CPU and vice versa (maybe depending on cache line size?). Frankly I don't see a point in trying to process few descriptors at once. The only true failure safe scenario I may think of - CPU waits until all descriptors are processed by GMAC and then CPU processes all Tx or Rx descriptors. In this case CPU won't corrupt the next descriptor used currently by CPU. But this approach will kill benefits of multiple buffers - operation will become synchronous as if we have only 1 buffer for Tx and one for Rx. Which might be a resolution but with penalty of speed/time (which we don't want). I remember that a similar patch (that looked a lot uglier BTW) solved it by doing uncached accesses to the descriptors, but that would require using arch-specific accessor macro's (and I'm not sure if all architectures support an 'uncached access' attribute/flag with load/store instructions). Word/byte-aligned uncached access is sweet but not universal - how many arches have it? As a summary - this problem is very generic (I saw other places like DW MMC driver which use similar approach - flushing/invalidating buffer descriptors) and without true uncached access (be it with means of MMU or specific accessors without MMU) it's not clear how to act completely fail-safe. Might be we need just to add compile-time warning for cases when cache line is longer than one unit of interest (buffer descriptor in our case) so people at least will be informed and suggest to disable D$ for starters. -Alexey ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
