Hi Peter, Peter Pearse wrote: >> -----Original Message----- >> From: Ben Warren [mailto:[EMAIL PROTECTED] >> Sent: 26 March 2008 20:08 >> To: Guennadi Liakhovetski >> Cc: [email protected]; 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 <[EMAIL PROTECTED]> >>> >>> 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? > > Well, I can't come up with a better answer than 'precedent', so I guess it's OK to keep the #defines in the C code. >> 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 > > By coding style I mean the nasty macros, not stuff like brackets and whitespace. If the code was more readable and addressed the bus width differences between chips, this would probably go in quickly. I'll have another look to see if I can be more helpful rather than curmudgeonly.
regards, Ben ------------------------------------------------------------------------- Check out the new SourceForge.net Marketplace. It's the best place to buy or sell services for just about anything Open Source. http://ad.doubleclick.net/clk;164216239;13503038;w?http://sf.net/marketplace _______________________________________________ U-Boot-Users mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/u-boot-users
