[U-Boot] [PATCH V2 0/8] net/designware: Bug fixes

This patchset does bug fixes for Synopsys' designware ethernet controller driver in SPEAr SoCs.
Changes since V1: - Added comment to explain the ratinale behind restoring hw mac address - Undefined TRUE/FALSE and used 1/0 instead while configuring phy
Amit Virdi (1): net/designware: Change timeout loop implementation
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 | 128 ++++++++++++++++++++++++++++++++-------------- drivers/net/designware.h | 3 +- 2 files changed, 92 insertions(+), 39 deletions(-)

From: Vipin KUMAR vipin.kumar@st.com
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.
Signed-off-by: Vipin Kumar vipin.kumar@st.com Signed-off-by: Amit Virdi amit.virdi@st.com --- drivers/net/designware.c | 7 ++++++- 1 files changed, 6 insertions(+), 1 deletions(-)
diff --git a/drivers/net/designware.c b/drivers/net/designware.c index 1e34436..ea8a406 100644 --- a/drivers/net/designware.c +++ b/drivers/net/designware.c @@ -148,6 +148,9 @@ static int dw_eth_init(struct eth_device *dev, bd_t *bis) if (mac_reset(dev) < 0) return -1;
+ /* Resore the HW MAC address as it has been lost during MAC reset */ + dw_write_hwaddr(dev); + writel(FIXEDBURST | PRIORXTX_41 | BURST_16, &dma_p->busmode);
@@ -300,8 +303,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--);

On Mon, Mar 26, 2012 at 5:09 AM, Amit Virdi amit.virdi@st.com wrote:
From: Vipin KUMAR vipin.kumar@st.com
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.
Signed-off-by: Vipin Kumar vipin.kumar@st.com Signed-off-by: Amit Virdi amit.virdi@st.com
Applied.
Thanks, -Joe

From: Vipin KUMAR vipin.kumar@st.com
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
Signed-off-by: Vipin Kumar vipin.kumar@st.com Signed-off-by: Amit Virdi amit.virdi@st.com --- drivers/net/designware.h | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/net/designware.h b/drivers/net/designware.h index e5828a6..42133b3 100644 --- a/drivers/net/designware.h +++ b/drivers/net/designware.h @@ -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;

On Mon, Mar 26, 2012 at 5:09 AM, Amit Virdi amit.virdi@st.com wrote:
From: Vipin KUMAR vipin.kumar@st.com
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
Signed-off-by: Vipin Kumar vipin.kumar@st.com Signed-off-by: Amit Virdi amit.virdi@st.com
Applied.
Thanks, -Joe

From: Vipin KUMAR vipin.kumar@st.com
The code assumes the phy address to be > 0, which is not true, the phy address can be in the range 0-31.
Signed-off-by: Vipin Kumar vipin.kumar@st.com Signed-off-by: Amit Virdi amit.virdi@st.com --- drivers/net/designware.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/net/designware.c b/drivers/net/designware.c index ea8a406..fc14b70 100644 --- a/drivers/net/designware.c +++ b/drivers/net/designware.c @@ -384,7 +384,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;

Hi Joe,
On 3/26/2012 3:39 PM, Amit VIRDI wrote:
From: Vipin KUMARvipin.kumar@st.com
The code assumes the phy address to be> 0, which is not true, the phy address can be in the range 0-31.
Signed-off-by: Vipin Kumarvipin.kumar@st.com Signed-off-by: Amit Virdiamit.virdi@st.com
Could you please confirm if you have applied this patch too?
Thanks n Best Regards Amit Virdi

Hi Amit,
On Thu, Apr 5, 2012 at 12:15 AM, Amit Virdi amit.virdi@st.com wrote:
Hi Joe,
On 3/26/2012 3:39 PM, Amit VIRDI wrote:
From: Vipin KUMARvipin.kumar@st.com
The code assumes the phy address to be> 0, which is not true, the phy address can be in the range 0-31.
Signed-off-by: Vipin Kumarvipin.kumar@st.com Signed-off-by: Amit Virdiamit.virdi@st.com
Could you please confirm if you have applied this patch too?
Yes... sorry... missed that one in the emails, but it is applied.
-Joe

From: Armando Visconti armando.visconti@st.com
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.
Signed-off-by: Armando Visconti armando.visconti@st.com Signed-off-by: Amit Virdi amit.virdi@st.com --- drivers/net/designware.c | 3 +-- 1 files changed, 1 insertions(+), 2 deletions(-)
diff --git a/drivers/net/designware.c b/drivers/net/designware.c index fc14b70..933032c 100644 --- a/drivers/net/designware.c +++ b/drivers/net/designware.c @@ -175,8 +175,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; }

From: Vipin Kumar vipin.kumar@st.com
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.
Signed-off-by: Vipin Kumar vipin.kumar@st.com Signed-off-by: Amit Virdi amit.virdi@st.com --- drivers/net/designware.c | 50 ++++++++++++++++++++++++++------------------- drivers/net/designware.h | 1 + 2 files changed, 30 insertions(+), 21 deletions(-)
diff --git a/drivers/net/designware.c b/drivers/net/designware.c index 933032c..ebb1fff 100644 --- a/drivers/net/designware.c +++ b/drivers/net/designware.c @@ -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 != 1) + configure_phy(dev); + /* Reset ethernet hardware */ if (mac_reset(dev) < 0) return -1; @@ -422,23 +427,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; @@ -455,6 +463,8 @@ static int configure_phy(struct eth_device *dev) else priv->speed = SPEED_10M; #endif + priv->phy_configured = 1; + return 0; }
@@ -515,14 +525,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 = 0;
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; diff --git a/drivers/net/designware.h b/drivers/net/designware.h index 42133b3..abf729d 100644 --- a/drivers/net/designware.h +++ b/drivers/net/designware.h @@ -238,6 +238,7 @@ struct dw_eth_dev { u32 duplex; u32 tx_currdescnum; u32 rx_currdescnum; + u32 phy_configured; u32 padding;
struct dmamacdescr tx_mac_descrtable[CONFIG_TX_DESCR_NUM];

On Mon, Mar 26, 2012 at 5:09 AM, Amit Virdi amit.virdi@st.com wrote:
From: Vipin Kumar vipin.kumar@st.com
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.
Signed-off-by: Vipin Kumar vipin.kumar@st.com Signed-off-by: Amit Virdi amit.virdi@st.com
Applied.
Thanks, -Joe

From: Vikas Manocha vikas.manocha@st.com
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.
Signed-off-by: Vikas Manocha vikas.manocha@st.com Signed-off-by: Amit Virdi amit.virdi@st.com --- drivers/net/designware.c | 43 +++++++++++++++++++++++++++++-------------- 1 files changed, 29 insertions(+), 14 deletions(-)
diff --git a/drivers/net/designware.c b/drivers/net/designware.c index ebb1fff..56f0c7a 100644 --- a/drivers/net/designware.c +++ b/drivers/net/designware.c @@ -399,8 +399,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;
@@ -428,23 +427,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

On Mon, Mar 26, 2012 at 5:09 AM, Amit Virdi amit.virdi@st.com wrote:
From: Vikas Manocha vikas.manocha@st.com
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.
Signed-off-by: Vikas Manocha vikas.manocha@st.com Signed-off-by: Amit Virdi amit.virdi@st.com
Applied.
Thanks, -Joe

From: Armando Visconti armando.visconti@st.com
This patch forces the advertised capabilities during auto negotiation to always be 10/100 Mbps and half/full as duplexing.
Signed-off-by: Armando Visconti armando.visconti@st.com Signed-off-by: Amit Virdi amit.virdi@st.com --- drivers/net/designware.c | 3 +++ 1 files changed, 3 insertions(+), 0 deletions(-)
diff --git a/drivers/net/designware.c b/drivers/net/designware.c index 56f0c7a..987469a 100644 --- a/drivers/net/designware.c +++ b/drivers/net/designware.c @@ -399,6 +399,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;

On Mon, Mar 26, 2012 at 5:09 AM, Amit Virdi amit.virdi@st.com wrote:
From: Armando Visconti armando.visconti@st.com
This patch forces the advertised capabilities during auto negotiation to always be 10/100 Mbps and half/full as duplexing.
Signed-off-by: Armando Visconti armando.visconti@st.com Signed-off-by: Amit Virdi amit.virdi@st.com
Applied.
Thanks, -Joe

The new implementation changes the timeout loop implementation to avoid 1 ms delay in each failing test. It also configures the delay to 10usec.
Signed-off-by: Amit Virdi amit.virdi@st.com --- drivers/net/designware.c | 54 ++++++++++++++++++++++++++++++++------------- 1 files changed, 38 insertions(+), 16 deletions(-)
diff --git a/drivers/net/designware.c b/drivers/net/designware.c index 987469a..e8e669b 100644 --- a/drivers/net/designware.c +++ b/drivers/net/designware.c @@ -108,16 +108,20 @@ static int mac_reset(struct eth_device *dev) struct eth_mac_regs *mac_p = priv->mac_regs_p; struct eth_dma_regs *dma_p = priv->dma_regs_p;
+ ulong start; int timeout = CONFIG_MACRESET_TIMEOUT;
writel(DMAMAC_SRST, &dma_p->busmode); writel(MII_PORTSELECT, &mac_p->conf);
- do { + start = get_timer(0); + while (get_timer(start) < timeout) { if (!(readl(&dma_p->busmode) & DMAMAC_SRST)) return 0; - udelay(1000); - } while (timeout--); + + /* Try again after 10usec */ + udelay(10); + };
return -1; } @@ -273,6 +277,7 @@ static int eth_mdio_read(struct eth_device *dev, u8 addr, u8 reg, u16 *val) { struct dw_eth_dev *priv = dev->priv; struct eth_mac_regs *mac_p = priv->mac_regs_p; + ulong start; u32 miiaddr; int timeout = CONFIG_MDIO_TIMEOUT;
@@ -281,13 +286,16 @@ static int eth_mdio_read(struct eth_device *dev, u8 addr, u8 reg, u16 *val)
writel(miiaddr | MII_CLKRANGE_150_250M | MII_BUSY, &mac_p->miiaddr);
- do { + start = get_timer(0); + while (get_timer(start) < timeout) { if (!(readl(&mac_p->miiaddr) & MII_BUSY)) { *val = readl(&mac_p->miidata); return 0; } - udelay(1000); - } while (timeout--); + + /* Try again after 10usec */ + udelay(10); + };
return -1; } @@ -296,6 +304,7 @@ static int eth_mdio_write(struct eth_device *dev, u8 addr, u8 reg, u16 val) { struct dw_eth_dev *priv = dev->priv; struct eth_mac_regs *mac_p = priv->mac_regs_p; + ulong start; u32 miiaddr; int ret = -1, timeout = CONFIG_MDIO_TIMEOUT; u16 value; @@ -306,13 +315,16 @@ 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 { + start = get_timer(0); + while (get_timer(start) < timeout) { if (!(readl(&mac_p->miiaddr) & MII_BUSY)) { ret = 0; break; } - udelay(1000); - } while (timeout--); + + /* Try again after 10usec */ + udelay(10); + };
/* Needed as a fix for ST-Phy */ eth_mdio_read(dev, addr, reg, &value); @@ -353,18 +365,23 @@ static int dw_reset_phy(struct eth_device *dev) { struct dw_eth_dev *priv = dev->priv; u16 ctrl; + ulong start; int timeout = CONFIG_PHYRESET_TIMEOUT; u32 phy_addr = priv->address;
eth_mdio_write(dev, phy_addr, MII_BMCR, BMCR_RESET); - do { + + start = get_timer(0); + while (get_timer(start) < timeout) { eth_mdio_read(dev, phy_addr, MII_BMCR, &ctrl); if (!(ctrl & BMCR_RESET)) break; - udelay(1000); - } while (timeout--);
- if (timeout < 0) + /* Try again after 10usec */ + udelay(10); + }; + + if (get_timer(start) >= CONFIG_PHYRESET_TIMEOUT) return -1;
#ifdef CONFIG_PHY_RESET_DELAY @@ -381,6 +398,7 @@ static int configure_phy(struct eth_device *dev) #if defined(CONFIG_DW_AUTONEG) u16 bmsr; u32 timeout; + ulong start; u16 anlpar, btsr; #else u16 ctrl; @@ -419,12 +437,16 @@ static int configure_phy(struct eth_device *dev) /* Read the phy status register and populate priv structure */ #if defined(CONFIG_DW_AUTONEG) timeout = CONFIG_AUTONEG_TIMEOUT; - do { + start = get_timer(0); + + while (get_timer(start) < timeout) { eth_mdio_read(dev, phy_addr, MII_BMSR, &bmsr); if (bmsr & BMSR_ANEGCOMPLETE) break; - udelay(1000); - } while (timeout--); + + /* Try again after 10usec */ + udelay(10); + };
eth_mdio_read(dev, phy_addr, MII_LPA, &anlpar); eth_mdio_read(dev, phy_addr, MII_STAT1000, &btsr);

On Mon, Mar 26, 2012 at 5:09 AM, Amit Virdi amit.virdi@st.com wrote:
The new implementation changes the timeout loop implementation to avoid 1 ms delay in each failing test. It also configures the delay to 10usec.
Signed-off-by: Amit Virdi amit.virdi@st.com
Applied.
Thanks, -Joe
participants (2)
-
Amit Virdi
-
Joe Hershberger