On Sun, Apr 24, 2011 at 6:50 PM, Wolfgang Denk <wd@denx.de> wrote:
> And please add documentation for the new CONFIG_PRESERVE_LOCAL_MAC to> the README.
We have something similar already on Freescale parts, but it does
sometimes cause problems. If the environment ethaddr is already set,
it is left alone. Otherwise, the MAC address is read from EEPROM.
So we could use CONFIG_PRESERVE_LOCAL_MAC to alter this behavior (i.e.
always use the EEPROM). However, the current default is already
CONFIG_PRESERVE_LOCAL_MAC, so can we change this to something like
CONFIG_OVERRIDE_LOCAL_MAC?

On Sun, Apr 24, 2011 at 7:50 PM, Wolfgang Denk <wd@denx.de> wrote:
> Dear Michael Spang,>> In message <1300391223-11879-3-git-send-email-mspang@csclub.uwaterloo.ca> you wrote:>> The MVGBE driver either gets the MAC from the environment, or invents>> one. This allows the driver to leave the existing address alone in>> case it is initialized before U-Boot starts.>> Who or what would be doing that?
The manufacturer's bootloader runs first, and cannot easily be
replaced. So U-Boot is loaded second, after the MAC is already set.
>>> while (!eth_getenv_enetaddr(s, dev->enetaddr)) {>> +#if defined(CONFIG_PRESERVE_LOCAL_MAC)>> + port_uc_addr_get(dmvgbe->regs, dmvgbe->dev.enetaddr);>> +#else>> For consistency, should this not be>> port_uc_addr_get(dmvgbe->regs, dev->enetaddr);>> ?
Yes.
>>>> /* Generate Private MAC addr if not set */>> dev->enetaddr[0] = 0x02;>> dev->enetaddr[1] = 0x50;>> @@ -734,6 +753,7 @@ error1:>> dev->enetaddr[4] = get_random_hex();>> dev->enetaddr[5] = get_random_hex();>> #endif>> +#endif>> eth_setenv_enetaddr(s, dev->enetaddr);>> }>> And please add documentation for the new CONFIG_PRESERVE_LOCAL_MAC to> the README.
Ok I'll add:
CONFIG_PRESERVE_LOCAL_MAC
If no MAC address is set in the environment, then
preserve the ethernet interface's current MAC address.
Used if the MAC is configured before U-Boot is loaded.
Thanks,
Michael

On Mon, Apr 25, 2011 at 7:37 AM, Tabi Timur-B04825 <B04825@freescale.com> wrote:
> On Sun, Apr 24, 2011 at 6:50 PM, Wolfgang Denk <wd@denx.de> wrote:>>> And please add documentation for the new CONFIG_PRESERVE_LOCAL_MAC to>> the README.>> We have something similar already on Freescale parts, but it does> sometimes cause problems. If the environment ethaddr is already set,> it is left alone. Otherwise, the MAC address is read from EEPROM.>> So we could use CONFIG_PRESERVE_LOCAL_MAC to alter this behavior (i.e.> always use the EEPROM). However, the current default is already> CONFIG_PRESERVE_LOCAL_MAC, so can we change this to something like> CONFIG_OVERRIDE_LOCAL_MAC?
I don't think the option you want is the same. If there's a MAC in the
environment, I want to use it. Otherwise, I want U-Boot to leave the
MAC alone because the manufacturer assigned MAC was written to the
interface's registers before U-Boot started.
Michael

On Tuesday, April 26, 2011 00:23:47 Michael Spang wrote:
> On Mon, Apr 25, 2011 at 7:37 AM, Tabi Timur-B04825 wrote:> > On Sun, Apr 24, 2011 at 6:50 PM, Wolfgang Denk wrote:> >> And please add documentation for the new CONFIG_PRESERVE_LOCAL_MAC to> >> the README.> > > > We have something similar already on Freescale parts, but it does> > sometimes cause problems. If the environment ethaddr is already set,> > it is left alone. Otherwise, the MAC address is read from EEPROM.> > > > So we could use CONFIG_PRESERVE_LOCAL_MAC to alter this behavior (i.e.> > always use the EEPROM). However, the current default is already> > CONFIG_PRESERVE_LOCAL_MAC, so can we change this to something like> > CONFIG_OVERRIDE_LOCAL_MAC?> > I don't think the option you want is the same. If there's a MAC in the> environment, I want to use it. Otherwise, I want U-Boot to leave the> MAC alone because the manufacturer assigned MAC was written to the> interface's registers before U-Boot started.
so implement this in your board file in misc_init_r or board_eth_init. have
the code do something like:
uchar enetaddr[6];
if (!eth_getenv_enetaddr("ethaddr", enetaddr)) {
/* ... read current MAC out of the driver's registers ... */
eth_setenv_enetaddr("ethaddr", enetaddr);
}
then you dont need ugly config hacks in random drivers
-mike

Mike Frysinger wrote:
> so implement this in your board file in misc_init_r or board_eth_init. have> the code do something like:> uchar enetaddr[6];> if (!eth_getenv_enetaddr("ethaddr", enetaddr)) {> /* ... read current MAC out of the driver's registers ... */> eth_setenv_enetaddr("ethaddr", enetaddr);> }>> then you dont need ugly config hacks in random drivers
This is a feature that could be applied to the e1000 drivers. The current situation is a mess. Some drivers ignore the environment, some of them always use it. This should probably be standardized.

On Sat, Apr 30, 2011 at 10:34, Tabi Timur-B04825 wrote:
> Mike Frysinger wrote:>> so implement this in your board file in misc_init_r or board_eth_init. have>> the code do something like:>> uchar enetaddr[6];>> if (!eth_getenv_enetaddr("ethaddr", enetaddr)) {>> /* ... read current MAC out of the driver's registers ... */>> eth_setenv_enetaddr("ethaddr", enetaddr);>> }>>>> then you dont need ugly config hacks in random drivers>> This is a feature that could be applied to the e1000 drivers. The current situation is a mess. Some drivers ignore the environment, some of them always use it. This should probably be standardized.
it is standardized already. no driver should be touching the
environment. it should only ever use its own eth_device->enetaddr.
the common eth code already takes care of syncing the env and that
member.
also, please fix your e-mailer to properly wrap long lines.
-mike