[U-Boot] [PATCH 0/7] 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(-)
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 | 6 +++++- 1 files changed, 5 insertions(+), 1 deletions(-)
diff --git a/drivers/net/designware.c b/drivers/net/designware.c index 1e34436..754ffae 100644 --- a/drivers/net/designware.c +++ b/drivers/net/designware.c @@ -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--);
On Wednesday 29 February 2012 05:37:20 Amit Virdi wrote:
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.
OK, but ...
--- a/drivers/net/designware.c +++ b/drivers/net/designware.c
- dw_write_hwaddr(dev);
... please add a comment above this line explaining why you must explicitly make this call -mike
On 3/1/2012 12:39 AM, Mike Frysinger wrote:
On Wednesday 29 February 2012 05:37:20 Amit Virdi wrote:
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.
OK, but ...
--- a/drivers/net/designware.c +++ b/drivers/net/designware.c
- dw_write_hwaddr(dev);
... please add a comment above this line explaining why you must explicitly make this call
Ok, Mike. I'll send V2 with comment added.
Thanks Amit Virdi
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;
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 754ffae..5847c17 100644 --- a/drivers/net/designware.c +++ b/drivers/net/designware.c @@ -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;
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 5847c17..b5e3974 100644 --- a/drivers/net/designware.c +++ b/drivers/net/designware.c @@ -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; }
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 | 4 +++ 2 files changed, 33 insertions(+), 21 deletions(-)
diff --git a/drivers/net/designware.c b/drivers/net/designware.c index b5e3974..46780e4 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 != 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; diff --git a/drivers/net/designware.h b/drivers/net/designware.h index 42133b3..42a375b 100644 --- a/drivers/net/designware.h +++ b/drivers/net/designware.h @@ -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];
On Wednesday 29 February 2012 05:37:24 Amit Virdi wrote:
--- a/drivers/net/designware.h +++ b/drivers/net/designware.h @@ -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)
NAK: we already have "true" and "false" and the "bool" type. use those instead. -mike
On 3/1/2012 12:41 AM, Mike Frysinger wrote:
- u32 phy_configured;
+#define FALSE (0) +#define TRUE (!FALSE)
NAK: we already have "true" and "false" and the "bool" type. use those instead.
Are you sure Mike? I couldn't find in u-boot code "TRUE" and "FALSE" defined in any common header file.
Thanks Amit Virdi
On Friday 02 March 2012 06:40:07 Amit Virdi wrote:
On 3/1/2012 12:41 AM, Mike Frysinger wrote:
- u32 phy_configured;
+#define FALSE (0) +#define TRUE (!FALSE)
NAK: we already have "true" and "false" and the "bool" type. use those instead.
Are you sure Mike? I couldn't find in u-boot code "TRUE" and "FALSE" defined in any common header file.
i said "true" and "false", not "TRUE" and "FALSE" -mike
Dear Mike,
On 3/3/2012 12:08 AM, Mike Frysinger wrote:
On Friday 02 March 2012 06:40:07 Amit Virdi wrote:
On 3/1/2012 12:41 AM, Mike Frysinger wrote:
- u32 phy_configured;
+#define FALSE (0) +#define TRUE (!FALSE)
NAK: we already have "true" and "false" and the "bool" type. use those instead.
Are you sure Mike? I couldn't find in u-boot code "TRUE" and "FALSE" defined in any common header file.
i said "true" and "false", not "TRUE" and "FALSE"
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,
In message 4F54AF08.1030003@st.com you wrote:
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
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
Dear Wonfgang,
On 3/6/2012 12:35 AM, Wolfgang Denk wrote:
Dear Amit Virdi,
In message4F54AF08.1030003@st.com you wrote:
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
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 for explaining so well. I shall be sending V2 incorporating your suggestion.
Best Regards Amit Virdi
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 46780e4..251ed92 100644 --- a/drivers/net/designware.c +++ b/drivers/net/designware.c @@ -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
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 251ed92..e194250 100644 --- a/drivers/net/designware.c +++ b/drivers/net/designware.c @@ -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;
Hello Ben,
On Wed, Feb 29, 2012 at 4:07 PM, Amit Virdi amit.virdi@st.com wrote:
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(-)
Any comment on this patchset from your side?
Thanks Amit Virdi
Hi Amit,
On Wednesday 07 March 2012 10:49:09 Amit Virdi wrote:
Hello Ben,
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@denx.de
On Wed, Mar 7, 2012 at 3:25 PM, Stefan Roese sr@denx.de wrote:
Hi Amit,
On Wednesday 07 March 2012 10:49:09 Amit Virdi wrote:
Hello Ben,
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 for the pointers Stefan.
Regards Amit Virdi
Mike/Simon,
On 3/7/2012 3:35 PM, Amit Virdi wrote:
On Wed, Mar 7, 2012 at 3:25 PM, Stefan Roesesr@denx.de wrote:
Hi Amit,
On Wednesday 07 March 2012 10:49:09 Amit Virdi wrote:
Hello Ben,
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?
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,
In message 201203071055.53996.sr@denx.de you wrote:
On Wednesday 07 March 2012 10:49:09 Amit Virdi wrote:
Hello Ben,
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?
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
Hi Wolfgang,
On Wed, Mar 7, 2012 at 6:36 AM, Wolfgang Denk wd@denx.de wrote:
Dear Stefan,
In message 201203071055.53996.sr@denx.de you wrote:
On Wednesday 07 March 2012 10:49:09 Amit Virdi wrote:
Hello Ben,
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?
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?
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,
In message CANr=Z=a8FW8hLDdp8CQ5SX4eJ4_kUHKtgn4aFD05JOpSsqLRYQ@mail.gmail.com you wrote:
I'm guessing you lost my email from yesterday (filter again?)...
Misplaced, not lost. And I found out which of my filter rules was a bit too generic (berger@gmail.com). Should be fixed now.
My boss is not preventing me from accepting... so I accept. I emailed my public key yesterday as well.
Great, thanks!!
You are set up - you should be able to push into the network git now.
Best regards,
Wolfgang Denk
participants (6)
-
Amit Virdi -
Amit Virdi -
Joe Hershberger -
Mike Frysinger -
Stefan Roese -
Wolfgang Denk