[U-Boot] [PATCH 1/3] net: Adds Fast Ethernet Controller driver for Armada100

[Marek Vasut]

On Monday, August 22, 2011 06:02:26 PM Mike Frysinger wrote:
> On Monday, August 22, 2011 01:11:57 Ajay Bhargav wrote:
> > +	writel((u32) darmdfec->htpr, &regs->htpr);
> 
> do you really need to cast it yourself ?  seems to show up a lot in this
> file.
> 

[...]

> make the reg base a parameter to armada100_fec_initialize()
> 
> > +	darmdfec = malloc(sizeof(struct armdfec_device));
> 
> sizeof(*darmdfec)

Why are you against sizeof(struct ...) ?
> 
> > +	while (!eth_getenv_enetaddr(s, dev->enetaddr)) {
> > +		/* Generate Private MAC addr if not set */
> > +		dev->enetaddr[0] = 0x00;
> > +		dev->enetaddr[1] = 0x50;
> > +		dev->enetaddr[2] = 0x43;
> > +#if defined(CONFIG_SKIP_LOCAL_MAC_RANDOMIZATION)
> > +		/* Generate fixed lower MAC half */
> > +		dev->enetaddr[3] = 0x11;
> > +		dev->enetaddr[4] = 0x22;
> > +		dev->enetaddr[5] = 0x33;
> > +#else
> > +		/* Generate random lower MAC half */
> > +		dev->enetaddr[3] = get_random_byte((u8)read_timer());
> > +		dev->enetaddr[4] = get_random_byte(dev->enetaddr[3]);
> > +		dev->enetaddr[5] = get_random_byte(dev->enetaddr[4]);
> > +#endif
> > +		eth_setenv_enetaddr(s, dev->enetaddr);
> > +	}
> 
> NAK on this whole thing.  initialize dev->write_hwaddr and that is the only
> thing you should do.  the higher eth layers will take care of calling that
> as necessary.

ACK on the comment here.