| Message ID | 1317383832-23480-3-git-send-email-nagabhushana.netagunte@ti.com |
|---|---|
| State | Changes Requested |
| Headers | show |
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
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.
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,
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