[U-Boot] [PATCH] arm: mxs: fix mac address of second interface

[Marek Vasut <marex@denx.de>]

On Monday, August 17, 2015 at 08:51:48 PM, Michael Heimpold wrote:
> Hi Marek,

Hi!

> Am Sonntag, 16. August 2015, 00:49:36 schrieb Marek Vasut:
> > On Sunday, August 16, 2015 at 12:16:21 AM, Michael Heimpold wrote:
> > > In the rare case that an overflow occurs, propagate it.
> > 
> > Hi!
> > 
> > > Signed-off-by: Michael Heimpold <mhei@heimpold.de>
> > > Cc: Stefano Babic <sbabic@denx.de>
> > > Cc: Marek Vasut <marex@denx.de>
> > > CC: Fabio Estevam <fabio.estevam@freescale.com>
> > > ---
> > > 
> > >  arch/arm/cpu/arm926ejs/mxs/mxs.c |   12 ++++++++++--
> > >  1 file changed, 10 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/arch/arm/cpu/arm926ejs/mxs/mxs.c
> > > b/arch/arm/cpu/arm926ejs/mxs/mxs.c index b1d8721..42057ad 100644
> > > --- a/arch/arm/cpu/arm926ejs/mxs/mxs.c
> > > +++ b/arch/arm/cpu/arm926ejs/mxs/mxs.c
> > > @@ -238,11 +238,19 @@ int cpu_eth_init(bd_t *bis)
> > > 
> > >  __weak void mx28_adjust_mac(int dev_id, unsigned char *mac)
> > >  {
> > > 
> > > +	uint32_t data;
> > > +
> > > 
> > >  	mac[0] = 0x00;
> > >  	mac[1] = 0x04; /* Use FSL vendor MAC address by default */
> > > 
> > > -	if (dev_id == 1) /* Let MAC1 be MAC0 + 1 by default */
> > > -		mac[5] += 1;
> > > +	if (dev_id == 1) { /* Let MAC1 be MAC0 + 1 by default */
> > > +		data = (mac[3] << 16) | (mac[4] << 8) | mac[5];
> > > +		data++;
> > > +
> > > +		mac[3] = (data >> 16) & 0xff;
> > > +		mac[4] = (data >> 8) & 0xff;
> > > +		mac[5] = data & 0xff;
> > > +	}
> > 
> > I'm not very fond of the added complexity. If an overflow happens,
> 
> At least for me, that would be the "algorithm of least surprise" and
> I don't feel that this is so complex but YMMV :-)
> 
> > the last nibble just becomes 0x00, which should be okayish. In case
> > you need some sort of special handling of the ethernet address, I'd
> > suggest to implement your own thing in board file.
> 
> If this is a NAK, then yes, I can handle this in every board file where
> needed because I've to adjust the vendor part anyway...

Let's hear from the others first.

Best regards,
Marek Vasut