Re: Centrino wireless review

[Jeff Garzik <jgarzik@pobox.com>]

James Ketrenos wrote:
> Jeff Garzik wrote:
> 
>>
>> Thanks much to you and Intel for publishing this!
> 
> 
> Wow.  Quick review turn around :)

I've been hoping a Centrino wireless driver would appear for a while now ;-)


>> 0) is the firmware blob truly a firmware blob, and not an x86 ELF 
>> blob?  In other words, will this firmware work on non-x86?
> 
> 
> The firmware data is just loaded from disk and handed off to the ipw2100 
> hardware; no linking with the kernel or running on the CPU.  To make 
> firmware loader work on an architecture w/ endian issues would require 
> some tweaks, but the firmware image itself wouldn't change (none of it 
> is executed by the CPU)
> 
> Or am I missing what you're asking?

Nope, that answered my question.  Thanks.


>> 24) I don't see that you need to list all 1001 (or so:)) PCI subsystem 
>> vendor ids...  just list the PCI vendor and device id.  (unless I'm 
>> missing something?)
> 
> 
> Apparently we have other cards that match the PCI vendor and device, but 
> are not IPW 2100[A]s, so the subsystem ids had to be added.

Sigh.


>> 26) I'm curious, what is the locking/exclusion on the ipw2100_wx_xxx() 
>> functions?
> 
> 
> Locking is lacking in a few places in the wx code.  Any data that needs 
> to be protected will end up being wrapped with the priv->low_lock 
> spinlock.  Is there something you saw in how we have it that might be 
> problematic?

In general I saw a lack of locking in the wx module, but I admit I could 
have missed these low_lock uses.

	jeff