[PATCH 0/6] iwlwifi: fixes intended for 4.15 2017-11-25

[PATCH 0/6] iwlwifi: fixes intended for 4.15 2017-11-25

[Luca Coelho]

From: Luca Coelho <luciano.coelho@intel.com>

Hi,

This is the second batch of fixes intended for 4.15.

These are the fixes:

* One fix in rate-scaling;
* One fix for the TX queue hang detection for AP/GO modes;
* Fix the TX queue hang timeout used in monitor interfaces;
* Fix packet injection;
* Remove a wrong error message when dumping PCI registers;
* Fix race condition with RF-kill;

As usual, I'm pushing this to a pending branch, for kbuild bot, and
will send a pull-request later.

Please review.

Cheers,
Luca.

Emmanuel Grumbach (3):
  iwlwifi: mvm: don't use transmit queue hang detection when it is not
    possible
  iwlwifi: mvm: fix the TX queue hang timeout for MONITOR vif type
  iwlwifi: mvm: fix packet injection

Sara Sharon (2):
  iwlwifi: pcie: fix erroneous "Read failed message"
  iwlwifi: fix access to prph when transport is stopped

Shaul Triebitz (1):
  iwlwifi: mvm: set correct chains in Rx status

 drivers/net/wireless/intel/iwlwifi/fw/api/txq.h    |  4 ++
 drivers/net/wireless/intel/iwlwifi/fw/dbg.h        |  2 -
 drivers/net/wireless/intel/iwlwifi/mvm/mac-ctxt.c  |  2 +-
 drivers/net/wireless/intel/iwlwifi/mvm/mvm.h       |  1 +
 drivers/net/wireless/intel/iwlwifi/mvm/ops.c       |  1 +
 drivers/net/wireless/intel/iwlwifi/mvm/rxmq.c      |  4 +-
 drivers/net/wireless/intel/iwlwifi/mvm/sta.c       | 53 ++++++++++++++++------
 drivers/net/wireless/intel/iwlwifi/mvm/tx.c        |  3 +-
 drivers/net/wireless/intel/iwlwifi/mvm/utils.c     | 13 +++++-
 .../net/wireless/intel/iwlwifi/pcie/trans-gen2.c   |  6 +++
 drivers/net/wireless/intel/iwlwifi/pcie/trans.c    | 10 ++++
 11 files changed, 80 insertions(+), 19 deletions(-)

-- 
2.15.0

[PATCH 1/6] iwlwifi: mvm: set correct chains in Rx status

[Luca Coelho]

From: Shaul Triebitz <shaul.triebitz@intel.com>

ieee80211_rx_status::chains was always set to zero.
That caused rate scaling to always start with the
lowest rate possible (rs_get_initial_rate).
Set it correctly according to the MPDU response.

Signed-off-by: Shaul Triebitz <shaul.triebitz@intel.com>
Signed-off-by: Luca Coelho <luciano.coelho@intel.com>
---
 drivers/net/wireless/intel/iwlwifi/mvm/rxmq.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/net/wireless/intel/iwlwifi/mvm/rxmq.c b/drivers/net/wireless/intel/iwlwifi/mvm/rxmq.c
index 76dc58381e1c..20fe23fbf040 100644
--- a/drivers/net/wireless/intel/iwlwifi/mvm/rxmq.c
+++ b/drivers/net/wireless/intel/iwlwifi/mvm/rxmq.c
@@ -213,6 +213,7 @@ static void iwl_mvm_get_signal_strength(struct iwl_mvm *mvm,
 					struct ieee80211_rx_status *rx_status)
 {
 	int energy_a, energy_b, max_energy;
+	u32 rate_flags = le32_to_cpu(desc->rate_n_flags);
 
 	energy_a = desc->energy_a;
 	energy_a = energy_a ? -energy_a : S8_MIN;
@@ -224,7 +225,8 @@ static void iwl_mvm_get_signal_strength(struct iwl_mvm *mvm,
 			energy_a, energy_b, max_energy);
 
 	rx_status->signal = max_energy;
-	rx_status->chains = 0; /* TODO: phy info */
+	rx_status->chains =
+		(rate_flags & RATE_MCS_ANT_AB_MSK) >> RATE_MCS_ANT_POS;
 	rx_status->chain_signal[0] = energy_a;
 	rx_status->chain_signal[1] = energy_b;
 	rx_status->chain_signal[2] = S8_MIN;
-- 
2.15.0

[PATCH 2/6] iwlwifi: mvm: don't use transmit queue hang detection when it is not possible

[Luca Coelho]

From: Emmanuel Grumbach <emmanuel.grumbach@intel.com>

When we act as an AP, new firmware versions handle
internally the power saving clients and the driver doesn't
know that the peers went to sleep. It is, hence, possible
that a peer goes to sleep for a long time and stop pulling
frames. This will cause its transmit queue to hang which is
a condition that triggers the recovery flow in the driver.

While this client is certainly buggy (it should have pulled
the frame based on the TIM IE in the beacon), we can't blow
up because of a buggy client.

Change the current implementation to not enable the
transmit queue hang detection on queues that serve peers
when we act as an AP / GO.

We can still enable this mechanism using the debug
configuration which can come in handy when we want to
debug why the client doesn't wake up.

Cc: stable@vger.kernel.org # v4.13
Signed-off-by: Emmanuel Grumbach <emmanuel.grumbach@intel.com>
Signed-off-by: Luca Coelho <luciano.coelho@intel.com>
---
 drivers/net/wireless/intel/iwlwifi/mvm/utils.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/drivers/net/wireless/intel/iwlwifi/mvm/utils.c b/drivers/net/wireless/intel/iwlwifi/mvm/utils.c
index d46115e2d69e..19c1d1f76e15 100644
--- a/drivers/net/wireless/intel/iwlwifi/mvm/utils.c
+++ b/drivers/net/wireless/intel/iwlwifi/mvm/utils.c
@@ -1134,9 +1134,18 @@ unsigned int iwl_mvm_get_wd_timeout(struct iwl_mvm *mvm,
 	unsigned int default_timeout =
 		cmd_q ? IWL_DEF_WD_TIMEOUT : mvm->cfg->base_params->wd_timeout;
 
-	if (!iwl_fw_dbg_trigger_enabled(mvm->fw, FW_DBG_TRIGGER_TXQ_TIMERS))
+	if (!iwl_fw_dbg_trigger_enabled(mvm->fw, FW_DBG_TRIGGER_TXQ_TIMERS)) {
+		/*
+		 * We can't know when the station is asleep or awake, so we
+		 * must disable the queue hang detection.
+		 */
+		if (fw_has_capa(&mvm->fw->ucode_capa,
+				IWL_UCODE_TLV_CAPA_STA_PM_NOTIF) &&
+		    vif && vif->type == NL80211_IFTYPE_AP)
+			return IWL_WATCHDOG_DISABLED;
 		return iwlmvm_mod_params.tfd_q_hang_detect ?
 			default_timeout : IWL_WATCHDOG_DISABLED;
+	}
 
 	trigger = iwl_fw_dbg_get_trigger(mvm->fw, FW_DBG_TRIGGER_TXQ_TIMERS);
 	txq_timer = (void *)trigger->data;
-- 
2.15.0

[PATCH 3/6] iwlwifi: mvm: fix the TX queue hang timeout for MONITOR vif type

[Luca Coelho]

From: Emmanuel Grumbach <emmanuel.grumbach@intel.com>

The MONITOR type is missing in the interface type switch.
Add it.

Signed-off-by: Emmanuel Grumbach <emmanuel.grumbach@intel.com>
Signed-off-by: Luca Coelho <luciano.coelho@intel.com>
---
 drivers/net/wireless/intel/iwlwifi/mvm/utils.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/net/wireless/intel/iwlwifi/mvm/utils.c b/drivers/net/wireless/intel/iwlwifi/mvm/utils.c
index 19c1d1f76e15..03ffd84786ca 100644
--- a/drivers/net/wireless/intel/iwlwifi/mvm/utils.c
+++ b/drivers/net/wireless/intel/iwlwifi/mvm/utils.c
@@ -1172,6 +1172,8 @@ unsigned int iwl_mvm_get_wd_timeout(struct iwl_mvm *mvm,
 		return le32_to_cpu(txq_timer->p2p_go);
 	case NL80211_IFTYPE_P2P_DEVICE:
 		return le32_to_cpu(txq_timer->p2p_device);
+	case NL80211_IFTYPE_MONITOR:
+		return default_timeout;
 	default:
 		WARN_ON(1);
 		return mvm->cfg->base_params->wd_timeout;
-- 
2.15.0

[PATCH 4/6] iwlwifi: mvm: fix packet injection

[Luca Coelho]

From: Emmanuel Grumbach <emmanuel.grumbach@intel.com>

We need to have a station and a queue for the monitor
interface to be able to inject traffic. We used to have
this traffic routed to the auxiliary queue, but this queue
isn't scheduled for the station we had linked to the
monitor vif.

Allocate a new queue, link it to the monitor vif's station
and make that queue use the BE fifo.

This fixes https://bugzilla.kernel.org/show_bug.cgi?id=196715

Cc: stable@vger.kernel.org
Signed-off-by: Emmanuel Grumbach <emmanuel.grumbach@intel.com>
Signed-off-by: Luca Coelho <luciano.coelho@intel.com>
---
 drivers/net/wireless/intel/iwlwifi/fw/api/txq.h   |  4 ++
 drivers/net/wireless/intel/iwlwifi/mvm/mac-ctxt.c |  2 +-
 drivers/net/wireless/intel/iwlwifi/mvm/mvm.h      |  1 +
 drivers/net/wireless/intel/iwlwifi/mvm/ops.c      |  1 +
 drivers/net/wireless/intel/iwlwifi/mvm/sta.c      | 53 +++++++++++++++++------
 drivers/net/wireless/intel/iwlwifi/mvm/tx.c       |  3 +-
 6 files changed, 49 insertions(+), 15 deletions(-)

diff --git a/drivers/net/wireless/intel/iwlwifi/fw/api/txq.h b/drivers/net/wireless/intel/iwlwifi/fw/api/txq.h
index 87b4434224a1..dfa111bb411e 100644
--- a/drivers/net/wireless/intel/iwlwifi/fw/api/txq.h
+++ b/drivers/net/wireless/intel/iwlwifi/fw/api/txq.h
@@ -68,6 +68,9 @@
  * @IWL_MVM_DQA_CMD_QUEUE: a queue reserved for sending HCMDs to the FW
  * @IWL_MVM_DQA_AUX_QUEUE: a queue reserved for aux frames
  * @IWL_MVM_DQA_P2P_DEVICE_QUEUE: a queue reserved for P2P device frames
+ * @IWL_MVM_DQA_INJECT_MONITOR_QUEUE: a queue reserved for injection using
+ *	monitor mode. Note this queue is the same as the queue for P2P device
+ *	but we can't have active monitor mode along with P2P device anyway.
  * @IWL_MVM_DQA_GCAST_QUEUE: a queue reserved for P2P GO/SoftAP GCAST frames
  * @IWL_MVM_DQA_BSS_CLIENT_QUEUE: a queue reserved for BSS activity, to ensure
  *	that we are never left without the possibility to connect to an AP.
@@ -87,6 +90,7 @@ enum iwl_mvm_dqa_txq {
 	IWL_MVM_DQA_CMD_QUEUE = 0,
 	IWL_MVM_DQA_AUX_QUEUE = 1,
 	IWL_MVM_DQA_P2P_DEVICE_QUEUE = 2,
+	IWL_MVM_DQA_INJECT_MONITOR_QUEUE = 2,
 	IWL_MVM_DQA_GCAST_QUEUE = 3,
 	IWL_MVM_DQA_BSS_CLIENT_QUEUE = 4,
 	IWL_MVM_DQA_MIN_MGMT_QUEUE = 5,
diff --git a/drivers/net/wireless/intel/iwlwifi/mvm/mac-ctxt.c b/drivers/net/wireless/intel/iwlwifi/mvm/mac-ctxt.c
index a2bf530eeae4..2f22e14e00fe 100644
--- a/drivers/net/wireless/intel/iwlwifi/mvm/mac-ctxt.c
+++ b/drivers/net/wireless/intel/iwlwifi/mvm/mac-ctxt.c
@@ -787,7 +787,7 @@ static int iwl_mvm_mac_ctxt_cmd_listener(struct iwl_mvm *mvm,
 					 u32 action)
 {
 	struct iwl_mac_ctx_cmd cmd = {};
-	u32 tfd_queue_msk = 0;
+	u32 tfd_queue_msk = BIT(mvm->snif_queue);
 	int ret;
 
 	WARN_ON(vif->type != NL80211_IFTYPE_MONITOR);
diff --git a/drivers/net/wireless/intel/iwlwifi/mvm/mvm.h b/drivers/net/wireless/intel/iwlwifi/mvm/mvm.h
index 4575595ab022..6a9a25beab3f 100644
--- a/drivers/net/wireless/intel/iwlwifi/mvm/mvm.h
+++ b/drivers/net/wireless/intel/iwlwifi/mvm/mvm.h
@@ -972,6 +972,7 @@ struct iwl_mvm {
 
 	/* Tx queues */
 	u16 aux_queue;
+	u16 snif_queue;
 	u16 probe_queue;
 	u16 p2p_dev_queue;
 
diff --git a/drivers/net/wireless/intel/iwlwifi/mvm/ops.c b/drivers/net/wireless/intel/iwlwifi/mvm/ops.c
index 7078b7e458be..45470b6b351a 100644
--- a/drivers/net/wireless/intel/iwlwifi/mvm/ops.c
+++ b/drivers/net/wireless/intel/iwlwifi/mvm/ops.c
@@ -624,6 +624,7 @@ iwl_op_mode_mvm_start(struct iwl_trans *trans, const struct iwl_cfg *cfg,
 	mvm->fw_restart = iwlwifi_mod_params.fw_restart ? -1 : 0;
 
 	mvm->aux_queue = IWL_MVM_DQA_AUX_QUEUE;
+	mvm->snif_queue = IWL_MVM_DQA_INJECT_MONITOR_QUEUE;
 	mvm->probe_queue = IWL_MVM_DQA_AP_PROBE_RESP_QUEUE;
 	mvm->p2p_dev_queue = IWL_MVM_DQA_P2P_DEVICE_QUEUE;
 
diff --git a/drivers/net/wireless/intel/iwlwifi/mvm/sta.c b/drivers/net/wireless/intel/iwlwifi/mvm/sta.c
index c19f98489d4e..1add5615fc3a 100644
--- a/drivers/net/wireless/intel/iwlwifi/mvm/sta.c
+++ b/drivers/net/wireless/intel/iwlwifi/mvm/sta.c
@@ -1709,29 +1709,29 @@ void iwl_mvm_dealloc_int_sta(struct iwl_mvm *mvm, struct iwl_mvm_int_sta *sta)
 	sta->sta_id = IWL_MVM_INVALID_STA;
 }
 
-static void iwl_mvm_enable_aux_queue(struct iwl_mvm *mvm)
+static void iwl_mvm_enable_aux_snif_queue(struct iwl_mvm *mvm, u16 *queue,
+					  u8 sta_id, u8 fifo)
 {
 	unsigned int wdg_timeout = iwlmvm_mod_params.tfd_q_hang_detect ?
 					mvm->cfg->base_params->wd_timeout :
 					IWL_WATCHDOG_DISABLED;
 
 	if (iwl_mvm_has_new_tx_api(mvm)) {
-		int queue = iwl_mvm_tvqm_enable_txq(mvm, mvm->aux_queue,
-						    mvm->aux_sta.sta_id,
-						    IWL_MAX_TID_COUNT,
-						    wdg_timeout);
-		mvm->aux_queue = queue;
+		int tvqm_queue =
+			iwl_mvm_tvqm_enable_txq(mvm, *queue, sta_id,
+						IWL_MAX_TID_COUNT,
+						wdg_timeout);
+		*queue = tvqm_queue;
 	} else {
 		struct iwl_trans_txq_scd_cfg cfg = {
-			.fifo = IWL_MVM_TX_FIFO_MCAST,
-			.sta_id = mvm->aux_sta.sta_id,
+			.fifo = fifo,
+			.sta_id = sta_id,
 			.tid = IWL_MAX_TID_COUNT,
 			.aggregate = false,
 			.frame_limit = IWL_FRAME_LIMIT,
 		};
 
-		iwl_mvm_enable_txq(mvm, mvm->aux_queue, mvm->aux_queue, 0, &cfg,
-				   wdg_timeout);
+		iwl_mvm_enable_txq(mvm, *queue, *queue, 0, &cfg, wdg_timeout);
 	}
 }
 
@@ -1750,7 +1750,9 @@ int iwl_mvm_add_aux_sta(struct iwl_mvm *mvm)
 
 	/* Map Aux queue to fifo - needs to happen before adding Aux station */
 	if (!iwl_mvm_has_new_tx_api(mvm))
-		iwl_mvm_enable_aux_queue(mvm);
+		iwl_mvm_enable_aux_snif_queue(mvm, &mvm->aux_queue,
+					      mvm->aux_sta.sta_id,
+					      IWL_MVM_TX_FIFO_MCAST);
 
 	ret = iwl_mvm_add_int_sta_common(mvm, &mvm->aux_sta, NULL,
 					 MAC_INDEX_AUX, 0);
@@ -1764,7 +1766,9 @@ int iwl_mvm_add_aux_sta(struct iwl_mvm *mvm)
 	 * to firmware so enable queue here - after the station was added
 	 */
 	if (iwl_mvm_has_new_tx_api(mvm))
-		iwl_mvm_enable_aux_queue(mvm);
+		iwl_mvm_enable_aux_snif_queue(mvm, &mvm->aux_queue,
+					      mvm->aux_sta.sta_id,
+					      IWL_MVM_TX_FIFO_MCAST);
 
 	return 0;
 }
@@ -1772,10 +1776,31 @@ int iwl_mvm_add_aux_sta(struct iwl_mvm *mvm)
 int iwl_mvm_add_snif_sta(struct iwl_mvm *mvm, struct ieee80211_vif *vif)
 {
 	struct iwl_mvm_vif *mvmvif = iwl_mvm_vif_from_mac80211(vif);
+	int ret;
 
 	lockdep_assert_held(&mvm->mutex);
-	return iwl_mvm_add_int_sta_common(mvm, &mvm->snif_sta, vif->addr,
+
+	/* Map snif queue to fifo - must happen before adding snif station */
+	if (!iwl_mvm_has_new_tx_api(mvm))
+		iwl_mvm_enable_aux_snif_queue(mvm, &mvm->snif_queue,
+					      mvm->snif_sta.sta_id,
+					      IWL_MVM_TX_FIFO_BE);
+
+	ret = iwl_mvm_add_int_sta_common(mvm, &mvm->snif_sta, vif->addr,
 					 mvmvif->id, 0);
+	if (ret)
+		return ret;
+
+	/*
+	 * For 22000 firmware and on we cannot add queue to a station unknown
+	 * to firmware so enable queue here - after the station was added
+	 */
+	if (iwl_mvm_has_new_tx_api(mvm))
+		iwl_mvm_enable_aux_snif_queue(mvm, &mvm->snif_queue,
+					      mvm->snif_sta.sta_id,
+					      IWL_MVM_TX_FIFO_BE);
+
+	return 0;
 }
 
 int iwl_mvm_rm_snif_sta(struct iwl_mvm *mvm, struct ieee80211_vif *vif)
@@ -1784,6 +1809,8 @@ int iwl_mvm_rm_snif_sta(struct iwl_mvm *mvm, struct ieee80211_vif *vif)
 
 	lockdep_assert_held(&mvm->mutex);
 
+	iwl_mvm_disable_txq(mvm, mvm->snif_queue, mvm->snif_queue,
+			    IWL_MAX_TID_COUNT, 0);
 	ret = iwl_mvm_rm_sta_common(mvm, mvm->snif_sta.sta_id);
 	if (ret)
 		IWL_WARN(mvm, "Failed sending remove station\n");
diff --git a/drivers/net/wireless/intel/iwlwifi/mvm/tx.c b/drivers/net/wireless/intel/iwlwifi/mvm/tx.c
index 593b7f97b29c..333bcb75b8af 100644
--- a/drivers/net/wireless/intel/iwlwifi/mvm/tx.c
+++ b/drivers/net/wireless/intel/iwlwifi/mvm/tx.c
@@ -657,7 +657,8 @@ int iwl_mvm_tx_skb_non_sta(struct iwl_mvm *mvm, struct sk_buff *skb)
 			if (ap_sta_id != IWL_MVM_INVALID_STA)
 				sta_id = ap_sta_id;
 		} else if (info.control.vif->type == NL80211_IFTYPE_MONITOR) {
-			queue = mvm->aux_queue;
+			queue = mvm->snif_queue;
+			sta_id = mvm->snif_sta.sta_id;
 		}
 	}
 
-- 
2.15.0

[PATCH 5/6] iwlwifi: pcie: fix erroneous "Read failed message"

[Luca Coelho]

From: Sara Sharon <sara.sharon@intel.com>

Current pci dumping code code is always falling to the error
path, resulting with a constant "Read failed" message, also
for the successful reads.

Fixes: a5c932e41fdd ("iwlwifi: pcie: dump registers when HW becomes inaccessible")
Signed-off-by: Sara Sharon <sara.sharon@intel.com>
Signed-off-by: Luca Coelho <luciano.coelho@intel.com>
---
 drivers/net/wireless/intel/iwlwifi/pcie/trans.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/wireless/intel/iwlwifi/pcie/trans.c b/drivers/net/wireless/intel/iwlwifi/pcie/trans.c
index b7a51603465b..3dee95e6a475 100644
--- a/drivers/net/wireless/intel/iwlwifi/pcie/trans.c
+++ b/drivers/net/wireless/intel/iwlwifi/pcie/trans.c
@@ -166,6 +166,7 @@ static void iwl_trans_pcie_dump_regs(struct iwl_trans *trans)
 		print_hex_dump(KERN_ERR, prefix, DUMP_PREFIX_OFFSET, 32,
 			       4, buf, i, 0);
 	}
+	goto out;
 
 err_read:
 	print_hex_dump(KERN_ERR, prefix, DUMP_PREFIX_OFFSET, 32, 4, buf, i, 0);
-- 
2.15.0

[PATCH 6/6] iwlwifi: fix access to prph when transport is stopped

[Luca Coelho]

From: Sara Sharon <sara.sharon@intel.com>

When getting HW rfkill we get stop_device being called from
two paths.
One path is the IRQ calling stop device, and updating op
mode and stack.
As a result, cfg80211 is running rfkill sync work that shuts
down all devices (second path).
In the second path, we eventually get to iwl_mvm_stop_device
which calls iwl_fw_dump_conf_clear->iwl_fw_dbg_stop_recording,
that access periphery registers.
The device may be stopped at this point from the first path,
which will result with a failure to access those registers.
Simply checking for the trans status is insufficient, since
the race will still exist, only minimized.
Instead, move the stop from iwl_fw_dump_conf_clear (which is
getting called only from stop path) to the transport stop
device function, where the access is always safe.
This has the added value, of actually stopping dbgc before
stopping device even when the stop is initiated from the
transport.

Fixes: 1efc3843a4ee ("iwlwifi: stop dbgc recording before stopping DMA")
Signed-off-by: Sara Sharon <sara.sharon@intel.com>
Signed-off-by: Luca Coelho <luciano.coelho@intel.com>
---
 drivers/net/wireless/intel/iwlwifi/fw/dbg.h          | 2 --
 drivers/net/wireless/intel/iwlwifi/pcie/trans-gen2.c | 6 ++++++
 drivers/net/wireless/intel/iwlwifi/pcie/trans.c      | 9 +++++++++
 3 files changed, 15 insertions(+), 2 deletions(-)

diff --git a/drivers/net/wireless/intel/iwlwifi/fw/dbg.h b/drivers/net/wireless/intel/iwlwifi/fw/dbg.h
index 9c889a32fe24..223fb77a3aa9 100644
--- a/drivers/net/wireless/intel/iwlwifi/fw/dbg.h
+++ b/drivers/net/wireless/intel/iwlwifi/fw/dbg.h
@@ -209,8 +209,6 @@ static inline void iwl_fw_dbg_stop_recording(struct iwl_fw_runtime *fwrt)
 
 static inline void iwl_fw_dump_conf_clear(struct iwl_fw_runtime *fwrt)
 {
-	iwl_fw_dbg_stop_recording(fwrt);
-
 	fwrt->dump.conf = FW_DBG_INVALID;
 }
 
diff --git a/drivers/net/wireless/intel/iwlwifi/pcie/trans-gen2.c b/drivers/net/wireless/intel/iwlwifi/pcie/trans-gen2.c
index c59f4581e972..ac05fd1e74c4 100644
--- a/drivers/net/wireless/intel/iwlwifi/pcie/trans-gen2.c
+++ b/drivers/net/wireless/intel/iwlwifi/pcie/trans-gen2.c
@@ -49,6 +49,7 @@
  *
  *****************************************************************************/
 #include "iwl-trans.h"
+#include "iwl-prph.h"
 #include "iwl-context-info.h"
 #include "internal.h"
 
@@ -156,6 +157,11 @@ void _iwl_trans_pcie_gen2_stop_device(struct iwl_trans *trans, bool low_power)
 
 	trans_pcie->is_down = true;
 
+	/* Stop dbgc before stopping device */
+	iwl_write_prph(trans, DBGC_IN_SAMPLE, 0);
+	udelay(100);
+	iwl_write_prph(trans, DBGC_OUT_CTRL, 0);
+
 	/* tell the device to stop sending interrupts */
 	iwl_disable_interrupts(trans);
 
diff --git a/drivers/net/wireless/intel/iwlwifi/pcie/trans.c b/drivers/net/wireless/intel/iwlwifi/pcie/trans.c
index 3dee95e6a475..4541c86881d6 100644
--- a/drivers/net/wireless/intel/iwlwifi/pcie/trans.c
+++ b/drivers/net/wireless/intel/iwlwifi/pcie/trans.c
@@ -1227,6 +1227,15 @@ static void _iwl_trans_pcie_stop_device(struct iwl_trans *trans, bool low_power)
 
 	trans_pcie->is_down = true;
 
+	/* Stop dbgc before stopping device */
+	if (trans->cfg->device_family == IWL_DEVICE_FAMILY_7000) {
+		iwl_set_bits_prph(trans, MON_BUFF_SAMPLE_CTL, 0x100);
+	} else {
+		iwl_write_prph(trans, DBGC_IN_SAMPLE, 0);
+		udelay(100);
+		iwl_write_prph(trans, DBGC_OUT_CTRL, 0);
+	}
+
 	/* tell the device to stop sending interrupts */
 	iwl_disable_interrupts(trans);
 
-- 
2.15.0

Re: [PATCH 6/6] iwlwifi: fix access to prph when transport is stopped

[Kalle Valo]

Luca Coelho <luca@coelho.fi> writes:

> From: Sara Sharon <sara.sharon@intel.com>
>
> When getting HW rfkill we get stop_device being called from two paths.
> One path is the IRQ calling stop device, and updating op mode and
> stack. As a result, cfg80211 is running rfkill sync work that shuts
> down all devices (second path). In the second path, we eventually get
> to iwl_mvm_stop_device which calls
> iwl_fw_dump_conf_clear->iwl_fw_dbg_stop_recording, that access
> periphery registers. The device may be stopped at this point from the
> first path, which will result with a failure to access those
> registers. Simply checking for the trans status is insufficient, since
> the race will still exist, only minimized. Instead, move the stop from
> iwl_fw_dump_conf_clear (which is getting called only from stop path)
> to the transport stop device function, where the access is always
> safe. This has the added value, of actually stopping dbgc before
> stopping device even when the stop is initiated from the transport.
>
> Fixes: 1efc3843a4ee ("iwlwifi: stop dbgc recording before stopping
> DMA")

No need to resend because of this, but Fixes should be in one line.

-- 
Kalle Valo

Re: [PATCH 6/6] iwlwifi: fix access to prph when transport is stopped

[Kalle Valo]

Kalle Valo <kvalo@codeaurora.org> writes:

> Luca Coelho <luca@coelho.fi> writes:
>
>> From: Sara Sharon <sara.sharon@intel.com>
>>
>> When getting HW rfkill we get stop_device being called from two paths.
>> One path is the IRQ calling stop device, and updating op mode and
>> stack. As a result, cfg80211 is running rfkill sync work that shuts
>> down all devices (second path). In the second path, we eventually get
>> to iwl_mvm_stop_device which calls
>> iwl_fw_dump_conf_clear->iwl_fw_dbg_stop_recording, that access
>> periphery registers. The device may be stopped at this point from the
>> first path, which will result with a failure to access those
>> registers. Simply checking for the trans status is insufficient, since
>> the race will still exist, only minimized. Instead, move the stop from
>> iwl_fw_dump_conf_clear (which is getting called only from stop path)
>> to the transport stop device function, where the access is always
>> safe. This has the added value, of actually stopping dbgc before
>> stopping device even when the stop is initiated from the transport.
>>
>> Fixes: 1efc3843a4ee ("iwlwifi: stop dbgc recording before stopping
>> DMA")
>
> No need to resend because of this, but Fixes should be in one line.

Please ignore, I was too clever and forgot that I had called
gnus-article-fill-cited-article which word wraps the mail :)

--=20
Kalle Valo=

Re: [PATCH 6/6] iwlwifi: fix access to prph when transport is stopped

[Kalle Valo]

Luca Coelho <luca@coelho.fi> writes:

> From: Sara Sharon <sara.sharon@intel.com>
>
> When getting HW rfkill we get stop_device being called from
> two paths.
> One path is the IRQ calling stop device, and updating op
> mode and stack.
> As a result, cfg80211 is running rfkill sync work that shuts
> down all devices (second path).
> In the second path, we eventually get to iwl_mvm_stop_device
> which calls iwl_fw_dump_conf_clear->iwl_fw_dbg_stop_recording,
> that access periphery registers.
> The device may be stopped at this point from the first path,
> which will result with a failure to access those registers.
> Simply checking for the trans status is insufficient, since
> the race will still exist, only minimized.
> Instead, move the stop from iwl_fw_dump_conf_clear (which is
> getting called only from stop path) to the transport stop
> device function, where the access is always safe.
> This has the added value, of actually stopping dbgc before
> stopping device even when the stop is initiated from the
> transport.
>
> Fixes: 1efc3843a4ee ("iwlwifi: stop dbgc recording before stopping DMA")
> Signed-off-by: Sara Sharon <sara.sharon@intel.com>
> Signed-off-by: Luca Coelho <luciano.coelho@intel.com>

But no wonder I called gnus-article-fill-cited-article, the commit log
is just weirdly wrapped. Are you using outlook or how do you get it so
ugly? :)

-- 
Kalle Valo

Re: [PATCH 6/6] iwlwifi: fix access to prph when transport is stopped

[Luca Coelho]

On Wed, 2017-11-29 at 12:23 +0200, Kalle Valo wrote:
> Luca Coelho <luca@coelho.fi> writes:
> 
> > From: Sara Sharon <sara.sharon@intel.com>
> > 
> > When getting HW rfkill we get stop_device being called from
> > two paths.
> > One path is the IRQ calling stop device, and updating op
> > mode and stack.
> > As a result, cfg80211 is running rfkill sync work that shuts
> > down all devices (second path).
> > In the second path, we eventually get to iwl_mvm_stop_device
> > which calls iwl_fw_dump_conf_clear->iwl_fw_dbg_stop_recording,
> > that access periphery registers.
> > The device may be stopped at this point from the first path,
> > which will result with a failure to access those registers.
> > Simply checking for the trans status is insufficient, since
> > the race will still exist, only minimized.
> > Instead, move the stop from iwl_fw_dump_conf_clear (which is
> > getting called only from stop path) to the transport stop
> > device function, where the access is always safe.
> > This has the added value, of actually stopping dbgc before
> > stopping device even when the stop is initiated from the
> > transport.
> > 
> > Fixes: 1efc3843a4ee ("iwlwifi: stop dbgc recording before stopping
> > DMA")
> > Signed-off-by: Sara Sharon <sara.sharon@intel.com>
> > Signed-off-by: Luca Coelho <luciano.coelho@intel.com>
> 
> But no wonder I called gnus-article-fill-cited-article, the commit
> log
> is just weirdly wrapped. Are you using outlook or how do you get it
> so
> ugly? :)

Heh! I don't think it's wrapped weirdly, it's just that the paragraphs
don't have spaces between them, right? ;)

I noticed this but was probably too tired to nitpick back when I merged
this in our internal tree.  I'll make sure people space the commit
messages better from now on.

Sorry for the trouble.

--
Cheers,
Luca.

Re: [PATCH 6/6] iwlwifi: fix access to prph when transport is stopped

[Kalle Valo]

Luca Coelho <luca@coelho.fi> writes:

> On Wed, 2017-11-29 at 12:23 +0200, Kalle Valo wrote:
>
>> But no wonder I called gnus-article-fill-cited-article, the commit
>> log is just weirdly wrapped. Are you using outlook or how do you get
>> it so ugly? :)
>
> Heh! I don't think it's wrapped weirdly, it's just that the paragraphs
> don't have spaces between them, right? ;)

Sorry, don't get what you mean with missing spaces. To me (and
patchwork[1] and git[2] seem to agree) the word wrapping is just broken
for this commit log, and I have seen it also with other iwlwifi commits.
For example, there are just two words in the second line "two paths."

The standard format for git commit logs is something like this:

----------------------------------------------------------------------
When getting HW rfkill we get stop_device being called from two paths.
One path is the IRQ calling stop device, and updating op mode and stack.
As a result, cfg80211 is running rfkill sync work that shuts down all
devices (second path). In the second path, we eventually get to
iwl_mvm_stop_device which calls iwl_fw_dump_conf_clear->iwl_fw_dbg_stop_recording,
that access periphery registers.

The device may be stopped at this point from the first path,
which will result with a failure to access those registers. Simply
checking for the trans status is insufficient, since the race will still
exist, only minimized. Instead, move the stop from
iwl_fw_dump_conf_clear (which is getting called only from stop path) to
the transport stop device function, where the access is always safe.
This has the added value, of actually stopping dbgc before stopping
device even when the stop is initiated from the transport.
----------------------------------------------------------------------

[1] https://patchwork.kernel.org/patch/10074849/

[2] https://git.kernel.org/pub/scm/linux/kernel/git/kvalo/wireless-drivers.git/commit/?id=0232d2cd7aa8e1b810fe84fb4059a0bd1eabe2ba

-- 
Kalle Valo

Re: [PATCH 6/6] iwlwifi: fix access to prph when transport is stopped

[Kalle Valo]

Kalle Valo <kvalo@codeaurora.org> writes:

> Luca Coelho <luca@coelho.fi> writes:
>
>> On Wed, 2017-11-29 at 12:23 +0200, Kalle Valo wrote:
>>
>>> But no wonder I called gnus-article-fill-cited-article, the commit
>>> log is just weirdly wrapped. Are you using outlook or how do you get
>>> it so ugly? :)
>>
>> Heh! I don't think it's wrapped weirdly, it's just that the paragraphs
>> don't have spaces between them, right? ;)
>
> Sorry, don't get what you mean with missing spaces. To me (and
> patchwork[1] and git[2] seem to agree) the word wrapping is just broken
> for this commit log, and I have seen it also with other iwlwifi commits.
> For example, there are just two words in the second line "two paths."
>
> The standard format for git commit logs is something like this:
>
> ----------------------------------------------------------------------
> When getting HW rfkill we get stop_device being called from two paths.
> One path is the IRQ calling stop device, and updating op mode and stack.
> As a result, cfg80211 is running rfkill sync work that shuts down all
> devices (second path). In the second path, we eventually get to
> iwl_mvm_stop_device which calls iwl_fw_dump_conf_clear->iwl_fw_dbg_stop_recording,
> that access periphery registers.
>
> The device may be stopped at this point from the first path,
> which will result with a failure to access those registers. Simply
> checking for the trans status is insufficient, since the race will still
> exist, only minimized. Instead, move the stop from
> iwl_fw_dump_conf_clear (which is getting called only from stop path) to
> the transport stop device function, where the access is always safe.
> This has the added value, of actually stopping dbgc before stopping
> device even when the stop is initiated from the transport.
> ----------------------------------------------------------------------
>
> [1] https://patchwork.kernel.org/patch/10074849/

Instead of trying to get what I mean, check instead the patchwork page
and compare there the original commit log with my reformatted version. I
think that visualises pretty well what I'm trying to say :)

-- 
Kalle Valo

Re: [PATCH 6/6] iwlwifi: fix access to prph when transport is stopped

[Luca Coelho]

On Fri, 2017-12-08 at 14:26 +0200, Kalle Valo wrote:
> Kalle Valo <kvalo@codeaurora.org> writes:
> 
> > Luca Coelho <luca@coelho.fi> writes:
> > 
> > > On Wed, 2017-11-29 at 12:23 +0200, Kalle Valo wrote:
> > > 
> > > > But no wonder I called gnus-article-fill-cited-article, the
> > > > commit
> > > > log is just weirdly wrapped. Are you using outlook or how do
> > > > you get
> > > > it so ugly? :)
> > > 
> > > Heh! I don't think it's wrapped weirdly, it's just that the
> > > paragraphs
> > > don't have spaces between them, right? ;)
> > 
> > Sorry, don't get what you mean with missing spaces. To me (and
> > patchwork[1] and git[2] seem to agree) the word wrapping is just
> > broken
> > for this commit log, and I have seen it also with other iwlwifi
> > commits.
> > For example, there are just two words in the second line "two
> > paths."
> > 
> > The standard format for git commit logs is something like this:
> > 
> > -----------------------------------------------------------------
> > -----
> > When getting HW rfkill we get stop_device being called from two
> > paths.
> > One path is the IRQ calling stop device, and updating op mode and
> > stack.
> > As a result, cfg80211 is running rfkill sync work that shuts down
> > all
> > devices (second path). In the second path, we eventually get to
> > iwl_mvm_stop_device which calls iwl_fw_dump_conf_clear-
> > >iwl_fw_dbg_stop_recording,
> > that access periphery registers.
> > 
> > The device may be stopped at this point from the first path,
> > which will result with a failure to access those registers. Simply
> > checking for the trans status is insufficient, since the race will
> > still
> > exist, only minimized. Instead, move the stop from
> > iwl_fw_dump_conf_clear (which is getting called only from stop
> > path) to
> > the transport stop device function, where the access is always
> > safe.
> > This has the added value, of actually stopping dbgc before stopping
> > device even when the stop is initiated from the transport.
> > -----------------------------------------------------------------
> > -----
> > 
> > [1] https://patchwork.kernel.org/patch/10074849/
> 
> Instead of trying to get what I mean, check instead the patchwork
> page
> and compare there the original commit log with my reformatted
> version. I
> think that visualises pretty well what I'm trying to say :)

Okay, granted, but that's not really because of alignment or line-
wrapping.  It's because the paragraphs are broken too often.

In any case, I'll pay more attention to this.  So my questions remains,
do you want me to remake my pull-req to fix this?

--
Luca.

Re: [PATCH 6/6] iwlwifi: fix access to prph when transport is stopped

[Kalle Valo]

Luca Coelho <luca@coelho.fi> writes:

> On Fri, 2017-12-08 at 14:26 +0200, Kalle Valo wrote:
>> Kalle Valo <kvalo@codeaurora.org> writes:
>> 
>> > Luca Coelho <luca@coelho.fi> writes:
>> > 
>> > > On Wed, 2017-11-29 at 12:23 +0200, Kalle Valo wrote:
>> > > 
>> > > > But no wonder I called gnus-article-fill-cited-article, the
>> > > > commit
>> > > > log is just weirdly wrapped. Are you using outlook or how do
>> > > > you get
>> > > > it so ugly? :)
>> > > 
>> > > Heh! I don't think it's wrapped weirdly, it's just that the
>> > > paragraphs
>> > > don't have spaces between them, right? ;)
>> > 
>> > Sorry, don't get what you mean with missing spaces. To me (and
>> > patchwork[1] and git[2] seem to agree) the word wrapping is just
>> > broken
>> > for this commit log, and I have seen it also with other iwlwifi
>> > commits.
>> > For example, there are just two words in the second line "two
>> > paths."
>> > 
>> > The standard format for git commit logs is something like this:
>> > 
>> > -----------------------------------------------------------------
>> > -----
>> > When getting HW rfkill we get stop_device being called from two
>> > paths.
>> > One path is the IRQ calling stop device, and updating op mode and
>> > stack.
>> > As a result, cfg80211 is running rfkill sync work that shuts down
>> > all
>> > devices (second path). In the second path, we eventually get to
>> > iwl_mvm_stop_device which calls iwl_fw_dump_conf_clear-
>> > >iwl_fw_dbg_stop_recording,
>> > that access periphery registers.
>> > 
>> > The device may be stopped at this point from the first path,
>> > which will result with a failure to access those registers. Simply
>> > checking for the trans status is insufficient, since the race will
>> > still
>> > exist, only minimized. Instead, move the stop from
>> > iwl_fw_dump_conf_clear (which is getting called only from stop
>> > path) to
>> > the transport stop device function, where the access is always
>> > safe.
>> > This has the added value, of actually stopping dbgc before stopping
>> > device even when the stop is initiated from the transport.
>> > -----------------------------------------------------------------
>> > -----
>> > 
>> > [1] https://patchwork.kernel.org/patch/10074849/
>> 
>> Instead of trying to get what I mean, check instead the patchwork
>> page
>> and compare there the original commit log with my reformatted
>> version. I
>> think that visualises pretty well what I'm trying to say :)
>
> Okay, granted, but that's not really because of alignment or line-
> wrapping.  It's because the paragraphs are broken too often.
>
> In any case, I'll pay more attention to this.  So my questions remains,
> do you want me to remake my pull-req to fix this?

This commit is already in wireless-drivers, so no need to resend. I was
just trying to give an example of what I was trying to say.

-- 
Kalle Valo

Re: [PATCH 6/6] iwlwifi: fix access to prph when transport is stopped

[Luca Coelho]

On Fri, 2017-12-08 at 14:22 +0200, Kalle Valo wrote:
> Luca Coelho <luca@coelho.fi> writes:
> 
> > On Wed, 2017-11-29 at 12:23 +0200, Kalle Valo wrote:
> > 
> > > But no wonder I called gnus-article-fill-cited-article, the
> > > commit
> > > log is just weirdly wrapped. Are you using outlook or how do you
> > > get
> > > it so ugly? :)
> > 
> > Heh! I don't think it's wrapped weirdly, it's just that the
> > paragraphs
> > don't have spaces between them, right? ;)
> 
> Sorry, don't get what you mean with missing spaces. To me (and
> patchwork[1] and git[2] seem to agree) the word wrapping is just
> broken
> for this commit log, and I have seen it also with other iwlwifi
> commits.
> For example, there are just two words in the second line "two paths."

That depends on how many characters per line you want to include.  "two
paths." is the last part of the first paragraph, I guess Sari's editor
is wrapping somewhere at <70 chars per line.

Then that is accentuated a bit by the lack of an empty line between
paragraphs.  But with those lines added, besides being a bit narrow, it
would look quite clear:

-----8<-----
When getting HW rfkill we get stop_device being called from
two paths.

One path is the IRQ calling stop device, and updating op
mode and stack.

As a result, cfg80211 is running rfkill sync work that shuts
down all devices (second path).

In the second path, we eventually get to iwl_mvm_stop_device
which calls iwl_fw_dump_conf_clear->iwl_fw_dbg_stop_recording,
that access periphery registers.

The device may be stopped at this point from the first path,
which will result with a failure to access those registers.

Simply checking for the trans status is insufficient, since
the race will still exist, only minimized.

Instead, move the stop from iwl_fw_dump_conf_clear (which is
getting called only from stop path) to the transport stop
device function, where the access is always safe.

This has the added value, of actually stopping dbgc before
stopping device even when the stop is initiated from the
transport.
----->8-----

Does it make sense?

I've already told Sari about your preference and I'll make sure the
next patches will look cleaner.  I can fix this patch if you want, but
is it worth remaking my pull-request just for this?

--
Cheers,
Luca.