| Message ID | 1374092167-27645-6-git-send-email-dmurphy@ti.com |
|---|---|
| State | Awaiting Upstream |
| Delegated to: | Marek Vasut |
| Headers | show |
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] */ > + 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
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
Dear Dan Murphy, > 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? That'd also make sense, yeah. Best regards, Marek Vasut
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. + */ + 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 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 */
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(+)