[PATCH v2 0/6] ath10k: pci related fixes 2014-10-09

[PATCH v2 0/6] ath10k: pci related fixes 2014-10-09

[Michal Kazior]

Hi,

First 2 patches address some irq issues.

Other 4 fix and cleanup device resetting.

Hopefully this will make ath10k a little more
robust when it comes to warm reset.

As a side effect of the changes ath10k now boots a
little faster (slightly over a second instead of
two).

v2:
 * don't remove power_up/down [Kalle]
 * fix CE crash [Janusz]
 * remove irq workaround in patch #2


Michal Kazior (6):
  ath10k: re-disable interrupts after target init
  ath10k: mask/unmask msi fw irq
  ath10k: split ce pipe init/alloc further
  ath10k: make warm reset a bit safer and faster
  ath10k: split reset logic from power up
  ath10k: don't reset chip on power_down

 drivers/net/wireless/ath/ath10k/ce.c  |  56 +++--
 drivers/net/wireless/ath/ath10k/ce.h  |   8 +-
 drivers/net/wireless/ath/ath10k/hw.h  |   1 +
 drivers/net/wireless/ath/ath10k/pci.c | 390 +++++++++++++++++-----------------
 4 files changed, 227 insertions(+), 228 deletions(-)

-- 
1.8.5.3

[PATCH v2 1/6] ath10k: re-disable interrupts after target init

[Michal Kazior]

If MSI isn't configured device ROM program expects
legacy interrupts to be enabled before it can
fully boot. Don't forget to disable legacy
interrupts after that.

While at it re-use the legacy irq enabling helper
instead of calling ath10k_pci_write32().

Signed-off-by: Michal Kazior <michal.kazior@tieto.com>
---
 drivers/net/wireless/ath/ath10k/pci.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/net/wireless/ath/ath10k/pci.c b/drivers/net/wireless/ath/ath10k/pci.c
index 730bb18..6077095 100644
--- a/drivers/net/wireless/ath/ath10k/pci.c
+++ b/drivers/net/wireless/ath/ath10k/pci.c
@@ -2279,14 +2279,13 @@ static int ath10k_pci_wait_for_target_init(struct ath10k *ar)
 
 		if (ar_pci->num_msi_intrs == 0)
 			/* Fix potential race by repeating CORE_BASE writes */
-			ath10k_pci_write32(ar, SOC_CORE_BASE_ADDRESS +
-					   PCIE_INTR_ENABLE_ADDRESS,
-					   PCIE_INTR_FIRMWARE_MASK |
-					   PCIE_INTR_CE_MASK_ALL);
+			ath10k_pci_enable_legacy_irq(ar);
 
 		mdelay(10);
 	} while (time_before(jiffies, timeout));
 
+	ath10k_pci_disable_and_clear_legacy_irq(ar);
+
 	if (val == 0xffffffff) {
 		ath10k_err(ar, "failed to read device register, device is gone\n");
 		return -EIO;
-- 
1.8.5.3

[PATCH v2 2/6] ath10k: mask/unmask msi fw irq

[Michal Kazior]

This was the final missing bit to making sure the
device doesn't assert interrupts to host.

This should fix possible race when target crashes
during driver teardown.

This also removes an early warm reset workaround
during pci probing.

Signed-off-by: Michal Kazior <michal.kazior@tieto.com>
---

Notes:
    v2:
     * remove workaround from pci probe that used warm reset

 drivers/net/wireless/ath/ath10k/hw.h  |  1 +
 drivers/net/wireless/ath/ath10k/pci.c | 53 ++++++++++++++++++++---------------
 2 files changed, 32 insertions(+), 22 deletions(-)

diff --git a/drivers/net/wireless/ath/ath10k/hw.h b/drivers/net/wireless/ath/ath10k/hw.h
index 5dd6551..6242319 100644
--- a/drivers/net/wireless/ath/ath10k/hw.h
+++ b/drivers/net/wireless/ath/ath10k/hw.h
@@ -289,6 +289,7 @@ struct ath10k_pktlog_hdr {
 #define SI_RX_DATA1_OFFSET			0x00000014
 
 #define CORE_CTRL_CPU_INTR_MASK			0x00002000
+#define CORE_CTRL_PCIE_REG_31_MASK		0x00000800
 #define CORE_CTRL_ADDRESS			0x0000
 #define PCIE_INTR_ENABLE_ADDRESS		0x0008
 #define PCIE_INTR_CAUSE_ADDRESS			0x000c
diff --git a/drivers/net/wireless/ath/ath10k/pci.c b/drivers/net/wireless/ath/ath10k/pci.c
index 6077095..5c5a9e7 100644
--- a/drivers/net/wireless/ath/ath10k/pci.c
+++ b/drivers/net/wireless/ath/ath10k/pci.c
@@ -1143,14 +1143,37 @@ static void ath10k_pci_hif_get_default_pipe(struct ath10k *ar,
 						 &dl_is_polled);
 }
 
-static void ath10k_pci_irq_disable(struct ath10k *ar)
+static void ath10k_pci_irq_msi_fw_mask(struct ath10k *ar)
 {
-	struct ath10k_pci *ar_pci = ath10k_pci_priv(ar);
-	int i;
+	u32 val;
+
+	val = ath10k_pci_read32(ar, SOC_CORE_BASE_ADDRESS + CORE_CTRL_ADDRESS);
+	val &= ~CORE_CTRL_PCIE_REG_31_MASK;
+
+	ath10k_pci_write32(ar, SOC_CORE_BASE_ADDRESS + CORE_CTRL_ADDRESS, val);
+}
+
+static void ath10k_pci_irq_msi_fw_unmask(struct ath10k *ar)
+{
+	u32 val;
+
+	val = ath10k_pci_read32(ar, SOC_CORE_BASE_ADDRESS + CORE_CTRL_ADDRESS);
+	val |= CORE_CTRL_PCIE_REG_31_MASK;
+
+	ath10k_pci_write32(ar, SOC_CORE_BASE_ADDRESS + CORE_CTRL_ADDRESS, val);
+}
 
+static void ath10k_pci_irq_disable(struct ath10k *ar)
+{
 	ath10k_ce_disable_interrupts(ar);
 	ath10k_pci_disable_and_clear_legacy_irq(ar);
-	/* FIXME: How to mask all MSI interrupts? */
+	ath10k_pci_irq_msi_fw_mask(ar);
+}
+
+static void ath10k_pci_irq_sync(struct ath10k *ar)
+{
+	struct ath10k_pci *ar_pci = ath10k_pci_priv(ar);
+	int i;
 
 	for (i = 0; i < max(1, ar_pci->num_msi_intrs); i++)
 		synchronize_irq(ar_pci->pdev->irq + i);
@@ -1160,7 +1183,7 @@ static void ath10k_pci_irq_enable(struct ath10k *ar)
 {
 	ath10k_ce_enable_interrupts(ar);
 	ath10k_pci_enable_legacy_irq(ar);
-	/* FIXME: How to unmask all MSI interrupts? */
+	ath10k_pci_irq_msi_fw_unmask(ar);
 }
 
 static int ath10k_pci_hif_start(struct ath10k *ar)
@@ -1288,6 +1311,7 @@ static void ath10k_pci_hif_stop(struct ath10k *ar)
 	ath10k_pci_warm_reset(ar);
 
 	ath10k_pci_irq_disable(ar);
+	ath10k_pci_irq_sync(ar);
 	ath10k_pci_flush(ar);
 }
 
@@ -2285,6 +2309,7 @@ static int ath10k_pci_wait_for_target_init(struct ath10k *ar)
 	} while (time_before(jiffies, timeout));
 
 	ath10k_pci_disable_and_clear_legacy_irq(ar);
+	ath10k_pci_irq_msi_fw_mask(ar);
 
 	if (val == 0xffffffff) {
 		ath10k_err(ar, "failed to read device register, device is gone\n");
@@ -2477,20 +2502,7 @@ static int ath10k_pci_probe(struct pci_dev *pdev,
 	}
 
 	ath10k_pci_ce_deinit(ar);
-
-	ret = ath10k_ce_disable_interrupts(ar);
-	if (ret) {
-		ath10k_err(ar, "failed to disable copy engine interrupts: %d\n",
-			   ret);
-		goto err_free_ce;
-	}
-
-	/* Workaround: There's no known way to mask all possible interrupts via
-	 * device CSR. The only way to make sure device doesn't assert
-	 * interrupts is to reset it. Interrupts are then disabled on host
-	 * after handlers are registered.
-	 */
-	ath10k_pci_warm_reset(ar);
+	ath10k_pci_irq_disable(ar);
 
 	ret = ath10k_pci_init_irq(ar);
 	if (ret) {
@@ -2508,9 +2520,6 @@ static int ath10k_pci_probe(struct pci_dev *pdev,
 		goto err_deinit_irq;
 	}
 
-	/* This shouldn't race as the device has been reset above. */
-	ath10k_pci_irq_disable(ar);
-
 	ret = ath10k_core_register(ar, chip_id);
 	if (ret) {
 		ath10k_err(ar, "failed to register driver core: %d\n", ret);
-- 
1.8.5.3

[PATCH v2 3/6] ath10k: split ce pipe init/alloc further

[Michal Kazior]

Calling init to reinit ce pipe state would also
re-set all static structure links and setting
(which don't change over driver lifecycle).

Make it so alloc links structures and initializes
static data and init part to setup state
variables and clear stuff.

Signed-off-by: Michal Kazior <michal.kazior@tieto.com>
---

Notes:
    v2:
     * add this patch to fix CE issue [Janusz]

 drivers/net/wireless/ath/ath10k/ce.c  | 56 ++++++++++++++---------------
 drivers/net/wireless/ath/ath10k/ce.h  |  8 ++---
 drivers/net/wireless/ath/ath10k/pci.c | 67 ++++++++++++++++-------------------
 3 files changed, 62 insertions(+), 69 deletions(-)

diff --git a/drivers/net/wireless/ath/ath10k/ce.c b/drivers/net/wireless/ath/ath10k/ce.c
index 4745ef2..9b89ac1 100644
--- a/drivers/net/wireless/ath/ath10k/ce.c
+++ b/drivers/net/wireless/ath/ath10k/ce.c
@@ -1023,37 +1023,10 @@ ath10k_ce_alloc_dest_ring(struct ath10k *ar, unsigned int ce_id,
  * initialized by software/firmware.
  */
 int ath10k_ce_init_pipe(struct ath10k *ar, unsigned int ce_id,
-			const struct ce_attr *attr,
-			void (*send_cb)(struct ath10k_ce_pipe *),
-			void (*recv_cb)(struct ath10k_ce_pipe *))
+			const struct ce_attr *attr)
 {
-	struct ath10k_pci *ar_pci = ath10k_pci_priv(ar);
-	struct ath10k_ce_pipe *ce_state = &ar_pci->ce_states[ce_id];
 	int ret;
 
-	/*
-	 * Make sure there's enough CE ringbuffer entries for HTT TX to avoid
-	 * additional TX locking checks.
-	 *
-	 * For the lack of a better place do the check here.
-	 */
-	BUILD_BUG_ON(2*TARGET_NUM_MSDU_DESC >
-		     (CE_HTT_H2T_MSG_SRC_NENTRIES - 1));
-	BUILD_BUG_ON(2*TARGET_10X_NUM_MSDU_DESC >
-		     (CE_HTT_H2T_MSG_SRC_NENTRIES - 1));
-
-	spin_lock_bh(&ar_pci->ce_lock);
-	ce_state->ar = ar;
-	ce_state->id = ce_id;
-	ce_state->ctrl_addr = ath10k_ce_base_address(ce_id);
-	ce_state->attr_flags = attr->flags;
-	ce_state->src_sz_max = attr->src_sz_max;
-	if (attr->src_nentries)
-		ce_state->send_cb = send_cb;
-	if (attr->dest_nentries)
-		ce_state->recv_cb = recv_cb;
-	spin_unlock_bh(&ar_pci->ce_lock);
-
 	if (attr->src_nentries) {
 		ret = ath10k_ce_init_src_ring(ar, ce_id, attr);
 		if (ret) {
@@ -1101,12 +1074,37 @@ void ath10k_ce_deinit_pipe(struct ath10k *ar, unsigned int ce_id)
 }
 
 int ath10k_ce_alloc_pipe(struct ath10k *ar, int ce_id,
-			 const struct ce_attr *attr)
+			 const struct ce_attr *attr,
+			 void (*send_cb)(struct ath10k_ce_pipe *),
+			 void (*recv_cb)(struct ath10k_ce_pipe *))
 {
 	struct ath10k_pci *ar_pci = ath10k_pci_priv(ar);
 	struct ath10k_ce_pipe *ce_state = &ar_pci->ce_states[ce_id];
 	int ret;
 
+	/*
+	 * Make sure there's enough CE ringbuffer entries for HTT TX to avoid
+	 * additional TX locking checks.
+	 *
+	 * For the lack of a better place do the check here.
+	 */
+	BUILD_BUG_ON(2*TARGET_NUM_MSDU_DESC >
+		     (CE_HTT_H2T_MSG_SRC_NENTRIES - 1));
+	BUILD_BUG_ON(2*TARGET_10X_NUM_MSDU_DESC >
+		     (CE_HTT_H2T_MSG_SRC_NENTRIES - 1));
+
+	ce_state->ar = ar;
+	ce_state->id = ce_id;
+	ce_state->ctrl_addr = ath10k_ce_base_address(ce_id);
+	ce_state->attr_flags = attr->flags;
+	ce_state->src_sz_max = attr->src_sz_max;
+
+	if (attr->src_nentries)
+		ce_state->send_cb = send_cb;
+
+	if (attr->dest_nentries)
+		ce_state->recv_cb = recv_cb;
+
 	if (attr->src_nentries) {
 		ce_state->src_ring = ath10k_ce_alloc_src_ring(ar, ce_id, attr);
 		if (IS_ERR(ce_state->src_ring)) {
diff --git a/drivers/net/wireless/ath/ath10k/ce.h b/drivers/net/wireless/ath/ath10k/ce.h
index 608262a..617a151 100644
--- a/drivers/net/wireless/ath/ath10k/ce.h
+++ b/drivers/net/wireless/ath/ath10k/ce.h
@@ -201,12 +201,12 @@ int ath10k_ce_completed_send_next_nolock(struct ath10k_ce_pipe *ce_state,
 /*==================CE Engine Initialization=======================*/
 
 int ath10k_ce_init_pipe(struct ath10k *ar, unsigned int ce_id,
-			const struct ce_attr *attr,
-			void (*send_cb)(struct ath10k_ce_pipe *),
-			void (*recv_cb)(struct ath10k_ce_pipe *));
+			const struct ce_attr *attr);
 void ath10k_ce_deinit_pipe(struct ath10k *ar, unsigned int ce_id);
 int ath10k_ce_alloc_pipe(struct ath10k *ar, int ce_id,
-			 const struct ce_attr *attr);
+			 const struct ce_attr *attr,
+			 void (*send_cb)(struct ath10k_ce_pipe *),
+			 void (*recv_cb)(struct ath10k_ce_pipe *));
 void ath10k_ce_free_pipe(struct ath10k *ar, int ce_id);
 
 /*==================CE Engine Shutdown=======================*/
diff --git a/drivers/net/wireless/ath/ath10k/pci.c b/drivers/net/wireless/ath/ath10k/pci.c
index 5c5a9e7..c8cf177 100644
--- a/drivers/net/wireless/ath/ath10k/pci.c
+++ b/drivers/net/wireless/ath/ath10k/pci.c
@@ -1615,23 +1615,40 @@ static int ath10k_pci_init_config(struct ath10k *ar)
 	return 0;
 }
 
-static int ath10k_pci_alloc_ce(struct ath10k *ar)
+static int ath10k_pci_alloc_pipes(struct ath10k *ar)
 {
+	struct ath10k_pci *ar_pci = ath10k_pci_priv(ar);
+	struct ath10k_pci_pipe *pipe;
 	int i, ret;
 
 	for (i = 0; i < CE_COUNT; i++) {
-		ret = ath10k_ce_alloc_pipe(ar, i, &host_ce_config_wlan[i]);
+		pipe = &ar_pci->pipe_info[i];
+		pipe->ce_hdl = &ar_pci->ce_states[i];
+		pipe->pipe_num = i;
+		pipe->hif_ce_state = ar;
+
+		ret = ath10k_ce_alloc_pipe(ar, i, &host_ce_config_wlan[i],
+					   ath10k_pci_ce_send_done,
+					   ath10k_pci_ce_recv_data);
 		if (ret) {
 			ath10k_err(ar, "failed to allocate copy engine pipe %d: %d\n",
 				   i, ret);
 			return ret;
 		}
+
+		/* Last CE is Diagnostic Window */
+		if (i == CE_COUNT - 1) {
+			ar_pci->ce_diag = pipe->ce_hdl;
+			continue;
+		}
+
+		pipe->buf_sz = (size_t)(host_ce_config_wlan[i].src_sz_max);
 	}
 
 	return 0;
 }
 
-static void ath10k_pci_free_ce(struct ath10k *ar)
+static void ath10k_pci_free_pipes(struct ath10k *ar)
 {
 	int i;
 
@@ -1639,39 +1656,17 @@ static void ath10k_pci_free_ce(struct ath10k *ar)
 		ath10k_ce_free_pipe(ar, i);
 }
 
-static int ath10k_pci_ce_init(struct ath10k *ar)
+static int ath10k_pci_init_pipes(struct ath10k *ar)
 {
-	struct ath10k_pci *ar_pci = ath10k_pci_priv(ar);
-	struct ath10k_pci_pipe *pipe_info;
-	const struct ce_attr *attr;
-	int pipe_num, ret;
+	int i, ret;
 
-	for (pipe_num = 0; pipe_num < CE_COUNT; pipe_num++) {
-		pipe_info = &ar_pci->pipe_info[pipe_num];
-		pipe_info->ce_hdl = &ar_pci->ce_states[pipe_num];
-		pipe_info->pipe_num = pipe_num;
-		pipe_info->hif_ce_state = ar;
-		attr = &host_ce_config_wlan[pipe_num];
-
-		ret = ath10k_ce_init_pipe(ar, pipe_num, attr,
-					  ath10k_pci_ce_send_done,
-					  ath10k_pci_ce_recv_data);
+	for (i = 0; i < CE_COUNT; i++) {
+		ret = ath10k_ce_init_pipe(ar, i, &host_ce_config_wlan[i]);
 		if (ret) {
 			ath10k_err(ar, "failed to initialize copy engine pipe %d: %d\n",
-				   pipe_num, ret);
+				   i, ret);
 			return ret;
 		}
-
-		if (pipe_num == CE_COUNT - 1) {
-			/*
-			 * Reserve the ultimate CE for
-			 * diagnostic Window support
-			 */
-			ar_pci->ce_diag = pipe_info->ce_hdl;
-			continue;
-		}
-
-		pipe_info->buf_sz = (size_t)(attr->src_sz_max);
 	}
 
 	return 0;
@@ -1825,7 +1820,7 @@ static int __ath10k_pci_hif_power_up(struct ath10k *ar, bool cold_reset)
 		goto err;
 	}
 
-	ret = ath10k_pci_ce_init(ar);
+	ret = ath10k_pci_init_pipes(ar);
 	if (ret) {
 		ath10k_err(ar, "failed to initialize CE: %d\n", ret);
 		goto err;
@@ -2494,7 +2489,7 @@ static int ath10k_pci_probe(struct pci_dev *pdev,
 		goto err_sleep;
 	}
 
-	ret = ath10k_pci_alloc_ce(ar);
+	ret = ath10k_pci_alloc_pipes(ar);
 	if (ret) {
 		ath10k_err(ar, "failed to allocate copy engine pipes: %d\n",
 			   ret);
@@ -2507,7 +2502,7 @@ static int ath10k_pci_probe(struct pci_dev *pdev,
 	ret = ath10k_pci_init_irq(ar);
 	if (ret) {
 		ath10k_err(ar, "failed to init irqs: %d\n", ret);
-		goto err_free_ce;
+		goto err_free_pipes;
 	}
 
 	ath10k_info(ar, "pci irq %s interrupts %d irq_mode %d reset_mode %d\n",
@@ -2535,8 +2530,8 @@ err_free_irq:
 err_deinit_irq:
 	ath10k_pci_deinit_irq(ar);
 
-err_free_ce:
-	ath10k_pci_free_ce(ar);
+err_free_pipes:
+	ath10k_pci_free_pipes(ar);
 
 err_sleep:
 	ath10k_pci_sleep(ar);
@@ -2570,7 +2565,7 @@ static void ath10k_pci_remove(struct pci_dev *pdev)
 	ath10k_pci_kill_tasklet(ar);
 	ath10k_pci_deinit_irq(ar);
 	ath10k_pci_ce_deinit(ar);
-	ath10k_pci_free_ce(ar);
+	ath10k_pci_free_pipes(ar);
 	ath10k_pci_sleep(ar);
 	ath10k_pci_release(ar);
 	ath10k_core_destroy(ar);
-- 
1.8.5.3

[PATCH v2 4/6] ath10k: make warm reset a bit safer and faster

[Michal Kazior]

One of the problems with warm reset I've found is
that it must be guaranteed that copy engine
registers are not being accessed while being
reset. Otherwise in worst case scenario the host
may lock up.

Instead of using sleeps and hoping the device is
operational in some arbitrary timeframes use
firmware indication register.

As a side effect this makes driver
boot/stop/recovery faster.

Signed-off-by: Michal Kazior <michal.kazior@tieto.com>
---

Notes:
    v2:
     * fix kernel panic on early fw crash due to CE not being fully initialized [Janusz]

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

diff --git a/drivers/net/wireless/ath/ath10k/pci.c b/drivers/net/wireless/ath/ath10k/pci.c
index c8cf177..54213ac 100644
--- a/drivers/net/wireless/ath/ath10k/pci.c
+++ b/drivers/net/wireless/ath/ath10k/pci.c
@@ -1707,89 +1707,75 @@ static void ath10k_pci_warm_reset_si0(struct ath10k *ar)
 	msleep(10);
 }
 
-static int ath10k_pci_warm_reset(struct ath10k *ar)
+static void ath10k_pci_warm_reset_cpu(struct ath10k *ar)
 {
 	u32 val;
 
-	ath10k_dbg(ar, ATH10K_DBG_BOOT, "boot warm reset\n");
-
-	spin_lock_bh(&ar->data_lock);
-
-	ar->stats.fw_warm_reset_counter++;
-
-	spin_unlock_bh(&ar->data_lock);
-
-	/* debug */
-	val = ath10k_pci_read32(ar, SOC_CORE_BASE_ADDRESS +
-				PCIE_INTR_CAUSE_ADDRESS);
-	ath10k_dbg(ar, ATH10K_DBG_BOOT, "boot host cpu intr cause: 0x%08x\n",
-		   val);
-
-	val = ath10k_pci_read32(ar, SOC_CORE_BASE_ADDRESS +
-				CPU_INTR_ADDRESS);
-	ath10k_dbg(ar, ATH10K_DBG_BOOT, "boot target cpu intr cause: 0x%08x\n",
-		   val);
-
-	/* disable pending irqs */
-	ath10k_pci_write32(ar, SOC_CORE_BASE_ADDRESS +
-			   PCIE_INTR_ENABLE_ADDRESS, 0);
-
-	ath10k_pci_write32(ar, SOC_CORE_BASE_ADDRESS +
-			   PCIE_INTR_CLR_ADDRESS, ~0);
-
-	msleep(100);
-
-	/* clear fw indicator */
 	ath10k_pci_write32(ar, FW_INDICATOR_ADDRESS, 0);
 
-	/* clear target LF timer interrupts */
 	val = ath10k_pci_read32(ar, RTC_SOC_BASE_ADDRESS +
-				SOC_LF_TIMER_CONTROL0_ADDRESS);
-	ath10k_pci_write32(ar, RTC_SOC_BASE_ADDRESS +
-			   SOC_LF_TIMER_CONTROL0_ADDRESS,
-			   val & ~SOC_LF_TIMER_CONTROL0_ENABLE_MASK);
+				SOC_RESET_CONTROL_ADDRESS);
+	ath10k_pci_write32(ar, RTC_SOC_BASE_ADDRESS + SOC_RESET_CONTROL_ADDRESS,
+			   val | SOC_RESET_CONTROL_CPU_WARM_RST_MASK);
+}
+
+static void ath10k_pci_warm_reset_ce(struct ath10k *ar)
+{
+	u32 val;
 
-	/* reset CE */
 	val = ath10k_pci_read32(ar, RTC_SOC_BASE_ADDRESS +
 				SOC_RESET_CONTROL_ADDRESS);
+
 	ath10k_pci_write32(ar, RTC_SOC_BASE_ADDRESS + SOC_RESET_CONTROL_ADDRESS,
 			   val | SOC_RESET_CONTROL_CE_RST_MASK);
-	val = ath10k_pci_read32(ar, RTC_SOC_BASE_ADDRESS +
-				SOC_RESET_CONTROL_ADDRESS);
 	msleep(10);
-
-	/* unreset CE */
 	ath10k_pci_write32(ar, RTC_SOC_BASE_ADDRESS + SOC_RESET_CONTROL_ADDRESS,
 			   val & ~SOC_RESET_CONTROL_CE_RST_MASK);
+}
+
+static void ath10k_pci_warm_reset_clear_lf(struct ath10k *ar)
+{
+	u32 val;
+
 	val = ath10k_pci_read32(ar, RTC_SOC_BASE_ADDRESS +
-				SOC_RESET_CONTROL_ADDRESS);
-	msleep(10);
+				SOC_LF_TIMER_CONTROL0_ADDRESS);
+	ath10k_pci_write32(ar, RTC_SOC_BASE_ADDRESS +
+			   SOC_LF_TIMER_CONTROL0_ADDRESS,
+			   val & ~SOC_LF_TIMER_CONTROL0_ENABLE_MASK);
+}
 
-	ath10k_pci_warm_reset_si0(ar);
+static int ath10k_pci_warm_reset(struct ath10k *ar)
+{
+	int ret;
+
+	ath10k_dbg(ar, ATH10K_DBG_BOOT, "boot warm reset\n");
 
-	/* debug */
-	val = ath10k_pci_read32(ar, SOC_CORE_BASE_ADDRESS +
-				PCIE_INTR_CAUSE_ADDRESS);
-	ath10k_dbg(ar, ATH10K_DBG_BOOT, "boot host cpu intr cause: 0x%08x\n",
-		   val);
+	spin_lock_bh(&ar->data_lock);
+	ar->stats.fw_warm_reset_counter++;
+	spin_unlock_bh(&ar->data_lock);
 
-	val = ath10k_pci_read32(ar, SOC_CORE_BASE_ADDRESS +
-				CPU_INTR_ADDRESS);
-	ath10k_dbg(ar, ATH10K_DBG_BOOT, "boot target cpu intr cause: 0x%08x\n",
-		   val);
+	ath10k_pci_irq_disable(ar);
 
-	/* CPU warm reset */
-	val = ath10k_pci_read32(ar, RTC_SOC_BASE_ADDRESS +
-				SOC_RESET_CONTROL_ADDRESS);
-	ath10k_pci_write32(ar, RTC_SOC_BASE_ADDRESS + SOC_RESET_CONTROL_ADDRESS,
-			   val | SOC_RESET_CONTROL_CPU_WARM_RST_MASK);
+	/* Make sure the target CPU is not doing anything dangerous, e.g. if it
+	 * were to access copy engine while host performs copy engine reset
+	 * then it is possible for the device to confuse pci-e controller to
+	 * the point of bringing host system to a complete stop (i.e. hang).
+	 */
+	ath10k_pci_warm_reset_si0(ar);
+	ath10k_pci_warm_reset_cpu(ar);
+	ath10k_pci_init_pipes(ar);
+	ath10k_pci_wait_for_target_init(ar);
 
-	val = ath10k_pci_read32(ar, RTC_SOC_BASE_ADDRESS +
-				SOC_RESET_CONTROL_ADDRESS);
-	ath10k_dbg(ar, ATH10K_DBG_BOOT, "boot target reset state: 0x%08x\n",
-		   val);
+	ath10k_pci_warm_reset_clear_lf(ar);
+	ath10k_pci_warm_reset_ce(ar);
+	ath10k_pci_warm_reset_cpu(ar);
+	ath10k_pci_init_pipes(ar);
 
-	msleep(100);
+	ret = ath10k_pci_wait_for_target_init(ar);
+	if (ret) {
+		ath10k_warn(ar, "failed to wait for target init: %d\n", ret);
+		return ret;
+	}
 
 	ath10k_dbg(ar, ATH10K_DBG_BOOT, "boot warm reset complete\n");
 
-- 
1.8.5.3

[PATCH v2 5/6] ath10k: split reset logic from power up

[Michal Kazior]

The power up procedure was overly complex due to
warm/cold reset workarounds and issues.

Signed-off-by: Michal Kazior <michal.kazior@tieto.com>
---

Notes:
    v2:
     * fix indentation [Kalle]

 drivers/net/wireless/ath/ath10k/pci.c | 151 ++++++++++++++++++----------------
 1 file changed, 80 insertions(+), 71 deletions(-)

diff --git a/drivers/net/wireless/ath/ath10k/pci.c b/drivers/net/wireless/ath/ath10k/pci.c
index 54213ac..afd6056 100644
--- a/drivers/net/wireless/ath/ath10k/pci.c
+++ b/drivers/net/wireless/ath/ath10k/pci.c
@@ -1782,10 +1782,86 @@ static int ath10k_pci_warm_reset(struct ath10k *ar)
 	return 0;
 }
 
-static int __ath10k_pci_hif_power_up(struct ath10k *ar, bool cold_reset)
+static int ath10k_pci_chip_reset(struct ath10k *ar)
+{
+	int i, ret;
+	u32 val;
+
+	ath10k_dbg(ar, ATH10K_DBG_BOOT, "boot chip reset\n");
+
+	/* Some hardware revisions (e.g. CUS223v2) has issues with cold reset.
+	 * It is thus preferred to use warm reset which is safer but may not be
+	 * able to recover the device from all possible fail scenarios.
+	 *
+	 * Warm reset doesn't always work on first try so attempt it a few
+	 * times before giving up.
+	 */
+	for (i = 0; i < ATH10K_PCI_NUM_WARM_RESET_ATTEMPTS; i++) {
+		ret = ath10k_pci_warm_reset(ar);
+		if (ret) {
+			ath10k_warn(ar, "failed to warm reset attempt %d of %d: %d\n",
+				    i + 1, ATH10K_PCI_NUM_WARM_RESET_ATTEMPTS,
+				    ret);
+			continue;
+		}
+
+		/* FIXME: Sometimes copy engine doesn't recover after warm
+		 * reset. In most cases this needs cold reset. In some of these
+		 * cases the device is in such a state that a cold reset may
+		 * lock up the host.
+		 *
+		 * Reading any host interest register via copy engine is
+		 * sufficient to verify if device is capable of booting
+		 * firmware blob.
+		 */
+		ret = ath10k_pci_init_pipes(ar);
+		if (ret) {
+			ath10k_warn(ar, "failed to init copy engine: %d\n",
+				    ret);
+			continue;
+		}
+
+		ret = ath10k_pci_diag_read32(ar, QCA988X_HOST_INTEREST_ADDRESS,
+					     &val);
+		if (ret) {
+			ath10k_warn(ar, "failed to poke copy engine: %d\n",
+				    ret);
+			continue;
+		}
+
+		ath10k_dbg(ar, ATH10K_DBG_BOOT, "boot chip reset complete (warm)\n");
+		return 0;
+	}
+
+	if (ath10k_pci_reset_mode == ATH10K_PCI_RESET_WARM_ONLY) {
+		ath10k_warn(ar, "refusing cold reset as requested\n");
+		return -EPERM;
+	}
+
+	ret = ath10k_pci_cold_reset(ar);
+	if (ret) {
+		ath10k_warn(ar, "failed to cold reset: %d\n", ret);
+		return ret;
+	}
+
+	ret = ath10k_pci_wait_for_target_init(ar);
+	if (ret) {
+		ath10k_warn(ar, "failed to wait for target after cold reset: %d\n",
+			    ret);
+		return ret;
+	}
+
+	ath10k_dbg(ar, ATH10K_DBG_BOOT, "boot chip reset complete (cold)\n");
+
+	return 0;
+}
+
+static int ath10k_pci_hif_power_up(struct ath10k *ar)
 {
 	int ret;
 
+	ath10k_dbg(ar, ATH10K_DBG_BOOT, "boot hif power up\n");
+
 	/*
 	 * Bring the target up cleanly.
 	 *
@@ -1796,13 +1872,9 @@ static int __ath10k_pci_hif_power_up(struct ath10k *ar, bool cold_reset)
 	 * is in an unexpected state. We try to catch that here in order to
 	 * reset the Target and retry the probe.
 	 */
-	if (cold_reset)
-		ret = ath10k_pci_cold_reset(ar);
-	else
-		ret = ath10k_pci_warm_reset(ar);
-
+	ret = ath10k_pci_chip_reset(ar);
 	if (ret) {
-		ath10k_err(ar, "failed to reset target: %d\n", ret);
+		ath10k_err(ar, "failed to reset chip: %d\n", ret);
 		goto err;
 	}
 
@@ -1812,12 +1884,6 @@ static int __ath10k_pci_hif_power_up(struct ath10k *ar, bool cold_reset)
 		goto err;
 	}
 
-	ret = ath10k_pci_wait_for_target_init(ar);
-	if (ret) {
-		ath10k_err(ar, "failed to wait for target to init: %d\n", ret);
-		goto err_ce;
-	}
-
 	ret = ath10k_pci_init_config(ar);
 	if (ret) {
 		ath10k_err(ar, "failed to setup init config: %d\n", ret);
@@ -1834,68 +1900,11 @@ static int __ath10k_pci_hif_power_up(struct ath10k *ar, bool cold_reset)
 
 err_ce:
 	ath10k_pci_ce_deinit(ar);
-	ath10k_pci_warm_reset(ar);
-err:
-	return ret;
-}
-
-static int ath10k_pci_hif_power_up_warm(struct ath10k *ar)
-{
-	int i, ret;
-
-	/*
-	 * Sometime warm reset succeeds after retries.
-	 *
-	 * FIXME: It might be possible to tune ath10k_pci_warm_reset() to work
-	 * at first try.
-	 */
-	for (i = 0; i < ATH10K_PCI_NUM_WARM_RESET_ATTEMPTS; i++) {
-		ret = __ath10k_pci_hif_power_up(ar, false);
-		if (ret == 0)
-			break;
-
-		ath10k_warn(ar, "failed to warm reset (attempt %d out of %d): %d\n",
-			    i + 1, ATH10K_PCI_NUM_WARM_RESET_ATTEMPTS, ret);
-	}
 
+err:
 	return ret;
 }
 
-static int ath10k_pci_hif_power_up(struct ath10k *ar)
-{
-	int ret;
-
-	ath10k_dbg(ar, ATH10K_DBG_BOOT, "boot hif power up\n");
-
-	/*
-	 * Hardware CUS232 version 2 has some issues with cold reset and the
-	 * preferred (and safer) way to perform a device reset is through a
-	 * warm reset.
-	 *
-	 * Warm reset doesn't always work though so fall back to cold reset may
-	 * be necessary.
-	 */
-	ret = ath10k_pci_hif_power_up_warm(ar);
-	if (ret) {
-		ath10k_warn(ar, "failed to power up target using warm reset: %d\n",
-			    ret);
-
-		if (ath10k_pci_reset_mode == ATH10K_PCI_RESET_WARM_ONLY)
-			return ret;
-
-		ath10k_warn(ar, "trying cold reset\n");
-
-		ret = __ath10k_pci_hif_power_up(ar, true);
-		if (ret) {
-			ath10k_err(ar, "failed to power up target using cold reset too (%d)\n",
-				   ret);
-			return ret;
-		}
-	}
-
-	return 0;
-}
-
 static void ath10k_pci_hif_power_down(struct ath10k *ar)
 {
 	ath10k_dbg(ar, ATH10K_DBG_BOOT, "boot hif power down\n");
-- 
1.8.5.3

[PATCH v2 6/6] ath10k: don't reset chip on power_down

[Michal Kazior]

Currently hif_power_up performs effectively a
reset and hif_stop resets the chip as well so
there's no point in resetting here.

Signed-off-by: Michal Kazior <michal.kazior@tieto.com>
---

Notes:
    v2:
     * this patch replaces `replace power up/down with reset callback` [Kalle]

 drivers/net/wireless/ath/ath10k/pci.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/net/wireless/ath/ath10k/pci.c b/drivers/net/wireless/ath/ath10k/pci.c
index afd6056..c1a2b84 100644
--- a/drivers/net/wireless/ath/ath10k/pci.c
+++ b/drivers/net/wireless/ath/ath10k/pci.c
@@ -1909,7 +1909,9 @@ static void ath10k_pci_hif_power_down(struct ath10k *ar)
 {
 	ath10k_dbg(ar, ATH10K_DBG_BOOT, "boot hif power down\n");
 
-	ath10k_pci_warm_reset(ar);
+	/* Currently hif_power_up performs effectively a reset and hif_stop
+	 * resets the chip as well so there's no point in resetting here.
+	 */
 }
 
 #ifdef CONFIG_PM
-- 
1.8.5.3

Re: [PATCH v2 4/6] ath10k: make warm reset a bit safer and faster

[Michal Kazior]

On 20 October 2014 14:14, Michal Kazior <michal.kazior@tieto.com> wrote:
> One of the problems with warm reset I've found is
> that it must be guaranteed that copy engine
> registers are not being accessed while being
> reset. Otherwise in worst case scenario the host
> may lock up.
>
> Instead of using sleeps and hoping the device is
> operational in some arbitrary timeframes use
> firmware indication register.
>
> As a side effect this makes driver
> boot/stop/recovery faster.
>
> Signed-off-by: Michal Kazior <michal.kazior@tieto.com>
> ---
>
> Notes:
>     v2:
>      * fix kernel panic on early fw crash due to CE not being fully initialized [Janusz]

Marek reported he sees a memory leak and git bisect blames this patch.
Please don't apply this patch yet.


Michał

Re: [PATCH v2 4/6] ath10k: make warm reset a bit safer and faster

[Kalle Valo]

Michal Kazior <michal.kazior@tieto.com> writes:

> On 20 October 2014 14:14, Michal Kazior <michal.kazior@tieto.com> wrote:
>> One of the problems with warm reset I've found is
>> that it must be guaranteed that copy engine
>> registers are not being accessed while being
>> reset. Otherwise in worst case scenario the host
>> may lock up.
>>
>> Instead of using sleeps and hoping the device is
>> operational in some arbitrary timeframes use
>> firmware indication register.
>>
>> As a side effect this makes driver
>> boot/stop/recovery faster.
>>
>> Signed-off-by: Michal Kazior <michal.kazior@tieto.com>
>> ---
>>
>> Notes:
>>     v2:
>>      * fix kernel panic on early fw crash due to CE not being fully initialized [Janusz]
>
> Marek reported he sees a memory leak and git bisect blames this patch.
> Please don't apply this patch yet.

Ok, thanks for letting me know. What about rest of the patches in the
patchset? I assume it's safe to apply them.

-- 
Kalle Valo

Re: [PATCH v2 4/6] ath10k: make warm reset a bit safer and faster

[Michal Kazior]

On 23 October 2014 15:14, Kalle Valo <kvalo@qca.qualcomm.com> wrote:
> Michal Kazior <michal.kazior@tieto.com> writes:
>
>> On 20 October 2014 14:14, Michal Kazior <michal.kazior@tieto.com> wrote:
>>> One of the problems with warm reset I've found is
>>> that it must be guaranteed that copy engine
>>> registers are not being accessed while being
>>> reset. Otherwise in worst case scenario the host
>>> may lock up.
>>>
>>> Instead of using sleeps and hoping the device is
>>> operational in some arbitrary timeframes use
>>> firmware indication register.
>>>
>>> As a side effect this makes driver
>>> boot/stop/recovery faster.
>>>
>>> Signed-off-by: Michal Kazior <michal.kazior@tieto.com>
>>> ---
>>>
>>> Notes:
>>>     v2:
>>>      * fix kernel panic on early fw crash due to CE not being fully initialized [Janusz]
>>
>> Marek reported he sees a memory leak and git bisect blames this patch.
>> Please don't apply this patch yet.
>
> Ok, thanks for letting me know. What about rest of the patches in the
> patchset? I assume it's safe to apply them.

It's safe to apply:

  ath10k: re-disable interrupts after target init
  ath10k: mask/unmask msi fw irq
  ath10k: split ce pipe init/alloc further

Please, do not apply the following yet:
  ath10k: make warm reset a bit safer and faster (this patch)
  ath10k: split reset logic from power up
  ath10k: don't reset chip on power_down


Michał

Re: [PATCH v2 0/6] ath10k: pci related fixes 2014-10-09

[Kalle Valo]

Michal Kazior <michal.kazior@tieto.com> writes:

> First 2 patches address some irq issues.
>
> Other 4 fix and cleanup device resetting.
>
> Hopefully this will make ath10k a little more
> robust when it comes to warm reset.
>
> As a side effect of the changes ath10k now boots a
> little faster (slightly over a second instead of
> two).
>
> v2:
>  * don't remove power_up/down [Kalle]
>  * fix CE crash [Janusz]
>  * remove irq workaround in patch #2
>
>
> Michal Kazior (6):
>   ath10k: re-disable interrupts after target init
>   ath10k: mask/unmask msi fw irq
>   ath10k: split ce pipe init/alloc further

I have applied these patches.

>   ath10k: make warm reset a bit safer and faster
>   ath10k: split reset logic from power up
>   ath10k: don't reset chip on power_down

And dropped these patches per your request.

-- 
Kalle Valo