Implement a common function to access the cycle counters (profile counters) for
use in ath5k and ath9k. This is necessary because we want to access the cycle
counters from different places: ANI uses it in it's algorithm to calculate the
"listen time" and at the same time we want to show it for debugging purposes
and add it to the "survey" command later.
This is a very simple implementation, which resets the HW counters to zero
after every read (to avoid overflows) and currently takes care of two different
counters, one for ANI and one for survey. If additional counters should be
required they would have to be added by hardcoding them in the function. This
is not the most flexible way to deal with this, but it's easy and simple and i
believe should be sufficient.
Users have to hold the lock while accessing the counters.
Signed-off-by: Bruno Randolf <[email protected]>
---
drivers/net/wireless/ath/ath.h | 23 +++++++++++++++++++++++
drivers/net/wireless/ath/hw.c | 40 ++++++++++++++++++++++++++++++++++++++++
drivers/net/wireless/ath/reg.h | 11 +++++++++++
3 files changed, 74 insertions(+), 0 deletions(-)
diff --git a/drivers/net/wireless/ath/ath.h b/drivers/net/wireless/ath/ath.h
index dd236c3..699c904 100644
--- a/drivers/net/wireless/ath/ath.h
+++ b/drivers/net/wireless/ath/ath.h
@@ -97,6 +97,13 @@ enum ath_cipher {
ATH_CIPHER_MIC = 127
};
+struct ath_cycle_counters {
+ u32 cycles;
+ u32 rx_busy; /* register is called "rx clear" but it's the inverse */
+ u32 rx_frame;
+ u32 tx_frame;
+};
+
/**
* struct ath_ops - Register read/write operations
*
@@ -148,6 +155,10 @@ struct ath_common {
DECLARE_BITMAP(tkip_keymap, ATH_KEYMAX);
enum ath_crypt_caps crypt_caps;
+ struct ath_cycle_counters cc_ani;
+ struct ath_cycle_counters cc_survey;
+ spinlock_t cc_lock;
+
struct ath_regulatory regulatory;
const struct ath_ops *ops;
const struct ath_bus_ops *bus_ops;
@@ -165,4 +176,16 @@ int ath_key_config(struct ath_common *common,
struct ieee80211_key_conf *key);
bool ath_hw_keyreset(struct ath_common *common, u16 entry);
+void ath_hw_cycle_counters_update(struct ath_common *common);
+
+static inline void ath_hw_cycle_counters_lock(struct ath_common *common)
+{
+ spin_lock_bh(&common->cc_lock);
+}
+
+static inline void ath_hw_cycle_counters_unlock(struct ath_common *common)
+{
+ spin_unlock_bh(&common->cc_lock);
+}
+
#endif /* ATH_H */
diff --git a/drivers/net/wireless/ath/hw.c b/drivers/net/wireless/ath/hw.c
index a8f81ea..f26730f 100644
--- a/drivers/net/wireless/ath/hw.c
+++ b/drivers/net/wireless/ath/hw.c
@@ -124,3 +124,43 @@ void ath_hw_setbssidmask(struct ath_common *common)
REG_WRITE(ah, get_unaligned_le16(common->bssidmask + 4), AR_BSSMSKU);
}
EXPORT_SYMBOL(ath_hw_setbssidmask);
+
+/**
+ * ath_hw_cycle_counters_update - common function to update cycle counters
+ *
+ * @common: the ath_common struct for the device.
+ *
+ * This function is used to update all cycle counters in one place.
+ * It has to be called while holding common->cc_lock!
+ */
+void ath_hw_cycle_counters_update(struct ath_common *common)
+{
+ u32 cycles, busy, rx, tx;
+
+ /* freeze */
+ REG_WRITE(common, AR_MIBC_FMC, AR_MIBC);
+ /* read */
+ cycles = REG_READ(common, AR_CCCNT);
+ busy = REG_READ(common, AR_RCCNT);
+ rx = REG_READ(common, AR_RFCNT);
+ tx = REG_READ(common, AR_TFCNT);
+ /* clear */
+ REG_WRITE(common, 0, AR_CCCNT);
+ REG_WRITE(common, 0, AR_RFCNT);
+ REG_WRITE(common, 0, AR_RCCNT);
+ REG_WRITE(common, 0, AR_TFCNT);
+ /* unfreeze */
+ REG_WRITE(common, 0, AR_MIBC);
+
+ /* update all cycle counters here */
+ common->cc_ani.cycles += cycles;
+ common->cc_ani.rx_busy += busy;
+ common->cc_ani.rx_frame += rx;
+ common->cc_ani.tx_frame += tx;
+
+ common->cc_survey.cycles += cycles;
+ common->cc_survey.rx_busy += busy;
+ common->cc_survey.rx_frame += rx;
+ common->cc_survey.tx_frame += tx;
+}
+EXPORT_SYMBOL(ath_hw_cycle_counters_update);
diff --git a/drivers/net/wireless/ath/reg.h b/drivers/net/wireless/ath/reg.h
index e798ef4..298e53f 100644
--- a/drivers/net/wireless/ath/reg.h
+++ b/drivers/net/wireless/ath/reg.h
@@ -17,6 +17,12 @@
#ifndef ATH_REGISTERS_H
#define ATH_REGISTERS_H
+#define AR_MIBC 0x0040
+#define AR_MIBC_COW 0x00000001
+#define AR_MIBC_FMC 0x00000002
+#define AR_MIBC_CMC 0x00000004
+#define AR_MIBC_MCS 0x00000008
+
/*
* BSSID mask registers. See ath_hw_set_bssid_mask()
* for detailed documentation about these registers.
@@ -24,6 +30,11 @@
#define AR_BSSMSKL 0x80e0
#define AR_BSSMSKU 0x80e4
+#define AR_TFCNT 0x80ec
+#define AR_RFCNT 0x80f0
+#define AR_RCCNT 0x80f4
+#define AR_CCCNT 0x80f8
+
#define AR_KEYTABLE_0 0x8800
#define AR_KEYTABLE(_n) (AR_KEYTABLE_0 + ((_n)*32))
#define AR_KEY_CACHE_SIZE 128
On Wed October 6 2010 05:14:07 Luis R. Rodriguez wrote:
> On Tue, Oct 05, 2010 at 02:55:21AM -0700, Bruno Randolf wrote:
> > Update ath9k to use the common cycle counters.
> >
> > This also includes other changes from Felix Fietkaus "[PATCH 2/4]
> > ath9k_hw: merge codepaths that access the cycle counter registers".
> >
> > Compile tested only. ath9k team please review...
>
> Can you move locking to be specific to the core part of
> the driver instead of the ath9k_hw ?
i'm not sure if i understand what you mean. locking is now implemented in
ath_common.
> We try to avoid locking
> all over the hardware code. Even ath9k_iowrite32() and
> ath9k_ioread32() are core driver helpers, not part of the
> hw code.
but, we need to make sure the cycle counters are always accessed exclusively.
e.g. ANI runs from a timer (in ath9k, afaik), and updates the cycle counters.
at the same time a user might issue a survey, or your debug print may be
executed, which would want to access the same counters. we need to make sure
the counters are not updated while they are read, etc...
bruno
Update ath5k to use the common cycle counters in it's ANI implementation.
Signed-off-by: Bruno Randolf <[email protected]>
---
drivers/net/wireless/ath/ath5k/ani.c | 34 ++++++++++++--------------------
drivers/net/wireless/ath/ath5k/ani.h | 5 +----
drivers/net/wireless/ath/ath5k/debug.c | 21 ++++++++++----------
3 files changed, 25 insertions(+), 35 deletions(-)
diff --git a/drivers/net/wireless/ath/ath5k/ani.c b/drivers/net/wireless/ath/ath5k/ani.c
index e4a5f04..67b41d9 100644
--- a/drivers/net/wireless/ath/ath5k/ani.c
+++ b/drivers/net/wireless/ath/ath5k/ani.c
@@ -360,36 +360,28 @@ ath5k_ani_lower_immunity(struct ath5k_hw *ah, struct ath5k_ani_state *as)
* Return an approximation of the time spent "listening" in milliseconds (ms)
* since the last call of this function by deducting the cycles spent
* transmitting and receiving from the total cycle count.
- * Save profile count values for debugging/statistics and because we might want
- * to use them later.
- *
- * We assume no one else clears these registers!
*/
static int
ath5k_hw_ani_get_listen_time(struct ath5k_hw *ah, struct ath5k_ani_state *as)
{
+ struct ath_common *common = ath5k_hw_common(ah);
int listen;
- /* freeze */
- ath5k_hw_reg_write(ah, AR5K_MIBC_FMC, AR5K_MIBC);
- /* read */
- as->pfc_cycles = ath5k_hw_reg_read(ah, AR5K_PROFCNT_CYCLE);
- as->pfc_busy = ath5k_hw_reg_read(ah, AR5K_PROFCNT_RXCLR);
- as->pfc_tx = ath5k_hw_reg_read(ah, AR5K_PROFCNT_TX);
- as->pfc_rx = ath5k_hw_reg_read(ah, AR5K_PROFCNT_RX);
- /* clear */
- ath5k_hw_reg_write(ah, 0, AR5K_PROFCNT_TX);
- ath5k_hw_reg_write(ah, 0, AR5K_PROFCNT_RX);
- ath5k_hw_reg_write(ah, 0, AR5K_PROFCNT_RXCLR);
- ath5k_hw_reg_write(ah, 0, AR5K_PROFCNT_CYCLE);
- /* un-freeze */
- ath5k_hw_reg_write(ah, 0, AR5K_MIBC);
+ ath_hw_cycle_counters_lock(common);
+ ath_hw_cycle_counters_update(common);
/* TODO: where does 44000 come from? (11g clock rate?) */
- listen = (as->pfc_cycles - as->pfc_rx - as->pfc_tx) / 44000;
+ listen = (common->cc_ani.cycles - common->cc_ani.rx_frame -
+ common->cc_ani.tx_frame) / 44000;
+
+ if (common->cc_ani.cycles == 0 || listen < 0)
+ listen = 0;
+
+ memcpy(&as->last_cc, &common->cc_ani, sizeof(struct ath_cycle_counters));
+ memset(&common->cc_ani, 0, sizeof(struct ath_cycle_counters));
+
+ ath_hw_cycle_counters_unlock(common);
- if (as->pfc_cycles == 0 || listen < 0)
- return 0;
return listen;
}
diff --git a/drivers/net/wireless/ath/ath5k/ani.h b/drivers/net/wireless/ath/ath5k/ani.h
index 55cf26d..d0a6640 100644
--- a/drivers/net/wireless/ath/ath5k/ani.h
+++ b/drivers/net/wireless/ath/ath5k/ani.h
@@ -75,10 +75,7 @@ struct ath5k_ani_state {
unsigned int cck_errors;
/* debug/statistics only: numbers from last ANI calibration */
- unsigned int pfc_tx;
- unsigned int pfc_rx;
- unsigned int pfc_busy;
- unsigned int pfc_cycles;
+ struct ath_cycle_counters last_cc;
unsigned int last_listen;
unsigned int last_ofdm_errors;
unsigned int last_cck_errors;
diff --git a/drivers/net/wireless/ath/ath5k/debug.c b/drivers/net/wireless/ath/ath5k/debug.c
index 0f06e84..f0f8e43 100644
--- a/drivers/net/wireless/ath/ath5k/debug.c
+++ b/drivers/net/wireless/ath/ath5k/debug.c
@@ -711,20 +711,21 @@ static ssize_t read_file_ani(struct file *file, char __user *user_buf,
len += snprintf(buf+len, sizeof(buf)-len,
"beacon RSSI average:\t%d\n",
sc->ah->ah_beacon_rssi_avg.avg);
+
+#define CC_PRINT(_struct, _field) \
+ _struct._field, \
+ _struct.cycles > 0 ? \
+ _struct._field*100/_struct.cycles : 0
+
len += snprintf(buf+len, sizeof(buf)-len, "profcnt tx\t\t%u\t(%d%%)\n",
- as->pfc_tx,
- as->pfc_cycles > 0 ?
- as->pfc_tx*100/as->pfc_cycles : 0);
+ CC_PRINT(as->last_cc, tx_frame));
len += snprintf(buf+len, sizeof(buf)-len, "profcnt rx\t\t%u\t(%d%%)\n",
- as->pfc_rx,
- as->pfc_cycles > 0 ?
- as->pfc_rx*100/as->pfc_cycles : 0);
+ CC_PRINT(as->last_cc, rx_frame));
len += snprintf(buf+len, sizeof(buf)-len, "profcnt busy\t\t%u\t(%d%%)\n",
- as->pfc_busy,
- as->pfc_cycles > 0 ?
- as->pfc_busy*100/as->pfc_cycles : 0);
+ CC_PRINT(as->last_cc, rx_busy));
+#undef CC_PRINT
len += snprintf(buf+len, sizeof(buf)-len, "profcnt cycles\t\t%u\n",
- as->pfc_cycles);
+ as->last_cc.cycles);
len += snprintf(buf+len, sizeof(buf)-len,
"listen time\t\t%d\tlast: %d\n",
as->listen_time, as->last_listen);
Update ath9k to use the common cycle counters.
This also includes other changes from Felix Fietkaus "[PATCH 2/4] ath9k_hw:
merge codepaths that access the cycle counter registers".
Compile tested only. ath9k team please review...
Signed-off-by: Felix Fietkau <[email protected]>
Signed-off-by: Bruno Randolf <[email protected]>
---
drivers/net/wireless/ath/ath9k/ani.c | 87 +++------------------------
drivers/net/wireless/ath/ath9k/ani.h | 6 --
drivers/net/wireless/ath/ath9k/ar5008_phy.c | 9 +--
drivers/net/wireless/ath/ath9k/ar9003_phy.c | 18 +++---
4 files changed, 22 insertions(+), 98 deletions(-)
diff --git a/drivers/net/wireless/ath/ath9k/ani.c b/drivers/net/wireless/ath/ath9k/ani.c
index 0496f96..0c0d01d 100644
--- a/drivers/net/wireless/ath/ath9k/ani.c
+++ b/drivers/net/wireless/ath/ath9k/ani.c
@@ -549,47 +549,21 @@ static u8 ath9k_hw_chan_2_clockrate_mhz(struct ath_hw *ah)
static int32_t ath9k_hw_ani_get_listen_time(struct ath_hw *ah)
{
- struct ar5416AniState *aniState;
struct ath_common *common = ath9k_hw_common(ah);
- u32 txFrameCount, rxFrameCount, cycleCount;
- int32_t listenTime;
+ int32_t listen_time;
+ int32_t clock_rate;
- txFrameCount = REG_READ(ah, AR_TFCNT);
- rxFrameCount = REG_READ(ah, AR_RFCNT);
- cycleCount = REG_READ(ah, AR_CCCNT);
+ ath_hw_cycle_counters_lock(common);
+ ath_hw_cycle_counters_update(common);
- aniState = ah->curani;
- if (aniState->cycleCount == 0 || aniState->cycleCount > cycleCount) {
- listenTime = 0;
- ah->stats.ast_ani_lzero++;
- ath_print(common, ATH_DBG_ANI,
- "1st call: aniState->cycleCount=%d\n",
- aniState->cycleCount);
- } else {
- int32_t ccdelta = cycleCount - aniState->cycleCount;
- int32_t rfdelta = rxFrameCount - aniState->rxFrameCount;
- int32_t tfdelta = txFrameCount - aniState->txFrameCount;
- int32_t clock_rate;
+ clock_rate = ath9k_hw_chan_2_clockrate_mhz(ah) * 1000;
+ listen_time = (common->cc_ani.cycles - common->cc_ani.rx_frame -
+ common->cc_ani.tx_frame) / clock_rate;
- /*
- * convert HW counter values to ms using mode
- * specifix clock rate
- */
- clock_rate = ath9k_hw_chan_2_clockrate_mhz(ah) * 1000;;
+ memset(&common->cc_ani, 0, sizeof(struct ath_cycle_counters));
+ ath_hw_cycle_counters_unlock(common);
- listenTime = (ccdelta - rfdelta - tfdelta) / clock_rate;
-
- ath_print(common, ATH_DBG_ANI,
- "cyclecount=%d, rfcount=%d, "
- "tfcount=%d, listenTime=%d CLOCK_RATE=%d\n",
- ccdelta, rfdelta, tfdelta, listenTime, clock_rate);
- }
-
- aniState->cycleCount = cycleCount;
- aniState->txFrameCount = txFrameCount;
- aniState->rxFrameCount = rxFrameCount;
-
- return listenTime;
+ return listen_time;
}
static void ath9k_ani_reset_old(struct ath_hw *ah, bool is_scanning)
@@ -1041,47 +1015,6 @@ void ath9k_hw_disable_mib_counters(struct ath_hw *ah)
}
EXPORT_SYMBOL(ath9k_hw_disable_mib_counters);
-u32 ath9k_hw_GetMibCycleCountsPct(struct ath_hw *ah,
- u32 *rxc_pcnt,
- u32 *rxf_pcnt,
- u32 *txf_pcnt)
-{
- struct ath_common *common = ath9k_hw_common(ah);
- static u32 cycles, rx_clear, rx_frame, tx_frame;
- u32 good = 1;
-
- u32 rc = REG_READ(ah, AR_RCCNT);
- u32 rf = REG_READ(ah, AR_RFCNT);
- u32 tf = REG_READ(ah, AR_TFCNT);
- u32 cc = REG_READ(ah, AR_CCCNT);
-
- if (cycles == 0 || cycles > cc) {
- ath_print(common, ATH_DBG_ANI,
- "cycle counter wrap. ExtBusy = 0\n");
- good = 0;
- } else {
- u32 cc_d = cc - cycles;
- u32 rc_d = rc - rx_clear;
- u32 rf_d = rf - rx_frame;
- u32 tf_d = tf - tx_frame;
-
- if (cc_d != 0) {
- *rxc_pcnt = rc_d * 100 / cc_d;
- *rxf_pcnt = rf_d * 100 / cc_d;
- *txf_pcnt = tf_d * 100 / cc_d;
- } else {
- good = 0;
- }
- }
-
- cycles = cc;
- rx_frame = rf;
- rx_clear = rc;
- tx_frame = tf;
-
- return good;
-}
-
/*
* Process a MIB interrupt. We may potentially be invoked because
* any of the MIB counters overflow/trigger so don't assume we're
diff --git a/drivers/net/wireless/ath/ath9k/ani.h b/drivers/net/wireless/ath/ath9k/ani.h
index f4d0a4d..41f1d63 100644
--- a/drivers/net/wireless/ath/ath9k/ani.h
+++ b/drivers/net/wireless/ath/ath9k/ani.h
@@ -130,9 +130,6 @@ struct ar5416AniState {
int32_t rssiThrLow;
int32_t rssiThrHigh;
u32 noiseFloor;
- u32 txFrameCount;
- u32 rxFrameCount;
- u32 cycleCount;
u32 ofdmPhyErrCount;
u32 cckPhyErrCount;
u32 ofdmPhyErrBase;
@@ -166,8 +163,7 @@ struct ar5416Stats {
void ath9k_enable_mib_counters(struct ath_hw *ah);
void ath9k_hw_disable_mib_counters(struct ath_hw *ah);
-u32 ath9k_hw_GetMibCycleCountsPct(struct ath_hw *ah, u32 *rxc_pcnt,
- u32 *rxf_pcnt, u32 *txf_pcnt);
+void ath9k_hw_update_cycle_counters(struct ath_hw *ah);
void ath9k_hw_ani_setup(struct ath_hw *ah);
void ath9k_hw_ani_init(struct ath_hw *ah);
int ath9k_hw_get_ani_channel_idx(struct ath_hw *ah,
diff --git a/drivers/net/wireless/ath/ath9k/ar5008_phy.c b/drivers/net/wireless/ath/ath9k/ar5008_phy.c
index 3937cfd..74b4213 100644
--- a/drivers/net/wireless/ath/ath9k/ar5008_phy.c
+++ b/drivers/net/wireless/ath/ath9k/ar5008_phy.c
@@ -1227,8 +1227,7 @@ static bool ar5008_hw_ani_control_old(struct ath_hw *ah,
aniState->firstepLevel,
aniState->listenTime);
ath_print(common, ATH_DBG_ANI,
- "cycleCount=%d, ofdmPhyErrCount=%d, cckPhyErrCount=%d\n\n",
- aniState->cycleCount,
+ "ofdmPhyErrCount=%d, cckPhyErrCount=%d\n\n",
aniState->ofdmPhyErrCount,
aniState->cckPhyErrCount);
@@ -1480,15 +1479,13 @@ static bool ar5008_hw_ani_control_new(struct ath_hw *ah,
ath_print(common, ATH_DBG_ANI,
"ANI parameters: SI=%d, ofdmWS=%s FS=%d "
- "MRCcck=%s listenTime=%d CC=%d listen=%d "
+ "MRCcck=%s listenTime=%d "
"ofdmErrs=%d cckErrs=%d\n",
aniState->spurImmunityLevel,
!aniState->ofdmWeakSigDetectOff ? "on" : "off",
aniState->firstepLevel,
!aniState->mrcCCKOff ? "on" : "off",
aniState->listenTime,
- aniState->cycleCount,
- aniState->listenTime,
aniState->ofdmPhyErrCount,
aniState->cckPhyErrCount);
return true;
@@ -1581,8 +1578,6 @@ static void ar5008_hw_ani_cache_ini_regs(struct ath_hw *ah)
aniState->firstepLevel = ATH9K_ANI_FIRSTEP_LVL_NEW;
aniState->ofdmWeakSigDetectOff = !ATH9K_ANI_USE_OFDM_WEAK_SIG;
aniState->mrcCCKOff = true; /* not available on pre AR9003 */
-
- aniState->cycleCount = 0;
}
static void ar5008_hw_set_nf_limits(struct ath_hw *ah)
diff --git a/drivers/net/wireless/ath/ath9k/ar9003_phy.c b/drivers/net/wireless/ath/ath9k/ar9003_phy.c
index a491854..68ef986 100644
--- a/drivers/net/wireless/ath/ath9k/ar9003_phy.c
+++ b/drivers/net/wireless/ath/ath9k/ar9003_phy.c
@@ -1005,15 +1005,13 @@ static bool ar9003_hw_ani_control(struct ath_hw *ah,
ath_print(common, ATH_DBG_ANI,
"ANI parameters: SI=%d, ofdmWS=%s FS=%d "
- "MRCcck=%s listenTime=%d CC=%d listen=%d "
+ "MRCcck=%s listenTime=%d "
"ofdmErrs=%d cckErrs=%d\n",
aniState->spurImmunityLevel,
!aniState->ofdmWeakSigDetectOff ? "on" : "off",
aniState->firstepLevel,
!aniState->mrcCCKOff ? "on" : "off",
aniState->listenTime,
- aniState->cycleCount,
- aniState->listenTime,
aniState->ofdmPhyErrCount,
aniState->cckPhyErrCount);
return true;
@@ -1116,8 +1114,6 @@ static void ar9003_hw_ani_cache_ini_regs(struct ath_hw *ah)
aniState->firstepLevel = ATH9K_ANI_FIRSTEP_LVL_NEW;
aniState->ofdmWeakSigDetectOff = !ATH9K_ANI_USE_OFDM_WEAK_SIG;
aniState->mrcCCKOff = !ATH9K_ANI_ENABLE_MRC_CCK;
-
- aniState->cycleCount = 0;
}
void ar9003_hw_attach_phy_ops(struct ath_hw *ah)
@@ -1232,7 +1228,7 @@ void ar9003_hw_bb_watchdog_read(struct ath_hw *ah)
void ar9003_hw_bb_watchdog_dbg_info(struct ath_hw *ah)
{
struct ath_common *common = ath9k_hw_common(ah);
- u32 rxc_pcnt = 0, rxf_pcnt = 0, txf_pcnt = 0, status;
+ u32 status;
if (likely(!(common->debug_mask & ATH_DBG_RESET)))
return;
@@ -1261,11 +1257,15 @@ void ar9003_hw_bb_watchdog_dbg_info(struct ath_hw *ah)
"** BB mode: BB_gen_controls=0x%08x **\n",
REG_READ(ah, AR_PHY_GEN_CTRL));
- if (ath9k_hw_GetMibCycleCountsPct(ah, &rxc_pcnt, &rxf_pcnt, &txf_pcnt))
+ ath_hw_cycle_counters_lock(common);
+ ath_hw_cycle_counters_update(common);
+#define PCT(_field) (common->cc_survey._field * 100 / common->cc_survey.cycles)
+ if (common->cc_survey.cycles)
ath_print(common, ATH_DBG_RESET,
- "** BB busy times: rx_clear=%d%%, "
+ "** BB busy times: rx_busy=%d%%, "
"rx_frame=%d%%, tx_frame=%d%% **\n",
- rxc_pcnt, rxf_pcnt, txf_pcnt);
+ PCT(rx_busy), PCT(rx_frame), PCT(tx_frame));
+ ath_hw_cycle_counters_unlock(common);
ath_print(common, ATH_DBG_RESET,
"==== BB update: done ====\n\n");
On Wed October 6 2010 11:45:46 Felix Fietkau wrote:
> On 2010-10-06 4:25 AM, Bruno Randolf wrote:
> > On Wed October 6 2010 07:38:34 Luis R. Rodriguez wrote:
> >> This is all being sucked out of common, so can we just share the
> >> filler for the survey between ath5k and ath9k? Note we should split up
> >> hw code and core driver code on the ath module by file.
> >
> > sure, that would be good! noise floor comes from ath5k_hw though, but
> > that can easily be moved to common.
> >
> > there's so much code which could be shared between ath5k and ath9k...
>
> That would need sharing of the channel list and some per-channel data
> though - so it may need some more code reshuffling...
also - we would need a common way to get ath_common from ieee80211_hw, which
works for both ath5k and ath9k. right now each has it's own structures. so...
i'd say: later...
bruno
On Thu, Oct 7, 2010 at 3:03 AM, Bruno Randolf <[email protected]> wrote:
> On Wed October 6 2010 18:54:37 Helmut Schaa wrote:
>> On Tue, Oct 5, 2010 at 11:55 AM, Bruno Randolf <[email protected]> wrote:
>> > This adds three new values to the survey results:
>> >
>> > ?* BUSY - percentage of time the channel was busy
>> > ?* BUSY_TX - percentage of time spent transmitting frames
>> > ?* BUSY_RX - percentage of time spent receiving frames
>>
>> Does BUSY include BUSY_RX and BUSY_TX already? At least on
>> rt2800 devices we only have access to BUSY which already includes
>> BUSY_RX and TX.
>
> yes, BUSY includes TX and RX.
>
>> And what about BUSY on the secondary channel in case of 40Mhz?
>
> i have no information about that. luis?
At least we've got a register for that in rt2800 ;)
That's why I'm asking.
Helmut
On Tue, Oct 5, 2010 at 6:10 PM, Bruno Randolf <[email protected]> wrote:
> On Wed October 6 2010 05:14:07 Luis R. Rodriguez wrote:
>> On Tue, Oct 05, 2010 at 02:55:21AM -0700, Bruno Randolf wrote:
>> > Update ath9k to use the common cycle counters.
>> >
>> > This also includes other changes from Felix Fietkaus "[PATCH 2/4]
>> > ath9k_hw: merge codepaths that access the cycle counter registers".
>> >
>> > Compile tested only. ath9k team please review...
>>
>> Can you move locking to be specific to the core part of
>> the driver instead of the ath9k_hw ?
>
> i'm not sure if i understand what you mean. locking is now implemented in
> ath_common.
Hrm...
>> We try to avoid locking
>> all over the hardware code. Even ath9k_iowrite32() and
>> ath9k_ioread32() are core driver helpers, not part of the
>> hw code.
>
> but, we need to make sure the cycle counters are always accessed exclusively.
> e.g. ANI runs from a timer (in ath9k, afaik), and updates the cycle counters.
> at the same time a user might issue a survey, or your debug print may be
> executed, which would want to access the same counters. we need to make sure
> the counters are not updated while they are read, etc...
That's fine, its just please keep any spin_lock calls away from ath9k_hw module.
Luis
On Thu, Oct 7, 2010 at 5:06 PM, Bruno Randolf <[email protected]> wrote:
> On Thu October 7 2010 15:51:49 Helmut Schaa wrote:
>> On Thu, Oct 7, 2010 at 3:03 AM, Bruno Randolf <[email protected]> wrote:
>> > On Wed October 6 2010 18:54:37 Helmut Schaa wrote:
>> >> On Tue, Oct 5, 2010 at 11:55 AM, Bruno Randolf <[email protected]> wrote:
>> >> > This adds three new values to the survey results:
>> >> >
>> >> > ?* BUSY - percentage of time the channel was busy
>> >> > ?* BUSY_TX - percentage of time spent transmitting frames
>> >> > ?* BUSY_RX - percentage of time spent receiving frames
>> >>
>> >> Does BUSY include BUSY_RX and BUSY_TX already? At least on
>> >> rt2800 devices we only have access to BUSY which already includes
>> >> BUSY_RX and TX.
>> >
>> > yes, BUSY includes TX and RX.
>> >
>> >> And what about BUSY on the secondary channel in case of 40Mhz?
>> >
>> > i have no information about that. luis?
>>
>> At least we've got a register for that in rt2800 ;)
>> That's why I'm asking.
>
> which units do you have for the busy time? what about other drivers?
>
> in ath[59]k we have it as cycles (which is about 1/44000 of a second,
> depending on the mode). so it counts up rather fast.
>
> i have been thinking more about it and i think my current approach is flawed.
> the idea was that we probably need to use a percentage as the least common
> denominator between all drivers, and i wanted to leave the time period up to
> userspace. so if userspace wants to have the percentage in the last second, it
> would poll every second, if it wants it every 100ms it would poll every 100ms.
> that would work, if there is only one userspace process querying this in
> regular intervals.
>
> but what if there is a second process who does the same, or a user issues "iw
> wlan0 survey dump", that would reduce the timespan for the other process... so
> that is obviously flawed...
>
> some absolute cumulative value would be better, so several userspace processes
> can poll and do their own averaging or percentage calculations.
>
> but which units to use? what would be a good way of solving that?
> any ideas???
My 2c:
I do something similar with my work. I output the amount of time in tx
& rx, as well as the 'measurement' time, being the elapsed time since
it was read last (all in ms). If you get a timer to go off
periodically (either ANI doing it, or something else if ANI is
disabled), then the user can calculate the % if required, based on the
elapsed time. Having a unit in ms seems to make the most sense as it
will be driver/clock agnostic, and easily match up to frame txtimes if
you wanted to de-construct the value further.
>
> bruno
>
--
Jonathan Guerin
On Tue, Oct 05, 2010 at 02:55:21AM -0700, Bruno Randolf wrote:
> Update ath9k to use the common cycle counters.
>
> This also includes other changes from Felix Fietkaus "[PATCH 2/4] ath9k_hw:
> merge codepaths that access the cycle counter registers".
>
> Compile tested only. ath9k team please review...
Can you move locking to be specific to the core part of
the driver instead of the ath9k_hw ? We try to avoid locking
all over the hardware code. Even ath9k_iowrite32() and
ath9k_ioread32() are core driver helpers, not part of the
hw code.
Luis
On Tue, Oct 5, 2010 at 2:55 AM, Bruno Randolf <[email protected]> wrote:
> This adds three new values to the survey results:
>
> * BUSY - percentage of time the channel was busy
> * BUSY_TX - percentage of time spent transmitting frames
> * BUSY_RX - percentage of time spent receiving frames
>
> They are defined to be a percentage of time, normalized to 255. That way they
> match the Channel Utilization of the BSS Load IE of 802.11-2007, chapter
> 7.3.2.28, in case we want to use that later.
>
> I admit that these three values are ath[59]k centric - it's what is available
> there, and i don't know if other chipsets have similar information.
>
> Signed-off-by: Bruno Randolf <[email protected]>
> ---
> include/linux/nl80211.h | 10 ++++++++++
> include/net/cfg80211.h | 6 ++++++
> net/wireless/nl80211.c | 9 +++++++++
> 3 files changed, 25 insertions(+), 0 deletions(-)
>
> diff --git a/include/linux/nl80211.h b/include/linux/nl80211.h
> index f0518b0..2cc74a2 100644
> --- a/include/linux/nl80211.h
> +++ b/include/linux/nl80211.h
> @@ -1400,6 +1400,13 @@ enum nl80211_reg_rule_flags {
> * @__NL80211_SURVEY_INFO_INVALID: attribute number 0 is reserved
> * @NL80211_SURVEY_INFO_FREQUENCY: center frequency of channel
> * @NL80211_SURVEY_INFO_NOISE: noise level of channel (u8, dBm)
> + * @NL80211_SURVEY_INFO_BUSY: channel busy ratio (u8, percentage of time,
> + * normalized to 255 so it matches Channel Utilization of the BSS Load IE
> + * of 802.11-2007 chapter 7.3.2.28)
Neat, so this could be used to help with roaming it seems.. Now as you
noted this is defined pretty specifically on 802.11-2007 chapter
7.3.2.28.
> + * @NL80211_SURVEY_INFO_BUSY_TX: percentage of time spent transmitting frames
> + * (u8, percent 0-255, see above)
But here you say look above, can you be more specific how this relates
to 802.11-2007 chapter 7.3.2.28 ?
> + * @NL80211_SURVEY_INFO_BUSY_RX: percentage of time spent receiving frames
> + * (u8, percent 0-255, see above)
Same here.
Luis
On Tue, Oct 5, 2010 at 2:55 AM, Bruno Randolf <[email protected]> wrote:
> Update ath9k to use the common cycle counters.
>
> This also includes other changes from Felix Fietkaus "[PATCH 2/4] ath9k_hw:
> merge codepaths that access the cycle counter registers".
>
> Compile tested only. ath9k team please review...
>
> Signed-off-by: Felix Fietkau <[email protected]>
> Signed-off-by: Bruno Randolf <[email protected]>
> ---
> drivers/net/wireless/ath/ath9k/ani.c | 87 +++------------------------
> drivers/net/wireless/ath/ath9k/ani.h | 6 --
> drivers/net/wireless/ath/ath9k/ar5008_phy.c | 9 +--
> drivers/net/wireless/ath/ath9k/ar9003_phy.c | 18 +++---
> 4 files changed, 22 insertions(+), 98 deletions(-)
>
> diff --git a/drivers/net/wireless/ath/ath9k/ani.c b/drivers/net/wireless/ath/ath9k/ani.c
> index 0496f96..0c0d01d 100644
> --- a/drivers/net/wireless/ath/ath9k/ani.c
> +++ b/drivers/net/wireless/ath/ath9k/ani.c
> @@ -549,47 +549,21 @@ static u8 ath9k_hw_chan_2_clockrate_mhz(struct ath_hw *ah)
>
> static int32_t ath9k_hw_ani_get_listen_time(struct ath_hw *ah)
> {
> - struct ar5416AniState *aniState;
> struct ath_common *common = ath9k_hw_common(ah);
> - u32 txFrameCount, rxFrameCount, cycleCount;
> - int32_t listenTime;
> + int32_t listen_time;
> + int32_t clock_rate;
>
> - txFrameCount = REG_READ(ah, AR_TFCNT);
> - rxFrameCount = REG_READ(ah, AR_RFCNT);
> - cycleCount = REG_READ(ah, AR_CCCNT);
> + ath_hw_cycle_counters_lock(common);
Note the lock call here. This is what I'd like to see avoided. Can you
perhaps have the lock called within the core driver instead, that is,
add the lock call for the caller of ath9k_hw_ani_get_listen_time(). In
this case I see ath9k_hw_ani_get_listen_time() is static though and so
is its caller but ultimately we get to ath9k_hw_ani_monitor(). What
I'm saying is how about calling the
ath_hw_cycle_counters_lock(common); right before
ath9k_hw_ani_monitor().
> + ath_hw_cycle_counters_update(common);
Luis
On Tue, Oct 5, 2010 at 7:50 PM, Bruno Randolf <[email protected]> wrote:
> On Wed October 6 2010 11:45:46 Felix Fietkau wrote:
>> On 2010-10-06 4:25 AM, Bruno Randolf wrote:
>> > On Wed October 6 2010 07:38:34 Luis R. Rodriguez wrote:
>> >> This is all being sucked out of common, so can we just share the
>> >> filler for the survey between ath5k and ath9k? Note we should split up
>> >> hw code and core driver code on the ath module by file.
>> >
>> > sure, that would be good! noise floor comes from ath5k_hw though, but
>> > that can easily be moved to common.
>> >
>> > there's so much code which could be shared between ath5k and ath9k...
>>
>> That would need sharing of the channel list and some per-channel data
>> though - so it may need some more code reshuffling...
>
> also - we would need a common way to get ath_common from ieee80211_hw, which
> works for both ath5k and ath9k. right now each has it's own structures. so...
> i'd say: later...
Lets also learn from the iwlwifi issues. The more you share the easier
it is to regress older drivers. So I'd love to share but I think I now
prefer to be more conservative towards code sharing, share only what
we know will not change at all and are 100% confident in, and fork off
a driver every now and then so we can maintain stability on the older
drivers.
Luis
Include the channel utilization (busy, rx, tx) in the survey results.
Signed-off-by: Bruno Randolf <[email protected]>
---
drivers/net/wireless/ath/ath5k/base.c | 17 +++++++++++++++++
1 files changed, 17 insertions(+), 0 deletions(-)
diff --git a/drivers/net/wireless/ath/ath5k/base.c b/drivers/net/wireless/ath/ath5k/base.c
index c158f2e..a33d9f2 100644
--- a/drivers/net/wireless/ath/ath5k/base.c
+++ b/drivers/net/wireless/ath/ath5k/base.c
@@ -3008,6 +3008,7 @@ static int ath5k_get_survey(struct ieee80211_hw *hw, int idx,
{
struct ath5k_softc *sc = hw->priv;
struct ieee80211_conf *conf = &hw->conf;
+ struct ath_common *common = ath5k_hw_common(sc->ah);
if (idx != 0)
return -ENOENT;
@@ -3016,6 +3017,22 @@ static int ath5k_get_survey(struct ieee80211_hw *hw, int idx,
survey->filled = SURVEY_INFO_NOISE_DBM;
survey->noise = sc->ah->ah_noise_floor;
+ ath_hw_cycle_counters_lock(common);
+ ath_hw_cycle_counters_update(common);
+ if (common->cc_survey.cycles > 0) {
+ survey->filled |= SURVEY_INFO_BUSY |
+ SURVEY_INFO_BUSY_TX | SURVEY_INFO_BUSY_RX;
+ survey->busy = common->cc_survey.rx_busy * 255 /
+ common->cc_survey.cycles;
+ survey->busy_tx = common->cc_survey.tx_frame * 255 /
+ common->cc_survey.cycles;
+ survey->busy_rx = common->cc_survey.rx_frame * 255 /
+ common->cc_survey.cycles;
+ }
+
+ memset(&common->cc_survey, 0, sizeof(struct ath_cycle_counters));
+ ath_hw_cycle_counters_unlock(common);
+
return 0;
}
On Tue, Oct 05, 2010 at 07:00:01PM -0700, Bruno Randolf wrote:
> On Wed October 6 2010 10:18:07 Luis R. Rodriguez wrote:
> > On Tue, Oct 5, 2010 at 2:55 AM, Bruno Randolf <[email protected]> wrote:
> > > Update ath9k to use the common cycle counters.
> > >
> > > This also includes other changes from Felix Fietkaus "[PATCH 2/4]
> > > ath9k_hw: merge codepaths that access the cycle counter registers".
> > >
> > > Compile tested only. ath9k team please review...
> > >
> > > Signed-off-by: Felix Fietkau <[email protected]>
> > > Signed-off-by: Bruno Randolf <[email protected]>
> > > ---
> > > drivers/net/wireless/ath/ath9k/ani.c | 87
> > > +++------------------------ drivers/net/wireless/ath/ath9k/ani.h
> > > | 6 --
> > > drivers/net/wireless/ath/ath9k/ar5008_phy.c | 9 +--
> > > drivers/net/wireless/ath/ath9k/ar9003_phy.c | 18 +++---
> > > 4 files changed, 22 insertions(+), 98 deletions(-)
> > >
> > > diff --git a/drivers/net/wireless/ath/ath9k/ani.c
> > > b/drivers/net/wireless/ath/ath9k/ani.c index 0496f96..0c0d01d 100644
> > > --- a/drivers/net/wireless/ath/ath9k/ani.c
> > > +++ b/drivers/net/wireless/ath/ath9k/ani.c
> > > @@ -549,47 +549,21 @@ static u8 ath9k_hw_chan_2_clockrate_mhz(struct
> > > ath_hw *ah)
> > >
> > > static int32_t ath9k_hw_ani_get_listen_time(struct ath_hw *ah)
> > > {
> > > - struct ar5416AniState *aniState;
> > > struct ath_common *common = ath9k_hw_common(ah);
> > > - u32 txFrameCount, rxFrameCount, cycleCount;
> > > - int32_t listenTime;
> > > + int32_t listen_time;
> > > + int32_t clock_rate;
> > >
> > > - txFrameCount = REG_READ(ah, AR_TFCNT);
> > > - rxFrameCount = REG_READ(ah, AR_RFCNT);
> > > - cycleCount = REG_READ(ah, AR_CCCNT);
> > > + ath_hw_cycle_counters_lock(common);
> >
> > Note the lock call here. This is what I'd like to see avoided. Can you
> > perhaps have the lock called within the core driver instead, that is,
> > add the lock call for the caller of ath9k_hw_ani_get_listen_time(). In
> > this case I see ath9k_hw_ani_get_listen_time() is static though and so
> > is its caller but ultimately we get to ath9k_hw_ani_monitor(). What
> > I'm saying is how about calling the
> > ath_hw_cycle_counters_lock(common); right before
> > ath9k_hw_ani_monitor().
>
> that would hold the lock for an unnecessarily long amount of time.
I see..
> i'm not working with ath9k, so can i leave it to you to deal with this in
> subsequent patches? in the mean time, if you prefer, i can omit the lock.
I can't think of a good alternative, so just ahead with it, if it creates a
problem we can remove it.
Luis
On Thu October 7 2010 15:51:49 Helmut Schaa wrote:
> On Thu, Oct 7, 2010 at 3:03 AM, Bruno Randolf <[email protected]> wrote:
> > On Wed October 6 2010 18:54:37 Helmut Schaa wrote:
> >> On Tue, Oct 5, 2010 at 11:55 AM, Bruno Randolf <[email protected]> wrote:
> >> > This adds three new values to the survey results:
> >> >
> >> > * BUSY - percentage of time the channel was busy
> >> > * BUSY_TX - percentage of time spent transmitting frames
> >> > * BUSY_RX - percentage of time spent receiving frames
> >>
> >> Does BUSY include BUSY_RX and BUSY_TX already? At least on
> >> rt2800 devices we only have access to BUSY which already includes
> >> BUSY_RX and TX.
> >
> > yes, BUSY includes TX and RX.
> >
> >> And what about BUSY on the secondary channel in case of 40Mhz?
> >
> > i have no information about that. luis?
>
> At least we've got a register for that in rt2800 ;)
> That's why I'm asking.
which units do you have for the busy time? what about other drivers?
in ath[59]k we have it as cycles (which is about 1/44000 of a second,
depending on the mode). so it counts up rather fast.
i have been thinking more about it and i think my current approach is flawed.
the idea was that we probably need to use a percentage as the least common
denominator between all drivers, and i wanted to leave the time period up to
userspace. so if userspace wants to have the percentage in the last second, it
would poll every second, if it wants it every 100ms it would poll every 100ms.
that would work, if there is only one userspace process querying this in
regular intervals.
but what if there is a second process who does the same, or a user issues "iw
wlan0 survey dump", that would reduce the timespan for the other process... so
that is obviously flawed...
some absolute cumulative value would be better, so several userspace processes
can poll and do their own averaging or percentage calculations.
but which units to use? what would be a good way of solving that?
any ideas???
bruno
On Wed October 6 2010 07:38:34 Luis R. Rodriguez wrote:
> On Tue, Oct 5, 2010 at 2:55 AM, Bruno Randolf <[email protected]> wrote:
> > Include the channel utilization (busy, rx, tx) in the survey results.
> >
> > Signed-off-by: Bruno Randolf <[email protected]>
> > ---
> > drivers/net/wireless/ath/ath5k/base.c | 17 +++++++++++++++++
> > 1 files changed, 17 insertions(+), 0 deletions(-)
> >
> > diff --git a/drivers/net/wireless/ath/ath5k/base.c
> > b/drivers/net/wireless/ath/ath5k/base.c index c158f2e..a33d9f2 100644
> > --- a/drivers/net/wireless/ath/ath5k/base.c
> > +++ b/drivers/net/wireless/ath/ath5k/base.c
> > @@ -3008,6 +3008,7 @@ static int ath5k_get_survey(struct ieee80211_hw
> > *hw, int idx, {
> > struct ath5k_softc *sc = hw->priv;
> > struct ieee80211_conf *conf = &hw->conf;
> > + struct ath_common *common = ath5k_hw_common(sc->ah);
> >
> > if (idx != 0)
> > return -ENOENT;
> > @@ -3016,6 +3017,22 @@ static int ath5k_get_survey(struct ieee80211_hw
> > *hw, int idx, survey->filled = SURVEY_INFO_NOISE_DBM;
> > survey->noise = sc->ah->ah_noise_floor;
> >
> > + ath_hw_cycle_counters_lock(common);
> > + ath_hw_cycle_counters_update(common);
> > + if (common->cc_survey.cycles > 0) {
> > + survey->filled |= SURVEY_INFO_BUSY |
> > + SURVEY_INFO_BUSY_TX |
> > SURVEY_INFO_BUSY_RX; + survey->busy =
> > common->cc_survey.rx_busy * 255 / +
> > common->cc_survey.cycles; + survey->busy_tx =
> > common->cc_survey.tx_frame * 255 / +
> > common->cc_survey.cycles; + survey->busy_rx =
> > common->cc_survey.rx_frame * 255 / +
> > common->cc_survey.cycles; + }
> > +
> > + memset(&common->cc_survey, 0, sizeof(struct ath_cycle_counters));
> > + ath_hw_cycle_counters_unlock(common);
> > +
>
> This is all being sucked out of common, so can we just share the
> filler for the survey between ath5k and ath9k? Note we should split up
> hw code and core driver code on the ath module by file.
sure, that would be good! noise floor comes from ath5k_hw though, but that can
easily be moved to common.
there's so much code which could be shared between ath5k and ath9k...
bruno
On 2010-10-06 4:25 AM, Bruno Randolf wrote:
> On Wed October 6 2010 07:38:34 Luis R. Rodriguez wrote:
>> This is all being sucked out of common, so can we just share the
>> filler for the survey between ath5k and ath9k? Note we should split up
>> hw code and core driver code on the ath module by file.
>
> sure, that would be good! noise floor comes from ath5k_hw though, but that can
> easily be moved to common.
>
> there's so much code which could be shared between ath5k and ath9k...
That would need sharing of the channel list and some per-channel data
though - so it may need some more code reshuffling...
- Felix
On Wed, Oct 6, 2010 at 6:03 PM, Bruno Randolf <[email protected]> wrote:
> On Wed October 6 2010 18:54:37 Helmut Schaa wrote:
>> On Tue, Oct 5, 2010 at 11:55 AM, Bruno Randolf <[email protected]> wrote:
>> > This adds three new values to the survey results:
>> >
>> > * BUSY - percentage of time the channel was busy
>> > * BUSY_TX - percentage of time spent transmitting frames
>> > * BUSY_RX - percentage of time spent receiving frames
>>
>> Does BUSY include BUSY_RX and BUSY_TX already? At least on
>> rt2800 devices we only have access to BUSY which already includes
>> BUSY_RX and TX.
>
> yes, BUSY includes TX and RX.
>
>> And what about BUSY on the secondary channel in case of 40Mhz?
>
> i have no information about that. luis?
You got me.
Luis
On Thu, 2010-10-07 at 09:44 +0200, Helmut Schaa wrote:
> So, I guess it makes sense to just return the calculated channel
> utilization value (u8)
> and keep it up to date within the driver.
Not good, that means drivers implementing this will have to wake the CPU
to do calculations even if nobody cares.
johannes
On Tue, Oct 5, 2010 at 11:55 AM, Bruno Randolf <[email protected]> wrote:
> This adds three new values to the survey results:
>
> ?* BUSY - percentage of time the channel was busy
> ?* BUSY_TX - percentage of time spent transmitting frames
> ?* BUSY_RX - percentage of time spent receiving frames
Does BUSY include BUSY_RX and BUSY_TX already? At least on
rt2800 devices we only have access to BUSY which already includes
BUSY_RX and TX.
And what about BUSY on the secondary channel in case of 40Mhz?
Thanks,
Helmut
On Wed October 6 2010 18:54:37 Helmut Schaa wrote:
> On Tue, Oct 5, 2010 at 11:55 AM, Bruno Randolf <[email protected]> wrote:
> > This adds three new values to the survey results:
> >
> > * BUSY - percentage of time the channel was busy
> > * BUSY_TX - percentage of time spent transmitting frames
> > * BUSY_RX - percentage of time spent receiving frames
>
> Does BUSY include BUSY_RX and BUSY_TX already? At least on
> rt2800 devices we only have access to BUSY which already includes
> BUSY_RX and TX.
yes, BUSY includes TX and RX.
> And what about BUSY on the secondary channel in case of 40Mhz?
i have no information about that. luis?
bruno
On Thu, Oct 7, 2010 at 9:06 AM, Bruno Randolf <[email protected]> wrote:
> which units do you have for the busy time? what about other drivers?
> in ath[59]k we have it as cycles (which is about 1/44000 of a second,
> depending on the mode). so it counts up rather fast.
We've got the busy time in [us] since the last read (the register is
cleared on read).
> some absolute cumulative value would be better, so several userspace processes
> can poll and do their own averaging or percentage calculations.
>
> but which units to use? what would be a good way of solving that?
> any ideas???
That's what 802.11-2007 tells us in regard to the BSS Load element:
<citation>
((channel busy time/(dot11ChannelUtilizationBeaconIntervals *
dot11BeaconPeriod * 1024)) * 255),
where channel busy time is defined to be the number of microseconds
during which the CS mechanism, as
defined in 9.2.1, has indicated a channel busy indication, and the MIB
attribute dot11ChannelUtilization-
BeaconIntervals represents the number of consecutive beacon intervals
during which the channel busy time
is measured. The default value of
dot11ChannelUtilizationBeaconIntervals is defined in Annex D.
</citation>
So, I guess it makes sense to just return the calculated channel
utilization value (u8)
and keep it up to date within the driver.
mac80211 could pass dot11ChannelUtilizationBeaconIntervals (using the default of
16 for now) to the drivers and the drivers would report the calculated value.
Helmut
On Tue, Oct 5, 2010 at 2:55 AM, Bruno Randolf <[email protected]> wrote:
> Include the channel utilization (busy, rx, tx) in the survey results.
>
> Signed-off-by: Bruno Randolf <[email protected]>
> ---
> drivers/net/wireless/ath/ath5k/base.c | 17 +++++++++++++++++
> 1 files changed, 17 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/net/wireless/ath/ath5k/base.c b/drivers/net/wireless/ath/ath5k/base.c
> index c158f2e..a33d9f2 100644
> --- a/drivers/net/wireless/ath/ath5k/base.c
> +++ b/drivers/net/wireless/ath/ath5k/base.c
> @@ -3008,6 +3008,7 @@ static int ath5k_get_survey(struct ieee80211_hw *hw, int idx,
> {
> struct ath5k_softc *sc = hw->priv;
> struct ieee80211_conf *conf = &hw->conf;
> + struct ath_common *common = ath5k_hw_common(sc->ah);
>
> if (idx != 0)
> return -ENOENT;
> @@ -3016,6 +3017,22 @@ static int ath5k_get_survey(struct ieee80211_hw *hw, int idx,
> survey->filled = SURVEY_INFO_NOISE_DBM;
> survey->noise = sc->ah->ah_noise_floor;
>
> + ath_hw_cycle_counters_lock(common);
> + ath_hw_cycle_counters_update(common);
> + if (common->cc_survey.cycles > 0) {
> + survey->filled |= SURVEY_INFO_BUSY |
> + SURVEY_INFO_BUSY_TX | SURVEY_INFO_BUSY_RX;
> + survey->busy = common->cc_survey.rx_busy * 255 /
> + common->cc_survey.cycles;
> + survey->busy_tx = common->cc_survey.tx_frame * 255 /
> + common->cc_survey.cycles;
> + survey->busy_rx = common->cc_survey.rx_frame * 255 /
> + common->cc_survey.cycles;
> + }
> +
> + memset(&common->cc_survey, 0, sizeof(struct ath_cycle_counters));
> + ath_hw_cycle_counters_unlock(common);
> +
This is all being sucked out of common, so can we just share the
filler for the survey between ath5k and ath9k? Note we should split up
hw code and core driver code on the ath module by file.
Luis
On Wed October 6 2010 10:18:07 Luis R. Rodriguez wrote:
> On Tue, Oct 5, 2010 at 2:55 AM, Bruno Randolf <[email protected]> wrote:
> > Update ath9k to use the common cycle counters.
> >
> > This also includes other changes from Felix Fietkaus "[PATCH 2/4]
> > ath9k_hw: merge codepaths that access the cycle counter registers".
> >
> > Compile tested only. ath9k team please review...
> >
> > Signed-off-by: Felix Fietkau <[email protected]>
> > Signed-off-by: Bruno Randolf <[email protected]>
> > ---
> > drivers/net/wireless/ath/ath9k/ani.c | 87
> > +++------------------------ drivers/net/wireless/ath/ath9k/ani.h
> > | 6 --
> > drivers/net/wireless/ath/ath9k/ar5008_phy.c | 9 +--
> > drivers/net/wireless/ath/ath9k/ar9003_phy.c | 18 +++---
> > 4 files changed, 22 insertions(+), 98 deletions(-)
> >
> > diff --git a/drivers/net/wireless/ath/ath9k/ani.c
> > b/drivers/net/wireless/ath/ath9k/ani.c index 0496f96..0c0d01d 100644
> > --- a/drivers/net/wireless/ath/ath9k/ani.c
> > +++ b/drivers/net/wireless/ath/ath9k/ani.c
> > @@ -549,47 +549,21 @@ static u8 ath9k_hw_chan_2_clockrate_mhz(struct
> > ath_hw *ah)
> >
> > static int32_t ath9k_hw_ani_get_listen_time(struct ath_hw *ah)
> > {
> > - struct ar5416AniState *aniState;
> > struct ath_common *common = ath9k_hw_common(ah);
> > - u32 txFrameCount, rxFrameCount, cycleCount;
> > - int32_t listenTime;
> > + int32_t listen_time;
> > + int32_t clock_rate;
> >
> > - txFrameCount = REG_READ(ah, AR_TFCNT);
> > - rxFrameCount = REG_READ(ah, AR_RFCNT);
> > - cycleCount = REG_READ(ah, AR_CCCNT);
> > + ath_hw_cycle_counters_lock(common);
>
> Note the lock call here. This is what I'd like to see avoided. Can you
> perhaps have the lock called within the core driver instead, that is,
> add the lock call for the caller of ath9k_hw_ani_get_listen_time(). In
> this case I see ath9k_hw_ani_get_listen_time() is static though and so
> is its caller but ultimately we get to ath9k_hw_ani_monitor(). What
> I'm saying is how about calling the
> ath_hw_cycle_counters_lock(common); right before
> ath9k_hw_ani_monitor().
that would hold the lock for an unnecessarily long amount of time.
i'm not working with ath9k, so can i leave it to you to deal with this in
subsequent patches? in the mean time, if you prefer, i can omit the lock.
bruno
Am Donnerstag 07 Oktober 2010 schrieb Johannes Berg:
> On Thu, 2010-10-07 at 09:44 +0200, Helmut Schaa wrote:
>
> > So, I guess it makes sense to just return the calculated channel
> > utilization value (u8)
> > and keep it up to date within the driver.
>
> Not good, that means drivers implementing this will have to wake the CPU
> to do calculations even if nobody cares.
You're obviously right. But that would be the same if mac80211 would do
the calculation. And if we would only calculate it when the survey is
requested the measurement window could be very large or very small which
both might be unwanted.
This could also introduce problems related to register overflows. Since
on rt2800 the register (32bit) is in units of [us] it would overflow after
a bit more then one hour making the calculation flawed in that (maybe
unusual) case.
Nevertheless I fully agree that it doesn't make sense to always calculate
the busy time values if nobody actually needs it. But if somebody cares it
would be nice to have it as a percentage value.
Helmut
This adds three new values to the survey results:
* BUSY - percentage of time the channel was busy
* BUSY_TX - percentage of time spent transmitting frames
* BUSY_RX - percentage of time spent receiving frames
They are defined to be a percentage of time, normalized to 255. That way they
match the Channel Utilization of the BSS Load IE of 802.11-2007, chapter
7.3.2.28, in case we want to use that later.
I admit that these three values are ath[59]k centric - it's what is available
there, and i don't know if other chipsets have similar information.
Signed-off-by: Bruno Randolf <[email protected]>
---
include/linux/nl80211.h | 10 ++++++++++
include/net/cfg80211.h | 6 ++++++
net/wireless/nl80211.c | 9 +++++++++
3 files changed, 25 insertions(+), 0 deletions(-)
diff --git a/include/linux/nl80211.h b/include/linux/nl80211.h
index f0518b0..2cc74a2 100644
--- a/include/linux/nl80211.h
+++ b/include/linux/nl80211.h
@@ -1400,6 +1400,13 @@ enum nl80211_reg_rule_flags {
* @__NL80211_SURVEY_INFO_INVALID: attribute number 0 is reserved
* @NL80211_SURVEY_INFO_FREQUENCY: center frequency of channel
* @NL80211_SURVEY_INFO_NOISE: noise level of channel (u8, dBm)
+ * @NL80211_SURVEY_INFO_BUSY: channel busy ratio (u8, percentage of time,
+ * normalized to 255 so it matches Channel Utilization of the BSS Load IE
+ * of 802.11-2007 chapter 7.3.2.28)
+ * @NL80211_SURVEY_INFO_BUSY_TX: percentage of time spent transmitting frames
+ * (u8, percent 0-255, see above)
+ * @NL80211_SURVEY_INFO_BUSY_RX: percentage of time spent receiving frames
+ * (u8, percent 0-255, see above)
* @NL80211_SURVEY_INFO_MAX: highest survey info attribute number
* currently defined
* @__NL80211_SURVEY_INFO_AFTER_LAST: internal use
@@ -1408,6 +1415,9 @@ enum nl80211_survey_info {
__NL80211_SURVEY_INFO_INVALID,
NL80211_SURVEY_INFO_FREQUENCY,
NL80211_SURVEY_INFO_NOISE,
+ NL80211_SURVEY_INFO_BUSY,
+ NL80211_SURVEY_INFO_BUSY_TX,
+ NL80211_SURVEY_INFO_BUSY_RX,
/* keep last */
__NL80211_SURVEY_INFO_AFTER_LAST,
diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
index a0613ff..e86c890 100644
--- a/include/net/cfg80211.h
+++ b/include/net/cfg80211.h
@@ -299,6 +299,9 @@ struct key_params {
*/
enum survey_info_flags {
SURVEY_INFO_NOISE_DBM = 1<<0,
+ SURVEY_INFO_BUSY = 1<<1,
+ SURVEY_INFO_BUSY_TX = 1<<2,
+ SURVEY_INFO_BUSY_RX = 1<<3,
};
/**
@@ -318,6 +321,9 @@ struct survey_info {
struct ieee80211_channel *channel;
u32 filled;
s8 noise;
+ u8 busy;
+ u8 busy_rx;
+ u8 busy_tx;
};
/**
diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index 9c84825..26062e1 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -3489,6 +3489,15 @@ static int nl80211_send_survey(struct sk_buff *msg, u32 pid, u32 seq,
if (survey->filled & SURVEY_INFO_NOISE_DBM)
NLA_PUT_U8(msg, NL80211_SURVEY_INFO_NOISE,
survey->noise);
+ if (survey->filled & SURVEY_INFO_BUSY)
+ NLA_PUT_U8(msg, NL80211_SURVEY_INFO_BUSY,
+ survey->busy);
+ if (survey->filled & SURVEY_INFO_BUSY_TX)
+ NLA_PUT_U8(msg, NL80211_SURVEY_INFO_BUSY_TX,
+ survey->busy_tx);
+ if (survey->filled & SURVEY_INFO_BUSY_RX)
+ NLA_PUT_U8(msg, NL80211_SURVEY_INFO_BUSY_RX,
+ survey->busy_rx);
nla_nest_end(msg, infoattr);
On Wed October 6 2010 07:36:30 Luis R. Rodriguez wrote:
> On Tue, Oct 5, 2010 at 2:55 AM, Bruno Randolf <[email protected]> wrote:
> > This adds three new values to the survey results:
> >
> > * BUSY - percentage of time the channel was busy
> > * BUSY_TX - percentage of time spent transmitting frames
> > * BUSY_RX - percentage of time spent receiving frames
> >
> > They are defined to be a percentage of time, normalized to 255. That way
> > they match the Channel Utilization of the BSS Load IE of 802.11-2007,
> > chapter 7.3.2.28, in case we want to use that later.
> >
> > I admit that these three values are ath[59]k centric - it's what is
> > available there, and i don't know if other chipsets have similar
> > information.
> >
> > Signed-off-by: Bruno Randolf <[email protected]>
> > ---
> > include/linux/nl80211.h | 10 ++++++++++
> > include/net/cfg80211.h | 6 ++++++
> > net/wireless/nl80211.c | 9 +++++++++
> > 3 files changed, 25 insertions(+), 0 deletions(-)
> >
> > diff --git a/include/linux/nl80211.h b/include/linux/nl80211.h
> > index f0518b0..2cc74a2 100644
> > --- a/include/linux/nl80211.h
> > +++ b/include/linux/nl80211.h
> > @@ -1400,6 +1400,13 @@ enum nl80211_reg_rule_flags {
> > * @__NL80211_SURVEY_INFO_INVALID: attribute number 0 is reserved
> > * @NL80211_SURVEY_INFO_FREQUENCY: center frequency of channel
> > * @NL80211_SURVEY_INFO_NOISE: noise level of channel (u8, dBm)
> > + * @NL80211_SURVEY_INFO_BUSY: channel busy ratio (u8, percentage of
> > time, + * normalized to 255 so it matches Channel Utilization of the
> > BSS Load IE + * of 802.11-2007 chapter 7.3.2.28)
>
> Neat, so this could be used to help with roaming it seems.. Now as you
> noted this is defined pretty specifically on 802.11-2007 chapter
> 7.3.2.28.
>
> > + * @NL80211_SURVEY_INFO_BUSY_TX: percentage of time spent transmitting
> > frames + * (u8, percent 0-255, see above)
>
> But here you say look above, can you be more specific how this relates
> to 802.11-2007 chapter 7.3.2.28 ?
hmm, i guess it doesn't. i was just refering to above for the range, but
802.11 doesn't have separate values for TX or RX. i'd better rephrase that.
bruno