Re: ipw driver

[Andrew Atrens <atrens@xxxxxxxxxxxxxxxxxx>]

Johannes Hofmann wrote:

> Now that the iwi driver has been commited, I think it is time to try to
> get in the ipw driver too. A diff to current cvs can be found at
> http://www.ecademix.com/JohannesHofmann/ipw.diff.gz
> It is currently not using tokens to do the locking, because I do not yet
> understand the details. So it might need some more work before inclusion.
> However it does work for me for quite a while now.
> 
>   Johannes

Johannes,

Looks good. A few things you may still wish to do -

In *_cache_firmware you may wish to make your bus_dmamem_alloc()'s do
BUS_DMA_WAITOK instead of *_NOWAIT. NO_WAIT will fail in low memory
conditions where WAIKOK will succeed. Since you're not in a big hurry
to cache the firmware WAITOK seems more appropriate and is more robust.

The RAWBPF stuff has been implemented, so you should get rid of the
ifdef IPW_RAWBPF's and re-work the bpf_mtap2() to use the
DragonFly api's instead.

You're missing a few dma_sync() operations. This may cause the ipw
firmware to fail under load. Since there are many similarities
between ipw and iwi you might want to look for places where iwi is
dma_synching and ipw does not (yet). :)

There's a couple of #if 0 blocks that look like try-it-out-maybe sort
of code that should be cleaned up (ie. leave it in (remove the #if 0),
or take it out (remove the code block).

Instead of doing the splimp()/splx()'s directly in the code, you may
wish to consider using a IPW_LOCK/IPW_UNLOCK macros - put these in you
if_ipwvar.h header file. This seems to be the most common approach in
the codebase currently.

You may wish to merge in ALTQ support - it's not that hard to do and
is pretty cool once you've got it working.

You may wish to turn on WARNS?=6 in your makefiles to ensure the code
is squeaky clean - there's currently an effort underway to make the
entire codebase WARNS?=6 clean, so might as well start the ipw driver
off on the right foot. :) :)

Andrew.