2007-10-11 19:39:30

by Luis R. Rodriguez

[permalink] [raw]
Subject: [PATCH 0/4] ath5k: promiscuous bug, multicast and some docs

This patch series adds a few things which have been sitting in my
queue. Nick has better fixes for two of my previous patches so
he'll be sending that. In this series I also added some new docs
for some hw interrupts.

Luis


2007-10-12 14:10:06

by Luis R. Rodriguez

[permalink] [raw]
Subject: Re: [PATCH 0/4] ath5k: promiscuous bug, multicast and some docs

On 10/12/07, Jiri Slaby <[email protected]> wrote:
> On 10/11/07, Luis R. Rodriguez <[email protected]> wrote:
> > This patch series adds a few things which have been sitting in my
> > queue. Nick has better fixes for two of my previous patches so
> > he'll be sending that. In this series I also added some new docs
> > for some hw interrupts.
>
> Hi,
>
> please check your patches with scripts/checkpatch.pl, patch 2, 3, 4
> and the followed fix doesn't comply with codingstyle.

Those pesky leading spaces.. thanks, I'll resend and add the new patch
to it with Nick's new comments on IRQs as well.

Luis

2007-10-11 19:49:44

by Luis R. Rodriguez

[permalink] [raw]
Subject: [PATCH 3/4] Don't read AR5K_RAC_PISR on AR5210, document ath5k_int

AR5210 does not have AR5K_RAC_PISR so do not read it. Also lets start
trying to document all hardware interrupts.

Changes-licensed-under: ISC
Signed-off-by: Luis R. Rodriguez <[email protected]>
---
drivers/net/wireless/ath5k/ath5k.h | 61 ++++++++++++++++++++++++++++++------
drivers/net/wireless/ath5k/hw.c | 16 ++++++---
2 files changed, 61 insertions(+), 16 deletions(-)

diff --git a/drivers/net/wireless/ath5k/ath5k.h b/drivers/net/wireless/ath5k/ath5k.h
index c1d3c9d..61c717f 100644
--- a/drivers/net/wireless/ath5k/ath5k.h
+++ b/drivers/net/wireless/ath5k/ath5k.h
@@ -720,18 +720,61 @@ enum ath5k_ant_setting {
* HAL interrupt abstraction
*/

-/*
+/**
+ * enum ath5k_int - Hardware interrupt masks helpers
+ *
+ * @AR5K_INT_RX: mask to identify received frame interrupts, of type
+ * AR5K_ISR_RXOK or AR5K_ISR_RXERR
+ * @AR5K_INT_RXDESC: ??
+ * @AR5K_INT_RXNOFRM: ??
+ * @AR5K_INT_RXEOL: received End Of List for VEOL (Virtual End Of List). The
+ * Queue Control Unit (QCU) signals an EOL interrupt only if a descriptor's
+ * LinkPtr is NULL. For more details, refer to:
+ * http://www.freepatentsonline.com/20030225739.html
+ * @AR5K_INT_RXORN: indicates a hardware reset is required
+ * @AR5K_INT_TX: mask to identify received frame interrupts, of type
+ * AR5K_ISR_TXOK or AR5K_ISR_TXERR
+ * @AR5K_INT_TXDESC: ??
+ * @AR5K_INT_TXURN: received when we should increase the TX trigger threshold
+ * We currently do increments on interrupt by
+ * (AR5K_TUNE_MAX_TX_FIFO_THRES - current_trigger_level) / 2
+ * @AR5K_INT_MIB: Indicates the Management Information Base counters should be
+ * checked. We should do this with ath5k_hw_update_mib_counters() but
+ * it seems we should also then do some noise immunity work.
+ * @AR5K_INT_RXPHY: ??
+ * @AR5K_INT_RXKCM: ??
+ * @AR5K_INT_SWBA: SoftWare Beacon Alert - indicates its time to send a
+ * beacon that must be handled in software. The alternative is if you
+ * have VEOL support, in that case you let the hardware deal with things.
+ * @AR5K_INT_BMISS: If in STA mode this indicates we have stopped seeing
+ * beacons from the AP have associated with, we should probably try to
+ * reassociate. When in IBSS mode this might mean we have not received
+ * any beacons from any local stations. Note that every station in an
+ * IBSS schedules to send beacons at the Target Beacon Transmission Time
+ * (TBTT) with a random backoff.
+ * @AR5K_INT_BNR: Beacon Not Ready interrupt - ??
+ * @AR5K_INT_GPIO: ??
+ * @AR5K_INT_FATAL: Fatal errors were encountered, typically caused by DMA
+ * errors. These types of errors we can enable seem to be of type
+ * AR5K_SIMR2_MCABT, AR5K_SIMR2_SSERR and AR5K_SIMR2_DPERR.
+ * @AR5K_INT_GLOBAL: Seems to be used to clear and set the IER
+ * @AR5K_INT_NOCARD: signals the card has been removed
+ * @AR5K_INT_COMMON: common interrupts shared amogst MACs with the same
+ * bit value
+ *
* These are mapped to take advantage of some common bits
- * between the MAC chips, to be able to set intr properties
- * easier. Some of them are not used yet inside OpenHAL.
+ * between the MACs, to be able to set intr properties
+ * easier. Some of them are not used yet inside hw.c. Most map
+ * to the respective hw interrupt value as they are common amogst different
+ * MACs.
*/
enum ath5k_int {
- AR5K_INT_RX = 0x00000001,
+ AR5K_INT_RX = 0x00000001, /* Not common */
AR5K_INT_RXDESC = 0x00000002,
AR5K_INT_RXNOFRM = 0x00000008,
AR5K_INT_RXEOL = 0x00000010,
AR5K_INT_RXORN = 0x00000020,
- AR5K_INT_TX = 0x00000040,
+ AR5K_INT_TX = 0x00000040, /* Not common */
AR5K_INT_TXDESC = 0x00000080,
AR5K_INT_TXURN = 0x00000800,
AR5K_INT_MIB = 0x00001000,
@@ -739,12 +782,11 @@ enum ath5k_int {
AR5K_INT_RXKCM = 0x00008000,
AR5K_INT_SWBA = 0x00010000,
AR5K_INT_BMISS = 0x00040000,
- AR5K_INT_BNR = 0x00100000,
+ AR5K_INT_BNR = 0x00100000, /* Not common */
AR5K_INT_GPIO = 0x01000000,
- AR5K_INT_FATAL = 0x40000000,
+ AR5K_INT_FATAL = 0x40000000, /* Not common */
AR5K_INT_GLOBAL = 0x80000000,

- /*A sum of all the common bits*/
AR5K_INT_COMMON = AR5K_INT_RXNOFRM
| AR5K_INT_RXDESC
| AR5K_INT_RXEOL
@@ -757,8 +799,7 @@ enum ath5k_int {
| AR5K_INT_SWBA
| AR5K_INT_BMISS
| AR5K_INT_GPIO,
- AR5K_INT_NOCARD = 0xffffffff /*Declare that the card
- has been removed*/
+ AR5K_INT_NOCARD = 0xffffffff
};

/*
diff --git a/drivers/net/wireless/ath5k/hw.c b/drivers/net/wireless/ath5k/hw.c
index 3fa36f5..58548ee 100644
--- a/drivers/net/wireless/ath5k/hw.c
+++ b/drivers/net/wireless/ath5k/hw.c
@@ -1471,11 +1471,13 @@ int ath5k_hw_get_isr(struct ath_hw *hal, enum ath5k_int *interrupt_mask)
return -ENODEV;
}
}
-
- /*
- * Read interrupt status from the Read-And-Clear shadow register
- */
- data = ath5k_hw_reg_read(hal, AR5K_RAC_PISR);
+ else {
+ /*
+ * Read interrupt status from the Read-And-Clear shadow register
+ * Note: PISR/SISR Not available on 5210
+ */
+ data = ath5k_hw_reg_read(hal, AR5K_RAC_PISR);
+ }

/*
* Get abstract interrupt mask (HAL-compatible)
@@ -1503,7 +1505,9 @@ int ath5k_hw_get_isr(struct ath_hw *hal, enum ath5k_int *interrupt_mask)

/*
* XXX: BMISS interrupts may occur after association.
- * I found this on 5210 code but it needs testing
+ * I found this on 5210 code but it needs testing. If this is
+ * true we should disable them before assoc and re-enable them
+ * after a successfull assoc + some jiffies.
*/
#if 0
interrupt_mask &= ~AR5K_INT_BMISS;
--
1.5.2.5


2007-10-11 19:50:28

by Luis R. Rodriguez

[permalink] [raw]
Subject: [PATCH 4/4] Add extensive documenation for the atheros bssid_mask

Add extensive documenation for the atheros bssid_mask.
Credit to David Kimdon for figuring this out. I am
just documenting it.

No need to check for ath5k_hw_hasbssidmask() as
ath5k_hw_set_bssid_mask() will do the check itself.

Also add link to Atheros patent 6677779 B1 about buffer
registers and control registers. Hope this helps.

Changes to base.c
Changes-licensed-under: 3-clause-BSD

Changes to hw.c, reg.h
Changes-licensed-under: ISC

Signed-off-by: Luis R. Rodriguez <[email protected]>
---
drivers/net/wireless/ath5k/base.c | 7 +--
drivers/net/wireless/ath5k/hw.c | 96 +++++++++++++++++++++++++++++++++++-
drivers/net/wireless/ath5k/reg.h | 17 ++++--
3 files changed, 107 insertions(+), 13 deletions(-)

diff --git a/drivers/net/wireless/ath5k/base.c b/drivers/net/wireless/ath5k/base.c
index f474660..0997577 100644
--- a/drivers/net/wireless/ath5k/base.c
+++ b/drivers/net/wireless/ath5k/base.c
@@ -2242,10 +2242,9 @@ static int ath_attach(struct pci_dev *pdev, struct ieee80211_hw *hw)

ath5k_hw_get_lladdr(ah, mac);
SET_IEEE80211_PERM_ADDR(hw, mac);
- if (ath5k_hw_hasbssidmask(ah)) {
- memset(sc->bssidmask, 0xff, ETH_ALEN);
- ath5k_hw_set_bssid_mask(ah, sc->bssidmask);
- }
+ /* All MAC address bits matter for ACKs */
+ memset(sc->bssidmask, 0xff, ETH_ALEN);
+ ath5k_hw_set_bssid_mask(sc->ah, sc->bssidmask);

ret = ieee80211_register_hw(hw);
if (ret) {
diff --git a/drivers/net/wireless/ath5k/hw.c b/drivers/net/wireless/ath5k/hw.c
index 58548ee..00ff6c3 100644
--- a/drivers/net/wireless/ath5k/hw.c
+++ b/drivers/net/wireless/ath5k/hw.c
@@ -2324,9 +2324,99 @@ void ath5k_hw_set_associd(struct ath_hw *hal, const u8 *bssid, u16 assoc_id)

ath5k_hw_enable_pspoll(hal, NULL, 0);
}
-
-/*
- * Set BSSID mask on 5212
+/**
+ * ath5k_hw_set_bssid_mask - set common bits we should listen to
+ *
+ * The bssid_mask is a utility used by AR5212 hardware to inform the hardware
+ * which bits of the interface's MAC address should be looked at when trying
+ * to decide which packets to ACK. In station mode every bit matters. In AP
+ * mode with a single BSS every bit matters as well. In AP mode with
+ * multiple BSSes not every bit matters.
+ *
+ * @hal: the &struct ath_hw
+ * @mask: the bssid_mask, a u8 array of size ETH_ALEN
+ *
+ * Note that this is a simple filter and *does* not filter out all
+ * relevant frames. Some non-relevant frames will get through, probability
+ * jocks are welcomed to compute.
+ *
+ * When handling multiple BSSes (or VAPs) you can get the BSSID mask by
+ * computing the set of:
+ *
+ * ~ ( MAC XOR BSSID )
+ *
+ * When you do this you are essentially computing the common bits. Later it
+ * is assumed the harware will "and" (&) the BSSID mask with the MAC address
+ * to obtain the relevant bits which should match on the destination frame.
+ *
+ * Simple example: on your card you have have two BSSes you have created with
+ * BSSID-01 and BSSID-02. Lets assume BSSID-01 will not use the MAC address.
+ * There is another BSSID-03 but you are not part of it. For simplicity's sake,
+ * assuming only 4 bits for a mac address and for BSSIDs you can then have:
+ *
+ * \
+ * MAC: 0001 |
+ * BSSID-01: 0100 | --> Belongs to us
+ * BSSID-02: 1001 |
+ * /
+ * -------------------
+ * BSSID-03: 0110 | --> External
+ * -------------------
+ *
+ * Our bssid_mask would then be:
+ *
+ * On loop iteration for BSSID-01:
+ * ~(0001 ^ 0100) -> ~(0101)
+ * -> 1010
+ * bssid_mask = 1010
+ *
+ * On loop iteration for BSSID-02:
+ * bssid_mask &= ~(0001 ^ 1001)
+ * bssid_mask = (1010) & ~(0001 ^ 1001)
+ * bssid_mask = (1010) & ~(1001)
+ * bssid_mask = (1010) & (0110)
+ * bssid_mask = 0010
+ *
+ * A bssid_mask of 0010 means "only pay attention to the second least
+ * significant bit". This is because its the only bit common
+ * amongst the MAC and all BSSIDs we support. To findout what the real
+ * common bit is we can simply "&" the bssid_mask now with any BSSID we have
+ * or our MAC address (we assume the hardware uses the MAC address).
+ *
+ * Now, suppose there's an incoming frame for BSSID-03:
+ *
+ * IFRAME-01: 0110
+ *
+ * An easy eye-inspeciton of this already should tell you that this frame
+ * will not pass our check. This is beacuse the bssid_mask tells the
+ * hardware to only look at the second least significant bit and the
+ * common bit amongst the MAC and BSSIDs is 0, this frame has the 2nd LSB
+ * as 1, which does not match 0.
+ *
+ * So with IFRAME-01 we *assume* the hardware will do:
+ *
+ * allow = (IFRAME-01 & bssid_mask) == (bssid_mask & MAC) ? 1 : 0;
+ * --> allow = (0110 & 0010) == (0010 & 0001) ? 1 : 0;
+ * --> allow = (0010) == 0000 ? 1 : 0;
+ * --> allow = 0
+ *
+ * Lets now test a frame that should work:
+ *
+ * IFRAME-02: 0001 (we should allow)
+ *
+ * allow = (0001 & 1010) == 1010
+ *
+ * allow = (IFRAME-02 & bssid_mask) == (bssid_mask & MAC) ? 1 : 0;
+ * --> allow = (0001 & 0010) == (0010 & 0001) ? 1 :0;
+ * --> allow = (0010) == (0010)
+ * --> allow = 1
+ *
+ * Other examples:
+ *
+ * IFRAME-03: 0100 --> allowed
+ * IFRAME-04: 1001 --> allowed
+ * IFRAME-05: 1101 --> allowed but its not for us!!!
+ *
*/
int ath5k_hw_set_bssid_mask(struct ath_hw *hal, const u8 *mask)
{
diff --git a/drivers/net/wireless/ath5k/reg.h b/drivers/net/wireless/ath5k/reg.h
index 0a7b312..1537517 100644
--- a/drivers/net/wireless/ath5k/reg.h
+++ b/drivers/net/wireless/ath5k/reg.h
@@ -1252,10 +1252,13 @@
#define AR5K_RX_FILTER_RADARERR_5212 0x00000200 /* Don't filter phy radar errors [5212+] */
#define AR5K_RX_FILTER_PHYERR_5211 0x00000040 /* [5211] */
#define AR5K_RX_FILTER_RADARERR_5211 0x00000080 /* [5211] */
-#define AR5K_RX_FILTER_PHYERR (ah->ah_version == AR5K_AR5211 ? \
- AR5K_RX_FILTER_PHYERR_5211 : AR5K_RX_FILTER_PHYERR_5212)
-#define AR5K_RX_FILTER_RADARERR (ah->ah_version == AR5K_AR5211 ? \
- AR5K_RX_FILTER_RADARERR_5211 : AR5K_RX_FILTER_RADARERR_5212)
+#define AR5K_RX_FILTER_PHYERR \
+ ((ah->ah_version == AR5K_AR5211 ? \
+ AR5K_RX_FILTER_PHYERR_5211 : AR5K_RX_FILTER_PHYERR_5212))
+#define AR5K_RX_FILTER_RADARERR \
+ ((ah->ah_version == AR5K_AR5211 ? \
+ AR5K_RX_FILTER_RADARERR_5211 : AR5K_RX_FILTER_RADARERR_5212))
+
/*
* Multicast filter register (lower 32 bits)
*/
@@ -1755,8 +1758,10 @@
* packet, i have no idea. So i'll name them BUFFER_CONTROL_X registers
* for now. It's interesting that they are also used for some other operations.
*
- * Also check out ath5k_hw.h and U.S. Patent 6677779 B1 (about buffer
- * registers and control registers)
+ * Also check out hw.h and U.S. Patent 6677779 B1 (about buffer
+ * registers and control registers):
+ *
+ * http://www.google.com/patents?id=qNURAAAAEBAJ
*/

#define AR5K_RF_BUFFER 0x989c
--
1.5.2.5


2007-10-12 14:06:28

by Luis R. Rodriguez

[permalink] [raw]
Subject: Re: [PATCH 3/4] Don't read AR5K_RAC_PISR on AR5210, document ath5k_int

On 10/11/07, Nick Kossifidis <[email protected]> wrote:
> I should have added something inside ath5kreg.h, sry for that...
>
> (?) = not sure
>
> > + * @AR5K_INT_RXDESC: ??
> Request RX descriptor/Read RX descriptor (?)
>
> > + * @AR5K_INT_RXNOFRM: ??
> No frame received (?)
>
> > + * @AR5K_INT_RXEOL: received End Of List for VEOL (Virtual End Of List). The
> > + * Queue Control Unit (QCU) signals an EOL interrupt only if a descriptor's
> > + * LinkPtr is NULL. For more details, refer to:
> > + * http://www.freepatentsonline.com/20030225739.html
>
> > + * @AR5K_INT_RXORN: indicates a hardware reset is required
> Rx overrun is not always fatal, on some chips we can continue
> operation without reseting the card, that's why int_fatal is not
> common for all chips ;-)
>
> > + * @AR5K_INT_TX: mask to identify received frame interrupts, of type
> > + * AR5K_ISR_TXOK or AR5K_ISR_TXERR
> > + * @AR5K_INT_TXDESC: ??
> Request TX descriptor/Read TX status descriptor (?)
>
> > + * @AR5K_INT_TXURN: received when we should increase the TX trigger threshold
> > + * We currently do increments on interrupt by
> > + * (AR5K_TUNE_MAX_TX_FIFO_THRES - current_trigger_level) / 2
> > + * @AR5K_INT_MIB: Indicates the Management Information Base counters should be
> > + * checked. We should do this with ath5k_hw_update_mib_counters() but
> > + * it seems we should also then do some noise immunity work.
>
> > + * @AR5K_INT_RXPHY: ??
> RX PHY Error
>
> > + * @AR5K_INT_RXKCM: ??
> > + * @AR5K_INT_SWBA: SoftWare Beacon Alert - indicates its time to send a
> > + * beacon that must be handled in software. The alternative is if you
> > + * have VEOL support, in that case you let the hardware deal with things.
> > + * @AR5K_INT_BMISS: If in STA mode this indicates we have stopped seeing
> > + * beacons from the AP have associated with, we should probably try to
> > + * reassociate. When in IBSS mode this might mean we have not received
> > + * any beacons from any local stations. Note that every station in an
> > + * IBSS schedules to send beacons at the Target Beacon Transmission Time
> > + * (TBTT) with a random backoff.
> > + * @AR5K_INT_BNR: Beacon Not Ready interrupt - ??
>
> > + * @AR5K_INT_GPIO: ??
> GPIO interrupt used for RF Kill, we should handle this inside
> interrupt handler, until then i've disabled it because it results an
> interrupt storm in case RF Kill switch is off.
>
> > + * @AR5K_INT_NOCARD: signals the card has been removed
> There is no such interrupt, it's software stuff used in hal. We can
> always check for pending interrupts by reading that register (check
> out is_intr_pending) and get rid of this.

Since I'm resending, I've added these new comments. However I'm
leaving @AR5K_INT_NOCARD for now as it does seem to be read in hw:



if (hal->ah_version == AR5K_AR5210) {
data = ath5k_hw_reg_read(hal, AR5K_ISR);
if (unlikely(data == AR5K_INT_NOCARD)) {
*interrupt_mask = data;
return -ENODEV;
}
} else {
/*
* Read interrupt status from the Read-And-Clear shadow register
* Note: PISR/SISR Not available on 5210
*/
data = ath5k_hw_reg_read(hal, AR5K_RAC_PISR);
}

/*
* Get abstract interrupt mask (HAL-compatible)
*/
*interrupt_mask = (data & AR5K_INT_COMMON) & hal->ah_imr;

if (unlikely(data == AR5K_INT_NOCARD))
return -ENODEV;

2007-10-11 20:10:04

by Nick Kossifidis

[permalink] [raw]
Subject: Re: [PATCH 3/4] Don't read AR5K_RAC_PISR on AR5210, document ath5k_int

I should have added something inside ath5kreg.h, sry for that...

(?) = not sure

> + * @AR5K_INT_RXDESC: ??
Request RX descriptor/Read RX descriptor (?)

> + * @AR5K_INT_RXNOFRM: ??
No frame received (?)

> + * @AR5K_INT_RXEOL: received End Of List for VEOL (Virtual End Of List). The
> + * Queue Control Unit (QCU) signals an EOL interrupt only if a descriptor's
> + * LinkPtr is NULL. For more details, refer to:
> + * http://www.freepatentsonline.com/20030225739.html

> + * @AR5K_INT_RXORN: indicates a hardware reset is required
Rx overrun is not always fatal, on some chips we can continue
operation without reseting the card, that's why int_fatal is not
common for all chips ;-)

> + * @AR5K_INT_TX: mask to identify received frame interrupts, of type
> + * AR5K_ISR_TXOK or AR5K_ISR_TXERR
> + * @AR5K_INT_TXDESC: ??
Request TX descriptor/Read TX status descriptor (?)

> + * @AR5K_INT_TXURN: received when we should increase the TX trigger threshold
> + * We currently do increments on interrupt by
> + * (AR5K_TUNE_MAX_TX_FIFO_THRES - current_trigger_level) / 2
> + * @AR5K_INT_MIB: Indicates the Management Information Base counters should be
> + * checked. We should do this with ath5k_hw_update_mib_counters() but
> + * it seems we should also then do some noise immunity work.

> + * @AR5K_INT_RXPHY: ??
RX PHY Error

> + * @AR5K_INT_RXKCM: ??
> + * @AR5K_INT_SWBA: SoftWare Beacon Alert - indicates its time to send a
> + * beacon that must be handled in software. The alternative is if you
> + * have VEOL support, in that case you let the hardware deal with things.
> + * @AR5K_INT_BMISS: If in STA mode this indicates we have stopped seeing
> + * beacons from the AP have associated with, we should probably try to
> + * reassociate. When in IBSS mode this might mean we have not received
> + * any beacons from any local stations. Note that every station in an
> + * IBSS schedules to send beacons at the Target Beacon Transmission Time
> + * (TBTT) with a random backoff.
> + * @AR5K_INT_BNR: Beacon Not Ready interrupt - ??

> + * @AR5K_INT_GPIO: ??
GPIO interrupt used for RF Kill, we should handle this inside
interrupt handler, until then i've disabled it because it results an
interrupt storm in case RF Kill switch is off.

> + * @AR5K_INT_NOCARD: signals the card has been removed
There is no such interrupt, it's software stuff used in hal. We can
always check for pending interrupts by reading that register (check
out is_intr_pending) and get rid of this.


--
GPG ID: 0xD21DB2DB
As you read this post global entropy rises. Have Fun ;-)
Nick

2007-10-11 19:46:30

by Luis R. Rodriguez

[permalink] [raw]
Subject: [PATCH 2/4] Add proper support for multicast

There seems to be several ways to enable multicast. We choose right now
MadWifi's old implementation. We can later try ath5k_hw_set_mcast_filterindex()
as well.

ath5k_hw_get_rx_filter() may enable AR5K_RX_FILTER_RADARERR or
AR5K_RX_FILTER_PHYERR. We choose to respect only AR5K_RX_FILTER_PHYERR
for now because:

a. Most radars don't seem to work on 5GHz band now
b. Some have reported a lot of unnecessary noise is captured
when trying to filter for radar
c. When and if someone wants to work on DFS for ath5k later
this can be enabled then

Changes-licensed-under: 3-clause-BSD

Signed-off-by: Luis R. Rodriguez <[email protected]>

---
drivers/net/wireless/ath5k/base.c | 60 ++++++++++++++++++++++++++++++-------
1 files changed, 49 insertions(+), 11 deletions(-)

diff --git a/drivers/net/wireless/ath5k/base.c b/drivers/net/wireless/ath5k/base.c
index cda922e..f474660 100644
--- a/drivers/net/wireless/ath5k/base.c
+++ b/drivers/net/wireless/ath5k/base.c
@@ -1400,8 +1400,12 @@ unlock:
FIF_BCN_PRBRESP_PROMISC
/*
* o always accept unicast, broadcast, and multicast traffic
- * o maintain current state of phy error reception (the hal
- * may enable phy error frames for noise immunity work)
+ * o multicast traffic for all BSSIDs will be enabled if mac80211
+ * says it should be
+ * o maintain current state of phy ofdm or phy cck error reception.
+ * If the hardware detects any of these type of errors then
+ * ath5k_hw_get_rx_filter() will pass to us the respective
+ * hardware filters to be able to receive these type of frames.
* o probe request frames are accepted only when operating in
* hostap, adhoc, or monitor modes
* o enable promiscuous mode according to the interface state
@@ -1419,15 +1423,22 @@ static void ath_configure_filter(struct ieee80211_hw *hw,
{
struct ath_softc *sc = hw->priv;
struct ath_hw *ah = sc->ah;
- u32 rfilt;
+ u32 mfilt[2], val, rfilt;
+ u8 pos;
+ int i;
+
+ mfilt[0] = mfilt[1] = 0;

/* Only deal with supported flags */
changed_flags &= SUPPORTED_FIF_FLAGS;
*new_flags &= SUPPORTED_FIF_FLAGS;

- /* XXX: Start by enabling broadcasts and Unicast, move this later
- * to mac802111 and add a flag for these */
- rfilt = AR5K_RX_FILTER_UCAST | AR5K_RX_FILTER_BCAST;
+ /* If HW detects any phy or radar errors, leave those filters on.
+ * Also, always enable Unicast, Broadcasts and Multicast
+ * XXX: move unicast, bssid broadcasts and multicast to mac80211 */
+ rfilt = (ath5k_hw_get_rx_filter(ah) & (AR5K_RX_FILTER_PHYERR)) |
+ ( AR5K_RX_FILTER_UCAST | AR5K_RX_FILTER_BCAST |
+ AR5K_RX_FILTER_MCAST);

if (changed_flags & (FIF_PROMISC_IN_BSS | FIF_OTHER_BSS)) {
if (*new_flags & FIF_PROMISC_IN_BSS) {
@@ -1438,18 +1449,43 @@ static void ath_configure_filter(struct ieee80211_hw *hw,
__clear_bit(ATH_STAT_PROMISC, sc->status);
}

- if (*new_flags & FIF_ALLMULTI)
- rfilt |= AR5K_RX_FILTER_MCAST;
+ /* Note, AR5K_RX_FILTER_MCAST is already enabled */
+ if (*new_flags & FIF_ALLMULTI) {
+ mfilt[0] = mfilt[1] = ~0;
+ } else {
+ for (i = 0; i < mc_count; i++) {
+ if (!mclist)
+ break;
+ /* calculate XOR of eight 6-bit values */
+ val = LE_READ_4(mclist->dmi_addr + 0);
+ pos = (val >> 18) ^ (val >> 12) ^ (val >> 6) ^ val;
+ val = LE_READ_4(mclist->dmi_addr + 3);
+ pos ^= (val >> 18) ^ (val >> 12) ^ (val >> 6) ^ val;
+ pos &= 0x3f;
+ mfilt[pos / 32] |= (1 << (pos % 32));
+ /* XXX: we might be able to just do this instead,
+ * but not sure, needs testing, if we do use this we'd
+ * neet to inform below to not reset the mcast */
+ //ath5k_hw_set_mcast_filterindex(ah,
+ // mclist->dmi_addr[5]);
+ mclist = mclist->next;
+ }
+ }
+
/* This is the best we can do */
if (*new_flags & (FIF_FCSFAIL | FIF_PLCPFAIL))
rfilt |= AR5K_RX_FILTER_PHYERR;
+
/* FIF_BCN_PRBRESP_PROMISC really means to enable beacons
* and probes for any BSSID, this needs testing */
if (*new_flags & FIF_BCN_PRBRESP_PROMISC)
rfilt |= AR5K_RX_FILTER_BEACON | AR5K_RX_FILTER_PROBEREQ;
- /* FIF_CONTROL doc says that FIF_PROMISC_IN_BSS is not set we should
- * only pass on control frames for this station. This needs testing.
- * I believe right now this enables *all* control frames */
+
+ /* FIF_CONTROL doc says that if FIF_PROMISC_IN_BSS is not
+ * set we should only pass on control frames for this
+ * station. This needs testing. I believe right now this
+ * enables *all* control frames, which is OK.. but
+ * but we should see if we can improve on granularity */
if (*new_flags & FIF_CONTROL)
rfilt |= AR5K_RX_FILTER_CONTROL;

@@ -1470,6 +1506,8 @@ static void ath_configure_filter(struct ieee80211_hw *hw,
rfilt |= AR5K_RX_FILTER_BEACON;
}

+ /* Set multicast bits */
+ ath5k_hw_set_mcast_filter(ah, mfilt[0], mfilt[1]);
/* Set the cached hw filter flags, this will alter actually
* be set in HW */
sc->filter_flags = rfilt;
--
1.5.2.5


2007-10-11 19:41:10

by Luis R. Rodriguez

[permalink] [raw]
Subject: [PATCH 1/4] ath5k: Fix a bug which pushed us to enable promiscuous

Fix a bug which pushed us to enable the promiscuous filter
all the time during normal operation. The real problem was caused
becuase we were reseting the bssid to the broadcast during
ath5k_hw_reset(). We now cache the bssid value and set the
bssid again during resets. This gives the driver considerably
more stability.

Known issue: if your DHCP server doesn't ACK your IP via the broadcast but
instead sends it to the IP it gives you (Cisco seems to do this) you
may not get the reply. Work is in progress to figure this issue out.
This issue was present before this patch.

Changes to base.c
Changes-licensed-under: 3-clause-BSD

Changes to ath5k.h, hw.c
Changes-licensed-under: ISC

Signed-off-by: Luis R. Rodriguez <[email protected]>
---
drivers/net/wireless/ath5k/ath5k.h | 4 ++++
drivers/net/wireless/ath5k/base.c | 15 ++++++++-------
drivers/net/wireless/ath5k/hw.c | 23 ++++++++++-------------
3 files changed, 22 insertions(+), 20 deletions(-)

diff --git a/drivers/net/wireless/ath5k/ath5k.h b/drivers/net/wireless/ath5k/ath5k.h
index 9308916..c1d3c9d 100644
--- a/drivers/net/wireless/ath5k/ath5k.h
+++ b/drivers/net/wireless/ath5k/ath5k.h
@@ -877,6 +877,10 @@ struct ath_hw {
enum ieee80211_if_types ah_op_mode;
enum ath5k_power_mode ah_power_mode;
struct ieee80211_channel ah_current_channel;
+ /* Current BSSID we are trying to assoc to / creating, this
+ * comes from ieee80211_if_conf. This is passed by mac80211 on
+ * config_interface() */
+ u8 bssid[ETH_ALEN];
bool ah_turbo;
bool ah_calibration;
bool ah_running;
diff --git a/drivers/net/wireless/ath5k/base.c b/drivers/net/wireless/ath5k/base.c
index afcc7e1..cda922e 100644
--- a/drivers/net/wireless/ath5k/base.c
+++ b/drivers/net/wireless/ath5k/base.c
@@ -1368,6 +1368,7 @@ static int ath_config_interface(struct ieee80211_hw *hw, int if_id,
struct ieee80211_if_conf *conf)
{
struct ath_softc *sc = hw->priv;
+ struct ath_hw *ah = sc->ah;
int ret;

/* Set to a reasonable value. Note that this will
@@ -1378,8 +1379,13 @@ static int ath_config_interface(struct ieee80211_hw *hw, int if_id,
ret = -EIO;
goto unlock;
}
- if (conf->bssid)
- ath5k_hw_set_associd(sc->ah, conf->bssid, 0 /* FIXME: aid */);
+ if (conf->bssid) {
+ /* Cache for later use during resets */
+ memcpy(ah->bssid, conf->bssid, ETH_ALEN);
+ /* XXX: assoc id is set to 0 for now, mac80211 doesn't have
+ * a clean way of letting us retrieve this yet. */
+ ath5k_hw_set_associd(ah, ah->bssid, 0);
+ }
mutex_unlock(&sc->lock);

return ath_reset(hw);
@@ -1462,11 +1468,6 @@ static void ath_configure_filter(struct ieee80211_hw *hw,
if (sc->opmode == IEEE80211_IF_TYPE_STA ||
sc->opmode == IEEE80211_IF_TYPE_IBSS) {
rfilt |= AR5K_RX_FILTER_BEACON;
- /* Note: AR5212 requires AR5K_RX_FILTER_PROM to receive broadcasts,
- * perhaps the flags are off, for now to be safe we'll enable it for
- * STA and ADHOC until we have this properly mapped */
- if (ah->ah_version == AR5K_AR5212)
- rfilt |= AR5K_RX_FILTER_PROM;
}

/* Set the cached hw filter flags, this will alter actually
diff --git a/drivers/net/wireless/ath5k/hw.c b/drivers/net/wireless/ath5k/hw.c
index ae4c5b5..3fa36f5 100644
--- a/drivers/net/wireless/ath5k/hw.c
+++ b/drivers/net/wireless/ath5k/hw.c
@@ -309,15 +309,6 @@ struct ath_hw *ath5k_hw_attach(u16 device, u8 mac_version, void *sc,

hal->ah_phy = AR5K_PHY(0);

- /* Set MAC to bcast: ff:ff:ff:ff:ff:ff, this is using 'mac' as a
- * temporary variable for setting our BSSID. Right bellow we update
- * it with ath5k_hw_get_lladdr() */
- memset(mac, 0xff, ETH_ALEN);
- ath5k_hw_set_associd(hal, mac, 0);
-
- ath5k_hw_get_lladdr(hal, mac);
- ath5k_hw_set_opmode(hal);
-
#ifdef AR5K_DEBUG
ath5k_hw_dump_state(hal);
#endif
@@ -349,6 +340,10 @@ struct ath_hw *ath5k_hw_attach(u16 device, u8 mac_version, void *sc,
}

ath5k_hw_set_lladdr(hal, mac);
+ /* Set BSSID to bcast address: ff:ff:ff:ff:ff:ff for now */
+ memset(hal->bssid, 0xff, ETH_ALEN);
+ ath5k_hw_set_associd(hal, hal->bssid, 0);
+ ath5k_hw_set_opmode(hal);

ath5k_hw_set_rfgain_opt(hal);

@@ -546,7 +541,6 @@ int ath5k_hw_reset(struct ath_hw *hal, enum ieee80211_if_types op_mode,
const struct ath5k_rate_table *rt;
struct ath5k_eeprom_info *ee = &hal->ah_capabilities.cap_eeprom;
u32 data, noise_floor, s_seq, s_ant, s_led[3];
- u8 mac[ETH_ALEN];
unsigned int i, mode, freq, ee_mode, ant[2];
int ret;

@@ -864,8 +858,9 @@ int ath5k_hw_reset(struct ath_hw *hal, enum ieee80211_if_types op_mode,
/*
* Misc
*/
- memset(mac, 0xff, ETH_ALEN);
- ath5k_hw_set_associd(hal, mac, 0);
+ /* XXX: add hal->aid once mac80211 gives this to us */
+ ath5k_hw_set_associd(hal, hal->bssid, 0);
+
ath5k_hw_set_opmode(hal);
/*PISR/SISR Not available on 5210*/
if (hal->ah_version != AR5K_AR5210) {
@@ -1061,7 +1056,9 @@ static int ath5k_hw_nic_reset(struct ath_hw *hal, u32 val)
ret = ath5k_hw_register_timeout(hal, AR5K_RESET_CTL, mask, val, false);

/*
- * Reset configuration register (for hw byte-swap)
+ * Reset configuration register (for hw byte-swap). Note that this
+ * is only set for big endian. We do the necessary magic in
+ * AR5K_INIT_CFG.
*/
if ((val & AR5K_RESET_CTL_PCU) == 0)
ath5k_hw_reg_write(hal, AR5K_INIT_CFG, AR5K_CFG);
--
1.5.2.5


2007-10-11 20:15:50

by Luis R. Rodriguez

[permalink] [raw]
Subject: Re: [PATCH 3/4] Don't read AR5K_RAC_PISR on AR5210, document ath5k_int

On 10/11/07, Nick Kossifidis <[email protected]> wrote:
> I should have added something inside ath5kreg.h, sry for that...
>
> (?) = not sure
>
> > + * @AR5K_INT_RXDESC: ??
> Request RX descriptor/Read RX descriptor (?)
>
> > + * @AR5K_INT_RXNOFRM: ??
> No frame received (?)
>
> > + * @AR5K_INT_RXEOL: received End Of List for VEOL (Virtual End Of List). The
> > + * Queue Control Unit (QCU) signals an EOL interrupt only if a descriptor's
> > + * LinkPtr is NULL. For more details, refer to:
> > + * http://www.freepatentsonline.com/20030225739.html
>
> > + * @AR5K_INT_RXORN: indicates a hardware reset is required
> Rx overrun is not always fatal, on some chips we can continue
> operation without reseting the card, that's why int_fatal is not
> common for all chips ;-)
>
> > + * @AR5K_INT_TX: mask to identify received frame interrupts, of type
> > + * AR5K_ISR_TXOK or AR5K_ISR_TXERR
> > + * @AR5K_INT_TXDESC: ??
> Request TX descriptor/Read TX status descriptor (?)
>
> > + * @AR5K_INT_TXURN: received when we should increase the TX trigger threshold
> > + * We currently do increments on interrupt by
> > + * (AR5K_TUNE_MAX_TX_FIFO_THRES - current_trigger_level) / 2
> > + * @AR5K_INT_MIB: Indicates the Management Information Base counters should be
> > + * checked. We should do this with ath5k_hw_update_mib_counters() but
> > + * it seems we should also then do some noise immunity work.
>
> > + * @AR5K_INT_RXPHY: ??
> RX PHY Error
>
> > + * @AR5K_INT_RXKCM: ??
> > + * @AR5K_INT_SWBA: SoftWare Beacon Alert - indicates its time to send a
> > + * beacon that must be handled in software. The alternative is if you
> > + * have VEOL support, in that case you let the hardware deal with things.
> > + * @AR5K_INT_BMISS: If in STA mode this indicates we have stopped seeing
> > + * beacons from the AP have associated with, we should probably try to
> > + * reassociate. When in IBSS mode this might mean we have not received
> > + * any beacons from any local stations. Note that every station in an
> > + * IBSS schedules to send beacons at the Target Beacon Transmission Time
> > + * (TBTT) with a random backoff.
> > + * @AR5K_INT_BNR: Beacon Not Ready interrupt - ??
>
> > + * @AR5K_INT_GPIO: ??
> GPIO interrupt used for RF Kill, we should handle this inside
> interrupt handler, until then i've disabled it because it results an
> interrupt storm in case RF Kill switch is off.
>
> > + * @AR5K_INT_NOCARD: signals the card has been removed
> There is no such interrupt, it's software stuff used in hal. We can
> always check for pending interrupts by reading that register (check
> out is_intr_pending) and get rid of this.

Thanks, please send a follow up patch based on this one with your additions.

Luis

2007-10-12 07:22:31

by Jiri Slaby

[permalink] [raw]
Subject: Re: [PATCH 0/4] ath5k: promiscuous bug, multicast and some docs

On 10/11/07, Luis R. Rodriguez <[email protected]> wrote:
> This patch series adds a few things which have been sitting in my
> queue. Nick has better fixes for two of my previous patches so
> he'll be sending that. In this series I also added some new docs
> for some hw interrupts.

Hi,

please check your patches with scripts/checkpatch.pl, patch 2, 3, 4
and the followed fix doesn't comply with codingstyle.