From mboxrd@z Thu Jan  1 00:00:00 1970
From: Stefan Roese <stefan.roese@gmail.com>
Date: Fri, 06 Dec 2013 11:37:04 +0100
Subject: [U-Boot] [PATCH v3] am3517_evm: activate Ethernet PHY
In-Reply-To: <1386325057-4582-1-git-send-email-yegorslists@googlemail.com>
References: <1386325057-4582-1-git-send-email-yegorslists@googlemail.com>
Message-ID: <52A1A8D0.5020407@gmail.com>
List-Id: <u-boot.lists.denx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: u-boot@lists.denx.de

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?

Thanks,
Stefan