ath10k: fixes

PATCH New Michal Kazior 2 participants 10 messages Jun 24, 2013 – Jul 5, 2013 Qualcomm Atheros ATH10K wireless driver

Share & tools

Michal Kazior

author

2013-06-24 07:52 UTC #0 lore raw

cover ath10k: fixes

Michal Kazior (3):
ath10k: prevent HTC from being used after stopping
ath10k: fix 5ghz channel definitions
ath10k: leave MMIC generation to the HW

 drivers/net/wireless/ath/ath10k/htc.c |   28 +++++++++----------
 drivers/net/wireless/ath/ath10k/htc.h |    4 +--
 drivers/net/wireless/ath/ath10k/mac.c |   49 ++++++++++++++++-----------------
 3 files changed, 39 insertions(+), 42 deletions(-)

Signature · 1 line

1.7.9.5

Michal Kazior

author

2013-06-24 07:52 UTC #1 lore raw

PATCH 1/3 ath10k: prevent HTC from being used after stopping

It was possible to submit new HTC commands
after/while HTC stopped. This led to memory
corruption in some rare cases.

Signed-off-by Michal Kazior <[email protected]>

---
 drivers/net/wireless/ath/ath10k/htc.c |   28 +++++++++++++---------------
 drivers/net/wireless/ath/ath10k/htc.h |    4 ++--
 2 files changed, 15 insertions(+), 17 deletions(-)

drivers/net/wireless/ath/ath10k/htc.c +13 ~~-15~~

index 7d5a366..f5610f1 100644
@@ -251,10 +251,14 @@ int ath10k_htc_send(struct ath10k_htc *htc,
 		return -ENOENT;
 	}
 
-	skb_push(skb, sizeof(struct ath10k_htc_hdr));
-
 	spin_lock_bh(&htc->tx_lock);
+	if (htc->stopped) {
+		spin_unlock_bh(&htc->tx_lock);
+		return -ESHUTDOWN;
+	}
+
 	__skb_queue_tail(&ep->tx_queue, skb);
+	skb_push(skb, sizeof(struct ath10k_htc_hdr));
 	spin_unlock_bh(&htc->tx_lock);
 
 	queue_work(htc->ar->workqueue, &ep->send_work);

@@ -267,23 +271,17 @@ static int ath10k_htc_tx_completion_handler(struct ath10k *ar,
 {
 	struct ath10k_htc *htc = &ar->htc;
 	struct ath10k_htc_ep *ep = &htc->endpoint[eid];
-	bool stopping;
 
 	ath10k_htc_notify_tx_completion(ep, skb);
 	/* the skb now belongs to the completion handler */
 
+	/* note: when using TX credit flow, the re-checking of queues happens
+	 * when credits flow back from the target.  in the non-TX credit case,
+	 * we recheck after the packet completes */
 	spin_lock_bh(&htc->tx_lock);
-	stopping = htc->stopping;
-	spin_unlock_bh(&htc->tx_lock);
-
-	if (!ep->tx_credit_flow_enabled && !stopping)
-		/*
-		 * note: when using TX credit flow, the re-checking of
-		 * queues happens when credits flow back from the target.
-		 * in the non-TX credit case, we recheck after the packet
-		 * completes
-		 */
+	if (!ep->tx_credit_flow_enabled && !htc->stopped)
 		queue_work(ar->workqueue, &ep->send_work);
+	spin_unlock_bh(&htc->tx_lock);
 
 	return 0;
 }

@@ -948,7 +946,7 @@ void ath10k_htc_stop(struct ath10k_htc *htc)
 	struct ath10k_htc_ep *ep;
 
 	spin_lock_bh(&htc->tx_lock);
-	htc->stopping = true;
+	htc->stopped = true;
 	spin_unlock_bh(&htc->tx_lock);
 
 	for (i = ATH10K_HTC_EP_0; i < ATH10K_HTC_EP_COUNT; i++) {

@@ -957,7 +955,6 @@ void ath10k_htc_stop(struct ath10k_htc *htc)
 	}
 
 	ath10k_hif_stop(htc->ar);
-	ath10k_htc_reset_endpoint_states(htc);
 }
 
 /* registered target arrival callback from the HIF layer */

@@ -969,6 +966,7 @@ int ath10k_htc_init(struct ath10k *ar)
 
 	spin_lock_init(&htc->tx_lock);
 
+	htc->stopped = false;
 	ath10k_htc_reset_endpoint_states(htc);
 
 	/* setup HIF layer callbacks */

drivers/net/wireless/ath/ath10k/htc.h +2 -2

index 1606c9f..e1dd8c7 100644
@@ -335,7 +335,7 @@ struct ath10k_htc {
 	struct ath10k *ar;
 	struct ath10k_htc_ep endpoint[ATH10K_HTC_EP_COUNT];
 
-	/* protects endpoint and stopping fields */
+	/* protects endpoint and stopped fields */
 	spinlock_t tx_lock;
 
 	struct ath10k_htc_ops htc_ops;

@@ -349,7 +349,7 @@ struct ath10k_htc {
 	struct ath10k_htc_svc_tx_credits service_tx_alloc[ATH10K_HTC_EP_COUNT];
 	int target_credit_size;
 
-	bool stopping;
+	bool stopped;
 };
 
 int ath10k_htc_init(struct ath10k *ar);

Signature · 1 line

1.7.9.5

Michal Kazior

author

2013-06-24 07:52 UTC #2 lore raw

PATCH 2/3 ath10k: fix 5ghz channel definitions

In reply to Michal Kazior

Nonsense channel flags were being set.

Signed-off-by Michal Kazior <[email protected]>

---
 drivers/net/wireless/ath/ath10k/mac.c |   48 ++++++++++++++++-----------------
 1 file changed, 24 insertions(+), 24 deletions(-)

drivers/net/wireless/ath/ath10k/mac.c +24 ~~-24~~

index 4867f30..441a3e7 100644
@@ -2798,30 +2798,30 @@ static const struct ieee80211_channel ath10k_2ghz_channels[] = {
 };
 
 static const struct ieee80211_channel ath10k_5ghz_channels[] = {
-	CHAN5G(36, 5180, 14),
-	CHAN5G(40, 5200, 15),
-	CHAN5G(44, 5220, 16),
-	CHAN5G(48, 5240, 17),
-	CHAN5G(52, 5260, 18),
-	CHAN5G(56, 5280, 19),
-	CHAN5G(60, 5300, 20),
-	CHAN5G(64, 5320, 21),
-	CHAN5G(100, 5500, 22),
-	CHAN5G(104, 5520, 23),
-	CHAN5G(108, 5540, 24),
-	CHAN5G(112, 5560, 25),
-	CHAN5G(116, 5580, 26),
-	CHAN5G(120, 5600, 27),
-	CHAN5G(124, 5620, 28),
-	CHAN5G(128, 5640, 29),
-	CHAN5G(132, 5660, 30),
-	CHAN5G(136, 5680, 31),
-	CHAN5G(140, 5700, 32),
-	CHAN5G(149, 5745, 33),
-	CHAN5G(153, 5765, 34),
-	CHAN5G(157, 5785, 35),
-	CHAN5G(161, 5805, 36),
-	CHAN5G(165, 5825, 37),
+	CHAN5G(36, 5180, 0),
+	CHAN5G(40, 5200, 0),
+	CHAN5G(44, 5220, 0),
+	CHAN5G(48, 5240, 0),
+	CHAN5G(52, 5260, 0),
+	CHAN5G(56, 5280, 0),
+	CHAN5G(60, 5300, 0),
+	CHAN5G(64, 5320, 0),
+	CHAN5G(100, 5500, 0),
+	CHAN5G(104, 5520, 0),
+	CHAN5G(108, 5540, 0),
+	CHAN5G(112, 5560, 0),
+	CHAN5G(116, 5580, 0),
+	CHAN5G(120, 5600, 0),
+	CHAN5G(124, 5620, 0),
+	CHAN5G(128, 5640, 0),
+	CHAN5G(132, 5660, 0),
+	CHAN5G(136, 5680, 0),
+	CHAN5G(140, 5700, 0),
+	CHAN5G(149, 5745, 0),
+	CHAN5G(153, 5765, 0),
+	CHAN5G(157, 5785, 0),
+	CHAN5G(161, 5805, 0),
+	CHAN5G(165, 5825, 0),
 };
 
 static struct ieee80211_rate ath10k_rates[] = {

Signature · 1 line

1.7.9.5

Kalle Valo

2013-06-25 19:56 UTC #3 lore raw

PATCH 2/3 ath10k: fix 5ghz channel definitions

Quoted 3 lines · Michal Kazior

Nonsense channel flags were being set.
Signed-off-by Michal Kazior <[email protected]>

What user visible bug does this fix? Or is it just code cleanup?

Signature · 1 line

Kalle Valo

Michal Kazior

author

2013-06-26 06:54 UTC #4 lore raw

PATCH v2 1/1 ath10k: fix 5ghz channel definitions

In reply to Michal Kazior

Nonsense channel flags were being set.

Although it doesn't seem this was visible to the
user the patch makes sure that channel
availability won't be crippled in the future if
ath_common behaviour changes.

Signed-off-by Michal Kazior <[email protected]>

---
 drivers/net/wireless/ath/ath10k/mac.c |   48 ++++++++++++++++-----------------
 1 file changed, 24 insertions(+), 24 deletions(-)

drivers/net/wireless/ath/ath10k/mac.c +24 ~~-24~~

index 4867f30..441a3e7 100644
@@ -2798,30 +2798,30 @@ static const struct ieee80211_channel ath10k_2ghz_channels[] = {
 };
 
 static const struct ieee80211_channel ath10k_5ghz_channels[] = {
-	CHAN5G(36, 5180, 14),
-	CHAN5G(40, 5200, 15),
-	CHAN5G(44, 5220, 16),
-	CHAN5G(48, 5240, 17),
-	CHAN5G(52, 5260, 18),
-	CHAN5G(56, 5280, 19),
-	CHAN5G(60, 5300, 20),
-	CHAN5G(64, 5320, 21),
-	CHAN5G(100, 5500, 22),
-	CHAN5G(104, 5520, 23),
-	CHAN5G(108, 5540, 24),
-	CHAN5G(112, 5560, 25),
-	CHAN5G(116, 5580, 26),
-	CHAN5G(120, 5600, 27),
-	CHAN5G(124, 5620, 28),
-	CHAN5G(128, 5640, 29),
-	CHAN5G(132, 5660, 30),
-	CHAN5G(136, 5680, 31),
-	CHAN5G(140, 5700, 32),
-	CHAN5G(149, 5745, 33),
-	CHAN5G(153, 5765, 34),
-	CHAN5G(157, 5785, 35),
-	CHAN5G(161, 5805, 36),
-	CHAN5G(165, 5825, 37),
+	CHAN5G(36, 5180, 0),
+	CHAN5G(40, 5200, 0),
+	CHAN5G(44, 5220, 0),
+	CHAN5G(48, 5240, 0),
+	CHAN5G(52, 5260, 0),
+	CHAN5G(56, 5280, 0),
+	CHAN5G(60, 5300, 0),
+	CHAN5G(64, 5320, 0),
+	CHAN5G(100, 5500, 0),
+	CHAN5G(104, 5520, 0),
+	CHAN5G(108, 5540, 0),
+	CHAN5G(112, 5560, 0),
+	CHAN5G(116, 5580, 0),
+	CHAN5G(120, 5600, 0),
+	CHAN5G(124, 5620, 0),
+	CHAN5G(128, 5640, 0),
+	CHAN5G(132, 5660, 0),
+	CHAN5G(136, 5680, 0),
+	CHAN5G(140, 5700, 0),
+	CHAN5G(149, 5745, 0),
+	CHAN5G(153, 5765, 0),
+	CHAN5G(157, 5785, 0),
+	CHAN5G(161, 5805, 0),
+	CHAN5G(165, 5825, 0),
 };
 
 static struct ieee80211_rate ath10k_rates[] = {

Signature · 1 line

1.7.9.5

Kalle Valo

2013-07-05 13:24 UTC #5 lore raw

PATCH v2 1/1 ath10k: fix 5ghz channel definitions

Quoted 8 lines · Michal Kazior

Nonsense channel flags were being set.
Although it doesn't seem this was visible to the
user the patch makes sure that channel
availability won't be crippled in the future if
ath_common behaviour changes.
Signed-off-by Michal Kazior <[email protected]>

John also applied this already.

Signature · 1 line

Kalle Valo

Michal Kazior

author

2013-06-24 07:52 UTC #6 lore raw

PATCH 3/3 ath10k: leave MMIC generation to the HW

In reply to Michal Kazior

Apparently HW doesn't require us to generate MMIC
for TKIP suite.

This fixes issues with some APs.

Signed-off-by Michal Kazior <[email protected]>

---
 drivers/net/wireless/ath/ath10k/mac.c |    1 -
 1 file changed, 1 deletion(-)

drivers/net/wireless/ath/ath10k/mac.c +0 -1

index 441a3e7..3033cf1 100644
@@ -56,7 +56,6 @@ static int ath10k_send_key(struct ath10k_vif *arvif,
 		key->flags |= IEEE80211_KEY_FLAG_SW_MGMT_TX;
 		break;
 	case WLAN_CIPHER_SUITE_TKIP:
-		key->flags |= IEEE80211_KEY_FLAG_GENERATE_MMIC;
 		arg.key_cipher = WMI_CIPHER_TKIP;
 		arg.key_txmic_len = 8;
 		arg.key_rxmic_len = 8;

Signature · 1 line

1.7.9.5

Kalle Valo

2013-06-25 19:59 UTC #7 lore raw

PATCH 3/3 ath10k: leave MMIC generation to the HW

Quoted 6 lines · Michal Kazior

Apparently HW doesn't require us to generate MMIC
for TKIP suite.
This fixes issues with some APs.
Signed-off-by Michal Kazior <[email protected]>

Please describe the user visible bug.

Signature · 1 line

Kalle Valo

Michal Kazior

author

2013-06-26 06:57 UTC #8 lore raw

PATCH v2 1/1 ath10k: leave MMIC generation to the HW

In reply to Michal Kazior

Apparently HW doesn't require us to generate MMIC for TKIP suite.

Each frame was 8 bytes longer than it should be and some APs would drop frames that exceed 1520 bytes of 802.11 payload. This could be observed during throughput tests or fragmented IP traffic.

Signed-off-by Michal Kazior <[email protected]>

---
 drivers/net/wireless/ath/ath10k/mac.c |    1 -
 1 file changed, 1 deletion(-)

drivers/net/wireless/ath/ath10k/mac.c +0 -1

index 4867f30..bfee06d 100644
@@ -56,7 +56,6 @@ static int ath10k_send_key(struct ath10k_vif *arvif,
 		key->flags |= IEEE80211_KEY_FLAG_SW_MGMT_TX;
 		break;
 	case WLAN_CIPHER_SUITE_TKIP:
-		key->flags |= IEEE80211_KEY_FLAG_GENERATE_MMIC;
 		arg.key_cipher = WMI_CIPHER_TKIP;
 		arg.key_txmic_len = 8;
 		arg.key_rxmic_len = 8;

Signature · 1 line

1.7.9.5

Kalle Valo

2013-07-05 13:25 UTC #9 lore raw

PATCH v2 1/1 ath10k: leave MMIC generation to the HW

Quoted 9 lines · Michal Kazior

Apparently HW doesn't require us to generate MMIC for TKIP suite.
Each frame was 8 bytes longer than it should be and some APs would drop frames that exceed 1520 bytes of 802.11 payload. This could be observed during throughput tests or fragmented IP traffic.
Signed-off-by Michal Kazior <[email protected]>

This is also applied.

Signature · 1 line

Kalle Valo