[PATCH 00/15] ath9k/mac80211/iwlwifi: rate control cleanup

[PATCH 00/15] ath9k/mac80211/iwlwifi: rate control cleanup

[Luis R. Rodriguez]

ath9k's rate control code is so convoluted that it really make it
difficult to spot bugs and fix them. Here's a bunch of cleanups for
ath9k rate control code, and a few for mac80211. I just tested ath9k.
Will have to test others later.

Luis R. Rodriguez (15):
  ath9k: fix oops by downgrading assert in rc.c
  ath9k: cleanup try count for MRR in rate control
  ath9k: remove unused min rate calculation code
  ath9k: remove unused stepdown when looking for the next rate
  ath9k: remove pointless wrapper ath_rc_rate_getidx()
  ath9k: rename ath_rc_get_nextlowervalid_txrate()
  ath9k: remove unused ath_rc_isvalid_txmask()
  ath9k: remove ATH9K_MODE_11B
  ath9k: remap ATH9K_MODE_*
  ath9k: rename ath_rc_ratefind_ht() to ath_rc_get_highest_rix()
  ath9k: remove unnecessary IEEE80211_TX_CTL_NO_ACK checks
  mac80211: make minstrel/pid RC use ieee80211_is_data(fc)
  iwlwifi: use ieee80211_is_data(fc)
  mac80211: move management / no-ack frame rate decision to mac80211
  ath9k: remove rate control wraper

 drivers/net/wireless/ath/ath9k/ath9k.h     |    1 -
 drivers/net/wireless/ath/ath9k/debug.h     |    1 +
 drivers/net/wireless/ath/ath9k/hw.c        |    1 -
 drivers/net/wireless/ath/ath9k/hw.h        |   17 ++--
 drivers/net/wireless/ath/ath9k/main.c      |    3 +-
 drivers/net/wireless/ath/ath9k/rc.c        |  190 ++++++++++------------------
 drivers/net/wireless/iwlwifi/iwl-3945-rs.c |   21 +---
 drivers/net/wireless/iwlwifi/iwl-agn-rs.c  |   10 --
 drivers/net/wireless/iwlwifi/iwl-sta.c     |    5 +-
 net/mac80211/rate.c                        |   29 ++++-
 net/mac80211/rc80211_minstrel.c            |   24 ----
 net/mac80211/rc80211_pid_algo.c            |   15 ---
 12 files changed, 111 insertions(+), 206 deletions(-)

[ath9k-devel] [PATCH 00/15] ath9k/mac80211/iwlwifi: rate control cleanup

[Luis R. Rodriguez]

ath9k's rate control code is so convoluted that it really make it
difficult to spot bugs and fix them. Here's a bunch of cleanups for
ath9k rate control code, and a few for mac80211. I just tested ath9k.
Will have to test others later.

Luis R. Rodriguez (15):
  ath9k: fix oops by downgrading assert in rc.c
  ath9k: cleanup try count for MRR in rate control
  ath9k: remove unused min rate calculation code
  ath9k: remove unused stepdown when looking for the next rate
  ath9k: remove pointless wrapper ath_rc_rate_getidx()
  ath9k: rename ath_rc_get_nextlowervalid_txrate()
  ath9k: remove unused ath_rc_isvalid_txmask()
  ath9k: remove ATH9K_MODE_11B
  ath9k: remap ATH9K_MODE_*
  ath9k: rename ath_rc_ratefind_ht() to ath_rc_get_highest_rix()
  ath9k: remove unnecessary IEEE80211_TX_CTL_NO_ACK checks
  mac80211: make minstrel/pid RC use ieee80211_is_data(fc)
  iwlwifi: use ieee80211_is_data(fc)
  mac80211: move management / no-ack frame rate decision to mac80211
  ath9k: remove rate control wraper

 drivers/net/wireless/ath/ath9k/ath9k.h     |    1 -
 drivers/net/wireless/ath/ath9k/debug.h     |    1 +
 drivers/net/wireless/ath/ath9k/hw.c        |    1 -
 drivers/net/wireless/ath/ath9k/hw.h        |   17 ++--
 drivers/net/wireless/ath/ath9k/main.c      |    3 +-
 drivers/net/wireless/ath/ath9k/rc.c        |  190 ++++++++++------------------
 drivers/net/wireless/iwlwifi/iwl-3945-rs.c |   21 +---
 drivers/net/wireless/iwlwifi/iwl-agn-rs.c  |   10 --
 drivers/net/wireless/iwlwifi/iwl-sta.c     |    5 +-
 net/mac80211/rate.c                        |   29 ++++-
 net/mac80211/rc80211_minstrel.c            |   24 ----
 net/mac80211/rc80211_pid_algo.c            |   15 ---
 12 files changed, 111 insertions(+), 206 deletions(-)

[PATCH 01/15] ath9k: fix oops by downgrading assert in rc.c

[Luis R. Rodriguez]

Unfortunately locking the driver rate control private area
doesn't resolve this issue and in fact causes some issues.
The root cause is yet undetermined. For now instead of crashing
lets just downgrades the ASSERT() and only show a trace when
we are debugging with ATH_DBG_CONFIG enabled. That attempt
and its results can be seen here:

http://marc.info/?l=linux-wireless&m=124399166108295&w=1

The ASSERT was happening because the rate control algorithm
figures it should find at least one valid dual stream or
single stream rate. *Something* is causing us to not find
such rate. This happens after association and only when HT
is enabled AFAICT. What we are doing by downgrading the assert
is informing the driver to use the lowest rate index. When
this happens lets disable multi rate retry and only try on
the lowest supported rate.

Traces of the ASSERT are available on this thread:

http://marc.info/?l=linux-wireless&m=124277331319024

At least one bug report (as side issue, mind you):

https://bugzilla.redhat.com/show_bug.cgi?id=503285

Signed-off-by: Luis R. Rodriguez <lrodriguez@atheros.com>
---
 drivers/net/wireless/ath/ath9k/debug.h |    1 +
 drivers/net/wireless/ath/ath9k/rc.c    |   40 +++++++++++++++++++++++++++----
 2 files changed, 35 insertions(+), 6 deletions(-)

diff --git a/drivers/net/wireless/ath/ath9k/debug.h b/drivers/net/wireless/ath/ath9k/debug.h
index edda15b..9d72bc8 100644
--- a/drivers/net/wireless/ath/ath9k/debug.h
+++ b/drivers/net/wireless/ath/ath9k/debug.h
@@ -30,6 +30,7 @@ enum ATH_DEBUG {
 	ATH_DBG_CONFIG		= 0x00000200,
 	ATH_DBG_FATAL		= 0x00000400,
 	ATH_DBG_PS		= 0x00000800,
+	ATH_DBG_RATE		= 0x00001000,
 	ATH_DBG_ANY		= 0xffffffff
 };
 
diff --git a/drivers/net/wireless/ath/ath9k/rc.c b/drivers/net/wireless/ath/ath9k/rc.c
index ba06e78..abad86b 100644
--- a/drivers/net/wireless/ath/ath9k/rc.c
+++ b/drivers/net/wireless/ath/ath9k/rc.c
@@ -741,10 +741,24 @@ static u8 ath_rc_ratefind_ht(struct ath_softc *sc,
 	if (rate > (ath_rc_priv->rate_table_size - 1))
 		rate = ath_rc_priv->rate_table_size - 1;
 
-	ASSERT((rate_table->info[rate].valid &&
-		(ath_rc_priv->ht_cap & WLAN_RC_DS_FLAG)) ||
-	       (rate_table->info[rate].valid_single_stream &&
-		!(ath_rc_priv->ht_cap & WLAN_RC_DS_FLAG)));
+	if (rate_table->info[rate].valid &&
+	    (ath_rc_priv->ht_cap & WLAN_RC_DS_FLAG))
+		return rate;
+
+	if (rate_table->info[rate].valid_single_stream &&
+	    !(ath_rc_priv->ht_cap & WLAN_RC_DS_FLAG));
+		return rate;
+
+	/*
+	 * This should not happen, but we know it does for now... This
+	 * needs a proper fix but we're still not sure how this is caused.
+	 * Its not *critical* though so lets just warn when debug is enabled
+	 * for configuration changes.
+	 */
+	if (sc->debug.debug_mask & ATH_DBG_RATE)
+		WARN_ON(1);
+
+	rate = ath_rc_priv->valid_rate_index[0];
 
 	return rate;
 }
@@ -837,13 +851,15 @@ static u8 ath_rc_rate_getidx(struct ath_softc *sc,
 
 static void ath_rc_ratefind(struct ath_softc *sc,
 			    struct ath_rate_priv *ath_rc_priv,
-			    struct ieee80211_tx_rate_control *txrc)
+			    struct ieee80211_tx_rate_control *txrc,
+			    struct ieee80211_sta *sta)
 {
 	const struct ath_rate_table *rate_table;
 	struct sk_buff *skb = txrc->skb;
 	struct ieee80211_tx_info *tx_info = IEEE80211_SKB_CB(skb);
 	struct ieee80211_tx_rate *rates = tx_info->control.rates;
 	struct ieee80211_hdr *hdr = (struct ieee80211_hdr *)skb->data;
+	struct ieee80211_supported_band *sband = txrc->sband;
 	__le16 fc = hdr->frame_control;
 	u8 try_per_rate = 0, i = 0, rix, nrix;
 	int is_probe = 0;
@@ -935,6 +951,18 @@ static void ath_rc_ratefind(struct ath_softc *sc,
 
 	/* Setup RTS/CTS */
 	ath_rc_rate_set_rtscts(sc, rate_table, tx_info);
+
+	/*
+	 * Fine tuning for when no decent rate was found, the
+	 * lowest should *not* be used under normal circumstances.
+	 */
+	if (rix == ath_rc_priv->valid_rate_index[0]) {
+		DPRINTF(sc, ATH_DBG_RATE, "lowest rate being used, "
+			"disabling MRR\n");
+		rates[0].idx = rate_lowest_index(sband, sta);
+		/* Disable MRR when ath_rc_ratefind_ht() found rate 0 */
+		rates[1].idx = -1;
+	}
 }
 
 static bool ath_rc_update_per(struct ath_softc *sc,
@@ -1582,7 +1610,7 @@ static void ath_get_rate(void *priv, struct ieee80211_sta *sta, void *priv_sta,
 	}
 
 	/* Find tx rate for unicast frames */
-	ath_rc_ratefind(sc, ath_rc_priv, txrc);
+	ath_rc_ratefind(sc, ath_rc_priv, txrc, sta);
 }
 
 static void ath_rate_init(void *priv, struct ieee80211_supported_band *sband,
-- 
1.6.0.6

[ath9k-devel] [PATCH 01/15] ath9k: fix oops by downgrading assert in rc.c

[Luis R. Rodriguez]

Unfortunately locking the driver rate control private area
doesn't resolve this issue and in fact causes some issues.
The root cause is yet undetermined. For now instead of crashing
lets just downgrades the ASSERT() and only show a trace when
we are debugging with ATH_DBG_CONFIG enabled. That attempt
and its results can be seen here:

http://marc.info/?l=linux-wireless&m=124399166108295&w=1

The ASSERT was happening because the rate control algorithm
figures it should find at least one valid dual stream or
single stream rate. *Something* is causing us to not find
such rate. This happens after association and only when HT
is enabled AFAICT. What we are doing by downgrading the assert
is informing the driver to use the lowest rate index. When
this happens lets disable multi rate retry and only try on
the lowest supported rate.

Traces of the ASSERT are available on this thread:

http://marc.info/?l=linux-wireless&m=124277331319024

At least one bug report (as side issue, mind you):

https://bugzilla.redhat.com/show_bug.cgi?id=503285

Signed-off-by: Luis R. Rodriguez <lrodriguez@atheros.com>
---
 drivers/net/wireless/ath/ath9k/debug.h |    1 +
 drivers/net/wireless/ath/ath9k/rc.c    |   40 +++++++++++++++++++++++++++----
 2 files changed, 35 insertions(+), 6 deletions(-)

diff --git a/drivers/net/wireless/ath/ath9k/debug.h b/drivers/net/wireless/ath/ath9k/debug.h
index edda15b..9d72bc8 100644
--- a/drivers/net/wireless/ath/ath9k/debug.h
+++ b/drivers/net/wireless/ath/ath9k/debug.h
@@ -30,6 +30,7 @@ enum ATH_DEBUG {
 	ATH_DBG_CONFIG		= 0x00000200,
 	ATH_DBG_FATAL		= 0x00000400,
 	ATH_DBG_PS		= 0x00000800,
+	ATH_DBG_RATE		= 0x00001000,
 	ATH_DBG_ANY		= 0xffffffff
 };
 
diff --git a/drivers/net/wireless/ath/ath9k/rc.c b/drivers/net/wireless/ath/ath9k/rc.c
index ba06e78..abad86b 100644
--- a/drivers/net/wireless/ath/ath9k/rc.c
+++ b/drivers/net/wireless/ath/ath9k/rc.c
@@ -741,10 +741,24 @@ static u8 ath_rc_ratefind_ht(struct ath_softc *sc,
 	if (rate > (ath_rc_priv->rate_table_size - 1))
 		rate = ath_rc_priv->rate_table_size - 1;
 
-	ASSERT((rate_table->info[rate].valid &&
-		(ath_rc_priv->ht_cap & WLAN_RC_DS_FLAG)) ||
-	       (rate_table->info[rate].valid_single_stream &&
-		!(ath_rc_priv->ht_cap & WLAN_RC_DS_FLAG)));
+	if (rate_table->info[rate].valid &&
+	    (ath_rc_priv->ht_cap & WLAN_RC_DS_FLAG))
+		return rate;
+
+	if (rate_table->info[rate].valid_single_stream &&
+	    !(ath_rc_priv->ht_cap & WLAN_RC_DS_FLAG));
+		return rate;
+
+	/*
+	 * This should not happen, but we know it does for now... This
+	 * needs a proper fix but we're still not sure how this is caused.
+	 * Its not *critical* though so lets just warn when debug is enabled
+	 * for configuration changes.
+	 */
+	if (sc->debug.debug_mask & ATH_DBG_RATE)
+		WARN_ON(1);
+
+	rate = ath_rc_priv->valid_rate_index[0];
 
 	return rate;
 }
@@ -837,13 +851,15 @@ static u8 ath_rc_rate_getidx(struct ath_softc *sc,
 
 static void ath_rc_ratefind(struct ath_softc *sc,
 			    struct ath_rate_priv *ath_rc_priv,
-			    struct ieee80211_tx_rate_control *txrc)
+			    struct ieee80211_tx_rate_control *txrc,
+			    struct ieee80211_sta *sta)
 {
 	const struct ath_rate_table *rate_table;
 	struct sk_buff *skb = txrc->skb;
 	struct ieee80211_tx_info *tx_info = IEEE80211_SKB_CB(skb);
 	struct ieee80211_tx_rate *rates = tx_info->control.rates;
 	struct ieee80211_hdr *hdr = (struct ieee80211_hdr *)skb->data;
+	struct ieee80211_supported_band *sband = txrc->sband;
 	__le16 fc = hdr->frame_control;
 	u8 try_per_rate = 0, i = 0, rix, nrix;
 	int is_probe = 0;
@@ -935,6 +951,18 @@ static void ath_rc_ratefind(struct ath_softc *sc,
 
 	/* Setup RTS/CTS */
 	ath_rc_rate_set_rtscts(sc, rate_table, tx_info);
+
+	/*
+	 * Fine tuning for when no decent rate was found, the
+	 * lowest should *not* be used under normal circumstances.
+	 */
+	if (rix == ath_rc_priv->valid_rate_index[0]) {
+		DPRINTF(sc, ATH_DBG_RATE, "lowest rate being used, "
+			"disabling MRR\n");
+		rates[0].idx = rate_lowest_index(sband, sta);
+		/* Disable MRR when ath_rc_ratefind_ht() found rate 0 */
+		rates[1].idx = -1;
+	}
 }
 
 static bool ath_rc_update_per(struct ath_softc *sc,
@@ -1582,7 +1610,7 @@ static void ath_get_rate(void *priv, struct ieee80211_sta *sta, void *priv_sta,
 	}
 
 	/* Find tx rate for unicast frames */
-	ath_rc_ratefind(sc, ath_rc_priv, txrc);
+	ath_rc_ratefind(sc, ath_rc_priv, txrc, sta);
 }
 
 static void ath_rate_init(void *priv, struct ieee80211_supported_band *sband,
-- 
1.6.0.6

[PATCH 02/15] ath9k: cleanup try count for MRR in rate control

[Luis R. Rodriguez]

This has no functional change and just cleans up the code
to be more legible and removes a useless variable for
Multi Rate Retry.

For regular frames we use 2 retries for MRR segments [0-2].
For the last MRR segment [3] we use 4.

MRR[0] = 2
MRR[1] = 2
MRR[2] = 2
MRR[3] = 4

Cc: Derek Smithies <derek@indranet.co.nz>
Cc: Chittajit Mitra <Chittajit.Mitra@Atheros.com>
Signed-off-by: Luis R. Rodriguez <lrodriguez@atheros.com>
---
 drivers/net/wireless/ath/ath9k/ath9k.h |    1 -
 drivers/net/wireless/ath/ath9k/main.c  |    3 ++-
 drivers/net/wireless/ath/ath9k/rc.c    |   29 ++++++++++++++++++++---------
 3 files changed, 22 insertions(+), 11 deletions(-)

diff --git a/drivers/net/wireless/ath/ath9k/ath9k.h b/drivers/net/wireless/ath/ath9k/ath9k.h
index 9cd523f..ca14642 100644
--- a/drivers/net/wireless/ath/ath9k/ath9k.h
+++ b/drivers/net/wireless/ath/ath9k/ath9k.h
@@ -169,7 +169,6 @@ void ath_descdma_cleanup(struct ath_softc *sc, struct ath_descdma *dd,
 #define WME_NUM_TID             16
 #define ATH_TXBUF               512
 #define ATH_TXMAXTRY            13
-#define ATH_11N_TXMAXTRY        10
 #define ATH_MGT_TXMAXTRY        4
 #define WME_BA_BMP_SIZE         64
 #define WME_MAX_BA              WME_BA_BMP_SIZE
diff --git a/drivers/net/wireless/ath/ath9k/main.c b/drivers/net/wireless/ath/ath9k/main.c
index f7baa40..2270166 100644
--- a/drivers/net/wireless/ath/ath9k/main.c
+++ b/drivers/net/wireless/ath/ath9k/main.c
@@ -1573,7 +1573,8 @@ void ath_set_hw_capab(struct ath_softc *sc, struct ieee80211_hw *hw)
 	hw->max_rates = 4;
 	hw->channel_change_time = 5000;
 	hw->max_listen_interval = 10;
-	hw->max_rate_tries = ATH_11N_TXMAXTRY;
+	/* Hardware supports 10 but we use 4 */
+	hw->max_rate_tries = 4;
 	hw->sta_data_size = sizeof(struct ath_node);
 	hw->vif_data_size = sizeof(struct ath_vif);
 
diff --git a/drivers/net/wireless/ath/ath9k/rc.c b/drivers/net/wireless/ath/ath9k/rc.c
index abad86b..4f619ae 100644
--- a/drivers/net/wireless/ath/ath9k/rc.c
+++ b/drivers/net/wireless/ath/ath9k/rc.c
@@ -861,9 +861,21 @@ static void ath_rc_ratefind(struct ath_softc *sc,
 	struct ieee80211_hdr *hdr = (struct ieee80211_hdr *)skb->data;
 	struct ieee80211_supported_band *sband = txrc->sband;
 	__le16 fc = hdr->frame_control;
-	u8 try_per_rate = 0, i = 0, rix, nrix;
+	u8 try_per_rate, i = 0, rix, nrix;
 	int is_probe = 0;
 
+	/*
+	 * For Multi Rate Retry we use a different number of
+	 * retry attempt counts. This ends up looking like this:
+	 *
+	 * MRR[0] = 2
+	 * MRR[1] = 2
+	 * MRR[2] = 2
+	 * MRR[3] = 4
+	 *
+	 */
+	try_per_rate = sc->hw->max_rate_tries;
+
 	rate_table = sc->cur_rate_table;
 	rix = ath_rc_ratefind_ht(sc, ath_rc_priv, rate_table, &is_probe);
 	nrix = rix;
@@ -874,7 +886,6 @@ static void ath_rc_ratefind(struct ath_softc *sc,
 		ath_rc_rate_set_series(rate_table, &rates[i++], txrc,
 				       1, nrix, 0);
 
-		try_per_rate = (ATH_11N_TXMAXTRY/4);
 		/* Get the next tried/allowed rate. No RTS for the next series
 		 * after the probe rate
 		 */
@@ -885,7 +896,6 @@ static void ath_rc_ratefind(struct ath_softc *sc,
 
 		tx_info->flags |= IEEE80211_TX_CTL_RATE_CTRL_PROBE;
 	} else {
-		try_per_rate = (ATH_11N_TXMAXTRY/4);
 		/* Set the choosen rate. No RTS for first series entry. */
 		ath_rc_rate_set_series(rate_table, &rates[i++], txrc,
 				       try_per_rate, nrix, 0);
@@ -893,18 +903,19 @@ static void ath_rc_ratefind(struct ath_softc *sc,
 
 	/* Fill in the other rates for multirate retry */
 	for ( ; i < 4; i++) {
-		u8 try_num;
 		u8 min_rate;
 
-		try_num = ((i + 1) == 4) ?
-			ATH_11N_TXMAXTRY - (try_per_rate * i) : try_per_rate ;
+		/* Use twice the number of tries for the last MRR segment. */
+		if (i + 1 == 4)
+			try_per_rate = 4;
+
 		min_rate = (((i + 1) == 4) && 0);
 
 		nrix = ath_rc_rate_getidx(sc, ath_rc_priv,
 					  rate_table, nrix, 1, min_rate);
 		/* All other rates in the series have RTS enabled */
 		ath_rc_rate_set_series(rate_table, &rates[i], txrc,
-				       try_num, nrix, 1);
+				       try_per_rate, nrix, 1);
 	}
 
 	/*
@@ -1549,7 +1560,7 @@ static void ath_tx_status(void *priv, struct ieee80211_supported_band *sband,
 	/*
 	 * If underrun error is seen assume it as an excessive retry only
 	 * if prefetch trigger level have reached the max (0x3f for 5416)
-	 * Adjust the long retry as if the frame was tried ATH_11N_TXMAXTRY
+	 * Adjust the long retry as if the frame was tried hw->max_rate_tries
 	 * times. This affects how ratectrl updates PER for the failed rate.
 	 */
 	if (tx_info_priv->tx.ts_flags &
@@ -1564,7 +1575,7 @@ static void ath_tx_status(void *priv, struct ieee80211_supported_band *sband,
 		tx_status = 1;
 
 	ath_rc_tx_status(sc, ath_rc_priv, tx_info, final_ts_idx, tx_status,
-			 (is_underrun) ? ATH_11N_TXMAXTRY :
+			 (is_underrun) ? sc->hw->max_rate_tries :
 			 tx_info_priv->tx.ts_longretry);
 
 	/* Check if aggregation has to be enabled for this tid */
-- 
1.6.0.6

[ath9k-devel] [PATCH 02/15] ath9k: cleanup try count for MRR in rate control

[Luis R. Rodriguez]

This has no functional change and just cleans up the code
to be more legible and removes a useless variable for
Multi Rate Retry.

For regular frames we use 2 retries for MRR segments [0-2].
For the last MRR segment [3] we use 4.

MRR[0] = 2
MRR[1] = 2
MRR[2] = 2
MRR[3] = 4

Cc: Derek Smithies <derek@indranet.co.nz>
Cc: Chittajit Mitra <Chittajit.Mitra@Atheros.com>
Signed-off-by: Luis R. Rodriguez <lrodriguez@atheros.com>
---
 drivers/net/wireless/ath/ath9k/ath9k.h |    1 -
 drivers/net/wireless/ath/ath9k/main.c  |    3 ++-
 drivers/net/wireless/ath/ath9k/rc.c    |   29 ++++++++++++++++++++---------
 3 files changed, 22 insertions(+), 11 deletions(-)

diff --git a/drivers/net/wireless/ath/ath9k/ath9k.h b/drivers/net/wireless/ath/ath9k/ath9k.h
index 9cd523f..ca14642 100644
--- a/drivers/net/wireless/ath/ath9k/ath9k.h
+++ b/drivers/net/wireless/ath/ath9k/ath9k.h
@@ -169,7 +169,6 @@ void ath_descdma_cleanup(struct ath_softc *sc, struct ath_descdma *dd,
 #define WME_NUM_TID             16
 #define ATH_TXBUF               512
 #define ATH_TXMAXTRY            13
-#define ATH_11N_TXMAXTRY        10
 #define ATH_MGT_TXMAXTRY        4
 #define WME_BA_BMP_SIZE         64
 #define WME_MAX_BA              WME_BA_BMP_SIZE
diff --git a/drivers/net/wireless/ath/ath9k/main.c b/drivers/net/wireless/ath/ath9k/main.c
index f7baa40..2270166 100644
--- a/drivers/net/wireless/ath/ath9k/main.c
+++ b/drivers/net/wireless/ath/ath9k/main.c
@@ -1573,7 +1573,8 @@ void ath_set_hw_capab(struct ath_softc *sc, struct ieee80211_hw *hw)
 	hw->max_rates = 4;
 	hw->channel_change_time = 5000;
 	hw->max_listen_interval = 10;
-	hw->max_rate_tries = ATH_11N_TXMAXTRY;
+	/* Hardware supports 10 but we use 4 */
+	hw->max_rate_tries = 4;
 	hw->sta_data_size = sizeof(struct ath_node);
 	hw->vif_data_size = sizeof(struct ath_vif);
 
diff --git a/drivers/net/wireless/ath/ath9k/rc.c b/drivers/net/wireless/ath/ath9k/rc.c
index abad86b..4f619ae 100644
--- a/drivers/net/wireless/ath/ath9k/rc.c
+++ b/drivers/net/wireless/ath/ath9k/rc.c
@@ -861,9 +861,21 @@ static void ath_rc_ratefind(struct ath_softc *sc,
 	struct ieee80211_hdr *hdr = (struct ieee80211_hdr *)skb->data;
 	struct ieee80211_supported_band *sband = txrc->sband;
 	__le16 fc = hdr->frame_control;
-	u8 try_per_rate = 0, i = 0, rix, nrix;
+	u8 try_per_rate, i = 0, rix, nrix;
 	int is_probe = 0;
 
+	/*
+	 * For Multi Rate Retry we use a different number of
+	 * retry attempt counts. This ends up looking like this:
+	 *
+	 * MRR[0] = 2
+	 * MRR[1] = 2
+	 * MRR[2] = 2
+	 * MRR[3] = 4
+	 *
+	 */
+	try_per_rate = sc->hw->max_rate_tries;
+
 	rate_table = sc->cur_rate_table;
 	rix = ath_rc_ratefind_ht(sc, ath_rc_priv, rate_table, &is_probe);
 	nrix = rix;
@@ -874,7 +886,6 @@ static void ath_rc_ratefind(struct ath_softc *sc,
 		ath_rc_rate_set_series(rate_table, &rates[i++], txrc,
 				       1, nrix, 0);
 
-		try_per_rate = (ATH_11N_TXMAXTRY/4);
 		/* Get the next tried/allowed rate. No RTS for the next series
 		 * after the probe rate
 		 */
@@ -885,7 +896,6 @@ static void ath_rc_ratefind(struct ath_softc *sc,
 
 		tx_info->flags |= IEEE80211_TX_CTL_RATE_CTRL_PROBE;
 	} else {
-		try_per_rate = (ATH_11N_TXMAXTRY/4);
 		/* Set the choosen rate. No RTS for first series entry. */
 		ath_rc_rate_set_series(rate_table, &rates[i++], txrc,
 				       try_per_rate, nrix, 0);
@@ -893,18 +903,19 @@ static void ath_rc_ratefind(struct ath_softc *sc,
 
 	/* Fill in the other rates for multirate retry */
 	for ( ; i < 4; i++) {
-		u8 try_num;
 		u8 min_rate;
 
-		try_num = ((i + 1) == 4) ?
-			ATH_11N_TXMAXTRY - (try_per_rate * i) : try_per_rate ;
+		/* Use twice the number of tries for the last MRR segment. */
+		if (i + 1 == 4)
+			try_per_rate = 4;
+
 		min_rate = (((i + 1) == 4) && 0);
 
 		nrix = ath_rc_rate_getidx(sc, ath_rc_priv,
 					  rate_table, nrix, 1, min_rate);
 		/* All other rates in the series have RTS enabled */
 		ath_rc_rate_set_series(rate_table, &rates[i], txrc,
-				       try_num, nrix, 1);
+				       try_per_rate, nrix, 1);
 	}
 
 	/*
@@ -1549,7 +1560,7 @@ static void ath_tx_status(void *priv, struct ieee80211_supported_band *sband,
 	/*
 	 * If underrun error is seen assume it as an excessive retry only
 	 * if prefetch trigger level have reached the max (0x3f for 5416)
-	 * Adjust the long retry as if the frame was tried ATH_11N_TXMAXTRY
+	 * Adjust the long retry as if the frame was tried hw->max_rate_tries
 	 * times. This affects how ratectrl updates PER for the failed rate.
 	 */
 	if (tx_info_priv->tx.ts_flags &
@@ -1564,7 +1575,7 @@ static void ath_tx_status(void *priv, struct ieee80211_supported_band *sband,
 		tx_status = 1;
 
 	ath_rc_tx_status(sc, ath_rc_priv, tx_info, final_ts_idx, tx_status,
-			 (is_underrun) ? ATH_11N_TXMAXTRY :
+			 (is_underrun) ? sc->hw->max_rate_tries :
 			 tx_info_priv->tx.ts_longretry);
 
 	/* Check if aggregation has to be enabled for this tid */
-- 
1.6.0.6

[PATCH 03/15] ath9k: remove unused min rate calculation code

[Luis R. Rodriguez]

This is not used, and when we need to get the lowest rate
we should simply use mac80211's own rate_lowest_index(sband, sta).

Cc: Derek Smithies <derek@indranet.co.nz>
Cc: Chittajit Mitra <Chittajit.Mitra@Atheros.com>
Signed-off-by: Luis R. Rodriguez <lrodriguez@atheros.com>
---
 drivers/net/wireless/ath/ath9k/rc.c |   33 +++++++++------------------------
 1 files changed, 9 insertions(+), 24 deletions(-)

diff --git a/drivers/net/wireless/ath/ath9k/rc.c b/drivers/net/wireless/ath/ath9k/rc.c
index 4f619ae..f62361c 100644
--- a/drivers/net/wireless/ath/ath9k/rc.c
+++ b/drivers/net/wireless/ath/ath9k/rc.c
@@ -823,28 +823,17 @@ static void ath_rc_rate_set_rtscts(struct ath_softc *sc,
 static u8 ath_rc_rate_getidx(struct ath_softc *sc,
 			     struct ath_rate_priv *ath_rc_priv,
 			     const struct ath_rate_table *rate_table,
-			     u8 rix, u16 stepdown,
-			     u16 min_rate)
+			     u8 rix, u16 stepdown)
 {
 	u32 j;
 	u8 nextindex = 0;
 
-	if (min_rate) {
-		for (j = RATE_TABLE_SIZE; j > 0; j--) {
-			if (ath_rc_get_nextlowervalid_txrate(rate_table,
-						ath_rc_priv, rix, &nextindex))
-				rix = nextindex;
-			else
-				break;
-		}
-	} else {
-		for (j = stepdown; j > 0; j--) {
-			if (ath_rc_get_nextlowervalid_txrate(rate_table,
-						ath_rc_priv, rix, &nextindex))
-				rix = nextindex;
-			else
-				break;
-		}
+	for (j = stepdown; j > 0; j--) {
+		if (ath_rc_get_nextlowervalid_txrate(rate_table,
+					ath_rc_priv, rix, &nextindex))
+			rix = nextindex;
+		else
+			break;
 	}
 	return rix;
 }
@@ -890,7 +879,7 @@ static void ath_rc_ratefind(struct ath_softc *sc,
 		 * after the probe rate
 		 */
 		nrix = ath_rc_rate_getidx(sc, ath_rc_priv,
-					  rate_table, nrix, 1, 0);
+					  rate_table, nrix, 1);
 		ath_rc_rate_set_series(rate_table, &rates[i++], txrc,
 				       try_per_rate, nrix, 0);
 
@@ -903,16 +892,12 @@ static void ath_rc_ratefind(struct ath_softc *sc,
 
 	/* Fill in the other rates for multirate retry */
 	for ( ; i < 4; i++) {
-		u8 min_rate;
-
 		/* Use twice the number of tries for the last MRR segment. */
 		if (i + 1 == 4)
 			try_per_rate = 4;
 
-		min_rate = (((i + 1) == 4) && 0);
-
 		nrix = ath_rc_rate_getidx(sc, ath_rc_priv,
-					  rate_table, nrix, 1, min_rate);
+					  rate_table, nrix, 1);
 		/* All other rates in the series have RTS enabled */
 		ath_rc_rate_set_series(rate_table, &rates[i], txrc,
 				       try_per_rate, nrix, 1);
-- 
1.6.0.6

[ath9k-devel] [PATCH 03/15] ath9k: remove unused min rate calculation code

[Luis R. Rodriguez]

This is not used, and when we need to get the lowest rate
we should simply use mac80211's own rate_lowest_index(sband, sta).

Cc: Derek Smithies <derek@indranet.co.nz>
Cc: Chittajit Mitra <Chittajit.Mitra@Atheros.com>
Signed-off-by: Luis R. Rodriguez <lrodriguez@atheros.com>
---
 drivers/net/wireless/ath/ath9k/rc.c |   33 +++++++++------------------------
 1 files changed, 9 insertions(+), 24 deletions(-)

diff --git a/drivers/net/wireless/ath/ath9k/rc.c b/drivers/net/wireless/ath/ath9k/rc.c
index 4f619ae..f62361c 100644
--- a/drivers/net/wireless/ath/ath9k/rc.c
+++ b/drivers/net/wireless/ath/ath9k/rc.c
@@ -823,28 +823,17 @@ static void ath_rc_rate_set_rtscts(struct ath_softc *sc,
 static u8 ath_rc_rate_getidx(struct ath_softc *sc,
 			     struct ath_rate_priv *ath_rc_priv,
 			     const struct ath_rate_table *rate_table,
-			     u8 rix, u16 stepdown,
-			     u16 min_rate)
+			     u8 rix, u16 stepdown)
 {
 	u32 j;
 	u8 nextindex = 0;
 
-	if (min_rate) {
-		for (j = RATE_TABLE_SIZE; j > 0; j--) {
-			if (ath_rc_get_nextlowervalid_txrate(rate_table,
-						ath_rc_priv, rix, &nextindex))
-				rix = nextindex;
-			else
-				break;
-		}
-	} else {
-		for (j = stepdown; j > 0; j--) {
-			if (ath_rc_get_nextlowervalid_txrate(rate_table,
-						ath_rc_priv, rix, &nextindex))
-				rix = nextindex;
-			else
-				break;
-		}
+	for (j = stepdown; j > 0; j--) {
+		if (ath_rc_get_nextlowervalid_txrate(rate_table,
+					ath_rc_priv, rix, &nextindex))
+			rix = nextindex;
+		else
+			break;
 	}
 	return rix;
 }
@@ -890,7 +879,7 @@ static void ath_rc_ratefind(struct ath_softc *sc,
 		 * after the probe rate
 		 */
 		nrix = ath_rc_rate_getidx(sc, ath_rc_priv,
-					  rate_table, nrix, 1, 0);
+					  rate_table, nrix, 1);
 		ath_rc_rate_set_series(rate_table, &rates[i++], txrc,
 				       try_per_rate, nrix, 0);
 
@@ -903,16 +892,12 @@ static void ath_rc_ratefind(struct ath_softc *sc,
 
 	/* Fill in the other rates for multirate retry */
 	for ( ; i < 4; i++) {
-		u8 min_rate;
-
 		/* Use twice the number of tries for the last MRR segment. */
 		if (i + 1 == 4)
 			try_per_rate = 4;
 
-		min_rate = (((i + 1) == 4) && 0);
-
 		nrix = ath_rc_rate_getidx(sc, ath_rc_priv,
-					  rate_table, nrix, 1, min_rate);
+					  rate_table, nrix, 1);
 		/* All other rates in the series have RTS enabled */
 		ath_rc_rate_set_series(rate_table, &rates[i], txrc,
 				       try_per_rate, nrix, 1);
-- 
1.6.0.6

[PATCH 04/15] ath9k: remove unused stepdown when looking for the next rate

[Luis R. Rodriguez]

This is not used, remove this.

Cc: Derek Smithies <derek@indranet.co.nz>
Cc: Chittajit Mitra <Chittajit.Mitra@Atheros.com>
Signed-off-by: Luis R. Rodriguez <lrodriguez@atheros.com>
---
 drivers/net/wireless/ath/ath9k/rc.c |   21 ++++++++-------------
 1 files changed, 8 insertions(+), 13 deletions(-)

diff --git a/drivers/net/wireless/ath/ath9k/rc.c b/drivers/net/wireless/ath/ath9k/rc.c
index f62361c..557f054 100644
--- a/drivers/net/wireless/ath/ath9k/rc.c
+++ b/drivers/net/wireless/ath/ath9k/rc.c
@@ -823,19 +823,14 @@ static void ath_rc_rate_set_rtscts(struct ath_softc *sc,
 static u8 ath_rc_rate_getidx(struct ath_softc *sc,
 			     struct ath_rate_priv *ath_rc_priv,
 			     const struct ath_rate_table *rate_table,
-			     u8 rix, u16 stepdown)
+			     u8 rix)
 {
-	u32 j;
 	u8 nextindex = 0;
-
-	for (j = stepdown; j > 0; j--) {
-		if (ath_rc_get_nextlowervalid_txrate(rate_table,
-					ath_rc_priv, rix, &nextindex))
-			rix = nextindex;
-		else
-			break;
-	}
-	return rix;
+	if (ath_rc_get_nextlowervalid_txrate(rate_table,
+					     ath_rc_priv, rix, &nextindex))
+		return nextindex;
+	else
+		return rix;
 }
 
 static void ath_rc_ratefind(struct ath_softc *sc,
@@ -879,7 +874,7 @@ static void ath_rc_ratefind(struct ath_softc *sc,
 		 * after the probe rate
 		 */
 		nrix = ath_rc_rate_getidx(sc, ath_rc_priv,
-					  rate_table, nrix, 1);
+					  rate_table, nrix);
 		ath_rc_rate_set_series(rate_table, &rates[i++], txrc,
 				       try_per_rate, nrix, 0);
 
@@ -897,7 +892,7 @@ static void ath_rc_ratefind(struct ath_softc *sc,
 			try_per_rate = 4;
 
 		nrix = ath_rc_rate_getidx(sc, ath_rc_priv,
-					  rate_table, nrix, 1);
+					  rate_table, nrix);
 		/* All other rates in the series have RTS enabled */
 		ath_rc_rate_set_series(rate_table, &rates[i], txrc,
 				       try_per_rate, nrix, 1);
-- 
1.6.0.6

[ath9k-devel] [PATCH 04/15] ath9k: remove unused stepdown when looking for the next rate

[Luis R. Rodriguez]

This is not used, remove this.

Cc: Derek Smithies <derek@indranet.co.nz>
Cc: Chittajit Mitra <Chittajit.Mitra@Atheros.com>
Signed-off-by: Luis R. Rodriguez <lrodriguez@atheros.com>
---
 drivers/net/wireless/ath/ath9k/rc.c |   21 ++++++++-------------
 1 files changed, 8 insertions(+), 13 deletions(-)

diff --git a/drivers/net/wireless/ath/ath9k/rc.c b/drivers/net/wireless/ath/ath9k/rc.c
index f62361c..557f054 100644
--- a/drivers/net/wireless/ath/ath9k/rc.c
+++ b/drivers/net/wireless/ath/ath9k/rc.c
@@ -823,19 +823,14 @@ static void ath_rc_rate_set_rtscts(struct ath_softc *sc,
 static u8 ath_rc_rate_getidx(struct ath_softc *sc,
 			     struct ath_rate_priv *ath_rc_priv,
 			     const struct ath_rate_table *rate_table,
-			     u8 rix, u16 stepdown)
+			     u8 rix)
 {
-	u32 j;
 	u8 nextindex = 0;
-
-	for (j = stepdown; j > 0; j--) {
-		if (ath_rc_get_nextlowervalid_txrate(rate_table,
-					ath_rc_priv, rix, &nextindex))
-			rix = nextindex;
-		else
-			break;
-	}
-	return rix;
+	if (ath_rc_get_nextlowervalid_txrate(rate_table,
+					     ath_rc_priv, rix, &nextindex))
+		return nextindex;
+	else
+		return rix;
 }
 
 static void ath_rc_ratefind(struct ath_softc *sc,
@@ -879,7 +874,7 @@ static void ath_rc_ratefind(struct ath_softc *sc,
 		 * after the probe rate
 		 */
 		nrix = ath_rc_rate_getidx(sc, ath_rc_priv,
-					  rate_table, nrix, 1);
+					  rate_table, nrix);
 		ath_rc_rate_set_series(rate_table, &rates[i++], txrc,
 				       try_per_rate, nrix, 0);
 
@@ -897,7 +892,7 @@ static void ath_rc_ratefind(struct ath_softc *sc,
 			try_per_rate = 4;
 
 		nrix = ath_rc_rate_getidx(sc, ath_rc_priv,
-					  rate_table, nrix, 1);
+					  rate_table, nrix);
 		/* All other rates in the series have RTS enabled */
 		ath_rc_rate_set_series(rate_table, &rates[i], txrc,
 				       try_per_rate, nrix, 1);
-- 
1.6.0.6

[PATCH 05/15] ath9k: remove pointless wrapper ath_rc_rate_getidx()

[Luis R. Rodriguez]

This is just calling another helper, so just use the other
helper directly. This should make it clear that when do not
find the next rate we stick to the current one.

Cc: Derek Smithies <derek@indranet.co.nz>
Cc: Chittajit Mitra <Chittajit.Mitra@Atheros.com>
Signed-off-by: Luis R. Rodriguez <lrodriguez@atheros.com>
---
 drivers/net/wireless/ath/ath9k/rc.c |   21 ++++-----------------
 1 files changed, 4 insertions(+), 17 deletions(-)

diff --git a/drivers/net/wireless/ath/ath9k/rc.c b/drivers/net/wireless/ath/ath9k/rc.c
index 557f054..6ccd565 100644
--- a/drivers/net/wireless/ath/ath9k/rc.c
+++ b/drivers/net/wireless/ath/ath9k/rc.c
@@ -820,19 +820,6 @@ static void ath_rc_rate_set_rtscts(struct ath_softc *sc,
 	tx_info->control.rts_cts_rate_idx = cix;
 }
 
-static u8 ath_rc_rate_getidx(struct ath_softc *sc,
-			     struct ath_rate_priv *ath_rc_priv,
-			     const struct ath_rate_table *rate_table,
-			     u8 rix)
-{
-	u8 nextindex = 0;
-	if (ath_rc_get_nextlowervalid_txrate(rate_table,
-					     ath_rc_priv, rix, &nextindex))
-		return nextindex;
-	else
-		return rix;
-}
-
 static void ath_rc_ratefind(struct ath_softc *sc,
 			    struct ath_rate_priv *ath_rc_priv,
 			    struct ieee80211_tx_rate_control *txrc,
@@ -873,8 +860,8 @@ static void ath_rc_ratefind(struct ath_softc *sc,
 		/* Get the next tried/allowed rate. No RTS for the next series
 		 * after the probe rate
 		 */
-		nrix = ath_rc_rate_getidx(sc, ath_rc_priv,
-					  rate_table, nrix);
+		ath_rc_get_nextlowervalid_txrate(rate_table, ath_rc_priv,
+						 rix, &nrix);
 		ath_rc_rate_set_series(rate_table, &rates[i++], txrc,
 				       try_per_rate, nrix, 0);
 
@@ -891,8 +878,8 @@ static void ath_rc_ratefind(struct ath_softc *sc,
 		if (i + 1 == 4)
 			try_per_rate = 4;
 
-		nrix = ath_rc_rate_getidx(sc, ath_rc_priv,
-					  rate_table, nrix);
+		ath_rc_get_nextlowervalid_txrate(rate_table, ath_rc_priv,
+						 rix, &nrix);
 		/* All other rates in the series have RTS enabled */
 		ath_rc_rate_set_series(rate_table, &rates[i], txrc,
 				       try_per_rate, nrix, 1);
-- 
1.6.0.6

[ath9k-devel] [PATCH 05/15] ath9k: remove pointless wrapper ath_rc_rate_getidx()

[Luis R. Rodriguez]

This is just calling another helper, so just use the other
helper directly. This should make it clear that when do not
find the next rate we stick to the current one.

Cc: Derek Smithies <derek@indranet.co.nz>
Cc: Chittajit Mitra <Chittajit.Mitra@Atheros.com>
Signed-off-by: Luis R. Rodriguez <lrodriguez@atheros.com>
---
 drivers/net/wireless/ath/ath9k/rc.c |   21 ++++-----------------
 1 files changed, 4 insertions(+), 17 deletions(-)

diff --git a/drivers/net/wireless/ath/ath9k/rc.c b/drivers/net/wireless/ath/ath9k/rc.c
index 557f054..6ccd565 100644
--- a/drivers/net/wireless/ath/ath9k/rc.c
+++ b/drivers/net/wireless/ath/ath9k/rc.c
@@ -820,19 +820,6 @@ static void ath_rc_rate_set_rtscts(struct ath_softc *sc,
 	tx_info->control.rts_cts_rate_idx = cix;
 }
 
-static u8 ath_rc_rate_getidx(struct ath_softc *sc,
-			     struct ath_rate_priv *ath_rc_priv,
-			     const struct ath_rate_table *rate_table,
-			     u8 rix)
-{
-	u8 nextindex = 0;
-	if (ath_rc_get_nextlowervalid_txrate(rate_table,
-					     ath_rc_priv, rix, &nextindex))
-		return nextindex;
-	else
-		return rix;
-}
-
 static void ath_rc_ratefind(struct ath_softc *sc,
 			    struct ath_rate_priv *ath_rc_priv,
 			    struct ieee80211_tx_rate_control *txrc,
@@ -873,8 +860,8 @@ static void ath_rc_ratefind(struct ath_softc *sc,
 		/* Get the next tried/allowed rate. No RTS for the next series
 		 * after the probe rate
 		 */
-		nrix = ath_rc_rate_getidx(sc, ath_rc_priv,
-					  rate_table, nrix);
+		ath_rc_get_nextlowervalid_txrate(rate_table, ath_rc_priv,
+						 rix, &nrix);
 		ath_rc_rate_set_series(rate_table, &rates[i++], txrc,
 				       try_per_rate, nrix, 0);
 
@@ -891,8 +878,8 @@ static void ath_rc_ratefind(struct ath_softc *sc,
 		if (i + 1 == 4)
 			try_per_rate = 4;
 
-		nrix = ath_rc_rate_getidx(sc, ath_rc_priv,
-					  rate_table, nrix);
+		ath_rc_get_nextlowervalid_txrate(rate_table, ath_rc_priv,
+						 rix, &nrix);
 		/* All other rates in the series have RTS enabled */
 		ath_rc_rate_set_series(rate_table, &rates[i], txrc,
 				       try_per_rate, nrix, 1);
-- 
1.6.0.6

[PATCH 06/15] ath9k: rename ath_rc_get_nextlowervalid_txrate()

[Luis R. Rodriguez]

What this does is get us our next lower rate so call it that,
ath_rc_get_lower_rix().

Cc: Derek Smithies <derek@indranet.co.nz>
Cc: Chittajit Mitra <Chittajit.Mitra@Atheros.com>
Signed-off-by: Luis R. Rodriguez <lrodriguez@atheros.com>
---
 drivers/net/wireless/ath/ath9k/rc.c |   16 +++++++---------
 1 files changed, 7 insertions(+), 9 deletions(-)

diff --git a/drivers/net/wireless/ath/ath9k/rc.c b/drivers/net/wireless/ath/ath9k/rc.c
index 6ccd565..eaa22e4 100644
--- a/drivers/net/wireless/ath/ath9k/rc.c
+++ b/drivers/net/wireless/ath/ath9k/rc.c
@@ -501,9 +501,9 @@ static int ath_rc_valid_phyrate(u32 phy, u32 capflag, int ignore_cw)
 }
 
 static inline int
-ath_rc_get_nextlowervalid_txrate(const struct ath_rate_table *rate_table,
-				 struct ath_rate_priv *ath_rc_priv,
-				 u8 cur_valid_txrate, u8 *next_idx)
+ath_rc_get_lower_rix(const struct ath_rate_table *rate_table,
+		     struct ath_rate_priv *ath_rc_priv,
+		     u8 cur_valid_txrate, u8 *next_idx)
 {
 	int8_t i;
 
@@ -860,8 +860,7 @@ static void ath_rc_ratefind(struct ath_softc *sc,
 		/* Get the next tried/allowed rate. No RTS for the next series
 		 * after the probe rate
 		 */
-		ath_rc_get_nextlowervalid_txrate(rate_table, ath_rc_priv,
-						 rix, &nrix);
+		ath_rc_get_lower_rix(rate_table, ath_rc_priv, rix, &nrix);
 		ath_rc_rate_set_series(rate_table, &rates[i++], txrc,
 				       try_per_rate, nrix, 0);
 
@@ -878,8 +877,7 @@ static void ath_rc_ratefind(struct ath_softc *sc,
 		if (i + 1 == 4)
 			try_per_rate = 4;
 
-		ath_rc_get_nextlowervalid_txrate(rate_table, ath_rc_priv,
-						 rix, &nrix);
+		ath_rc_get_lower_rix(rate_table, ath_rc_priv, rix, &nrix);
 		/* All other rates in the series have RTS enabled */
 		ath_rc_rate_set_series(rate_table, &rates[i], txrc,
 				       try_per_rate, nrix, 1);
@@ -1176,8 +1174,8 @@ static void ath_rc_update_ht(struct ath_softc *sc,
 	if (ath_rc_priv->state[tx_rate].per >= 55 && tx_rate > 0 &&
 	    rate_table->info[tx_rate].ratekbps <=
 	    rate_table->info[ath_rc_priv->rate_max_phy].ratekbps) {
-		ath_rc_get_nextlowervalid_txrate(rate_table, ath_rc_priv,
-				 (u8)tx_rate, &ath_rc_priv->rate_max_phy);
+		ath_rc_get_lower_rix(rate_table, ath_rc_priv,
+				     (u8)tx_rate, &ath_rc_priv->rate_max_phy);
 
 		/* Don't probe for a little while. */
 		ath_rc_priv->probe_time = now_msec;
-- 
1.6.0.6

[ath9k-devel] [PATCH 06/15] ath9k: rename ath_rc_get_nextlowervalid_txrate()

[Luis R. Rodriguez]

What this does is get us our next lower rate so call it that,
ath_rc_get_lower_rix().

Cc: Derek Smithies <derek@indranet.co.nz>
Cc: Chittajit Mitra <Chittajit.Mitra@Atheros.com>
Signed-off-by: Luis R. Rodriguez <lrodriguez@atheros.com>
---
 drivers/net/wireless/ath/ath9k/rc.c |   16 +++++++---------
 1 files changed, 7 insertions(+), 9 deletions(-)

diff --git a/drivers/net/wireless/ath/ath9k/rc.c b/drivers/net/wireless/ath/ath9k/rc.c
index 6ccd565..eaa22e4 100644
--- a/drivers/net/wireless/ath/ath9k/rc.c
+++ b/drivers/net/wireless/ath/ath9k/rc.c
@@ -501,9 +501,9 @@ static int ath_rc_valid_phyrate(u32 phy, u32 capflag, int ignore_cw)
 }
 
 static inline int
-ath_rc_get_nextlowervalid_txrate(const struct ath_rate_table *rate_table,
-				 struct ath_rate_priv *ath_rc_priv,
-				 u8 cur_valid_txrate, u8 *next_idx)
+ath_rc_get_lower_rix(const struct ath_rate_table *rate_table,
+		     struct ath_rate_priv *ath_rc_priv,
+		     u8 cur_valid_txrate, u8 *next_idx)
 {
 	int8_t i;
 
@@ -860,8 +860,7 @@ static void ath_rc_ratefind(struct ath_softc *sc,
 		/* Get the next tried/allowed rate. No RTS for the next series
 		 * after the probe rate
 		 */
-		ath_rc_get_nextlowervalid_txrate(rate_table, ath_rc_priv,
-						 rix, &nrix);
+		ath_rc_get_lower_rix(rate_table, ath_rc_priv, rix, &nrix);
 		ath_rc_rate_set_series(rate_table, &rates[i++], txrc,
 				       try_per_rate, nrix, 0);
 
@@ -878,8 +877,7 @@ static void ath_rc_ratefind(struct ath_softc *sc,
 		if (i + 1 == 4)
 			try_per_rate = 4;
 
-		ath_rc_get_nextlowervalid_txrate(rate_table, ath_rc_priv,
-						 rix, &nrix);
+		ath_rc_get_lower_rix(rate_table, ath_rc_priv, rix, &nrix);
 		/* All other rates in the series have RTS enabled */
 		ath_rc_rate_set_series(rate_table, &rates[i], txrc,
 				       try_per_rate, nrix, 1);
@@ -1176,8 +1174,8 @@ static void ath_rc_update_ht(struct ath_softc *sc,
 	if (ath_rc_priv->state[tx_rate].per >= 55 && tx_rate > 0 &&
 	    rate_table->info[tx_rate].ratekbps <=
 	    rate_table->info[ath_rc_priv->rate_max_phy].ratekbps) {
-		ath_rc_get_nextlowervalid_txrate(rate_table, ath_rc_priv,
-				 (u8)tx_rate, &ath_rc_priv->rate_max_phy);
+		ath_rc_get_lower_rix(rate_table, ath_rc_priv,
+				     (u8)tx_rate, &ath_rc_priv->rate_max_phy);
 
 		/* Don't probe for a little while. */
 		ath_rc_priv->probe_time = now_msec;
-- 
1.6.0.6

[PATCH 07/15] ath9k: remove unused ath_rc_isvalid_txmask()

[Luis R. Rodriguez]

Cc: Derek Smithies <derek@indranet.co.nz>
Cc: Chittajit Mitra <Chittajit.Mitra@Atheros.com>
Signed-off-by: Luis R. Rodriguez <lrodriguez@atheros.com>
---
 drivers/net/wireless/ath/ath9k/rc.c |    7 -------
 1 files changed, 0 insertions(+), 7 deletions(-)

diff --git a/drivers/net/wireless/ath/ath9k/rc.c b/drivers/net/wireless/ath/ath9k/rc.c
index eaa22e4..5b986b8 100644
--- a/drivers/net/wireless/ath/ath9k/rc.c
+++ b/drivers/net/wireless/ath/ath9k/rc.c
@@ -454,13 +454,6 @@ static inline void ath_rc_set_valid_txmask(struct ath_rate_priv *ath_rc_priv,
 	ath_rc_priv->valid_rate_index[index] = valid_tx_rate ? 1 : 0;
 }
 
-static inline int ath_rc_isvalid_txmask(struct ath_rate_priv *ath_rc_priv,
-					u8 index)
-{
-	ASSERT(index <= ath_rc_priv->rate_table_size);
-	return ath_rc_priv->valid_rate_index[index];
-}
-
 static inline
 int ath_rc_get_nextvalid_txrate(const struct ath_rate_table *rate_table,
 				struct ath_rate_priv *ath_rc_priv,
-- 
1.6.0.6

[ath9k-devel] [PATCH 07/15] ath9k: remove unused ath_rc_isvalid_txmask()

[Luis R. Rodriguez]

Cc: Derek Smithies <derek@indranet.co.nz>
Cc: Chittajit Mitra <Chittajit.Mitra@Atheros.com>
Signed-off-by: Luis R. Rodriguez <lrodriguez@atheros.com>
---
 drivers/net/wireless/ath/ath9k/rc.c |    7 -------
 1 files changed, 0 insertions(+), 7 deletions(-)

diff --git a/drivers/net/wireless/ath/ath9k/rc.c b/drivers/net/wireless/ath/ath9k/rc.c
index eaa22e4..5b986b8 100644
--- a/drivers/net/wireless/ath/ath9k/rc.c
+++ b/drivers/net/wireless/ath/ath9k/rc.c
@@ -454,13 +454,6 @@ static inline void ath_rc_set_valid_txmask(struct ath_rate_priv *ath_rc_priv,
 	ath_rc_priv->valid_rate_index[index] = valid_tx_rate ? 1 : 0;
 }
 
-static inline int ath_rc_isvalid_txmask(struct ath_rate_priv *ath_rc_priv,
-					u8 index)
-{
-	ASSERT(index <= ath_rc_priv->rate_table_size);
-	return ath_rc_priv->valid_rate_index[index];
-}
-
 static inline
 int ath_rc_get_nextvalid_txrate(const struct ath_rate_table *rate_table,
 				struct ath_rate_priv *ath_rc_priv,
-- 
1.6.0.6

[PATCH 08/15] ath9k: remove ATH9K_MODE_11B

[Luis R. Rodriguez]

This saves us 2733 bytes.

   text    data     bss     dec     hex filename
 252265    3628    1584  257477   3edc5 ath9k-has-b-rate.ko
 249905    3628    1584  255117   3e48d ath9k.ko

Cc: Derek Smithies <derek@indranet.co.nz>
Cc: Chittajit Mitra <Chittajit.Mitra@Atheros.com>
Siged-off-by: Luis R. Rodriguez <lrodriguez@atheros.com>
---
 drivers/net/wireless/ath/ath9k/hw.c |    1 -
 drivers/net/wireless/ath/ath9k/hw.h |    1 -
 drivers/net/wireless/ath/ath9k/rc.c |   23 -----------------------
 3 files changed, 0 insertions(+), 25 deletions(-)

diff --git a/drivers/net/wireless/ath/ath9k/hw.c b/drivers/net/wireless/ath/ath9k/hw.c
index 1579c94..de2c000 100644
--- a/drivers/net/wireless/ath/ath9k/hw.c
+++ b/drivers/net/wireless/ath/ath9k/hw.c
@@ -3294,7 +3294,6 @@ void ath9k_hw_fill_cap_info(struct ath_hw *ah)
 	}
 
 	if (eeval & AR5416_OPFLAGS_11G) {
-		set_bit(ATH9K_MODE_11B, pCap->wireless_modes);
 		set_bit(ATH9K_MODE_11G, pCap->wireless_modes);
 		if (ah->config.ht_enable) {
 			if (!(eeval & AR5416_OPFLAGS_N_2G_HT20))
diff --git a/drivers/net/wireless/ath/ath9k/hw.h b/drivers/net/wireless/ath/ath9k/hw.h
index dd8508e..73d859e 100644
--- a/drivers/net/wireless/ath/ath9k/hw.h
+++ b/drivers/net/wireless/ath/ath9k/hw.h
@@ -113,7 +113,6 @@
 
 enum wireless_mode {
 	ATH9K_MODE_11A = 0,
-	ATH9K_MODE_11B = 2,
 	ATH9K_MODE_11G = 3,
 	ATH9K_MODE_11NA_HT20 = 6,
 	ATH9K_MODE_11NG_HT20 = 7,
diff --git a/drivers/net/wireless/ath/ath9k/rc.c b/drivers/net/wireless/ath/ath9k/rc.c
index 5b986b8..7cea944 100644
--- a/drivers/net/wireless/ath/ath9k/rc.c
+++ b/drivers/net/wireless/ath/ath9k/rc.c
@@ -380,27 +380,6 @@ static const struct ath_rate_table ar5416_11g_ratetable = {
 	0,   /* Phy rates allowed initially */
 };
 
-static const struct ath_rate_table ar5416_11b_ratetable = {
-	4,
-	{
-		{ VALID, VALID, WLAN_RC_PHY_CCK, 1000, /* 1 Mb */
-			900, 0x1b,  0x00, (0x80|2),
-			0, 0, 1, 0, 0 },
-		{ VALID, VALID, WLAN_RC_PHY_CCK, 2000, /* 2 Mb */
-			1800, 0x1a, 0x04, (0x80|4),
-			1, 1, 1, 1, 0 },
-		{ VALID, VALID, WLAN_RC_PHY_CCK, 5500, /* 5.5 Mb */
-			4300, 0x19, 0x04, (0x80|11),
-			1, 2, 2, 2, 0 },
-		{ VALID, VALID, WLAN_RC_PHY_CCK, 11000, /* 11 Mb */
-			7100, 0x18, 0x04, (0x80|22),
-			1, 4, 100, 3, 0 },
-	},
-	100, /* probe interval */
-	100, /* rssi reduce interval */
-	0,   /* Phy rates allowed initially */
-};
-
 static inline int8_t median(int8_t a, int8_t b, int8_t c)
 {
 	if (a >= b) {
@@ -1722,8 +1701,6 @@ static struct rate_control_ops ath_rate_ops = {
 
 void ath_rate_attach(struct ath_softc *sc)
 {
-	sc->hw_rate_table[ATH9K_MODE_11B] =
-		&ar5416_11b_ratetable;
 	sc->hw_rate_table[ATH9K_MODE_11A] =
 		&ar5416_11a_ratetable;
 	sc->hw_rate_table[ATH9K_MODE_11G] =
-- 
1.6.0.6

[ath9k-devel] [PATCH 08/15] ath9k: remove ATH9K_MODE_11B

[Luis R. Rodriguez]

This saves us 2733 bytes.

   text    data     bss     dec     hex filename
 252265    3628    1584  257477   3edc5 ath9k-has-b-rate.ko
 249905    3628    1584  255117   3e48d ath9k.ko

Cc: Derek Smithies <derek@indranet.co.nz>
Cc: Chittajit Mitra <Chittajit.Mitra@Atheros.com>
Siged-off-by: Luis R. Rodriguez <lrodriguez@atheros.com>
---
 drivers/net/wireless/ath/ath9k/hw.c |    1 -
 drivers/net/wireless/ath/ath9k/hw.h |    1 -
 drivers/net/wireless/ath/ath9k/rc.c |   23 -----------------------
 3 files changed, 0 insertions(+), 25 deletions(-)

diff --git a/drivers/net/wireless/ath/ath9k/hw.c b/drivers/net/wireless/ath/ath9k/hw.c
index 1579c94..de2c000 100644
--- a/drivers/net/wireless/ath/ath9k/hw.c
+++ b/drivers/net/wireless/ath/ath9k/hw.c
@@ -3294,7 +3294,6 @@ void ath9k_hw_fill_cap_info(struct ath_hw *ah)
 	}
 
 	if (eeval & AR5416_OPFLAGS_11G) {
-		set_bit(ATH9K_MODE_11B, pCap->wireless_modes);
 		set_bit(ATH9K_MODE_11G, pCap->wireless_modes);
 		if (ah->config.ht_enable) {
 			if (!(eeval & AR5416_OPFLAGS_N_2G_HT20))
diff --git a/drivers/net/wireless/ath/ath9k/hw.h b/drivers/net/wireless/ath/ath9k/hw.h
index dd8508e..73d859e 100644
--- a/drivers/net/wireless/ath/ath9k/hw.h
+++ b/drivers/net/wireless/ath/ath9k/hw.h
@@ -113,7 +113,6 @@
 
 enum wireless_mode {
 	ATH9K_MODE_11A = 0,
-	ATH9K_MODE_11B = 2,
 	ATH9K_MODE_11G = 3,
 	ATH9K_MODE_11NA_HT20 = 6,
 	ATH9K_MODE_11NG_HT20 = 7,
diff --git a/drivers/net/wireless/ath/ath9k/rc.c b/drivers/net/wireless/ath/ath9k/rc.c
index 5b986b8..7cea944 100644
--- a/drivers/net/wireless/ath/ath9k/rc.c
+++ b/drivers/net/wireless/ath/ath9k/rc.c
@@ -380,27 +380,6 @@ static const struct ath_rate_table ar5416_11g_ratetable = {
 	0,   /* Phy rates allowed initially */
 };
 
-static const struct ath_rate_table ar5416_11b_ratetable = {
-	4,
-	{
-		{ VALID, VALID, WLAN_RC_PHY_CCK, 1000, /* 1 Mb */
-			900, 0x1b,  0x00, (0x80|2),
-			0, 0, 1, 0, 0 },
-		{ VALID, VALID, WLAN_RC_PHY_CCK, 2000, /* 2 Mb */
-			1800, 0x1a, 0x04, (0x80|4),
-			1, 1, 1, 1, 0 },
-		{ VALID, VALID, WLAN_RC_PHY_CCK, 5500, /* 5.5 Mb */
-			4300, 0x19, 0x04, (0x80|11),
-			1, 2, 2, 2, 0 },
-		{ VALID, VALID, WLAN_RC_PHY_CCK, 11000, /* 11 Mb */
-			7100, 0x18, 0x04, (0x80|22),
-			1, 4, 100, 3, 0 },
-	},
-	100, /* probe interval */
-	100, /* rssi reduce interval */
-	0,   /* Phy rates allowed initially */
-};
-
 static inline int8_t median(int8_t a, int8_t b, int8_t c)
 {
 	if (a >= b) {
@@ -1722,8 +1701,6 @@ static struct rate_control_ops ath_rate_ops = {
 
 void ath_rate_attach(struct ath_softc *sc)
 {
-	sc->hw_rate_table[ATH9K_MODE_11B] =
-		&ar5416_11b_ratetable;
 	sc->hw_rate_table[ATH9K_MODE_11A] =
 		&ar5416_11a_ratetable;
 	sc->hw_rate_table[ATH9K_MODE_11G] =
-- 
1.6.0.6

[PATCH 09/15] ath9k: remap ATH9K_MODE_*

[Luis R. Rodriguez]

There are a lot of gaps here.

Cc: Derek Smithies <derek@indranet.co.nz>
Cc: Chittajit Mitra <Chittajit.Mitra@Atheros.com>
Signed-off-by: Luis R. Rodriguez <lrodriguez@atheros.com>
---
 drivers/net/wireless/ath/ath9k/hw.h |   16 ++++++++--------
 1 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/net/wireless/ath/ath9k/hw.h b/drivers/net/wireless/ath/ath9k/hw.h
index 73d859e..08eb5ae 100644
--- a/drivers/net/wireless/ath/ath9k/hw.h
+++ b/drivers/net/wireless/ath/ath9k/hw.h
@@ -113,14 +113,14 @@
 
 enum wireless_mode {
 	ATH9K_MODE_11A = 0,
-	ATH9K_MODE_11G = 3,
-	ATH9K_MODE_11NA_HT20 = 6,
-	ATH9K_MODE_11NG_HT20 = 7,
-	ATH9K_MODE_11NA_HT40PLUS = 8,
-	ATH9K_MODE_11NA_HT40MINUS = 9,
-	ATH9K_MODE_11NG_HT40PLUS = 10,
-	ATH9K_MODE_11NG_HT40MINUS = 11,
-	ATH9K_MODE_MAX
+	ATH9K_MODE_11G,
+	ATH9K_MODE_11NA_HT20,
+	ATH9K_MODE_11NG_HT20,
+	ATH9K_MODE_11NA_HT40PLUS,
+	ATH9K_MODE_11NA_HT40MINUS,
+	ATH9K_MODE_11NG_HT40PLUS,
+	ATH9K_MODE_11NG_HT40MINUS,
+	ATH9K_MODE_MAX,
 };
 
 enum ath9k_hw_caps {
-- 
1.6.0.6

[ath9k-devel] [PATCH 09/15] ath9k: remap ATH9K_MODE_*

[Luis R. Rodriguez]

There are a lot of gaps here.

Cc: Derek Smithies <derek@indranet.co.nz>
Cc: Chittajit Mitra <Chittajit.Mitra@Atheros.com>
Signed-off-by: Luis R. Rodriguez <lrodriguez@atheros.com>
---
 drivers/net/wireless/ath/ath9k/hw.h |   16 ++++++++--------
 1 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/net/wireless/ath/ath9k/hw.h b/drivers/net/wireless/ath/ath9k/hw.h
index 73d859e..08eb5ae 100644
--- a/drivers/net/wireless/ath/ath9k/hw.h
+++ b/drivers/net/wireless/ath/ath9k/hw.h
@@ -113,14 +113,14 @@
 
 enum wireless_mode {
 	ATH9K_MODE_11A = 0,
-	ATH9K_MODE_11G = 3,
-	ATH9K_MODE_11NA_HT20 = 6,
-	ATH9K_MODE_11NG_HT20 = 7,
-	ATH9K_MODE_11NA_HT40PLUS = 8,
-	ATH9K_MODE_11NA_HT40MINUS = 9,
-	ATH9K_MODE_11NG_HT40PLUS = 10,
-	ATH9K_MODE_11NG_HT40MINUS = 11,
-	ATH9K_MODE_MAX
+	ATH9K_MODE_11G,
+	ATH9K_MODE_11NA_HT20,
+	ATH9K_MODE_11NG_HT20,
+	ATH9K_MODE_11NA_HT40PLUS,
+	ATH9K_MODE_11NA_HT40MINUS,
+	ATH9K_MODE_11NG_HT40PLUS,
+	ATH9K_MODE_11NG_HT40MINUS,
+	ATH9K_MODE_MAX,
 };
 
 enum ath9k_hw_caps {
-- 
1.6.0.6

[PATCH 10/15] ath9k: rename ath_rc_ratefind_ht() to ath_rc_get_highest_rix()

[Luis R. Rodriguez]

The purpose is to find the highest rate we can use.

Cc: Derek Smithies <derek@indranet.co.nz>
Cc: Chittajit Mitra <Chittajit.Mitra@Atheros.com>
Signed-off-by: Luis R. Rodriguez <lrodriguez@atheros.com>
---
 drivers/net/wireless/ath/ath9k/rc.c |   13 +++++++------
 1 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/drivers/net/wireless/ath/ath9k/rc.c b/drivers/net/wireless/ath/ath9k/rc.c
index 7cea944..16e41b1 100644
--- a/drivers/net/wireless/ath/ath9k/rc.c
+++ b/drivers/net/wireless/ath/ath9k/rc.c
@@ -601,10 +601,11 @@ static u8 ath_rc_setvalid_htrates(struct ath_rate_priv *ath_rc_priv,
 	return hi;
 }
 
-static u8 ath_rc_ratefind_ht(struct ath_softc *sc,
-			     struct ath_rate_priv *ath_rc_priv,
-			     const struct ath_rate_table *rate_table,
-			     int *is_probing)
+/* Finds the highest rate index we can use */
+static u8 ath_rc_get_highest_rix(struct ath_softc *sc,
+			         struct ath_rate_priv *ath_rc_priv,
+				 const struct ath_rate_table *rate_table,
+				 int *is_probing)
 {
 	u32 dt, best_thruput, this_thruput, now_msec;
 	u8 rate, next_rate, best_rate, maxindex, minindex;
@@ -820,7 +821,7 @@ static void ath_rc_ratefind(struct ath_softc *sc,
 	try_per_rate = sc->hw->max_rate_tries;
 
 	rate_table = sc->cur_rate_table;
-	rix = ath_rc_ratefind_ht(sc, ath_rc_priv, rate_table, &is_probe);
+	rix = ath_rc_get_highest_rix(sc, ath_rc_priv, rate_table, &is_probe);
 	nrix = rix;
 
 	if (is_probe) {
@@ -908,7 +909,7 @@ static void ath_rc_ratefind(struct ath_softc *sc,
 		DPRINTF(sc, ATH_DBG_RATE, "lowest rate being used, "
 			"disabling MRR\n");
 		rates[0].idx = rate_lowest_index(sband, sta);
-		/* Disable MRR when ath_rc_ratefind_ht() found rate 0 */
+		/* Disable MRR when ath_rc_get_highest_rix() found rate 0 */
 		rates[1].idx = -1;
 	}
 }
-- 
1.6.0.6

[ath9k-devel] [PATCH 10/15] ath9k: rename ath_rc_ratefind_ht() to ath_rc_get_highest_rix()

[Luis R. Rodriguez]

The purpose is to find the highest rate we can use.

Cc: Derek Smithies <derek@indranet.co.nz>
Cc: Chittajit Mitra <Chittajit.Mitra@Atheros.com>
Signed-off-by: Luis R. Rodriguez <lrodriguez@atheros.com>
---
 drivers/net/wireless/ath/ath9k/rc.c |   13 +++++++------
 1 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/drivers/net/wireless/ath/ath9k/rc.c b/drivers/net/wireless/ath/ath9k/rc.c
index 7cea944..16e41b1 100644
--- a/drivers/net/wireless/ath/ath9k/rc.c
+++ b/drivers/net/wireless/ath/ath9k/rc.c
@@ -601,10 +601,11 @@ static u8 ath_rc_setvalid_htrates(struct ath_rate_priv *ath_rc_priv,
 	return hi;
 }
 
-static u8 ath_rc_ratefind_ht(struct ath_softc *sc,
-			     struct ath_rate_priv *ath_rc_priv,
-			     const struct ath_rate_table *rate_table,
-			     int *is_probing)
+/* Finds the highest rate index we can use */
+static u8 ath_rc_get_highest_rix(struct ath_softc *sc,
+			         struct ath_rate_priv *ath_rc_priv,
+				 const struct ath_rate_table *rate_table,
+				 int *is_probing)
 {
 	u32 dt, best_thruput, this_thruput, now_msec;
 	u8 rate, next_rate, best_rate, maxindex, minindex;
@@ -820,7 +821,7 @@ static void ath_rc_ratefind(struct ath_softc *sc,
 	try_per_rate = sc->hw->max_rate_tries;
 
 	rate_table = sc->cur_rate_table;
-	rix = ath_rc_ratefind_ht(sc, ath_rc_priv, rate_table, &is_probe);
+	rix = ath_rc_get_highest_rix(sc, ath_rc_priv, rate_table, &is_probe);
 	nrix = rix;
 
 	if (is_probe) {
@@ -908,7 +909,7 @@ static void ath_rc_ratefind(struct ath_softc *sc,
 		DPRINTF(sc, ATH_DBG_RATE, "lowest rate being used, "
 			"disabling MRR\n");
 		rates[0].idx = rate_lowest_index(sband, sta);
-		/* Disable MRR when ath_rc_ratefind_ht() found rate 0 */
+		/* Disable MRR when ath_rc_get_highest_rix() found rate 0 */
 		rates[1].idx = -1;
 	}
 }
-- 
1.6.0.6

[PATCH 11/15] ath9k: remove unnecessary IEEE80211_TX_CTL_NO_ACK checks

[Luis R. Rodriguez]

We check for this condition early on in our mac80211 get_rate()
callback ath_get_rate(), so remove this check later down the path.

Cc: Derek Smithies <derek@indranet.co.nz>
Cc: Chittajit Mitra <Chittajit.Mitra@Atheros.com>
Signed-off-by: Luis R. Rodriguez <lrodriguez@atheros.com>
---
 drivers/net/wireless/ath/ath9k/rc.c |    6 ++----
 1 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/net/wireless/ath/ath9k/rc.c b/drivers/net/wireless/ath/ath9k/rc.c
index 16e41b1..344b996 100644
--- a/drivers/net/wireless/ath/ath9k/rc.c
+++ b/drivers/net/wireless/ath/ath9k/rc.c
@@ -783,7 +783,6 @@ static void ath_rc_rate_set_rtscts(struct ath_softc *sc,
 	 * just CTS.  Note that this is only done for OFDM/HT unicast frames.
 	 */
 	if ((sc->sc_flags & SC_OP_PROTECT_ENABLE) &&
-	    !(tx_info->flags & IEEE80211_TX_CTL_NO_ACK) &&
 	    (rate_table->info[rix].phy == WLAN_RC_PHY_OFDM ||
 	     WLAN_RC_PHY_HT(rate_table->info[rix].phy))) {
 		rates[0].flags |= IEEE80211_TX_RC_USE_CTS_PROTECT;
@@ -890,9 +889,8 @@ static void ath_rc_ratefind(struct ath_softc *sc,
 	 *
 	 * FIXME: Fix duration
 	 */
-	if (!(tx_info->flags & IEEE80211_TX_CTL_NO_ACK) &&
-	    (ieee80211_has_morefrags(fc) ||
-	     (le16_to_cpu(hdr->seq_ctrl) & IEEE80211_SCTL_FRAG))) {
+	if (ieee80211_has_morefrags(fc) ||
+	    (le16_to_cpu(hdr->seq_ctrl) & IEEE80211_SCTL_FRAG)) {
 		rates[1].count = rates[2].count = rates[3].count = 0;
 		rates[1].idx = rates[2].idx = rates[3].idx = 0;
 		rates[0].count = ATH_TXMAXTRY;
-- 
1.6.0.6

[ath9k-devel] [PATCH 11/15] ath9k: remove unnecessary IEEE80211_TX_CTL_NO_ACK checks

[Luis R. Rodriguez]

We check for this condition early on in our mac80211 get_rate()
callback ath_get_rate(), so remove this check later down the path.

Cc: Derek Smithies <derek@indranet.co.nz>
Cc: Chittajit Mitra <Chittajit.Mitra@Atheros.com>
Signed-off-by: Luis R. Rodriguez <lrodriguez@atheros.com>
---
 drivers/net/wireless/ath/ath9k/rc.c |    6 ++----
 1 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/net/wireless/ath/ath9k/rc.c b/drivers/net/wireless/ath/ath9k/rc.c
index 16e41b1..344b996 100644
--- a/drivers/net/wireless/ath/ath9k/rc.c
+++ b/drivers/net/wireless/ath/ath9k/rc.c
@@ -783,7 +783,6 @@ static void ath_rc_rate_set_rtscts(struct ath_softc *sc,
 	 * just CTS.  Note that this is only done for OFDM/HT unicast frames.
 	 */
 	if ((sc->sc_flags & SC_OP_PROTECT_ENABLE) &&
-	    !(tx_info->flags & IEEE80211_TX_CTL_NO_ACK) &&
 	    (rate_table->info[rix].phy == WLAN_RC_PHY_OFDM ||
 	     WLAN_RC_PHY_HT(rate_table->info[rix].phy))) {
 		rates[0].flags |= IEEE80211_TX_RC_USE_CTS_PROTECT;
@@ -890,9 +889,8 @@ static void ath_rc_ratefind(struct ath_softc *sc,
 	 *
 	 * FIXME: Fix duration
 	 */
-	if (!(tx_info->flags & IEEE80211_TX_CTL_NO_ACK) &&
-	    (ieee80211_has_morefrags(fc) ||
-	     (le16_to_cpu(hdr->seq_ctrl) & IEEE80211_SCTL_FRAG))) {
+	if (ieee80211_has_morefrags(fc) ||
+	    (le16_to_cpu(hdr->seq_ctrl) & IEEE80211_SCTL_FRAG)) {
 		rates[1].count = rates[2].count = rates[3].count = 0;
 		rates[1].idx = rates[2].idx = rates[3].idx = 0;
 		rates[0].count = ATH_TXMAXTRY;
-- 
1.6.0.6

[PATCH 12/15] mac80211: make minstrel/pid RC use ieee80211_is_data(fc)

[Luis R. Rodriguez]

Cc: Felix Fietkau <nbd@openwrt.org>
Cc: Derek Smithies <derek@indranet.co.nz>
Cc: Chittajit Mitra <Chittajit.Mitra@Atheros.com>
Signed-off-by: Luis R. Rodriguez <lrodriguez@atheros.com>
---
 net/mac80211/rc80211_minstrel.c |    7 +++----
 net/mac80211/rc80211_pid_algo.c |    5 ++---
 2 files changed, 5 insertions(+), 7 deletions(-)

diff --git a/net/mac80211/rc80211_minstrel.c b/net/mac80211/rc80211_minstrel.c
index 0a11515..0e1db92 100644
--- a/net/mac80211/rc80211_minstrel.c
+++ b/net/mac80211/rc80211_minstrel.c
@@ -75,12 +75,11 @@ use_low_rate(struct sk_buff *skb)
 {
 	struct ieee80211_hdr *hdr = (struct ieee80211_hdr *) skb->data;
 	struct ieee80211_tx_info *info = IEEE80211_SKB_CB(skb);
-	u16 fc;
+	__le16 fc;
 
-	fc = le16_to_cpu(hdr->frame_control);
+	fc = hdr->frame_control;
 
-	return ((info->flags & IEEE80211_TX_CTL_NO_ACK) ||
-		(fc & IEEE80211_FCTL_FTYPE) != IEEE80211_FTYPE_DATA);
+	return ((info->flags & IEEE80211_TX_CTL_NO_ACK) || !ieee80211_is_data(fc));
 }
 
 
diff --git a/net/mac80211/rc80211_pid_algo.c b/net/mac80211/rc80211_pid_algo.c
index a0bef76..ac6dad6 100644
--- a/net/mac80211/rc80211_pid_algo.c
+++ b/net/mac80211/rc80211_pid_algo.c
@@ -280,7 +280,7 @@ rate_control_pid_get_rate(void *priv, struct ieee80211_sta *sta,
 	struct ieee80211_tx_info *info = IEEE80211_SKB_CB(skb);
 	struct rc_pid_sta_info *spinfo = priv_sta;
 	int rateidx;
-	u16 fc;
+	__le16 fc;
 
 	if (txrc->rts)
 		info->control.rates[0].count =
@@ -291,8 +291,7 @@ rate_control_pid_get_rate(void *priv, struct ieee80211_sta *sta,
 
 	/* Send management frames and NO_ACK data using lowest rate. */
 	fc = le16_to_cpu(hdr->frame_control);
-	if (!sta || !spinfo ||
-	    (fc & IEEE80211_FCTL_FTYPE) != IEEE80211_FTYPE_DATA ||
+	if (!sta || !spinfo || !ieee80211_is_data(fc) ||
 	    info->flags & IEEE80211_TX_CTL_NO_ACK) {
 		info->control.rates[0].idx = rate_lowest_index(sband, sta);
 		if (info->flags & IEEE80211_TX_CTL_NO_ACK)
-- 
1.6.0.6

[ath9k-devel] [PATCH 12/15] mac80211: make minstrel/pid RC use ieee80211_is_data(fc)

[Luis R. Rodriguez]

Cc: Felix Fietkau <nbd@openwrt.org>
Cc: Derek Smithies <derek@indranet.co.nz>
Cc: Chittajit Mitra <Chittajit.Mitra@Atheros.com>
Signed-off-by: Luis R. Rodriguez <lrodriguez@atheros.com>
---
 net/mac80211/rc80211_minstrel.c |    7 +++----
 net/mac80211/rc80211_pid_algo.c |    5 ++---
 2 files changed, 5 insertions(+), 7 deletions(-)

diff --git a/net/mac80211/rc80211_minstrel.c b/net/mac80211/rc80211_minstrel.c
index 0a11515..0e1db92 100644
--- a/net/mac80211/rc80211_minstrel.c
+++ b/net/mac80211/rc80211_minstrel.c
@@ -75,12 +75,11 @@ use_low_rate(struct sk_buff *skb)
 {
 	struct ieee80211_hdr *hdr = (struct ieee80211_hdr *) skb->data;
 	struct ieee80211_tx_info *info = IEEE80211_SKB_CB(skb);
-	u16 fc;
+	__le16 fc;
 
-	fc = le16_to_cpu(hdr->frame_control);
+	fc = hdr->frame_control;
 
-	return ((info->flags & IEEE80211_TX_CTL_NO_ACK) ||
-		(fc & IEEE80211_FCTL_FTYPE) != IEEE80211_FTYPE_DATA);
+	return ((info->flags & IEEE80211_TX_CTL_NO_ACK) || !ieee80211_is_data(fc));
 }
 
 
diff --git a/net/mac80211/rc80211_pid_algo.c b/net/mac80211/rc80211_pid_algo.c
index a0bef76..ac6dad6 100644
--- a/net/mac80211/rc80211_pid_algo.c
+++ b/net/mac80211/rc80211_pid_algo.c
@@ -280,7 +280,7 @@ rate_control_pid_get_rate(void *priv, struct ieee80211_sta *sta,
 	struct ieee80211_tx_info *info = IEEE80211_SKB_CB(skb);
 	struct rc_pid_sta_info *spinfo = priv_sta;
 	int rateidx;
-	u16 fc;
+	__le16 fc;
 
 	if (txrc->rts)
 		info->control.rates[0].count =
@@ -291,8 +291,7 @@ rate_control_pid_get_rate(void *priv, struct ieee80211_sta *sta,
 
 	/* Send management frames and NO_ACK data using lowest rate. */
 	fc = le16_to_cpu(hdr->frame_control);
-	if (!sta || !spinfo ||
-	    (fc & IEEE80211_FCTL_FTYPE) != IEEE80211_FTYPE_DATA ||
+	if (!sta || !spinfo || !ieee80211_is_data(fc) ||
 	    info->flags & IEEE80211_TX_CTL_NO_ACK) {
 		info->control.rates[0].idx = rate_lowest_index(sband, sta);
 		if (info->flags & IEEE80211_TX_CTL_NO_ACK)
-- 
1.6.0.6

[PATCH 13/15] iwlwifi: use ieee80211_is_data(fc)

[Luis R. Rodriguez]

Cc: Zhu Yi <yi.zhu@intel.com>
Cc: Reinette Chatre <reinette.chatre@intel.com>
Cc: ipw3945-devel@lists.sourceforge.net
Signed-off-by: Luis R. Rodriguez <lrodriguez@atheros.com>
---
 drivers/net/wireless/iwlwifi/iwl-3945-rs.c |    7 +++----
 drivers/net/wireless/iwlwifi/iwl-sta.c     |    5 ++---
 2 files changed, 5 insertions(+), 7 deletions(-)

diff --git a/drivers/net/wireless/iwlwifi/iwl-3945-rs.c b/drivers/net/wireless/iwlwifi/iwl-3945-rs.c
index 5eb538d..bd2f709 100644
--- a/drivers/net/wireless/iwlwifi/iwl-3945-rs.c
+++ b/drivers/net/wireless/iwlwifi/iwl-3945-rs.c
@@ -673,7 +673,7 @@ static void rs_get_rate(void *priv_r, struct ieee80211_sta *sta,
 	s8 scale_action = 0;
 	unsigned long flags;
 	struct ieee80211_hdr *hdr = (struct ieee80211_hdr *)skb->data;
-	u16 fc;
+	__le16 fc;
 	u16 rate_mask = 0;
 	s8 max_rate_idx = -1;
 	struct iwl_priv *priv = (struct iwl_priv *)priv_r;
@@ -685,9 +685,8 @@ static void rs_get_rate(void *priv_r, struct ieee80211_sta *sta,
 		rate_mask = sta->supp_rates[sband->band];
 
 	/* Send management frames and NO_ACK data using lowest rate. */
-	fc = le16_to_cpu(hdr->frame_control);
-	if ((fc & IEEE80211_FCTL_FTYPE) != IEEE80211_FTYPE_DATA ||
-	    info->flags & IEEE80211_TX_CTL_NO_ACK ||
+	fc = hdr->frame_control;
+	if (!ieee80211_is_data(fc) || info->flags & IEEE80211_TX_CTL_NO_ACK ||
 	    !sta || !priv_sta) {
 		IWL_DEBUG_RATE(priv, "leave: No STA priv data to update!\n");
 		if (!rate_mask)
diff --git a/drivers/net/wireless/iwlwifi/iwl-sta.c b/drivers/net/wireless/iwlwifi/iwl-sta.c
index 2addf73..afa1633 100644
--- a/drivers/net/wireless/iwlwifi/iwl-sta.c
+++ b/drivers/net/wireless/iwlwifi/iwl-sta.c
@@ -1044,11 +1044,10 @@ EXPORT_SYMBOL(iwl_rxon_add_station);
 int iwl_get_sta_id(struct iwl_priv *priv, struct ieee80211_hdr *hdr)
 {
 	int sta_id;
-	u16 fc = le16_to_cpu(hdr->frame_control);
+	__le16 fc = hdr->frame_control;
 
 	/* If this frame is broadcast or management, use broadcast station id */
-	if (((fc & IEEE80211_FCTL_FTYPE) != IEEE80211_FTYPE_DATA) ||
-	    is_multicast_ether_addr(hdr->addr1))
+	if (!ieee80211_is_data(fc) ||  is_multicast_ether_addr(hdr->addr1))
 		return priv->hw_params.bcast_sta_id;
 
 	switch (priv->iw_mode) {
-- 
1.6.0.6

[ath9k-devel] [PATCH 13/15] iwlwifi: use ieee80211_is_data(fc)

[Luis R. Rodriguez]

Cc: Zhu Yi <yi.zhu@intel.com>
Cc: Reinette Chatre <reinette.chatre@intel.com>
Cc: ipw3945-devel at lists.sourceforge.net
Signed-off-by: Luis R. Rodriguez <lrodriguez@atheros.com>
---
 drivers/net/wireless/iwlwifi/iwl-3945-rs.c |    7 +++----
 drivers/net/wireless/iwlwifi/iwl-sta.c     |    5 ++---
 2 files changed, 5 insertions(+), 7 deletions(-)

diff --git a/drivers/net/wireless/iwlwifi/iwl-3945-rs.c b/drivers/net/wireless/iwlwifi/iwl-3945-rs.c
index 5eb538d..bd2f709 100644
--- a/drivers/net/wireless/iwlwifi/iwl-3945-rs.c
+++ b/drivers/net/wireless/iwlwifi/iwl-3945-rs.c
@@ -673,7 +673,7 @@ static void rs_get_rate(void *priv_r, struct ieee80211_sta *sta,
 	s8 scale_action = 0;
 	unsigned long flags;
 	struct ieee80211_hdr *hdr = (struct ieee80211_hdr *)skb->data;
-	u16 fc;
+	__le16 fc;
 	u16 rate_mask = 0;
 	s8 max_rate_idx = -1;
 	struct iwl_priv *priv = (struct iwl_priv *)priv_r;
@@ -685,9 +685,8 @@ static void rs_get_rate(void *priv_r, struct ieee80211_sta *sta,
 		rate_mask = sta->supp_rates[sband->band];
 
 	/* Send management frames and NO_ACK data using lowest rate. */
-	fc = le16_to_cpu(hdr->frame_control);
-	if ((fc & IEEE80211_FCTL_FTYPE) != IEEE80211_FTYPE_DATA ||
-	    info->flags & IEEE80211_TX_CTL_NO_ACK ||
+	fc = hdr->frame_control;
+	if (!ieee80211_is_data(fc) || info->flags & IEEE80211_TX_CTL_NO_ACK ||
 	    !sta || !priv_sta) {
 		IWL_DEBUG_RATE(priv, "leave: No STA priv data to update!\n");
 		if (!rate_mask)
diff --git a/drivers/net/wireless/iwlwifi/iwl-sta.c b/drivers/net/wireless/iwlwifi/iwl-sta.c
index 2addf73..afa1633 100644
--- a/drivers/net/wireless/iwlwifi/iwl-sta.c
+++ b/drivers/net/wireless/iwlwifi/iwl-sta.c
@@ -1044,11 +1044,10 @@ EXPORT_SYMBOL(iwl_rxon_add_station);
 int iwl_get_sta_id(struct iwl_priv *priv, struct ieee80211_hdr *hdr)
 {
 	int sta_id;
-	u16 fc = le16_to_cpu(hdr->frame_control);
+	__le16 fc = hdr->frame_control;
 
 	/* If this frame is broadcast or management, use broadcast station id */
-	if (((fc & IEEE80211_FCTL_FTYPE) != IEEE80211_FTYPE_DATA) ||
-	    is_multicast_ether_addr(hdr->addr1))
+	if (!ieee80211_is_data(fc) ||  is_multicast_ether_addr(hdr->addr1))
 		return priv->hw_params.bcast_sta_id;
 
 	switch (priv->iw_mode) {
-- 
1.6.0.6

[PATCH 14/15] mac80211: move management / no-ack frame rate decision to mac80211

[Luis R. Rodriguez]

All rate control algorithms agree to send management and no-ack
frames at the lowest rate. They also agree to do this when sta
and the private rate control data is NULL. We move this to mac80211
and simplify the rate control algorithm code.

Cc: Zhu Yi <yi.zhu@intel.com>
Cc: Reinette Chatre <reinette.chatre@intel.com>
Cc: ipw3945-devel@lists.sourceforge.net
Cc: Gabor Juhos <juhosg@openwrt.org>
Cc: Felix Fietkau <nbd@openwrt.org>
Cc: Derek Smithies <derek@indranet.co.nz>
Cc: Chittajit Mitra <Chittajit.Mitra@Atheros.com>
Signed-off-by: Luis R. Rodriguez <lrodriguez@atheros.com>
---
 drivers/net/wireless/ath/ath9k/rc.c        |   15 --------------
 drivers/net/wireless/iwlwifi/iwl-3945-rs.c |   20 +------------------
 drivers/net/wireless/iwlwifi/iwl-agn-rs.c  |   10 ---------
 net/mac80211/rate.c                        |   29 +++++++++++++++++++++++++++-
 net/mac80211/rc80211_minstrel.c            |   23 ----------------------
 net/mac80211/rc80211_pid_algo.c            |   14 -------------
 6 files changed, 29 insertions(+), 82 deletions(-)

diff --git a/drivers/net/wireless/ath/ath9k/rc.c b/drivers/net/wireless/ath/ath9k/rc.c
index 344b996..2180180 100644
--- a/drivers/net/wireless/ath/ath9k/rc.c
+++ b/drivers/net/wireless/ath/ath9k/rc.c
@@ -1538,23 +1538,8 @@ exit:
 static void ath_get_rate(void *priv, struct ieee80211_sta *sta, void *priv_sta,
 			 struct ieee80211_tx_rate_control *txrc)
 {
-	struct ieee80211_supported_band *sband = txrc->sband;
-	struct sk_buff *skb = txrc->skb;
-	struct ieee80211_hdr *hdr = (struct ieee80211_hdr *)skb->data;
-	struct ieee80211_tx_info *tx_info = IEEE80211_SKB_CB(skb);
 	struct ath_softc *sc = priv;
 	struct ath_rate_priv *ath_rc_priv = priv_sta;
-	__le16 fc = hdr->frame_control;
-
-	/* lowest rate for management and NO_ACK frames */
-	if (!ieee80211_is_data(fc) ||
-	    tx_info->flags & IEEE80211_TX_CTL_NO_ACK || !sta) {
-		tx_info->control.rates[0].idx = rate_lowest_index(sband, sta);
-		tx_info->control.rates[0].count =
-			(tx_info->flags & IEEE80211_TX_CTL_NO_ACK) ?
-				1 : ATH_MGT_TXMAXTRY;
-		return;
-	}
 
 	/* Find tx rate for unicast frames */
 	ath_rc_ratefind(sc, ath_rc_priv, txrc, sta);
diff --git a/drivers/net/wireless/iwlwifi/iwl-3945-rs.c b/drivers/net/wireless/iwlwifi/iwl-3945-rs.c
index bd2f709..7b6567b 100644
--- a/drivers/net/wireless/iwlwifi/iwl-3945-rs.c
+++ b/drivers/net/wireless/iwlwifi/iwl-3945-rs.c
@@ -673,7 +673,6 @@ static void rs_get_rate(void *priv_r, struct ieee80211_sta *sta,
 	s8 scale_action = 0;
 	unsigned long flags;
 	struct ieee80211_hdr *hdr = (struct ieee80211_hdr *)skb->data;
-	__le16 fc;
 	u16 rate_mask = 0;
 	s8 max_rate_idx = -1;
 	struct iwl_priv *priv = (struct iwl_priv *)priv_r;
@@ -681,24 +680,7 @@ static void rs_get_rate(void *priv_r, struct ieee80211_sta *sta,
 
 	IWL_DEBUG_RATE(priv, "enter\n");
 
-	if (sta)
-		rate_mask = sta->supp_rates[sband->band];
-
-	/* Send management frames and NO_ACK data using lowest rate. */
-	fc = hdr->frame_control;
-	if (!ieee80211_is_data(fc) || info->flags & IEEE80211_TX_CTL_NO_ACK ||
-	    !sta || !priv_sta) {
-		IWL_DEBUG_RATE(priv, "leave: No STA priv data to update!\n");
-		if (!rate_mask)
-			info->control.rates[0].idx =
-					rate_lowest_index(sband, NULL);
-		else
-			info->control.rates[0].idx =
-					rate_lowest_index(sband, sta);
-		if (info->flags & IEEE80211_TX_CTL_NO_ACK)
-			info->control.rates[0].count = 1;
-		return;
-	}
+	rate_mask = sta->supp_rates[sband->band];
 
 	/* get user max rate if set */
 	max_rate_idx = txrc->max_rate_idx;
diff --git a/drivers/net/wireless/iwlwifi/iwl-agn-rs.c b/drivers/net/wireless/iwlwifi/iwl-agn-rs.c
index ff20e50..0052e8c 100644
--- a/drivers/net/wireless/iwlwifi/iwl-agn-rs.c
+++ b/drivers/net/wireless/iwlwifi/iwl-agn-rs.c
@@ -2012,7 +2012,6 @@ static void rs_rate_scale_perform(struct iwl_priv *priv,
 {
 	struct ieee80211_hw *hw = priv->hw;
 	struct ieee80211_conf *conf = &hw->conf;
-	struct ieee80211_tx_info *info = IEEE80211_SKB_CB(skb);
 	struct ieee80211_hdr *hdr = (struct ieee80211_hdr *)skb->data;
 	int low = IWL_RATE_INVALID;
 	int high = IWL_RATE_INVALID;
@@ -2039,15 +2038,6 @@ static void rs_rate_scale_perform(struct iwl_priv *priv,
 
 	IWL_DEBUG_RATE(priv, "rate scale calculate new rate for skb\n");
 
-	/* Send management frames and NO_ACK data using lowest rate. */
-	/* TODO: this could probably be improved.. */
-	if (!ieee80211_is_data(hdr->frame_control) ||
-	    info->flags & IEEE80211_TX_CTL_NO_ACK)
-		return;
-
-	if (!sta || !lq_sta)
-		return;
-
 	lq_sta->supp_rates = sta->supp_rates[lq_sta->band];
 
 	tid = rs_tl_add_packet(lq_sta, hdr);
diff --git a/net/mac80211/rate.c b/net/mac80211/rate.c
index 4641f00..2795f70 100644
--- a/net/mac80211/rate.c
+++ b/net/mac80211/rate.c
@@ -198,11 +198,24 @@ static void rate_control_release(struct kref *kref)
 	kfree(ctrl_ref);
 }
 
+static bool rc_no_data_or_no_ack(struct ieee80211_tx_rate_control *txrc)
+{
+	struct sk_buff *skb = txrc->skb;
+	struct ieee80211_hdr *hdr = (struct ieee80211_hdr *) skb->data;
+	struct ieee80211_tx_info *info = IEEE80211_SKB_CB(skb);
+	__le16 fc;
+
+	fc = hdr->frame_control;
+
+	return ((info->flags & IEEE80211_TX_CTL_NO_ACK) || !ieee80211_is_data(fc));
+}
+
 void rate_control_get_rate(struct ieee80211_sub_if_data *sdata,
 			   struct sta_info *sta,
 			   struct ieee80211_tx_rate_control *txrc)
 {
 	struct rate_control_ref *ref = sdata->local->rate_ctrl;
+	struct ieee80211_hw *hw = &sdata->local->hw;
 	void *priv_sta = NULL;
 	struct ieee80211_sta *ista = NULL;
 	struct ieee80211_tx_info *info = IEEE80211_SKB_CB(txrc->skb);
@@ -222,7 +235,21 @@ void rate_control_get_rate(struct ieee80211_sub_if_data *sdata,
 	if (sta && sdata->force_unicast_rateidx > -1) {
 		info->control.rates[0].idx = sdata->force_unicast_rateidx;
 	} else {
-		ref->ops->get_rate(ref->priv, ista, priv_sta, txrc);
+		/*
+		 * Lets not bother rate control algos with this as they
+		 * all agree to use the lowest rate to send management frames
+		 * and NO_ACK data with the respective hw retries. This also
+		 * guarantees we don't call the rate control get_rate()
+		 * callback with a NULL sta or NULL private RC data.
+		 */
+		if (!ista || !priv_sta || rc_no_data_or_no_ack(txrc)) {
+			info->control.rates[0].idx =
+				rate_lowest_index(txrc->sband, ista);
+			info->control.rates[0].count =
+				(info->flags & IEEE80211_TX_CTL_NO_ACK) ?
+				1 : hw->max_rate_tries;
+		} else
+			ref->ops->get_rate(ref->priv, ista, priv_sta, txrc);
 		info->flags |= IEEE80211_TX_INTFL_RCALGO;
 	}
 
diff --git a/net/mac80211/rc80211_minstrel.c b/net/mac80211/rc80211_minstrel.c
index 0e1db92..40c1ba4 100644
--- a/net/mac80211/rc80211_minstrel.c
+++ b/net/mac80211/rc80211_minstrel.c
@@ -70,19 +70,6 @@ rix_to_ndx(struct minstrel_sta_info *mi, int rix)
 	return i;
 }
 
-static inline bool
-use_low_rate(struct sk_buff *skb)
-{
-	struct ieee80211_hdr *hdr = (struct ieee80211_hdr *) skb->data;
-	struct ieee80211_tx_info *info = IEEE80211_SKB_CB(skb);
-	__le16 fc;
-
-	fc = hdr->frame_control;
-
-	return ((info->flags & IEEE80211_TX_CTL_NO_ACK) || !ieee80211_is_data(fc));
-}
-
-
 static void
 minstrel_update_stats(struct minstrel_priv *mp, struct minstrel_sta_info *mi)
 {
@@ -228,7 +215,6 @@ minstrel_get_rate(void *priv, struct ieee80211_sta *sta,
 		  void *priv_sta, struct ieee80211_tx_rate_control *txrc)
 {
 	struct sk_buff *skb = txrc->skb;
-	struct ieee80211_supported_band *sband = txrc->sband;
 	struct ieee80211_tx_info *info = IEEE80211_SKB_CB(skb);
 	struct minstrel_sta_info *mi = priv_sta;
 	struct minstrel_priv *mp = priv;
@@ -241,15 +227,6 @@ minstrel_get_rate(void *priv, struct ieee80211_sta *sta,
 	int mrr_ndx[3];
 	int sample_rate;
 
-	if (!sta || !mi || use_low_rate(skb)) {
-		ar[0].idx = rate_lowest_index(sband, sta);
-		if (info->flags & IEEE80211_TX_CTL_NO_ACK)
-			ar[0].count = 1;
-		else
-			ar[0].count = mp->max_retry;
-		return;
-	}
-
 	mrr = mp->has_mrr && !txrc->rts && !txrc->bss_conf->use_cts_prot;
 
 	if (time_after(jiffies, mi->stats_update + (mp->update_interval *
diff --git a/net/mac80211/rc80211_pid_algo.c b/net/mac80211/rc80211_pid_algo.c
index ac6dad6..1dad177 100644
--- a/net/mac80211/rc80211_pid_algo.c
+++ b/net/mac80211/rc80211_pid_algo.c
@@ -274,13 +274,10 @@ rate_control_pid_get_rate(void *priv, struct ieee80211_sta *sta,
 			  void *priv_sta,
 			  struct ieee80211_tx_rate_control *txrc)
 {
-	struct sk_buff *skb = txrc->skb;
 	struct ieee80211_supported_band *sband = txrc->sband;
-	struct ieee80211_hdr *hdr = (struct ieee80211_hdr *) skb->data;
 	struct ieee80211_tx_info *info = IEEE80211_SKB_CB(skb);
 	struct rc_pid_sta_info *spinfo = priv_sta;
 	int rateidx;
-	__le16 fc;
 
 	if (txrc->rts)
 		info->control.rates[0].count =
@@ -289,17 +286,6 @@ rate_control_pid_get_rate(void *priv, struct ieee80211_sta *sta,
 		info->control.rates[0].count =
 			txrc->hw->conf.short_frame_max_tx_count;
 
-	/* Send management frames and NO_ACK data using lowest rate. */
-	fc = le16_to_cpu(hdr->frame_control);
-	if (!sta || !spinfo || !ieee80211_is_data(fc) ||
-	    info->flags & IEEE80211_TX_CTL_NO_ACK) {
-		info->control.rates[0].idx = rate_lowest_index(sband, sta);
-		if (info->flags & IEEE80211_TX_CTL_NO_ACK)
-			info->control.rates[0].count = 1;
-
-		return;
-	}
-
 	rateidx = spinfo->txrate_idx;
 
 	if (rateidx >= sband->n_bitrates)
-- 
1.6.0.6

[ath9k-devel] [PATCH 14/15] mac80211: move management / no-ack frame rate decision to mac80211

[Luis R. Rodriguez]

All rate control algorithms agree to send management and no-ack
frames at the lowest rate. They also agree to do this when sta
and the private rate control data is NULL. We move this to mac80211
and simplify the rate control algorithm code.

Cc: Zhu Yi <yi.zhu@intel.com>
Cc: Reinette Chatre <reinette.chatre@intel.com>
Cc: ipw3945-devel at lists.sourceforge.net
Cc: Gabor Juhos <juhosg@openwrt.org>
Cc: Felix Fietkau <nbd@openwrt.org>
Cc: Derek Smithies <derek@indranet.co.nz>
Cc: Chittajit Mitra <Chittajit.Mitra@Atheros.com>
Signed-off-by: Luis R. Rodriguez <lrodriguez@atheros.com>
---
 drivers/net/wireless/ath/ath9k/rc.c        |   15 --------------
 drivers/net/wireless/iwlwifi/iwl-3945-rs.c |   20 +------------------
 drivers/net/wireless/iwlwifi/iwl-agn-rs.c  |   10 ---------
 net/mac80211/rate.c                        |   29 +++++++++++++++++++++++++++-
 net/mac80211/rc80211_minstrel.c            |   23 ----------------------
 net/mac80211/rc80211_pid_algo.c            |   14 -------------
 6 files changed, 29 insertions(+), 82 deletions(-)

diff --git a/drivers/net/wireless/ath/ath9k/rc.c b/drivers/net/wireless/ath/ath9k/rc.c
index 344b996..2180180 100644
--- a/drivers/net/wireless/ath/ath9k/rc.c
+++ b/drivers/net/wireless/ath/ath9k/rc.c
@@ -1538,23 +1538,8 @@ exit:
 static void ath_get_rate(void *priv, struct ieee80211_sta *sta, void *priv_sta,
 			 struct ieee80211_tx_rate_control *txrc)
 {
-	struct ieee80211_supported_band *sband = txrc->sband;
-	struct sk_buff *skb = txrc->skb;
-	struct ieee80211_hdr *hdr = (struct ieee80211_hdr *)skb->data;
-	struct ieee80211_tx_info *tx_info = IEEE80211_SKB_CB(skb);
 	struct ath_softc *sc = priv;
 	struct ath_rate_priv *ath_rc_priv = priv_sta;
-	__le16 fc = hdr->frame_control;
-
-	/* lowest rate for management and NO_ACK frames */
-	if (!ieee80211_is_data(fc) ||
-	    tx_info->flags & IEEE80211_TX_CTL_NO_ACK || !sta) {
-		tx_info->control.rates[0].idx = rate_lowest_index(sband, sta);
-		tx_info->control.rates[0].count =
-			(tx_info->flags & IEEE80211_TX_CTL_NO_ACK) ?
-				1 : ATH_MGT_TXMAXTRY;
-		return;
-	}
 
 	/* Find tx rate for unicast frames */
 	ath_rc_ratefind(sc, ath_rc_priv, txrc, sta);
diff --git a/drivers/net/wireless/iwlwifi/iwl-3945-rs.c b/drivers/net/wireless/iwlwifi/iwl-3945-rs.c
index bd2f709..7b6567b 100644
--- a/drivers/net/wireless/iwlwifi/iwl-3945-rs.c
+++ b/drivers/net/wireless/iwlwifi/iwl-3945-rs.c
@@ -673,7 +673,6 @@ static void rs_get_rate(void *priv_r, struct ieee80211_sta *sta,
 	s8 scale_action = 0;
 	unsigned long flags;
 	struct ieee80211_hdr *hdr = (struct ieee80211_hdr *)skb->data;
-	__le16 fc;
 	u16 rate_mask = 0;
 	s8 max_rate_idx = -1;
 	struct iwl_priv *priv = (struct iwl_priv *)priv_r;
@@ -681,24 +680,7 @@ static void rs_get_rate(void *priv_r, struct ieee80211_sta *sta,
 
 	IWL_DEBUG_RATE(priv, "enter\n");
 
-	if (sta)
-		rate_mask = sta->supp_rates[sband->band];
-
-	/* Send management frames and NO_ACK data using lowest rate. */
-	fc = hdr->frame_control;
-	if (!ieee80211_is_data(fc) || info->flags & IEEE80211_TX_CTL_NO_ACK ||
-	    !sta || !priv_sta) {
-		IWL_DEBUG_RATE(priv, "leave: No STA priv data to update!\n");
-		if (!rate_mask)
-			info->control.rates[0].idx =
-					rate_lowest_index(sband, NULL);
-		else
-			info->control.rates[0].idx =
-					rate_lowest_index(sband, sta);
-		if (info->flags & IEEE80211_TX_CTL_NO_ACK)
-			info->control.rates[0].count = 1;
-		return;
-	}
+	rate_mask = sta->supp_rates[sband->band];
 
 	/* get user max rate if set */
 	max_rate_idx = txrc->max_rate_idx;
diff --git a/drivers/net/wireless/iwlwifi/iwl-agn-rs.c b/drivers/net/wireless/iwlwifi/iwl-agn-rs.c
index ff20e50..0052e8c 100644
--- a/drivers/net/wireless/iwlwifi/iwl-agn-rs.c
+++ b/drivers/net/wireless/iwlwifi/iwl-agn-rs.c
@@ -2012,7 +2012,6 @@ static void rs_rate_scale_perform(struct iwl_priv *priv,
 {
 	struct ieee80211_hw *hw = priv->hw;
 	struct ieee80211_conf *conf = &hw->conf;
-	struct ieee80211_tx_info *info = IEEE80211_SKB_CB(skb);
 	struct ieee80211_hdr *hdr = (struct ieee80211_hdr *)skb->data;
 	int low = IWL_RATE_INVALID;
 	int high = IWL_RATE_INVALID;
@@ -2039,15 +2038,6 @@ static void rs_rate_scale_perform(struct iwl_priv *priv,
 
 	IWL_DEBUG_RATE(priv, "rate scale calculate new rate for skb\n");
 
-	/* Send management frames and NO_ACK data using lowest rate. */
-	/* TODO: this could probably be improved.. */
-	if (!ieee80211_is_data(hdr->frame_control) ||
-	    info->flags & IEEE80211_TX_CTL_NO_ACK)
-		return;
-
-	if (!sta || !lq_sta)
-		return;
-
 	lq_sta->supp_rates = sta->supp_rates[lq_sta->band];
 
 	tid = rs_tl_add_packet(lq_sta, hdr);
diff --git a/net/mac80211/rate.c b/net/mac80211/rate.c
index 4641f00..2795f70 100644
--- a/net/mac80211/rate.c
+++ b/net/mac80211/rate.c
@@ -198,11 +198,24 @@ static void rate_control_release(struct kref *kref)
 	kfree(ctrl_ref);
 }
 
+static bool rc_no_data_or_no_ack(struct ieee80211_tx_rate_control *txrc)
+{
+	struct sk_buff *skb = txrc->skb;
+	struct ieee80211_hdr *hdr = (struct ieee80211_hdr *) skb->data;
+	struct ieee80211_tx_info *info = IEEE80211_SKB_CB(skb);
+	__le16 fc;
+
+	fc = hdr->frame_control;
+
+	return ((info->flags & IEEE80211_TX_CTL_NO_ACK) || !ieee80211_is_data(fc));
+}
+
 void rate_control_get_rate(struct ieee80211_sub_if_data *sdata,
 			   struct sta_info *sta,
 			   struct ieee80211_tx_rate_control *txrc)
 {
 	struct rate_control_ref *ref = sdata->local->rate_ctrl;
+	struct ieee80211_hw *hw = &sdata->local->hw;
 	void *priv_sta = NULL;
 	struct ieee80211_sta *ista = NULL;
 	struct ieee80211_tx_info *info = IEEE80211_SKB_CB(txrc->skb);
@@ -222,7 +235,21 @@ void rate_control_get_rate(struct ieee80211_sub_if_data *sdata,
 	if (sta && sdata->force_unicast_rateidx > -1) {
 		info->control.rates[0].idx = sdata->force_unicast_rateidx;
 	} else {
-		ref->ops->get_rate(ref->priv, ista, priv_sta, txrc);
+		/*
+		 * Lets not bother rate control algos with this as they
+		 * all agree to use the lowest rate to send management frames
+		 * and NO_ACK data with the respective hw retries. This also
+		 * guarantees we don't call the rate control get_rate()
+		 * callback with a NULL sta or NULL private RC data.
+		 */
+		if (!ista || !priv_sta || rc_no_data_or_no_ack(txrc)) {
+			info->control.rates[0].idx =
+				rate_lowest_index(txrc->sband, ista);
+			info->control.rates[0].count =
+				(info->flags & IEEE80211_TX_CTL_NO_ACK) ?
+				1 : hw->max_rate_tries;
+		} else
+			ref->ops->get_rate(ref->priv, ista, priv_sta, txrc);
 		info->flags |= IEEE80211_TX_INTFL_RCALGO;
 	}
 
diff --git a/net/mac80211/rc80211_minstrel.c b/net/mac80211/rc80211_minstrel.c
index 0e1db92..40c1ba4 100644
--- a/net/mac80211/rc80211_minstrel.c
+++ b/net/mac80211/rc80211_minstrel.c
@@ -70,19 +70,6 @@ rix_to_ndx(struct minstrel_sta_info *mi, int rix)
 	return i;
 }
 
-static inline bool
-use_low_rate(struct sk_buff *skb)
-{
-	struct ieee80211_hdr *hdr = (struct ieee80211_hdr *) skb->data;
-	struct ieee80211_tx_info *info = IEEE80211_SKB_CB(skb);
-	__le16 fc;
-
-	fc = hdr->frame_control;
-
-	return ((info->flags & IEEE80211_TX_CTL_NO_ACK) || !ieee80211_is_data(fc));
-}
-
-
 static void
 minstrel_update_stats(struct minstrel_priv *mp, struct minstrel_sta_info *mi)
 {
@@ -228,7 +215,6 @@ minstrel_get_rate(void *priv, struct ieee80211_sta *sta,
 		  void *priv_sta, struct ieee80211_tx_rate_control *txrc)
 {
 	struct sk_buff *skb = txrc->skb;
-	struct ieee80211_supported_band *sband = txrc->sband;
 	struct ieee80211_tx_info *info = IEEE80211_SKB_CB(skb);
 	struct minstrel_sta_info *mi = priv_sta;
 	struct minstrel_priv *mp = priv;
@@ -241,15 +227,6 @@ minstrel_get_rate(void *priv, struct ieee80211_sta *sta,
 	int mrr_ndx[3];
 	int sample_rate;
 
-	if (!sta || !mi || use_low_rate(skb)) {
-		ar[0].idx = rate_lowest_index(sband, sta);
-		if (info->flags & IEEE80211_TX_CTL_NO_ACK)
-			ar[0].count = 1;
-		else
-			ar[0].count = mp->max_retry;
-		return;
-	}
-
 	mrr = mp->has_mrr && !txrc->rts && !txrc->bss_conf->use_cts_prot;
 
 	if (time_after(jiffies, mi->stats_update + (mp->update_interval *
diff --git a/net/mac80211/rc80211_pid_algo.c b/net/mac80211/rc80211_pid_algo.c
index ac6dad6..1dad177 100644
--- a/net/mac80211/rc80211_pid_algo.c
+++ b/net/mac80211/rc80211_pid_algo.c
@@ -274,13 +274,10 @@ rate_control_pid_get_rate(void *priv, struct ieee80211_sta *sta,
 			  void *priv_sta,
 			  struct ieee80211_tx_rate_control *txrc)
 {
-	struct sk_buff *skb = txrc->skb;
 	struct ieee80211_supported_band *sband = txrc->sband;
-	struct ieee80211_hdr *hdr = (struct ieee80211_hdr *) skb->data;
 	struct ieee80211_tx_info *info = IEEE80211_SKB_CB(skb);
 	struct rc_pid_sta_info *spinfo = priv_sta;
 	int rateidx;
-	__le16 fc;
 
 	if (txrc->rts)
 		info->control.rates[0].count =
@@ -289,17 +286,6 @@ rate_control_pid_get_rate(void *priv, struct ieee80211_sta *sta,
 		info->control.rates[0].count =
 			txrc->hw->conf.short_frame_max_tx_count;
 
-	/* Send management frames and NO_ACK data using lowest rate. */
-	fc = le16_to_cpu(hdr->frame_control);
-	if (!sta || !spinfo || !ieee80211_is_data(fc) ||
-	    info->flags & IEEE80211_TX_CTL_NO_ACK) {
-		info->control.rates[0].idx = rate_lowest_index(sband, sta);
-		if (info->flags & IEEE80211_TX_CTL_NO_ACK)
-			info->control.rates[0].count = 1;
-
-		return;
-	}
-
 	rateidx = spinfo->txrate_idx;
 
 	if (rateidx >= sband->n_bitrates)
-- 
1.6.0.6

[PATCH 15/15] ath9k: remove rate control wraper

[Luis R. Rodriguez]

After the cleanup we just use get_rate as a wrapper, skip
the wrapper.

Cc: Derek Smithies <derek@indranet.co.nz>
Cc: Chittajit Mitra <Chittajit.Mitra@Atheros.com>
Signed-off-by: Luis R. Rodriguez <lrodriguez@atheros.com>
---
 drivers/net/wireless/ath/ath9k/rc.c |   18 ++++--------------
 1 files changed, 4 insertions(+), 14 deletions(-)

diff --git a/drivers/net/wireless/ath/ath9k/rc.c b/drivers/net/wireless/ath/ath9k/rc.c
index 2180180..3b9b819 100644
--- a/drivers/net/wireless/ath/ath9k/rc.c
+++ b/drivers/net/wireless/ath/ath9k/rc.c
@@ -792,11 +792,11 @@ static void ath_rc_rate_set_rtscts(struct ath_softc *sc,
 	tx_info->control.rts_cts_rate_idx = cix;
 }
 
-static void ath_rc_ratefind(struct ath_softc *sc,
-			    struct ath_rate_priv *ath_rc_priv,
-			    struct ieee80211_tx_rate_control *txrc,
-			    struct ieee80211_sta *sta)
+static void ath_get_rate(void *priv, struct ieee80211_sta *sta, void *priv_sta,
+			 struct ieee80211_tx_rate_control *txrc)
 {
+	struct ath_softc *sc = priv;
+	struct ath_rate_priv *ath_rc_priv = priv_sta;
 	const struct ath_rate_table *rate_table;
 	struct sk_buff *skb = txrc->skb;
 	struct ieee80211_tx_info *tx_info = IEEE80211_SKB_CB(skb);
@@ -1535,16 +1535,6 @@ exit:
 	kfree(tx_info_priv);
 }
 
-static void ath_get_rate(void *priv, struct ieee80211_sta *sta, void *priv_sta,
-			 struct ieee80211_tx_rate_control *txrc)
-{
-	struct ath_softc *sc = priv;
-	struct ath_rate_priv *ath_rc_priv = priv_sta;
-
-	/* Find tx rate for unicast frames */
-	ath_rc_ratefind(sc, ath_rc_priv, txrc, sta);
-}
-
 static void ath_rate_init(void *priv, struct ieee80211_supported_band *sband,
                           struct ieee80211_sta *sta, void *priv_sta)
 {
-- 
1.6.0.6

[ath9k-devel] [PATCH 15/15] ath9k: remove rate control wraper

[Luis R. Rodriguez]

After the cleanup we just use get_rate as a wrapper, skip
the wrapper.

Cc: Derek Smithies <derek@indranet.co.nz>
Cc: Chittajit Mitra <Chittajit.Mitra@Atheros.com>
Signed-off-by: Luis R. Rodriguez <lrodriguez@atheros.com>
---
 drivers/net/wireless/ath/ath9k/rc.c |   18 ++++--------------
 1 files changed, 4 insertions(+), 14 deletions(-)

diff --git a/drivers/net/wireless/ath/ath9k/rc.c b/drivers/net/wireless/ath/ath9k/rc.c
index 2180180..3b9b819 100644
--- a/drivers/net/wireless/ath/ath9k/rc.c
+++ b/drivers/net/wireless/ath/ath9k/rc.c
@@ -792,11 +792,11 @@ static void ath_rc_rate_set_rtscts(struct ath_softc *sc,
 	tx_info->control.rts_cts_rate_idx = cix;
 }
 
-static void ath_rc_ratefind(struct ath_softc *sc,
-			    struct ath_rate_priv *ath_rc_priv,
-			    struct ieee80211_tx_rate_control *txrc,
-			    struct ieee80211_sta *sta)
+static void ath_get_rate(void *priv, struct ieee80211_sta *sta, void *priv_sta,
+			 struct ieee80211_tx_rate_control *txrc)
 {
+	struct ath_softc *sc = priv;
+	struct ath_rate_priv *ath_rc_priv = priv_sta;
 	const struct ath_rate_table *rate_table;
 	struct sk_buff *skb = txrc->skb;
 	struct ieee80211_tx_info *tx_info = IEEE80211_SKB_CB(skb);
@@ -1535,16 +1535,6 @@ exit:
 	kfree(tx_info_priv);
 }
 
-static void ath_get_rate(void *priv, struct ieee80211_sta *sta, void *priv_sta,
-			 struct ieee80211_tx_rate_control *txrc)
-{
-	struct ath_softc *sc = priv;
-	struct ath_rate_priv *ath_rc_priv = priv_sta;
-
-	/* Find tx rate for unicast frames */
-	ath_rc_ratefind(sc, ath_rc_priv, txrc, sta);
-}
-
 static void ath_rate_init(void *priv, struct ieee80211_supported_band *sband,
                           struct ieee80211_sta *sta, void *priv_sta)
 {
-- 
1.6.0.6

Re: [PATCH 00/15] ath9k/mac80211/iwlwifi: rate control cleanup

[Luis R. Rodriguez]

On Thu, Jun 4, 2009 at 10:41 PM, Luis R. Rodriguez
<lrodriguez@atheros.com> wrote:
> ath9k's rate control code is so convoluted that it really make it
> difficult to spot bugs and fix them. Here's a bunch of cleanups for
> ath9k rate control code, and a few for mac80211. I just tested ath9k.
> Will have to test others later.

Here's the all-in-one file patch:

http://bombadil.infradead.org/~mcgrof/patches/ath9k/2009-06-04/rate-all.patch

Testing appreciated.

  Luis

[ath9k-devel] [PATCH 00/15] ath9k/mac80211/iwlwifi: rate control cleanup

[Luis R. Rodriguez]

On Thu, Jun 4, 2009 at 10:41 PM, Luis R. Rodriguez
<lrodriguez@atheros.com> wrote:
> ath9k's rate control code is so convoluted that it really make it
> difficult to spot bugs and fix them. Here's a bunch of cleanups for
> ath9k rate control code, and a few for mac80211. I just tested ath9k.
> Will have to test others later.

Here's the all-in-one file patch:

http://bombadil.infradead.org/~mcgrof/patches/ath9k/2009-06-04/rate-all.patch

Testing appreciated.

  Luis

Re: [PATCH 12/15] mac80211: make minstrel/pid RC use ieee80211_is_data(fc)

[Johannes Berg]

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

On Fri, 2009-06-05 at 01:41 -0400, Luis R. Rodriguez wrote:

> -	u16 fc;
> +	__le16 fc;
>  
>  	if (txrc->rts)
>  		info->control.rates[0].count =
> @@ -291,8 +291,7 @@ rate_control_pid_get_rate(void *priv, struct ieee80211_sta *sta,
>  
>  	/* Send management frames and NO_ACK data using lowest rate. */
>  	fc = le16_to_cpu(hdr->frame_control);
> -	if (!sta || !spinfo ||
> -	    (fc & IEEE80211_FCTL_FTYPE) != IEEE80211_FTYPE_DATA ||
> +	if (!sta || !spinfo || !ieee80211_is_data(fc) ||

you might want to run sparse on this entire patchset.

johannes

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

[ath9k-devel] [PATCH 12/15] mac80211: make minstrel/pid RC use ieee80211_is_data(fc)

[Johannes Berg]

On Fri, 2009-06-05 at 01:41 -0400, Luis R. Rodriguez wrote:

> -	u16 fc;
> +	__le16 fc;
>  
>  	if (txrc->rts)
>  		info->control.rates[0].count =
> @@ -291,8 +291,7 @@ rate_control_pid_get_rate(void *priv, struct ieee80211_sta *sta,
>  
>  	/* Send management frames and NO_ACK data using lowest rate. */
>  	fc = le16_to_cpu(hdr->frame_control);
> -	if (!sta || !spinfo ||
> -	    (fc & IEEE80211_FCTL_FTYPE) != IEEE80211_FTYPE_DATA ||
> +	if (!sta || !spinfo || !ieee80211_is_data(fc) ||

you might want to run sparse on this entire patchset.

johannes
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 801 bytes
Desc: This is a digitally signed message part
Url : http://lists.ath9k.org/pipermail/ath9k-devel/attachments/20090605/de004bd8/attachment.pgp 

Re: [PATCH 14/15] mac80211: move management / no-ack frame rate decision to mac80211

[Johannes Berg]

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

On Fri, 2009-06-05 at 01:41 -0400, Luis R. Rodriguez wrote:
> All rate control algorithms agree to send management and no-ack
> frames at the lowest rate. They also agree to do this when sta
> and the private rate control data is NULL. We move this to mac80211
> and simplify the rate control algorithm code.

We previously thought this would constrain our rate control algorithms
more than we would like for future experimentation with higher multicast
rates. Has that changed?

johannes

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

[ath9k-devel] [PATCH 14/15] mac80211: move management / no-ack frame rate decision to mac80211

[Johannes Berg]

On Fri, 2009-06-05 at 01:41 -0400, Luis R. Rodriguez wrote:
> All rate control algorithms agree to send management and no-ack
> frames at the lowest rate. They also agree to do this when sta
> and the private rate control data is NULL. We move this to mac80211
> and simplify the rate control algorithm code.

We previously thought this would constrain our rate control algorithms
more than we would like for future experimentation with higher multicast
rates. Has that changed?

johannes
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 801 bytes
Desc: This is a digitally signed message part
Url : http://lists.ath9k.org/pipermail/ath9k-devel/attachments/20090605/d47f1498/attachment.pgp 

Re: [PATCH 01/15] ath9k: fix oops by downgrading assert in rc.c

[Vasanthakumar Thiagarajan]

> +
> +       /*
> +        * Fine tuning for when no decent rate was found, the
> +        * lowest should *not* be used under normal circumstances.
> +        */
> +       if (rix == ath_rc_priv->valid_rate_index[0]) {
> +               DPRINTF(sc, ATH_DBG_RATE, "lowest rate being used, "
> +                       "disabling MRR\n");
> +               rates[0].idx = rate_lowest_index(sband, sta);
> +               /* Disable MRR when ath_rc_ratefind_ht() found rate 0 */
> +               rates[1].idx = -1;
> +       }

I think we can still fill other rates (1..3) with the lowest rate
index as we dont differentiate the situation where the lowest rate
is chosen truely by the algorithm from this particular case. 

Vasanth

[ath9k-devel] [PATCH 01/15] ath9k: fix oops by downgrading assert in rc.c

[Vasanthakumar Thiagarajan]

> +
> +       /*
> +        * Fine tuning for when no decent rate was found, the
> +        * lowest should *not* be used under normal circumstances.
> +        */
> +       if (rix == ath_rc_priv->valid_rate_index[0]) {
> +               DPRINTF(sc, ATH_DBG_RATE, "lowest rate being used, "
> +                       "disabling MRR\n");
> +               rates[0].idx = rate_lowest_index(sband, sta);
> +               /* Disable MRR when ath_rc_ratefind_ht() found rate 0 */
> +               rates[1].idx = -1;
> +       }

I think we can still fill other rates (1..3) with the lowest rate
index as we dont differentiate the situation where the lowest rate
is chosen truely by the algorithm from this particular case. 

Vasanth

Re: [PATCH 12/15] mac80211: make minstrel/pid RC use ieee80211_is_data(fc)

[Luis R. Rodriguez]

On Thu, Jun 4, 2009 at 11:24 PM, Johannes Berg
<johannes@sipsolutions.net> wrote:
> On Fri, 2009-06-05 at 01:41 -0400, Luis R. Rodriguez wrote:
>
>> -     u16 fc;
>> +     __le16 fc;
>>
>>       if (txrc->rts)
>>               info->control.rates[0].count =
>> @@ -291,8 +291,7 @@ rate_control_pid_get_rate(void *priv, struct ieee80211_sta *sta,
>>
>>       /* Send management frames and NO_ACK data using lowest rate. */
>>       fc = le16_to_cpu(hdr->frame_control);
>> -     if (!sta || !spinfo ||
>> -         (fc & IEEE80211_FCTL_FTYPE) != IEEE80211_FTYPE_DATA ||
>> +     if (!sta || !spinfo || !ieee80211_is_data(fc) ||
>
> you might want to run sparse on this entire patchset.

I did, PID was not on my kconfig though, so its the only thing I
didn't test compile.

 Luis

[ath9k-devel] [PATCH 12/15] mac80211: make minstrel/pid RC use ieee80211_is_data(fc)

[Luis R. Rodriguez]

On Thu, Jun 4, 2009 at 11:24 PM, Johannes Berg
<johannes@sipsolutions.net> wrote:
> On Fri, 2009-06-05 at 01:41 -0400, Luis R. Rodriguez wrote:
>
>> - ? ? u16 fc;
>> + ? ? __le16 fc;
>>
>> ? ? ? if (txrc->rts)
>> ? ? ? ? ? ? ? info->control.rates[0].count =
>> @@ -291,8 +291,7 @@ rate_control_pid_get_rate(void *priv, struct ieee80211_sta *sta,
>>
>> ? ? ? /* Send management frames and NO_ACK data using lowest rate. */
>> ? ? ? fc = le16_to_cpu(hdr->frame_control);
>> - ? ? if (!sta || !spinfo ||
>> - ? ? ? ? (fc & IEEE80211_FCTL_FTYPE) != IEEE80211_FTYPE_DATA ||
>> + ? ? if (!sta || !spinfo || !ieee80211_is_data(fc) ||
>
> you might want to run sparse on this entire patchset.

I did, PID was not on my kconfig though, so its the only thing I
didn't test compile.

 Luis

Re: [PATCH 12/15] mac80211: make minstrel/pid RC use ieee80211_is_data(fc)

[Johannes Berg]

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

On Thu, 2009-06-04 at 23:34 -0700, Luis R. Rodriguez wrote:
> On Thu, Jun 4, 2009 at 11:24 PM, Johannes Berg
> <johannes@sipsolutions.net> wrote:
> > On Fri, 2009-06-05 at 01:41 -0400, Luis R. Rodriguez wrote:
> >
> >> -     u16 fc;
> >> +     __le16 fc;
> >>
> >>       if (txrc->rts)
> >>               info->control.rates[0].count =
> >> @@ -291,8 +291,7 @@ rate_control_pid_get_rate(void *priv, struct ieee80211_sta *sta,
> >>
> >>       /* Send management frames and NO_ACK data using lowest rate. */
> >>       fc = le16_to_cpu(hdr->frame_control);
> >> -     if (!sta || !spinfo ||
> >> -         (fc & IEEE80211_FCTL_FTYPE) != IEEE80211_FTYPE_DATA ||
> >> +     if (!sta || !spinfo || !ieee80211_is_data(fc) ||
> >
> > you might want to run sparse on this entire patchset.
> 
> I did, PID was not on my kconfig though, so its the only thing I
> didn't test compile.

Ok. Curious I accidentally found the only bug then :)

johannes

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

[ath9k-devel] [PATCH 12/15] mac80211: make minstrel/pid RC use ieee80211_is_data(fc)

[Johannes Berg]

On Thu, 2009-06-04 at 23:34 -0700, Luis R. Rodriguez wrote:
> On Thu, Jun 4, 2009 at 11:24 PM, Johannes Berg
> <johannes@sipsolutions.net> wrote:
> > On Fri, 2009-06-05 at 01:41 -0400, Luis R. Rodriguez wrote:
> >
> >> -     u16 fc;
> >> +     __le16 fc;
> >>
> >>       if (txrc->rts)
> >>               info->control.rates[0].count =
> >> @@ -291,8 +291,7 @@ rate_control_pid_get_rate(void *priv, struct ieee80211_sta *sta,
> >>
> >>       /* Send management frames and NO_ACK data using lowest rate. */
> >>       fc = le16_to_cpu(hdr->frame_control);
> >> -     if (!sta || !spinfo ||
> >> -         (fc & IEEE80211_FCTL_FTYPE) != IEEE80211_FTYPE_DATA ||
> >> +     if (!sta || !spinfo || !ieee80211_is_data(fc) ||
> >
> > you might want to run sparse on this entire patchset.
> 
> I did, PID was not on my kconfig though, so its the only thing I
> didn't test compile.

Ok. Curious I accidentally found the only bug then :)

johannes
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 801 bytes
Desc: This is a digitally signed message part
Url : http://lists.ath9k.org/pipermail/ath9k-devel/attachments/20090605/2ae86fdd/attachment.pgp 

Re: [ath9k-devel] [PATCH 14/15] mac80211: move management / no-ack frame rate decision to mac80211

[Jouni Malinen]

On Thu, 2009-06-04 at 23:28 -0700, Johannes Berg wrote:
> On Fri, 2009-06-05 at 01:41 -0400, Luis R. Rodriguez wrote:
> > All rate control algorithms agree to send management and no-ack
> > frames at the lowest rate. They also agree to do this when sta
> > and the private rate control data is NULL. We move this to mac80211
> > and simplify the rate control algorithm code.
> 
> We previously thought this would constrain our rate control algorithms
> more than we would like for future experimentation with higher multicast
> rates. Has that changed?

I hope not. I don't like the idea of forcing the lowest rate in
mac80211; we should allow rate control algorithms to be improved to do
more intelligent selection of rate at least for multicast/broadcast
frames. That might also be of use for some (likely post-association)
unicast management frames since their use is going to increase in the
future.

- Jouni

[ath9k-devel] [PATCH 14/15] mac80211: move management / no-ack frame rate decision to mac80211

[Jouni Malinen]

On Thu, 2009-06-04 at 23:28 -0700, Johannes Berg wrote:
> On Fri, 2009-06-05 at 01:41 -0400, Luis R. Rodriguez wrote:
> > All rate control algorithms agree to send management and no-ack
> > frames at the lowest rate. They also agree to do this when sta
> > and the private rate control data is NULL. We move this to mac80211
> > and simplify the rate control algorithm code.
> 
> We previously thought this would constrain our rate control algorithms
> more than we would like for future experimentation with higher multicast
> rates. Has that changed?

I hope not. I don't like the idea of forcing the lowest rate in
mac80211; we should allow rate control algorithms to be improved to do
more intelligent selection of rate at least for multicast/broadcast
frames. That might also be of use for some (likely post-association)
unicast management frames since their use is going to increase in the
future.

- Jouni

Re: [ath9k-devel] [PATCH 14/15] mac80211: move management / no-ack frame rate decision to mac80211

[Luis R. Rodriguez]

On Fri, Jun 5, 2009 at 12:42 AM, Jouni Malinen<jouni.malinen@atheros.com> wrote:
> On Thu, 2009-06-04 at 23:28 -0700, Johannes Berg wrote:
>> On Fri, 2009-06-05 at 01:41 -0400, Luis R. Rodriguez wrote:
>> > All rate control algorithms agree to send management and no-ack
>> > frames at the lowest rate. They also agree to do this when sta
>> > and the private rate control data is NULL. We move this to mac80211
>> > and simplify the rate control algorithm code.
>>
>> We previously thought this would constrain our rate control algorithms
>> more than we would like for future experimentation with higher multicast
>> rates. Has that changed?
>
> I hope not. I don't like the idea of forcing the lowest rate in
> mac80211; we should allow rate control algorithms to be improved to do
> more intelligent selection of rate at least for multicast/broadcast
> frames. That might also be of use for some (likely post-association)
> unicast management frames since their use is going to increase in the
> future.

How about a generic call for now then and have each RC use it for now.

  Luis

[ath9k-devel] [PATCH 14/15] mac80211: move management / no-ack frame rate decision to mac80211

[Luis R. Rodriguez]

On Fri, Jun 5, 2009 at 12:42 AM, Jouni Malinen<jouni.malinen@atheros.com> wrote:
> On Thu, 2009-06-04 at 23:28 -0700, Johannes Berg wrote:
>> On Fri, 2009-06-05 at 01:41 -0400, Luis R. Rodriguez wrote:
>> > All rate control algorithms agree to send management and no-ack
>> > frames at the lowest rate. They also agree to do this when sta
>> > and the private rate control data is NULL. We move this to mac80211
>> > and simplify the rate control algorithm code.
>>
>> We previously thought this would constrain our rate control algorithms
>> more than we would like for future experimentation with higher multicast
>> rates. Has that changed?
>
> I hope not. I don't like the idea of forcing the lowest rate in
> mac80211; we should allow rate control algorithms to be improved to do
> more intelligent selection of rate at least for multicast/broadcast
> frames. That might also be of use for some (likely post-association)
> unicast management frames since their use is going to increase in the
> future.

How about a generic call for now then and have each RC use it for now.

  Luis

Re: [PATCH 01/15] ath9k: fix oops by downgrading assert in rc.c

[Luis R. Rodriguez]

On Thu, Jun 4, 2009 at 11:30 PM, Vasanthakumar
Thiagarajan<vasanth@atheros.com> wrote:
>
>> +
>> +       /*
>> +        * Fine tuning for when no decent rate was found, the
>> +        * lowest should *not* be used under normal circumstances.
>> +        */
>> +       if (rix == ath_rc_priv->valid_rate_index[0]) {
>> +               DPRINTF(sc, ATH_DBG_RATE, "lowest rate being used, "
>> +                       "disabling MRR\n");
>> +               rates[0].idx = rate_lowest_index(sband, sta);
>> +               /* Disable MRR when ath_rc_ratefind_ht() found rate 0 */
>> +               rates[1].idx = -1;
>> +       }
>
> I think we can still fill other rates (1..3) with the lowest rate
> index as we dont differentiate the situation where the lowest rate
> is chosen truely by the algorithm from this particular case.

I thought about that as well, but does it really make sense for us to
use MRR with the same lowest rate? That's why I just used one segment.
Thoughts?

  Luis

[ath9k-devel] [PATCH 01/15] ath9k: fix oops by downgrading assert in rc.c

[Luis R. Rodriguez]

On Thu, Jun 4, 2009 at 11:30 PM, Vasanthakumar
Thiagarajan<vasanth@atheros.com> wrote:
>
>> +
>> + ? ? ? /*
>> + ? ? ? ?* Fine tuning for when no decent rate was found, the
>> + ? ? ? ?* lowest should *not* be used under normal circumstances.
>> + ? ? ? ?*/
>> + ? ? ? if (rix == ath_rc_priv->valid_rate_index[0]) {
>> + ? ? ? ? ? ? ? DPRINTF(sc, ATH_DBG_RATE, "lowest rate being used, "
>> + ? ? ? ? ? ? ? ? ? ? ? "disabling MRR\n");
>> + ? ? ? ? ? ? ? rates[0].idx = rate_lowest_index(sband, sta);
>> + ? ? ? ? ? ? ? /* Disable MRR when ath_rc_ratefind_ht() found rate 0 */
>> + ? ? ? ? ? ? ? rates[1].idx = -1;
>> + ? ? ? }
>
> I think we can still fill other rates (1..3) with the lowest rate
> index as we dont differentiate the situation where the lowest rate
> is chosen truely by the algorithm from this particular case.

I thought about that as well, but does it really make sense for us to
use MRR with the same lowest rate? That's why I just used one segment.
Thoughts?

  Luis

Re: [PATCH 01/15] ath9k: fix oops by downgrading assert in rc.c

[Vasanthakumar Thiagarajan]

On Fri, Jun 05, 2009 at 01:17:16PM +0530, Luis Rodriguez wrote:
> On Thu, Jun 4, 2009 at 11:30 PM, Vasanthakumar
> Thiagarajan<vasanth@atheros.com> wrote:
> >
> >> +
> >> +       /*
> >> +        * Fine tuning for when no decent rate was found, the
> >> +        * lowest should *not* be used under normal circumstances.
> >> +        */
> >> +       if (rix == ath_rc_priv->valid_rate_index[0]) {
> >> +               DPRINTF(sc, ATH_DBG_RATE, "lowest rate being used, "
> >> +                       "disabling MRR\n");
> >> +               rates[0].idx = rate_lowest_index(sband, sta);
> >> +               /* Disable MRR when ath_rc_ratefind_ht() found rate 0 */
> >> +               rates[1].idx = -1;
> >> +       }
> >
> > I think we can still fill other rates (1..3) with the lowest rate
> > index as we dont differentiate the situation where the lowest rate
> > is chosen truely by the algorithm from this particular case.
> 
> I thought about that as well, but does it really make sense for us to
> use MRR with the same lowest rate? That's why I just used one segment.
> Thoughts?

or we can try for max_retry (4) times. In that case the rate indices of
other rates (just not 1) should be made -1 or this segment should be
moved just below the rate find.

Vasanth

[ath9k-devel] [PATCH 01/15] ath9k: fix oops by downgrading assert in rc.c

[Vasanthakumar Thiagarajan]

On Fri, Jun 05, 2009 at 01:17:16PM +0530, Luis Rodriguez wrote:
> On Thu, Jun 4, 2009 at 11:30 PM, Vasanthakumar
> Thiagarajan<vasanth@atheros.com> wrote:
> >
> >> +
> >> +       /*
> >> +        * Fine tuning for when no decent rate was found, the
> >> +        * lowest should *not* be used under normal circumstances.
> >> +        */
> >> +       if (rix == ath_rc_priv->valid_rate_index[0]) {
> >> +               DPRINTF(sc, ATH_DBG_RATE, "lowest rate being used, "
> >> +                       "disabling MRR\n");
> >> +               rates[0].idx = rate_lowest_index(sband, sta);
> >> +               /* Disable MRR when ath_rc_ratefind_ht() found rate 0 */
> >> +               rates[1].idx = -1;
> >> +       }
> >
> > I think we can still fill other rates (1..3) with the lowest rate
> > index as we dont differentiate the situation where the lowest rate
> > is chosen truely by the algorithm from this particular case.
> 
> I thought about that as well, but does it really make sense for us to
> use MRR with the same lowest rate? That's why I just used one segment.
> Thoughts?

or we can try for max_retry (4) times. In that case the rate indices of
other rates (just not 1) should be made -1 or this segment should be
moved just below the rate find.

Vasanth

Help ath5k

[José Luis Vargas Araoz]

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


Good morning all.
I wanna configured LinuxVoyage (Debian distro kernel 2.6.26) in 802.11a with Ath5k.
How i've to modify /etc/network/interfaces?

Thanks for your time.


_________________________________________________________________
News, entertainment and everything you care about at Live.com. Get it now!
http://www.live.com/getstarted.aspx

[-- Attachment #2: Type: text/html, Size: 538 bytes --]

[ath9k-devel] Help ath5k

[José Luis Vargas Araoz]

Good morning all.
I wanna configured LinuxVoyage (Debian distro kernel 2.6.26) in 802.11a with Ath5k.
How i've to modify /etc/network/interfaces?

Thanks for your time.


_________________________________________________________________
News, entertainment and everything you care about at Live.com. Get it now!
http://www.live.com/getstarted.aspx
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.ath9k.org/pipermail/ath9k-devel/attachments/20090605/680b4e2d/attachment.htm 

Re: [PATCH 01/15] ath9k: fix oops by downgrading assert in rc.c

[Luis R. Rodriguez]

On Fri, Jun 05, 2009 at 12:52:52AM -0700, Vasanth Thiagarajan wrote:
> On Fri, Jun 05, 2009 at 01:17:16PM +0530, Luis Rodriguez wrote:
> > On Thu, Jun 4, 2009 at 11:30 PM, Vasanthakumar
> > Thiagarajan<vasanth@atheros.com> wrote:
> > >
> > >> +
> > >> +       /*
> > >> +        * Fine tuning for when no decent rate was found, the
> > >> +        * lowest should *not* be used under normal circumstances.
> > >> +        */
> > >> +       if (rix == ath_rc_priv->valid_rate_index[0]) {
> > >> +               DPRINTF(sc, ATH_DBG_RATE, "lowest rate being used, "
> > >> +                       "disabling MRR\n");
> > >> +               rates[0].idx = rate_lowest_index(sband, sta);
> > >> +               /* Disable MRR when ath_rc_ratefind_ht() found rate 0 */
> > >> +               rates[1].idx = -1;
> > >> +       }
> > >
> > > I think we can still fill other rates (1..3) with the lowest rate
> > > index as we dont differentiate the situation where the lowest rate
> > > is chosen truely by the algorithm from this particular case.
> >
> > I thought about that as well, but does it really make sense for us to
> > use MRR with the same lowest rate? That's why I just used one segment.
> > Thoughts?
> 
> or we can try for max_retry (4) times. In that case the rate indices of
> other rates (just not 1) should be made -1 or this segment should be
> moved just below the rate find.

So the count should have already been set up to 4, and the next segment [1]
is set to -1. Please let me know if there is anything else you see needs
change.

  Luis

[ath9k-devel] [PATCH 01/15] ath9k: fix oops by downgrading assert in rc.c

[Luis R. Rodriguez]

On Fri, Jun 05, 2009 at 12:52:52AM -0700, Vasanth Thiagarajan wrote:
> On Fri, Jun 05, 2009 at 01:17:16PM +0530, Luis Rodriguez wrote:
> > On Thu, Jun 4, 2009 at 11:30 PM, Vasanthakumar
> > Thiagarajan<vasanth@atheros.com> wrote:
> > >
> > >> +
> > >> +       /*
> > >> +        * Fine tuning for when no decent rate was found, the
> > >> +        * lowest should *not* be used under normal circumstances.
> > >> +        */
> > >> +       if (rix == ath_rc_priv->valid_rate_index[0]) {
> > >> +               DPRINTF(sc, ATH_DBG_RATE, "lowest rate being used, "
> > >> +                       "disabling MRR\n");
> > >> +               rates[0].idx = rate_lowest_index(sband, sta);
> > >> +               /* Disable MRR when ath_rc_ratefind_ht() found rate 0 */
> > >> +               rates[1].idx = -1;
> > >> +       }
> > >
> > > I think we can still fill other rates (1..3) with the lowest rate
> > > index as we dont differentiate the situation where the lowest rate
> > > is chosen truely by the algorithm from this particular case.
> >
> > I thought about that as well, but does it really make sense for us to
> > use MRR with the same lowest rate? That's why I just used one segment.
> > Thoughts?
> 
> or we can try for max_retry (4) times. In that case the rate indices of
> other rates (just not 1) should be made -1 or this segment should be
> moved just below the rate find.

So the count should have already been set up to 4, and the next segment [1]
is set to -1. Please let me know if there is anything else you see needs
change.

  Luis

Re: [PATCH 01/15] ath9k: fix oops by downgrading assert in rc.c

[Gábor Stefanik]

On Fri, Jun 5, 2009 at 7:41 AM, Luis R. Rodriguez<lrodriguez@atheros.com> wrote:
> Unfortunately locking the driver rate control private area
> doesn't resolve this issue and in fact causes some issues.
> The root cause is yet undetermined. For now instead of crashing
> lets just downgrades the ASSERT() and only show a trace when
> we are debugging with ATH_DBG_CONFIG enabled. That attempt
> and its results can be seen here:
>
> http://marc.info/?l=linux-wireless&m=124399166108295&w=1
>
> The ASSERT was happening because the rate control algorithm
> figures it should find at least one valid dual stream or
> single stream rate. *Something* is causing us to not find
> such rate. This happens after association and only when HT
> is enabled AFAICT. What we are doing by downgrading the assert
> is informing the driver to use the lowest rate index. When
> this happens lets disable multi rate retry and only try on
> the lowest supported rate.
>
> Traces of the ASSERT are available on this thread:
>
> http://marc.info/?l=linux-wireless&m=124277331319024
>
> At least one bug report (as side issue, mind you):
>
> https://bugzilla.redhat.com/show_bug.cgi?id=503285
>
> Signed-off-by: Luis R. Rodriguez <lrodriguez@atheros.com>
> ---
>  drivers/net/wireless/ath/ath9k/debug.h |    1 +
>  drivers/net/wireless/ath/ath9k/rc.c    |   40 +++++++++++++++++++++++++++----
>  2 files changed, 35 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/net/wireless/ath/ath9k/debug.h b/drivers/net/wireless/ath/ath9k/debug.h
> index edda15b..9d72bc8 100644
> --- a/drivers/net/wireless/ath/ath9k/debug.h
> +++ b/drivers/net/wireless/ath/ath9k/debug.h
> @@ -30,6 +30,7 @@ enum ATH_DEBUG {
>        ATH_DBG_CONFIG          = 0x00000200,
>        ATH_DBG_FATAL           = 0x00000400,
>        ATH_DBG_PS              = 0x00000800,
> +       ATH_DBG_RATE            = 0x00001000,
>        ATH_DBG_ANY             = 0xffffffff
>  };
>
> diff --git a/drivers/net/wireless/ath/ath9k/rc.c b/drivers/net/wireless/ath/ath9k/rc.c
> index ba06e78..abad86b 100644
> --- a/drivers/net/wireless/ath/ath9k/rc.c
> +++ b/drivers/net/wireless/ath/ath9k/rc.c
> @@ -741,10 +741,24 @@ static u8 ath_rc_ratefind_ht(struct ath_softc *sc,
>        if (rate > (ath_rc_priv->rate_table_size - 1))
>                rate = ath_rc_priv->rate_table_size - 1;
>
> -       ASSERT((rate_table->info[rate].valid &&
> -               (ath_rc_priv->ht_cap & WLAN_RC_DS_FLAG)) ||
> -              (rate_table->info[rate].valid_single_stream &&
> -               !(ath_rc_priv->ht_cap & WLAN_RC_DS_FLAG)));
> +       if (rate_table->info[rate].valid &&
> +           (ath_rc_priv->ht_cap & WLAN_RC_DS_FLAG))
> +               return rate;
> +
> +       if (rate_table->info[rate].valid_single_stream &&
> +           !(ath_rc_priv->ht_cap & WLAN_RC_DS_FLAG));
> +               return rate;
> +
> +       /*
> +        * This should not happen, but we know it does for now... This
> +        * needs a proper fix but we're still not sure how this is caused.
> +        * Its not *critical* though so lets just warn when debug is enabled
> +        * for configuration changes.
> +        */
> +       if (sc->debug.debug_mask & ATH_DBG_RATE)
> +               WARN_ON(1);

WARN_ON(sc->debug.debug_mask & ATH_DBG_RATE)

-- 
Vista: [V]iruses, [I]ntruders, [S]pyware, [T]rojans and [A]dware. :-)

[ath9k-devel] [PATCH 01/15] ath9k: fix oops by downgrading assert in rc.c

[Gábor Stefanik]

On Fri, Jun 5, 2009 at 7:41 AM, Luis R. Rodriguez<lrodriguez@atheros.com> wrote:
> Unfortunately locking the driver rate control private area
> doesn't resolve this issue and in fact causes some issues.
> The root cause is yet undetermined. For now instead of crashing
> lets just downgrades the ASSERT() and only show a trace when
> we are debugging with ATH_DBG_CONFIG enabled. That attempt
> and its results can be seen here:
>
> http://marc.info/?l=linux-wireless&m=124399166108295&w=1
>
> The ASSERT was happening because the rate control algorithm
> figures it should find at least one valid dual stream or
> single stream rate. *Something* is causing us to not find
> such rate. This happens after association and only when HT
> is enabled AFAICT. What we are doing by downgrading the assert
> is informing the driver to use the lowest rate index. When
> this happens lets disable multi rate retry and only try on
> the lowest supported rate.
>
> Traces of the ASSERT are available on this thread:
>
> http://marc.info/?l=linux-wireless&m=124277331319024
>
> At least one bug report (as side issue, mind you):
>
> https://bugzilla.redhat.com/show_bug.cgi?id=503285
>
> Signed-off-by: Luis R. Rodriguez <lrodriguez@atheros.com>
> ---
> ?drivers/net/wireless/ath/ath9k/debug.h | ? ?1 +
> ?drivers/net/wireless/ath/ath9k/rc.c ? ?| ? 40 +++++++++++++++++++++++++++----
> ?2 files changed, 35 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/net/wireless/ath/ath9k/debug.h b/drivers/net/wireless/ath/ath9k/debug.h
> index edda15b..9d72bc8 100644
> --- a/drivers/net/wireless/ath/ath9k/debug.h
> +++ b/drivers/net/wireless/ath/ath9k/debug.h
> @@ -30,6 +30,7 @@ enum ATH_DEBUG {
> ? ? ? ?ATH_DBG_CONFIG ? ? ? ? ?= 0x00000200,
> ? ? ? ?ATH_DBG_FATAL ? ? ? ? ? = 0x00000400,
> ? ? ? ?ATH_DBG_PS ? ? ? ? ? ? ?= 0x00000800,
> + ? ? ? ATH_DBG_RATE ? ? ? ? ? ?= 0x00001000,
> ? ? ? ?ATH_DBG_ANY ? ? ? ? ? ? = 0xffffffff
> ?};
>
> diff --git a/drivers/net/wireless/ath/ath9k/rc.c b/drivers/net/wireless/ath/ath9k/rc.c
> index ba06e78..abad86b 100644
> --- a/drivers/net/wireless/ath/ath9k/rc.c
> +++ b/drivers/net/wireless/ath/ath9k/rc.c
> @@ -741,10 +741,24 @@ static u8 ath_rc_ratefind_ht(struct ath_softc *sc,
> ? ? ? ?if (rate > (ath_rc_priv->rate_table_size - 1))
> ? ? ? ? ? ? ? ?rate = ath_rc_priv->rate_table_size - 1;
>
> - ? ? ? ASSERT((rate_table->info[rate].valid &&
> - ? ? ? ? ? ? ? (ath_rc_priv->ht_cap & WLAN_RC_DS_FLAG)) ||
> - ? ? ? ? ? ? ?(rate_table->info[rate].valid_single_stream &&
> - ? ? ? ? ? ? ? !(ath_rc_priv->ht_cap & WLAN_RC_DS_FLAG)));
> + ? ? ? if (rate_table->info[rate].valid &&
> + ? ? ? ? ? (ath_rc_priv->ht_cap & WLAN_RC_DS_FLAG))
> + ? ? ? ? ? ? ? return rate;
> +
> + ? ? ? if (rate_table->info[rate].valid_single_stream &&
> + ? ? ? ? ? !(ath_rc_priv->ht_cap & WLAN_RC_DS_FLAG));
> + ? ? ? ? ? ? ? return rate;
> +
> + ? ? ? /*
> + ? ? ? ?* This should not happen, but we know it does for now... This
> + ? ? ? ?* needs a proper fix but we're still not sure how this is caused.
> + ? ? ? ?* Its not *critical* though so lets just warn when debug is enabled
> + ? ? ? ?* for configuration changes.
> + ? ? ? ?*/
> + ? ? ? if (sc->debug.debug_mask & ATH_DBG_RATE)
> + ? ? ? ? ? ? ? WARN_ON(1);

WARN_ON(sc->debug.debug_mask & ATH_DBG_RATE)

-- 
Vista: [V]iruses, [I]ntruders, [S]pyware, [T]rojans and [A]dware. :-)

Re: [PATCH 01/15] ath9k: fix oops by downgrading assert in rc.c

[Luis R. Rodriguez]

2009/6/5 Gábor Stefanik <netrolller.3d@gmail.com>:
> On Fri, Jun 5, 2009 at 7:41 AM, Luis R. Rodriguez<lrodriguez@atheros.com> wrote:

>> index ba06e78..abad86b 100644
>> --- a/drivers/net/wireless/ath/ath9k/rc.c
>> +++ b/drivers/net/wireless/ath/ath9k/rc.c
>> @@ -741,10 +741,24 @@ static u8 ath_rc_ratefind_ht(struct ath_softc *sc,
>>        if (rate > (ath_rc_priv->rate_table_size - 1))
>>                rate = ath_rc_priv->rate_table_size - 1;
>>
>> -       ASSERT((rate_table->info[rate].valid &&
>> -               (ath_rc_priv->ht_cap & WLAN_RC_DS_FLAG)) ||
>> -              (rate_table->info[rate].valid_single_stream &&
>> -               !(ath_rc_priv->ht_cap & WLAN_RC_DS_FLAG)));
>> +       if (rate_table->info[rate].valid &&
>> +           (ath_rc_priv->ht_cap & WLAN_RC_DS_FLAG))
>> +               return rate;
>> +
>> +       if (rate_table->info[rate].valid_single_stream &&
>> +           !(ath_rc_priv->ht_cap & WLAN_RC_DS_FLAG));
>> +               return rate;
>> +
>> +       /*
>> +        * This should not happen, but we know it does for now... This
>> +        * needs a proper fix but we're still not sure how this is caused.
>> +        * Its not *critical* though so lets just warn when debug is enabled
>> +        * for configuration changes.
>> +        */
>> +       if (sc->debug.debug_mask & ATH_DBG_RATE)
>> +               WARN_ON(1);
>
> WARN_ON(sc->debug.debug_mask & ATH_DBG_RATE)

Will do, thanks.

  Luis

[ath9k-devel] [PATCH 01/15] ath9k: fix oops by downgrading assert in rc.c

[Luis R. Rodriguez]

2009/6/5 G?bor Stefanik <netrolller.3d@gmail.com>:
> On Fri, Jun 5, 2009 at 7:41 AM, Luis R. Rodriguez<lrodriguez@atheros.com> wrote:

>> index ba06e78..abad86b 100644
>> --- a/drivers/net/wireless/ath/ath9k/rc.c
>> +++ b/drivers/net/wireless/ath/ath9k/rc.c
>> @@ -741,10 +741,24 @@ static u8 ath_rc_ratefind_ht(struct ath_softc *sc,
>> ? ? ? ?if (rate > (ath_rc_priv->rate_table_size - 1))
>> ? ? ? ? ? ? ? ?rate = ath_rc_priv->rate_table_size - 1;
>>
>> - ? ? ? ASSERT((rate_table->info[rate].valid &&
>> - ? ? ? ? ? ? ? (ath_rc_priv->ht_cap & WLAN_RC_DS_FLAG)) ||
>> - ? ? ? ? ? ? ?(rate_table->info[rate].valid_single_stream &&
>> - ? ? ? ? ? ? ? !(ath_rc_priv->ht_cap & WLAN_RC_DS_FLAG)));
>> + ? ? ? if (rate_table->info[rate].valid &&
>> + ? ? ? ? ? (ath_rc_priv->ht_cap & WLAN_RC_DS_FLAG))
>> + ? ? ? ? ? ? ? return rate;
>> +
>> + ? ? ? if (rate_table->info[rate].valid_single_stream &&
>> + ? ? ? ? ? !(ath_rc_priv->ht_cap & WLAN_RC_DS_FLAG));
>> + ? ? ? ? ? ? ? return rate;
>> +
>> + ? ? ? /*
>> + ? ? ? ?* This should not happen, but we know it does for now... This
>> + ? ? ? ?* needs a proper fix but we're still not sure how this is caused.
>> + ? ? ? ?* Its not *critical* though so lets just warn when debug is enabled
>> + ? ? ? ?* for configuration changes.
>> + ? ? ? ?*/
>> + ? ? ? if (sc->debug.debug_mask & ATH_DBG_RATE)
>> + ? ? ? ? ? ? ? WARN_ON(1);
>
> WARN_ON(sc->debug.debug_mask & ATH_DBG_RATE)

Will do, thanks.

  Luis

RE: [PATCH 01/15] ath9k: fix oops by downgrading assert in rc.c

[Vasanth Thiagarajan]

________________________________________

> > >> +       /*
> > >> +        * Fine tuning for when no decent rate was found, the
> > >> +        * lowest should *not* be used under normal circumstances.
> > >> +        */
> > >> +       if (rix == ath_rc_priv->valid_rate_index[0]) {
> > >> +               DPRINTF(sc, ATH_DBG_RATE, "lowest rate being used, "
> > >> +                       "disabling MRR\n");
> > >> +               rates[0].idx = rate_lowest_index(sband, sta);
> > >> +               /* Disable MRR when ath_rc_ratefind_ht() found rate 0 */
> > >> +               rates[1].idx = -1;
> > >> +       }
> > >
> > > I think we can still fill other rates (1..3) with the lowest rate
> > > index as we dont differentiate the situation where the lowest rate
> > > is chosen truely by the algorithm from this particular case.
> >
> > I thought about that as well, but does it really make sense for us to
> > use MRR with the same lowest rate? That's why I just used one segment.
> > Thoughts?
>
> or we can try for max_retry (4) times. In that case the rate indices of
> other rates (just not 1) should be made -1 or this segment should
> moved just below the rate find.

and the next segment [1]
is set to -1. Please let me know if there is anything else you see needs
change.

Setting rate index of the rate series[1] is not enough as you are still filling the others rate
segments(2 and 3) by ath_rc_rate_getidx() in the for..loop, so other segments are also be
set to -1, but it looks hacky, one clean way of doing this can be, moving you code segment to
just below ath_rc_ratefind_ht(), like the following diff.


        rate_table = sc->cur_rate_table;
        rix = ath_rc_ratefind_ht(sc, ath_rc_priv, rate_table, &is_probe);
+
+       if (rix == ath_rc_priv->valid_rate_index[0]) {
+               DPRINTF(sc, ATH_DBG_RATE, "lowest rate being used, "
+                               "disabling MRR\n");
+
+               ath_rc_rate_set_series(rate_table, &rates[0], txrc,
+                                      4, rix, 0);
+               rates[0].idx = rate_lowest_index(sband, sta);
+               goto find_ctrl_rateix;
+       }
+
        nrix = rix;

        if (is_probe) {
@@ -933,6 +944,7 @@ static void ath_rc_ratefind(struct ath_softc *sc,
                rates[0].count = ATH_TXMAXTRY;
        }

+find_ctrl_rateix:
        /* Setup RTS/CTS */
        ath_rc_rate_set_rtscts(sc, rate_table, tx_info);
 }


Vasanth

[ath9k-devel] [PATCH 01/15] ath9k: fix oops by downgrading assert in rc.c

[Vasanth Thiagarajan]

________________________________________

> > >> +       /*
> > >> +        * Fine tuning for when no decent rate was found, the
> > >> +        * lowest should *not* be used under normal circumstances.
> > >> +        */
> > >> +       if (rix == ath_rc_priv->valid_rate_index[0]) {
> > >> +               DPRINTF(sc, ATH_DBG_RATE, "lowest rate being used, "
> > >> +                       "disabling MRR\n");
> > >> +               rates[0].idx = rate_lowest_index(sband, sta);
> > >> +               /* Disable MRR when ath_rc_ratefind_ht() found rate 0 */
> > >> +               rates[1].idx = -1;
> > >> +       }
> > >
> > > I think we can still fill other rates (1..3) with the lowest rate
> > > index as we dont differentiate the situation where the lowest rate
> > > is chosen truely by the algorithm from this particular case.
> >
> > I thought about that as well, but does it really make sense for us to
> > use MRR with the same lowest rate? That's why I just used one segment.
> > Thoughts?
>
> or we can try for max_retry (4) times. In that case the rate indices of
> other rates (just not 1) should be made -1 or this segment should
> moved just below the rate find.

and the next segment [1]
is set to -1. Please let me know if there is anything else you see needs
change.

Setting rate index of the rate series[1] is not enough as you are still filling the others rate
segments(2 and 3) by ath_rc_rate_getidx() in the for..loop, so other segments are also be
set to -1, but it looks hacky, one clean way of doing this can be, moving you code segment to
just below ath_rc_ratefind_ht(), like the following diff.


        rate_table = sc->cur_rate_table;
        rix = ath_rc_ratefind_ht(sc, ath_rc_priv, rate_table, &is_probe);
+
+       if (rix == ath_rc_priv->valid_rate_index[0]) {
+               DPRINTF(sc, ATH_DBG_RATE, "lowest rate being used, "
+                               "disabling MRR\n");
+
+               ath_rc_rate_set_series(rate_table, &rates[0], txrc,
+                                      4, rix, 0);
+               rates[0].idx = rate_lowest_index(sband, sta);
+               goto find_ctrl_rateix;
+       }
+
        nrix = rix;

        if (is_probe) {
@@ -933,6 +944,7 @@ static void ath_rc_ratefind(struct ath_softc *sc,
                rates[0].count = ATH_TXMAXTRY;
        }

+find_ctrl_rateix:
        /* Setup RTS/CTS */
        ath_rc_rate_set_rtscts(sc, rate_table, tx_info);
 }


Vasanth

Re: [PATCH 01/15] ath9k: fix oops by downgrading assert in rc.c

[Luis R. Rodriguez]

On Fri, Jun 5, 2009 at 11:55 PM, Vasanth
Thiagarajan<Vasanth.Thiagarajan@atheros.com> wrote:
>
> ________________________________________
>
>> > >> +       /*
>> > >> +        * Fine tuning for when no decent rate was found, the
>> > >> +        * lowest should *not* be used under normal circumstances.
>> > >> +        */
>> > >> +       if (rix == ath_rc_priv->valid_rate_index[0]) {
>> > >> +               DPRINTF(sc, ATH_DBG_RATE, "lowest rate being used, "
>> > >> +                       "disabling MRR\n");
>> > >> +               rates[0].idx = rate_lowest_index(sband, sta);
>> > >> +               /* Disable MRR when ath_rc_ratefind_ht() found rate 0 */
>> > >> +               rates[1].idx = -1;
>> > >> +       }
>> > >
>> > > I think we can still fill other rates (1..3) with the lowest rate
>> > > index as we dont differentiate the situation where the lowest rate
>> > > is chosen truely by the algorithm from this particular case.
>> >
>> > I thought about that as well, but does it really make sense for us to
>> > use MRR with the same lowest rate? That's why I just used one segment.
>> > Thoughts?
>>
>> or we can try for max_retry (4) times. In that case the rate indices of
>> other rates (just not 1) should be made -1 or this segment should
>> moved just below the rate find.
>
> and the next segment [1]
> is set to -1. Please let me know if there is anything else you see needs
> change.
>
> Setting rate index of the rate series[1] is not enough as you are still filling the others rate
> segments(2 and 3) by ath_rc_rate_getidx() in the for..loop, so other segments are also be
> set to -1, but it looks hacky, one clean way of doing this can be, moving you code segment to
> just below ath_rc_ratefind_ht(), like the following diff.
>
>
>        rate_table = sc->cur_rate_table;
>        rix = ath_rc_ratefind_ht(sc, ath_rc_priv, rate_table, &is_probe);
> +
> +       if (rix == ath_rc_priv->valid_rate_index[0]) {
> +               DPRINTF(sc, ATH_DBG_RATE, "lowest rate being used, "
> +                               "disabling MRR\n");
> +
> +               ath_rc_rate_set_series(rate_table, &rates[0], txrc,
> +                                      4, rix, 0);

The above sets the rate[0].idx to rix

> +               rates[0].idx = rate_lowest_index(sband, sta);

and then here we set it to rate_lowest_index(sband, sta) comes up
with. They should be the same, but this just goes to show we need to
clean this better.

ath_rc_rate_set_series() is also doing some flag checks which I'm not
so sure we need to do all the time so I'd like to avoid it. And
there's also that is_probe check and the flags that sets.

> +               goto find_ctrl_rateix;

This seems like a good idea but in fact I also think this is useless,
not sure why mac80211 can't figure this out for us. More cleanup.

> +       }
> +
>        nrix = rix;
>
>        if (is_probe) {
> @@ -933,6 +944,7 @@ static void ath_rc_ratefind(struct ath_softc *sc,
>                rates[0].count = ATH_TXMAXTRY;
>        }
>
> +find_ctrl_rateix:
>        /* Setup RTS/CTS */
>        ath_rc_rate_set_rtscts(sc, rate_table, tx_info);
>  }

I'll respin again.

  Luis

[ath9k-devel] [PATCH 01/15] ath9k: fix oops by downgrading assert in rc.c

[Luis R. Rodriguez]

On Fri, Jun 5, 2009 at 11:55 PM, Vasanth
Thiagarajan<Vasanth.Thiagarajan@atheros.com> wrote:
>
> ________________________________________
>
>> > >> + ? ? ? /*
>> > >> + ? ? ? ?* Fine tuning for when no decent rate was found, the
>> > >> + ? ? ? ?* lowest should *not* be used under normal circumstances.
>> > >> + ? ? ? ?*/
>> > >> + ? ? ? if (rix == ath_rc_priv->valid_rate_index[0]) {
>> > >> + ? ? ? ? ? ? ? DPRINTF(sc, ATH_DBG_RATE, "lowest rate being used, "
>> > >> + ? ? ? ? ? ? ? ? ? ? ? "disabling MRR\n");
>> > >> + ? ? ? ? ? ? ? rates[0].idx = rate_lowest_index(sband, sta);
>> > >> + ? ? ? ? ? ? ? /* Disable MRR when ath_rc_ratefind_ht() found rate 0 */
>> > >> + ? ? ? ? ? ? ? rates[1].idx = -1;
>> > >> + ? ? ? }
>> > >
>> > > I think we can still fill other rates (1..3) with the lowest rate
>> > > index as we dont differentiate the situation where the lowest rate
>> > > is chosen truely by the algorithm from this particular case.
>> >
>> > I thought about that as well, but does it really make sense for us to
>> > use MRR with the same lowest rate? That's why I just used one segment.
>> > Thoughts?
>>
>> or we can try for max_retry (4) times. In that case the rate indices of
>> other rates (just not 1) should be made -1 or this segment should
>> moved just below the rate find.
>
> and the next segment [1]
> is set to -1. Please let me know if there is anything else you see needs
> change.
>
> Setting rate index of the rate series[1] is not enough as you are still filling the others rate
> segments(2 and 3) by ath_rc_rate_getidx() in the for..loop, so other segments are also be
> set to -1, but it looks hacky, one clean way of doing this can be, moving you code segment to
> just below ath_rc_ratefind_ht(), like the following diff.
>
>
> ? ? ? ?rate_table = sc->cur_rate_table;
> ? ? ? ?rix = ath_rc_ratefind_ht(sc, ath_rc_priv, rate_table, &is_probe);
> +
> + ? ? ? if (rix == ath_rc_priv->valid_rate_index[0]) {
> + ? ? ? ? ? ? ? DPRINTF(sc, ATH_DBG_RATE, "lowest rate being used, "
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? "disabling MRR\n");
> +
> + ? ? ? ? ? ? ? ath_rc_rate_set_series(rate_table, &rates[0], txrc,
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?4, rix, 0);

The above sets the rate[0].idx to rix

> + ? ? ? ? ? ? ? rates[0].idx = rate_lowest_index(sband, sta);

and then here we set it to rate_lowest_index(sband, sta) comes up
with. They should be the same, but this just goes to show we need to
clean this better.

ath_rc_rate_set_series() is also doing some flag checks which I'm not
so sure we need to do all the time so I'd like to avoid it. And
there's also that is_probe check and the flags that sets.

> + ? ? ? ? ? ? ? goto find_ctrl_rateix;

This seems like a good idea but in fact I also think this is useless,
not sure why mac80211 can't figure this out for us. More cleanup.

> + ? ? ? }
> +
> ? ? ? ?nrix = rix;
>
> ? ? ? ?if (is_probe) {
> @@ -933,6 +944,7 @@ static void ath_rc_ratefind(struct ath_softc *sc,
> ? ? ? ? ? ? ? ?rates[0].count = ATH_TXMAXTRY;
> ? ? ? ?}
>
> +find_ctrl_rateix:
> ? ? ? ?/* Setup RTS/CTS */
> ? ? ? ?ath_rc_rate_set_rtscts(sc, rate_table, tx_info);
> ?}

I'll respin again.

  Luis

Re: [PATCH 01/15] ath9k: fix oops by downgrading assert in rc.c

[Luis R. Rodriguez]

On Mon, Jun 08, 2009 at 05:26:41PM -0700, Luis R. Rodriguez wrote:
> On Fri, Jun 5, 2009 at 11:55 PM, Vasanth
> Thiagarajan<Vasanth.Thiagarajan@atheros.com> wrote:
> >
> > ________________________________________
> >
> >> > >> +       /*
> >> > >> +        * Fine tuning for when no decent rate was found, the
> >> > >> +        * lowest should *not* be used under normal circumstances.
> >> > >> +        */
> >> > >> +       if (rix == ath_rc_priv->valid_rate_index[0]) {
> >> > >> +               DPRINTF(sc, ATH_DBG_RATE, "lowest rate being used, "
> >> > >> +                       "disabling MRR\n");
> >> > >> +               rates[0].idx = rate_lowest_index(sband, sta);
> >> > >> +               /* Disable MRR when ath_rc_ratefind_ht() found rate 0 */
> >> > >> +               rates[1].idx = -1;
> >> > >> +       }
> >> > >
> >> > > I think we can still fill other rates (1..3) with the lowest rate
> >> > > index as we dont differentiate the situation where the lowest rate
> >> > > is chosen truely by the algorithm from this particular case.
> >> >
> >> > I thought about that as well, but does it really make sense for us to
> >> > use MRR with the same lowest rate? That's why I just used one segment.
> >> > Thoughts?
> >>
> >> or we can try for max_retry (4) times. In that case the rate indices of
> >> other rates (just not 1) should be made -1 or this segment should
> >> moved just below the rate find.
> >
> > and the next segment [1]
> > is set to -1. Please let me know if there is anything else you see needs
> > change.
> >
> > Setting rate index of the rate series[1] is not enough as you are still filling the others rate
> > segments(2 and 3) by ath_rc_rate_getidx() in the for..loop, so other segments are also be
> > set to -1, but it looks hacky, one clean way of doing this can be, moving you code segment to
> > just below ath_rc_ratefind_ht(), like the following diff.
> >
> >
> >        rate_table = sc->cur_rate_table;
> >        rix = ath_rc_ratefind_ht(sc, ath_rc_priv, rate_table, &is_probe);
> > +
> > +       if (rix == ath_rc_priv->valid_rate_index[0]) {
> > +               DPRINTF(sc, ATH_DBG_RATE, "lowest rate being used, "
> > +                               "disabling MRR\n");
> > +
> > +               ath_rc_rate_set_series(rate_table, &rates[0], txrc,
> > +                                      4, rix, 0);
> 
> The above sets the rate[0].idx to rix
> 
> > +               rates[0].idx = rate_lowest_index(sband, sta);
> 
> and then here we set it to rate_lowest_index(sband, sta) comes up
> with. They should be the same, but this just goes to show we need to
> clean this better.

I just found this is wrong too.. the rate table used to find
rix is different than the rate table for the mode. ie, in 5ghz
when associated to an 11n station this could not include any
legacy rates.

  Luis

[ath9k-devel] [PATCH 01/15] ath9k: fix oops by downgrading assert in rc.c

[Luis R. Rodriguez]

On Mon, Jun 08, 2009 at 05:26:41PM -0700, Luis R. Rodriguez wrote:
> On Fri, Jun 5, 2009 at 11:55 PM, Vasanth
> Thiagarajan<Vasanth.Thiagarajan@atheros.com> wrote:
> >
> > ________________________________________
> >
> >> > >> +       /*
> >> > >> +        * Fine tuning for when no decent rate was found, the
> >> > >> +        * lowest should *not* be used under normal circumstances.
> >> > >> +        */
> >> > >> +       if (rix == ath_rc_priv->valid_rate_index[0]) {
> >> > >> +               DPRINTF(sc, ATH_DBG_RATE, "lowest rate being used, "
> >> > >> +                       "disabling MRR\n");
> >> > >> +               rates[0].idx = rate_lowest_index(sband, sta);
> >> > >> +               /* Disable MRR when ath_rc_ratefind_ht() found rate 0 */
> >> > >> +               rates[1].idx = -1;
> >> > >> +       }
> >> > >
> >> > > I think we can still fill other rates (1..3) with the lowest rate
> >> > > index as we dont differentiate the situation where the lowest rate
> >> > > is chosen truely by the algorithm from this particular case.
> >> >
> >> > I thought about that as well, but does it really make sense for us to
> >> > use MRR with the same lowest rate? That's why I just used one segment.
> >> > Thoughts?
> >>
> >> or we can try for max_retry (4) times. In that case the rate indices of
> >> other rates (just not 1) should be made -1 or this segment should
> >> moved just below the rate find.
> >
> > and the next segment [1]
> > is set to -1. Please let me know if there is anything else you see needs
> > change.
> >
> > Setting rate index of the rate series[1] is not enough as you are still filling the others rate
> > segments(2 and 3) by ath_rc_rate_getidx() in the for..loop, so other segments are also be
> > set to -1, but it looks hacky, one clean way of doing this can be, moving you code segment to
> > just below ath_rc_ratefind_ht(), like the following diff.
> >
> >
> >        rate_table = sc->cur_rate_table;
> >        rix = ath_rc_ratefind_ht(sc, ath_rc_priv, rate_table, &is_probe);
> > +
> > +       if (rix == ath_rc_priv->valid_rate_index[0]) {
> > +               DPRINTF(sc, ATH_DBG_RATE, "lowest rate being used, "
> > +                               "disabling MRR\n");
> > +
> > +               ath_rc_rate_set_series(rate_table, &rates[0], txrc,
> > +                                      4, rix, 0);
> 
> The above sets the rate[0].idx to rix
> 
> > +               rates[0].idx = rate_lowest_index(sband, sta);
> 
> and then here we set it to rate_lowest_index(sband, sta) comes up
> with. They should be the same, but this just goes to show we need to
> clean this better.

I just found this is wrong too.. the rate table used to find
rix is different than the rate table for the mode. ie, in 5ghz
when associated to an 11n station this could not include any
legacy rates.

  Luis

Re: [PATCH 01/15] ath9k: fix oops by downgrading assert in rc.c

[Vasanthakumar Thiagarajan]

On Tue, Jun 09, 2009 at 05:56:41AM +0530, Luis R. Rodriguez wrote:
> On Fri, Jun 5, 2009 at 11:55 PM, Vasanth
> Thiagarajan<Vasanth.Thiagarajan@atheros.com> wrote:
> >
> > ________________________________________
> >
> >> > >> +       /*
> >> > >> +        * Fine tuning for when no decent rate was found, the
> >> > >> +        * lowest should *not* be used under normal circumstances.
> >> > >> +        */
> >> > >> +       if (rix == ath_rc_priv->valid_rate_index[0]) {
> >> > >> +               DPRINTF(sc, ATH_DBG_RATE, "lowest rate being used, "
> >> > >> +                       "disabling MRR\n");
> >> > >> +               rates[0].idx = rate_lowest_index(sband, sta);
> >> > >> +               /* Disable MRR when ath_rc_ratefind_ht() found rate 0 */
> >> > >> +               rates[1].idx = -1;
> >> > >> +       }
> >> > >
> >> > > I think we can still fill other rates (1..3) with the lowest rate
> >> > > index as we dont differentiate the situation where the lowest rate
> >> > > is chosen truely by the algorithm from this particular case.
> >> >
> >> > I thought about that as well, but does it really make sense for us to
> >> > use MRR with the same lowest rate? That's why I just used one segment.
> >> > Thoughts?
> >>
> >> or we can try for max_retry (4) times. In that case the rate indices of
> >> other rates (just not 1) should be made -1 or this segment should
> >> moved just below the rate find.
> >
> > and the next segment [1]
> > is set to -1. Please let me know if there is anything else you see needs
> > change.
> >
> > Setting rate index of the rate series[1] is not enough as you are still filling the others rate
> > segments(2 and 3) by ath_rc_rate_getidx() in the for..loop, so other segments are also be
> > set to -1, but it looks hacky, one clean way of doing this can be, moving you code segment to
> > just below ath_rc_ratefind_ht(), like the following diff.
> >
> >
> >        rate_table = sc->cur_rate_table;
> >        rix = ath_rc_ratefind_ht(sc, ath_rc_priv, rate_table, &is_probe);
> > +
> > +       if (rix == ath_rc_priv->valid_rate_index[0]) {
> > +               DPRINTF(sc, ATH_DBG_RATE, "lowest rate being used, "
> > +                               "disabling MRR\n");
> > +
> > +               ath_rc_rate_set_series(rate_table, &rates[0], txrc,
> > +                                      4, rix, 0);
> 
> The above sets the rate[0].idx to rix
> 
> > +               rates[0].idx = rate_lowest_index(sband, sta);
> 
> and then here we set it to rate_lowest_index(sband, sta) comes up
> with. They should be the same, but this just goes to show we need to
> clean this better.

yeah, set rix to lowest rate index before passing rix to
ath_rc_rate_set_series().
> 
> ath_rc_rate_set_series() is also doing some flag checks which I'm not
> so sure we need to do all the time so I'd like to avoid it.

We need preamble and RTS_CTS bits in rate flags.

> there's also that is_probe check and the flags that sets.

This should be ignored for lowest rate index as there is no way
that a new rate has been probed in this case.


Vasanth

[ath9k-devel] [PATCH 01/15] ath9k: fix oops by downgrading assert in rc.c

[Vasanthakumar Thiagarajan]

On Tue, Jun 09, 2009 at 05:56:41AM +0530, Luis R. Rodriguez wrote:
> On Fri, Jun 5, 2009 at 11:55 PM, Vasanth
> Thiagarajan<Vasanth.Thiagarajan@atheros.com> wrote:
> >
> > ________________________________________
> >
> >> > >> +       /*
> >> > >> +        * Fine tuning for when no decent rate was found, the
> >> > >> +        * lowest should *not* be used under normal circumstances.
> >> > >> +        */
> >> > >> +       if (rix == ath_rc_priv->valid_rate_index[0]) {
> >> > >> +               DPRINTF(sc, ATH_DBG_RATE, "lowest rate being used, "
> >> > >> +                       "disabling MRR\n");
> >> > >> +               rates[0].idx = rate_lowest_index(sband, sta);
> >> > >> +               /* Disable MRR when ath_rc_ratefind_ht() found rate 0 */
> >> > >> +               rates[1].idx = -1;
> >> > >> +       }
> >> > >
> >> > > I think we can still fill other rates (1..3) with the lowest rate
> >> > > index as we dont differentiate the situation where the lowest rate
> >> > > is chosen truely by the algorithm from this particular case.
> >> >
> >> > I thought about that as well, but does it really make sense for us to
> >> > use MRR with the same lowest rate? That's why I just used one segment.
> >> > Thoughts?
> >>
> >> or we can try for max_retry (4) times. In that case the rate indices of
> >> other rates (just not 1) should be made -1 or this segment should
> >> moved just below the rate find.
> >
> > and the next segment [1]
> > is set to -1. Please let me know if there is anything else you see needs
> > change.
> >
> > Setting rate index of the rate series[1] is not enough as you are still filling the others rate
> > segments(2 and 3) by ath_rc_rate_getidx() in the for..loop, so other segments are also be
> > set to -1, but it looks hacky, one clean way of doing this can be, moving you code segment to
> > just below ath_rc_ratefind_ht(), like the following diff.
> >
> >
> >        rate_table = sc->cur_rate_table;
> >        rix = ath_rc_ratefind_ht(sc, ath_rc_priv, rate_table, &is_probe);
> > +
> > +       if (rix == ath_rc_priv->valid_rate_index[0]) {
> > +               DPRINTF(sc, ATH_DBG_RATE, "lowest rate being used, "
> > +                               "disabling MRR\n");
> > +
> > +               ath_rc_rate_set_series(rate_table, &rates[0], txrc,
> > +                                      4, rix, 0);
> 
> The above sets the rate[0].idx to rix
> 
> > +               rates[0].idx = rate_lowest_index(sband, sta);
> 
> and then here we set it to rate_lowest_index(sband, sta) comes up
> with. They should be the same, but this just goes to show we need to
> clean this better.

yeah, set rix to lowest rate index before passing rix to
ath_rc_rate_set_series().
> 
> ath_rc_rate_set_series() is also doing some flag checks which I'm not
> so sure we need to do all the time so I'd like to avoid it.

We need preamble and RTS_CTS bits in rate flags.

> there's also that is_probe check and the flags that sets.

This should be ignored for lowest rate index as there is no way
that a new rate has been probed in this case.


Vasanth

Re: [PATCH 01/15] ath9k: fix oops by downgrading assert in rc.c

[Vasanthakumar Thiagarajan]

On Tue, Jun 09, 2009 at 06:28:21AM +0530, Luis Rodriguez wrote:
> On Mon, Jun 08, 2009 at 05:26:41PM -0700, Luis R. Rodriguez wrote:
> > On Fri, Jun 5, 2009 at 11:55 PM, Vasanth
> > Thiagarajan<Vasanth.Thiagarajan@atheros.com> wrote:
> > >
> > > ________________________________________
> > >
> > >> > >> +       /*
> > >> > >> +        * Fine tuning for when no decent rate was found, the
> > >> > >> +        * lowest should *not* be used under normal circumstances.
> > >> > >> +        */
> > >> > >> +       if (rix == ath_rc_priv->valid_rate_index[0]) {
> > >> > >> +               DPRINTF(sc, ATH_DBG_RATE, "lowest rate being used, "
> > >> > >> +                       "disabling MRR\n");
> > >> > >> +               rates[0].idx = rate_lowest_index(sband, sta);
> > >> > >> +               /* Disable MRR when ath_rc_ratefind_ht() found rate 0 */
> > >> > >> +               rates[1].idx = -1;
> > >> > >> +       }
> > >> > >
> > >> > > I think we can still fill other rates (1..3) with the lowest rate
> > >> > > index as we dont differentiate the situation where the lowest rate
> > >> > > is chosen truely by the algorithm from this particular case.
> > >> >
> > >> > I thought about that as well, but does it really make sense for us to
> > >> > use MRR with the same lowest rate? That's why I just used one segment.
> > >> > Thoughts?
> > >>
> > >> or we can try for max_retry (4) times. In that case the rate indices of
> > >> other rates (just not 1) should be made -1 or this segment should
> > >> moved just below the rate find.
> > >
> > > and the next segment [1]
> > > is set to -1. Please let me know if there is anything else you see needs
> > > change.
> > >
> > > Setting rate index of the rate series[1] is not enough as you are still filling the others rate
> > > segments(2 and 3) by ath_rc_rate_getidx() in the for..loop, so other segments are also be
> > > set to -1, but it looks hacky, one clean way of doing this can be, moving you code segment to
> > > just below ath_rc_ratefind_ht(), like the following diff.
> > >
> > >
> > >        rate_table = sc->cur_rate_table;
> > >        rix = ath_rc_ratefind_ht(sc, ath_rc_priv, rate_table, &is_probe);
> > > +
> > > +       if (rix == ath_rc_priv->valid_rate_index[0]) {
> > > +               DPRINTF(sc, ATH_DBG_RATE, "lowest rate being used, "
> > > +                               "disabling MRR\n");
> > > +
> > > +               ath_rc_rate_set_series(rate_table, &rates[0], txrc,
> > > +                                      4, rix, 0);
> >
> > The above sets the rate[0].idx to rix
> >
> > > +               rates[0].idx = rate_lowest_index(sband, sta);
> >
> > and then here we set it to rate_lowest_index(sband, sta) comes up
> > with. They should be the same, but this just goes to show we need to
> > clean this better.
> 
> I just found this is wrong too.. the rate table used to find
> rix is different than the rate table for the mode. ie, in 5ghz
> when associated to an 11n station this could not include any
> legacy rates.

This is wrong. All basci rates still be used in 11n mode.

Vasanth

[ath9k-devel] [PATCH 01/15] ath9k: fix oops by downgrading assert in rc.c

[Vasanthakumar Thiagarajan]

On Tue, Jun 09, 2009 at 06:28:21AM +0530, Luis Rodriguez wrote:
> On Mon, Jun 08, 2009 at 05:26:41PM -0700, Luis R. Rodriguez wrote:
> > On Fri, Jun 5, 2009 at 11:55 PM, Vasanth
> > Thiagarajan<Vasanth.Thiagarajan@atheros.com> wrote:
> > >
> > > ________________________________________
> > >
> > >> > >> +       /*
> > >> > >> +        * Fine tuning for when no decent rate was found, the
> > >> > >> +        * lowest should *not* be used under normal circumstances.
> > >> > >> +        */
> > >> > >> +       if (rix == ath_rc_priv->valid_rate_index[0]) {
> > >> > >> +               DPRINTF(sc, ATH_DBG_RATE, "lowest rate being used, "
> > >> > >> +                       "disabling MRR\n");
> > >> > >> +               rates[0].idx = rate_lowest_index(sband, sta);
> > >> > >> +               /* Disable MRR when ath_rc_ratefind_ht() found rate 0 */
> > >> > >> +               rates[1].idx = -1;
> > >> > >> +       }
> > >> > >
> > >> > > I think we can still fill other rates (1..3) with the lowest rate
> > >> > > index as we dont differentiate the situation where the lowest rate
> > >> > > is chosen truely by the algorithm from this particular case.
> > >> >
> > >> > I thought about that as well, but does it really make sense for us to
> > >> > use MRR with the same lowest rate? That's why I just used one segment.
> > >> > Thoughts?
> > >>
> > >> or we can try for max_retry (4) times. In that case the rate indices of
> > >> other rates (just not 1) should be made -1 or this segment should
> > >> moved just below the rate find.
> > >
> > > and the next segment [1]
> > > is set to -1. Please let me know if there is anything else you see needs
> > > change.
> > >
> > > Setting rate index of the rate series[1] is not enough as you are still filling the others rate
> > > segments(2 and 3) by ath_rc_rate_getidx() in the for..loop, so other segments are also be
> > > set to -1, but it looks hacky, one clean way of doing this can be, moving you code segment to
> > > just below ath_rc_ratefind_ht(), like the following diff.
> > >
> > >
> > >        rate_table = sc->cur_rate_table;
> > >        rix = ath_rc_ratefind_ht(sc, ath_rc_priv, rate_table, &is_probe);
> > > +
> > > +       if (rix == ath_rc_priv->valid_rate_index[0]) {
> > > +               DPRINTF(sc, ATH_DBG_RATE, "lowest rate being used, "
> > > +                               "disabling MRR\n");
> > > +
> > > +               ath_rc_rate_set_series(rate_table, &rates[0], txrc,
> > > +                                      4, rix, 0);
> >
> > The above sets the rate[0].idx to rix
> >
> > > +               rates[0].idx = rate_lowest_index(sband, sta);
> >
> > and then here we set it to rate_lowest_index(sband, sta) comes up
> > with. They should be the same, but this just goes to show we need to
> > clean this better.
> 
> I just found this is wrong too.. the rate table used to find
> rix is different than the rate table for the mode. ie, in 5ghz
> when associated to an 11n station this could not include any
> legacy rates.

This is wrong. All basci rates still be used in 11n mode.

Vasanth

Re: [PATCH 01/15] ath9k: fix oops by downgrading assert in rc.c

[Vasanthakumar Thiagarajan]

On Tue, Jun 09, 2009 at 10:54:00AM +0530, Vasanth Thiagarajan wrote:
> On Tue, Jun 09, 2009 at 06:28:21AM +0530, Luis Rodriguez wrote:
> > On Mon, Jun 08, 2009 at 05:26:41PM -0700, Luis R. Rodriguez wrote:
> > > On Fri, Jun 5, 2009 at 11:55 PM, Vasanth
> > > Thiagarajan<Vasanth.Thiagarajan@atheros.com> wrote:
> > > >
> > > > ________________________________________
> > > >
> > > >> > >> +       /*
> > > >> > >> +        * Fine tuning for when no decent rate was found, the
> > > >> > >> +        * lowest should *not* be used under normal circumstances.
> > > >> > >> +        */
> > > >> > >> +       if (rix == ath_rc_priv->valid_rate_index[0]) {
> > > >> > >> +               DPRINTF(sc, ATH_DBG_RATE, "lowest rate being used, "
> > > >> > >> +                       "disabling MRR\n");
> > > >> > >> +               rates[0].idx = rate_lowest_index(sband, sta);
> > > >> > >> +               /* Disable MRR when ath_rc_ratefind_ht() found rate 0 */
> > > >> > >> +               rates[1].idx = -1;
> > > >> > >> +       }
> > > >> > >
> > > >> > > I think we can still fill other rates (1..3) with the lowest rate
> > > >> > > index as we dont differentiate the situation where the lowest rate
> > > >> > > is chosen truely by the algorithm from this particular case.
> > > >> >
> > > >> > I thought about that as well, but does it really make sense for us to
> > > >> > use MRR with the same lowest rate? That's why I just used one segment.
> > > >> > Thoughts?
> > > >>
> > > >> or we can try for max_retry (4) times. In that case the rate indices of
> > > >> other rates (just not 1) should be made -1 or this segment should
> > > >> moved just below the rate find.
> > > >
> > > > and the next segment [1]
> > > > is set to -1. Please let me know if there is anything else you see needs
> > > > change.
> > > >
> > > > Setting rate index of the rate series[1] is not enough as you are still filling the others rate
> > > > segments(2 and 3) by ath_rc_rate_getidx() in the for..loop, so other segments are also be
> > > > set to -1, but it looks hacky, one clean way of doing this can be, moving you code segment to
> > > > just below ath_rc_ratefind_ht(), like the following diff.
> > > >
> > > >
> > > >        rate_table = sc->cur_rate_table;
> > > >        rix = ath_rc_ratefind_ht(sc, ath_rc_priv, rate_table, &is_probe);
> > > > +
> > > > +       if (rix == ath_rc_priv->valid_rate_index[0]) {
> > > > +               DPRINTF(sc, ATH_DBG_RATE, "lowest rate being used, "
> > > > +                               "disabling MRR\n");
> > > > +
> > > > +               ath_rc_rate_set_series(rate_table, &rates[0], txrc,
> > > > +                                      4, rix, 0);
> > >
> > > The above sets the rate[0].idx to rix
> > >
> > > > +               rates[0].idx = rate_lowest_index(sband, sta);
> > >
> > > and then here we set it to rate_lowest_index(sband, sta) comes up
> > > with. They should be the same, but this just goes to show we need to
> > > clean this better.
> >
> > I just found this is wrong too.. the rate table used to find
> > rix is different than the rate table for the mode. ie, in 5ghz
> > when associated to an 11n station this could not include any
> > legacy rates.
> 
> This is wrong. All basci rates still be used in 11n mode.

Never mind, i think this is only true in 11ng mode

[ath9k-devel] [PATCH 01/15] ath9k: fix oops by downgrading assert in rc.c

[Vasanthakumar Thiagarajan]

On Tue, Jun 09, 2009 at 10:54:00AM +0530, Vasanth Thiagarajan wrote:
> On Tue, Jun 09, 2009 at 06:28:21AM +0530, Luis Rodriguez wrote:
> > On Mon, Jun 08, 2009 at 05:26:41PM -0700, Luis R. Rodriguez wrote:
> > > On Fri, Jun 5, 2009 at 11:55 PM, Vasanth
> > > Thiagarajan<Vasanth.Thiagarajan@atheros.com> wrote:
> > > >
> > > > ________________________________________
> > > >
> > > >> > >> +       /*
> > > >> > >> +        * Fine tuning for when no decent rate was found, the
> > > >> > >> +        * lowest should *not* be used under normal circumstances.
> > > >> > >> +        */
> > > >> > >> +       if (rix == ath_rc_priv->valid_rate_index[0]) {
> > > >> > >> +               DPRINTF(sc, ATH_DBG_RATE, "lowest rate being used, "
> > > >> > >> +                       "disabling MRR\n");
> > > >> > >> +               rates[0].idx = rate_lowest_index(sband, sta);
> > > >> > >> +               /* Disable MRR when ath_rc_ratefind_ht() found rate 0 */
> > > >> > >> +               rates[1].idx = -1;
> > > >> > >> +       }
> > > >> > >
> > > >> > > I think we can still fill other rates (1..3) with the lowest rate
> > > >> > > index as we dont differentiate the situation where the lowest rate
> > > >> > > is chosen truely by the algorithm from this particular case.
> > > >> >
> > > >> > I thought about that as well, but does it really make sense for us to
> > > >> > use MRR with the same lowest rate? That's why I just used one segment.
> > > >> > Thoughts?
> > > >>
> > > >> or we can try for max_retry (4) times. In that case the rate indices of
> > > >> other rates (just not 1) should be made -1 or this segment should
> > > >> moved just below the rate find.
> > > >
> > > > and the next segment [1]
> > > > is set to -1. Please let me know if there is anything else you see needs
> > > > change.
> > > >
> > > > Setting rate index of the rate series[1] is not enough as you are still filling the others rate
> > > > segments(2 and 3) by ath_rc_rate_getidx() in the for..loop, so other segments are also be
> > > > set to -1, but it looks hacky, one clean way of doing this can be, moving you code segment to
> > > > just below ath_rc_ratefind_ht(), like the following diff.
> > > >
> > > >
> > > >        rate_table = sc->cur_rate_table;
> > > >        rix = ath_rc_ratefind_ht(sc, ath_rc_priv, rate_table, &is_probe);
> > > > +
> > > > +       if (rix == ath_rc_priv->valid_rate_index[0]) {
> > > > +               DPRINTF(sc, ATH_DBG_RATE, "lowest rate being used, "
> > > > +                               "disabling MRR\n");
> > > > +
> > > > +               ath_rc_rate_set_series(rate_table, &rates[0], txrc,
> > > > +                                      4, rix, 0);
> > >
> > > The above sets the rate[0].idx to rix
> > >
> > > > +               rates[0].idx = rate_lowest_index(sband, sta);
> > >
> > > and then here we set it to rate_lowest_index(sband, sta) comes up
> > > with. They should be the same, but this just goes to show we need to
> > > clean this better.
> >
> > I just found this is wrong too.. the rate table used to find
> > rix is different than the rate table for the mode. ie, in 5ghz
> > when associated to an 11n station this could not include any
> > legacy rates.
> 
> This is wrong. All basci rates still be used in 11n mode.

Never mind, i think this is only true in 11ng mode

Re: [PATCH 01/15] ath9k: fix oops by downgrading assert in rc.c

[Vasanthakumar Thiagarajan]

On Tue, Jun 09, 2009 at 10:54:00AM +0530, Vasanth Thiagarajan wrote:
> On Tue, Jun 09, 2009 at 06:28:21AM +0530, Luis Rodriguez wrote:
> > On Mon, Jun 08, 2009 at 05:26:41PM -0700, Luis R. Rodriguez wrote:
> > > On Fri, Jun 5, 2009 at 11:55 PM, Vasanth
> > > Thiagarajan<Vasanth.Thiagarajan@atheros.com> wrote:
> > > >
> > > > ________________________________________
> > > >
> > > >> > >> +       /*
> > > >> > >> +        * Fine tuning for when no decent rate was found, the
> > > >> > >> +        * lowest should *not* be used under normal circumstances.
> > > >> > >> +        */
> > > >> > >> +       if (rix == ath_rc_priv->valid_rate_index[0]) {
> > > >> > >> +               DPRINTF(sc, ATH_DBG_RATE, "lowest rate being used, "
> > > >> > >> +                       "disabling MRR\n");
> > > >> > >> +               rates[0].idx = rate_lowest_index(sband, sta);
> > > >> > >> +               /* Disable MRR when ath_rc_ratefind_ht() found rate 0 */
> > > >> > >> +               rates[1].idx = -1;
> > > >> > >> +       }
> > > >> > >
> > > >> > > I think we can still fill other rates (1..3) with the lowest rate
> > > >> > > index as we dont differentiate the situation where the lowest rate
> > > >> > > is chosen truely by the algorithm from this particular case.
> > > >> >
> > > >> > I thought about that as well, but does it really make sense for us to
> > > >> > use MRR with the same lowest rate? That's why I just used one segment.
> > > >> > Thoughts?
> > > >>
> > > >> or we can try for max_retry (4) times. In that case the rate indices of
> > > >> other rates (just not 1) should be made -1 or this segment should
> > > >> moved just below the rate find.
> > > >
> > > > and the next segment [1]
> > > > is set to -1. Please let me know if there is anything else you see needs
> > > > change.
> > > >
> > > > Setting rate index of the rate series[1] is not enough as you are still filling the others rate
> > > > segments(2 and 3) by ath_rc_rate_getidx() in the for..loop, so other segments are also be
> > > > set to -1, but it looks hacky, one clean way of doing this can be, moving you code segment to
> > > > just below ath_rc_ratefind_ht(), like the following diff.
> > > >
> > > >
> > > >        rate_table = sc->cur_rate_table;
> > > >        rix = ath_rc_ratefind_ht(sc, ath_rc_priv, rate_table, &is_probe);
> > > > +
> > > > +       if (rix == ath_rc_priv->valid_rate_index[0]) {
> > > > +               DPRINTF(sc, ATH_DBG_RATE, "lowest rate being used, "
> > > > +                               "disabling MRR\n");
> > > > +
> > > > +               ath_rc_rate_set_series(rate_table, &rates[0], txrc,
> > > > +                                      4, rix, 0);
> > >
> > > The above sets the rate[0].idx to rix
> > >
> > > > +               rates[0].idx = rate_lowest_index(sband, sta);
> > >
> > > and then here we set it to rate_lowest_index(sband, sta) comes up
> > > with. They should be the same, but this just goes to show we need to
> > > clean this better.
> >
> > I just found this is wrong too.. the rate table used to find
> > rix is different than the rate table for the mode. ie, in 5ghz
> > when associated to an 11n station this could not include any
> > legacy rates.
> 
> This is wrong. All basci rates still be used in 11n mode.

Never mind, this is only for 11ng.
> 
> Vasanth

[ath9k-devel] [PATCH 01/15] ath9k: fix oops by downgrading assert in rc.c

[Vasanthakumar Thiagarajan]

On Tue, Jun 09, 2009 at 10:54:00AM +0530, Vasanth Thiagarajan wrote:
> On Tue, Jun 09, 2009 at 06:28:21AM +0530, Luis Rodriguez wrote:
> > On Mon, Jun 08, 2009 at 05:26:41PM -0700, Luis R. Rodriguez wrote:
> > > On Fri, Jun 5, 2009 at 11:55 PM, Vasanth
> > > Thiagarajan<Vasanth.Thiagarajan@atheros.com> wrote:
> > > >
> > > > ________________________________________
> > > >
> > > >> > >> +       /*
> > > >> > >> +        * Fine tuning for when no decent rate was found, the
> > > >> > >> +        * lowest should *not* be used under normal circumstances.
> > > >> > >> +        */
> > > >> > >> +       if (rix == ath_rc_priv->valid_rate_index[0]) {
> > > >> > >> +               DPRINTF(sc, ATH_DBG_RATE, "lowest rate being used, "
> > > >> > >> +                       "disabling MRR\n");
> > > >> > >> +               rates[0].idx = rate_lowest_index(sband, sta);
> > > >> > >> +               /* Disable MRR when ath_rc_ratefind_ht() found rate 0 */
> > > >> > >> +               rates[1].idx = -1;
> > > >> > >> +       }
> > > >> > >
> > > >> > > I think we can still fill other rates (1..3) with the lowest rate
> > > >> > > index as we dont differentiate the situation where the lowest rate
> > > >> > > is chosen truely by the algorithm from this particular case.
> > > >> >
> > > >> > I thought about that as well, but does it really make sense for us to
> > > >> > use MRR with the same lowest rate? That's why I just used one segment.
> > > >> > Thoughts?
> > > >>
> > > >> or we can try for max_retry (4) times. In that case the rate indices of
> > > >> other rates (just not 1) should be made -1 or this segment should
> > > >> moved just below the rate find.
> > > >
> > > > and the next segment [1]
> > > > is set to -1. Please let me know if there is anything else you see needs
> > > > change.
> > > >
> > > > Setting rate index of the rate series[1] is not enough as you are still filling the others rate
> > > > segments(2 and 3) by ath_rc_rate_getidx() in the for..loop, so other segments are also be
> > > > set to -1, but it looks hacky, one clean way of doing this can be, moving you code segment to
> > > > just below ath_rc_ratefind_ht(), like the following diff.
> > > >
> > > >
> > > >        rate_table = sc->cur_rate_table;
> > > >        rix = ath_rc_ratefind_ht(sc, ath_rc_priv, rate_table, &is_probe);
> > > > +
> > > > +       if (rix == ath_rc_priv->valid_rate_index[0]) {
> > > > +               DPRINTF(sc, ATH_DBG_RATE, "lowest rate being used, "
> > > > +                               "disabling MRR\n");
> > > > +
> > > > +               ath_rc_rate_set_series(rate_table, &rates[0], txrc,
> > > > +                                      4, rix, 0);
> > >
> > > The above sets the rate[0].idx to rix
> > >
> > > > +               rates[0].idx = rate_lowest_index(sband, sta);
> > >
> > > and then here we set it to rate_lowest_index(sband, sta) comes up
> > > with. They should be the same, but this just goes to show we need to
> > > clean this better.
> >
> > I just found this is wrong too.. the rate table used to find
> > rix is different than the rate table for the mode. ie, in 5ghz
> > when associated to an 11n station this could not include any
> > legacy rates.
> 
> This is wrong. All basci rates still be used in 11n mode.

Never mind, this is only for 11ng.
> 
> Vasanth