[U-Boot] [PATCH v2 1/2] da850evm: add support to read mac address from SPI flash

[Hadli, Manjunath]

Mike,

On Sat, Feb 11, 2012 at 03:19:24, Mike Frysinger wrote:
> On Friday 10 February 2012 01:22:24 Manjunath Hadli wrote:
> > --- a/board/davinci/da8xxevm/da850evm.c
> > +++ b/board/davinci/da8xxevm/da850evm.c
> > 
> > +#define CFG_MAC_ADDR_SPI_BUS	0
> > +#define CFG_MAC_ADDR_SPI_CS	0
> > +#define CFG_MAC_ADDR_SPI_MAX_HZ	CONFIG_SF_DEFAULT_SPEED
> > +#define CFG_MAC_ADDR_SPI_MODE	SPI_MODE_3
> 
> these defines get used only once below, so doesn't seem terribly useful to have as defines.  up to you.
I would like to have these :), If you still insist I'll get rid of these.
> 
> >  int misc_init_r(void)
> >  {
> >  	dspwake();
> > +
> > +#ifdef CONFIG_MAC_ADDR_IN_SPIFLASH
> > +	uchar env_enetaddr[6];
> > +	int enetaddr_found;
> > +	int spi_mac_read;
> > +	uchar buff[6];
> > +
> > +	enetaddr_found = eth_getenv_enetaddr("ethaddr", env_enetaddr);
> > +	spi_mac_read = get_mac_addr(buff);
> 
> you always read the SPI flash even if the env is available.  that sounds like a waste of time to me.  i'd expect the logic to simply be:
> 
> 	if (!eth_getenv_enetaddr("ethaddr", env_enetaddr)) {
> 		if (get_mac_addr(buff) == 0)
> 			eth_setenv_enetaddr("ethaddr", buff);
> 	}
> 
> no need to warn in misc_init_r() since your get_mac_addr() already issues a warning -mike
> 
My earlier patch series did the same thing, but Wolfgang suggested,
"If we have multiple storage for the MAC address, we should always
raise a warning if these contain different information."
http://www.mail-archive.com/u-boot@lists.denx.de/msg76666.html
the above link should sort out things clearer.

Regards,
--Manju