[U-Boot] [PATCH v3 5/7] ARM: OMAP5-uevm: Add USB MAC ethernet address

[Dan Murphy <dmurphy@ti.com>]

On 07/17/2013 11:28 PM, Marek Vasut wrote:
> Dear Dan Murphy,
>
>> Set the usbethaddr based on the OMAP DIE_ID registers
>> which should be unique for each processor.
>>
>> Then set this as the usb ethernet MAC address.
>>
>> Signed-off-by: Dan Murphy <dmurphy@ti.com>
>> ---
>> v3 - new patch
>>
>>  board/ti/omap5_uevm/evm.c    |   26 ++++++++++++++++++++++++++
>>  include/configs/omap5_uevm.h |    2 ++
>>  2 files changed, 28 insertions(+)
>>
>> diff --git a/board/ti/omap5_uevm/evm.c b/board/ti/omap5_uevm/evm.c
>> index bf14cd2..9add0fd 100644
>> --- a/board/ti/omap5_uevm/evm.c
>> +++ b/board/ti/omap5_uevm/evm.c
>> @@ -33,6 +33,11 @@
>>  #include <usb.h>
>>  #include <asm/arch/ehci.h>
>>  #include <asm/ehci-omap.h>
>> +
>> +#define MAX_DEVICE_MAC		20
>> +#define DIE_ID_REG_BASE         (OMAP54XX_L4_CORE_BASE + 0x2000)
>> +#define DIE_ID_REG_OFFSET	0x200
>> +
>>  #endif
>>
>>  DECLARE_GLOBAL_DATA_PTR;
>> @@ -121,6 +126,27 @@ int ehci_hcd_init(int index, struct ehci_hccr **hccr,
>> struct ehci_hcor **hcor) {
>>  	int ret;
>>  	int auxclk;
>> +	int val[4] = { 0 };
>> +	int reg;
>> +	char device_mac[MAX_DEVICE_MAC];
>> +
>> +	reg = DIE_ID_REG_BASE + DIE_ID_REG_OFFSET;
>> +
>> +	val[0] = readl(reg);
>> +	val[1] = readl(reg + 0x8);
>> +	val[2] = readl(reg + 0xC);
>> +	val[3] = readl(reg + 0x10);
>> +
>> +	/* create a fake MAC address from the processor ID code.
>> +	 * first byte is 0x02 to signify locally administered.
>> +	 */
> /*
>  * valid
>  * multiline
>  * comment ... has that leading newline on the top. [1]
>  */
>
Will fix
>> +	snprintf(device_mac, MAX_DEVICE_MAC, "%02X:%02X:%02X:%02X:%02X:%02X",
>> +				0x02, val[3] & 0xff, val[2] & 0xff, val[1] & 
> 0xff,
>> +				val[0] & 0xff, (val[0] >> 8 & 0xff));
>> +
>> +#ifdef CONFIG_ENV_VARS_UBOOT_RUNTIME_CONFIG
>> +	setenv("usbethaddr", device_mac);
>> +#endif
> Urgh ... this is a big fat NAK. No, just don't do this, please. A kitten will 
> die if you set variables in the code like this. Another one will die for such a 
> config option.
>
> You might work around this problem by setting a different env variable (yet I am 
> still unhappy to see this) and then in the board environment have a command to 
> load the USB mac address from that other variable. But do not enforce the mac 
> address on users.

I will modify the usbethaddr only if the user has not already set the variable.
This is the way I see it in other code as well.

Is that acceptable?
>
>>  	auxclk = readl((*prcm)->scrm_auxclk1);
>>  	/* Request auxilary clock */
>> diff --git a/include/configs/omap5_uevm.h b/include/configs/omap5_uevm.h
>> index 0740a32..53b86ec 100644
>> --- a/include/configs/omap5_uevm.h
>> +++ b/include/configs/omap5_uevm.h
>> @@ -79,5 +79,7 @@
>>
>>  #define CONSOLEDEV		"ttyO2"
>>
>> +#define CONFIG_ENV_VARS_UBOOT_RUNTIME_CONFIG
>> +
>>  #define CONFIG_OMAP_PLATFORM_RESET_TIME_MAX_USEC	16296
>>  #endif /* __CONFIG_OMAP5_EVM_H */
> [1] http://www.denx.de/wiki/U-Boot/CodingStyle
>
> Best regards,
> Marek Vasut


-- 
------------------
Dan Murphy