Re: [PATCH] macsonic/jazzsonic drivers update

[Jeff Garzik <jgarzik@pobox.com>]

Finn Thain wrote:
> Here is the latest sonic driver update, which I've been rattling on about 
> for some time now. There are several reasons for this patch,
> 
> - Adopt the DMA API (jazzsonic, macsonic & core driver)
> 
> - Adopt the driver model (macsonic)
> 
>   This part was cribbed from jazzsonic. As a consequence, macsonic once 
>   again works as a module. Driver model is also used by the DMA calls.
> 
> - Support 16 bit cards (macsonic & core driver, also affects jazzsonic)
> 
>   This code was adapted from the mac68k linux 2.2 kernel, where it has 
>   languished for a long time.
> 
> - Support more mac cards (macsonic)
> 
>   Also from mac68k repo.
> 
> - Zero-copy buffer handling (core driver)
> 
>   Provides a nice performance improvement. The new algorithm incidentally 
>   helped to replace the old Jazz DMA code.
> 
> A nice consequence of this patch is that one can now diff jazzsonic.c and 
> macsonic.c and get a comprehensible result. Maybe that will make them 
> easier to maintain.
> 
> The patch has been tested on a variety of macs (several 32-bit quadra 
> built-in NICs, a 16-bit LC PDS NIC and a 16-bit comm-slot NIC), but only 
> compile tested for MIPS Jazz.
> 
> The DMA API has been implemented for m68k by Roman Zippel, and his patch 
> is now required for macsonic to function. There is also a patch available 
> which implements that API for Jazz machines (by Thomas I believe).
> 
> It would be nice if someone could test this on a little endian Jazz 
> machine.
> 
> Comments and criticism welcomed.
> 
> -f
> 
> 
> Signed-off-by: Finn Thain <fthain@telegraphics.com.au>

Patch looks OK to me.  Comments:

1) Either Geert or Ralf can merge this, with my ACK.

2) Would be nice to get it tested on the machines you list as untested.

3) [possible problem in driver, not your changes] I wonder if 
IRQ_HANDLED is ever returned for shared interrupts?  I don't know enough 
about the platform interrupt architecture to answer this question.

4) Remove casts to/from void.  This is especially noticable in all the 
casts of the netdev_priv() return value.

5) If it doesn't cause too much patch noise, consider using enums rather 
than #defines, for numeric constants.  This gives the compiler more type 
information and makes the symbols visible in a debugger.  This is a 
-maintainer preference- issue overall, so don't sweat it if you disagree.