[PATCH 1/6] ath9k: Reset chip on baseband hang

[PATCH 1/6] ath9k: Reset chip on baseband hang

[Rajkumar Manoharan]

Resetting hardware helps to recover from baseband
hang/panic for AR9003 based chips.

Cc: stable@kernel.org
Signed-off-by: Rajkumar Manoharan <rmanoharan@atheros.com>
---
 drivers/net/wireless/ath/ath9k/main.c |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/drivers/net/wireless/ath/ath9k/main.c b/drivers/net/wireless/ath/ath9k/main.c
index a198ee3..2ca351f 100644
--- a/drivers/net/wireless/ath/ath9k/main.c
+++ b/drivers/net/wireless/ath/ath9k/main.c
@@ -670,7 +670,8 @@ void ath9k_tasklet(unsigned long data)
 	u32 status = sc->intrstatus;
 	u32 rxmask;
 
-	if (status & ATH9K_INT_FATAL) {
+	if ((status & ATH9K_INT_FATAL) ||
+	    (status & ATH9K_INT_BB_WATCHDOG)) {
 		ath_reset(sc, true);
 		return;
 	}
@@ -737,6 +738,7 @@ irqreturn_t ath_isr(int irq, void *dev)
 {
 #define SCHED_INTR (				\
 		ATH9K_INT_FATAL |		\
+		ATH9K_INT_BB_WATCHDOG |		\
 		ATH9K_INT_RXORN |		\
 		ATH9K_INT_RXEOL |		\
 		ATH9K_INT_RX |			\
-- 
1.7.5.1

[PATCH 3/6] ath9k: Handle IBSS status changes in BSS_CHANGED_IBSS

[Rajkumar Manoharan]

Configure ibss node's bss_changes under BSS_CHANGED_IBSS.
And also start/stop ani timer only if the station join/leave
the group.

Signed-off-by: Rajkumar Manoharan <rmanoharan@atheros.com>
---
 drivers/net/wireless/ath/ath9k/main.c |   85 +++++++++++++++++----------------
 1 files changed, 44 insertions(+), 41 deletions(-)

diff --git a/drivers/net/wireless/ath/ath9k/main.c b/drivers/net/wireless/ath/ath9k/main.c
index 2ca351f..81708ed 100644
--- a/drivers/net/wireless/ath/ath9k/main.c
+++ b/drivers/net/wireless/ath/ath9k/main.c
@@ -1412,7 +1412,7 @@ static void ath9k_calculate_summary_state(struct ieee80211_hw *hw,
 	ath9k_hw_set_interrupts(ah, ah->imask);
 
 	/* Set up ANI */
-	if ((iter_data.naps + iter_data.nadhocs) > 0) {
+	if (iter_data.naps > 0) {
 		sc->sc_ah->stats.avgbrssi = ATH_RSSI_DUMMY_MARKER;
 		sc->sc_flags |= SC_OP_ANI_RUN;
 		ath_start_ani(common);
@@ -1952,50 +1952,35 @@ static void ath9k_bss_iter(void *data, u8 *mac, struct ieee80211_vif *vif)
 	struct ieee80211_bss_conf *bss_conf = &vif->bss_conf;
 	struct ath_vif *avp = (void *)vif->drv_priv;
 
-	switch (sc->sc_ah->opmode) {
-	case NL80211_IFTYPE_ADHOC:
-		/* There can be only one vif available */
+	/*
+	 * Skip iteration if primary station vif's bss info
+	 * was not changed
+	 */
+	if (sc->sc_flags & SC_OP_PRIM_STA_VIF)
+		return;
+
+	if (bss_conf->assoc) {
+		sc->sc_flags |= SC_OP_PRIM_STA_VIF;
+		avp->primary_sta_vif = true;
 		memcpy(common->curbssid, bss_conf->bssid, ETH_ALEN);
 		common->curaid = bss_conf->aid;
 		ath9k_hw_write_associd(sc->sc_ah);
-		/* configure beacon */
-		if (bss_conf->enable_beacon)
-			ath_beacon_config(sc, vif);
-		break;
-	case NL80211_IFTYPE_STATION:
-		/*
-		 * Skip iteration if primary station vif's bss info
-		 * was not changed
-		 */
-		if (sc->sc_flags & SC_OP_PRIM_STA_VIF)
-			break;
-
-		if (bss_conf->assoc) {
-			sc->sc_flags |= SC_OP_PRIM_STA_VIF;
-			avp->primary_sta_vif = true;
-			memcpy(common->curbssid, bss_conf->bssid, ETH_ALEN);
-			common->curaid = bss_conf->aid;
-			ath9k_hw_write_associd(sc->sc_ah);
-			ath_dbg(common, ATH_DBG_CONFIG,
+		ath_dbg(common, ATH_DBG_CONFIG,
 				"Bss Info ASSOC %d, bssid: %pM\n",
 				bss_conf->aid, common->curbssid);
-			ath_beacon_config(sc, vif);
-			/*
-			 * Request a re-configuration of Beacon related timers
-			 * on the receipt of the first Beacon frame (i.e.,
-			 * after time sync with the AP).
-			 */
-			sc->ps_flags |= PS_BEACON_SYNC | PS_WAIT_FOR_BEACON;
-			/* Reset rssi stats */
-			sc->last_rssi = ATH_RSSI_DUMMY_MARKER;
-			sc->sc_ah->stats.avgbrssi = ATH_RSSI_DUMMY_MARKER;
+		ath_beacon_config(sc, vif);
+		/*
+		 * Request a re-configuration of Beacon related timers
+		 * on the receipt of the first Beacon frame (i.e.,
+		 * after time sync with the AP).
+		 */
+		sc->ps_flags |= PS_BEACON_SYNC | PS_WAIT_FOR_BEACON;
+		/* Reset rssi stats */
+		sc->last_rssi = ATH_RSSI_DUMMY_MARKER;
+		sc->sc_ah->stats.avgbrssi = ATH_RSSI_DUMMY_MARKER;
 
-			sc->sc_flags |= SC_OP_ANI_RUN;
-			ath_start_ani(common);
-		}
-		break;
-	default:
-		break;
+		sc->sc_flags |= SC_OP_ANI_RUN;
+		ath_start_ani(common);
 	}
 }
 
@@ -2005,6 +1990,9 @@ static void ath9k_config_bss(struct ath_softc *sc, struct ieee80211_vif *vif)
 	struct ieee80211_bss_conf *bss_conf = &vif->bss_conf;
 	struct ath_vif *avp = (void *)vif->drv_priv;
 
+	if (sc->sc_ah->opmode != NL80211_IFTYPE_STATION)
+		return;
+
 	/* Reconfigure bss info */
 	if (avp->primary_sta_vif && !bss_conf->assoc) {
 		ath_dbg(common, ATH_DBG_CONFIG,
@@ -2023,8 +2011,7 @@ static void ath9k_config_bss(struct ath_softc *sc, struct ieee80211_vif *vif)
 	 * None of station vifs are associated.
 	 * Clear bssid & aid
 	 */
-	if ((sc->sc_ah->opmode == NL80211_IFTYPE_STATION) &&
-	    !(sc->sc_flags & SC_OP_PRIM_STA_VIF)) {
+	if (!(sc->sc_flags & SC_OP_PRIM_STA_VIF)) {
 		ath9k_hw_write_associd(sc->sc_ah);
 		/* Stop ANI */
 		sc->sc_flags &= ~SC_OP_ANI_RUN;
@@ -2054,6 +2041,22 @@ static void ath9k_bss_info_changed(struct ieee80211_hw *hw,
 			common->curbssid, common->curaid);
 	}
 
+	if (changed & BSS_CHANGED_IBSS) {
+		/* There can be only one vif available */
+		memcpy(common->curbssid, bss_conf->bssid, ETH_ALEN);
+		common->curaid = bss_conf->aid;
+		ath9k_hw_write_associd(sc->sc_ah);
+
+		if (bss_conf->ibss_joined) {
+			sc->sc_ah->stats.avgbrssi = ATH_RSSI_DUMMY_MARKER;
+			sc->sc_flags |= SC_OP_ANI_RUN;
+			ath_start_ani(common);
+		} else {
+			sc->sc_flags &= ~SC_OP_ANI_RUN;
+			del_timer_sync(&common->ani.timer);
+		}
+	}
+
 	/* Enable transmission of beacons (AP, IBSS, MESH) */
 	if ((changed & BSS_CHANGED_BEACON) ||
 	    ((changed & BSS_CHANGED_BEACON_ENABLED) && bss_conf->enable_beacon)) {
-- 
1.7.5.1

[PATCH 4/6] ath9k_hw: disable phy restart on baseband panic caused by RXSM

[Rajkumar Manoharan]

While receiving unsupported rate frame rx state machine
gets into a state 0xb and if phy_restart happens in that
state, BB would go hang. If RXSM is in 0xb state after
first bb panic, ensure to disable the phy_restart.

Signed-off-by: Rajkumar Manoharan <rmanoharan@atheros.com>
---
 drivers/net/wireless/ath/ath9k/ar9003_phy.c |   22 ++++++++++++++++++++++
 drivers/net/wireless/ath/ath9k/hw.c         |    5 ++++-
 drivers/net/wireless/ath/ath9k/hw.h         |    2 ++
 3 files changed, 28 insertions(+), 1 deletions(-)

diff --git a/drivers/net/wireless/ath/ath9k/ar9003_phy.c b/drivers/net/wireless/ath/ath9k/ar9003_phy.c
index eee23ec..892c48b 100644
--- a/drivers/net/wireless/ath/ath9k/ar9003_phy.c
+++ b/drivers/net/wireless/ath/ath9k/ar9003_phy.c
@@ -1381,3 +1381,25 @@ void ar9003_hw_bb_watchdog_dbg_info(struct ath_hw *ah)
 		"==== BB update: done ====\n\n");
 }
 EXPORT_SYMBOL(ar9003_hw_bb_watchdog_dbg_info);
+
+void ar9003_hw_disable_phy_restart(struct ath_hw *ah)
+{
+	u32 val;
+
+	/* While receiving unsupported rate frame rx state machine
+	 * gets into a state 0xb and if phy_restart happens in that
+	 * state, BB would go hang. If RXSM is in 0xb state after
+	 * first bb panic, ensure to disable the phy_restart.
+	 */
+	if (!((MS(ah->bb_watchdog_last_status,
+		  AR_PHY_WATCHDOG_RX_OFDM_SM) == 0xb) ||
+	    ah->bb_hang_rx_ofdm))
+		return;
+
+	ah->bb_hang_rx_ofdm = true;
+	val = REG_READ(ah, AR_PHY_RESTART);
+	val &= ~AR_PHY_RESTART_ENA;
+
+	REG_WRITE(ah, AR_PHY_RESTART, val);
+}
+EXPORT_SYMBOL(ar9003_hw_disable_phy_restart);
diff --git a/drivers/net/wireless/ath/ath9k/hw.c b/drivers/net/wireless/ath/ath9k/hw.c
index 8300ed1..6de2655 100644
--- a/drivers/net/wireless/ath/ath9k/hw.c
+++ b/drivers/net/wireless/ath/ath9k/hw.c
@@ -1555,9 +1555,12 @@ int ath9k_hw_reset(struct ath_hw *ah, struct ath9k_channel *chan,
 	if (ah->btcoex_hw.enabled)
 		ath9k_hw_btcoex_enable(ah);
 
-	if (AR_SREV_9300_20_OR_LATER(ah))
+	if (AR_SREV_9300_20_OR_LATER(ah)) {
 		ar9003_hw_bb_watchdog_config(ah);
 
+		ar9003_hw_disable_phy_restart(ah);
+	}
+
 	ath9k_hw_apply_gpio_override(ah);
 
 	return 0;
diff --git a/drivers/net/wireless/ath/ath9k/hw.h b/drivers/net/wireless/ath/ath9k/hw.h
index f8a626d..5111553 100644
--- a/drivers/net/wireless/ath/ath9k/hw.h
+++ b/drivers/net/wireless/ath/ath9k/hw.h
@@ -841,6 +841,7 @@ struct ath_hw {
 
 	u32 bb_watchdog_last_status;
 	u32 bb_watchdog_timeout_ms; /* in ms, 0 to disable */
+	u8 bb_hang_rx_ofdm; /* true if bb hang due to rx_ofdm */
 
 	unsigned int paprd_target_power;
 	unsigned int paprd_training_power;
@@ -989,6 +990,7 @@ void ar9002_hw_enable_wep_aggregation(struct ath_hw *ah);
 void ar9003_hw_bb_watchdog_config(struct ath_hw *ah);
 void ar9003_hw_bb_watchdog_read(struct ath_hw *ah);
 void ar9003_hw_bb_watchdog_dbg_info(struct ath_hw *ah);
+void ar9003_hw_disable_phy_restart(struct ath_hw *ah);
 void ar9003_paprd_enable(struct ath_hw *ah, bool val);
 void ar9003_paprd_populate_single_table(struct ath_hw *ah,
 					struct ath9k_hw_cal_data *caldata,
-- 
1.7.5.1

[PATCH 5/6] ath9k: set 40 Mhz rate only if hw is configured in ht40

[Rajkumar Manoharan]

Whenever there is a channel width change from 40 Mhz to 20 Mhz,
the hardware is reconfigured to ht20. Meantime before doing
the rate control updation, the packets are being transmitted are
selected rate with IEEE80211_TX_RC_40_MHZ_WIDTH.

While transmitting ht40 rate packets in ht20 mode is causing
baseband panic with AR9003 based chips.

==== BB update: BB status=0x02001109 ====
ath: ** BB state: wd=1 det=1 rdar=0 rOFDM=1 rCCK=1 tOFDM=0 tCCK=0 agc=2
src=0 **
ath: ** BB WD cntl: cntl1=0xffff0085 cntl2=0x00000004 **
ath: ** BB mode: BB_gen_controls=0x000033c0 **
ath: ** BB busy times: rx_clear=99%, rx_frame=0%, tx_frame=0% **
ath: ==== BB update: done ====

Cc: stable@kernel.org
Signed-off-by: Rajkumar Manoharan <rmanoharan@atheros.com>
---
 drivers/net/wireless/ath/ath9k/rc.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/drivers/net/wireless/ath/ath9k/rc.c b/drivers/net/wireless/ath/ath9k/rc.c
index 1754221..ba7f36a 100644
--- a/drivers/net/wireless/ath/ath9k/rc.c
+++ b/drivers/net/wireless/ath/ath9k/rc.c
@@ -689,7 +689,8 @@ static void ath_rc_rate_set_series(const struct ath_rate_table *rate_table,
 
 	if (WLAN_RC_PHY_HT(rate_table->info[rix].phy)) {
 		rate->flags |= IEEE80211_TX_RC_MCS;
-		if (WLAN_RC_PHY_40(rate_table->info[rix].phy))
+		if (WLAN_RC_PHY_40(rate_table->info[rix].phy) &&
+		    conf_is_ht40(&txrc->hw->conf))
 			rate->flags |= IEEE80211_TX_RC_40_MHZ_WIDTH;
 		if (WLAN_RC_PHY_SGI(rate_table->info[rix].phy))
 			rate->flags |= IEEE80211_TX_RC_SHORT_GI;
-- 
1.7.5.1

[PATCH 6/6] mac80211: stop queues before rate control updation

[Rajkumar Manoharan]

Stop tx queues before updating rate control to ensure
proper rate selection. Otherwise packets can be transmitted
in 40 Mhz whereas hw is configured in HT20.

Signed-off-by: Rajkumar Manoharan <rmanoharan@atheros.com>
---
 net/mac80211/mlme.c |    6 ++++++
 1 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/net/mac80211/mlme.c b/net/mac80211/mlme.c
index 4f6b267..7465955 100644
--- a/net/mac80211/mlme.c
+++ b/net/mac80211/mlme.c
@@ -232,6 +232,9 @@ static u32 ieee80211_enable_ht(struct ieee80211_sub_if_data *sdata,
 		WARN_ON(!ieee80211_set_channel_type(local, sdata, channel_type));
 	}
 
+	ieee80211_stop_queues_by_reason(&sdata->local->hw,
+					IEEE80211_QUEUE_STOP_REASON_CSA);
+
 	/* channel_type change automatically detected */
 	ieee80211_hw_config(local, 0);
 
@@ -245,6 +248,9 @@ static u32 ieee80211_enable_ht(struct ieee80211_sub_if_data *sdata,
 		rcu_read_unlock();
 	}
 
+	ieee80211_wake_queues_by_reason(&sdata->local->hw,
+					IEEE80211_QUEUE_STOP_REASON_CSA);
+
 	ht_opmode = le16_to_cpu(hti->operation_mode);
 
 	/* if bss configuration changed store the new one */
-- 
1.7.5.1

[PATCH 2/6] ath9k: Remove ATH9K_BEACON_PERIOD mask

[Rajkumar Manoharan]

Earlier beacon_interval is used to hold interval value and
some flags (ATH9K_BEACON_ENA &ATH9K_BEACON_PERIOD). So to
extract interval ATH9K_BEACON_PERIOD is used. Those flags
were completely removed. So masking beacon_interval is
not required.

Signed-off-by: Rajkumar Manoharan <rmanoharan@atheros.com>
---
 drivers/net/wireless/ath/ath9k/beacon.c         |    6 +++---
 drivers/net/wireless/ath/ath9k/htc_drv_beacon.c |    8 ++++----
 drivers/net/wireless/ath/ath9k/hw.c             |    6 +++---
 drivers/net/wireless/ath/ath9k/hw.h             |    1 -
 4 files changed, 10 insertions(+), 11 deletions(-)

diff --git a/drivers/net/wireless/ath/ath9k/beacon.c b/drivers/net/wireless/ath/ath9k/beacon.c
index d4d8cec..8b85d7c 100644
--- a/drivers/net/wireless/ath/ath9k/beacon.c
+++ b/drivers/net/wireless/ath/ath9k/beacon.c
@@ -496,7 +496,7 @@ static void ath_beacon_config_ap(struct ath_softc *sc,
        u32 nexttbtt, intval;

        /* NB: the beacon interval is kept internally in TU's */
-       intval = TU_TO_USEC(conf->beacon_interval & ATH9K_BEACON_PERIOD);
+       intval = TU_TO_USEC(conf->beacon_interval);
        intval /= ATH_BCBUF;    /* for staggered beacons */
        nexttbtt = intval;

@@ -543,7 +543,7 @@ static void ath_beacon_config_sta(struct ath_softc *sc,
        }

        memset(&bs, 0, sizeof(bs));
-       intval = conf->beacon_interval & ATH9K_BEACON_PERIOD;
+       intval = conf->beacon_interval;

        /*
         * Setup dtim and cfp parameters according to
@@ -657,7 +657,7 @@ static void ath_beacon_config_adhoc(struct ath_softc *sc,
        ath9k_reset_beacon_status(sc);

        tsf = ath9k_hw_gettsf32(ah) + TU_TO_USEC(FUDGE);
-       intval = TU_TO_USEC(conf->beacon_interval & ATH9K_BEACON_PERIOD);
+       intval = TU_TO_USEC(conf->beacon_interval);

        if (!sc->beacon.bc_tstamp)
                nexttbtt = tsf + intval;
diff --git a/drivers/net/wireless/ath/ath9k/htc_drv_beacon.c b/drivers/net/wireless/ath/ath9k/htc_drv_beacon.c
index aa6a731..57fe22b 100644
--- a/drivers/net/wireless/ath/ath9k/htc_drv_beacon.c
+++ b/drivers/net/wireless/ath/ath9k/htc_drv_beacon.c
@@ -79,7 +79,7 @@ static void ath9k_htc_beacon_config_sta(struct ath9k_htc_priv *priv,

        memset(&bs, 0, sizeof(bs));

-       intval = bss_conf->beacon_interval & ATH9K_BEACON_PERIOD;
+       intval = bss_conf->beacon_interval;
        bmiss_timeout = (ATH_DEFAULT_BMISS_LIMIT * bss_conf->beacon_interval);

        /*
@@ -194,7 +194,7 @@ static void ath9k_htc_beacon_config_ap(struct ath9k_htc_priv *priv,
        u8 cmd_rsp;
        u64 tsf;

-       intval = bss_conf->beacon_interval & ATH9K_BEACON_PERIOD;
+       intval = bss_conf->beacon_interval;
        intval /= ATH9K_HTC_MAX_BCN_VIF;
        nexttbtt = intval;

@@ -250,7 +250,7 @@ static void ath9k_htc_beacon_config_adhoc(struct ath9k_htc_priv *priv,
        u8 cmd_rsp;
        u64 tsf;

-       intval = bss_conf->beacon_interval & ATH9K_BEACON_PERIOD;
+       intval = bss_conf->beacon_interval;
        nexttbtt = intval;

        /*
@@ -427,7 +427,7 @@ static int ath9k_htc_choose_bslot(struct ath9k_htc_priv *priv,
        u16 intval;
        int slot;

-       intval = priv->cur_beacon_conf.beacon_interval & ATH9K_BEACON_PERIOD;
+       intval = priv->cur_beacon_conf.beacon_interval;

        tsf = be64_to_cpu(swba->tsf);
        tsftu = TSF_TO_TU(tsf >> 32, tsf);
diff --git a/drivers/net/wireless/ath/ath9k/hw.c b/drivers/net/wireless/ath/ath9k/hw.c
index 72543ce..8300ed1 100644
--- a/drivers/net/wireless/ath/ath9k/hw.c
+++ b/drivers/net/wireless/ath/ath9k/hw.c
@@ -1782,16 +1782,16 @@ void ath9k_hw_set_sta_beacon_timers(struct ath_hw *ah,
        REG_WRITE(ah, AR_NEXT_TBTT_TIMER, TU_TO_USEC(bs->bs_nexttbtt));

        REG_WRITE(ah, AR_BEACON_PERIOD,
-                 TU_TO_USEC(bs->bs_intval & ATH9K_BEACON_PERIOD));
+                 TU_TO_USEC(bs->bs_intval));
        REG_WRITE(ah, AR_DMA_BEACON_PERIOD,
-                 TU_TO_USEC(bs->bs_intval & ATH9K_BEACON_PERIOD));
+                 TU_TO_USEC(bs->bs_intval));

        REGWRITE_BUFFER_FLUSH(ah);

        REG_RMW_FIELD(ah, AR_RSSI_THR,
                      AR_RSSI_THR_BM_THR, bs->bs_bmissthreshold);

-       beaconintval = bs->bs_intval & ATH9K_BEACON_PERIOD;
+       beaconintval = bs->bs_intval;

        if (bs->bs_sleepduration > beaconintval)
                beaconintval = bs->bs_sleepduration;
diff --git a/drivers/net/wireless/ath/ath9k/hw.h b/drivers/net/wireless/ath/ath9k/hw.h
index 57435ce..f8a626d 100644
--- a/drivers/net/wireless/ath/ath9k/hw.h
+++ b/drivers/net/wireless/ath/ath9k/hw.h
@@ -403,7 +403,6 @@ struct ath9k_beacon_state {
        u32 bs_nexttbtt;
        u32 bs_nextdtim;
        u32 bs_intval;
-#define ATH9K_BEACON_PERIOD       0x0000ffff
 #define ATH9K_TSFOOR_THRESHOLD    0x00004240 /* 16k us */
        u32 bs_dtimperiod;
        u16 bs_cfpperiod;
--
1.7.5.1

Re: [PATCH 6/6] mac80211: stop queues before rate control updation

[Johannes Berg]

On Fri, 2011-05-20 at 17:52 +0530, Rajkumar Manoharan wrote:
> Stop tx queues before updating rate control to ensure
> proper rate selection. Otherwise packets can be transmitted
> in 40 Mhz whereas hw is configured in HT20.

Looks like I completely missed this since you hid it in an ath9k
patchset. DON'T DO THAT.

Anyway, John, please revert. This is completely useless. Not only is
abusing the CSA stop reason a show-stopper, the whole patch is also just
not right, it seems like a workaround around a rate control algorithm
that isn't able to do an atomic HT change by itself. Also, it won't even
do what you want, there may be packets being processed concurrently
while stopping the queue -- calling stop_queues() is no guarantee that
no packet will be processed afterwards.

johannes

Re: [PATCH 6/6] mac80211: stop queues before rate control updation

[Adrian Chadd]

On 31 May 2011 14:35, Johannes Berg <johannes@sipsolutions.net> wrote:

> Looks like I completely missed this since you hid it in an ath9k
> patchset. DON'T DO THAT.
>
> Anyway, John, please revert. This is completely useless. Not only is
> abusing the CSA stop reason a show-stopper, the whole patch is also just
> not right, it seems like a workaround around a rate control algorithm
> that isn't able to do an atomic HT change by itself. Also, it won't even
> do what you want, there may be packets being processed concurrently
> while stopping the queue -- calling stop_queues() is no guarantee that
> no packet will be processed afterwards.

I'm unsure of what's going on in mac80211 and ath9k here.

FreeBSD handles it very simply - it goes via ath_reset() which drains
each TX queue, freeing existing packets. Inefficient (ie, packet loss)
but fine for now.

How is ath9k handling the situation where the hardware currently has
HT40 packets queued in the TX queues and you do a 40->20 change?
The 40->20 change in the ath9k instance requires reinit'ing of the
card. What happens to currently queued packets?

Adrian

Re: [PATCH 6/6] mac80211: stop queues before rate control updation

[John W. Linville]

On Tue, May 31, 2011 at 08:35:17AM +0200, Johannes Berg wrote:
> On Fri, 2011-05-20 at 17:52 +0530, Rajkumar Manoharan wrote:
> > Stop tx queues before updating rate control to ensure
> > proper rate selection. Otherwise packets can be transmitted
> > in 40 Mhz whereas hw is configured in HT20.
> 
> Looks like I completely missed this since you hid it in an ath9k
> patchset. DON'T DO THAT.
> 
> Anyway, John, please revert. This is completely useless. Not only is
> abusing the CSA stop reason a show-stopper, the whole patch is also just
> not right, it seems like a workaround around a rate control algorithm
> that isn't able to do an atomic HT change by itself. Also, it won't even
> do what you want, there may be packets being processed concurrently
> while stopping the queue -- calling stop_queues() is no guarantee that
> no packet will be processed afterwards.

Rajkumar, do you have an alternative fix to propose?

-- 
John W. Linville		Someday the world will need a hero, and you
linville@tuxdriver.com			might be all we have.  Be ready.

RE: [PATCH 6/6] mac80211: stop queues before rate control updation

[Manoharan, Rajkumar]

> On Fri, 2011-05-20 at 17:52 +0530, Rajkumar Manoharan wrote:
> > Stop tx queues before updating rate control to ensure
> > proper rate selection. Otherwise packets can be transmitted
> > in 40 Mhz whereas hw is configured in HT20.
> 
> Looks like I completely missed this since you hid it in an ath9k
> patchset. DON'T DO THAT.

Sorry for the delayed response. I was on vacation. 
>
> Anyway, John, please revert. This is completely useless. Not only is
> abusing the CSA stop reason a show-stopper, the whole patch is also just
> not right, it seems like a workaround around a rate control algorithm
> that isn't able to do an atomic HT change by itself. Also, it won't even
> do what you want, there may be packets being processed concurrently
> while stopping the queue -- calling stop_queues() is no guarantee that
> no packet will be processed afterwards.

During the channel type change, the pending tx frames in hw queues are dropped by hw config.
But before updating rate control, the packets can be queued again with older HT rates.
This contradicts with hw config mode and sometimes is causing baseband issues. This issue
was observed only on flooding uplink traffic. To ensure that the frames are always xmitted with
updated rates, the queues are stopped before hw config and waken up after rc updation.

--
Rajkumar

Re: [PATCH 6/6] mac80211: stop queues before rate control updation

[John W. Linville]

On Fri, Jun 03, 2011 at 05:00:01AM +0000, Manoharan, Rajkumar wrote:
> > On Fri, 2011-05-20 at 17:52 +0530, Rajkumar Manoharan wrote:
> > > Stop tx queues before updating rate control to ensure
> > > proper rate selection. Otherwise packets can be transmitted
> > > in 40 Mhz whereas hw is configured in HT20.
> > 
> > Looks like I completely missed this since you hid it in an ath9k
> > patchset. DON'T DO THAT.
> 
> Sorry for the delayed response. I was on vacation. 
> >
> > Anyway, John, please revert. This is completely useless. Not only is
> > abusing the CSA stop reason a show-stopper, the whole patch is also just
> > not right, it seems like a workaround around a rate control algorithm
> > that isn't able to do an atomic HT change by itself. Also, it won't even
> > do what you want, there may be packets being processed concurrently
> > while stopping the queue -- calling stop_queues() is no guarantee that
> > no packet will be processed afterwards.
> 
> During the channel type change, the pending tx frames in hw queues are dropped by hw config.
> But before updating rate control, the packets can be queued again with older HT rates.
> This contradicts with hw config mode and sometimes is causing baseband issues. This issue
> was observed only on flooding uplink traffic. To ensure that the frames are always xmitted with
> updated rates, the queues are stopped before hw config and waken up after rc updation.

Johannes, do you find this explanation satisfactory (perhaps with
some new queue stop reason definition)?

John
-- 
John W. Linville		Someday the world will need a hero, and you
linville@tuxdriver.com			might be all we have.  Be ready.

Re: [PATCH 6/6] mac80211: stop queues before rate control updation

[Johannes Berg]

On Mon, 2011-06-06 at 14:44 -0400, John W. Linville wrote:

> > During the channel type change, the pending tx frames in hw queues are dropped by hw config.
> > But before updating rate control, the packets can be queued again with older HT rates.
> > This contradicts with hw config mode and sometimes is causing baseband issues. This issue
> > was observed only on flooding uplink traffic. To ensure that the frames are always xmitted with
> > updated rates, the queues are stopped before hw config and waken up after rc updation.
> 
> Johannes, do you find this explanation satisfactory (perhaps with
> some new queue stop reason definition)?

No, it doesn't address the fact that any packet that is pending will
still be processed -- as I said before, the stop doesn't include a
flush.

johannes

RE: [PATCH 6/6] mac80211: stop queues before rate control updation

[Manoharan, Rajkumar]

> > On Mon, 2011-06-06 at 14:44 -0400, John W. Linville wrote:
> 
> > > During the channel type change, the pending tx frames in hw queues are dropped by hw config.
> > > But before updating rate control, the packets can be queued again with older HT rates.
> > > This contradicts with hw config mode and sometimes is causing baseband issues. This issue
> > > was observed only on flooding uplink traffic. To ensure that the frames are always xmitted with
> > > updated rates, the queues are stopped before hw config and waken up after rc updation.
>> 
> > Johannes, do you find this explanation satisfactory (perhaps with
> > some new queue stop reason definition)?
> 
> No, it doesn't address the fact that any packet that is pending will
> still be processed -- as I said before, the stop doesn't include a
> flush.

Yes. I agree. Without flushing, still the packets can choose wrong rates. I missed that.
But I assumed that it would be better to stop queues before rc update. Thus we can avoid revisiting
the queued frames chosen with older (this case ht40) rate after rc changes. And fixing at mac80211 would help
to other rate controls too.

--
Rajkumar

Re: [PATCH 6/6] mac80211: stop queues before rate control updation

[John W. Linville]

On Tue, Jun 07, 2011 at 08:39:57AM +0000, Manoharan, Rajkumar wrote:
> > > On Mon, 2011-06-06 at 14:44 -0400, John W. Linville wrote:
> > 
> > > > During the channel type change, the pending tx frames in hw queues are dropped by hw config.
> > > > But before updating rate control, the packets can be queued again with older HT rates.
> > > > This contradicts with hw config mode and sometimes is causing baseband issues. This issue
> > > > was observed only on flooding uplink traffic. To ensure that the frames are always xmitted with
> > > > updated rates, the queues are stopped before hw config and waken up after rc updation.
> >> 
> > > Johannes, do you find this explanation satisfactory (perhaps with
> > > some new queue stop reason definition)?
> > 
> > No, it doesn't address the fact that any packet that is pending will
> > still be processed -- as I said before, the stop doesn't include a
> > flush.
> 
> Yes. I agree. Without flushing, still the packets can choose wrong rates. I missed that.
> But I assumed that it would be better to stop queues before rc update. Thus we can avoid revisiting
> the queued frames chosen with older (this case ht40) rate after rc changes. And fixing at mac80211 would help
> to other rate controls too.

OK, it seems like this isn't getting resolved quickly.  I'm going to
revert it for now, and hope for a more widely acceptable solution soon.

John
-- 
John W. Linville		Someday the world will need a hero, and you
linville@tuxdriver.com			might be all we have.  Be ready.