[U-Boot] [PATCH] TI: DaVinci: Updating EMAC driver for DM365, DM646x and DA8XX

[U-Boot] [PATCH] TI: DaVinci: Updating EMAC driver for DM365, DM646x and DA8XX

[Nick Thompson]

The EMAC IP on DM365, DM646x and DA830 is slightly different
from that on DM644x. This change updates the DaVinci EMAC driver
so that EMAC becomes operational on SOCs with EMAC v2.

Signed-off-by: Nick Thompson <nick.thompson@ge.com>
Signed-off-by: Sandeep Paulraj <s-paulraj@ti.com>
---
Applies to: u-boot-ti

This is a combined patch with Sandeep's DM365 and DM646x changes
and additional changes for DA830. It replaces previous submissions
for EMAC support on these devices.

 drivers/net/davinci_emac.c               |  131 ++++++++++++++++++++++++-----
 include/asm-arm/arch-davinci/emac_defs.h |   60 +++++++++++++-
 2 files changed, 164 insertions(+), 27 deletions(-)

diff --git a/drivers/net/davinci_emac.c b/drivers/net/davinci_emac.c
index fa8cee4..dbf94d2 100644
--- a/drivers/net/davinci_emac.c
+++ b/drivers/net/davinci_emac.c
@@ -42,6 +42,7 @@
 #include <miiphy.h>
 #include <malloc.h>
 #include <asm/arch/emac_defs.h>
+#include <asm/io.h>
 
 unsigned int	emac_dbg = 0;
 #define debug_emac(fmt,args...)	if (emac_dbg) printf(fmt,##args)
@@ -107,6 +108,35 @@ static void davinci_eth_mdio_enable(void)
 	while (adap_mdio->CONTROL & MDIO_CONTROL_IDLE) {;}
 }
 
+/* Read a PHY register via MDIO inteface */
+int mdio_read(int phy_addr, int reg_num)
+{
+	writel(MDIO_USERACCESS0_GO |
+	       MDIO_USERACCESS0_WRITE_READ |
+	       ((reg_num & 0x1F) << 21) |
+	       ((phy_addr & 0x1F) << 16),
+	       &adap_mdio->USERACCESS0);
+
+	/* Wait for command to complete */
+	while (readl(&adap_mdio->USERACCESS0) & MDIO_USERACCESS0_GO);
+
+	return readl(&adap_mdio->USERACCESS0) & 0xFFFF;
+}
+
+/* Write to a PHY register via MDIO inteface */
+void mdio_write(int phy_addr, int reg_num, unsigned int data)
+{
+	/* Wait for User access register to be ready */
+	while (readl(&adap_mdio->USERACCESS0) & MDIO_USERACCESS0_GO);
+
+	writel(MDIO_USERACCESS0_GO |
+	       MDIO_USERACCESS0_WRITE_WRITE |
+	       ((reg_num & 0x1F) << 21) |
+	       ((phy_addr & 0x1F) << 16) |
+	       (data & 0xFFFF),
+	       &adap_mdio->USERACCESS0);
+}
+
 /*
  * Tries to find an active connected PHY. Returns 1 if address if found.
  * If no active PHY (or more than one PHY) found returns 0.
@@ -119,7 +149,8 @@ static int davinci_eth_phy_detect(void)
 
 	active_phy_addr = 0xff;
 
-	if ((phy_act_state = adap_mdio->ALIVE) == 0)
+	phy_act_state = adap_mdio->ALIVE & EMAC_MDIO_PHY_MASK;
+	if (phy_act_state == 0)
 		return(0);				/* No active PHYs */
 
 	debug_emac("davinci_eth_phy_detect(), ALIVE = 0x%08x\n", phy_act_state);
@@ -245,9 +276,33 @@ static int davinci_mii_phy_write(char *devname, unsigned char addr, unsigned cha
 {
 	return(davinci_eth_phy_write(addr, reg, value) ? 0 : 1);
 }
-
 #endif
 
+static void emac_gigabit_enable(void)
+{
+#ifdef DAVINCI_EMAC_GIG_ENABLE
+	int temp
+
+	if (mdio_read(EMAC_MDIO_PHY_NUM, 0) & (1 << 6)) {
+		/*
+		 * Check if link detected is giga-bit
+		 * If Gigabit mode detected, enable gigbit in MAC and PHY
+		 */
+		writel(EMAC_MACCONTROL_GIGFORCE |
+		       EMAC_MACCONTROL_GIGABIT_ENABLE,
+		       &adap_emac->MACCONTROL);
+
+		/*
+		 * The SYS_CLK which feeds the SOC for giga-bit operation
+		 * does not seem to be enabled after reset as expected.
+		 * Force enabling SYS_CLK by writing to the PHY
+		 */
+		temp = mdio_read(EMAC_MDIO_PHY_NUM, 22);
+		temp |= (1 << 4);
+		mdio_write(EMAC_MDIO_PHY_NUM, 22, temp);
+	}
+#endif
+}
 
 /* Eth device open */
 static int davinci_eth_open(struct eth_device *dev, bd_t *bis)
@@ -255,16 +310,23 @@ static int davinci_eth_open(struct eth_device *dev, bd_t *bis)
 	dv_reg_p		addr;
 	u_int32_t		clkdiv, cnt;
 	volatile emac_desc	*rx_desc;
+	unsigned long		mac_hi;
+	unsigned long		mac_lo;
 
 	debug_emac("+ emac_open\n");
 
 	/* Reset EMAC module and disable interrupts in wrapper */
 	adap_emac->SOFTRESET = 1;
 	while (adap_emac->SOFTRESET != 0) {;}
+#if defined(DAVINCI_EMAC_VERSION2)
+	writel(1, &adap_ewrap->softrst);
+	while (readl(&adap_ewrap->softrst) != 0);
+#else
 	adap_ewrap->EWCTL = 0;
 	for (cnt = 0; cnt < 5; cnt++) {
 		clkdiv = adap_ewrap->EWCTL;
 	}
+#endif
 
 	rx_desc = emac_rx_desc;
 
@@ -274,27 +336,27 @@ static int davinci_eth_open(struct eth_device *dev, bd_t *bis)
 	/* Set MAC Addresses & Init multicast Hash to 0 (disable any multicast receive) */
 	/* Using channel 0 only - other channels are disabled */
 	adap_emac->MACINDEX = 0;
-	adap_emac->MACADDRHI =
-		(davinci_eth_mac_addr[3] << 24) |
-		(davinci_eth_mac_addr[2] << 16) |
-		(davinci_eth_mac_addr[1] << 8)  |
-		(davinci_eth_mac_addr[0]);
-	adap_emac->MACADDRLO =
-		(davinci_eth_mac_addr[5] << 8) |
-		(davinci_eth_mac_addr[4]);
+	mac_hi = (davinci_eth_mac_addr[3] << 24) |
+		 (davinci_eth_mac_addr[2] << 16) |
+		 (davinci_eth_mac_addr[1] << 8)  |
+		 (davinci_eth_mac_addr[0]);
+	mac_lo = (davinci_eth_mac_addr[5] << 8) |
+		 (davinci_eth_mac_addr[4]);
+
+	writel(mac_hi, &adap_emac->MACADDRHI);
+#if defined(DAVINCI_EMAC_VERSION2)
+	writel(mac_lo | EMAC_MAC_ADDR_IS_VALID | EMAC_MAC_ADDR_MATCH,
+	       &adap_emac->MACADDRLO);
+#else
+	writel(mac_lo, &adap_emac->MACADDRLO);
+#endif
 
 	adap_emac->MACHASH1 = 0;
 	adap_emac->MACHASH2 = 0;
 
 	/* Set source MAC address - REQUIRED */
-	adap_emac->MACSRCADDRHI =
-		(davinci_eth_mac_addr[3] << 24) |
-		(davinci_eth_mac_addr[2] << 16) |
-		(davinci_eth_mac_addr[1] << 8)  |
-		(davinci_eth_mac_addr[0]);
-	adap_emac->MACSRCADDRLO =
-		(davinci_eth_mac_addr[4] << 8) |
-		(davinci_eth_mac_addr[5]);
+	writel(mac_hi, &adap_emac->MACSRCADDRHI);
+	writel(mac_lo, &adap_emac->MACSRCADDRLO);
 
 	/* Set DMA 8 TX / 8 RX Head pointers to 0 */
 	addr = &adap_emac->TX0HDP;
@@ -341,14 +403,30 @@ static int davinci_eth_open(struct eth_device *dev, bd_t *bis)
 	adap_emac->RXUNICASTSET = 0x01;
 
 	/* Enable MII interface and Full duplex mode */
-	adap_emac->MACCONTROL = (EMAC_MACCONTROL_MIIEN_ENABLE | EMAC_MACCONTROL_FULLDUPLEX_ENABLE);
+#ifdef CONFIG_SOC_DA8XX
+	writel((EMAC_MACCONTROL_MIIEN_ENABLE |
+		EMAC_MACCONTROL_FULLDUPLEX_ENABLE |
+		EMAC_MACCONTROL_RMIISPEED_100),
+	       &adap_emac->MACCONTROL);
+#else
+	writel((EMAC_MACCONTROL_MIIEN_ENABLE |
+		EMAC_MACCONTROL_FULLDUPLEX_ENABLE),
+	       &adap_emac->MACCONTROL);
+#endif
 
 	/* Init MDIO & get link state */
 	clkdiv = (EMAC_MDIO_BUS_FREQ / EMAC_MDIO_CLOCK_FREQ) - 1;
 	adap_mdio->CONTROL = ((clkdiv & 0xff) | MDIO_CONTROL_ENABLE | MDIO_CONTROL_FAULT);
 
+#if defined(DAVINCI_EMAC_VERSION2)
+	/* We need to wait for MDIO to start */
+	udelay(1000);
+#endif
+
 	if (!phy.get_link_speed(active_phy_addr))
 		return(0);
+	else
+		emac_gigabit_enable();
 
 	/* Start receive process */
 	adap_emac->RX0HDP = (u_int32_t)emac_rx_desc;
@@ -410,8 +488,12 @@ static void davinci_eth_close(struct eth_device *dev)
 	davinci_eth_ch_teardown(EMAC_CH_RX);	/* RX Channel teardown */
 
 	/* Reset EMAC module and disable interrupts in wrapper */
-	adap_emac->SOFTRESET = 1;
-	adap_ewrap->EWCTL = 0;
+	writel(1, &adap_emac->SOFTRESET);
+#if defined(DAVINCI_EMAC_VERSION2)
+	writel(1, &adap_ewrap->softrst);
+#else
+	writel(0, &adap_ewrap->EWCTL);
+#endif
 
 	debug_emac("- emac_close\n");
 }
@@ -433,7 +515,8 @@ static int davinci_eth_send_packet (struct eth_device *dev,
 	if (!phy.get_link_speed (active_phy_addr)) {
 		printf ("WARN: emac_send_packet: No link\n");
 		return (ret_status);
-	}
+	} else
+		emac_gigabit_enable();
 
 	/* Check packet size and if < EMAC_MIN_ETHERNET_PKT_SIZE, pad it up */
 	if (length < EMAC_MIN_ETHERNET_PKT_SIZE) {
@@ -456,7 +539,9 @@ static int davinci_eth_send_packet (struct eth_device *dev,
 		if (!phy.get_link_speed (active_phy_addr)) {
 			davinci_eth_ch_teardown (EMAC_CH_TX);
 			return (ret_status);
-		}
+		} else
+			emac_gigabit_enable();
+
 		if (adap_emac->TXINTSTATRAW & 0x01) {
 			ret_status = length;
 			break;
diff --git a/include/asm-arm/arch-davinci/emac_defs.h b/include/asm-arm/arch-davinci/emac_defs.h
index 96bc80e..5a0bcaf 100644
--- a/include/asm-arm/arch-davinci/emac_defs.h
+++ b/include/asm-arm/arch-davinci/emac_defs.h
@@ -43,6 +43,14 @@
 #define EMAC_WRAPPER_BASE_ADDR		(0x01d0a000)
 #define EMAC_WRAPPER_RAM_ADDR		(0x01d08000)
 #define EMAC_MDIO_BASE_ADDR		(0x01d0b000)
+#define DAVINCI_EMAC_VERSION2
+#define DAVINCI_EMAC_GIG_ENABLE
+#elif defined(CONFIG_SOC_DA8XX)
+#define EMAC_BASE_ADDR			DAVINCI_EMAC_CNTRL_REGS_BASE
+#define EMAC_WRAPPER_BASE_ADDR		DAVINCI_EMAC_WRAPPER_CNTRL_REGS_BASE
+#define EMAC_WRAPPER_RAM_ADDR		DAVINCI_EMAC_WRAPPER_RAM_BASE
+#define EMAC_MDIO_BASE_ADDR		DAVINCI_MDIO_CNTRL_REGS_BASE
+#define DAVINCI_EMAC_VERSION2
 #else
 #define EMAC_BASE_ADDR			(0x01c80000)
 #define EMAC_WRAPPER_BASE_ADDR		(0x01c81000)
@@ -51,6 +59,11 @@
 #endif
 
 #ifdef CONFIG_SOC_DM646X
+#define DAVINCI_EMAC_VERSION2
+#define DAVINCI_EMAC_GIG_ENABLE
+#endif
+
+#ifdef CONFIG_SOC_DM646X
 /* MDIO module input frequency */
 #define EMAC_MDIO_BUS_FREQ		76500000
 /* MDIO clock output frequency */
@@ -60,6 +73,11 @@
 #define EMAC_MDIO_BUS_FREQ		121500000
 /* MDIO clock output frequency */
 #define EMAC_MDIO_CLOCK_FREQ		2200000		/* 2.2 MHz */
+#elif defined(CONFIG_SOC_DA8XX)
+/* MDIO module input frequency */
+#define EMAC_MDIO_BUS_FREQ		clk_get(DAVINCI_MDIO_CLKID)
+/* MDIO clock output frequency */
+#define EMAC_MDIO_CLOCK_FREQ		2000000		/* 2.0 MHz */
 #else
 /* MDIO module input frequency */
 #define EMAC_MDIO_BUS_FREQ		99000000	/* PLL/6 - 99 MHz */
@@ -128,6 +146,10 @@ typedef volatile struct _emac_desc
 #define EMAC_MACCONTROL_FULLDUPLEX_ENABLE	(0x1)
 #define EMAC_MACCONTROL_GIGABIT_ENABLE		(1 << 7)
 #define EMAC_MACCONTROL_GIGFORCE		(1 << 17)
+#define EMAC_MACCONTROL_RMIISPEED_100		(1 << 15)
+
+#define EMAC_MAC_ADDR_MATCH		(1 << 19)
+#define EMAC_MAC_ADDR_IS_VALID		(1 << 20)
 
 #define EMAC_RXMBPENABLE_RXCAFEN_ENABLE	(0x200000)
 #define EMAC_RXMBPENABLE_RXBROADEN	(0x2000)
@@ -283,10 +305,40 @@ typedef struct  {
 
 /* EMAC Wrapper Registers Structure */
 typedef struct  {
-#if defined(CONFIG_SOC_DM646X) || defined(CONFIG_SOC_DM365)
-	dv_reg		IDVER;
-	dv_reg		SOFTRST;
-	dv_reg		EMCTRL;
+#ifdef DAVINCI_EMAC_VERSION2
+	dv_reg		idver;
+	dv_reg		softrst;
+	dv_reg		emctrl;
+	dv_reg		c0rxthreshen;
+	dv_reg		c0rxen;
+	dv_reg		c0txen;
+	dv_reg		c0miscen;
+	dv_reg		c1rxthreshen;
+	dv_reg		c1rxen;
+	dv_reg		c1txen;
+	dv_reg		c1miscen;
+	dv_reg		c2rxthreshen;
+	dv_reg		c2rxen;
+	dv_reg		c2txen;
+	dv_reg		c2miscen;
+	dv_reg		c0rxthreshstat;
+	dv_reg		c0rxstat;
+	dv_reg		c0txstat;
+	dv_reg		c0miscstat;
+	dv_reg		c1rxthreshstat;
+	dv_reg		c1rxstat;
+	dv_reg		c1txstat;
+	dv_reg		c1miscstat;
+	dv_reg		c2rxthreshstat;
+	dv_reg		c2rxstat;
+	dv_reg		c2txstat;
+	dv_reg		c2miscstat;
+	dv_reg		c0rximax;
+	dv_reg		c0tximax;
+	dv_reg		c1rximax;
+	dv_reg		c1tximax;
+	dv_reg		c2rximax;
+	dv_reg		c2tximax;
 #else
 	u_int8_t	RSVD0[4100];
 	dv_reg		EWCTL;
-- 
1.6.3.3

[U-Boot] [PATCH] TI: DaVinci: Updating EMAC driver for DM365, DM646x and DA8XX

[Wolfgang Denk]

Dear Nick Thompson,

In message <4B2770F8.5090607@ge.com> you wrote:
> The EMAC IP on DM365, DM646x and DA830 is slightly different
> from that on DM644x. This change updates the DaVinci EMAC driver
> so that EMAC becomes operational on SOCs with EMAC v2.
> 
> Signed-off-by: Nick Thompson <nick.thompson@ge.com>
> Signed-off-by: Sandeep Paulraj <s-paulraj@ti.com>
> ---
> Applies to: u-boot-ti
> 
> This is a combined patch with Sandeep's DM365 and DM646x changes
> and additional changes for DA830. It replaces previous submissions
> for EMAC support on these devices.
> 
>  drivers/net/davinci_emac.c               |  131 ++++++++++++++++++++++++-----
>  include/asm-arm/arch-davinci/emac_defs.h |   60 +++++++++++++-
>  2 files changed, 164 insertions(+), 27 deletions(-)
> 
> diff --git a/drivers/net/davinci_emac.c b/drivers/net/davinci_emac.c
> index fa8cee4..dbf94d2 100644
> --- a/drivers/net/davinci_emac.c
> +++ b/drivers/net/davinci_emac.c
> @@ -42,6 +42,7 @@
>  #include <miiphy.h>
>  #include <malloc.h>
>  #include <asm/arch/emac_defs.h>
> +#include <asm/io.h>
>  
>  unsigned int	emac_dbg = 0;
>  #define debug_emac(fmt,args...)	if (emac_dbg) printf(fmt,##args)
> @@ -107,6 +108,35 @@ static void davinci_eth_mdio_enable(void)
>  	while (adap_mdio->CONTROL & MDIO_CONTROL_IDLE) {;}

Please fix this as well while we are here. Please make this:

	while (adap_mdio->CONTROL & MDIO_CONTROL_IDLE)
		;
	

> +	/* Wait for command to complete */
> +	while (readl(&adap_mdio->USERACCESS0) & MDIO_USERACCESS0_GO);

Please make this:

	while (readl(&adap_mdio->USERACCESS0) & MDIO_USERACCESS0_GO)
		;


> +static void emac_gigabit_enable(void)
> +{
> +#ifdef DAVINCI_EMAC_GIG_ENABLE
> +	int temp
> +
> +	if (mdio_read(EMAC_MDIO_PHY_NUM, 0) & (1 << 6)) {
> +		/*
> +		 * Check if link detected is giga-bit
> +		 * If Gigabit mode detected, enable gigbit in MAC and PHY
> +		 */
> +		writel(EMAC_MACCONTROL_GIGFORCE |
> +		       EMAC_MACCONTROL_GIGABIT_ENABLE,
> +		       &adap_emac->MACCONTROL);
> +
> +		/*
> +		 * The SYS_CLK which feeds the SOC for giga-bit operation
> +		 * does not seem to be enabled after reset as expected.
> +		 * Force enabling SYS_CLK by writing to the PHY
> +		 */
> +		temp = mdio_read(EMAC_MDIO_PHY_NUM, 22);
> +		temp |= (1 << 4);
> +		mdio_write(EMAC_MDIO_PHY_NUM, 22, temp);
> +	}
> +#endif
> +}

Can we - instead of providing an empty function when
DAVINCI_EMAC_GIG_ENABLE is not set - either omit this function
completely, or use a weak implementation instead?

>  	if (!phy.get_link_speed(active_phy_addr))
>  		return(0);
> +	else
> +		emac_gigabit_enable();

No "else" is needed here. Remove it, and un-indent the
emac_gigabit_enable() call.

>  	if (!phy.get_link_speed (active_phy_addr)) {
>  		printf ("WARN: emac_send_packet: No link\n");
>  		return (ret_status);
> -	}
> +	} else
> +		emac_gigabit_enable();

Ditto.

>  		if (!phy.get_link_speed (active_phy_addr)) {
>  			davinci_eth_ch_teardown (EMAC_CH_TX);
>  			return (ret_status);
> -		}
> +		} else
> +			emac_gigabit_enable();

And again.

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
No problem is insoluble.
	-- Dr. Janet Wallace, "The Deadly Years", stardate 3479.4

[U-Boot] [PATCH] TI: DaVinci: Updating EMAC driver for DM365, DM646x and DA8XX

[Nick Thompson]

On 16/12/09 22:00, Wolfgang Denk wrote:
> Dear Nick Thompson,
> 
> In message <4B2770F8.5090607@ge.com> you wrote:
>> The EMAC IP on DM365, DM646x and DA830 is slightly different
>> from that on DM644x. This change updates the DaVinci EMAC driver
>> so that EMAC becomes operational on SOCs with EMAC v2.
>>
>> Signed-off-by: Nick Thompson <nick.thompson@ge.com>
>> Signed-off-by: Sandeep Paulraj <s-paulraj@ti.com>
>> ---
>> Applies to: u-boot-ti
>>
>> This is a combined patch with Sandeep's DM365 and DM646x changes
>> and additional changes for DA830. It replaces previous submissions
>> for EMAC support on these devices.
>>
>>  drivers/net/davinci_emac.c               |  131 ++++++++++++++++++++++++-----
>>  include/asm-arm/arch-davinci/emac_defs.h |   60 +++++++++++++-
>>  2 files changed, 164 insertions(+), 27 deletions(-)
>>
>> diff --git a/drivers/net/davinci_emac.c b/drivers/net/davinci_emac.c
>> index fa8cee4..dbf94d2 100644
>> --- a/drivers/net/davinci_emac.c
>> +++ b/drivers/net/davinci_emac.c
>> @@ -42,6 +42,7 @@
>>  #include <miiphy.h>
>>  #include <malloc.h>
>>  #include <asm/arch/emac_defs.h>
>> +#include <asm/io.h>
>>  
>>  unsigned int	emac_dbg = 0;
>>  #define debug_emac(fmt,args...)	if (emac_dbg) printf(fmt,##args)
>> @@ -107,6 +108,35 @@ static void davinci_eth_mdio_enable(void)
>>  	while (adap_mdio->CONTROL & MDIO_CONTROL_IDLE) {;}
> 
> Please fix this as well while we are here. Please make this:
> 
> 	while (adap_mdio->CONTROL & MDIO_CONTROL_IDLE)
> 		;

Yes, will do, here and elsewhere in the file. I will also change
all these cases to use readl().

> 	
> 
>> +	/* Wait for command to complete */
>> +	while (readl(&adap_mdio->USERACCESS0) & MDIO_USERACCESS0_GO);
> 
> Please make this:
> 
> 	while (readl(&adap_mdio->USERACCESS0) & MDIO_USERACCESS0_GO)
> 		;

Done.

> 
> 
>> +static void emac_gigabit_enable(void)
>> +{
>> +#ifdef DAVINCI_EMAC_GIG_ENABLE
>> +	int temp
>> +
>> +	if (mdio_read(EMAC_MDIO_PHY_NUM, 0) & (1 << 6)) {
>> +		/*
>> +		 * Check if link detected is giga-bit
>> +		 * If Gigabit mode detected, enable gigbit in MAC and PHY
>> +		 */
>> +		writel(EMAC_MACCONTROL_GIGFORCE |
>> +		       EMAC_MACCONTROL_GIGABIT_ENABLE,
>> +		       &adap_emac->MACCONTROL);
>> +
>> +		/*
>> +		 * The SYS_CLK which feeds the SOC for giga-bit operation
>> +		 * does not seem to be enabled after reset as expected.
>> +		 * Force enabling SYS_CLK by writing to the PHY
>> +		 */
>> +		temp = mdio_read(EMAC_MDIO_PHY_NUM, 22);
>> +		temp |= (1 << 4);
>> +		mdio_write(EMAC_MDIO_PHY_NUM, 22, temp);
>> +	}
>> +#endif
>> +}
> 
> Can we - instead of providing an empty function when
> DAVINCI_EMAC_GIG_ENABLE is not set - either omit this function
> completely, or use a weak implementation instead?

I don't want to use weak as it implies the function maybe replaced. This is
not the intention here. To avoid ifdefs all over the place I have added:

#ifdef DAVINCI_EMAC_GIG_ENABLE
#define mdio_gigabit_detect(phy)	(mdio_read(phy, 0) & (1 << 6))
#else
#define mdio_gigabit_detect(phy)	0
#endif

and changed the if in the above function to:

	if (mdio_gigabit_detect(EMAC_MDIO_PHY_NUM)) {
		int temp;

		...

The function is always present, but optimised away if there is a 0 method for
detecting gigabit in the phy. Is that acceptable?

> 
>>  	if (!phy.get_link_speed(active_phy_addr))
>>  		return(0);
>> +	else
>> +		emac_gigabit_enable();
> 
> No "else" is needed here. Remove it, and un-indent the
> emac_gigabit_enable() call.

Ahh yes - removed in all three cases.

I will submit a new patch tomorrow.

Thanks,
Nick.

[U-Boot] [PATCH] TI: DaVinci: Updating EMAC driver for DM365, DM646x and DA8XX

[Sriramakrishnan]

Nick Thompson <nick.thompson <at> ge.com> writes:

> +/* Write to a PHY register via MDIO inteface */
> +void mdio_write(int phy_addr, int reg_num, unsigned int data)
> +{
> +	/* Wait for User access register to be ready */
> +	while (readl(&adap_mdio->USERACCESS0) & MDIO_USERACCESS0_GO);
> +

Arent these functions duplicating functionality in davinci_eth_phy_write() and 
davinci_eth_phy_read()?  Is the intent here to move from accessing registers 
as overlay to using writel/readl functions. If so, we need to get rid of one 
of these implementations

> +static void emac_gigabit_enable(void)
> +{
> +#ifdef DAVINCI_EMAC_GIG_ENABLE
> +	int temp
> +
> +	if (mdio_read(EMAC_MDIO_PHY_NUM, 0) & (1 << 6)) {
> +		/*
> +		 * Check if link detected is giga-bit
> +		 * If Gigabit mode detected, enable gigbit in MAC and PHY
> +		 */
> +		writel(EMAC_MACCONTROL_GIGFORCE |
> +		       EMAC_MACCONTROL_GIGABIT_ENABLE,
> +		       &adap_emac->MACCONTROL);
> +
> +		/*
> +		 * The SYS_CLK which feeds the SOC for giga-bit operation
> +		 * does not seem to be enabled after reset as expected.
> +		 * Force enabling SYS_CLK by writing to the PHY
> +		 */
> +		temp = mdio_read(EMAC_MDIO_PHY_NUM, 22);
> +		temp |= (1 << 4);
> +		mdio_write(EMAC_MDIO_PHY_NUM, 22, temp);
> +	}
> +#endif
> +}
PHY register 22 is vendor specific(specific to a PHY) and the same PHY need 
not be Used on all platforms which use this driver. Also, can this not be done 
once during initialization alone?

> 
> +#if defined(DAVINCI_EMAC_VERSION2)
> +	writel(1, &adap_ewrap->softrst);
> +	while (readl(&adap_ewrap->softrst) != 0);
> +#else
>  	adap_ewrap->EWCTL = 0;
>  	for (cnt = 0; cnt < 5; cnt++) {
>  		clkdiv = adap_ewrap->EWCTL;
>  	}
> +#endif

Again some registers are accessed through overlays and some are accessed 
through readl/writel(). Should we make this consistent? Also the register 
names use CAPs in some instances and not for others - is this intentional?

>  	/* Init MDIO & get link state */
>  	clkdiv = (EMAC_MDIO_BUS_FREQ / EMAC_MDIO_CLOCK_FREQ) - 1;
>  	adap_mdio->CONTROL = ((clkdiv & 0xff) | MDIO_CONTROL_ENABLE | 
MDIO_CONTROL_FAULT);
> 
> +#if defined(DAVINCI_EMAC_VERSION2)
> +	/* We need to wait for MDIO to start */
> +	udelay(1000);
> +#endif
> +
 
The controller doesnt specify/expect a delay for EMAC version2. How is the 
delay specific to EMAC version2?