| Message ID | 1300391223-11879-3-git-send-email-mspang@csclub.uwaterloo.ca |
|---|---|
| State | Changes Requested |
| Headers | show |
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? > 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); ? > /* 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. Best regards, Wolfgang Denk
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
diff --git a/drivers/net/mvgbe.c b/drivers/net/mvgbe.c index c701f43..bab55b3 100644 --- a/drivers/net/mvgbe.c +++ b/drivers/net/mvgbe.c @@ -380,6 +380,22 @@ static void port_uc_addr_set(struct mvgbe_registers *regs, u8 * p_addr) } /* + * port_uc_addr_get - This function gets the port unicast address. + */ +static void port_uc_addr_get(struct mvgbe_registers *regs, u8 * p_addr) +{ + u32 mac_l = MVGBE_REG_RD(regs->macal); + u32 mac_h = MVGBE_REG_RD(regs->macah); + + p_addr[0] = (mac_h >> 24); + p_addr[1] = (mac_h >> 16); + p_addr[2] = (mac_h >> 8); + p_addr[3] = (mac_h >> 0); + p_addr[4] = (mac_l >> 8); + p_addr[5] = (mac_l >> 0); +} + +/* * mvgbe_init_rx_desc_ring - Curve a Rx chain desc list and buffer in memory. */ static void mvgbe_init_rx_desc_ring(struct mvgbe_device *dmvgbe) @@ -719,6 +735,9 @@ error1: } while (!eth_getenv_enetaddr(s, dev->enetaddr)) { +#if defined(CONFIG_PRESERVE_LOCAL_MAC) + port_uc_addr_get(dmvgbe->regs, dmvgbe->dev.enetaddr); +#else /* 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); }
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. Signed-off-by: Michael Spang <mspang@csclub.uwaterloo.ca> --- drivers/net/mvgbe.c | 20 ++++++++++++++++++++ 1 files changed, 20 insertions(+), 0 deletions(-)