netdev
[Top] [All Lists]

Re: Centrino wireless review

To: James Ketrenos <jketreno@xxxxxxxxxxxxxxxxxx>
Subject: Re: Centrino wireless review
From: Jeff Garzik <jgarzik@xxxxxxxxx>
Date: Tue, 09 Mar 2004 21:47:59 -0500
Cc: Netdev <netdev@xxxxxxxxxxx>, Linus Torvalds <torvalds@xxxxxxxx>
In-reply-to: <404E795A.9050906@xxxxxxxxxxxxxxxxxx>
References: <404E3DB8.1000000@xxxxxxxxx> <404E795A.9050906@xxxxxxxxxxxxxxxxxx>
Sender: netdev-bounce@xxxxxxxxxxx
User-agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.4) Gecko/20030703
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




<Prev in Thread] Current Thread [Next in Thread>