net/designware: Bug fixes

This patchset does bug fixes for Synopsys' designware ethernet controller driver in SPEAr SoCs.

Armando Visconti (2):
net/designware: Consecutive writes must have delay net/designware: Set ANAR to 0x1e1

Vikas Manocha (1):
net/designware: Program phy registers when auto-negotiation is ON

Vipin KUMAR (3):
net/designware: Fix to restore hw mac address
net/designware: Fix the max frame length size
net/designware: Phy address fix

Vipin Kumar (1):
net/designware: Try configuring phy on each dw_eth_init

 drivers/net/designware.c |   73 ++++++++++++++++++++++++++++++++--------------
 drivers/net/designware.h |    6 +++-
 2 files changed, 56 insertions(+), 23 deletions(-)

1.7.2.2

From: Vipin KUMAR <[email protected]>

The network controller mac resets hardware address stored in MAC_HI and MAC_LO registers if mac is resetted. So, hw mac address needs to be restored in case mac is explicitly resetted from driver.

---
 drivers/net/designware.c |    6 +++++-
 1 files changed, 5 insertions(+), 1 deletions(-)

index 1e34436..754ffae 100644
@@ -148,6 +148,8 @@ static int dw_eth_init(struct eth_device *dev, bd_t *bis)
 	if (mac_reset(dev) < 0)
 		return -1;
 
+	dw_write_hwaddr(dev);
+
 	writel(FIXEDBURST | PRIORXTX_41 | BURST_16,
 			&dma_p->busmode);
 

@@ -300,8 +302,10 @@ static int eth_mdio_write(struct eth_device *dev, u8 addr, u8 reg, u16 val)
 	writel(miiaddr | MII_CLKRANGE_150_250M | MII_BUSY, &mac_p->miiaddr);
 
 	do {
-		if (!(readl(&mac_p->miiaddr) & MII_BUSY))
+		if (!(readl(&mac_p->miiaddr) & MII_BUSY)) {
 			ret = 0;
+			break;
+		}
 		udelay(1000);
 	} while (timeout--);
 

1.7.2.2

OK, but ...

Ok, Mike. I'll send V2 with comment added.

Thanks
Amit Virdi

From: Vipin KUMAR <[email protected]>

The max frame length for normal descriptor can be 0x7FF i.e 2047. It was wrongly specified as 2048. Currently, the max descriptor length is around 1500, so redefining the mask to 1600

---
 drivers/net/designware.h |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

index e5828a6..42133b3 100644
@@ -121,7 +121,7 @@ struct eth_dma_regs {
 #define RXSTART			(1 << 1)
 
 /* Descriptior related definitions */
-#define MAC_MAX_FRAME_SZ	(2048)
+#define MAC_MAX_FRAME_SZ	(1600)
 
 struct dmamacdescr {
 	u32 txrx_status;

1.7.2.2

From: Vipin KUMAR <[email protected]>

The code assumes the phy address to be > 0, which is not true, the phy address can be in the range 0-31.

---
 drivers/net/designware.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

index 754ffae..5847c17 100644
@@ -383,7 +383,7 @@ static int configure_phy(struct eth_device *dev)
 
 #if defined(CONFIG_DW_SEARCH_PHY)
 	phy_addr = find_phy(dev);
-	if (phy_addr > 0)
+	if (phy_addr >= 0)
 		priv->address = phy_addr;
 	else
 		return -1;

1.7.2.2

From: Armando Visconti <[email protected]>

This patch solves a TX/RX problem which happens at 10Mbps, due to the fact that we are not respecting 4 cyles of the phy_clk (2.5MHz) between two consecutive writes on the same register.

---
 drivers/net/designware.c |    3 +--
 1 files changed, 1 insertions(+), 2 deletions(-)

index 5847c17..b5e3974 100644
@@ -174,8 +174,7 @@ static int dw_eth_init(struct eth_device *dev, bd_t *bis)
 	writel(readl(&dma_p->opmode) | RXSTART, &dma_p->opmode);
 	writel(readl(&dma_p->opmode) | TXSTART, &dma_p->opmode);
 
-	writel(readl(&mac_p->conf) | RXENABLE, &mac_p->conf);
-	writel(readl(&mac_p->conf) | TXENABLE, &mac_p->conf);
+	writel(readl(&mac_p->conf) | RXENABLE | TXENABLE, &mac_p->conf);
 
 	return 0;
 }

1.7.2.2

From: Vipin Kumar <[email protected]>

Phy autonegotiation works only when the ethernet cable is plugged in. Since the phy was configured only at the init time, a plugged in cable was necessary to initialize the phy properly.

This patch keeps a flag to check if the phy initialization has succeeded, and calls configure_phy routine at every init if this flag reports otherwise.

---
 drivers/net/designware.c |   50 ++++++++++++++++++++++++++-------------------
 drivers/net/designware.h |    4 +++
 2 files changed, 33 insertions(+), 21 deletions(-)

index b5e3974..46780e4 100644
@@ -32,6 +32,8 @@
 #include <asm/io.h>
 #include "designware.h"
 
+static int configure_phy(struct eth_device *dev);
+
 static void tx_descs_init(struct eth_device *dev)
 {
 	struct dw_eth_dev *priv = dev->priv;

@@ -144,6 +146,9 @@ static int dw_eth_init(struct eth_device *dev, bd_t *bis)
 	struct eth_dma_regs *dma_p = priv->dma_regs_p;
 	u32 conf;
 
+	if (priv->phy_configured != TRUE)
+		configure_phy(dev);
+
 	/* Reset ethernet hardware */
 	if (mac_reset(dev) < 0)
 		return -1;

@@ -421,23 +426,26 @@ static int configure_phy(struct eth_device *dev)
 	eth_mdio_read(dev, phy_addr, MII_LPA, &anlpar);
 	eth_mdio_read(dev, phy_addr, MII_STAT1000, &btsr);
 
-	if (btsr & (PHY_1000BTSR_1000FD | PHY_1000BTSR_1000HD)) {
-		priv->speed = SPEED_1000M;
-		if (btsr & PHY_1000BTSR_1000FD)
-			priv->duplex = FULL_DUPLEX;
-		else
-			priv->duplex = HALF_DUPLEX;
-	} else {
-		if (anlpar & LPA_100)
-			priv->speed = SPEED_100M;
-		else
-			priv->speed = SPEED_10M;
-
-		if (anlpar & (LPA_10FULL | LPA_100FULL))
-			priv->duplex = FULL_DUPLEX;
-		else
-			priv->duplex = HALF_DUPLEX;
-	}
+	if (bmsr & BMSR_ANEGCOMPLETE) {
+		if (btsr & (PHY_1000BTSR_1000FD | PHY_1000BTSR_1000HD)) {
+			priv->speed = SPEED_1000M;
+			if (btsr & PHY_1000BTSR_1000FD)
+				priv->duplex = FULL_DUPLEX;
+			else
+				priv->duplex = HALF_DUPLEX;
+		} else {
+			if (anlpar & LPA_100)
+				priv->speed = SPEED_100M;
+			else
+				priv->speed = SPEED_10M;
+
+			if (anlpar & (LPA_10FULL | LPA_100FULL))
+				priv->duplex = FULL_DUPLEX;
+			else
+				priv->duplex = HALF_DUPLEX;
+		}
+	} else
+		return -1;
 #else
 	if (eth_mdio_read(dev, phy_addr, MII_BMCR, &ctrl) < 0)
 		return -1;

@@ -454,6 +462,8 @@ static int configure_phy(struct eth_device *dev)
 	else
 		priv->speed = SPEED_10M;
 #endif
+	priv->phy_configured = TRUE;
+
 	return 0;
 }
 

@@ -514,14 +524,12 @@ int designware_initialize(u32 id, ulong base_addr, u32 phy_addr)
 	priv->dma_regs_p = (struct eth_dma_regs *)(base_addr +
 			DW_DMA_BASE_OFFSET);
 	priv->address = phy_addr;
+	priv->phy_configured = FALSE;
 
 	if (mac_reset(dev) < 0)
 		return -1;
 
-	if (configure_phy(dev) < 0) {
-		printf("Phy could not be configured\n");
-		return -1;
-	}
+	configure_phy(dev);
 
 	dev->init = dw_eth_init;
 	dev->send = dw_eth_send;

index 42133b3..42a375b 100644
@@ -238,6 +238,10 @@ struct dw_eth_dev {
 	u32 duplex;
 	u32 tx_currdescnum;
 	u32 rx_currdescnum;
+	u32 phy_configured;
+#define FALSE	(0)
+#define TRUE	(!FALSE)
+
 	u32 padding;
 
 	struct dmamacdescr tx_mac_descrtable[CONFIG_TX_DESCR_NUM];

1.7.2.2

Are you sure Mike? I couldn't find in u-boot code "TRUE" and "FALSE" defined in any common header file.

Thanks
Amit Virdi

i said "true" and "false", not "TRUE" and "FALSE"
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20120302/8ff217d2/attachment.pgp>

Dear Mike,

Surprisingly, I could still not find "true" and "false" defined for ARM architecture or in a common file that I can include. Here's what grep gives me

----
bash-3.2$ git grep -i "define true" -- *.[h]
arch/arm/cpu/ixp/npe/include/IxOsalTypes.h:#define TRUE 1L
board/Marvell/db64360/mv_eth.h:#define TRUE 1
board/Marvell/db64460/mv_eth.h:#define TRUE 1
board/amcc/bamboo/bamboo.h:#define TRUE 1
board/bf533-ezkit/flash-defines.h:#define TRUE 0x1
board/bf533-stamp/video.h:#define true 1
board/esd/common/lcd.h:#define TRUE (!FALSE)
board/esd/cpci750/mv_eth.h:#define TRUE 1
board/prodrive/p3mx/mv_eth.h:#define TRUE 1
board/sacsng/clkinit.h:#define TRUE (!FALSE)
board/xilinx/common/xbasic_types.h:#define TRUE 1
drivers/block/sata_dwc.h:#define TRUE 1
drivers/net/armada100_fec.h:#define TRUE 1
drivers/net/ne2000_base.h:#define true 1
include/at91rm9200_net.h:#define TRUE 1
include/bedbug/ppc.h:#define TRUE (!FALSE)
include/fpga.h:#define TRUE (!FALSE)
include/scsi.h:#define TRUE 1
include/sym53c8xx.h:#define TRUE 1
include/xyzModem.h:#define true 1
lib/bzlib_private.h:#define True ((Bool)1)
lib/lzma/Types.h:#define True 1
----

You might be pointing to arch/blackfin/include/asm/posix_types.h:typedef enum { false = 0, true = 1 } bool;
but this is for blackfin architecture.

Can you please suggest me which bool types are you referring here?

Thanks
Amit Virdi

Dear Amit Virdi,

So don't use these.

In theory, any use of #define TRUE / FALSE or enum bool or plain use of raw 0 and 1 is fine - but only as long as you make sure to use this consistently within the whole project.

If I am forced to use something like this, I prefer the enum as this can be decoded by a debugger. But in general, I try to avod it, and use plain C logic instead - because then I can just read and understand the code, in all other cases I have to look up definitions, especially when it comes to parameter passings.

In your specific case, you use TRUE and FALSE just in one single place, for one single variable. Dumpt that. Use 0 and 1, and everybody can read the code.

Thanks.

Best regards,

Wolfgang Denk

DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
What is tolerance? -- it is the consequence of humanity. We  are  all
formed  of frailty and error; let us pardon reciprocally each other's
folly -- that is the first law of nature.                  - Voltaire

Dear Wonfgang,

Thanks for explaining so well. I shall be sending V2 incorporating your suggestion.

Best Regards
Amit Virdi

From: Vikas Manocha <[email protected]>

If AN(auto-negotiation) is ON, speed bit of control register are not applicable. Also phy registers were not getting programmed as per the result of AN. This patch sets only AN bit & restart AN bit for AN ON selection & programs PHY registers as per AN result.

---
 drivers/net/designware.c |   43 +++++++++++++++++++++++++++++--------------
 1 files changed, 29 insertions(+), 14 deletions(-)

index 46780e4..251ed92 100644
@@ -398,8 +398,7 @@ static int configure_phy(struct eth_device *dev)
 		return -1;
 
 #if defined(CONFIG_DW_AUTONEG)
-	bmcr = BMCR_ANENABLE | BMCR_ANRESTART | BMCR_SPEED100 | \
-	       BMCR_FULLDPLX | BMCR_SPEED1000;
+	bmcr = BMCR_ANENABLE | BMCR_ANRESTART;
 #else
 	bmcr = BMCR_SPEED100 | BMCR_FULLDPLX;
 

@@ -427,23 +426,39 @@ static int configure_phy(struct eth_device *dev)
 	eth_mdio_read(dev, phy_addr, MII_STAT1000, &btsr);
 
 	if (bmsr & BMSR_ANEGCOMPLETE) {
-		if (btsr & (PHY_1000BTSR_1000FD | PHY_1000BTSR_1000HD)) {
+		if (btsr & PHY_1000BTSR_1000FD) {
 			priv->speed = SPEED_1000M;
-			if (btsr & PHY_1000BTSR_1000FD)
-				priv->duplex = FULL_DUPLEX;
-			else
-				priv->duplex = HALF_DUPLEX;
+			bmcr |= BMCR_SPEED1000;
+			priv->duplex = FULL_DUPLEX;
+			bmcr |= BMCR_FULLDPLX;
+		} else if (btsr & PHY_1000BTSR_1000HD) {
+			priv->speed = SPEED_1000M;
+			bmcr |= BMCR_SPEED1000;
+			priv->duplex = HALF_DUPLEX;
+			bmcr &= ~BMCR_FULLDPLX;
+		} else if (anlpar & LPA_100FULL) {
+			priv->speed = SPEED_100M;
+			bmcr |= BMCR_SPEED100;
+			priv->duplex = FULL_DUPLEX;
+			bmcr |= BMCR_FULLDPLX;
+		} else if (anlpar & LPA_100HALF) {
+			priv->speed = SPEED_100M;
+			bmcr |= BMCR_SPEED100;
+			priv->duplex = HALF_DUPLEX;
+			bmcr &= ~BMCR_FULLDPLX;
+		} else if (anlpar & LPA_10FULL) {
+			priv->speed = SPEED_10M;
+			bmcr &= ~BMCR_SPEED100;
+			priv->duplex = FULL_DUPLEX;
+			bmcr |= BMCR_FULLDPLX;
 		} else {
-			if (anlpar & LPA_100)
-				priv->speed = SPEED_100M;
-			else
 				priv->speed = SPEED_10M;
-
-			if (anlpar & (LPA_10FULL | LPA_100FULL))
-				priv->duplex = FULL_DUPLEX;
-			else
+				bmcr &= ~BMCR_SPEED100;
 				priv->duplex = HALF_DUPLEX;
+				bmcr &= ~BMCR_FULLDPLX;
 		}
+		if (eth_mdio_write(dev, phy_addr, MII_BMCR, bmcr) < 0)
+			return -1;
 	} else
 		return -1;
 #else

1.7.2.2

From: Armando Visconti <[email protected]>

This patch forces the advertised capabilities during auto negotiation to always be 10/100 Mbps and half/full as duplexing.

---
 drivers/net/designware.c |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

index 251ed92..e194250 100644
@@ -398,6 +398,9 @@ static int configure_phy(struct eth_device *dev)
 		return -1;
 
 #if defined(CONFIG_DW_AUTONEG)
+	/* Set Auto-Neg Advertisement capabilities to 10/100 half/full */
+	eth_mdio_write(dev, phy_addr, MII_ADVERTISE, 0x1E1);
+
 	bmcr = BMCR_ANENABLE | BMCR_ANRESTART;
 #else
 	bmcr = BMCR_SPEED100 | BMCR_FULLDPLX;

1.7.2.2

Hello Ben,

Any comment on this patchset from your side?

Thanks
Amit Virdi

Hi Amit,

Ben resigned as net custodian. IIRC, Simon and Mike (added to Cc) volunteered to take over. I'm not sure about the outcome though. Simon, Mike, is this now decided?

Thanks,
Stefan

--
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-0 Fax: (+49)-8142-66989-80 Email: office at denx.de

Thanks for the pointers Stefan.

Regards
Amit Virdi

Mike/Simon,

As I understood, the patches shall be applied only after the forthcoming release. Mike, I have incorporated your comments in the V2. Please let me know if you guys have any comments for other patches in this patchset.

Thanks
Amit Virdi

Dear Stefan,

Nothing is decided yet. Actually I was discussing off list with Joe Hershberger if he was wiling to do this work - IMO this would make a lot of sense, if you look who did which network related work in the last year or so. Joe agreed in principle, but was still waiting for comments from his boss.

Joe, any updates?

Best regards,

Wolfgang Denk

DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
I have the simplest tastes.  I am always satisfied with the best.
                                                       -- Oscar Wilde

Hi Wolfgang,

I'm guessing you lost my email from yesterday (filter again?)...

My boss is not preventing me from accepting... so I accept. I emailed my public key yesterday as well.

Best regards,
-Joe

Dear Joe,

Misplaced, not lost. And I found out which of my filter rules was a bit too generic (berger at gmail.com). Should be fixed now.

Great, thanks!!

You are set up - you should be able to push into the network git now.

Best regards,

Wolfgang Denk

DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
There are certain things men must do to remain men.
	-- Kirk, "The Ultimate Computer", stardate 4929.4