[PATCH 1/6] carl9170: remove non-standard ba session teardown

[PATCH 1/6] carl9170: remove non-standard ba session teardown

[Christian Lamparter]

802.11n-2009 demands in 11.2.1: "
When a STA enters normal (non-APSD) PS mode, any downlink
Block ACK agreement without an associated schedule is
suspended for the duration of this PS mode."

The operative word is "suspended" and not terminated.

Signed-off-by: Christian Lamparter <chunkeey@googlemail.com>
---
 drivers/net/wireless/ath/carl9170/main.c |    3 ---
 1 files changed, 0 insertions(+), 3 deletions(-)

diff --git a/drivers/net/wireless/ath/carl9170/main.c b/drivers/net/wireless/ath/carl9170/main.c
index 43de9df..d28b4ff 100644
--- a/drivers/net/wireless/ath/carl9170/main.c
+++ b/drivers/net/wireless/ath/carl9170/main.c
@@ -1453,9 +1453,6 @@ static void carl9170_op_sta_notify(struct ieee80211_hw *hw,
 				while ((skb = __skb_dequeue(&tid_info->queue)))
 					__skb_queue_tail(&free, skb);
 				spin_unlock_bh(&tid_info->lock);
-
-				ieee80211_stop_tx_ba_session(sta,
-					tid_info->tid);
 			}
 			rcu_read_unlock();
 		}
-- 
1.7.1

[PATCH 2/6] carl9170: angle brackets for wiki link

[Christian Lamparter]

"The convention seems to be angle brackets around
URLS in Kconfig."
 -- Finn Thain (to update web addresses in the kernel)

Signed-off-by: Christian Lamparter <chunkeey@googlemail.com>
---
 drivers/net/wireless/ath/carl9170/Kconfig |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/net/wireless/ath/carl9170/Kconfig b/drivers/net/wireless/ath/carl9170/Kconfig
index c5d3a3f..2d1b821 100644
--- a/drivers/net/wireless/ath/carl9170/Kconfig
+++ b/drivers/net/wireless/ath/carl9170/Kconfig
@@ -10,7 +10,7 @@ config CARL9170
 	  but it needs a special firmware (carl9170-1.fw) to do that.
 
 	  The firmware can be downloaded from our wiki here:
-	  http://wireless.kernel.org/en/users/Drivers/carl9170
+	  <http://wireless.kernel.org/en/users/Drivers/carl9170>
 
 	  If you choose to build a module, it'll be called carl9170.
 
-- 
1.7.1

[PATCH 3/6] carl9170: remove lost-frame workaround

[Christian Lamparter]

This patch removes some cruft, which survived
the RFC review phase.

Originally, carl9170_tx_ampdu_queue erroneously
dropped a lot of frames. As a result the ampdu
scheduler bogged down quite frequently and the
affected BA session timed out.

However this bug has been fixed and the WA and
its debugfs counter is no longer useful.

Signed-off-by: Christian Lamparter <chunkeey@googlemail.com>
---
 drivers/net/wireless/ath/carl9170/carl9170.h |    1 -
 drivers/net/wireless/ath/carl9170/debug.c    |    4 ---
 drivers/net/wireless/ath/carl9170/tx.c       |   37 +------------------------
 3 files changed, 2 insertions(+), 40 deletions(-)

diff --git a/drivers/net/wireless/ath/carl9170/carl9170.h b/drivers/net/wireless/ath/carl9170/carl9170.h
index d7c5482..20f2a77 100644
--- a/drivers/net/wireless/ath/carl9170/carl9170.h
+++ b/drivers/net/wireless/ath/carl9170/carl9170.h
@@ -364,7 +364,6 @@ struct ar9170 {
 	unsigned int tx_dropped;
 	unsigned int tx_ack_failures;
 	unsigned int tx_fcs_errors;
-	unsigned int tx_ampdu_timeout;
 	unsigned int rx_dropped;
 
 	/* EEPROM */
diff --git a/drivers/net/wireless/ath/carl9170/debug.c b/drivers/net/wireless/ath/carl9170/debug.c
index dbb35c3..19f45e9 100644
--- a/drivers/net/wireless/ath/carl9170/debug.c
+++ b/drivers/net/wireless/ath/carl9170/debug.c
@@ -799,8 +799,6 @@ DEBUGFS_READONLY_FILE(tx_total_queued, 20, "%d",
 		      atomic_read(&ar->tx_total_queued));
 DEBUGFS_READONLY_FILE(tx_ampdu_scheduler, 20, "%d",
 		      atomic_read(&ar->tx_ampdu_scheduler));
-DEBUGFS_READONLY_FILE(tx_ampdu_timeout, 20, "%d",
-		      ar->tx_ampdu_timeout);
 
 DEBUGFS_READONLY_FILE(tx_total_pending, 20, "%d",
 		      atomic_read(&ar->tx_total_pending));
@@ -873,8 +871,6 @@ void carl9170_debugfs_register(struct ar9170 *ar)
 	DEBUGFS_ADD(ampdu_density);
 	DEBUGFS_ADD(ampdu_factor);
 
-	DEBUGFS_ADD(tx_ampdu_timeout);
-
 	DEBUGFS_ADD(tx_janitor_last_run);
 
 	DEBUGFS_ADD(tx_status_0);
diff --git a/drivers/net/wireless/ath/carl9170/tx.c b/drivers/net/wireless/ath/carl9170/tx.c
index e0d2374..e6be5e6 100644
--- a/drivers/net/wireless/ath/carl9170/tx.c
+++ b/drivers/net/wireless/ath/carl9170/tx.c
@@ -1042,41 +1042,8 @@ retry:
 		queue = TID_TO_WME_AC(tid_info->tid);
 
 		spin_lock_bh(&tid_info->lock);
-		if (tid_info->state != CARL9170_TID_STATE_XMIT) {
-			first = skb_peek(&tid_info->queue);
-			if (first) {
-				struct ieee80211_tx_info *txinfo;
-				struct carl9170_tx_info *arinfo;
-
-				txinfo = IEEE80211_SKB_CB(first);
-				arinfo = (void *) txinfo->rate_driver_data;
-
-				if (time_is_after_jiffies(arinfo->timeout +
-				    msecs_to_jiffies(CARL9170_QUEUE_TIMEOUT))
-				    == true)
-					goto processed;
-
-				/*
-				 * We've been waiting for the frame which
-				 * matches "snx" (start sequence of the
-				 * next aggregate) for some time now.
-				 *
-				 * But it never arrived. Therefore
-				 * jump to the next available frame
-				 * and kick-start the transmission.
-				 *
-				 * Note: This might induce odd latency
-				 * spikes because the receiver will be
-				 * waiting for the lost frame too.
-				 */
-				ar->tx_ampdu_timeout++;
-
-				tid_info->snx = carl9170_get_seq(first);
-				tid_info->state = CARL9170_TID_STATE_XMIT;
-			} else {
-				goto processed;
-			}
-		}
+		if (tid_info->state != CARL9170_TID_STATE_XMIT)
+			goto processed;
 
 		tid_info->counter++;
 		first = skb_peek(&tid_info->queue);
-- 
1.7.1

[PATCH 4/6] carl9170: fix tx_ampdu_upload counter

[Christian Lamparter]

tx_ampdu_upload was not decreased when an a-MPDU
frame had to be kicked out from the tx_pending
queues.

This broke ampdu aggregation, because the scheduler
waits until tx_ampdu_upload drops to zero, before
making the next aggregate.

Signed-off-by: Christian Lamparter <chunkeey@googlemail.com>
---
 drivers/net/wireless/ath/carl9170/main.c |   15 ++++++++++++++-
 1 files changed, 14 insertions(+), 1 deletions(-)

diff --git a/drivers/net/wireless/ath/carl9170/main.c b/drivers/net/wireless/ath/carl9170/main.c
index d28b4ff..a8b0cec 100644
--- a/drivers/net/wireless/ath/carl9170/main.c
+++ b/drivers/net/wireless/ath/carl9170/main.c
@@ -230,8 +230,15 @@ static void carl9170_flush(struct ar9170 *ar, bool drop_queued)
 		for (i = 0; i < ar->hw->queues; i++) {
 			struct sk_buff *skb;
 
-			while ((skb = skb_dequeue(&ar->tx_pending[i])))
+			while ((skb = skb_dequeue(&ar->tx_pending[i]))) {
+				struct ieee80211_tx_info *info;
+
+				info = IEEE80211_SKB_CB(skb);
+				if (info->flags & IEEE80211_TX_CTL_AMPDU)
+					atomic_dec(&ar->tx_ampdu_upload);
+
 				carl9170_tx_status(ar, skb, false);
+			}
 		}
 	}
 
@@ -1462,6 +1469,7 @@ static void carl9170_op_sta_notify(struct ieee80211_hw *hw,
 			skb_queue_walk_safe(&ar->tx_pending[i], skb, tmp) {
 				struct _carl9170_tx_superframe *super;
 				struct ieee80211_hdr *hdr;
+				struct ieee80211_tx_info *info;
 
 				super = (void *) skb->data;
 				hdr = (void *) super->frame_data;
@@ -1470,6 +1478,11 @@ static void carl9170_op_sta_notify(struct ieee80211_hw *hw,
 					continue;
 
 				__skb_unlink(skb, &ar->tx_pending[i]);
+
+				info = IEEE80211_SKB_CB(skb);
+				if (info->flags & IEEE80211_TX_CTL_AMPDU)
+					atomic_dec(&ar->tx_ampdu_upload);
+
 				carl9170_tx_status(ar, skb, false);
 			}
 			spin_unlock_bh(&ar->tx_pending[i].lock);
-- 
1.7.1

[PATCH 5/6] carl9170: fix WARN_ON triggered by Broadcom HT STAs

[Christian Lamparter]

Broadcom's Windows driver for the 4313 advertises
an ampdu density of 7 => 16 us. The AR9170 MAC on
the other hand only supports densities up to 8 us.

This patch removes the noisy WARN_ON, because
there is nothing we can do about it.

Signed-off-by: Christian Lamparter <chunkeey@googlemail.com>
---
 drivers/net/wireless/ath/carl9170/main.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/net/wireless/ath/carl9170/main.c b/drivers/net/wireless/ath/carl9170/main.c
index a8b0cec..84bd38e 100644
--- a/drivers/net/wireless/ath/carl9170/main.c
+++ b/drivers/net/wireless/ath/carl9170/main.c
@@ -1248,7 +1248,7 @@ static int carl9170_op_ampdu_action(struct ieee80211_hw *hw,
 
 	switch (action) {
 	case IEEE80211_AMPDU_TX_START:
-		if (WARN_ON_ONCE(!sta_info->ht_sta))
+		if (!sta_info->ht_sta)
 			return -EOPNOTSUPP;
 
 		rcu_read_lock();
-- 
1.7.1

[PATCH 6/6] carl9170: revamp carl9170_tx_prepare

[Christian Lamparter]

David Miller complained about the driver's excessive use
of variables in __packed structs. While I did not fully
agree with his sole "performance" argument on all accounts.
I do see some room for improvement in hot-paths on
architectures without an efficient access to unaligned
elements.

This first patch (dare I say?) optimizes an important tx
hot-path in the driver: carl9170_tx_prepare.

Signed-off-by: Christian Lamparter <chunkeey@googlemail.com>
---
 drivers/net/wireless/ath/carl9170/tx.c |  155 +++++++++++++++----------------
 1 files changed, 75 insertions(+), 80 deletions(-)

diff --git a/drivers/net/wireless/ath/carl9170/tx.c b/drivers/net/wireless/ath/carl9170/tx.c
index e6be5e6..b575c86 100644
--- a/drivers/net/wireless/ath/carl9170/tx.c
+++ b/drivers/net/wireless/ath/carl9170/tx.c
@@ -760,8 +760,8 @@ static int carl9170_tx_prepare(struct ar9170 *ar, struct sk_buff *skb)
 	struct carl9170_tx_info *arinfo;
 	unsigned int hw_queue;
 	int i;
-	u16 keytype = 0;
-	u16 len, icv = 0;
+	__le16 mac_tmp;
+	u16 len;
 	bool ampdu, no_ack;
 
 	BUILD_BUG_ON(sizeof(*arinfo) > sizeof(info->rate_driver_data));
@@ -773,6 +773,10 @@ static int carl9170_tx_prepare(struct ar9170 *ar, struct sk_buff *skb)
 
 	BUILD_BUG_ON(IEEE80211_TX_MAX_RATES < CARL9170_TX_MAX_RATES);
 
+	BUILD_BUG_ON(AR9170_MAX_VIRTUAL_MAC >
+		((CARL9170_TX_SUPER_MISC_VIF_ID >>
+		 CARL9170_TX_SUPER_MISC_VIF_ID_S) + 1));
+
 	hw_queue = ar9170_qmap[carl9170_get_queue(ar, skb)];
 
 	hdr = (void *)skb->data;
@@ -793,20 +797,37 @@ static int carl9170_tx_prepare(struct ar9170 *ar, struct sk_buff *skb)
 	txc = (void *)skb_push(skb, sizeof(*txc));
 	memset(txc, 0, sizeof(*txc));
 
-	ampdu = !!(info->flags & IEEE80211_TX_CTL_AMPDU);
+	SET_VAL(CARL9170_TX_SUPER_MISC_QUEUE, txc->s.misc, hw_queue);
+
+	if (likely(cvif))
+		SET_VAL(CARL9170_TX_SUPER_MISC_VIF_ID, txc->s.misc, cvif->id);
+
+	if (unlikely(info->flags & IEEE80211_TX_CTL_SEND_AFTER_DTIM))
+		txc->s.misc |= CARL9170_TX_SUPER_MISC_CAB;
+
+	if (unlikely(ieee80211_is_probe_resp(hdr->frame_control)))
+		txc->s.misc |= CARL9170_TX_SUPER_MISC_FILL_IN_TSF;
+
+	mac_tmp = cpu_to_le16(AR9170_TX_MAC_HW_DURATION |
+			      AR9170_TX_MAC_BACKOFF);
+	mac_tmp |= cpu_to_le16((hw_queue << AR9170_TX_MAC_QOS_S) &&
+			       AR9170_TX_MAC_QOS);
+
 	no_ack = !!(info->flags & IEEE80211_TX_CTL_NO_ACK);
+	if (unlikely(no_ack))
+		mac_tmp |= cpu_to_le16(AR9170_TX_MAC_NO_ACK);
 
 	if (info->control.hw_key) {
-		icv = info->control.hw_key->icv_len;
+		len += info->control.hw_key->icv_len;
 
 		switch (info->control.hw_key->cipher) {
 		case WLAN_CIPHER_SUITE_WEP40:
 		case WLAN_CIPHER_SUITE_WEP104:
 		case WLAN_CIPHER_SUITE_TKIP:
-			keytype = AR9170_TX_MAC_ENCR_RC4;
+			mac_tmp |= cpu_to_le16(AR9170_TX_MAC_ENCR_RC4);
 			break;
 		case WLAN_CIPHER_SUITE_CCMP:
-			keytype = AR9170_TX_MAC_ENCR_AES;
+			mac_tmp |= cpu_to_le16(AR9170_TX_MAC_ENCR_AES);
 			break;
 		default:
 			WARN_ON(1);
@@ -814,48 +835,58 @@ static int carl9170_tx_prepare(struct ar9170 *ar, struct sk_buff *skb)
 		}
 	}
 
-	BUILD_BUG_ON(AR9170_MAX_VIRTUAL_MAC >
-		((CARL9170_TX_SUPER_MISC_VIF_ID >>
-		 CARL9170_TX_SUPER_MISC_VIF_ID_S) + 1));
-
-	txc->s.len = cpu_to_le16(len + sizeof(*txc));
-	txc->f.length = cpu_to_le16(len + icv + 4);
-	SET_VAL(CARL9170_TX_SUPER_MISC_VIF_ID, txc->s.misc,
-		cvif ? cvif->id : 0);
+	ampdu = !!(info->flags & IEEE80211_TX_CTL_AMPDU);
+	if (ampdu) {
+		unsigned int density, factor;
 
-	txc->f.mac_control = cpu_to_le16(AR9170_TX_MAC_HW_DURATION |
-					 AR9170_TX_MAC_BACKOFF);
+		if (unlikely(!sta || !cvif))
+			goto err_out;
 
-	SET_VAL(CARL9170_TX_SUPER_MISC_QUEUE, txc->s.misc, hw_queue);
+		factor = min_t(unsigned int, 1u,
+			 info->control.sta->ht_cap.ampdu_factor);
 
-	txc->f.mac_control |= cpu_to_le16(hw_queue << AR9170_TX_MAC_QOS_S);
-	txc->f.mac_control |= cpu_to_le16(keytype);
-	txc->f.phy_control = cpu_to_le32(0);
+		density = info->control.sta->ht_cap.ampdu_density;
 
-	if (no_ack)
-		txc->f.mac_control |= cpu_to_le16(AR9170_TX_MAC_NO_ACK);
+		if (density) {
+			/*
+			 * Watch out!
+			 *
+			 * Otus uses slightly different density values than
+			 * those from the 802.11n spec.
+			 */
 
-	if (info->flags & IEEE80211_TX_CTL_SEND_AFTER_DTIM)
-		txc->s.misc |= CARL9170_TX_SUPER_MISC_CAB;
+			density = max_t(unsigned int, density + 1, 7u);
+		}
 
-	txrate = &info->control.rates[0];
-	if (carl9170_tx_rts_check(ar, txrate, ampdu, no_ack))
-		txc->f.mac_control |= cpu_to_le16(AR9170_TX_MAC_PROT_RTS);
-	else if (carl9170_tx_cts_check(ar, txrate))
-		txc->f.mac_control |= cpu_to_le16(AR9170_TX_MAC_PROT_CTS);
+		SET_VAL(CARL9170_TX_SUPER_AMPDU_DENSITY,
+			txc->s.ampdu_settings, density);
 
-	SET_VAL(CARL9170_TX_SUPER_RI_TRIES, txc->s.ri[0], txrate->count);
-	txc->f.phy_control |= carl9170_tx_physet(ar, info, txrate);
+		SET_VAL(CARL9170_TX_SUPER_AMPDU_FACTOR,
+			txc->s.ampdu_settings, factor);
 
-	if (info->flags & IEEE80211_TX_CTL_AMPDU) {
-		for (i = 1; i < CARL9170_TX_MAX_RATES; i++) {
+		for (i = 0; i < CARL9170_TX_MAX_RATES; i++) {
 			txrate = &info->control.rates[i];
-			if (txrate->idx >= 0)
+			if (txrate->idx >= 0) {
+				txc->s.ri[i] =
+					CARL9170_TX_SUPER_RI_AMPDU;
+
+				if (WARN_ON(!(txrate->flags &
+					      IEEE80211_TX_RC_MCS))) {
+					/*
+					 * Not sure if it's even possible
+					 * to aggregate non-ht rates with
+					 * this HW.
+					 */
+					goto err_out;
+				}
 				continue;
+			}
 
 			txrate->idx = 0;
 			txrate->count = ar->hw->max_rate_tries;
 		}
+
+		mac_tmp |= cpu_to_le16(AR9170_TX_MAC_AGGR);
 	}
 
 	/*
@@ -878,57 +909,21 @@ static int carl9170_tx_prepare(struct ar9170 *ar, struct sk_buff *skb)
 			txc->s.ri[i] |= (AR9170_TX_MAC_PROT_CTS <<
 				CARL9170_TX_SUPER_RI_ERP_PROT_S);
 
-		/*
-		 * unaggregated fallback, in case aggregation
-		 * proves to be unsuccessful and unreliable.
-		 */
-		if (ampdu && i < 3)
-			txc->s.ri[i] |= CARL9170_TX_SUPER_RI_AMPDU;
-
 		txc->s.rr[i - 1] = carl9170_tx_physet(ar, info, txrate);
 	}
 
-	if (ieee80211_is_probe_resp(hdr->frame_control))
-		txc->s.misc |= CARL9170_TX_SUPER_MISC_FILL_IN_TSF;
-
-	if (ampdu) {
-		unsigned int density, factor;
-
-		if (unlikely(!sta || !cvif))
-			goto err_out;
-
-		density = info->control.sta->ht_cap.ampdu_density;
-		factor = info->control.sta->ht_cap.ampdu_factor;
-
-		if (density) {
-			/*
-			 * Watch out!
-			 *
-			 * Otus uses slightly different density values than
-			 * those from the 802.11n spec.
-			 */
-
-			density = max_t(unsigned int, density + 1, 7u);
-		}
-
-		factor = min_t(unsigned int, 1u, factor);
-
-		SET_VAL(CARL9170_TX_SUPER_AMPDU_DENSITY,
-			txc->s.ampdu_settings, density);
+	txrate = &info->control.rates[0];
+	SET_VAL(CARL9170_TX_SUPER_RI_TRIES, txc->s.ri[0], txrate->count);
 
-		SET_VAL(CARL9170_TX_SUPER_AMPDU_FACTOR,
-			txc->s.ampdu_settings, factor);
+	if (carl9170_tx_rts_check(ar, txrate, ampdu, no_ack))
+		mac_tmp |= cpu_to_le16(AR9170_TX_MAC_PROT_RTS);
+	else if (carl9170_tx_cts_check(ar, txrate))
+		mac_tmp |= cpu_to_le16(AR9170_TX_MAC_PROT_CTS);
 
-		if (info->control.rates[0].flags & IEEE80211_TX_RC_MCS) {
-			txc->f.mac_control |= cpu_to_le16(AR9170_TX_MAC_AGGR);
-		} else {
-			/*
-			 * Not sure if it's even possible to aggregate
-			 * non-ht rates with this HW.
-			 */
-			WARN_ON_ONCE(1);
-		}
-	}
+	txc->s.len = cpu_to_le16(skb->len);
+	txc->f.length = cpu_to_le16(len + FCS_LEN);
+	txc->f.mac_control = mac_tmp;
+	txc->f.phy_control = carl9170_tx_physet(ar, info, txrate);
 
 	arinfo = (void *)info->rate_driver_data;
 	arinfo->timeout = jiffies;
-- 
1.7.1

[PATCH 0/6] carl9170: updates

[Christian Lamparter]

A few more random changes.
Note: I do have some problems with -rc5, so this series
might need some external testing.

Christian Lamparter (6):
  carl9170: remove non-standard ba session teardown
  carl9170: angle brackets for wiki link
  carl9170: remove lost-frame workaround
  carl9170: fix tx_ampdu_upload counter
  carl9170: fix WARN_ON triggered by Broadcom HT STAs
  carl9170: revamp carl9170_tx_prepare

 drivers/net/wireless/ath/carl9170/Kconfig    |    2 +-
 drivers/net/wireless/ath/carl9170/carl9170.h |    1 -
 drivers/net/wireless/ath/carl9170/debug.c    |    4 -
 drivers/net/wireless/ath/carl9170/main.c     |   20 ++-
 drivers/net/wireless/ath/carl9170/tx.c       |  192 ++++++++++---------------
 5 files changed, 93 insertions(+), 126 deletions(-)

Re: [PATCH 0/6] carl9170: updates

[John W. Linville]

On Mon, Sep 27, 2010 at 01:41:36AM +0200, Christian Lamparter wrote:
> A few more random changes.
> Note: I do have some problems with -rc5, so this series
> might need some external testing.
> 
> Christian Lamparter (6):
>   carl9170: remove non-standard ba session teardown
>   carl9170: angle brackets for wiki link
>   carl9170: remove lost-frame workaround
>   carl9170: fix tx_ampdu_upload counter
>   carl9170: fix WARN_ON triggered by Broadcom HT STAs
>   carl9170: revamp carl9170_tx_prepare

Nothing to address davem's _packed comments?

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

Re: [PATCH 0/6] carl9170: updates

[John W. Linville]

On Tue, Sep 28, 2010 at 03:40:41PM -0400, John W. Linville wrote:
> On Mon, Sep 27, 2010 at 01:41:36AM +0200, Christian Lamparter wrote:
> > A few more random changes.
> > Note: I do have some problems with -rc5, so this series
> > might need some external testing.
> > 
> > Christian Lamparter (6):
> >   carl9170: remove non-standard ba session teardown
> >   carl9170: angle brackets for wiki link
> >   carl9170: remove lost-frame workaround
> >   carl9170: fix tx_ampdu_upload counter
> >   carl9170: fix WARN_ON triggered by Broadcom HT STAs
> >   carl9170: revamp carl9170_tx_prepare
> 
> Nothing to address davem's _packed comments?

Please ignore this, I found them. :-)

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