[U-Boot-Users] [PATCH v2 4/7] add SMSC LAN9x1x Network driver

[Peter Pearse <peter.pearse@arm.com>]

> -----Original Message-----
> From: Ben Warren [mailto:biggerbadderben at gmail.com] 
> Sent: 26 March 2008 20:08
> To: Guennadi Liakhovetski
> Cc: u-boot-users at lists.sourceforge.net; Wolfgang Denk; Peter Pearse
> Subject: Re: [U-Boot-Users] [PATCH v2 4/7] add SMSC LAN9x1x 
> Network driver
> 
> Hi Guennadi,
> 
> Guennadi Liakhovetski wrote:
> > From: Sascha Hauer <s.hauer@pengutronix.de>
> >
> > This patch adds a driver for the following smsc network controllers:
> > LAN9115
> > LAN9116
> > LAN9117
> > LAN9215
> > LAN9216
> > LAN9217
> >
> >   
> How many of these have been tested, and on what platforms.  
> I'm asking because the code seems to assume a 32-bit 
> interface and these aren't all 32-bit chips.

Comments please Sascha.

---snip---

> > diff --git a/drivers/net/smc911x.c b/drivers/net/smc911x.c new file 
> > mode 100644 index 0000000..5830368
> > --- /dev/null
> > +++ b/drivers/net/smc911x.c

---snip---

> > +
> > +#ifdef CONFIG_DRIVER_SMC911X
> > +
> >   
> This should be moved to the Makefile. 

Agreed


---snip---

> >   
> Register and bitfield definitions should be in a header file.

Not these file specific ones.
Ben - where else would they be applicable? 

>  More generally, only register addresses and bitfields should 
> be defined.  

Using macros to encapsulate both address and 
> function is bad form, IMHO.

Agreed

> 
> I haven't even gotten into the functionality, because I think 
> there's a lot of work to be done just in coding style 

Ben - perhaps you could help by pointing out some more examples 

> before 
> we look at substance.

Regards

Peter