[U-Boot] [PATCH 6/8] AM35xx: Read and set ethaddr for EMAC

[Tom Rini <trini@ti.com>]

On 12/07/2011 07:48 AM, Igor Grinberg wrote:
> 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);
> }

That's reasonable enough.  I wasn't sure where the custom boards were
getting their MAC as the EVM and Crane both need this change.

-- 
Tom