From mboxrd@z Thu Jan  1 00:00:00 1970
From: Igor Grinberg <grinberg@compulab.co.il>
Date: Wed, 07 Dec 2011 16:48:24 +0200
Subject: [U-Boot] [PATCH 6/8] AM35xx: Read and set ethaddr for EMAC
In-Reply-To: <1323186582-2811-7-git-send-email-trini@ti.com>
References: <1323186582-2811-1-git-send-email-trini@ti.com>
	<1323186582-2811-7-git-send-email-trini@ti.com>
Message-ID: <4EDF7CB8.3030205@compulab.co.il>
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

Hi Steve, Tom,

On 12/06/11 17:49, Tom Rini wrote:
> From: Steve Kipisz <s-kipisz2@ti.com>
> 
> Signed-off-by: Steve Kipisz <s-kipisz2@ti.com>
> Signed-off-by: Tom Rini <trini@ti.com>
> ---
>  arch/arm/cpu/armv7/omap3/emac.c             |   20 +++++++++++++++++++-
>  arch/arm/include/asm/arch-omap3/emac_defs.h |    3 +++
>  2 files changed, 22 insertions(+), 1 deletions(-)
> 
> diff --git a/arch/arm/cpu/armv7/omap3/emac.c b/arch/arm/cpu/armv7/omap3/emac.c
> index 14667f1..d400bef 100644
> --- a/arch/arm/cpu/armv7/omap3/emac.c
> +++ b/arch/arm/cpu/armv7/omap3/emac.c
> @@ -26,6 +26,7 @@
>  #include <netdev.h>
>  #include <asm/io.h>
>  #include <asm/arch/am35x_def.h>
> +#include <asm/arch/emac_defs.h>
>  
>  /*
>   * Initializes on-chip ethernet controllers.
> @@ -33,12 +34,29 @@
>   */
>  int cpu_eth_init(bd_t *bis)
>  {
> -	u32 reset;
> +	u32 reset, msb, lsb;
> +	u_int8_t macaddr[6];
> +	int i;
>  
>  	/* ensure that the module is out of reset */
>  	reset = readl(&am35x_scm_general_regs->ip_sw_reset);
>  	reset &= ~CPGMACSS_SW_RST;
>  	writel(reset, &am35x_scm_general_regs->ip_sw_reset);
>  
> +	/* Read MAC address */
> +	msb = readl(EMAC_MACADDR_MSB);
> +	lsb = readl(EMAC_MACADDR_LSB);
> +
> +	for (i = 0; i < 3; i++) {
> +		macaddr[5 - i] = lsb & 0xFF;
> +		lsb >>= 8;
> +	}
> +
> +	for (i = 0; i < 3; i++) {
> +		macaddr[2 - i] = msb & 0xFF;
> +		msb >>= 8;
> +	}
> +	eth_setenv_enetaddr("ethaddr", macaddr);
> +

This is a wrong place for this code...
You force every board to use the EFUSE'd MAC address - this is wrong.
The board must have a freedom to choose what MAC address it wants to use.
You don't even check if ethaddr variable is already set...

What you can do is put this implementation into a separate function
and let board to make a decision if it wants to call it or use another
MAC address.
Something like:
int am3517_get_efuse_enetaddr(u8 *enetaddr)
{
	/* read the address and shift or whatever */
	...
	return is_valid_ether_addr(enetaddr);
}

>  	return davinci_emac_initialize();
>  }
> diff --git a/arch/arm/include/asm/arch-omap3/emac_defs.h b/arch/arm/include/asm/arch-omap3/emac_defs.h
> index 8506c55..c3c96c0 100644
> --- a/arch/arm/include/asm/arch-omap3/emac_defs.h
> +++ b/arch/arm/include/asm/arch-omap3/emac_defs.h
> @@ -42,6 +42,9 @@
>  #define EMAC_MDIO_BASE_ADDR            0x5C030000
>  #define EMAC_HW_RAM_ADDR               0x01E20000
>  
> +#define EMAC_MACADDR_LSB	       0x48002380
> +#define EMAC_MACADDR_MSB	       0x48002384
> +
>  #define EMAC_MDIO_BUS_FREQ             166000000       /* 166 MHZ check */
>  #define EMAC_MDIO_CLOCK_FREQ           1000000         /* 2.0 MHz */
>  

-- 
Regards,
Igor.