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
