Patchwork [U-Boot,2/4] da830: emac: add support for RMII

login
register
mail settings
Submitter nagabhushana.netagunte@ti.com
Date Sept. 30, 2011, 11:57 a.m.
Message ID <1317383832-23480-3-git-send-email-nagabhushana.netagunte@ti.com>
Download mbox | patch
Permalink /patch/117109/
State Changes Requested
Headers show

Comments

nagabhushana.netagunte@ti.com - Sept. 30, 2011, 11:57 a.m.
From: Nagabhushana Netagunte <nagabhushana.netagunte@ti.com>

add support for RMII in davinci EMAC driver for da830. da850 RMII support
existed already in the driver. New configs are added to extend this support
for da830.

Signed-off-by: Sudhakar Rajashekhara <sudhakar.raj@ti.com>
Signed-off-by: Nagabhushana Netagunte <nagabhushana.netagunte@ti.com>
---
 drivers/net/davinci_emac.c        |    6 +++---
 include/configs/da830evm.h        |    1 +
 include/configs/da850_am18xxevm.h |    1 +
 include/configs/da850_l138evm.h   |    1 +
 4 files changed, 6 insertions(+), 3 deletions(-)
Mike Frysinger - Sept. 30, 2011, 3:48 p.m.
On Friday, September 30, 2011 07:57:10 nagabhushana.netagunte@ti.com wrote:
> --- a/drivers/net/davinci_emac.c
> +++ b/drivers/net/davinci_emac.c
> @@ -246,7 +246,7 @@ static int gen_get_link_speed(int phy_addr)
>  	if (davinci_eth_phy_read(phy_addr, MII_STATUS_REG, &tmp) &&
>  			(tmp & 0x04)) {
>  #if defined(CONFIG_DRIVER_TI_EMAC_USE_RMII) && \

there's a common CONFIG_RMII symbol already ...

> -		defined(CONFIG_MACH_DAVINCI_DA850_EVM)
> +		defined(CONFIG_MACH_DAVINCI_DA8XX_EVM)

maybe it's just me, but board level defines in an emac driver make no sense
-mike
Igor Grinberg - Oct. 2, 2011, 8:38 a.m.
On 09/30/11 18:48, Mike Frysinger wrote:
> On Friday, September 30, 2011 07:57:10 nagabhushana.netagunte@ti.com wrote:
>> --- a/drivers/net/davinci_emac.c
>> +++ b/drivers/net/davinci_emac.c
>> @@ -246,7 +246,7 @@ static int gen_get_link_speed(int phy_addr)
>>  	if (davinci_eth_phy_read(phy_addr, MII_STATUS_REG, &tmp) &&
>>  			(tmp & 0x04)) {
>>  #if defined(CONFIG_DRIVER_TI_EMAC_USE_RMII) && \
> 
> there's a common CONFIG_RMII symbol already ...
> 
>> -		defined(CONFIG_MACH_DAVINCI_DA850_EVM)
>> +		defined(CONFIG_MACH_DAVINCI_DA8XX_EVM)
> 
> maybe it's just me, but board level defines in an emac driver make no sense

No, it is not just you ;)
Davinci EMAC is used also in non davinci SoCs and restricting its
features to some board (or family of boards) indeed makes no sense.
There must be a good reason for doing so, otherwise that board
specific config should be removed.
Laurence Withers - Oct. 3, 2011, 2:07 p.m.
On Fri, Sep 30, 2011 at 11:48:13AM -0400, Mike Frysinger wrote:
> On Friday, September 30, 2011 07:57:10 nagabhushana.netagunte@ti.com wrote:
> > --- a/drivers/net/davinci_emac.c
> > +++ b/drivers/net/davinci_emac.c
> > @@ -246,7 +246,7 @@ static int gen_get_link_speed(int phy_addr)
> >  	if (davinci_eth_phy_read(phy_addr, MII_STATUS_REG, &tmp) &&
> >  			(tmp & 0x04)) {
> >  #if defined(CONFIG_DRIVER_TI_EMAC_USE_RMII) && \
> 
> there's a common CONFIG_RMII symbol already ...
> 
> > -		defined(CONFIG_MACH_DAVINCI_DA850_EVM)
> > +		defined(CONFIG_MACH_DAVINCI_DA8XX_EVM)
> 
> maybe it's just me, but board level defines in an emac driver make no sense

This led me to look a bit more closely at this code, and I think there is
something wrong with it. Under Linux, I am able to run in RMII mode at either
10BASE-T or 100BASE-TX, either by forcing it on the board with ethtool or by
forcing it on the managed switch.

However, the driver in U-Boot fails to work with 10BASE-T altogether. A
simple change to #if defined(CONFIG_SOC_DA8XX) doesn't seem to resolve the
situation so it would appear there are some bugs in the TI EMAC driver. I
also note it doesn't reset the PHY properly as if I reboot from Linux with
the PHY forced to 10BASE-T then U-Boot is no longer able to get the link up.

Bye for now,

Patch

diff --git a/drivers/net/davinci_emac.c b/drivers/net/davinci_emac.c
index be3aabf..0d67a06 100644
--- a/drivers/net/davinci_emac.c
+++ b/drivers/net/davinci_emac.c
@@ -246,7 +246,7 @@  static int gen_get_link_speed(int phy_addr)
 	if (davinci_eth_phy_read(phy_addr, MII_STATUS_REG, &tmp) &&
 			(tmp & 0x04)) {
 #if defined(CONFIG_DRIVER_TI_EMAC_USE_RMII) && \
-		defined(CONFIG_MACH_DAVINCI_DA850_EVM)
+		defined(CONFIG_MACH_DAVINCI_DA8XX_EVM)
 		davinci_eth_phy_read(phy_addr, MII_LPA, &tmp);
 
 		/* Speed doesn't matter, there is no setting for it in EMAC. */
@@ -381,7 +381,7 @@  static int davinci_eth_open(struct eth_device *dev, bd_t *bis)
 #endif
 
 #if defined(CONFIG_DRIVER_TI_EMAC_USE_RMII) && \
-	defined(CONFIG_MACH_DAVINCI_DA850_EVM)
+	defined(CONFIG_MACH_DAVINCI_DA8XX_EVM)
 	adap_ewrap->c0rxen = adap_ewrap->c1rxen = adap_ewrap->c2rxen = 0;
 	adap_ewrap->c0txen = adap_ewrap->c1txen = adap_ewrap->c2txen = 0;
 	adap_ewrap->c0miscen = adap_ewrap->c1miscen = adap_ewrap->c2miscen = 0;
@@ -541,7 +541,7 @@  static void davinci_eth_close(struct eth_device *dev)
 #endif
 
 #if defined(CONFIG_DRIVER_TI_EMAC_USE_RMII) && \
-	defined(CONFIG_MACH_DAVINCI_DA850_EVM)
+	defined(CONFIG_MACH_DAVINCI_DA8XX_EVM)
 	adap_ewrap->c0rxen = adap_ewrap->c1rxen = adap_ewrap->c2rxen = 0;
 	adap_ewrap->c0txen = adap_ewrap->c1txen = adap_ewrap->c2txen = 0;
 	adap_ewrap->c0miscen = adap_ewrap->c1miscen = adap_ewrap->c2miscen = 0;
diff --git a/include/configs/da830evm.h b/include/configs/da830evm.h
index a451513..b61443a 100644
--- a/include/configs/da830evm.h
+++ b/include/configs/da830evm.h
@@ -36,6 +36,7 @@ 
 #define CONFIG_MACH_DAVINCI_DA830_EVM
 #define CONFIG_ARM926EJS		/* arm926ejs CPU core */
 #define CONFIG_SOC_DA8XX		/* TI DA8xx SoC */
+#define CONFIG_MACH_DAVINCI_DA8XX_EVM
 #define CONFIG_SYS_CLK_FREQ		clk_get(DAVINCI_ARM_CLKID)
 #define CONFIG_SYS_OSCIN_FREQ		24000000
 #define CONFIG_SYS_TIMERBASE		DAVINCI_TIMER0_BASE
diff --git a/include/configs/da850_am18xxevm.h b/include/configs/da850_am18xxevm.h
index b525f14..8a65956 100644
--- a/include/configs/da850_am18xxevm.h
+++ b/include/configs/da850_am18xxevm.h
@@ -36,6 +36,7 @@ 
 #define CONFIG_MACH_DAVINCI_DA850_EVM
 #define CONFIG_ARM926EJS		/* arm926ejs CPU core */
 #define CONFIG_SOC_DA8XX		/* TI DA8xx SoC */
+#define CONFIG_MACH_DAVINCI_DA8XX_EVM
 #define CONFIG_SYS_CLK_FREQ		clk_get(DAVINCI_ARM_CLKID)
 #define CONFIG_SYS_OSCIN_FREQ		24000000
 #define CONFIG_SYS_TIMERBASE		DAVINCI_TIMER0_BASE
diff --git a/include/configs/da850_l138evm.h b/include/configs/da850_l138evm.h
index 9e4a652..afe00e8 100644
--- a/include/configs/da850_l138evm.h
+++ b/include/configs/da850_l138evm.h
@@ -36,6 +36,7 @@ 
 #define CONFIG_MACH_DAVINCI_DA850_EVM
 #define CONFIG_ARM926EJS		/* arm926ejs CPU core */
 #define CONFIG_SOC_DA8XX		/* TI DA8xx SoC */
+#define CONFIG_MACH_DAVINCI_DA8XX_EVM
 #define CONFIG_SYS_CLK_FREQ		clk_get(DAVINCI_ARM_CLKID)
 #define CONFIG_SYS_OSCIN_FREQ		24000000
 #define CONFIG_SYS_TIMERBASE		DAVINCI_TIMER0_BASE