[U-Boot] [PATCH v3] am3517_evm: activate Ethernet PHY

[Tom Rini]

On Fri, Dec 06, 2013 at 06:59:52AM -0500, Tom Rini wrote:
> On Fri, Dec 06, 2013 at 11:47:38AM +0100, Yegor Yefremov wrote:
> > On Fri, Dec 6, 2013 at 11:37 AM, Stefan Roese <stefan.roese at gmail.com> wrote:
> > > On 06.12.2013 11:17, yegorslists at googlemail.com wrote:
> > >> From: Yegor Yefremov <yegorslists at googlemail.com>
> > >>
> > >> Pin 30 is connected to PHY's RESET# signal, so it must be
> > >> put to high. Otherwise PHY won't be found via MDIO interface.
> > >>
> > >> Signed-off-by: Yegor Yefremov <yegorslists at googlemail.com>
> > >
> > > Looks good. One questions below though:
> > >
> > >> ---
> > >> Changes:
> > >>       v3: use "__maybe_unused", instead of #if defined statement (Stefan Roese)
> > >>       v2: put ctr and reset under #if defined statement, to avoid compiler warnings, when EMAC is not selected
> > >>
> > >>  board/logicpd/am3517evm/am3517evm.c |   34 ++++++++++++++++++++++++++++++++++
> > >>  board/logicpd/am3517evm/am3517evm.h |    2 +-
> > >>  2 files changed, 35 insertions(+), 1 deletions(-)
> > >>
> > >> diff --git a/board/logicpd/am3517evm/am3517evm.c b/board/logicpd/am3517evm/am3517evm.c
> > >> index 1569905..3b1dfd1 100644
> > >> --- a/board/logicpd/am3517evm/am3517evm.c
> > >> +++ b/board/logicpd/am3517evm/am3517evm.c
> > >> @@ -22,6 +22,7 @@
> > >>  #include <asm/arch/musb.h>
> > >>  #include <asm/mach-types.h>
> > >>  #include <asm/errno.h>
> > >> +#include <asm/gpio.h>
> > >>  #include <linux/usb/ch9.h>
> > >>  #include <linux/usb/gadget.h>
> > >>  #include <linux/usb/musb.h>
> > >> @@ -31,6 +32,9 @@
> > >>
> > >>  DECLARE_GLOBAL_DATA_PTR;
> > >>
> > >> +#define AM3517_IP_SW_RESET   0x48002598
> > >> +#define CPGMACSS_SW_RST              (1 << 1)
> > >> +
> > >>  /*
> > >>   * Routine: board_init
> > >>   * Description: Early hardware init.
> > >> @@ -98,6 +102,9 @@ static void am3517_evm_musb_init(void)
> > >>   */
> > >>  int misc_init_r(void)
> > >>  {
> > >> +     __maybe_unused volatile unsigned int ctr;
> > >> +     __maybe_unused u32 reset;
> > >> +
> > >>  #ifdef CONFIG_SYS_I2C_OMAP34XX
> > >>       i2c_init(CONFIG_SYS_OMAP24_I2C_SPEED, CONFIG_SYS_OMAP24_I2C_SLAVE);
> > >>  #endif
> > >> @@ -106,6 +113,33 @@ int misc_init_r(void)
> > >>
> > >>       am3517_evm_musb_init();
> > >>
> > >> +#if defined(CONFIG_DRIVER_TI_EMAC)
> > >> +     /* activate PHY reset */
> > >> +     gpio_direction_output(30, 0);
> > >> +     gpio_set_value(30, 0);
> > >> +
> > >> +     ctr  = 0;
> > >> +     do {
> > >> +             udelay(1000);
> > >> +             ctr++;
> > >> +     } while (ctr < 300);
> > >> +
> > >> +     /* deactivate PHY reset */
> > >> +     gpio_set_value(30, 1);
> > >> +
> > >> +     /* allow the PHY to stabilize and settle down */
> > >> +     ctr = 0;
> > >> +     do {
> > >> +             udelay(1000);
> > >> +             ctr++;
> > >> +     } while (ctr < 300);
> > >> +
> > >> +     /* ensure that the module is out of reset */
> > >> +     reset = readl(AM3517_IP_SW_RESET);
> > >> +     reset &= (~CPGMACSS_SW_RST);
> > >> +     writel(reset,AM3517_IP_SW_RESET);
> > >> +#endif
> > >
> > > Why do you need to put this "#if defined(CONFIG_DRIVER_TI_EMAC)" here at
> > > all? Isn't CONFIG_DRIVER_TI_EMAC enabled for this board always?
> > 
> > It is enabled in arago repository:
> > http://arago-project.org/git/projects/?p=u-boot-omap3.git;a=blob;f=include/configs/am3517_evm.h;h=b37b81e2aa5d635d14db87393e751ac25cb3611e;hb=HEAD,
> > but not in upstream u-boot. I was testing the stuff with my own board,
> > so my am3517_evm.h is a little bit messy for now. That's why I wanted
> > to have this change only in board/logicpd/am3517evm/am3517evm.c. It
> > does no harm so far. I could patch am3517_evm.h later.
> 
> Ah, I forgot that I marked http://patchwork.ozlabs.org/patch/129720/ as
> Deferred way back when rather than merge it.  I don't know why I didn't
> need the bit you pulled in when I was testing eth.
> 
> The right answer here is to do both of these so that we are using and
> testing the feature we add.  I'll see if I can dig up and boot up my
> am3517_evm.

Bah, it's not quite trivial to apply the old patch, some other stuff
needs to be enabled too, so I'll set this aside for now.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20131206/b8f3e6e1/attachment.pgp>