[U-Boot] [PATCH v3 0/5] net/designware: fixes for data cache, phylib and burst size

[U-Boot] [PATCH v3 0/5] net/designware: fixes for data cache, phylib and burst size

[Ian Campbell]

Pulling together various fixes for designware into a single series. I'm
calling this v3, although some of these never had a v2.

Most of these have been floating around for a few weeks, the cache line
issues in particular mean that the driver is unlikely to work on any ARM
system. I've been testing these on top of my sunxi mainlining series.

I've also thrown them onto gitorious. Pull-request details are below.

Thanks,
Ian.

The following changes since commit dda0dbfc69f3d560c87f5be85f127ed862ea6721:

  Prepare v2014.04 (2014-04-14 15:19:24 -0400)

are available in the git repository at:

  git://gitorious.org/ijc/u-boot.git designware

for you to fetch changes up to 5e3e5c37bc7a867ca42fa43f69ad764c8942b631:

  net/designware: Make DMA burst length configurable and reduce by default (2014-05-08 22:16:33 +0100)

----------------------------------------------------------------
Ian Campbell (5):
      net/designware: call phy_connect_dev() to properly setup phylib device
      net/designware: ensure device private data is DMA aligned.
      net/designware: ensure cache invalidations are aligned to ARCH_DMA_MINALIGN
      net/designware: reorder struct dw_eth_dev to pack more efficiently.
      net/designware: Make DMA burst length configurable and reduce by default

 drivers/net/designware.c | 25 ++++++++++++++++++-------
 drivers/net/designware.h | 22 +++++++++++-----------
 2 files changed, 29 insertions(+), 18 deletions(-)

[U-Boot] [PATCH v3 1/5] net/designware: call phy_connect_dev() to properly setup phylib device

[Ian Campbell]

This sets up the linkage from the phydev back to the ethernet device. This
symptom of not doing this which I noticed was:
    <NULL> Waiting for PHY auto negotiation to complete....
rather than:
    dwmac.1c50000 Waiting for PHY auto negotiation to complete....

Signed-off-by: Ian Campbell <ijc@hellion.org.uk>
Cc: Alexey Brodkin <Alexey.Brodkin@synopsys.com>
---
 drivers/net/designware.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/net/designware.c b/drivers/net/designware.c
index c45593b..78751b2 100644
--- a/drivers/net/designware.c
+++ b/drivers/net/designware.c
@@ -390,6 +390,8 @@ static int dw_phy_init(struct eth_device *dev)
 	if (!phydev)
 		return -1;
 
+	phy_connect_dev(phydev, dev);
+
 	phydev->supported &= PHY_GBIT_FEATURES;
 	phydev->advertising = phydev->supported;
 
-- 
1.9.0

[U-Boot] [PATCH v3 2/5] net/designware: ensure device private data is DMA aligned.

[Ian Campbell]

struct dw_eth_dev contains fields which are accessed via DMA, so make sure it
is aligned to a dma boundary. Without this I see:
    ERROR: v7_dcache_inval_range - start address is not aligned - 0x7fb677e0

Signed-off-by: Ian Campbell <ijc@hellion.org.uk>
Reviewed-by: Alexey Brodkin <abrodkin@synopsys.com>
Acked-by: Marek Vasut <marex@denx.de>
---
v2: Sign of with my own mail not my work mail. I made these changes on my
own time, but a .gitconfig vcsh mismerge caused "git commit -s" to use the
wrong thing. (I wondered why everyone was CCing me at work...)
---
 drivers/net/designware.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/net/designware.c b/drivers/net/designware.c
index 78751b2..41ab3ac 100644
--- a/drivers/net/designware.c
+++ b/drivers/net/designware.c
@@ -414,7 +414,8 @@ int designware_initialize(ulong base_addr, u32 interface)
 	 * Since the priv structure contains the descriptors which need a strict
 	 * buswidth alignment, memalign is used to allocate memory
 	 */
-	priv = (struct dw_eth_dev *) memalign(16, sizeof(struct dw_eth_dev));
+	priv = (struct dw_eth_dev *) memalign(ARCH_DMA_MINALIGN,
+					      sizeof(struct dw_eth_dev));
 	if (!priv) {
 		free(dev);
 		return -ENOMEM;
-- 
1.9.0

[U-Boot] [PATCH v3 3/5] net/designware: ensure cache invalidations are aligned to ARCH_DMA_MINALIGN

[Ian Campbell]

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 41ab3ac..fa816bf 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);
 
-- 
1.9.0

[U-Boot] [PATCH v3 4/5] net/designware: reorder struct dw_eth_dev to pack more efficiently.

[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.

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.

The {r,t}xbuffs fields also need to be aligned. Previously this was done
implicitly because they immediately followed the descriptor tables. Make this
explicit and also move to the head of the struct.

Signed-off-by: Ian Campbell <ijc@hellion.org.uk>
Cc: Alexey Brodkin <abrodkin@synopsys.com>
---
v3: Also align tx and rx bufs
---
 drivers/net/designware.h | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/net/designware.h b/drivers/net/designware.h
index 382b0c7..eac5e72 100644
--- a/drivers/net/designware.h
+++ b/drivers/net/designware.h
@@ -215,15 +215,15 @@ 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];
+	char txbuffs[TX_TOTAL_BUFSIZE] __aligned(ARCH_DMA_MINALIGN);
+	char rxbuffs[RX_TOTAL_BUFSIZE] __aligned(ARCH_DMA_MINALIGN);
 
 	struct eth_mac_regs *mac_regs_p;
 	struct eth_dma_regs *dma_regs_p;
-- 
1.9.0

[U-Boot] [PATCH v3 5/5] net/designware: Make DMA burst length configurable and reduce by default

[Ian Campbell]

The correct value for this setting can vary across SoCs and boards, so make it
configurable.

Also reduce the default value to 8, which is the same default as used in the
Linux driver.

Signed-off-by: Ian Campbell <ijc@hellion.org.uk>
Cc: Alexey Brodkin <abrodkin@synopsys.com>
---
v3: Pulled into this series instead of sunxi series.
---
 drivers/net/designware.c |  2 +-
 drivers/net/designware.h | 12 ++++++------
 2 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/net/designware.c b/drivers/net/designware.c
index fa816bf..7186e3b 100644
--- a/drivers/net/designware.c
+++ b/drivers/net/designware.c
@@ -249,7 +249,7 @@ static int dw_eth_init(struct eth_device *dev, bd_t *bis)
 	rx_descs_init(dev);
 	tx_descs_init(dev);
 
-	writel(FIXEDBURST | PRIORXTX_41 | BURST_16, &dma_p->busmode);
+	writel(FIXEDBURST | PRIORXTX_41 | DMA_PBL, &dma_p->busmode);
 
 	writel(readl(&dma_p->opmode) | FLUSHTXFIFO | STOREFORWARD,
 	       &dma_p->opmode);
diff --git a/drivers/net/designware.h b/drivers/net/designware.h
index eac5e72..e5b5e20 100644
--- a/drivers/net/designware.h
+++ b/drivers/net/designware.h
@@ -77,18 +77,18 @@ struct eth_dma_regs {
 
 #define DW_DMA_BASE_OFFSET	(0x1000)
 
+/* Default DMA Burst length */
+#ifndef CONFIG_DW_GMAC_DEFAULT_DMA_PBL
+#define CONFIG_DW_GMAC_DEFAULT_DMA_PBL 8
+#endif
+
 /* Bus mode register definitions */
 #define FIXEDBURST		(1 << 16)
 #define PRIORXTX_41		(3 << 14)
 #define PRIORXTX_31		(2 << 14)
 #define PRIORXTX_21		(1 << 14)
 #define PRIORXTX_11		(0 << 14)
-#define BURST_1			(1 << 8)
-#define BURST_2			(2 << 8)
-#define BURST_4			(4 << 8)
-#define BURST_8			(8 << 8)
-#define BURST_16		(16 << 8)
-#define BURST_32		(32 << 8)
+#define DMA_PBL			(CONFIG_DW_GMAC_DEFAULT_DMA_PBL<<8)
 #define RXHIGHPRIO		(1 << 1)
 #define DMAMAC_SRST		(1 << 0)
 
-- 
1.9.0

[U-Boot] [PATCH v3 4/5] net/designware: reorder struct dw_eth_dev to pack more efficiently.

[Ian Campbell]

On Thu, 2014-05-08 at 22:26 +0100, Ian Campbell wrote:
> The {r,t}xbuffs fields also need to be aligned. Previously this was done
> implicitly because they immediately followed the descriptor tables. Make this
> explicit and also move to the head of the struct.

Looks like I managed to not actually commit the move of the field to the
head of the struct! v3.1 follows....

Ian.

8<------------

[U-Boot] [PATCH v3 4/5] net/designware: reorder struct dw_eth_dev to pack more efficiently.

[Siarhei Siamashka]

On Wed, 14 May 2014 19:30:29 +0100
Ian Campbell <ijc@hellion.org.uk> wrote:

> On Thu, 2014-05-08 at 22:26 +0100, Ian Campbell wrote:
> > The {r,t}xbuffs fields also need to be aligned. Previously this was done
> > implicitly because they immediately followed the descriptor tables. Make this
> > explicit and also move to the head of the struct.
> 
> Looks like I managed to not actually commit the move of the field to the
> head of the struct! v3.1 follows....
> 
> Ian.
> 
> 8<------------
> 
> From 2937ba01841887317f6792709ed57cb86b5fc0cd Mon Sep 17 00:00:00 2001
> From: Ian Campbell <ijc@hellion.org.uk>
> Date: Thu, 1 May 2014 19:45:15 +0100
> Subject: [PATCH] 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.
> 
> The {r,t}xbuffs fields also need to be aligned. Previously this was done
> implicitly because they immediately followed the descriptor tables. Make this
> explicit and also move to the head of the struct.
> 
> Signed-off-by: Ian Campbell <ijc@hellion.org.uk>
> Cc: Alexey Brodkin <abrodkin@synopsys.com>
> ---
> v3.1: Actually move to the head of the struct, like the commit log said...
> v3: Also align tx and rx bufs
> ---
>  drivers/net/designware.h | 11 +++++------
>  1 file changed, 5 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/net/designware.h b/drivers/net/designware.h
> index 382b0c7..de2fdcb 100644
> --- a/drivers/net/designware.h
> +++ b/drivers/net/designware.h
> @@ -215,15 +215,14 @@ struct dmamacdescr {
>  #endif
>  
>  struct dw_eth_dev {
> -	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] __aligned(ARCH_DMA_MINALIGN);
> +	char rxbuffs[RX_TOTAL_BUFSIZE] __aligned(ARCH_DMA_MINALIGN);
>  
> -	char txbuffs[TX_TOTAL_BUFSIZE];
> -	char rxbuffs[RX_TOTAL_BUFSIZE];
> +	u32 interface;
> +	u32 tx_currdescnum;
> +	u32 rx_currdescnum;
>  
>  	struct eth_mac_regs *mac_regs_p;
>  	struct eth_dma_regs *dma_regs_p;

Thanks for your hard work addressing this nasty tftp breakage
regression on the Cubietruck (and on the other ARM hardware, which
happens to use the same designware driver). Now everything looks
perfect. The whole v3 patch set with this v3.1 reorder fix is

Tested-by: Siarhei Siamashka <siarhei.siamashka@gmail.com>

and also if anybody cares

Reviewed-by: Siarhei Siamashka <siarhei.siamashka@gmail.com>

-- 
Best regards,
Siarhei Siamashka

[U-Boot] [PATCH v3 0/5] net/designware: fixes for data cache, phylib and burst size

[Albert ARIBAUD]

Hi Ian,

On Thu, 08 May 2014 22:26:06 +0100, Ian Campbell <ijc@hellion.org.uk>
wrote:

> Pulling together various fixes for designware into a single series. I'm
> calling this v3, although some of these never had a v2.
> 
> Most of these have been floating around for a few weeks, the cache line
> issues in particular mean that the driver is unlikely to work on any ARM
> system. I've been testing these on top of my sunxi mainlining series.
> 
> I've also thrown them onto gitorious. Pull-request details are below.
> 
> Thanks,
> Ian.
> 
> The following changes since commit dda0dbfc69f3d560c87f5be85f127ed862ea6721:
> 
>   Prepare v2014.04 (2014-04-14 15:19:24 -0400)
> 
> are available in the git repository at:
> 
>   git://gitorious.org/ijc/u-boot.git designware
> 
> for you to fetch changes up to 5e3e5c37bc7a867ca42fa43f69ad764c8942b631:
> 
>   net/designware: Make DMA burst length configurable and reduce by default (2014-05-08 22:16:33 +0100)
> 
> ----------------------------------------------------------------
> Ian Campbell (5):
>       net/designware: call phy_connect_dev() to properly setup phylib device
>       net/designware: ensure device private data is DMA aligned.
>       net/designware: ensure cache invalidations are aligned to ARCH_DMA_MINALIGN
>       net/designware: reorder struct dw_eth_dev to pack more efficiently.
>       net/designware: Make DMA burst length configurable and reduce by default

1/5 was already applied by Tom (but still assigned to me on Patchwork).

2/5 to 5/5 (with v3.1 of 4/5) applied to u-boot-arm/master, thanks!

Amicalement,
-- 
Albert.