[PATCH 4/4] ath9k: Add new Atheros IEEE 802.11n driver [reference to URL]

[PATCH 4/4] ath9k: Add new Atheros IEEE 802.11n driver [reference to URL]

[Luis R. Rodriguez]

This adds the new mac80211 11n ath9k Atheros driver. Only STA support
is currently enabled and tested.

Signed-off-by: Senthil Balasubramanian <senthilkumar@atheros.com>
Signed-off-by: Felix Fietkau <nbd@openwrt.org>
Signed-off-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Jack Howarth <howarth@bromo.msbb.uc.edu>
Signed-off-by: Jouni Malinen <jouni.malinen@atheros.com>
Signed-off-by: Sujith Manoharan <Sujith.Manoharan@atheros.com>
Signed-off-by: Luis R. Rodriguez <lrodriguez@atheros.com>
Signed-off-by: Pavel Roskin <proski@gnu.org>
Signed-off-by: Vasanthakumar Thiagarajan <vasanth@atheros.com>

This patch is about 1MB so you can download it from:

http://www.kernel.org/pub/linux/kernel/people/mcgrof/patches/ath9k/add-ath9k-02/0004-ath9k-Add-new-Atheros-IEEE-802.11n-driver.patch

  Luis

Re: [PATCH 4/4] ath9k: Add new Atheros IEEE 802.11n driver [reference to URL]

[Johannes Berg]

[-- Attachment #1: Type: text/plain, Size: 1734 bytes --]

On Mon, 2008-08-04 at 00:30 -0700, Luis R. Rodriguez wrote:
> This adds the new mac80211 11n ath9k Atheros driver. Only STA support
> is currently enabled and tested.

Couple of comments:

ath_softc is _huge_, and the spinlocks in it are at the worst possible
place. Placing them together with the data they protect should be
better.


There's talk about sysctls that do not exist.


+/* Until mac80211 includes these fields */

please just post a patch doing so.


You haven't explained ath_scan_end et al. yet.


+               /* For HT capable stations, we save tidno for later use.
+                * We also override seqno set by upper layer with the
one
+                * in tx aggregation state.
+                *
+                * First, the fragmentation stat is determined.
+                * If fragmentation is on, the sequence number is
+                * not overridden, since it has been
+                * incremented by the fragmentation routine.

do you really still need to override the seqno? Did I make a mistake? Or
is this just older code? Or is it necessary because the aggregation
callback wants the seqno? In that case I can preassign it there to the
right thing...


+        * Calculate duration.  This logically belongs in the 802.11
+        * layer but it lacks sufficient information to calculate it.

what information does it lack? It _does_ in fact calculate the duration.
Something wrong there with aggregation? Please realise that you can, in
fact, change the upper layer.


+#ifdef USE_LEGACY_HAL

eh?


I'd appreciate somebody else reviewing the driver from a stack
perspective too, I don't have all that much time right now.

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]