[U-Boot] [PATCH 2/2] mvgbe: fix network device indices

[Prafulla Wadaskar]

> -----Original Message-----
> From: Mike Frysinger [mailto:vapier at gentoo.org]
> Sent: Saturday, November 05, 2011 4:37 AM
> To: Prafulla Wadaskar
> Cc: Michael Walle; u-boot at lists.denx.de; Wolfgang Denk
> Subject: Re: [PATCH 2/2] mvgbe: fix network device indices
> 
> On Friday 04 November 2011 02:29:24 Prafulla Wadaskar wrote:
> > Mike Frysinger:
> > > On Thursday 03 November 2011 19:02:26 Michael Walle wrote:
> > > > Am Donnerstag 03 November 2011, 19:10:57 schrieb Mike
> Frysinger:
> > > > > On Thursday 27 October 2011 17:31:36 Michael Walle
> wrote:
> > > > > > --- a/drivers/net/mvgbe.c
> > > > > > +++ b/drivers/net/mvgbe.c
> > > > > >
> > > > > > +		/* Extract the MAC address from the environment
> */
> > > > > > +		while (!eth_getenv_enetaddr_by_index("eth", dev-
> index,
> > > > > > +					dev->enetaddr)) {
> > > > > >
> > > > > >  			/* Generate Private MAC addr if not set */
> > > > > >  			dev->enetaddr[0] = 0x02;
> > > > > >  			dev->enetaddr[1] = 0x50;
> > > > >
> > > > > this is wrong.  net drivers should not be touching the
> env
> > > > > at all.  please fix your driver to not do that first.
> > > >
> > > > i guess this whole mac randomization/generation code
> belongs to board
> > > > specific files.
> > >
> > > correct
> >
> > We can move mac randomization code to the board specific
> files, but it will
> > be needed for each board and there will be code duplication.
> 
> there's two issues here.  (1) no net driver should touch the
> env.  this is why
> we have the dev->enetaddr field in the first place.  (2)
> drivers should be
> seeding dev->enetaddr with values from storage directly related
> to it.  so for
> parts that have dedicated EEPROM interfaces, reading the MAC
> out of that
> storage makes sense.  if no storage like that exists, then it
> is up to the
> board to figure out where to find the address.
> 
> that means this could should be moved to the boards file.
> 
> > How about supporting standalone mac randomization feature?
> 
> i think Wolfgang would be opposed to that.  mac randomization
> should not be
> the first line of defense.  your board is supposed to be
> managing this sanely.
> from the mvgbe code, it seems that this is not the case and
> these boards are a
> bit insane.

Hi Mike

I had posted feedback for your patch first and seen this email latter.

On some boards, using mvbge random mac generation is needed since there is no eeprom to hold this. Further practically it is not possible to hardcode mac address and recompile, nor it is suggested to have any mac/ip/server address definition in board config files.

Random mac address helps to resolve dhcp issues with more similar boards in the same network in such cases and will be applicable for any board.

Your patch is clean to abstract out mac randomization, on the other hand you should not remove the support for other boards which are already using it.

May be some more patches on the top to move support in board/arch specific files are needed. (if framework can not be entertained)

Regards..
Prafulla . .