From mboxrd@z Thu Jan  1 00:00:00 1970
From: Stefano Babic <sbabic@denx.de>
Date: Fri, 20 May 2011 10:47:14 +0200
Subject: [U-Boot] [PATCH] mx51evk: Make use of the i.MX GPIO API
In-Reply-To: <1305832522-20712-1-git-send-email-fabio.estevam@freescale.com>
References: <1305832522-20712-1-git-send-email-fabio.estevam@freescale.com>
Message-ID: <4DD62A92.6010001@denx.de>
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 05/19/2011 09:15 PM, Fabio Estevam wrote:
> Make use of the i.MX GPIO API.

Hi Fabio,

thanks for cleanup this code.

> 
> While at it created a separate function for the FEC PHY reset,
> so that it can be decoupled from the power_init function.
> 
> Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com>
> ---
>  board/freescale/mx51evk/mx51evk.c |   33 +++++++++++++++++----------------
>  1 files changed, 17 insertions(+), 16 deletions(-)
> 
> diff --git a/board/freescale/mx51evk/mx51evk.c b/board/freescale/mx51evk/mx51evk.c
> index ef7c6e4..2dabef9 100644
> --- a/board/freescale/mx51evk/mx51evk.c
> +++ b/board/freescale/mx51evk/mx51evk.c
> @@ -33,6 +33,11 @@
>  #include <fsl_esdhc.h>
>  #include <fsl_pmic.h>
>  #include <mc13892.h>
> +#include <mxc_gpio.h>
> +
> +#define MX51EVK_PHY_RESET	(1 * 32 + 14)	/* GPIO2_14 */
> +#define MX51EVK_SD1_CD		(0 * 32 + 0)	/* GPIO1_0 */
> +#define MX51EVK_SD2_CD		(0 * 32 + 6)	/* GPIO1_6 */
>  
>  DECLARE_GLOBAL_DATA_PTR;
>  
> @@ -261,27 +266,22 @@ static void power_init(void)
>  	pmic_reg_write(REG_MODE_1, val);
>  	udelay(200);
>  
> -	reg = readl(GPIO2_BASE_ADDR + 0x0);
> -	reg &= ~0x4000;  /* Lower reset line */
> -	writel(reg, GPIO2_BASE_ADDR + 0x0);
> -
> -	reg = readl(GPIO2_BASE_ADDR + 0x4);
> -	reg |= 0x4000;	/* configure GPIO lines as output */
> -	writel(reg, GPIO2_BASE_ADDR + 0x4);
> -
> -	/* Reset the ethernet controller over GPIO */
> -	writel(0x1, IOMUXC_BASE_ADDR + 0x0AC);
> -
>  	/* Enable VGEN3, VCAM, VAUDIO, VVIDEO, VSD regulators */
>  	val = VGEN3EN | VGEN3CONFIG | VCAMEN | VCAMCONFIG |
>  		VVIDEOEN | VAUDIOEN  | VSDEN;
>  	pmic_reg_write(REG_MODE_1, val);
> +}
> +
> +static void reset_phy_fec(void)
> +{
> +	mxc_request_iomux(MX51_PIN_EIM_A20, IOMUX_CONFIG_ALT1);
> +	mxc_gpio_direction(MX51EVK_PHY_RESET, MXC_GPIO_DIRECTION_OUT);
>  
> +	mxc_gpio_set(MX51EVK_PHY_RESET, 0);	/* Lower reset line */
> +
>  	udelay(500);
>  
> -	reg = readl(GPIO2_BASE_ADDR + 0x0);
> -	reg |= 0x4000;
> -	writel(reg, GPIO2_BASE_ADDR + 0x0);
> +	mxc_gpio_set(MX51EVK_PHY_RESET, 1);	/* Set reset line to high */
>  }

Something is not clear to me. To get the same functionality, I am
expecting that the phy is reset, then the PMIC is programmed, and at the
end the phy is freed from reset.

You add reset_phy_fec(), that sets the phy in reset. In actual code, at
the end of power_init the phy is out of reset. Now who remove the reset
? I do not see in your code.

Best regards,
Stefano Babic

-- 
=====================================================================
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80  Email: office at denx.de
=====================================================================