On 16/12/09 22:00, Wolfgang Denk wrote: > Dear Nick Thompson, > > In message <4b2770f8.5090...@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.thomp...@ge.com> >> Signed-off-by: Sandeep Paulraj <s-paul...@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 mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot