All of lore.kernel.org
 help / color / mirror / Atom feed
From: Tom Rini <trini@ti.com>
To: u-boot@lists.denx.de
Subject: [U-Boot] [PATCH v3] am3517_evm: activate Ethernet PHY
Date: Fri, 6 Dec 2013 06:59:52 -0500	[thread overview]
Message-ID: <20131206115952.GR420@bill-the-cat> (raw)
In-Reply-To: <CAGm1_kscvjxqtcmhyLoHwXcQgcZ89gUiqtVv0rtUM5TE5pSicg@mail.gmail.com>

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@gmail.com> wrote:
> > On 06.12.2013 11:17, yegorslists at googlemail.com wrote:
> >> From: Yegor Yefremov <yegorslists@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@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.

-- 
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/11ca15f5/attachment.pgp>

  reply	other threads:[~2013-12-06 11:59 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-12-06 10:17 [U-Boot] [PATCH v3] am3517_evm: activate Ethernet PHY yegorslists at googlemail.com
2013-12-06 10:37 ` Stefan Roese
2013-12-06 10:47   ` Yegor Yefremov
2013-12-06 11:59     ` Tom Rini [this message]
2013-12-06 12:14       ` Tom Rini
2013-12-06 13:30         ` Yegor Yefremov

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20131206115952.GR420@bill-the-cat \
    --to=trini@ti.com \
    --cc=u-boot@lists.denx.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.