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

[Marek Vasut]

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 at 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]
 */


> +	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.

> 
>  	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