In message 20040221003551.37064063.fryman@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