* [PATCH 0/5] ath5k: promiscuous bug, multicast, modes and docs
@ 2007-10-12 15:00 Luis R. Rodriguez
2007-10-12 15:03 ` [PATCH 1/5] ath5k: Fix a bug which pushed us to enable promiscuous Luis R. Rodriguez
0 siblings, 1 reply; 15+ messages in thread
From: Luis R. Rodriguez @ 2007-10-12 15:00 UTC (permalink / raw)
To: John Linville; +Cc: linux-wireless, Jiri Slaby, Nick Kossifidis
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 mode
fixes, and added Nick's new comments the hw interrupts docs.
I've taken the time to run checkpatch.pl now ;)
Luis
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 1/5] ath5k: Fix a bug which pushed us to enable promiscuous
2007-10-12 15:00 [PATCH 0/5] ath5k: promiscuous bug, multicast, modes and docs Luis R. Rodriguez
@ 2007-10-12 15:03 ` Luis R. Rodriguez
2007-10-12 15:04 ` [PATCH 2/5] Add proper support for multicast Luis R. Rodriguez
0 siblings, 1 reply; 15+ messages in thread
From: Luis R. Rodriguez @ 2007-10-12 15:03 UTC (permalink / raw)
To: John Linville; +Cc: linux-wireless, Jiri Slaby, Nick Kossifidis
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 <mcgrof@gmail.com>
---
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
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH 2/5] Add proper support for multicast
2007-10-12 15:03 ` [PATCH 1/5] ath5k: Fix a bug which pushed us to enable promiscuous Luis R. Rodriguez
@ 2007-10-12 15:04 ` Luis R. Rodriguez
2007-10-12 15:05 ` [PATCH 3/5] Don't read AR5K_RAC_PISR on AR5210, document ath5k_int Luis R. Rodriguez
0 siblings, 1 reply; 15+ messages in thread
From: Luis R. Rodriguez @ 2007-10-12 15:04 UTC (permalink / raw)
To: John Linville; +Cc: linux-wireless, Jiri Slaby, Nick Kossifidis
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 <mcgrof@gmail.com>
---
drivers/net/wireless/ath5k/base.c | 62 ++++++++++++++++++++++++++++++------
1 files changed, 51 insertions(+), 11 deletions(-)
diff --git a/drivers/net/wireless/ath5k/base.c b/drivers/net/wireless/ath5k/base.c
index cda922e..3e46d8f 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,23 @@ 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] = 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 +1450,44 @@ 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] = ~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 +1508,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
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH 3/5] Don't read AR5K_RAC_PISR on AR5210, document ath5k_int
2007-10-12 15:04 ` [PATCH 2/5] Add proper support for multicast Luis R. Rodriguez
@ 2007-10-12 15:05 ` Luis R. Rodriguez
2007-10-12 15:07 ` [PATCH 4/5] Add extensive documenation for the atheros bssid_mask Luis R. Rodriguez
0 siblings, 1 reply; 15+ messages in thread
From: Luis R. Rodriguez @ 2007-10-12 15:05 UTC (permalink / raw)
To: John Linville; +Cc: linux-wireless, Jiri Slaby, Nick Kossifidis
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 <mcgrof@gmail.com>
---
drivers/net/wireless/ath5k/ath5k.h | 65 ++++++++++++++++++++++++++++++-----
drivers/net/wireless/ath5k/hw.c | 15 +++++---
2 files changed, 64 insertions(+), 16 deletions(-)
diff --git a/drivers/net/wireless/ath5k/ath5k.h b/drivers/net/wireless/ath5k/ath5k.h
index c1d3c9d..09c920b 100644
--- a/drivers/net/wireless/ath5k/ath5k.h
+++ b/drivers/net/wireless/ath5k/ath5k.h
@@ -720,18 +720,65 @@ 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: 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 on certain hardware.
+ * Note that 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 is used for RF Kill, disabled for now
+ * until properly handled
+ * @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 +786,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 +803,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..b106ead 100644
--- a/drivers/net/wireless/ath5k/hw.c
+++ b/drivers/net/wireless/ath5k/hw.c
@@ -1470,14 +1470,15 @@ int ath5k_hw_get_isr(struct ath_hw *hal, enum ath5k_int *interrupt_mask)
*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);
}
/*
- * Read interrupt status from the Read-And-Clear shadow register
- */
- data = ath5k_hw_reg_read(hal, AR5K_RAC_PISR);
-
- /*
* Get abstract interrupt mask (HAL-compatible)
*/
*interrupt_mask = (data & AR5K_INT_COMMON) & hal->ah_imr;
@@ -1503,7 +1504,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
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH 4/5] Add extensive documenation for the atheros bssid_mask
2007-10-12 15:05 ` [PATCH 3/5] Don't read AR5K_RAC_PISR on AR5210, document ath5k_int Luis R. Rodriguez
@ 2007-10-12 15:07 ` Luis R. Rodriguez
2007-10-12 15:07 ` [PATCH 5/5] ath5k: Fix and clean mode initialization, prefer G for AR5212 Luis R. Rodriguez
2007-10-16 15:07 ` [PATCH 4/5] Add extensive documenation for the atheros bssid_mask Randy Dunlap
0 siblings, 2 replies; 15+ messages in thread
From: Luis R. Rodriguez @ 2007-10-12 15:07 UTC (permalink / raw)
To: John Linville; +Cc: linux-wireless, Jiri Slaby, Nick Kossifidis
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 <mcgrof@gmail.com>
---
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 3e46d8f..18ee995 100644
--- a/drivers/net/wireless/ath5k/base.c
+++ b/drivers/net/wireless/ath5k/base.c
@@ -2244,10 +2244,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 b106ead..fc00667 100644
--- a/drivers/net/wireless/ath5k/hw.c
+++ b/drivers/net/wireless/ath5k/hw.c
@@ -2323,9 +2323,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
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH 5/5] ath5k: Fix and clean mode initialization, prefer G for AR5212
2007-10-12 15:07 ` [PATCH 4/5] Add extensive documenation for the atheros bssid_mask Luis R. Rodriguez
@ 2007-10-12 15:07 ` Luis R. Rodriguez
2007-10-12 21:33 ` Jiri Slaby
2007-10-16 15:07 ` [PATCH 4/5] Add extensive documenation for the atheros bssid_mask Randy Dunlap
1 sibling, 1 reply; 15+ messages in thread
From: Luis R. Rodriguez @ 2007-10-12 15:07 UTC (permalink / raw)
To: John Linville; +Cc: linux-wireless, Jiri Slaby, Nick Kossifidis
Currently you get locked on B mode with AR5212s. This could be partly
mac80211's fault with a recent regression introduced but ath5k mode
initialization right now is pretty sloppy. For AR5212s we also currently
start scanning in 5GHz. I've made the mode initialization on ath5k a bit
clearer and only am registering G mode now instead of both B and G for
AR5212s. 11Mbps is still the only stable rate but at least now we can
work and test the other rates again.
Note: mac80211 simple rate algo throws us to 1Mbps after assoc, this is by
design. I recommend users to set rate to 11M for now after assoc.
Changes-licensed-under: 3-clause-BSD
Signed-off-by: Luis R. Rodriguez <mcgrof@gmail.com>
---
drivers/net/wireless/ath5k/base.c | 104 +++++++++++++++++++-----------------
1 files changed, 55 insertions(+), 49 deletions(-)
diff --git a/drivers/net/wireless/ath5k/base.c b/drivers/net/wireless/ath5k/base.c
index 18ee995..5cb48d4 100644
--- a/drivers/net/wireless/ath5k/base.c
+++ b/drivers/net/wireless/ath5k/base.c
@@ -1919,67 +1919,73 @@ static void ath_dump_modes(struct ieee80211_hw_mode *modes)
static inline void ath_dump_modes(struct ieee80211_hw_mode *modes) {}
#endif
+#define REGISTER_MODE(mode) do { \
+ if (modes[mode].num_channels) { \
+ ret = ieee80211_register_hwmode(hw, &modes[mode]); \
+ if (ret) { \
+ printk(KERN_ERR "can't register hwmode %u\n", mode); \
+ goto err; \
+ } \
+ } \
+} while (0)
+
static int ath_getchannels(struct ieee80211_hw *hw)
{
struct ath_softc *sc = hw->priv;
struct ath_hw *ah = sc->ah;
struct ieee80211_hw_mode *modes = sc->modes;
- unsigned int i, max;
+ unsigned int i, max_r, max_c;
int ret;
- enum {
- A = MODE_IEEE80211A,
- B = MODE_IEEE80211G, /* this is not a typo, but workaround */
- G = MODE_IEEE80211B, /* to prefer g over b */
- T = MODE_ATHEROS_TURBO,
- TG = MODE_ATHEROS_TURBOG,
- };
BUILD_BUG_ON(ARRAY_SIZE(sc->modes) < 3);
ah->ah_country_code = countrycode;
- modes[A].mode = MODE_IEEE80211A;
- modes[B].mode = MODE_IEEE80211B;
- modes[G].mode = MODE_IEEE80211G;
-
- max = ARRAY_SIZE(sc->rates);
- modes[A].rates = sc->rates;
- max -= modes[A].num_rates = ath_copy_rates(modes[A].rates,
- ath5k_hw_get_rate_table(ah, MODE_IEEE80211A), max);
- modes[B].rates = &modes[A].rates[modes[A].num_rates];
- max -= modes[B].num_rates = ath_copy_rates(modes[B].rates,
- ath5k_hw_get_rate_table(ah, MODE_IEEE80211B), max);
- modes[G].rates = &modes[B].rates[modes[B].num_rates];
- max -= modes[G].num_rates = ath_copy_rates(modes[G].rates,
- ath5k_hw_get_rate_table(ah, MODE_IEEE80211G), max);
-
- if (!max)
- printk(KERN_WARNING "yet another rates found, but there is not "
- "sufficient space to store them\n");
-
- max = ARRAY_SIZE(sc->channels);
- modes[A].channels = sc->channels;
- max -= modes[A].num_channels = ath_copy_channels(ah, modes[A].channels,
- MODE_IEEE80211A, max);
- modes[B].channels = &modes[A].channels[modes[A].num_channels];
- max -= modes[B].num_channels = ath_copy_channels(ah, modes[B].channels,
- MODE_IEEE80211B, max);
- modes[G].channels = &modes[B].channels[modes[B].num_channels];
- max -= modes[G].num_channels = ath_copy_channels(ah, modes[G].channels,
- MODE_IEEE80211G, max);
-
- if (!max)
- printk(KERN_WARNING "yet another modes found, but there is not "
- "sufficient space to store them\n");
-
- for (i = 0; i < ARRAY_SIZE(sc->modes); i++)
- if (modes[i].num_channels) {
- ret = ieee80211_register_hwmode(hw, &modes[i]);
- if (ret) {
- printk(KERN_ERR "can't register hwmode %u\n",i);
- goto err;
- }
+ modes[0].mode = MODE_IEEE80211G;
+ modes[1].mode = MODE_IEEE80211B;
+ modes[2].mode = MODE_IEEE80211A;
+
+ max_r = ARRAY_SIZE(sc->rates);
+ max_c = ARRAY_SIZE(sc->channels);
+ modes[0].rates = sc->rates;
+
+ for (i = 0; i <= 2; i++) {
+ struct ieee80211_hw_mode *mode = &modes[i];
+ const struct ath5k_rate_table *hw_rates;
+
+ if (i == 0) {
+ mode->rates = sc->rates;
+ modes->channels = sc->channels;
+ } else {
+ struct ieee80211_hw_mode *prev_mode = &modes[i-1];
+ int prev_num_r = prev_mode->num_rates;
+ int prev_num_c = prev_mode->num_channels;
+ mode->rates = &prev_mode->rates[prev_num_r];
+ mode->channels = &prev_mode->channels[prev_num_c];
}
+
+ hw_rates = ath5k_hw_get_rate_table(ah, mode->mode);
+ mode->num_rates = ath_copy_rates(mode->rates, hw_rates,
+ max_r);
+ mode->num_channels = ath_copy_channels(ah, mode->channels,
+ mode->mode, max_c);
+ max_r -= modes->num_rates;
+ max_c -= mode->num_channels;
+ }
+
+ switch (ah->ah_version) {
+ case AR5K_AR5210:
+ REGISTER_MODE(MODE_IEEE80211A);
+ break;
+ case AR5K_AR5211:
+ REGISTER_MODE(MODE_IEEE80211B);
+ break;
+ case AR5K_AR5212:
+ if (!ah->ah_single_chip)
+ REGISTER_MODE(MODE_IEEE80211A);
+ REGISTER_MODE(MODE_IEEE80211G);
+ break;
+ }
ath_dump_modes(modes);
return 0;
--
1.5.2.5
^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH 5/5] ath5k: Fix and clean mode initialization, prefer G for AR5212
2007-10-12 15:07 ` [PATCH 5/5] ath5k: Fix and clean mode initialization, prefer G for AR5212 Luis R. Rodriguez
@ 2007-10-12 21:33 ` Jiri Slaby
2007-10-12 22:37 ` Luis R. Rodriguez
0 siblings, 1 reply; 15+ messages in thread
From: Jiri Slaby @ 2007-10-12 21:33 UTC (permalink / raw)
To: Luis R. Rodriguez; +Cc: John Linville, linux-wireless, Nick Kossifidis
I didn't sleep too much due to travelling, maybe I'm wrong in some
cases, some comments follows.
On 10/12/07, Luis R. Rodriguez <mcgrof@gmail.com> wrote:
> Currently you get locked on B mode with AR5212s. This could be partly
> mac80211's fault with a recent regression introduced but ath5k mode
> initialization right now is pretty sloppy. For AR5212s we also currently
> start scanning in 5GHz. I've made the mode initialization on ath5k a bit
> clearer and only am registering G mode now instead of both B and G for
> AR5212s. 11Mbps is still the only stable rate but at least now we can
> work and test the other rates again.
>
> Note: mac80211 simple rate algo throws us to 1Mbps after assoc, this is by
> design. I recommend users to set rate to 11M for now after assoc.
>
> Changes-licensed-under: 3-clause-BSD
> Signed-off-by: Luis R. Rodriguez <mcgrof@gmail.com>
> ---
> drivers/net/wireless/ath5k/base.c | 104 +++++++++++++++++++-----------------
> 1 files changed, 55 insertions(+), 49 deletions(-)
>
> diff --git a/drivers/net/wireless/ath5k/base.c b/drivers/net/wireless/ath5k/base.c
> index 18ee995..5cb48d4 100644
> --- a/drivers/net/wireless/ath5k/base.c
> +++ b/drivers/net/wireless/ath5k/base.c
> @@ -1919,67 +1919,73 @@ static void ath_dump_modes(struct ieee80211_hw_mode *modes)
> static inline void ath_dump_modes(struct ieee80211_hw_mode *modes) {}
> #endif
>
> +#define REGISTER_MODE(mode) do { \
> + if (modes[mode].num_channels) { \
> + ret = ieee80211_register_hwmode(hw, &modes[mode]); \
> + if (ret) { \
> + printk(KERN_ERR "can't register hwmode %u\n", mode); \
> + goto err; \
> + } \
> + } \
> +} while (0)
> +
> static int ath_getchannels(struct ieee80211_hw *hw)
> {
> struct ath_softc *sc = hw->priv;
> struct ath_hw *ah = sc->ah;
> struct ieee80211_hw_mode *modes = sc->modes;
> - unsigned int i, max;
> + unsigned int i, max_r, max_c;
> int ret;
> - enum {
> - A = MODE_IEEE80211A,
> - B = MODE_IEEE80211G, /* this is not a typo, but workaround */
> - G = MODE_IEEE80211B, /* to prefer g over b */
> - T = MODE_ATHEROS_TURBO,
> - TG = MODE_ATHEROS_TURBOG,
> - };
>
> BUILD_BUG_ON(ARRAY_SIZE(sc->modes) < 3);
>
> ah->ah_country_code = countrycode;
>
> - modes[A].mode = MODE_IEEE80211A;
> - modes[B].mode = MODE_IEEE80211B;
> - modes[G].mode = MODE_IEEE80211G;
> -
> - max = ARRAY_SIZE(sc->rates);
> - modes[A].rates = sc->rates;
> - max -= modes[A].num_rates = ath_copy_rates(modes[A].rates,
> - ath5k_hw_get_rate_table(ah, MODE_IEEE80211A), max);
> - modes[B].rates = &modes[A].rates[modes[A].num_rates];
> - max -= modes[B].num_rates = ath_copy_rates(modes[B].rates,
> - ath5k_hw_get_rate_table(ah, MODE_IEEE80211B), max);
> - modes[G].rates = &modes[B].rates[modes[B].num_rates];
> - max -= modes[G].num_rates = ath_copy_rates(modes[G].rates,
> - ath5k_hw_get_rate_table(ah, MODE_IEEE80211G), max);
> -
> - if (!max)
> - printk(KERN_WARNING "yet another rates found, but there is not "
> - "sufficient space to store them\n");
> -
> - max = ARRAY_SIZE(sc->channels);
> - modes[A].channels = sc->channels;
> - max -= modes[A].num_channels = ath_copy_channels(ah, modes[A].channels,
> - MODE_IEEE80211A, max);
> - modes[B].channels = &modes[A].channels[modes[A].num_channels];
> - max -= modes[B].num_channels = ath_copy_channels(ah, modes[B].channels,
> - MODE_IEEE80211B, max);
> - modes[G].channels = &modes[B].channels[modes[B].num_channels];
> - max -= modes[G].num_channels = ath_copy_channels(ah, modes[G].channels,
> - MODE_IEEE80211G, max);
> -
> - if (!max)
> - printk(KERN_WARNING "yet another modes found, but there is not "
> - "sufficient space to store them\n");
> -
> - for (i = 0; i < ARRAY_SIZE(sc->modes); i++)
> - if (modes[i].num_channels) {
> - ret = ieee80211_register_hwmode(hw, &modes[i]);
> - if (ret) {
> - printk(KERN_ERR "can't register hwmode %u\n",i);
> - goto err;
> - }
> + modes[0].mode = MODE_IEEE80211G;
> + modes[1].mode = MODE_IEEE80211B;
> + modes[2].mode = MODE_IEEE80211A;
> +
> + max_r = ARRAY_SIZE(sc->rates);
> + max_c = ARRAY_SIZE(sc->channels);
> + modes[0].rates = sc->rates;
> +
> + for (i = 0; i <= 2; i++) {
> + struct ieee80211_hw_mode *mode = &modes[i];
> + const struct ath5k_rate_table *hw_rates;
> +
> + if (i == 0) {
> + mode->rates = sc->rates;
> + modes->channels = sc->channels;
mode/modes, it's doesn't matter here, but it doesn't look well
> + } else {
> + struct ieee80211_hw_mode *prev_mode = &modes[i-1];
> + int prev_num_r = prev_mode->num_rates;
> + int prev_num_c = prev_mode->num_channels;
> + mode->rates = &prev_mode->rates[prev_num_r];
> + mode->channels = &prev_mode->channels[prev_num_c];
> }
> +
> + hw_rates = ath5k_hw_get_rate_table(ah, mode->mode);
> + mode->num_rates = ath_copy_rates(mode->rates, hw_rates,
> + max_r);
> + mode->num_channels = ath_copy_channels(ah, mode->channels,
> + mode->mode, max_c);
> + max_r -= modes->num_rates;
modes? but here it matters...
> + max_c -= mode->num_channels;
> + }
> +
> + switch (ah->ah_version) {
> + case AR5K_AR5210:
> + REGISTER_MODE(MODE_IEEE80211A);
> + break;
> + case AR5K_AR5211:
> + REGISTER_MODE(MODE_IEEE80211B);
> + break;
> + case AR5K_AR5212:
> + if (!ah->ah_single_chip)
> + REGISTER_MODE(MODE_IEEE80211A);
> + REGISTER_MODE(MODE_IEEE80211G);
And this seems totally wrong. you use 0, 1, 2 indexes above and here
you index it by mac80211 integers. That's exactly why the enum was
there (a shortcut like in intel drivers) -- to not get into it.
> + break;
> + }
> ath_dump_modes(modes);
>
> return 0;
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 5/5] ath5k: Fix and clean mode initialization, prefer G for AR5212
2007-10-12 21:33 ` Jiri Slaby
@ 2007-10-12 22:37 ` Luis R. Rodriguez
2007-10-13 7:34 ` Jiri Slaby
2007-10-13 9:21 ` Nick Kossifidis
0 siblings, 2 replies; 15+ messages in thread
From: Luis R. Rodriguez @ 2007-10-12 22:37 UTC (permalink / raw)
To: Jiri Slaby; +Cc: John Linville, linux-wireless, Nick Kossifidis
On Fri, Oct 12, 2007 at 11:33:05PM +0200, Jiri Slaby wrote:
> I didn't sleep too much due to travelling, maybe I'm wrong in some
> cases, some comments follows.
Seems it was me that didn't get the sleep ;)
> On 10/12/07, Luis R. Rodriguez <mcgrof@gmail.com> wrote:
> > + modes[0].mode = MODE_IEEE80211G;
> > + modes[1].mode = MODE_IEEE80211B;
> > + modes[2].mode = MODE_IEEE80211A;
> > +
> > + max_r = ARRAY_SIZE(sc->rates);
> > + max_c = ARRAY_SIZE(sc->channels);
> > + modes[0].rates = sc->rates;
I'll delete this line above.
> > +
> > + for (i = 0; i <= 2; i++) {
> > + struct ieee80211_hw_mode *mode = &modes[i];
> > + const struct ath5k_rate_table *hw_rates;
> > +
> > + if (i == 0) {
> > + mode->rates = sc->rates;
> > + modes->channels = sc->channels;
>
> mode/modes, it's doesn't matter here, but it doesn't look well
And put it above as modes[0].rates = sc->rates then.
> > + } else {
> > + struct ieee80211_hw_mode *prev_mode = &modes[i-1];
> > + int prev_num_r = prev_mode->num_rates;
> > + int prev_num_c = prev_mode->num_channels;
> > + mode->rates = &prev_mode->rates[prev_num_r];
> > + mode->channels = &prev_mode->channels[prev_num_c];
> > }
> > +
> > + hw_rates = ath5k_hw_get_rate_table(ah, mode->mode);
> > + mode->num_rates = ath_copy_rates(mode->rates, hw_rates,
> > + max_r);
> > + mode->num_channels = ath_copy_channels(ah, mode->channels,
> > + mode->mode, max_c);
> > + max_r -= modes->num_rates;
>
> modes? but here it matters...
Good catch.
> > + max_c -= mode->num_channels;
> > + }
> > +
> > + switch (ah->ah_version) {
> > + case AR5K_AR5210:
> > + REGISTER_MODE(MODE_IEEE80211A);
> > + break;
> > + case AR5K_AR5211:
> > + REGISTER_MODE(MODE_IEEE80211B);
> > + break;
> > + case AR5K_AR5212:
> > + if (!ah->ah_single_chip)
> > + REGISTER_MODE(MODE_IEEE80211A);
> > + REGISTER_MODE(MODE_IEEE80211G);
>
> And this seems totally wrong. you use 0, 1, 2 indexes above and here
> you index it by mac80211 integers. That's exactly why the enum was
> there (a shortcut like in intel drivers) -- to not get into it.
Actually you are right, that was not what I intended though. The A, B,
and G enums seemed to add confusion so I just got rid of them. What I
really *intended* on doing was:
#define REGISTER_MODE(m) do { \
for (i=0; i<2; i++) { \
if (modes[i].mode != m || !modes[i].num_channels) \
continue; \
ret = ieee80211_register_hwmode(hw, &modes[i]); \
if (ret) { \
printk(KERN_ERR "can't register hwmode %u\n",m); \
goto err; \
} \
} \
} while (0)
Also, you forgot to catch I wasn't registering MODE_IEEE80211A for
AR5211, I've added it now :). BTW with johill's patch,
[PATCH v2] mac80211: fix set_channel regression
the mode that is used first changes now is reversed again depending on
the order in which you registered it so I've moved things around to assure
we prefer keep preferring G for AR5212s and B for AR5211s.
Below you'll find the new revised patch. This was tested with AR5212
and AR5211. Checkpatch was run too ;)
Luis
[PATCH 5/5] ath5k: Fix and clean mode initialization, prefer G for AR5212
Currently you get locked on B mode with AR5212s. This could be partly
mac80211's fault with a recent regression introduced but ath5k mode
initialization right now is pretty sloppy. For AR5212s we also currently
start scanning in 5GHz. I've made the mode initialization on ath5k a bit
clearer and only am registering G mode now instead of both B and G for
AR5212s. 11Mbps is still the only stable rate but at least now we can
work and test the other rates again.
Note: mac80211 simple rate algo throws us to 1Mbps after assoc, this is by
design. I recommend users to set rate to 11M for now after assoc.
Changes-licensed-under: 3-clause-BSD
Signed-off-by: Luis R. Rodriguez <mcgrof@gmail.com>
---
drivers/net/wireless/ath5k/base.c | 106 ++++++++++++++++++++-----------------
net/mac80211/ieee80211_ioctl.c | 5 ++-
2 files changed, 61 insertions(+), 50 deletions(-)
diff --git a/drivers/net/wireless/ath5k/base.c b/drivers/net/wireless/ath5k/base.c
index 18ee995..8413950 100644
--- a/drivers/net/wireless/ath5k/base.c
+++ b/drivers/net/wireless/ath5k/base.c
@@ -1919,67 +1919,75 @@ static void ath_dump_modes(struct ieee80211_hw_mode *modes)
static inline void ath_dump_modes(struct ieee80211_hw_mode *modes) {}
#endif
+#define REGISTER_MODE(m) do { \
+ for (i = 0; i <= 2; i++) { \
+ if (modes[i].mode != m || !modes[i].num_channels) \
+ continue; \
+ ret = ieee80211_register_hwmode(hw, &modes[i]); \
+ if (ret) { \
+ printk(KERN_ERR "can't register hwmode %u\n", m); \
+ goto err; \
+ } \
+ } \
+} while (0)
+
static int ath_getchannels(struct ieee80211_hw *hw)
{
struct ath_softc *sc = hw->priv;
struct ath_hw *ah = sc->ah;
struct ieee80211_hw_mode *modes = sc->modes;
- unsigned int i, max;
+ unsigned int i, max_r, max_c;
int ret;
- enum {
- A = MODE_IEEE80211A,
- B = MODE_IEEE80211G, /* this is not a typo, but workaround */
- G = MODE_IEEE80211B, /* to prefer g over b */
- T = MODE_ATHEROS_TURBO,
- TG = MODE_ATHEROS_TURBOG,
- };
BUILD_BUG_ON(ARRAY_SIZE(sc->modes) < 3);
ah->ah_country_code = countrycode;
- modes[A].mode = MODE_IEEE80211A;
- modes[B].mode = MODE_IEEE80211B;
- modes[G].mode = MODE_IEEE80211G;
-
- max = ARRAY_SIZE(sc->rates);
- modes[A].rates = sc->rates;
- max -= modes[A].num_rates = ath_copy_rates(modes[A].rates,
- ath5k_hw_get_rate_table(ah, MODE_IEEE80211A), max);
- modes[B].rates = &modes[A].rates[modes[A].num_rates];
- max -= modes[B].num_rates = ath_copy_rates(modes[B].rates,
- ath5k_hw_get_rate_table(ah, MODE_IEEE80211B), max);
- modes[G].rates = &modes[B].rates[modes[B].num_rates];
- max -= modes[G].num_rates = ath_copy_rates(modes[G].rates,
- ath5k_hw_get_rate_table(ah, MODE_IEEE80211G), max);
-
- if (!max)
- printk(KERN_WARNING "yet another rates found, but there is not "
- "sufficient space to store them\n");
-
- max = ARRAY_SIZE(sc->channels);
- modes[A].channels = sc->channels;
- max -= modes[A].num_channels = ath_copy_channels(ah, modes[A].channels,
- MODE_IEEE80211A, max);
- modes[B].channels = &modes[A].channels[modes[A].num_channels];
- max -= modes[B].num_channels = ath_copy_channels(ah, modes[B].channels,
- MODE_IEEE80211B, max);
- modes[G].channels = &modes[B].channels[modes[B].num_channels];
- max -= modes[G].num_channels = ath_copy_channels(ah, modes[G].channels,
- MODE_IEEE80211G, max);
-
- if (!max)
- printk(KERN_WARNING "yet another modes found, but there is not "
- "sufficient space to store them\n");
-
- for (i = 0; i < ARRAY_SIZE(sc->modes); i++)
- if (modes[i].num_channels) {
- ret = ieee80211_register_hwmode(hw, &modes[i]);
- if (ret) {
- printk(KERN_ERR "can't register hwmode %u\n",i);
- goto err;
- }
+ modes[0].mode = MODE_IEEE80211G;
+ modes[1].mode = MODE_IEEE80211B;
+ modes[2].mode = MODE_IEEE80211A;
+
+ max_r = ARRAY_SIZE(sc->rates);
+ max_c = ARRAY_SIZE(sc->channels);
+
+ for (i = 0; i <= 2; i++) {
+ struct ieee80211_hw_mode *mode = &modes[i];
+ const struct ath5k_rate_table *hw_rates;
+
+ if (i == 0) {
+ modes[0].rates = sc->rates;
+ modes->channels = sc->channels;
+ } else {
+ struct ieee80211_hw_mode *prev_mode = &modes[i-1];
+ int prev_num_r = prev_mode->num_rates;
+ int prev_num_c = prev_mode->num_channels;
+ mode->rates = &prev_mode->rates[prev_num_r];
+ mode->channels = &prev_mode->channels[prev_num_c];
}
+
+ hw_rates = ath5k_hw_get_rate_table(ah, mode->mode);
+ mode->num_rates = ath_copy_rates(mode->rates, hw_rates,
+ max_r);
+ mode->num_channels = ath_copy_channels(ah, mode->channels,
+ mode->mode, max_c);
+ max_r -= mode->num_rates;
+ max_c -= mode->num_channels;
+ }
+
+ switch (ah->ah_version) {
+ case AR5K_AR5210:
+ REGISTER_MODE(MODE_IEEE80211A);
+ break;
+ case AR5K_AR5211:
+ REGISTER_MODE(MODE_IEEE80211B);
+ REGISTER_MODE(MODE_IEEE80211A);
+ break;
+ case AR5K_AR5212:
+ REGISTER_MODE(MODE_IEEE80211G);
+ if (!ah->ah_single_chip)
+ REGISTER_MODE(MODE_IEEE80211A);
+ break;
+ }
ath_dump_modes(modes);
return 0;
^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH 5/5] ath5k: Fix and clean mode initialization, prefer G for AR5212
2007-10-12 22:37 ` Luis R. Rodriguez
@ 2007-10-13 7:34 ` Jiri Slaby
2007-10-13 9:21 ` Nick Kossifidis
1 sibling, 0 replies; 15+ messages in thread
From: Jiri Slaby @ 2007-10-13 7:34 UTC (permalink / raw)
To: Luis R. Rodriguez; +Cc: John Linville, linux-wireless, Nick Kossifidis
On 10/13/2007 12:37 AM, Luis R. Rodriguez wrote:
> [PATCH 5/5] ath5k: Fix and clean mode initialization, prefer G for AR5212
>
> Currently you get locked on B mode with AR5212s. This could be partly
> mac80211's fault with a recent regression introduced but ath5k mode
> initialization right now is pretty sloppy. For AR5212s we also currently
> start scanning in 5GHz. I've made the mode initialization on ath5k a bit
> clearer and only am registering G mode now instead of both B and G for
> AR5212s. 11Mbps is still the only stable rate but at least now we can
> work and test the other rates again.
>
> Note: mac80211 simple rate algo throws us to 1Mbps after assoc, this is by
> design. I recommend users to set rate to 11M for now after assoc.
>
> Changes-licensed-under: 3-clause-BSD
> Signed-off-by: Luis R. Rodriguez <mcgrof@gmail.com>
Acked-by: Jiri Slaby <jirislaby@gmail.com>
> ---
> drivers/net/wireless/ath5k/base.c | 106 ++++++++++++++++++++-----------------
> net/mac80211/ieee80211_ioctl.c | 5 ++-
> 2 files changed, 61 insertions(+), 50 deletions(-)
>
> diff --git a/drivers/net/wireless/ath5k/base.c b/drivers/net/wireless/ath5k/base.c
> index 18ee995..8413950 100644
> --- a/drivers/net/wireless/ath5k/base.c
> +++ b/drivers/net/wireless/ath5k/base.c
[...]
> + modes[0].mode = MODE_IEEE80211G;
> + modes[1].mode = MODE_IEEE80211B;
> + modes[2].mode = MODE_IEEE80211A;
> +
> + max_r = ARRAY_SIZE(sc->rates);
> + max_c = ARRAY_SIZE(sc->channels);
> +
> + for (i = 0; i <= 2; i++) {
> + struct ieee80211_hw_mode *mode = &modes[i];
> + const struct ath5k_rate_table *hw_rates;
> +
> + if (i == 0) {
> + modes[0].rates = sc->rates;
> + modes->channels = sc->channels;
:) some kind of personal diversity :D
> ath_dump_modes(modes);
BTW. have you tried to compare outputs from this function before the change and
after it?
regards,
--
Jiri Slaby (jirislaby@gmail.com)
Faculty of Informatics, Masaryk University
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 5/5] ath5k: Fix and clean mode initialization, prefer G for AR5212
2007-10-12 22:37 ` Luis R. Rodriguez
2007-10-13 7:34 ` Jiri Slaby
@ 2007-10-13 9:21 ` Nick Kossifidis
2007-10-13 9:30 ` Nick Kossifidis
1 sibling, 1 reply; 15+ messages in thread
From: Nick Kossifidis @ 2007-10-13 9:21 UTC (permalink / raw)
To: Luis R. Rodriguez; +Cc: Jiri Slaby, John Linville, linux-wireless
You can also check card capabilities before initializing the modes
instead of ah_version since you have access on cap.mode ;-)
2007/10/13, Luis R. Rodriguez <mcgrof@gmail.com>:
> On Fri, Oct 12, 2007 at 11:33:05PM +0200, Jiri Slaby wrote:
> > I didn't sleep too much due to travelling, maybe I'm wrong in some
> > cases, some comments follows.
>
> Seems it was me that didn't get the sleep ;)
>
> > On 10/12/07, Luis R. Rodriguez <mcgrof@gmail.com> wrote:
> > > + modes[0].mode = MODE_IEEE80211G;
> > > + modes[1].mode = MODE_IEEE80211B;
> > > + modes[2].mode = MODE_IEEE80211A;
> > > +
> > > + max_r = ARRAY_SIZE(sc->rates);
> > > + max_c = ARRAY_SIZE(sc->channels);
> > > + modes[0].rates = sc->rates;
>
> I'll delete this line above.
>
> > > +
> > > + for (i = 0; i <= 2; i++) {
> > > + struct ieee80211_hw_mode *mode = &modes[i];
> > > + const struct ath5k_rate_table *hw_rates;
> > > +
> > > + if (i == 0) {
> > > + mode->rates = sc->rates;
> > > + modes->channels = sc->channels;
> >
> > mode/modes, it's doesn't matter here, but it doesn't look well
>
> And put it above as modes[0].rates = sc->rates then.
>
> > > + } else {
> > > + struct ieee80211_hw_mode *prev_mode = &modes[i-1];
> > > + int prev_num_r = prev_mode->num_rates;
> > > + int prev_num_c = prev_mode->num_channels;
> > > + mode->rates = &prev_mode->rates[prev_num_r];
> > > + mode->channels = &prev_mode->channels[prev_num_c];
> > > }
> > > +
> > > + hw_rates = ath5k_hw_get_rate_table(ah, mode->mode);
> > > + mode->num_rates = ath_copy_rates(mode->rates, hw_rates,
> > > + max_r);
> > > + mode->num_channels = ath_copy_channels(ah, mode->channels,
> > > + mode->mode, max_c);
> > > + max_r -= modes->num_rates;
> >
> > modes? but here it matters...
>
> Good catch.
>
> > > + max_c -= mode->num_channels;
> > > + }
> > > +
> > > + switch (ah->ah_version) {
> > > + case AR5K_AR5210:
> > > + REGISTER_MODE(MODE_IEEE80211A);
> > > + break;
> > > + case AR5K_AR5211:
> > > + REGISTER_MODE(MODE_IEEE80211B);
> > > + break;
> > > + case AR5K_AR5212:
> > > + if (!ah->ah_single_chip)
> > > + REGISTER_MODE(MODE_IEEE80211A);
> > > + REGISTER_MODE(MODE_IEEE80211G);
> >
> > And this seems totally wrong. you use 0, 1, 2 indexes above and here
> > you index it by mac80211 integers. That's exactly why the enum was
> > there (a shortcut like in intel drivers) -- to not get into it.
>
> Actually you are right, that was not what I intended though. The A, B,
> and G enums seemed to add confusion so I just got rid of them. What I
> really *intended* on doing was:
>
> #define REGISTER_MODE(m) do { \
> for (i=0; i<2; i++) { \
> if (modes[i].mode != m || !modes[i].num_channels) \
> continue; \
> ret = ieee80211_register_hwmode(hw, &modes[i]); \
> if (ret) { \
> printk(KERN_ERR "can't register hwmode %u\n",m); \
> goto err; \
> } \
> } \
> } while (0)
>
> Also, you forgot to catch I wasn't registering MODE_IEEE80211A for
> AR5211, I've added it now :). BTW with johill's patch,
> [PATCH v2] mac80211: fix set_channel regression
> the mode that is used first changes now is reversed again depending on
> the order in which you registered it so I've moved things around to assure
> we prefer keep preferring G for AR5212s and B for AR5211s.
>
> Below you'll find the new revised patch. This was tested with AR5212
> and AR5211. Checkpatch was run too ;)
>
> Luis
>
> [PATCH 5/5] ath5k: Fix and clean mode initialization, prefer G for AR5212
>
> Currently you get locked on B mode with AR5212s. This could be partly
> mac80211's fault with a recent regression introduced but ath5k mode
> initialization right now is pretty sloppy. For AR5212s we also currently
> start scanning in 5GHz. I've made the mode initialization on ath5k a bit
> clearer and only am registering G mode now instead of both B and G for
> AR5212s. 11Mbps is still the only stable rate but at least now we can
> work and test the other rates again.
>
> Note: mac80211 simple rate algo throws us to 1Mbps after assoc, this is by
> design. I recommend users to set rate to 11M for now after assoc.
>
> Changes-licensed-under: 3-clause-BSD
> Signed-off-by: Luis R. Rodriguez <mcgrof@gmail.com>
> ---
> drivers/net/wireless/ath5k/base.c | 106 ++++++++++++++++++++-----------------
> net/mac80211/ieee80211_ioctl.c | 5 ++-
> 2 files changed, 61 insertions(+), 50 deletions(-)
>
> diff --git a/drivers/net/wireless/ath5k/base.c b/drivers/net/wireless/ath5k/base.c
> index 18ee995..8413950 100644
> --- a/drivers/net/wireless/ath5k/base.c
> +++ b/drivers/net/wireless/ath5k/base.c
> @@ -1919,67 +1919,75 @@ static void ath_dump_modes(struct ieee80211_hw_mode *modes)
> static inline void ath_dump_modes(struct ieee80211_hw_mode *modes) {}
> #endif
>
> +#define REGISTER_MODE(m) do { \
> + for (i = 0; i <= 2; i++) { \
> + if (modes[i].mode != m || !modes[i].num_channels) \
> + continue; \
> + ret = ieee80211_register_hwmode(hw, &modes[i]); \
> + if (ret) { \
> + printk(KERN_ERR "can't register hwmode %u\n", m); \
> + goto err; \
> + } \
> + } \
> +} while (0)
> +
> static int ath_getchannels(struct ieee80211_hw *hw)
> {
> struct ath_softc *sc = hw->priv;
> struct ath_hw *ah = sc->ah;
> struct ieee80211_hw_mode *modes = sc->modes;
> - unsigned int i, max;
> + unsigned int i, max_r, max_c;
> int ret;
> - enum {
> - A = MODE_IEEE80211A,
> - B = MODE_IEEE80211G, /* this is not a typo, but workaround */
> - G = MODE_IEEE80211B, /* to prefer g over b */
> - T = MODE_ATHEROS_TURBO,
> - TG = MODE_ATHEROS_TURBOG,
> - };
>
> BUILD_BUG_ON(ARRAY_SIZE(sc->modes) < 3);
>
> ah->ah_country_code = countrycode;
>
> - modes[A].mode = MODE_IEEE80211A;
> - modes[B].mode = MODE_IEEE80211B;
> - modes[G].mode = MODE_IEEE80211G;
> -
> - max = ARRAY_SIZE(sc->rates);
> - modes[A].rates = sc->rates;
> - max -= modes[A].num_rates = ath_copy_rates(modes[A].rates,
> - ath5k_hw_get_rate_table(ah, MODE_IEEE80211A), max);
> - modes[B].rates = &modes[A].rates[modes[A].num_rates];
> - max -= modes[B].num_rates = ath_copy_rates(modes[B].rates,
> - ath5k_hw_get_rate_table(ah, MODE_IEEE80211B), max);
> - modes[G].rates = &modes[B].rates[modes[B].num_rates];
> - max -= modes[G].num_rates = ath_copy_rates(modes[G].rates,
> - ath5k_hw_get_rate_table(ah, MODE_IEEE80211G), max);
> -
> - if (!max)
> - printk(KERN_WARNING "yet another rates found, but there is not "
> - "sufficient space to store them\n");
> -
> - max = ARRAY_SIZE(sc->channels);
> - modes[A].channels = sc->channels;
> - max -= modes[A].num_channels = ath_copy_channels(ah, modes[A].channels,
> - MODE_IEEE80211A, max);
> - modes[B].channels = &modes[A].channels[modes[A].num_channels];
> - max -= modes[B].num_channels = ath_copy_channels(ah, modes[B].channels,
> - MODE_IEEE80211B, max);
> - modes[G].channels = &modes[B].channels[modes[B].num_channels];
> - max -= modes[G].num_channels = ath_copy_channels(ah, modes[G].channels,
> - MODE_IEEE80211G, max);
> -
> - if (!max)
> - printk(KERN_WARNING "yet another modes found, but there is not "
> - "sufficient space to store them\n");
> -
> - for (i = 0; i < ARRAY_SIZE(sc->modes); i++)
> - if (modes[i].num_channels) {
> - ret = ieee80211_register_hwmode(hw, &modes[i]);
> - if (ret) {
> - printk(KERN_ERR "can't register hwmode %u\n",i);
> - goto err;
> - }
> + modes[0].mode = MODE_IEEE80211G;
> + modes[1].mode = MODE_IEEE80211B;
> + modes[2].mode = MODE_IEEE80211A;
> +
> + max_r = ARRAY_SIZE(sc->rates);
> + max_c = ARRAY_SIZE(sc->channels);
> +
> + for (i = 0; i <= 2; i++) {
> + struct ieee80211_hw_mode *mode = &modes[i];
> + const struct ath5k_rate_table *hw_rates;
> +
> + if (i == 0) {
> + modes[0].rates = sc->rates;
> + modes->channels = sc->channels;
> + } else {
> + struct ieee80211_hw_mode *prev_mode = &modes[i-1];
> + int prev_num_r = prev_mode->num_rates;
> + int prev_num_c = prev_mode->num_channels;
> + mode->rates = &prev_mode->rates[prev_num_r];
> + mode->channels = &prev_mode->channels[prev_num_c];
> }
> +
> + hw_rates = ath5k_hw_get_rate_table(ah, mode->mode);
> + mode->num_rates = ath_copy_rates(mode->rates, hw_rates,
> + max_r);
> + mode->num_channels = ath_copy_channels(ah, mode->channels,
> + mode->mode, max_c);
> + max_r -= mode->num_rates;
> + max_c -= mode->num_channels;
> + }
> +
> + switch (ah->ah_version) {
> + case AR5K_AR5210:
> + REGISTER_MODE(MODE_IEEE80211A);
> + break;
> + case AR5K_AR5211:
> + REGISTER_MODE(MODE_IEEE80211B);
> + REGISTER_MODE(MODE_IEEE80211A);
> + break;
> + case AR5K_AR5212:
> + REGISTER_MODE(MODE_IEEE80211G);
> + if (!ah->ah_single_chip)
> + REGISTER_MODE(MODE_IEEE80211A);
> + break;
> + }
> ath_dump_modes(modes);
>
> return 0;
>
--
GPG ID: 0xD21DB2DB
As you read this post global entropy rises. Have Fun ;-)
Nick
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 5/5] ath5k: Fix and clean mode initialization, prefer G for AR5212
2007-10-13 9:21 ` Nick Kossifidis
@ 2007-10-13 9:30 ` Nick Kossifidis
2007-10-13 9:38 ` Jiri Slaby
0 siblings, 1 reply; 15+ messages in thread
From: Nick Kossifidis @ 2007-10-13 9:30 UTC (permalink / raw)
To: Luis R. Rodriguez; +Cc: Jiri Slaby, John Linville, linux-wireless
I mean something like this...
if(test_bit(MODE_IEEE80211B,hal->ah_capabilities.cap_mode)) {
REGISTER_MODE(MODE_IEEE80211B);
}
this is a better approach since supported modes depend on what MAC/PHY
combination we havek, not only MAC chip, eg. a 5212 with a 2112 phy
won't support 802.11a. EEPROM info is more reliable ;-)
2007/10/13, Nick Kossifidis <mickflemm@gmail.com>:
> You can also check card capabilities before initializing the modes
> instead of ah_version since you have access on cap.mode ;-)
>
> 2007/10/13, Luis R. Rodriguez <mcgrof@gmail.com>:
> > On Fri, Oct 12, 2007 at 11:33:05PM +0200, Jiri Slaby wrote:
> > > I didn't sleep too much due to travelling, maybe I'm wrong in some
> > > cases, some comments follows.
> >
> > Seems it was me that didn't get the sleep ;)
> >
> > > On 10/12/07, Luis R. Rodriguez <mcgrof@gmail.com> wrote:
> > > > + modes[0].mode = MODE_IEEE80211G;
> > > > + modes[1].mode = MODE_IEEE80211B;
> > > > + modes[2].mode = MODE_IEEE80211A;
> > > > +
> > > > + max_r = ARRAY_SIZE(sc->rates);
> > > > + max_c = ARRAY_SIZE(sc->channels);
> > > > + modes[0].rates = sc->rates;
> >
> > I'll delete this line above.
> >
> > > > +
> > > > + for (i = 0; i <= 2; i++) {
> > > > + struct ieee80211_hw_mode *mode = &modes[i];
> > > > + const struct ath5k_rate_table *hw_rates;
> > > > +
> > > > + if (i == 0) {
> > > > + mode->rates = sc->rates;
> > > > + modes->channels = sc->channels;
> > >
> > > mode/modes, it's doesn't matter here, but it doesn't look well
> >
> > And put it above as modes[0].rates = sc->rates then.
> >
> > > > + } else {
> > > > + struct ieee80211_hw_mode *prev_mode = &modes[i-1];
> > > > + int prev_num_r = prev_mode->num_rates;
> > > > + int prev_num_c = prev_mode->num_channels;
> > > > + mode->rates = &prev_mode->rates[prev_num_r];
> > > > + mode->channels = &prev_mode->channels[prev_num_c];
> > > > }
> > > > +
> > > > + hw_rates = ath5k_hw_get_rate_table(ah, mode->mode);
> > > > + mode->num_rates = ath_copy_rates(mode->rates, hw_rates,
> > > > + max_r);
> > > > + mode->num_channels = ath_copy_channels(ah, mode->channels,
> > > > + mode->mode, max_c);
> > > > + max_r -= modes->num_rates;
> > >
> > > modes? but here it matters...
> >
> > Good catch.
> >
> > > > + max_c -= mode->num_channels;
> > > > + }
> > > > +
> > > > + switch (ah->ah_version) {
> > > > + case AR5K_AR5210:
> > > > + REGISTER_MODE(MODE_IEEE80211A);
> > > > + break;
> > > > + case AR5K_AR5211:
> > > > + REGISTER_MODE(MODE_IEEE80211B);
> > > > + break;
> > > > + case AR5K_AR5212:
> > > > + if (!ah->ah_single_chip)
> > > > + REGISTER_MODE(MODE_IEEE80211A);
> > > > + REGISTER_MODE(MODE_IEEE80211G);
> > >
> > > And this seems totally wrong. you use 0, 1, 2 indexes above and here
> > > you index it by mac80211 integers. That's exactly why the enum was
> > > there (a shortcut like in intel drivers) -- to not get into it.
> >
> > Actually you are right, that was not what I intended though. The A, B,
> > and G enums seemed to add confusion so I just got rid of them. What I
> > really *intended* on doing was:
> >
> > #define REGISTER_MODE(m) do { \
> > for (i=0; i<2; i++) { \
> > if (modes[i].mode != m || !modes[i].num_channels) \
> > continue; \
> > ret = ieee80211_register_hwmode(hw, &modes[i]); \
> > if (ret) { \
> > printk(KERN_ERR "can't register hwmode %u\n",m); \
> > goto err; \
> > } \
> > } \
> > } while (0)
> >
> > Also, you forgot to catch I wasn't registering MODE_IEEE80211A for
> > AR5211, I've added it now :). BTW with johill's patch,
> > [PATCH v2] mac80211: fix set_channel regression
> > the mode that is used first changes now is reversed again depending on
> > the order in which you registered it so I've moved things around to assure
> > we prefer keep preferring G for AR5212s and B for AR5211s.
> >
> > Below you'll find the new revised patch. This was tested with AR5212
> > and AR5211. Checkpatch was run too ;)
> >
> > Luis
> >
> > [PATCH 5/5] ath5k: Fix and clean mode initialization, prefer G for AR5212
> >
> > Currently you get locked on B mode with AR5212s. This could be partly
> > mac80211's fault with a recent regression introduced but ath5k mode
> > initialization right now is pretty sloppy. For AR5212s we also currently
> > start scanning in 5GHz. I've made the mode initialization on ath5k a bit
> > clearer and only am registering G mode now instead of both B and G for
> > AR5212s. 11Mbps is still the only stable rate but at least now we can
> > work and test the other rates again.
> >
> > Note: mac80211 simple rate algo throws us to 1Mbps after assoc, this is by
> > design. I recommend users to set rate to 11M for now after assoc.
> >
> > Changes-licensed-under: 3-clause-BSD
> > Signed-off-by: Luis R. Rodriguez <mcgrof@gmail.com>
> > ---
> > drivers/net/wireless/ath5k/base.c | 106 ++++++++++++++++++++-----------------
> > net/mac80211/ieee80211_ioctl.c | 5 ++-
> > 2 files changed, 61 insertions(+), 50 deletions(-)
> >
> > diff --git a/drivers/net/wireless/ath5k/base.c b/drivers/net/wireless/ath5k/base.c
> > index 18ee995..8413950 100644
> > --- a/drivers/net/wireless/ath5k/base.c
> > +++ b/drivers/net/wireless/ath5k/base.c
> > @@ -1919,67 +1919,75 @@ static void ath_dump_modes(struct ieee80211_hw_mode *modes)
> > static inline void ath_dump_modes(struct ieee80211_hw_mode *modes) {}
> > #endif
> >
> > +#define REGISTER_MODE(m) do { \
> > + for (i = 0; i <= 2; i++) { \
> > + if (modes[i].mode != m || !modes[i].num_channels) \
> > + continue; \
> > + ret = ieee80211_register_hwmode(hw, &modes[i]); \
> > + if (ret) { \
> > + printk(KERN_ERR "can't register hwmode %u\n", m); \
> > + goto err; \
> > + } \
> > + } \
> > +} while (0)
> > +
> > static int ath_getchannels(struct ieee80211_hw *hw)
> > {
> > struct ath_softc *sc = hw->priv;
> > struct ath_hw *ah = sc->ah;
> > struct ieee80211_hw_mode *modes = sc->modes;
> > - unsigned int i, max;
> > + unsigned int i, max_r, max_c;
> > int ret;
> > - enum {
> > - A = MODE_IEEE80211A,
> > - B = MODE_IEEE80211G, /* this is not a typo, but workaround */
> > - G = MODE_IEEE80211B, /* to prefer g over b */
> > - T = MODE_ATHEROS_TURBO,
> > - TG = MODE_ATHEROS_TURBOG,
> > - };
> >
> > BUILD_BUG_ON(ARRAY_SIZE(sc->modes) < 3);
> >
> > ah->ah_country_code = countrycode;
> >
> > - modes[A].mode = MODE_IEEE80211A;
> > - modes[B].mode = MODE_IEEE80211B;
> > - modes[G].mode = MODE_IEEE80211G;
> > -
> > - max = ARRAY_SIZE(sc->rates);
> > - modes[A].rates = sc->rates;
> > - max -= modes[A].num_rates = ath_copy_rates(modes[A].rates,
> > - ath5k_hw_get_rate_table(ah, MODE_IEEE80211A), max);
> > - modes[B].rates = &modes[A].rates[modes[A].num_rates];
> > - max -= modes[B].num_rates = ath_copy_rates(modes[B].rates,
> > - ath5k_hw_get_rate_table(ah, MODE_IEEE80211B), max);
> > - modes[G].rates = &modes[B].rates[modes[B].num_rates];
> > - max -= modes[G].num_rates = ath_copy_rates(modes[G].rates,
> > - ath5k_hw_get_rate_table(ah, MODE_IEEE80211G), max);
> > -
> > - if (!max)
> > - printk(KERN_WARNING "yet another rates found, but there is not "
> > - "sufficient space to store them\n");
> > -
> > - max = ARRAY_SIZE(sc->channels);
> > - modes[A].channels = sc->channels;
> > - max -= modes[A].num_channels = ath_copy_channels(ah, modes[A].channels,
> > - MODE_IEEE80211A, max);
> > - modes[B].channels = &modes[A].channels[modes[A].num_channels];
> > - max -= modes[B].num_channels = ath_copy_channels(ah, modes[B].channels,
> > - MODE_IEEE80211B, max);
> > - modes[G].channels = &modes[B].channels[modes[B].num_channels];
> > - max -= modes[G].num_channels = ath_copy_channels(ah, modes[G].channels,
> > - MODE_IEEE80211G, max);
> > -
> > - if (!max)
> > - printk(KERN_WARNING "yet another modes found, but there is not "
> > - "sufficient space to store them\n");
> > -
> > - for (i = 0; i < ARRAY_SIZE(sc->modes); i++)
> > - if (modes[i].num_channels) {
> > - ret = ieee80211_register_hwmode(hw, &modes[i]);
> > - if (ret) {
> > - printk(KERN_ERR "can't register hwmode %u\n",i);
> > - goto err;
> > - }
> > + modes[0].mode = MODE_IEEE80211G;
> > + modes[1].mode = MODE_IEEE80211B;
> > + modes[2].mode = MODE_IEEE80211A;
> > +
> > + max_r = ARRAY_SIZE(sc->rates);
> > + max_c = ARRAY_SIZE(sc->channels);
> > +
> > + for (i = 0; i <= 2; i++) {
> > + struct ieee80211_hw_mode *mode = &modes[i];
> > + const struct ath5k_rate_table *hw_rates;
> > +
> > + if (i == 0) {
> > + modes[0].rates = sc->rates;
> > + modes->channels = sc->channels;
> > + } else {
> > + struct ieee80211_hw_mode *prev_mode = &modes[i-1];
> > + int prev_num_r = prev_mode->num_rates;
> > + int prev_num_c = prev_mode->num_channels;
> > + mode->rates = &prev_mode->rates[prev_num_r];
> > + mode->channels = &prev_mode->channels[prev_num_c];
> > }
> > +
> > + hw_rates = ath5k_hw_get_rate_table(ah, mode->mode);
> > + mode->num_rates = ath_copy_rates(mode->rates, hw_rates,
> > + max_r);
> > + mode->num_channels = ath_copy_channels(ah, mode->channels,
> > + mode->mode, max_c);
> > + max_r -= mode->num_rates;
> > + max_c -= mode->num_channels;
> > + }
> > +
> > + switch (ah->ah_version) {
> > + case AR5K_AR5210:
> > + REGISTER_MODE(MODE_IEEE80211A);
> > + break;
> > + case AR5K_AR5211:
> > + REGISTER_MODE(MODE_IEEE80211B);
> > + REGISTER_MODE(MODE_IEEE80211A);
> > + break;
> > + case AR5K_AR5212:
> > + REGISTER_MODE(MODE_IEEE80211G);
> > + if (!ah->ah_single_chip)
> > + REGISTER_MODE(MODE_IEEE80211A);
> > + break;
> > + }
> > ath_dump_modes(modes);
> >
> > return 0;
> >
>
>
> --
> GPG ID: 0xD21DB2DB
> As you read this post global entropy rises. Have Fun ;-)
> Nick
>
--
GPG ID: 0xD21DB2DB
As you read this post global entropy rises. Have Fun ;-)
Nick
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 5/5] ath5k: Fix and clean mode initialization, prefer G for AR5212
2007-10-13 9:30 ` Nick Kossifidis
@ 2007-10-13 9:38 ` Jiri Slaby
2007-10-13 20:08 ` Luis R. Rodriguez
0 siblings, 1 reply; 15+ messages in thread
From: Jiri Slaby @ 2007-10-13 9:38 UTC (permalink / raw)
To: Nick Kossifidis; +Cc: Luis R. Rodriguez, John Linville, linux-wireless
On 10/13/2007 11:30 AM, Nick Kossifidis wrote:
> I mean something like this...
>
> if(test_bit(MODE_IEEE80211B,hal->ah_capabilities.cap_mode)) {
this reminds me I wanted to get rid of locked set_bit and use __set_bit instead ;-).
> REGISTER_MODE(MODE_IEEE80211B);
> }
or do it directly in register_mode. I'm for making function from the macro in
that case.
regards,
--
Jiri Slaby (jirislaby@gmail.com)
Faculty of Informatics, Masaryk University
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 5/5] ath5k: Fix and clean mode initialization, prefer G for AR5212
2007-10-13 9:38 ` Jiri Slaby
@ 2007-10-13 20:08 ` Luis R. Rodriguez
2007-10-13 21:51 ` Jiri Slaby
0 siblings, 1 reply; 15+ messages in thread
From: Luis R. Rodriguez @ 2007-10-13 20:08 UTC (permalink / raw)
To: Jiri Slaby; +Cc: Nick Kossifidis, John Linville, linux-wireless
On Sat, Oct 13, 2007 at 11:38:28AM +0200, Jiri Slaby wrote:
> On 10/13/2007 11:30 AM, Nick Kossifidis wrote:
> > I mean something like this...
> >
> > if(test_bit(MODE_IEEE80211B,hal->ah_capabilities.cap_mode)) {
>
> this reminds me I wanted to get rid of locked set_bit and use __set_bit
> instead ;-).
>
> > REGISTER_MODE(MODE_IEEE80211B);
> > }
>
> or do it directly in register_mode. I'm for making function from the macro in
> that case.
OK well here's another patch taking these things into consideration.
Resulting ath_dump_modes()'s is below as well for AR5210, AR5211, and
AR5212.
----------------------- AR5210 --------------------------------------
Mode 0: channels 0, rates 0
channels:
rates:
Mode 1: channels 0, rates 0
channels:
rates:
Mode 2: channels 8, rates 8
channels:
36 5180 0140 0007
40 5200 0140 0007
44 5220 0140 0007
48 5240 0140 0007
52 5260 0140 0007
56 5280 0140 0007
60 5300 0140 0007
64 5320 0140 0007
rates:
60 000b 0132 0000
90 000f 0030 0000
120 000a 0132 0000
180 000e 0030 0000
240 0009 0132 0000
360 000d 0030 0000
480 0008 0030 0000
540 000c 0030 0000
phy4: Selected rate control algorithm 'simple'
ath_pci 0000:15:00.0: AR5210 chip found: mac 0.0 phy 0.3
----------------------- AR5211 --------------------------------------
Mode 0: channels 0, rates 0
channels:
rates:
Mode 1: channels 11, rates 4
channels:
1 2412 00a0 0007
2 2417 00a0 0007
3 2422 00a0 0007
4 2427 00a0 0007
5 2432 00a0 0007
6 2437 00a0 0007
7 2442 00a0 0007
8 2447 00a0 0007
9 2452 00a0 0007
10 2457 00a0 0007
11 2462 00a0 0007
rates:
10 001b 0152 0000
20 001a 0056 0000
55 0019 0054 0000
110 0018 0054 0000
Mode 2: channels 13, rates 8
channels:
36 5180 0140 0007
40 5200 0140 0007
44 5220 0140 0007
48 5240 0140 0007
52 5260 0140 0007
56 5280 0140 0007
60 5300 0140 0007
64 5320 0140 0007
149 5745 0140 0007
153 5765 0140 0007
157 5785 0140 0007
161 5805 0140 0007
165 5825 0140 0007
rates:
60 000b 0132 0000
90 000f 0030 0000
120 000a 0132 0000
180 000e 0030 0000
240 0009 0132 0000
360 000d 0030 0000
480 0008 0030 0000
540 000c 0030 0000
phy3: Selected rate control algorithm 'simple'
ath_pci 0000:15:00.0: AR5211 chip found: mac 4.4 phy 3.0
----------------------- AR5212 --------------------------------------
Mode 0: channels 11, rates 10
channels:
1 2412 00c0 0007
2 2417 00c0 0007
3 2422 00c0 0007
4 2427 00c0 0007
5 2432 00c0 0007
6 2437 00c0 0007
7 2442 00c0 0007
8 2447 00c0 0007
9 2452 00c0 0007
10 2457 00c0 0007
11 2462 00c0 0007
rates:
10 001b 0152 0000
20 001a 0156 0000
55 0019 0156 0000
110 0018 0156 0000
120 000a 0131 0000
180 000e 0031 0000
240 0009 0131 0000
360 000d 0031 0000
480 0008 0031 0000
540 000c 0031 0000
Mode 1: channels 11, rates 4
channels:
1 2412 00a0 0000
2 2417 00a0 0000
3 2422 00a0 0000
4 2427 00a0 0000
5 2432 00a0 0000
6 2437 00a0 0000
7 2442 00a0 0000
8 2447 00a0 0000
9 2452 00a0 0000
10 2457 00a0 0000
11 2462 00a0 0000
rates:
10 001b 0040 0000
20 001a 0044 0000
55 0019 0044 0000
110 0018 0044 0000
Mode 2: channels 13, rates 8
channels:
36 5180 0140 0007
40 5200 0140 0007
44 5220 0140 0007
48 5240 0140 0007
52 5260 0140 0007
56 5280 0140 0007
60 5300 0140 0007
64 5320 0140 0007
149 5745 0140 0007
153 5765 0140 0007
157 5785 0140 0007
161 5805 0140 0007
165 5825 0140 0007
rates:
60 000b 0132 0000
90 000f 0030 0000
120 000a 0132 0000
180 000e 0030 0000
240 0009 0132 0000
360 000d 0030 0000
480 0008 0030 0000
540 000c 0030 0000
phy1: Selected rate control algorithm 'simple'
ath_pci 0000:15:00.0: AR5212 chip found: mac 5.5 phy 4.3
---------------------------------------------------------------------
[PATCH 5/5] ath5k: Fix and clean mode initialization, prefer G for AR5212
Currently you get locked on B mode with AR5212s. This could be partly
mac80211's fault with a recent regression introduced but ath5k mode
initialization right now is pretty sloppy. For AR5212s we also currently
start scanning in 5GHz. I've made the mode initialization on ath5k a bit
clearer and only am registering G mode now instead of both B and G for
AR5212s. We now also check eeprom reading for capabilities before registering
a specific mode. For AR5212 I get reliable rates only up to 18Mbps for a
throughput of up to 5 Mbits/sec. At least now we can work and test the
other rates again.
Also use a static driver specific NUM_DRIVER_MODES instead of mac80211's
NUM_IEEE80211_MODES.
This patch has been tested on AR5210, AR5211 and AR5212.
Note: mac80211 simple rate algo throws us to 1Mbps after assoc, this is by
design. I recommend users to set rate to 11M for now after assoc.
Changes to base.[ch]
Changes-licensed-under: 3-clause-BSD
Changes to ath5k.h
Changes-licensed-under: ISC
Signed-off-by: Luis R. Rodriguez <mcgrof@gmail.com>
---
drivers/net/wireless/ath5k/ath5k.h | 5 +-
drivers/net/wireless/ath5k/base.c | 125 +++++++++++++++++++++---------------
drivers/net/wireless/ath5k/base.h | 5 +-
3 files changed, 79 insertions(+), 56 deletions(-)
diff --git a/drivers/net/wireless/ath5k/ath5k.h b/drivers/net/wireless/ath5k/ath5k.h
index 09c920b..bcf1041 100644
--- a/drivers/net/wireless/ath5k/ath5k.h
+++ b/drivers/net/wireless/ath5k/ath5k.h
@@ -214,6 +214,9 @@ enum ath5k_vendor_mode {
MODE_ATHEROS_TURBOG
};
+/* Number of supported mac80211 enum ieee80211_phymode modes by this driver */
+#define NUM_DRIVER_MODES 3
+
/* adding this flag to rate_code enables short preamble, see ar5212_reg.h */
#define AR5K_SET_SHORT_PREAMBLE 0x04
@@ -865,7 +868,7 @@ struct ath5k_capabilities {
* Supported PHY modes
* (ie. CHANNEL_A, CHANNEL_B, ...)
*/
- DECLARE_BITMAP(cap_mode, NUM_IEEE80211_MODES);
+ DECLARE_BITMAP(cap_mode, NUM_DRIVER_MODES);
/*
* Frequency range (without regulation restrictions)
diff --git a/drivers/net/wireless/ath5k/base.c b/drivers/net/wireless/ath5k/base.c
index 18ee995..db37ceb 100644
--- a/drivers/net/wireless/ath5k/base.c
+++ b/drivers/net/wireless/ath5k/base.c
@@ -1896,7 +1896,7 @@ static void ath_dump_modes(struct ieee80211_hw_mode *modes)
{
unsigned int m, i;
- for (m = 0; m < NUM_IEEE80211_MODES; m++) {
+ for (m = 0; m < NUM_DRIVER_MODES; m++) {
printk(KERN_DEBUG "Mode %u: channels %d, rates %d\n", m,
modes[m].num_channels, modes[m].num_rates);
printk(KERN_DEBUG " channels:\n");
@@ -1919,71 +1919,92 @@ static void ath_dump_modes(struct ieee80211_hw_mode *modes)
static inline void ath_dump_modes(struct ieee80211_hw_mode *modes) {}
#endif
+static inline int ath5k_register_mode(struct ieee80211_hw *hw, u8 m)
+{
+ struct ath_softc *sc = hw->priv;
+ struct ieee80211_hw_mode *modes = sc->modes;
+ int i, ret;
+
+ for (i = 0; i < NUM_DRIVER_MODES; i++) {
+ if (modes[i].mode != m || !modes[i].num_channels)
+ continue;
+ ret = ieee80211_register_hwmode(hw, &modes[i]);
+ if (ret) {
+ printk(KERN_ERR "can't register hwmode %u\n", m);
+ return ret;
+ }
+ return 0;
+ }
+ return 1;
+}
+
+/* Only tries to register modes our EEPROM says it can support */
+#define REGISTER_MODE(m) do { \
+ if (test_bit(m, ah->ah_capabilities.cap_mode)) { \
+ ret = ath5k_register_mode(hw, m); \
+ if (ret) \
+ return ret; \
+ } \
+} while (0) \
+
static int ath_getchannels(struct ieee80211_hw *hw)
{
struct ath_softc *sc = hw->priv;
struct ath_hw *ah = sc->ah;
struct ieee80211_hw_mode *modes = sc->modes;
- unsigned int i, max;
+ unsigned int i, max_r, max_c;
int ret;
- enum {
- A = MODE_IEEE80211A,
- B = MODE_IEEE80211G, /* this is not a typo, but workaround */
- G = MODE_IEEE80211B, /* to prefer g over b */
- T = MODE_ATHEROS_TURBO,
- TG = MODE_ATHEROS_TURBOG,
- };
BUILD_BUG_ON(ARRAY_SIZE(sc->modes) < 3);
ah->ah_country_code = countrycode;
- modes[A].mode = MODE_IEEE80211A;
- modes[B].mode = MODE_IEEE80211B;
- modes[G].mode = MODE_IEEE80211G;
-
- max = ARRAY_SIZE(sc->rates);
- modes[A].rates = sc->rates;
- max -= modes[A].num_rates = ath_copy_rates(modes[A].rates,
- ath5k_hw_get_rate_table(ah, MODE_IEEE80211A), max);
- modes[B].rates = &modes[A].rates[modes[A].num_rates];
- max -= modes[B].num_rates = ath_copy_rates(modes[B].rates,
- ath5k_hw_get_rate_table(ah, MODE_IEEE80211B), max);
- modes[G].rates = &modes[B].rates[modes[B].num_rates];
- max -= modes[G].num_rates = ath_copy_rates(modes[G].rates,
- ath5k_hw_get_rate_table(ah, MODE_IEEE80211G), max);
-
- if (!max)
- printk(KERN_WARNING "yet another rates found, but there is not "
- "sufficient space to store them\n");
-
- max = ARRAY_SIZE(sc->channels);
- modes[A].channels = sc->channels;
- max -= modes[A].num_channels = ath_copy_channels(ah, modes[A].channels,
- MODE_IEEE80211A, max);
- modes[B].channels = &modes[A].channels[modes[A].num_channels];
- max -= modes[B].num_channels = ath_copy_channels(ah, modes[B].channels,
- MODE_IEEE80211B, max);
- modes[G].channels = &modes[B].channels[modes[B].num_channels];
- max -= modes[G].num_channels = ath_copy_channels(ah, modes[G].channels,
- MODE_IEEE80211G, max);
-
- if (!max)
- printk(KERN_WARNING "yet another modes found, but there is not "
- "sufficient space to store them\n");
-
- for (i = 0; i < ARRAY_SIZE(sc->modes); i++)
- if (modes[i].num_channels) {
- ret = ieee80211_register_hwmode(hw, &modes[i]);
- if (ret) {
- printk(KERN_ERR "can't register hwmode %u\n",i);
- goto err;
- }
+ /* The order here does not matter */
+ modes[0].mode = MODE_IEEE80211G;
+ modes[1].mode = MODE_IEEE80211B;
+ modes[2].mode = MODE_IEEE80211A;
+
+ max_r = ARRAY_SIZE(sc->rates);
+ max_c = ARRAY_SIZE(sc->channels);
+
+ for (i = 0; i < NUM_DRIVER_MODES; i++) {
+ struct ieee80211_hw_mode *mode = &modes[i];
+ const struct ath5k_rate_table *hw_rates;
+
+ if (i == 0) {
+ modes[0].rates = sc->rates;
+ modes->channels = sc->channels;
+ } else {
+ struct ieee80211_hw_mode *prev_mode = &modes[i-1];
+ int prev_num_r = prev_mode->num_rates;
+ int prev_num_c = prev_mode->num_channels;
+ mode->rates = &prev_mode->rates[prev_num_r];
+ mode->channels = &prev_mode->channels[prev_num_c];
}
+
+ hw_rates = ath5k_hw_get_rate_table(ah, mode->mode);
+ mode->num_rates = ath_copy_rates(mode->rates, hw_rates,
+ max_r);
+ mode->num_channels = ath_copy_channels(ah, mode->channels,
+ mode->mode, max_c);
+ max_r -= mode->num_rates;
+ max_c -= mode->num_channels;
+ }
+
+ /* We try to register all modes this driver supports. We don't bother
+ * with MODE_IEEE80211B for AR5212 as MODE_IEEE80211G already accounts
+ * for that as per mac80211. Then, REGISTER_MODE() will will actually
+ * check the eeprom reading for more reliable capability information.
+ * Order matters here as per mac80211's latest preference. This will
+ * all hopefullly soon go away. */
+
+ REGISTER_MODE(MODE_IEEE80211G);
+ if (ah->ah_version != AR5K_AR5212)
+ REGISTER_MODE(MODE_IEEE80211B);
+ REGISTER_MODE(MODE_IEEE80211A);
+
ath_dump_modes(modes);
- return 0;
-err:
return ret;
}
diff --git a/drivers/net/wireless/ath5k/base.h b/drivers/net/wireless/ath5k/base.h
index 4a624cc..390d3d7 100644
--- a/drivers/net/wireless/ath5k/base.h
+++ b/drivers/net/wireless/ath5k/base.h
@@ -116,7 +116,6 @@ struct ath_txq {
#define ATH_CHAN_MAX (14+14+14+252+20) /* XXX what's the max? */
#endif
-
/* Software Carrier, keeps track of the driver state
* associated with an instance of a device */
struct ath_softc {
@@ -126,9 +125,9 @@ struct ath_softc {
struct ieee80211_tx_queue_stats tx_stats;
struct ieee80211_low_level_stats ll_stats;
struct ieee80211_hw *hw; /* IEEE 802.11 common */
- struct ieee80211_hw_mode modes[NUM_IEEE80211_MODES];
+ struct ieee80211_hw_mode modes[NUM_DRIVER_MODES];
struct ieee80211_channel channels[ATH_CHAN_MAX];
- struct ieee80211_rate rates[AR5K_MAX_RATES * NUM_IEEE80211_MODES];
+ struct ieee80211_rate rates[AR5K_MAX_RATES * NUM_DRIVER_MODES];
enum ieee80211_if_types opmode;
struct ath_hw *ah; /* Atheros HW */
^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH 5/5] ath5k: Fix and clean mode initialization, prefer G for AR5212
2007-10-13 20:08 ` Luis R. Rodriguez
@ 2007-10-13 21:51 ` Jiri Slaby
0 siblings, 0 replies; 15+ messages in thread
From: Jiri Slaby @ 2007-10-13 21:51 UTC (permalink / raw)
To: Luis R. Rodriguez; +Cc: Nick Kossifidis, John Linville, linux-wireless
On 10/13/2007 10:08 PM, Luis R. Rodriguez wrote:
> [PATCH 5/5] ath5k: Fix and clean mode initialization, prefer G for AR5212
[...]
> diff --git a/drivers/net/wireless/ath5k/base.c b/drivers/net/wireless/ath5k/base.c
> index 18ee995..db37ceb 100644
> --- a/drivers/net/wireless/ath5k/base.c
> +++ b/drivers/net/wireless/ath5k/base.c
> @@ -1896,7 +1896,7 @@ static void ath_dump_modes(struct ieee80211_hw_mode *modes)
> {
> unsigned int m, i;
>
> - for (m = 0; m < NUM_IEEE80211_MODES; m++) {
> + for (m = 0; m < NUM_DRIVER_MODES; m++) {
> printk(KERN_DEBUG "Mode %u: channels %d, rates %d\n", m,
> modes[m].num_channels, modes[m].num_rates);
> printk(KERN_DEBUG " channels:\n");
> @@ -1919,71 +1919,92 @@ static void ath_dump_modes(struct ieee80211_hw_mode *modes)
> static inline void ath_dump_modes(struct ieee80211_hw_mode *modes) {}
> #endif
>
> +static inline int ath5k_register_mode(struct ieee80211_hw *hw, u8 m)
> +{
> + struct ath_softc *sc = hw->priv;
> + struct ieee80211_hw_mode *modes = sc->modes;
> + int i, ret;
unsigned is usually used for positive iterators, it generates better code almost
for all platforms.
> +
> + for (i = 0; i < NUM_DRIVER_MODES; i++) {
> + if (modes[i].mode != m || !modes[i].num_channels)
> + continue;
> + ret = ieee80211_register_hwmode(hw, &modes[i]);
> + if (ret) {
> + printk(KERN_ERR "can't register hwmode %u\n", m);
> + return ret;
> + }
> + return 0;
> + }
Maybe we should oops here, since it is a bug to get that far. I think, you
should put BUG() here to not mess userspace with positive retvals.
> + return 1;
> +}
> +
> +/* Only tries to register modes our EEPROM says it can support */
> +#define REGISTER_MODE(m) do { \
> + if (test_bit(m, ah->ah_capabilities.cap_mode)) { \
> + ret = ath5k_register_mode(hw, m); \
> + if (ret) \
> + return ret; \
> + } \
I though you will put the test inside the new ath5k_register_mode function too
(and return 0 in the case when unsupported) and get rid of the macro. But ok,
not global macro we can tolerate return-ing from it :).
knowing to be a pain in the ass,
--
Jiri Slaby (jirislaby@gmail.com)
Faculty of Informatics, Masaryk University
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 4/5] Add extensive documenation for the atheros bssid_mask
2007-10-12 15:07 ` [PATCH 4/5] Add extensive documenation for the atheros bssid_mask Luis R. Rodriguez
2007-10-12 15:07 ` [PATCH 5/5] ath5k: Fix and clean mode initialization, prefer G for AR5212 Luis R. Rodriguez
@ 2007-10-16 15:07 ` Randy Dunlap
1 sibling, 0 replies; 15+ messages in thread
From: Randy Dunlap @ 2007-10-16 15:07 UTC (permalink / raw)
To: Luis R. Rodriguez
Cc: John Linville, linux-wireless, Jiri Slaby, Nick Kossifidis
On Fri, 12 Oct 2007 11:07:09 -0400 Luis R. Rodriguez wrote:
> Add extensive documenation for the atheros bssid_mask.
> Credit to David Kimdon for figuring this out. I am
> just documenting it.
>
> Signed-off-by: Luis R. Rodriguez <mcgrof@gmail.com>
> ---
> 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/hw.c b/drivers/net/wireless/ath5k/hw.c
> index b106ead..fc00667 100644
> --- a/drivers/net/wireless/ath5k/hw.c
> +++ b/drivers/net/wireless/ath5k/hw.c
> @@ -2323,9 +2323,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
> +/**
"/**" indicates kernel-doc notation, so I'll make a few comments here.
> + * ath5k_hw_set_bssid_mask - set common bits we should listen to
The list of parameters (@hal & @mask) need to be immediatly after
the function name/short description line, followed by a blank (" *")
line, then any other function comments/documentation that you want
to include here.
> + *
> + * 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!!!
> + *
> */
Thanks.
---
~Randy
^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2007-10-16 15:10 UTC | newest]
Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2007-10-12 15:00 [PATCH 0/5] ath5k: promiscuous bug, multicast, modes and docs Luis R. Rodriguez
2007-10-12 15:03 ` [PATCH 1/5] ath5k: Fix a bug which pushed us to enable promiscuous Luis R. Rodriguez
2007-10-12 15:04 ` [PATCH 2/5] Add proper support for multicast Luis R. Rodriguez
2007-10-12 15:05 ` [PATCH 3/5] Don't read AR5K_RAC_PISR on AR5210, document ath5k_int Luis R. Rodriguez
2007-10-12 15:07 ` [PATCH 4/5] Add extensive documenation for the atheros bssid_mask Luis R. Rodriguez
2007-10-12 15:07 ` [PATCH 5/5] ath5k: Fix and clean mode initialization, prefer G for AR5212 Luis R. Rodriguez
2007-10-12 21:33 ` Jiri Slaby
2007-10-12 22:37 ` Luis R. Rodriguez
2007-10-13 7:34 ` Jiri Slaby
2007-10-13 9:21 ` Nick Kossifidis
2007-10-13 9:30 ` Nick Kossifidis
2007-10-13 9:38 ` Jiri Slaby
2007-10-13 20:08 ` Luis R. Rodriguez
2007-10-13 21:51 ` Jiri Slaby
2007-10-16 15:07 ` [PATCH 4/5] Add extensive documenation for the atheros bssid_mask Randy Dunlap
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.