Hi Peter,
Peter Pearse wrote:
-----Original Message----- From: Ben Warren [mailto:biggerbadderben@gmail.com] Sent: 26 March 2008 20:08 To: Guennadi Liakhovetski Cc: u-boot-users@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?
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