[PATCH v2 0/6] ath10k: ath10k: fixes 2014-08-07, part 2

[PATCH v2 0/6] ath10k: ath10k: fixes 2014-08-07, part 2

[Michal Kazior]

Hi,

This is part 2 (of 3) of my patches. Split for
your reviewing pleasure.

This is related to irq handling only. Prepares for
more start/restart sequences fixes.

v2:
 * patch [1/6] is from part 1
 * patch [5/6] is new
 * some fixes, see notes
 * based on
   f2c7cb774e3e0a8152b1134015c245ba0abddfc6 and
   4 crash dump patches (v8) from Kalle


Michal Kazior (6):
  ath10k: move fw init print
  ath10k: fix legacy irq workaround
  ath10k: setup irq method in probe
  ath10k: split ce irq/handler setup
  ath10k: make sure to really disable irqs
  ath10k: remove early irq handling

 drivers/net/wireless/ath/ath10k/ce.c   |  36 ++--
 drivers/net/wireless/ath/ath10k/ce.h   |  12 +-
 drivers/net/wireless/ath/ath10k/core.c |   6 +-
 drivers/net/wireless/ath/ath10k/core.h |   1 -
 drivers/net/wireless/ath/ath10k/pci.c  | 320 +++++++++++++--------------------
 drivers/net/wireless/ath/ath10k/pci.h  |   1 -
 6 files changed, 142 insertions(+), 234 deletions(-)

-- 
1.8.5.3

[PATCH v2 1/6] ath10k: move fw init print

[Michal Kazior]

Firmware probing is done only once when driver is
registered and firmware version is guaranteed to
remain the same until driver is unregistered.

Signed-off-by: Michal Kazior <michal.kazior@tieto.com>
---
 drivers/net/wireless/ath/ath10k/core.c | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/drivers/net/wireless/ath/ath10k/core.c b/drivers/net/wireless/ath/ath10k/core.c
index 28f0ade..23ba321 100644
--- a/drivers/net/wireless/ath/ath10k/core.c
+++ b/drivers/net/wireless/ath/ath10k/core.c
@@ -815,11 +815,6 @@ int ath10k_core_start(struct ath10k *ar)
 
 	INIT_LIST_HEAD(&ar->arvifs);
 
-	if (!test_bit(ATH10K_FLAG_FIRST_BOOT_DONE, &ar->dev_flags))
-		ath10k_print_driver_info(ar);
-
-	__set_bit(ATH10K_FLAG_FIRST_BOOT_DONE, &ar->dev_flags);
-
 	return 0;
 
 err_hif_stop:
@@ -924,6 +919,7 @@ static int ath10k_core_probe_fw(struct ath10k *ar)
 		return ret;
 	}
 
+	ath10k_print_driver_info(ar);
 	ath10k_core_stop(ar);
 
 	mutex_unlock(&ar->conf_mutex);
-- 
1.8.5.3

[PATCH v2 2/6] ath10k: fix legacy irq workaround

[Michal Kazior]

Wrong register was being set up. This could
prevent firmware from booting in some rare cases
when using legacy interrupts.

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

diff --git a/drivers/net/wireless/ath/ath10k/pci.c b/drivers/net/wireless/ath/ath10k/pci.c
index bb1e473..4a7a5fe 100644
--- a/drivers/net/wireless/ath/ath10k/pci.c
+++ b/drivers/net/wireless/ath/ath10k/pci.c
@@ -2465,9 +2465,10 @@ 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_soc_write32(ar, PCIE_INTR_ENABLE_ADDRESS,
-					       PCIE_INTR_FIRMWARE_MASK |
-					       PCIE_INTR_CE_MASK_ALL);
+			ath10k_pci_write32(ar, SOC_CORE_BASE_ADDRESS +
+					   PCIE_INTR_ENABLE_ADDRESS,
+					   PCIE_INTR_FIRMWARE_MASK |
+					   PCIE_INTR_CE_MASK_ALL);
 
 		mdelay(10);
 	} while (time_before(jiffies, timeout));
-- 
1.8.5.3

[PATCH v2 3/6] ath10k: setup irq method in probe

[Michal Kazior]

It doesn't make sense to re-init irqs completely
whenever transport is started/stopped. Do it just
once upon probing/removing.

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

Notes:
    v2:
     * tweak print style [Kalle]
     * use ce_deinit instead of ce_init

 drivers/net/wireless/ath/ath10k/core.h |  1 -
 drivers/net/wireless/ath/ath10k/pci.c  | 73 ++++++++++++++++++----------------
 2 files changed, 39 insertions(+), 35 deletions(-)

diff --git a/drivers/net/wireless/ath/ath10k/core.h b/drivers/net/wireless/ath/ath10k/core.h
index 8da77c7..4ef4760 100644
--- a/drivers/net/wireless/ath/ath10k/core.h
+++ b/drivers/net/wireless/ath/ath10k/core.h
@@ -357,7 +357,6 @@ enum ath10k_fw_features {
 enum ath10k_dev_flags {
 	/* Indicates that ath10k device is during CAC phase of DFS */
 	ATH10K_CAC_RUNNING,
-	ATH10K_FLAG_FIRST_BOOT_DONE,
 	ATH10K_FLAG_CORE_REGISTERED,
 };
 
diff --git a/drivers/net/wireless/ath/ath10k/pci.c b/drivers/net/wireless/ath/ath10k/pci.c
index 4a7a5fe..c3b3f62 100644
--- a/drivers/net/wireless/ath/ath10k/pci.c
+++ b/drivers/net/wireless/ath/ath10k/pci.c
@@ -306,6 +306,18 @@ static void ath10k_pci_free_early_irq(struct ath10k *ar)
 	free_irq(ath10k_pci_priv(ar)->pdev->irq, ar);
 }
 
+static inline const char *ath10k_pci_get_irq_method(struct ath10k *ar)
+{
+	struct ath10k_pci *ar_pci = ath10k_pci_priv(ar);
+
+	if (ar_pci->num_msi_intrs > 1)
+		return "msi-x";
+	else if (ar_pci->num_msi_intrs == 1)
+		return "msi";
+	else
+		return "legacy";
+}
+
 /*
  * Diagnostic read/write access is provided for startup/config/debug usage.
  * Caller must guarantee proper alignment, when applicable, and single user
@@ -1922,8 +1934,6 @@ static int ath10k_pci_warm_reset(struct ath10k *ar)
 
 static int __ath10k_pci_hif_power_up(struct ath10k *ar, bool cold_reset)
 {
-	struct ath10k_pci *ar_pci = ath10k_pci_priv(ar);
-	const char *irq_mode;
 	int ret;
 
 	/*
@@ -1952,22 +1962,10 @@ static int __ath10k_pci_hif_power_up(struct ath10k *ar, bool cold_reset)
 		goto err;
 	}
 
-	ret = ath10k_ce_disable_interrupts(ar);
-	if (ret) {
-		ath10k_err("failed to disable CE interrupts: %d\n", ret);
-		goto err_ce;
-	}
-
-	ret = ath10k_pci_init_irq(ar);
-	if (ret) {
-		ath10k_err("failed to init irqs: %d\n", ret);
-		goto err_ce;
-	}
-
 	ret = ath10k_pci_request_early_irq(ar);
 	if (ret) {
 		ath10k_err("failed to request early irq: %d\n", ret);
-		goto err_deinit_irq;
+		goto err_ce;
 	}
 
 	ret = ath10k_pci_wait_for_target_init(ar);
@@ -1988,24 +1986,10 @@ static int __ath10k_pci_hif_power_up(struct ath10k *ar, bool cold_reset)
 		goto err_free_early_irq;
 	}
 
-	if (ar_pci->num_msi_intrs > 1)
-		irq_mode = "MSI-X";
-	else if (ar_pci->num_msi_intrs == 1)
-		irq_mode = "MSI";
-	else
-		irq_mode = "legacy";
-
-	if (!test_bit(ATH10K_FLAG_FIRST_BOOT_DONE, &ar->dev_flags))
-		ath10k_info("pci irq %s irq_mode %d reset_mode %d\n",
-			    irq_mode, ath10k_pci_irq_mode,
-			    ath10k_pci_reset_mode);
-
 	return 0;
 
 err_free_early_irq:
 	ath10k_pci_free_early_irq(ar);
-err_deinit_irq:
-	ath10k_pci_deinit_irq(ar);
 err_ce:
 	ath10k_pci_ce_deinit(ar);
 	ath10k_pci_warm_reset(ar);
@@ -2076,8 +2060,6 @@ static void ath10k_pci_hif_power_down(struct ath10k *ar)
 
 	ath10k_pci_free_early_irq(ar);
 	ath10k_pci_kill_tasklet(ar);
-	ath10k_pci_deinit_irq(ar);
-	ath10k_pci_ce_deinit(ar);
 	ath10k_pci_warm_reset(ar);
 }
 
@@ -2369,8 +2351,7 @@ static int ath10k_pci_init_irq(struct ath10k *ar)
 
 	ath10k_pci_init_irq_tasklets(ar);
 
-	if (ath10k_pci_irq_mode != ATH10K_PCI_IRQ_AUTO &&
-	    !test_bit(ATH10K_FLAG_FIRST_BOOT_DONE, &ar->dev_flags))
+	if (ath10k_pci_irq_mode != ATH10K_PCI_IRQ_AUTO)
 		ath10k_info("limiting irq mode to: %d\n", ath10k_pci_irq_mode);
 
 	/* Try MSI-X */
@@ -2655,14 +2636,36 @@ static int ath10k_pci_probe(struct pci_dev *pdev,
 		goto err_sleep;
 	}
 
+	ath10k_pci_ce_deinit(ar);
+
+	ret = ath10k_ce_disable_interrupts(ar);
+	if (ret) {
+		ath10k_err("failed to disable copy engine interrupts: %d\n",
+			   ret);
+		goto err_free_ce;
+	}
+
+	ret = ath10k_pci_init_irq(ar);
+	if (ret) {
+		ath10k_err("failed to init irqs: %d\n", ret);
+		goto err_free_ce;
+	}
+
+	ath10k_info("pci irq %s interrupts %d irq_mode %d reset_mode %d\n",
+		    ath10k_pci_get_irq_method(ar), ar_pci->num_msi_intrs,
+		    ath10k_pci_irq_mode, ath10k_pci_reset_mode);
+
 	ret = ath10k_core_register(ar, chip_id);
 	if (ret) {
 		ath10k_err("failed to register driver core: %d\n", ret);
-		goto err_free_ce;
+		goto err_deinit_irq;
 	}
 
 	return 0;
 
+err_deinit_irq:
+	ath10k_pci_deinit_irq(ar);
+
 err_free_ce:
 	ath10k_pci_free_ce(ar);
 
@@ -2694,6 +2697,8 @@ static void ath10k_pci_remove(struct pci_dev *pdev)
 		return;
 
 	ath10k_core_unregister(ar);
+	ath10k_pci_deinit_irq(ar);
+	ath10k_pci_ce_deinit(ar);
 	ath10k_pci_free_ce(ar);
 	ath10k_pci_sleep(ar);
 	ath10k_pci_release(ar);
-- 
1.8.5.3

[PATCH v2 4/6] ath10k: split ce irq/handler setup

[Michal Kazior]

It doesn't make much sense to overwrite send_cb
and recv_cb callbacks over and over again whenever
transport starts. Just make sure to unmask copy
engine interrupts when starting.

Signed-off-by: Michal Kazior <michal.kazior@tieto.com>
---
 drivers/net/wireless/ath/ath10k/ce.c  | 36 +++++++++++-------------------
 drivers/net/wireless/ath/ath10k/ce.h  | 12 ++++------
 drivers/net/wireless/ath/ath10k/pci.c | 41 ++++-------------------------------
 3 files changed, 21 insertions(+), 68 deletions(-)

diff --git a/drivers/net/wireless/ath/ath10k/ce.c b/drivers/net/wireless/ath/ath10k/ce.c
index 5117705..8cbc0ab 100644
--- a/drivers/net/wireless/ath/ath10k/ce.c
+++ b/drivers/net/wireless/ath/ath10k/ce.c
@@ -769,11 +769,11 @@ void ath10k_ce_per_engine_service_any(struct ath10k *ar)
  *
  * Called with ce_lock held.
  */
-static void ath10k_ce_per_engine_handler_adjust(struct ath10k_ce_pipe *ce_state,
-						int disable_copy_compl_intr)
+static void ath10k_ce_per_engine_handler_adjust(struct ath10k_ce_pipe *ce_state)
 {
 	u32 ctrl_addr = ce_state->ctrl_addr;
 	struct ath10k *ar = ce_state->ar;
+	bool disable_copy_compl_intr = ce_state->attr_flags & CE_ATTR_DIS_INTR;
 
 	if ((!disable_copy_compl_intr) &&
 	    (ce_state->send_cb || ce_state->recv_cb))
@@ -799,29 +799,13 @@ int ath10k_ce_disable_interrupts(struct ath10k *ar)
 	return 0;
 }
 
-void ath10k_ce_send_cb_register(struct ath10k_ce_pipe *ce_state,
-				void (*send_cb)(struct ath10k_ce_pipe *),
-				int disable_interrupts)
+void ath10k_ce_enable_interrupts(struct ath10k *ar)
 {
-	struct ath10k *ar = ce_state->ar;
-	struct ath10k_pci *ar_pci = ath10k_pci_priv(ar);
-
-	spin_lock_bh(&ar_pci->ce_lock);
-	ce_state->send_cb = send_cb;
-	ath10k_ce_per_engine_handler_adjust(ce_state, disable_interrupts);
-	spin_unlock_bh(&ar_pci->ce_lock);
-}
-
-void ath10k_ce_recv_cb_register(struct ath10k_ce_pipe *ce_state,
-				void (*recv_cb)(struct ath10k_ce_pipe *))
-{
-	struct ath10k *ar = ce_state->ar;
 	struct ath10k_pci *ar_pci = ath10k_pci_priv(ar);
+	int ce_id;
 
-	spin_lock_bh(&ar_pci->ce_lock);
-	ce_state->recv_cb = recv_cb;
-	ath10k_ce_per_engine_handler_adjust(ce_state, 0);
-	spin_unlock_bh(&ar_pci->ce_lock);
+	for (ce_id = 0; ce_id < CE_COUNT; ce_id++)
+		ath10k_ce_per_engine_handler_adjust(&ar_pci->ce_states[ce_id]);
 }
 
 static int ath10k_ce_init_src_ring(struct ath10k *ar,
@@ -1023,7 +1007,9 @@ 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)
+			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];
@@ -1046,6 +1032,10 @@ int ath10k_ce_init_pipe(struct ath10k *ar, unsigned int 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) {
diff --git a/drivers/net/wireless/ath/ath10k/ce.h b/drivers/net/wireless/ath/ath10k/ce.h
index 7a5a36f..d48dbb9 100644
--- a/drivers/net/wireless/ath/ath10k/ce.h
+++ b/drivers/net/wireless/ath/ath10k/ce.h
@@ -162,10 +162,6 @@ int ath10k_ce_send_nolock(struct ath10k_ce_pipe *ce_state,
 
 void __ath10k_ce_send_revert(struct ath10k_ce_pipe *pipe);
 
-void ath10k_ce_send_cb_register(struct ath10k_ce_pipe *ce_state,
-				void (*send_cb)(struct ath10k_ce_pipe *),
-				int disable_interrupts);
-
 int ath10k_ce_num_free_src_entries(struct ath10k_ce_pipe *pipe);
 
 /*==================Recv=======================*/
@@ -184,9 +180,6 @@ int ath10k_ce_recv_buf_enqueue(struct ath10k_ce_pipe *ce_state,
 			       void *per_transfer_recv_context,
 			       u32 buffer);
 
-void ath10k_ce_recv_cb_register(struct ath10k_ce_pipe *ce_state,
-				void (*recv_cb)(struct ath10k_ce_pipe *));
-
 /* recv flags */
 /* Data is byte-swapped */
 #define CE_RECV_FLAG_SWAPPED	1
@@ -214,7 +207,9 @@ int ath10k_ce_completed_send_next(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);
+			const struct ce_attr *attr,
+			void (*send_cb)(struct ath10k_ce_pipe *),
+			void (*recv_cb)(struct ath10k_ce_pipe *));
 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);
@@ -245,6 +240,7 @@ int ath10k_ce_cancel_send_next(struct ath10k_ce_pipe *ce_state,
 void ath10k_ce_per_engine_service_any(struct ath10k *ar);
 void ath10k_ce_per_engine_service(struct ath10k *ar, unsigned int ce_id);
 int ath10k_ce_disable_interrupts(struct ath10k *ar);
+void ath10k_ce_enable_interrupts(struct ath10k *ar);
 
 /* ce_attr.flags values */
 /* Use NonSnooping PCIe accesses? */
diff --git a/drivers/net/wireless/ath/ath10k/pci.c b/drivers/net/wireless/ath/ath10k/pci.c
index c3b3f62..c764dd7 100644
--- a/drivers/net/wireless/ath/ath10k/pci.c
+++ b/drivers/net/wireless/ath/ath10k/pci.c
@@ -941,37 +941,6 @@ static void ath10k_pci_hif_set_callbacks(struct ath10k *ar,
 	       sizeof(ar_pci->msg_callbacks_current));
 }
 
-static int ath10k_pci_setup_ce_irq(struct ath10k *ar)
-{
-	struct ath10k_pci *ar_pci = ath10k_pci_priv(ar);
-	const struct ce_attr *attr;
-	struct ath10k_pci_pipe *pipe_info;
-	int pipe_num, disable_interrupts;
-
-	for (pipe_num = 0; pipe_num < CE_COUNT; pipe_num++) {
-		pipe_info = &ar_pci->pipe_info[pipe_num];
-
-		/* Handle Diagnostic CE specially */
-		if (pipe_info->ce_hdl == ar_pci->ce_diag)
-			continue;
-
-		attr = &host_ce_config_wlan[pipe_num];
-
-		if (attr->src_nentries) {
-			disable_interrupts = attr->flags & CE_ATTR_DIS_INTR;
-			ath10k_ce_send_cb_register(pipe_info->ce_hdl,
-						   ath10k_pci_ce_send_done,
-						   disable_interrupts);
-		}
-
-		if (attr->dest_nentries)
-			ath10k_ce_recv_cb_register(pipe_info->ce_hdl,
-						   ath10k_pci_ce_recv_data);
-	}
-
-	return 0;
-}
-
 static void ath10k_pci_kill_tasklet(struct ath10k *ar)
 {
 	struct ath10k_pci *ar_pci = ath10k_pci_priv(ar);
@@ -1169,11 +1138,7 @@ static int ath10k_pci_hif_start(struct ath10k *ar)
 		goto err_early_irq;
 	}
 
-	ret = ath10k_pci_setup_ce_irq(ar);
-	if (ret) {
-		ath10k_warn("failed to setup CE interrupts: %d\n", ret);
-		goto err_stop;
-	}
+	ath10k_ce_enable_interrupts(ar);
 
 	/* Post buffers once to start things off. */
 	ret = ath10k_pci_post_rx(ar);
@@ -1786,7 +1751,9 @@ static int ath10k_pci_ce_init(struct ath10k *ar)
 		pipe_info->hif_ce_state = ar;
 		attr = &host_ce_config_wlan[pipe_num];
 
-		ret = ath10k_ce_init_pipe(ar, pipe_num, attr);
+		ret = ath10k_ce_init_pipe(ar, pipe_num, attr,
+					  ath10k_pci_ce_send_done,
+					  ath10k_pci_ce_recv_data);
 		if (ret) {
 			ath10k_err("failed to initialize copy engine pipe %d: %d\n",
 				   pipe_num, ret);
-- 
1.8.5.3

[PATCH v2 5/6] ath10k: make sure to really disable irqs

[Michal Kazior]

This fixes two corner cases.

One is a race between disabling copy engine
interrupts and unhandled pending interrupts on the
host. This could end up with a runaway tasklet and
consequently memory leak of a few copy engine
rx buffers.

The other one is an unexpected (and non-maskable
via device CSR) MSI fw indication interrupt during
teardown. This could trigger the same problem as
the first corner case.

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

diff --git a/drivers/net/wireless/ath/ath10k/pci.c b/drivers/net/wireless/ath/ath10k/pci.c
index c764dd7..6224952 100644
--- a/drivers/net/wireless/ath/ath10k/pci.c
+++ b/drivers/net/wireless/ath/ath10k/pci.c
@@ -1121,6 +1121,40 @@ static int ath10k_pci_post_rx(struct ath10k *ar)
 	return 0;
 }
 
+static void ath10k_pci_irq_disable(struct ath10k *ar)
+{
+	struct ath10k_pci *ar_pci = ath10k_pci_priv(ar);
+	int i;
+
+	ath10k_ce_disable_interrupts(ar);
+
+	/* Regardless how many interrupts were assigned for MSI the first one
+	 * is always used for firmware indications (crashes). There's no way to
+	 * mask the irq in the device so call disable_irq(). Legacy (shared)
+	 * interrupts can be masked on the device though.
+	 */
+	if (ar_pci->num_msi_intrs > 0)
+		disable_irq(ar_pci->pdev->irq);
+	else
+		ath10k_pci_disable_and_clear_legacy_irq(ar);
+
+	for (i = 0; i < max(1, ar_pci->num_msi_intrs); i++)
+		synchronize_irq(ar_pci->pdev->irq + i);
+}
+
+static void ath10k_pci_irq_enable(struct ath10k *ar)
+{
+	struct ath10k_pci *ar_pci = ath10k_pci_priv(ar);
+
+	ath10k_ce_enable_interrupts(ar);
+
+	/* See comment in ath10k_pci_irq_disable() */
+	if (ar_pci->num_msi_intrs > 0)
+		enable_irq(ar_pci->pdev->irq);
+	else
+		ath10k_pci_enable_legacy_irq(ar);
+}
+
 static int ath10k_pci_hif_start(struct ath10k *ar)
 {
 	struct ath10k_pci *ar_pci = ath10k_pci_priv(ar);
@@ -1138,7 +1172,7 @@ static int ath10k_pci_hif_start(struct ath10k *ar)
 		goto err_early_irq;
 	}
 
-	ath10k_ce_enable_interrupts(ar);
+	ath10k_pci_irq_enable(ar);
 
 	/* Post buffers once to start things off. */
 	ret = ath10k_pci_post_rx(ar);
@@ -1152,7 +1186,7 @@ static int ath10k_pci_hif_start(struct ath10k *ar)
 	return 0;
 
 err_stop:
-	ath10k_ce_disable_interrupts(ar);
+	ath10k_pci_irq_disable(ar);
 	ath10k_pci_free_irq(ar);
 	ath10k_pci_kill_tasklet(ar);
 err_early_irq:
@@ -1275,10 +1309,7 @@ static void ath10k_pci_hif_stop(struct ath10k *ar)
 	if (WARN_ON(!ar_pci->started))
 		return;
 
-	ret = ath10k_ce_disable_interrupts(ar);
-	if (ret)
-		ath10k_warn("failed to disable CE interrupts: %d\n", ret);
-
+	ath10k_pci_irq_disable(ar);
 	ath10k_pci_free_irq(ar);
 	ath10k_pci_kill_tasklet(ar);
 
-- 
1.8.5.3

[PATCH v2 6/6] ath10k: remove early irq handling

[Michal Kazior]

It's not really necessary to have a dedicated irq
handler just for the sake of catching early fw
crashes anymore. It is now safe to use one handler
even during early stages of device boot up.

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

Notes:
    v2:
     * remove early_tasket_struct [Kalle]
     * s/ath10k_pci_fw_crashed/ath10k_pci_has_fw_crashed [Kalle]
     * perform warm reset before initializing irq

 drivers/net/wireless/ath/ath10k/pci.c | 172 ++++++++++------------------------
 drivers/net/wireless/ath/ath10k/pci.h |   1 -
 2 files changed, 48 insertions(+), 125 deletions(-)

diff --git a/drivers/net/wireless/ath/ath10k/pci.c b/drivers/net/wireless/ath/ath10k/pci.c
index 6224952..ffb980c 100644
--- a/drivers/net/wireless/ath/ath10k/pci.c
+++ b/drivers/net/wireless/ath/ath10k/pci.c
@@ -266,46 +266,6 @@ static void ath10k_pci_enable_legacy_irq(struct ath10k *ar)
 				 PCIE_INTR_ENABLE_ADDRESS);
 }
 
-static irqreturn_t ath10k_pci_early_irq_handler(int irq, void *arg)
-{
-	struct ath10k *ar = arg;
-	struct ath10k_pci *ar_pci = ath10k_pci_priv(ar);
-
-	if (ar_pci->num_msi_intrs == 0) {
-		if (!ath10k_pci_irq_pending(ar))
-			return IRQ_NONE;
-
-		ath10k_pci_disable_and_clear_legacy_irq(ar);
-	}
-
-	tasklet_schedule(&ar_pci->early_irq_tasklet);
-
-	return IRQ_HANDLED;
-}
-
-static int ath10k_pci_request_early_irq(struct ath10k *ar)
-{
-	struct ath10k_pci *ar_pci = ath10k_pci_priv(ar);
-	int ret;
-
-	/* Regardless whether MSI-X/MSI/legacy irqs have been set up the first
-	 * interrupt from irq vector is triggered in all cases for FW
-	 * indication/errors */
-	ret = request_irq(ar_pci->pdev->irq, ath10k_pci_early_irq_handler,
-			  IRQF_SHARED, "ath10k_pci (early)", ar);
-	if (ret) {
-		ath10k_warn("failed to request early irq: %d\n", ret);
-		return ret;
-	}
-
-	return 0;
-}
-
-static void ath10k_pci_free_early_irq(struct ath10k *ar)
-{
-	free_irq(ath10k_pci_priv(ar)->pdev->irq, ar);
-}
-
 static inline const char *ath10k_pci_get_irq_method(struct ath10k *ar)
 {
 	struct ath10k_pci *ar_pci = ath10k_pci_priv(ar);
@@ -948,7 +908,6 @@ static void ath10k_pci_kill_tasklet(struct ath10k *ar)
 
 	tasklet_kill(&ar_pci->intr_tq);
 	tasklet_kill(&ar_pci->msi_fw_err);
-	tasklet_kill(&ar_pci->early_irq_tasklet);
 
 	for (i = 0; i < CE_COUNT; i++)
 		tasklet_kill(&ar_pci->pipe_info[i].intr);
@@ -1158,20 +1117,10 @@ static void ath10k_pci_irq_enable(struct ath10k *ar)
 static int ath10k_pci_hif_start(struct ath10k *ar)
 {
 	struct ath10k_pci *ar_pci = ath10k_pci_priv(ar);
-	int ret, ret_early;
+	int ret;
 
 	ath10k_dbg(ATH10K_DBG_BOOT, "boot hif start\n");
 
-	ath10k_pci_free_early_irq(ar);
-	ath10k_pci_kill_tasklet(ar);
-
-	ret = ath10k_pci_request_irq(ar);
-	if (ret) {
-		ath10k_warn("failed to post RX buffers for all pipes: %d\n",
-			    ret);
-		goto err_early_irq;
-	}
-
 	ath10k_pci_irq_enable(ar);
 
 	/* Post buffers once to start things off. */
@@ -1187,15 +1136,7 @@ static int ath10k_pci_hif_start(struct ath10k *ar)
 
 err_stop:
 	ath10k_pci_irq_disable(ar);
-	ath10k_pci_free_irq(ar);
 	ath10k_pci_kill_tasklet(ar);
-err_early_irq:
-	/* Though there should be no interrupts (device was reset)
-	 * power_down() expects the early IRQ to be installed as per the
-	 * driver lifecycle. */
-	ret_early = ath10k_pci_request_early_irq(ar);
-	if (ret_early)
-		ath10k_warn("failed to re-enable early irq: %d\n", ret_early);
 
 	return ret;
 }
@@ -1302,7 +1243,6 @@ static void ath10k_pci_ce_deinit(struct ath10k *ar)
 static void ath10k_pci_hif_stop(struct ath10k *ar)
 {
 	struct ath10k_pci *ar_pci = ath10k_pci_priv(ar);
-	int ret;
 
 	ath10k_dbg(ATH10K_DBG_BOOT, "boot hif stop\n");
 
@@ -1310,17 +1250,7 @@ static void ath10k_pci_hif_stop(struct ath10k *ar)
 		return;
 
 	ath10k_pci_irq_disable(ar);
-	ath10k_pci_free_irq(ar);
 	ath10k_pci_kill_tasklet(ar);
-
-	ret = ath10k_pci_request_early_irq(ar);
-	if (ret)
-		ath10k_warn("failed to re-enable early irq: %d\n", ret);
-
-	/* At this point, asynchronous threads are stopped, the target should
-	 * not DMA nor interrupt. We process the leftovers and then free
-	 * everything else up. */
-
 	ath10k_pci_buffer_cleanup(ar);
 
 	/* Make the sure the device won't access any structures on the host by
@@ -1806,28 +1736,19 @@ static int ath10k_pci_ce_init(struct ath10k *ar)
 	return 0;
 }
 
-static void ath10k_pci_fw_interrupt_handler(struct ath10k *ar)
+static bool ath10k_pci_has_fw_crashed(struct ath10k *ar)
 {
-	struct ath10k_pci *ar_pci = ath10k_pci_priv(ar);
-	u32 fw_indicator;
-
-	fw_indicator = ath10k_pci_read32(ar, FW_INDICATOR_ADDRESS);
+	return ath10k_pci_read32(ar, FW_INDICATOR_ADDRESS) &
+	       FW_IND_EVENT_PENDING;
+}
 
-	if (fw_indicator & FW_IND_EVENT_PENDING) {
-		/* ACK: clear Target-side pending event */
-		ath10k_pci_write32(ar, FW_INDICATOR_ADDRESS,
-				   fw_indicator & ~FW_IND_EVENT_PENDING);
+static void ath10k_pci_fw_crashed_clear(struct ath10k *ar)
+{
+	u32 val;
 
-		if (ar_pci->started) {
-			ath10k_pci_fw_crashed_dump(ar);
-		} else {
-			/*
-			 * Probable Target failure before we're prepared
-			 * to handle it.  Generally unexpected.
-			 */
-			ath10k_warn("early firmware event indicated\n");
-		}
-	}
+	val = ath10k_pci_read32(ar, FW_INDICATOR_ADDRESS);
+	val &= ~FW_IND_EVENT_PENDING;
+	ath10k_pci_write32(ar, FW_INDICATOR_ADDRESS, val);
 }
 
 /* this function effectively clears target memory controller assert line */
@@ -1960,34 +1881,26 @@ static int __ath10k_pci_hif_power_up(struct ath10k *ar, bool cold_reset)
 		goto err;
 	}
 
-	ret = ath10k_pci_request_early_irq(ar);
-	if (ret) {
-		ath10k_err("failed to request early irq: %d\n", ret);
-		goto err_ce;
-	}
-
 	ret = ath10k_pci_wait_for_target_init(ar);
 	if (ret) {
 		ath10k_err("failed to wait for target to init: %d\n", ret);
-		goto err_free_early_irq;
+		goto err_ce;
 	}
 
 	ret = ath10k_pci_init_config(ar);
 	if (ret) {
 		ath10k_err("failed to setup init config: %d\n", ret);
-		goto err_free_early_irq;
+		goto err_ce;
 	}
 
 	ret = ath10k_pci_wake_target_cpu(ar);
 	if (ret) {
 		ath10k_err("could not wake up target CPU: %d\n", ret);
-		goto err_free_early_irq;
+		goto err_ce;
 	}
 
 	return 0;
 
-err_free_early_irq:
-	ath10k_pci_free_early_irq(ar);
 err_ce:
 	ath10k_pci_ce_deinit(ar);
 	ath10k_pci_warm_reset(ar);
@@ -2056,8 +1969,6 @@ static void ath10k_pci_hif_power_down(struct ath10k *ar)
 {
 	ath10k_dbg(ATH10K_DBG_BOOT, "boot hif power down\n");
 
-	ath10k_pci_free_early_irq(ar);
-	ath10k_pci_kill_tasklet(ar);
 	ath10k_pci_warm_reset(ar);
 }
 
@@ -2140,7 +2051,13 @@ static void ath10k_msi_err_tasklet(unsigned long data)
 {
 	struct ath10k *ar = (struct ath10k *)data;
 
-	ath10k_pci_fw_interrupt_handler(ar);
+	if (!ath10k_pci_has_fw_crashed(ar)) {
+		ath10k_warn("received unsolicited fw crash interrupt\n");
+		return;
+	}
+
+	ath10k_pci_fw_crashed_clear(ar);
+	ath10k_pci_fw_crashed_dump(ar);
 }
 
 /*
@@ -2201,27 +2118,17 @@ static irqreturn_t ath10k_pci_interrupt_handler(int irq, void *arg)
 	return IRQ_HANDLED;
 }
 
-static void ath10k_pci_early_irq_tasklet(unsigned long data)
+static void ath10k_pci_tasklet(unsigned long data)
 {
 	struct ath10k *ar = (struct ath10k *)data;
-	u32 fw_ind;
+	struct ath10k_pci *ar_pci = ath10k_pci_priv(ar);
 
-	fw_ind = ath10k_pci_read32(ar, FW_INDICATOR_ADDRESS);
-	if (fw_ind & FW_IND_EVENT_PENDING) {
-		ath10k_pci_write32(ar, FW_INDICATOR_ADDRESS,
-				   fw_ind & ~FW_IND_EVENT_PENDING);
+	if (ath10k_pci_has_fw_crashed(ar)) {
+		ath10k_pci_fw_crashed_clear(ar);
 		ath10k_pci_fw_crashed_dump(ar);
+		return;
 	}
 
-	ath10k_pci_enable_legacy_irq(ar);
-}
-
-static void ath10k_pci_tasklet(unsigned long data)
-{
-	struct ath10k *ar = (struct ath10k *)data;
-	struct ath10k_pci *ar_pci = ath10k_pci_priv(ar);
-
-	ath10k_pci_fw_interrupt_handler(ar); /* FIXME: Handle FW error */
 	ath10k_ce_per_engine_service_any(ar);
 
 	/* Re-enable legacy irq that was disabled in the irq handler */
@@ -2332,8 +2239,6 @@ static void ath10k_pci_init_irq_tasklets(struct ath10k *ar)
 	tasklet_init(&ar_pci->intr_tq, ath10k_pci_tasklet, (unsigned long)ar);
 	tasklet_init(&ar_pci->msi_fw_err, ath10k_msi_err_tasklet,
 		     (unsigned long)ar);
-	tasklet_init(&ar_pci->early_irq_tasklet, ath10k_pci_early_irq_tasklet,
-		     (unsigned long)ar);
 
 	for (i = 0; i < CE_COUNT; i++) {
 		ar_pci->pipe_info[i].ar_pci = ar_pci;
@@ -2459,8 +2364,7 @@ static int ath10k_pci_wait_for_target_init(struct ath10k *ar)
 
 	if (val & FW_IND_EVENT_PENDING) {
 		ath10k_warn("device has crashed during init\n");
-		ath10k_pci_write32(ar, FW_INDICATOR_ADDRESS,
-				   val & ~FW_IND_EVENT_PENDING);
+		ath10k_pci_fw_crashed_clear(ar);
 		ath10k_pci_fw_crashed_dump(ar);
 		return -ECOMM;
 	}
@@ -2643,6 +2547,13 @@ static int ath10k_pci_probe(struct pci_dev *pdev,
 		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);
+
 	ret = ath10k_pci_init_irq(ar);
 	if (ret) {
 		ath10k_err("failed to init irqs: %d\n", ret);
@@ -2653,14 +2564,26 @@ static int ath10k_pci_probe(struct pci_dev *pdev,
 		    ath10k_pci_get_irq_method(ar), ar_pci->num_msi_intrs,
 		    ath10k_pci_irq_mode, ath10k_pci_reset_mode);
 
+	ret = ath10k_pci_request_irq(ar);
+	if (ret) {
+		ath10k_warn("failed to request irqs: %d\n", ret);
+		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("failed to register driver core: %d\n", ret);
-		goto err_deinit_irq;
+		goto err_free_irq;
 	}
 
 	return 0;
 
+err_free_irq:
+	ath10k_pci_free_irq(ar);
+
 err_deinit_irq:
 	ath10k_pci_deinit_irq(ar);
 
@@ -2695,6 +2618,7 @@ static void ath10k_pci_remove(struct pci_dev *pdev)
 		return;
 
 	ath10k_core_unregister(ar);
+	ath10k_pci_free_irq(ar);
 	ath10k_pci_deinit_irq(ar);
 	ath10k_pci_ce_deinit(ar);
 	ath10k_pci_free_ce(ar);
diff --git a/drivers/net/wireless/ath/ath10k/pci.h b/drivers/net/wireless/ath/ath10k/pci.h
index 294a72e..caed918 100644
--- a/drivers/net/wireless/ath/ath10k/pci.h
+++ b/drivers/net/wireless/ath/ath10k/pci.h
@@ -166,7 +166,6 @@ struct ath10k_pci {
 
 	struct tasklet_struct intr_tq;
 	struct tasklet_struct msi_fw_err;
-	struct tasklet_struct early_irq_tasklet;
 
 	int started;
 
-- 
1.8.5.3

Re: [PATCH v2 0/6] ath10k: ath10k: fixes 2014-08-07, part 2

[Michal Kazior]

On 22 August 2014 14:23, Michal Kazior <michal.kazior@tieto.com> wrote:
> Hi,
>
> This is part 2 (of 3) of my patches. Split for
> your reviewing pleasure.
>
> This is related to irq handling only. Prepares for
> more start/restart sequences fixes.
>
> v2:
>  * patch [1/6] is from part 1
>  * patch [5/6] is new
>  * some fixes, see notes
>  * based on
>    f2c7cb774e3e0a8152b1134015c245ba0abddfc6 and
>    4 crash dump patches (v8) from Kalle

Forgot to add I also dropped `ath10k: unmask ce irqs after posting rx
buffers` as it didn't really make much sense to swap the order.


Michał

Re: [PATCH v2 0/6] ath10k: ath10k: fixes 2014-08-07, part 2

[Kalle Valo]

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

> Hi,
>
> This is part 2 (of 3) of my patches. Split for
> your reviewing pleasure.
>
> This is related to irq handling only. Prepares for
> more start/restart sequences fixes.
>
> v2:
>  * patch [1/6] is from part 1
>  * patch [5/6] is new
>  * some fixes, see notes
>  * based on
>    f2c7cb774e3e0a8152b1134015c245ba0abddfc6 and
>    4 crash dump patches (v8) from Kalle
>
>
> Michal Kazior (6):
>   ath10k: move fw init print
>   ath10k: fix legacy irq workaround
>   ath10k: setup irq method in probe
>   ath10k: split ce irq/handler setup
>   ath10k: make sure to really disable irqs
>   ath10k: remove early irq handling

Thanks, all six patches applied.

-- 
Kalle Valo