[U-Boot-Users] patch for SMsC 91C96 ethernet driver

[Wolfgang Denk]

In message <20040221003551.37064063.fryman at cc.gatech.edu> you wrote:
> 
> the patch does a variety of things.  i'd appreciate feedback, corrections,
> reports of breakage, etc.  i'm also not sure that this is necessarily the
> "best" solution for some of the problems we had.
> 
>  - support little-endian mode to push MAC address correctly
> 
>  - removal of the hard-coded "attribute space" and replacement with a 
>    #define constant
> 
>  - clean up the hardware-init, smc-open, and smc-enable to more closely
>    follow the manual in how to set parameters
> 
>  - a couple of minor cleanups

No CHANGELOG entry.

Violation of coding standards (trailing white space; brace style; line length

@@ -263,14 +274,25 @@
 
 static int poll4int (byte mask, int timeout)
 {
-	int tmo = get_timer (0) + timeout * CFG_HZ;
-	int is_timeout = 0;
-	word old_bank = SMC_inw (LAN91C96_BANK_SELECT);
-
-	PRINTK2 ("Polling...\n");
-	SMC_SELECT_BANK (2);
...
+	int tmo;
+	int is_timeout;
+	word old_bank, tmp, last;
+
+	PRINTK("LAN91C96 IRQ Polling...\n");
+	
+	tmo = get_timer(0) + timeout * CFG_HZ;
+	is_timeout = 0;
+	old_bank = ioaddr[ LAN91C96_BANK_SELECT ];
+	
+	SMC_SELECT_BANK(2);
...


What makes you think your new code is better than the old one? I  see
no reason to change it...

...
-	PRINTK3 ("%s:smc_hardware_send_packet\n", SMC_DEV_NAME);
+	PRINTK("%s:smc_hardware_send_packet ,len=0x%x\n", SMC_DEV_NAME, packet_length);
...
-	printf ("Transmitting Packet\n");
-	print_packet (buf, length);
+	printf("Transmitting Packet\n");
+	print_packet(buf, length);
...

What makes you think your new formatting is better than the old  one?
Actually it's less readable.

 
-	status_test = SMC_inw (LAN91C96_BANK_SELECT);
...
+	tmp = ioaddr[ LAN91C96_BANK_SELECT ] ;

Also, I tend to  prefer  the  old  style  of  using  a  "input  word"
function/macro over your new magic array reference.


This patch needs some overhauling. Rejected for now.

Best regards,

Wolfgang Denk

-- 
Software Engineering:  Embedded and Realtime Systems,  Embedded Linux
Phone: (+49)-8142-4596-87  Fax: (+49)-8142-4596-88  Email: wd at denx.de
"Intelligence without character is a dangerous thing."   - G. Steinem