[U-Boot] [PATCH 17/30] lan91c96/smc91111/smc911x: get mac address from environment

[Wolfgang Denk]

Dear Mike Frysinger,

In message <200906092030.49884.vapier at gentoo.org> you wrote:
>
> > This patch (see also commit 03f3d8d3b39c,
> > http://git.denx.de/?p=u-boot.git;a=commit;h=03f3d8d3b39cf85c0ce7ca903b43670
> >1e8aa610b) changed behaviour of some network drivers.
> >
> > As I just learned (sorry, I missed this in the initial review) it
> > drops a warning printed by the old code, when there were valid MAC
> > addresses stored both in the U-Boot environment ("ethaddr" variable)
> >
> > and in the controller's EEPROM:
> > > -	if (env_present && rom_valid) { /* if both env and ROM are good */
> > > -		if (memcmp (v_env_mac, v_rom_mac, 6) != 0) {
> > > -			printf ("\nWarning: MAC addresses don't match:\n");
> > > -			printf ("\tHW MAC address:  "
> > > -				"%02X:%02X:%02X:%02X:%02X:%02X\n",
> > > -				v_rom_mac[0], v_rom_mac[1],
> > > -				v_rom_mac[2], v_rom_mac[3],
> > > -				v_rom_mac[4], v_rom_mac[5] );
> > > -			printf ("\t\"ethaddr\" value: "
> > > -				"%02X:%02X:%02X:%02X:%02X:%02X\n",
> > > -				v_env_mac[0], v_env_mac[1],
> > > -				v_env_mac[2], v_env_mac[3],
> > > -				v_env_mac[4], v_env_mac[5]) ;
> > > -			debug ("### Set MAC addr from environment\n");
> > > -		}
> > > -	}
> >
> > This affects other drivers as well (cs8900 for example, in another
> > patch).
> >
> > Can you please explain what your rationale was for removing this code?
>
> i thought the u-boot design was to not touch hardware it shouldnt be.  if the 

Um...yes - but what has thjis to do with that here?

> canonical location for eth addr is the env, then what is in the attached 
> eeproms is irrelevant as it wont be used if the env is available.  it's also 

But printing a warning message is probably still a pretty good  idea,
especially  since  you  don't know what the driver in some OS will do
that might be bootet later - and not only users,  even  some  servers
and routers may get confused when a device changes MAC addresses like
that.

> not specific to either of these drivers, so if we did choose to make this 
> behavior optional via some define, it would make more sense to do it in the 
> common eth code rather than copying & pasting it everywhere.

Agreed.  Do you think you have time and resources to craft such a patch?

Thanks in advance.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
I don't know if it's what you want, but it's what you get.  :-)
                      - Larry Wall in <10502 at jpl-devvax.JPL.NASA.GOV>