Skip to contentSkip to conversation

pull request: iwlwifi 2014-02-03

PATCH In review Emmanuel Grumbach 4 participants 14 messages Feb 3, 2014 – Feb 10, 2014 Networking drivers (wireless)

lore
Share & tools
lore
#0 lore raw

Hi John,

since you updated your trees, here is a set of fixes for 3.14.

I have an important fix that disables A band in case the driver thought it was enabled, and the firmware disagreed. We ended up making the firmware unhappy. I also fix the station table in AP mode and fix the scan while we have BT working.
Johannes removes a static variable that could potentially lead to to issues on multi-device setups and disables scheduled scan to avoid issues with old versions of wpa_supplicant.
A small fix from David on scan and a few new device IDs for 7265.

Let me know about issues you may have!

The following changes since commit 9b0cd304f26b9fca140de15deeac2bf357d1f388:

Merge branch 'drm-next' of git://people.freedesktop.org/~airlied/linux
(2014-01-29 20:49:12 -0800)

are available in the git repository at:

git://git.kernel.org/pub/scm/linux/kernel/git/iwlwifi/iwlwifi-fixes.git
for-john

for you to fetch changes up to c512865446e6dd5b6e91e81187e75b734ad7cfc7:

iwlwifi: mvm: don't allow A band if SKU forbids it (2014-02-02
11:08:42 +0200)

----------------------------------------------------------------
David Spinadel (1):
iwlwifi: mvm: notify match found without filtering

Emmanuel Grumbach (4):
iwlwifi: mvm: print the version of the firmware when it asserts
iwlwifi: mvm: don't leak a station when we drain
iwlwifi: mvm: BT Coex - disable BT when TXing probe request in scan
iwlwifi: mvm: don't allow A band if SKU forbids it

Johannes Berg (2):
iwlwifi: mvm: make local pointer non-static
iwlwifi: mvm: disable scheduled scan

Oren Givon (1):
iwlwifi: add more 7265 HW IDs

 drivers/net/wireless/iwlwifi/iwl-nvm-parse.c   |    5 ++
 drivers/net/wireless/iwlwifi/mvm/fw-api-scan.h |    4 +-
 drivers/net/wireless/iwlwifi/mvm/mac80211.c    |    2 +-
 drivers/net/wireless/iwlwifi/mvm/scan.c        |    5 +-
 drivers/net/wireless/iwlwifi/mvm/sta.c         |    2 +-
 drivers/net/wireless/iwlwifi/mvm/tx.c          |   73
++++++++++++------------
 drivers/net/wireless/iwlwifi/mvm/utils.c       |    2 +
 drivers/net/wireless/iwlwifi/pcie/drv.c        |    7 ++-
 8 files changed, 59 insertions(+), 41 deletions(-)
#1 lore raw

PATCH 1/8 iwlwifi: mvm: make local pointer non-static

From: Johannes Berg <[email protected]>

The address pointer used in the function shouldn't be static since it's local data only. Having it static causes races if a single machine has two devices, as the pointer would be shared between instances.

---
 drivers/net/wireless/iwlwifi/mvm/sta.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/net/wireless/iwlwifi/mvm/sta.c +1 -1
index ec18121..3397f59 100644
@@ -652,7 +652,7 @@ int iwl_mvm_send_bcast_sta(struct iwl_mvm *mvm, struct ieee80211_vif *vif,
 {
 	struct iwl_mvm_vif *mvmvif = iwl_mvm_vif_from_mac80211(vif);
 	static const u8 _baddr[] = {0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF};
-	static const u8 *baddr = _baddr;
+	const u8 *baddr = _baddr;
 
 	lockdep_assert_held(&mvm->mutex);
 
Signature · 1 line
1.7.9.5
#2 lore raw

PATCH 2/8 iwlwifi: mvm: disable scheduled scan

In reply to Emmanuel Grumbach

From: Johannes Berg <[email protected]>

The iwlwifi scheduled scan implementation doesn't adhere to the userspace API correctly - the API assumes that any new incoming 'incompatible' request (like scan or remain-on-channel for this driver) will just cancel the scheduled scan. Instead our driver relies on userspace cancelling it, thus breaking existing wpa_s versions.

Fixes 35a000b7c1bb ("iwlwifi: mvm: support sched scan if supported by the fw")
---
 drivers/net/wireless/iwlwifi/mvm/mac80211.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/net/wireless/iwlwifi/mvm/mac80211.c +1 -1
index c49b507..6bf9766 100644
@@ -246,7 +246,7 @@ int iwl_mvm_mac_setup_register(struct iwl_mvm *mvm)
 	else
 		hw->wiphy->flags &= ~WIPHY_FLAG_PS_ON_BY_DEFAULT;
 
-	if (mvm->fw->ucode_capa.flags & IWL_UCODE_TLV_FLAGS_SCHED_SCAN) {
+	if (0 && mvm->fw->ucode_capa.flags & IWL_UCODE_TLV_FLAGS_SCHED_SCAN) {
 		hw->wiphy->flags |= WIPHY_FLAG_SUPPORTS_SCHED_SCAN;
 		hw->wiphy->max_sched_scan_ssids = PROBE_OPTION_MAX;
 		hw->wiphy->max_match_sets = IWL_SCAN_MAX_PROFILES;
Signature · 1 line
1.7.9.5
#3 lore raw

Re: [PATCH 2/8] iwlwifi: mvm: disable scheduled scan

Quoted 8 lines · Emmanuel Grumbach

From: Johannes Berg<[email protected]>

The iwlwifi scheduled scan implementation doesn't adhere to the userspace API correctly - the API assumes that any new incoming 'incompatible' request (like scan or remain-on-channel for this driver) will just cancel the scheduled scan. Instead our driver relies on userspace cancelling it, thus breaking existing wpa_s versions.

I think iwlwifi is not alone in this. At least I have to put a new task on my todo list ;-)

Regards,
Arend

maintainer
#4 lore raw

Re: [PATCH 2/8] iwlwifi: mvm: disable scheduled scan

Quoted 10 lines · Arend van Spriel
Quoted 8 lines · Emmanuel Grumbach

From: Johannes Berg<[email protected]>

The iwlwifi scheduled scan implementation doesn't adhere to the userspace API correctly - the API assumes that any new incoming 'incompatible' request (like scan or remain-on-channel for this driver) will just cancel the scheduled scan. Instead our driver relies on userspace cancelling it, thus breaking existing wpa_s versions.

Quoted 2 lines

I think iwlwifi is not alone in this. At least I have to put a new task on my todo list ;-)

:)
You may decide to not care - newer supplicant seems to always cancel scheduled scan first (although that should never have been required, Broadcom's proprietary Android driver also behaves to require that.)

However, you may find that you don't just need to support the latest upstream wpa_supplicant, so ...

johannes

#5 lore raw

Re: [PATCH 2/8] iwlwifi: mvm: disable scheduled scan

Quoted 11 lines · Johannes Berg
Quoted 10 lines · Arend van Spriel
Quoted 8 lines · Emmanuel Grumbach

From: Johannes Berg<[email protected]>

The iwlwifi scheduled scan implementation doesn't adhere to the userspace API correctly - the API assumes that any new incoming 'incompatible' request (like scan or remain-on-channel for this driver) will just cancel the scheduled scan. Instead our driver relies on userspace cancelling it, thus breaking existing wpa_s versions.

Quoted 7 lines
Quoted 2 lines

I think iwlwifi is not alone in this. At least I have to put a new task on my todo list ;-)

:)
You may decide to not care - newer supplicant seems to always cancel scheduled scan first (although that should never have been required, Broadcom's proprietary Android driver also behaves to require that.)

When implementing scheduled scan, I found our firmware can handle a regular scan regardless whether a scheduled scan is active or not.

Quoted 2 lines

However, you may find that you don't just need to support the latest upstream wpa_supplicant, so ...

Yeah, seems to happen more often than not. Also when people start using backports they are advised to upgrade user-space apps as well, but tend to stick to what the distro has, so ...

Quoted 1 line

johannes

Arend

maintainer
#6 lore raw

Re: [PATCH 2/8] iwlwifi: mvm: disable scheduled scan

Quoted 6 lines · Arend van Spriel
Quoted 3 lines

You may decide to not care - newer supplicant seems to always cancel scheduled scan first (although that should never have been required, Broadcom's proprietary Android driver also behaves to require that.)

When implementing scheduled scan, I found our firmware can handle a regular scan regardless whether a scheduled scan is active or not.

Interesting. I wonder why the other broadcom folks went to wpa_supplicant to make it cancel scheduled scan ... well, actually, I don't care all that much :-D

johannes

#7 lore raw

PATCH 3/8 iwlwifi: mvm: print the version of the firmware when it asserts

In reply to Emmanuel Grumbach

From: Emmanuel Grumbach <[email protected]>

This can be useful to be able to spot the firmware version from the error reports without needing to fetch it from another place.

---
 drivers/net/wireless/iwlwifi/mvm/utils.c |    2 ++
 1 file changed, 2 insertions(+)
drivers/net/wireless/iwlwifi/mvm/utils.c +2 -0
index a4a5e25..86989df 100644
@@ -411,6 +411,8 @@ void iwl_mvm_dump_nic_error_log(struct iwl_mvm *mvm)
 			mvm->status, table.valid);
 	}
 
+	IWL_ERR(mvm, "Loaded firmware version: %s\n", mvm->fw->fw_version);
+
 	trace_iwlwifi_dev_ucode_error(trans->dev, table.error_id, table.tsf_low,
 				      table.data1, table.data2, table.data3,
 				      table.blink1, table.blink2, table.ilink1,
Signature · 1 line
1.7.9.5
#8 lore raw

PATCH 4/8 iwlwifi: add more 7265 HW IDs

In reply to Emmanuel Grumbach

From: Oren Givon <[email protected]>

Add 6 new HW IDs for the 7265 series.

---
 drivers/net/wireless/iwlwifi/pcie/drv.c |    7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)
drivers/net/wireless/iwlwifi/pcie/drv.c +6 -1
index 3040924..f47bcbe 100644
@@ -359,20 +359,25 @@ static DEFINE_PCI_DEVICE_TABLE(iwl_hw_card_ids) = {
 /* 7265 Series */
 	{IWL_PCI_DEVICE(0x095A, 0x5010, iwl7265_2ac_cfg)},
 	{IWL_PCI_DEVICE(0x095A, 0x5110, iwl7265_2ac_cfg)},
+	{IWL_PCI_DEVICE(0x095A, 0x5112, iwl7265_2ac_cfg)},
+	{IWL_PCI_DEVICE(0x095A, 0x5100, iwl7265_2ac_cfg)},
+	{IWL_PCI_DEVICE(0x095A, 0x510A, iwl7265_2ac_cfg)},
 	{IWL_PCI_DEVICE(0x095B, 0x5310, iwl7265_2ac_cfg)},
 	{IWL_PCI_DEVICE(0x095B, 0x5302, iwl7265_2ac_cfg)},
 	{IWL_PCI_DEVICE(0x095B, 0x5210, iwl7265_2ac_cfg)},
 	{IWL_PCI_DEVICE(0x095A, 0x5012, iwl7265_2ac_cfg)},
-	{IWL_PCI_DEVICE(0x095A, 0x500A, iwl7265_2ac_cfg)},
 	{IWL_PCI_DEVICE(0x095A, 0x5410, iwl7265_2ac_cfg)},
 	{IWL_PCI_DEVICE(0x095A, 0x5400, iwl7265_2ac_cfg)},
 	{IWL_PCI_DEVICE(0x095A, 0x1010, iwl7265_2ac_cfg)},
 	{IWL_PCI_DEVICE(0x095A, 0x5000, iwl7265_2n_cfg)},
+	{IWL_PCI_DEVICE(0x095A, 0x500A, iwl7265_2n_cfg)},
 	{IWL_PCI_DEVICE(0x095B, 0x5200, iwl7265_2n_cfg)},
 	{IWL_PCI_DEVICE(0x095A, 0x5002, iwl7265_n_cfg)},
 	{IWL_PCI_DEVICE(0x095B, 0x5202, iwl7265_n_cfg)},
 	{IWL_PCI_DEVICE(0x095A, 0x9010, iwl7265_2ac_cfg)},
+	{IWL_PCI_DEVICE(0x095A, 0x9012, iwl7265_2ac_cfg)},
 	{IWL_PCI_DEVICE(0x095A, 0x9110, iwl7265_2ac_cfg)},
+	{IWL_PCI_DEVICE(0x095A, 0x9112, iwl7265_2ac_cfg)},
 	{IWL_PCI_DEVICE(0x095A, 0x9210, iwl7265_2ac_cfg)},
 	{IWL_PCI_DEVICE(0x095A, 0x9510, iwl7265_2ac_cfg)},
 	{IWL_PCI_DEVICE(0x095A, 0x9310, iwl7265_2ac_cfg)},
Signature · 1 line
1.7.9.5
#9 lore raw

PATCH 5/8 iwlwifi: mvm: notify match found without filtering

In reply to Emmanuel Grumbach

From: David Spinadel <[email protected]>

Configure scheduled scan to notify match found on every beacon or probe response if the scan request doesn't contain valid ssid list for filtering.
Without this configuration the FW passes all beacons to the host but doesn't notify the stack that the scan results are ready for processing.

---
 drivers/net/wireless/iwlwifi/mvm/fw-api-scan.h |    4 +++-
 drivers/net/wireless/iwlwifi/mvm/scan.c        |    2 ++
 2 files changed, 5 insertions(+), 1 deletion(-)
drivers/net/wireless/iwlwifi/mvm/fw-api-scan.h +3 -1
index 73cbba7..9426905 100644
@@ -504,6 +504,7 @@ struct iwl_scan_offload_profile {
  * @match_notify:	clients waiting for match found notification
  * @pass_match:		clients waiting for the results
  * @active_clients:	active clients bitmap - enum scan_framework_client
+ * @any_beacon_notify:	clients waiting for match notification without match
  */
 struct iwl_scan_offload_profile_cfg {
 	struct iwl_scan_offload_profile profiles[IWL_SCAN_MAX_PROFILES];
@@ -512,7 +513,8 @@ struct iwl_scan_offload_profile_cfg {
 	u8 match_notify;
 	u8 pass_match;
 	u8 active_clients;
-	u8 reserved[3];
+	u8 any_beacon_notify;
+	u8 reserved[2];
 } __packed;
 
 /**
drivers/net/wireless/iwlwifi/mvm/scan.c +2 -0
index 0e00079..c35f35c 100644
@@ -807,6 +807,8 @@ int iwl_mvm_config_sched_scan_profiles(struct iwl_mvm *mvm,
 	profile_cfg->active_clients = SCAN_CLIENT_SCHED_SCAN;
 	profile_cfg->pass_match = SCAN_CLIENT_SCHED_SCAN;
 	profile_cfg->match_notify = SCAN_CLIENT_SCHED_SCAN;
+	if (!req->n_match_sets || !req->match_sets[0].ssid.ssid_len)
+		profile_cfg->any_beacon_notify = SCAN_CLIENT_SCHED_SCAN;
 
 	for (i = 0; i < req->n_match_sets; i++) {
 		profile = &profile_cfg->profiles[i];
Signature · 1 line
1.7.9.5
#10 lore raw

PATCH 6/8 iwlwifi: mvm: don't leak a station when we drain

In reply to Emmanuel Grumbach

From: Emmanuel Grumbach <[email protected]>

We had a bug that prevented us from removing a station after we entered the drain flow:

We assign sta to be NULL if it was an error value. Then we tested it against -EBUSY, but forget to retrieve the value again from mvm->fw_id_to_mac_id[sta_id].

Due to this bug, we ended up never removing the STA from the firmware. This led to an firmware assert when we remove the GO vif.

---
 drivers/net/wireless/iwlwifi/mvm/tx.c |   73 +++++++++++++++++----------------
 1 file changed, 37 insertions(+), 36 deletions(-)
drivers/net/wireless/iwlwifi/mvm/tx.c +37 -36
index 90378c2..4df12fa 100644
@@ -659,8 +659,14 @@ static void iwl_mvm_rx_tx_cmd_single(struct iwl_mvm *mvm,
 	rcu_read_lock();
 
 	sta = rcu_dereference(mvm->fw_id_to_mac_id[sta_id]);
+	/*
+	 * sta can't be NULL otherwise it'd mean that the sta has been freed in
+	 * the firmware while we still have packets for it in the Tx queues.
+	 */
+	if (WARN_ON_ONCE(!sta))
+		goto out;
 
-	if (!IS_ERR_OR_NULL(sta)) {
+	if (!IS_ERR(sta)) {
 		mvmsta = iwl_mvm_sta_from_mac80211(sta);
 
 		if (tid != IWL_TID_NON_QOS) {
@@ -675,7 +681,6 @@ static void iwl_mvm_rx_tx_cmd_single(struct iwl_mvm *mvm,
 			spin_unlock_bh(&mvmsta->lock);
 		}
 	} else {
-		sta = NULL;
 		mvmsta = NULL;
 	}
 
@@ -683,42 +688,38 @@ static void iwl_mvm_rx_tx_cmd_single(struct iwl_mvm *mvm,
 	 * If the txq is not an AMPDU queue, there is no chance we freed
 	 * several skbs. Check that out...
 	 */
-	if (txq_id < mvm->first_agg_queue && !WARN_ON(skb_freed > 1) &&
-	    atomic_sub_and_test(skb_freed, &mvm->pending_frames[sta_id])) {
-		if (mvmsta) {
-			/*
-			 * If there are no pending frames for this STA, notify
-			 * mac80211 that this station can go to sleep in its
-			 * STA table.
-			 */
-			if (mvmsta->vif->type == NL80211_IFTYPE_AP)
-				ieee80211_sta_block_awake(mvm->hw, sta, false);
-			/*
-			 * We might very well have taken mvmsta pointer while
-			 * the station was being removed. The remove flow might
-			 * have seen a pending_frame (because we didn't take
-			 * the lock) even if now the queues are drained. So make
-			 * really sure now that this the station is not being
-			 * removed. If it is, run the drain worker to remove it.
-			 */
-			spin_lock_bh(&mvmsta->lock);
-			sta = rcu_dereference(mvm->fw_id_to_mac_id[sta_id]);
-			if (!sta || PTR_ERR(sta) == -EBUSY) {
-				/*
-				 * Station disappeared in the meantime:
-				 * so we are draining.
-				 */
-				set_bit(sta_id, mvm->sta_drained);
-				schedule_work(&mvm->sta_drained_wk);
-			}
-			spin_unlock_bh(&mvmsta->lock);
-		} else if (!mvmsta && PTR_ERR(sta) == -EBUSY) {
-			/* Tx response without STA, so we are draining */
-			set_bit(sta_id, mvm->sta_drained);
-			schedule_work(&mvm->sta_drained_wk);
-		}
+	if (txq_id >= mvm->first_agg_queue)
+		goto out;
+
+	/* We can't free more than one frame at once on a shared queue */
+	WARN_ON(skb_freed > 1);
+
+	/* If we have still frames from this STA nothing to do here */
+	if (!atomic_sub_and_test(skb_freed, &mvm->pending_frames[sta_id]))
+		goto out;
+
+	if (mvmsta && mvmsta->vif->type == NL80211_IFTYPE_AP) {
+		/*
+		 * If there are no pending frames for this STA, notify
+		 * mac80211 that this station can go to sleep in its
+		 * STA table.
+		 * If mvmsta is not NULL, sta is valid.
+		 */
+		ieee80211_sta_block_awake(mvm->hw, sta, false);
+	}
+
+	if (PTR_ERR(sta) == -EBUSY || PTR_ERR(sta) == -ENOENT) {
+		/*
+		 * We are draining and this was the last packet - pre_rcu_remove
+		 * has been called already. We might be after the
+		 * synchronize_net already.
+		 * Don't rely on iwl_mvm_rm_sta to see the empty Tx queues.
+		 */
+		set_bit(sta_id, mvm->sta_drained);
+		schedule_work(&mvm->sta_drained_wk);
 	}
 
+out:
 	rcu_read_unlock();
 }
 
Signature · 1 line
1.7.9.5
#11 lore raw

PATCH 7/8 iwlwifi: mvm: BT Coex - disable BT when TXing probe request in scan

In reply to Emmanuel Grumbach

From: Emmanuel Grumbach <[email protected]>

Not doing so will let BT kill our probe requests leading to failures in scan.

---
 drivers/net/wireless/iwlwifi/mvm/scan.c |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
drivers/net/wireless/iwlwifi/mvm/scan.c +2 -1
index c35f35c..742afc4 100644
@@ -344,7 +344,8 @@ int iwl_mvm_scan_request(struct iwl_mvm *mvm,
 
 	iwl_mvm_scan_fill_ssids(cmd, req, basic_ssid ? 1 : 0);
 
-	cmd->tx_cmd.tx_flags = cpu_to_le32(TX_CMD_FLG_SEQ_CTL);
+	cmd->tx_cmd.tx_flags = cpu_to_le32(TX_CMD_FLG_SEQ_CTL |
+					   TX_CMD_FLG_BT_DIS);
 	cmd->tx_cmd.sta_id = mvm->aux_sta.sta_id;
 	cmd->tx_cmd.life_time = cpu_to_le32(TX_CMD_LIFE_TIME_INFINITE);
 	cmd->tx_cmd.rate_n_flags =
Signature · 1 line
1.7.9.5
#12 lore raw

PATCH 8/8 iwlwifi: mvm: don't allow A band if SKU forbids it

In reply to Emmanuel Grumbach

From: Emmanuel Grumbach <[email protected]>

The driver wasn't reading the NVM properly. While this didn't lead to any issue until now, it seems that there is an old version of the NVM in the wild.
In this version, the A band channels appear to be valid but the SKU capabilities (another field of the NVM) says that A band isn't supported at all.
With this specific version of the NVM, the driver would think that A band is supported while the HW / firmware don't. This leads to asserts.

---
 drivers/net/wireless/iwlwifi/iwl-nvm-parse.c |    5 +++++
 1 file changed, 5 insertions(+)
drivers/net/wireless/iwlwifi/iwl-nvm-parse.c +5 -0
index f06f4cb..725e954 100644
@@ -182,6 +182,11 @@ static int iwl_init_channel_map(struct device *dev, const struct iwl_cfg *cfg,
 
 	for (ch_idx = 0; ch_idx < IWL_NUM_CHANNELS; ch_idx++) {
 		ch_flags = __le16_to_cpup(nvm_ch_flags + ch_idx);
+
+		if (ch_idx >= NUM_2GHZ_CHANNELS &&
+		    !data->sku_cap_band_52GHz_enable)
+			ch_flags &= ~NVM_CHANNEL_VALID;
+
 		if (!(ch_flags & NVM_CHANNEL_VALID)) {
 			IWL_DEBUG_EEPROM(dev,
 					 "Ch. %d Flags %x [%sGHz] - No traffic\n",
Signature · 1 line
1.7.9.5
#13 lore raw
In reply to Emmanuel Grumbach
Quoted 24 lines · Emmanuel Grumbach

Hi John,

since you updated your trees, here is a set of fixes for 3.14.

I have an important fix that disables A band in case the driver thought it was enabled, and the firmware disagreed. We ended up making the firmware unhappy. I also fix the station table in AP mode and fix the scan while we have BT working.
Johannes removes a static variable that could potentially lead to to issues on multi-device setups and disables scheduled scan to avoid issues with old versions of wpa_supplicant.
A small fix from David on scan and a few new device IDs for 7265.

Let me know about issues you may have!

The following changes since commit 9b0cd304f26b9fca140de15deeac2bf357d1f388:

Merge branch 'drm-next' of git://people.freedesktop.org/~airlied/linux
(2014-01-29 20:49:12 -0800)

are available in the git repository at:

Quoted 7 lines

git://git.kernel.org/pub/scm/linux/kernel/git/iwlwifi/iwlwifi-fixes.git
for-john

for you to fetch changes up to c512865446e6dd5b6e91e81187e75b734ad7cfc7:

iwlwifi: mvm: don't allow A band if SKU forbids it (2014-02-02
11:08:42 +0200)

Pulling now...

Signature · 2 lines
John W. Linville		Someday the world will need a hero, and you
[email protected]			might be all we have.  Be ready.