[ath9k-devel] [PATCH 0/6] ath10k: pci cleanup

[ath9k-devel] [PATCH 0/6] ath10k: pci cleanup

[Michal Kazior]

This gets rid of the ugly device/target resetting
retries. It also tries to simplify pci probing
logic.

Michal Kazior (6):
  ath10k: fix pci_set_drvpriv ordering
  ath10k: refactor pci interrupt functions
  ath10k: remove pci probe retrying
  ath10k: remove ath10k_pci_configure()
  ath10k: remove ath10k_pci_probe_device()
  ath10k: simplify pci target reset

 drivers/net/wireless/ath/ath10k/pci.c |  452 ++++++++++++++-------------------
 1 file changed, 186 insertions(+), 266 deletions(-)

-- 
1.7.9.5

[ath9k-devel] [PATCH 1/6] ath10k: fix pci_set_drvpriv ordering

[Michal Kazior]

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

diff --git a/drivers/net/wireless/ath/ath10k/pci.c b/drivers/net/wireless/ath/ath10k/pci.c
index 0d1523b..5819c21 100644
--- a/drivers/net/wireless/ath/ath10k/pci.c
+++ b/drivers/net/wireless/ath/ath10k/pci.c
@@ -2082,7 +2082,6 @@ err_stalled:
 	ath10k_pci_nointrs(ar);
 err_intr:
 	pci_disable_msi(ar_pci->pdev);
-	pci_set_drvdata(ar_pci->pdev, NULL);
 
 	return ret;
 }
@@ -2093,7 +2092,6 @@ static void ath10k_pci_teardown(struct ath10k *ar)
 
 	ath10k_pci_nointrs(ar);
 	pci_disable_msi(ar_pci->pdev);
-	pci_set_drvdata(ar_pci->pdev, NULL);
 }
 
 static void ath10k_pci_device_reset(struct ath10k_pci *ar_pci)
@@ -2336,7 +2334,6 @@ retry:
 	return 0;
 
 err_tgtstate:
-	pci_set_drvdata(pdev, NULL);
 	ath10k_pci_device_reset(ar_pci);
 err_iomap:
 	pci_iounmap(pdev, mem);
@@ -2347,6 +2344,7 @@ err_region:
 err_device:
 	pci_disable_device(pdev);
 err_ar:
+	pci_set_drvdata(pdev, NULL);
 	ath10k_core_destroy(ar);
 err_ar_pci:
 	/* call HIF PCI free here */
-- 
1.7.9.5

[ath9k-devel] [PATCH 2/6] ath10k: refactor pci interrupt functions

[Michal Kazior]

This patch splits the interrupt registering
function into separate functions. Hopefully this
is more readable and maintanable.

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

diff --git a/drivers/net/wireless/ath/ath10k/pci.c b/drivers/net/wireless/ath/ath10k/pci.c
index 5819c21..df13454 100644
--- a/drivers/net/wireless/ath/ath10k/pci.c
+++ b/drivers/net/wireless/ath/ath10k/pci.c
@@ -1861,47 +1861,99 @@ static void ath10k_pci_tasklet(unsigned long data)
 	}
 }
 
-static void ath10k_pci_nointrs(struct ath10k *ar)
+static int ath10k_pci_start_intr_msix(struct ath10k *ar, int num)
 {
-	int i;
 	struct ath10k_pci *ar_pci = ath10k_pci_priv(ar);
+	int ret;
+	int i;
 
-	if (ar_pci->num_msi_intrs > 0) {
-		/* MSI interrupt(s) */
-		for (i = 0; i < ar_pci->num_msi_intrs; i++)
+	ret = pci_enable_msi_block(ar_pci->pdev, num);
+	if (ret)
+		return ret;
+
+	ar_pci->num_msi_intrs = num;
+	ret = request_irq(ar_pci->pdev->irq + MSI_ASSIGN_FW,
+			  ath10k_pci_msi_fw_handler,
+			  IRQF_SHARED, "ath10k_pci", ar);
+	if (ret)
+		return ret;
+
+	for (i = MSI_ASSIGN_CE_INITIAL; i <= MSI_ASSIGN_CE_MAX; i++) {
+		ret = request_irq(ar_pci->pdev->irq + i,
+				  ath10k_pci_per_engine_handler,
+				  IRQF_SHARED, "ath10k_pci", ar);
+		if (ret) {
+			ath10k_warn("request_irq(%d) failed %d\n",
+				    ar_pci->pdev->irq + i, ret);
+
+			for (; i >= MSI_ASSIGN_CE_INITIAL; i--)
+				free_irq(ar_pci->pdev->irq, ar);
+
+			pci_disable_msi(ar_pci->pdev);
+			return ret;
+		}
+	}
+
+	ret = request_irq(ar_pci->pdev->irq,
+			  ath10k_pci_interrupt_handler,
+			  IRQF_SHARED, "ath10k_pci", ar);
+	if (ret) {
+		for (i = MSI_ASSIGN_CE_INITIAL; i <= MSI_ASSIGN_CE_MAX; i++)
 			free_irq(ar_pci->pdev->irq + i, ar);
-		ar_pci->num_msi_intrs = 0;
-	} else
-		/* Legacy PCI line interrupt */
-		free_irq(ar_pci->pdev->irq, ar);
+
+		pci_disable_msi(ar_pci->pdev);
+		return ret;
+	}
+
+	return 0;
 }
 
-static int ath10k_pci_reset_target(struct ath10k *ar)
+static int ath10k_pci_start_intr_msi(struct ath10k *ar)
 {
 	struct ath10k_pci *ar_pci = ath10k_pci_priv(ar);
-	int wait_limit = 300; /* 3 sec */
+	int ret;
 
-	while (wait_limit-- &&
-	       !(ioread32(ar_pci->mem + FW_INDICATOR_ADDRESS) &
-		 FW_IND_INITIALIZED)) {
+	ret = pci_enable_msi(ar_pci->pdev);
+	if (ret < 0)
+		return ret;
 
-		if (ar_pci->num_msi_intrs == 0)
-			/* Fix potential race by repeating CORE_BASE writes */
-			iowrite32(PCIE_INTR_FIRMWARE_MASK |
-				  PCIE_INTR_CE_MASK_ALL,
-				  ar_pci->mem + (SOC_CORE_BASE_ADDRESS |
-						 PCIE_INTR_ENABLE_ADDRESS));
-		mdelay(10);
+	ret = request_irq(ar_pci->pdev->irq,
+			  ath10k_pci_interrupt_handler,
+			  IRQF_SHARED, "ath10k_pci", ar);
+	if (ret < 0) {
+		pci_disable_msi(ar_pci->pdev);
+		return ret;
 	}
 
-	if (wait_limit < 0) {
-		ath10k_err("Target stalled\n");
-		iowrite32(PCIE_SOC_WAKE_RESET,
-			  ar_pci->mem + PCIE_LOCAL_BASE_ADDRESS +
-			  PCIE_SOC_WAKE_ADDRESS);
-		return -EIO;
-	}
+	return 0;
+}
+
+static int ath10k_pci_start_intr_legacy(struct ath10k *ar)
+{
+	struct ath10k_pci *ar_pci = ath10k_pci_priv(ar);
 
+	/*
+	 * Make sure to wake the Target before enabling Legacy
+	 * Interrupt.
+	 */
+	iowrite32(PCIE_SOC_WAKE_V_MASK,
+		  ar_pci->mem + PCIE_LOCAL_BASE_ADDRESS +
+		  PCIE_SOC_WAKE_ADDRESS);
+
+	ath10k_pci_wait_for_target_to_awake(ar);
+
+	/*
+	 * A potential race occurs here: The CORE_BASE write
+	 * depends on target correctly decoding AXI address but
+	 * host won't know when target writes BAR to CORE_CTRL.
+	 * This write might get lost if target has NOT written BAR.
+	 * For now, fix the race by repeating the write in below
+	 * synchronization checking.
+	 */
+	iowrite32(PCIE_INTR_FIRMWARE_MASK |
+		  PCIE_INTR_CE_MASK_ALL,
+		  ar_pci->mem + (SOC_CORE_BASE_ADDRESS |
+				 PCIE_INTR_ENABLE_ADDRESS));
 	iowrite32(PCIE_SOC_WAKE_RESET,
 		  ar_pci->mem + PCIE_LOCAL_BASE_ADDRESS +
 		  PCIE_SOC_WAKE_ADDRESS);
@@ -1909,11 +1961,11 @@ static int ath10k_pci_reset_target(struct ath10k *ar)
 	return 0;
 }
 
-static int ath10k_pci_configure(struct ath10k *ar)
+static int ath10k_pci_start_intr(struct ath10k *ar)
 {
 	struct ath10k_pci *ar_pci = ath10k_pci_priv(ar);
-	int ret = 0;
-	int num_msi_desired = MSI_NUM_REQUEST;
+	int num = MSI_NUM_REQUEST;
+	int ret;
 	int i;
 
 	tasklet_init(&ar_pci->intr_tq, ath10k_pci_tasklet, (unsigned long) ar);
@@ -1928,170 +1980,113 @@ static int ath10k_pci_configure(struct ath10k *ar)
 	}
 
 	if (!test_bit(ATH10K_PCI_FEATURE_MSI_X, ar_pci->features))
-		num_msi_desired = 1;
-
-	/*
-	 * Interrupt Management is divided into these scenarios :
-	 * A) We wish to use MSI and Multiple MSI is supported and we
-	 *    are able to obtain the number of MSI interrupts desired
-	 *    (best performance)
-	 * B) We wish to use MSI and Single MSI is supported and we are
-	 *    able to obtain a single MSI interrupt
-	 * C) We don't want to use MSI or MSI is not supported and we
-	 *    are able to obtain a legacy interrupt
-	 * D) Failure
-	 */
+		num = 1;
 
-	ath10k_dbg(ATH10K_DBG_PCI, "MSI set to %d\n", num_msi_desired);
+	if (num > 1) {
+		ret = ath10k_pci_start_intr_msix(ar, num);
+		if (ret == 0)
+			goto exit;
 
-	if (num_msi_desired > 1) {
-		int i;
+		ath10k_info("MSI-X didn't succeed (%d), trying MSI\n", ret);
+		num = 1;
+	}
 
-		ret = pci_enable_msi_block(ar_pci->pdev, num_msi_desired);
-		if (ret == 0) {
-			ar_pci->num_msi_intrs = num_msi_desired;
-			ret = request_irq(ar_pci->pdev->irq +
-					  MSI_ASSIGN_FW,
-					  ath10k_pci_msi_fw_handler,
-					  IRQF_SHARED,
-					  "ath10k_pci",
-					  ar);
-			if (ret) {
-				ath10k_err("request_irq failed (%d)\n", ret);
-				goto err_intr;
-			}
+	if (num == 1) {
+		ret = ath10k_pci_start_intr_msi(ar);
+		if (ret == 0)
+			goto exit;
 
-			for (i = MSI_ASSIGN_CE_INITIAL;
-			     i <= MSI_ASSIGN_CE_MAX; i++) {
-				ret = request_irq(ar_pci->pdev->irq + i,
-						ath10k_pci_per_engine_handler,
-						IRQF_SHARED,
-						"ath10k_pci",
-						ar);
-				if (ret) {
-					ath10k_err("request_irq failed (%d)\n",
-						   ret);
-					goto err_intr;
-				}
-			}
-		} else if (ret < 0)
-			/*
-			 * Can't get any MSI, try for
-			 * legacy line interrupts.
-			 */
-			num_msi_desired = 0;
-		else
-			/*
-			 * Can't get enough MSI interrupts,
-			 * try for just 1.
-			 */
-			num_msi_desired = 1;
+		ath10k_info("MSI didn't succeed (%d), trying Legacy INTR\n", ret);
+		num = 0;
 	}
 
-	if (num_msi_desired == 1) {
-		/*
-		 * We are here because either the platform only supports
-		 * single MSI or because we couldn't get all the MSI interrupts
-		 * that we wanted so we fall back to a single MSI.
-		 */
-		ath10k_dbg(ATH10K_DBG_PCI, "Falling back for single MSI\n");
+	ret = ath10k_pci_start_intr_legacy(ar);
 
-		if (pci_enable_msi(ar_pci->pdev) < 0) {
-			ath10k_err("single MSI interrupt allocation failed\n");
-			/* Try for legacy PCI line interrupts */
-			num_msi_desired = 0;
-		} else {
-			/*
-			 * Use a single Host-side MSI interrupt handler for
-			 * all interrupts.
-			 */
-			num_msi_desired = 1;
-		}
-	}
+exit:
+	ar_pci->num_msi_intrs = num;
+	ar_pci->ce_count = CE_COUNT;
+	return ret;
+}
 
-	if (num_msi_desired <= 1) {
-		/*
-		 * We are here because we want to multiplex a single host
-		 * interrupt among all Target interrupt sources.
-		 */
-		ret = request_irq(ar_pci->pdev->irq,
-				  ath10k_pci_interrupt_handler,
-				  IRQF_SHARED, "ath10k_pci", ar);
-		if (ret) {
-			ath10k_err("request_irq failed (%d)\n", ret);
-			goto err_intr;
-		}
-	}
+static void ath10k_pci_stop_intr(struct ath10k *ar)
+{
+	int i;
+	struct ath10k_pci *ar_pci = ath10k_pci_priv(ar);
 
-	if (num_msi_desired == 0) {
-		ath10k_dbg(ATH10K_DBG_PCI, "using PCI Legacy Interrupt\n");
+	if (ar_pci->num_msi_intrs > 0) {
+		/* MSI interrupt(s) */
+		for (i = 0; i < ar_pci->num_msi_intrs; i++)
+			free_irq(ar_pci->pdev->irq + i, ar);
+		ar_pci->num_msi_intrs = 0;
+		pci_disable_msi(ar_pci->pdev);
+	} else
+		/* Legacy PCI line interrupt */
+		free_irq(ar_pci->pdev->irq, ar);
+}
 
-		/*
-		 * Make sure to wake the Target before enabling Legacy
-		 * Interrupt.
-		 */
-		iowrite32(PCIE_SOC_WAKE_V_MASK,
-			  ar_pci->mem + PCIE_LOCAL_BASE_ADDRESS +
-			  PCIE_SOC_WAKE_ADDRESS);
+static int ath10k_pci_reset_target(struct ath10k *ar)
+{
+	struct ath10k_pci *ar_pci = ath10k_pci_priv(ar);
+	int wait_limit = 300; /* 3 sec */
+
+	/* Wait for Target to finish initialization before we proceed. */
+	iowrite32(PCIE_SOC_WAKE_V_MASK,
+		  ar_pci->mem + PCIE_LOCAL_BASE_ADDRESS +
+		  PCIE_SOC_WAKE_ADDRESS);
 
-		ath10k_pci_wait_for_target_to_awake(ar);
+	ath10k_pci_wait_for_target_to_awake(ar);
 
-		/*
-		 * A potential race occurs here: The CORE_BASE write
-		 * depends on target correctly decoding AXI address but
-		 * host won't know when target writes BAR to CORE_CTRL.
-		 * This write might get lost if target has NOT written BAR.
-		 * For now, fix the race by repeating the write in below
-		 * synchronization checking.
-		 */
-		iowrite32(PCIE_INTR_FIRMWARE_MASK |
-			  PCIE_INTR_CE_MASK_ALL,
-			  ar_pci->mem + (SOC_CORE_BASE_ADDRESS |
-					 PCIE_INTR_ENABLE_ADDRESS));
+	while (wait_limit-- &&
+	       !(ioread32(ar_pci->mem + FW_INDICATOR_ADDRESS) &
+		 FW_IND_INITIALIZED)) {
+
+		if (ar_pci->num_msi_intrs == 0)
+			/* Fix potential race by repeating CORE_BASE writes */
+			iowrite32(PCIE_INTR_FIRMWARE_MASK |
+				  PCIE_INTR_CE_MASK_ALL,
+				  ar_pci->mem + (SOC_CORE_BASE_ADDRESS |
+						 PCIE_INTR_ENABLE_ADDRESS));
+		mdelay(10);
+	}
+
+	if (wait_limit < 0) {
+		ath10k_err("Target stalled\n");
 		iowrite32(PCIE_SOC_WAKE_RESET,
 			  ar_pci->mem + PCIE_LOCAL_BASE_ADDRESS +
 			  PCIE_SOC_WAKE_ADDRESS);
+		return -EIO;
 	}
 
-	ar_pci->num_msi_intrs = num_msi_desired;
-	ar_pci->ce_count = CE_COUNT;
-
-	/*
-	 * Synchronization point: Wait for Target to finish initialization
-	 * before we proceed.
-	 */
-	iowrite32(PCIE_SOC_WAKE_V_MASK,
+	iowrite32(PCIE_SOC_WAKE_RESET,
 		  ar_pci->mem + PCIE_LOCAL_BASE_ADDRESS +
 		  PCIE_SOC_WAKE_ADDRESS);
 
-	ath10k_pci_wait_for_target_to_awake(ar);
+	return 0;
+}
+
+static int ath10k_pci_configure(struct ath10k *ar)
+{
+	int ret = 0;
+
+	ret = ath10k_pci_start_intr(ar);
+	if (ret) {
+		ath10k_err("could not start interrupt handling (%d)\n", ret);
+		return ret;
+	}
 
 	ret = ath10k_pci_reset_target(ar);
-	if (ret)
-		goto err_stalled;
+	if (ret) {
+		ath10k_pci_stop_intr(ar);
+		return ret;
+	}
 
 	if (ath10k_pci_probe_device(ar)) {
 		ath10k_err("Target probe failed\n");
-		ret = -EIO;
-		goto err_stalled;
+		ath10k_pci_stop_intr(ar);
+		return -EIO;
 	}
 
 	return 0;
-
-err_stalled:
-	ath10k_pci_nointrs(ar);
-err_intr:
-	pci_disable_msi(ar_pci->pdev);
-
-	return ret;
-}
-
-static void ath10k_pci_teardown(struct ath10k *ar)
-{
-	struct ath10k_pci *ar_pci = ath10k_pci_priv(ar);
-
-	ath10k_pci_nointrs(ar);
-	pci_disable_msi(ar_pci->pdev);
 }
 
 static void ath10k_pci_device_reset(struct ath10k_pci *ar_pci)
@@ -2327,7 +2322,7 @@ retry:
 
 	ret = ath10k_core_register(ar);
 	if (ret) {
-		ath10k_pci_teardown(ar);
+		ath10k_pci_stop_intr(ar);
 		goto err_iomap;
 	}
 
@@ -2378,10 +2373,9 @@ static void ath10k_pci_remove(struct pci_dev *pdev)
 	tasklet_kill(&ar_pci->msi_fw_err);
 
 	ath10k_core_unregister(ar);
-	ath10k_pci_nointrs(ar);
+	ath10k_pci_stop_intr(ar);
 	ath10k_pci_device_reset(ar_pci);
 
-	pci_disable_msi(pdev);
 	pci_set_drvdata(pdev, NULL);
 	pci_iounmap(pdev, ar_pci->mem);
 	pci_release_region(pdev, BAR_NUM);
-- 
1.7.9.5

[ath9k-devel] [PATCH 3/6] ath10k: remove pci probe retrying

[Michal Kazior]

This is an ugly hack. It is better to simply reset
the target and wait until it wakes up. If that is
not enough the platform may be broken.

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

diff --git a/drivers/net/wireless/ath/ath10k/pci.c b/drivers/net/wireless/ath/ath10k/pci.c
index df13454..78f7c72 100644
--- a/drivers/net/wireless/ath/ath10k/pci.c
+++ b/drivers/net/wireless/ath/ath10k/pci.c
@@ -2153,23 +2153,52 @@ static void ath10k_pci_dump_features(struct ath10k_pci *ar_pci)
 	}
 }
 
+static int ath10k_pci_bring_up(struct ath10k *ar)
+{
+	struct ath10k_pci *ar_pci = ath10k_pci_priv(ar);
+	u32 fw_indicator;
+
+	/*
+	 * Bring the target up cleanly.
+	 *
+	 * The target may be in an undefined state with an AUX-powered Target
+	 * and a Host in WoW mode. If the Host crashes, loses power, or is
+	 * restarted (without unloading the driver) then the Target is left
+	 * (aux) powered and running. On a subsequent driver load, the Target
+	 * is in an unexpected state. We try to catch that here in order to
+	 * reset the Target and retry the probe.
+	 */
+	ath10k_pci_device_reset(ar_pci);
+
+	iowrite32(PCIE_SOC_WAKE_V_MASK,
+		  ar_pci->mem + PCIE_LOCAL_BASE_ADDRESS +
+		  PCIE_SOC_WAKE_ADDRESS);
+
+	ath10k_pci_wait_for_target_to_awake(ar);
+
+	fw_indicator = ioread32(ar_pci->mem + FW_INDICATOR_ADDRESS);
+	iowrite32(PCIE_SOC_WAKE_RESET,
+		  ar_pci->mem + PCIE_LOCAL_BASE_ADDRESS +
+		  PCIE_SOC_WAKE_ADDRESS);
+
+	if (fw_indicator & FW_IND_INITIALIZED)
+		return -EIO;
+
+	return 0;
+}
+
 static int ath10k_pci_probe(struct pci_dev *pdev,
 			    const struct pci_device_id *pci_dev)
 {
 	void __iomem *mem;
-	int ret;
+	int ret = 0;
 	u32 hif_type;
-	int probe_again = 0;
 	struct ath10k *ar;
 	struct ath10k_pci *ar_pci;
-	u32 fw_indicator;
 	u32 lcr_val;
-	int retries = 3;
 	u32 target_type;
 
 	ath10k_dbg(ATH10K_DBG_PCI, "%s\n", __func__);
-retry:
-	ret = 0;
 
 	ar_pci = kzalloc(sizeof(*ar_pci), GFP_KERNEL);
 	if (ar_pci == NULL)
@@ -2280,42 +2309,12 @@ retry:
 
 	ar_pci->cacheline_sz = dma_get_cache_alignment();
 
-	/*
-	 * Verify that the Target was started cleanly.
-	 *
-	 * The case where this is most likely is with an AUX-powered
-	 * Target and a Host in WoW mode. If the Host crashes,
-	 * loses power, or is restarted (without unloading the driver)
-	 * then the Target is left (aux) powered and running.  On a
-	 * subsequent driver load, the Target is in an unexpected state.
-	 * We try to catch that here in order to reset the Target and
-	 * retry the probe.
-	 */
-	iowrite32(PCIE_SOC_WAKE_V_MASK,
-		  mem + PCIE_LOCAL_BASE_ADDRESS +
-		  PCIE_SOC_WAKE_ADDRESS);
-	ath10k_pci_wait_for_target_to_awake(ar);
-
-	fw_indicator = ioread32(mem + FW_INDICATOR_ADDRESS);
-	iowrite32(PCIE_SOC_WAKE_RESET,
-		  mem + PCIE_LOCAL_BASE_ADDRESS +
-		  PCIE_SOC_WAKE_ADDRESS);
-
-	if (fw_indicator & FW_IND_INITIALIZED) {
-		probe_again++;
-		ath10k_err("target is in an unknown state. "
-			   "resetting (attempt %d).\n", probe_again);
-		/* ath10k_pci_device_reset, below, will reset the target */
-		ret = -EIO;
-		goto err_tgtstate;
+	ret = ath10k_pci_bring_up(ar);
+	if (ret) {
+		ath10k_err("could not bring up target\n");
+		goto err_iomap;
 	}
 
-	/*
-	 * retries are meant for early hw setup;
-	 * beyond this point it makes no sense to retry
-	 */
-	retries = 0;
-
 	ret = ath10k_pci_configure(ar);
 	if (ret)
 		goto err_iomap;
@@ -2328,8 +2327,6 @@ retry:
 
 	return 0;
 
-err_tgtstate:
-	ath10k_pci_device_reset(ar_pci);
 err_iomap:
 	pci_iounmap(pdev, mem);
 err_master:
@@ -2345,13 +2342,6 @@ err_ar_pci:
 	/* call HIF PCI free here */
 	kfree(ar_pci);
 
-	/*
-	 * FIXME: for some reason qca_main loops probe
-	 * ATH_PCI_PROBE_RETRY_MAX times, do we need that in ath10k?
-	 */
-	if (ret && retries--)
-		goto retry;
-
 	return ret;
 }
 
@@ -2374,7 +2364,6 @@ static void ath10k_pci_remove(struct pci_dev *pdev)
 
 	ath10k_core_unregister(ar);
 	ath10k_pci_stop_intr(ar);
-	ath10k_pci_device_reset(ar_pci);
 
 	pci_set_drvdata(pdev, NULL);
 	pci_iounmap(pdev, ar_pci->mem);
-- 
1.7.9.5

[ath9k-devel] [PATCH 4/6] ath10k: remove ath10k_pci_configure()

[Michal Kazior]

This moves the contents of the function directly
into the pci probing function. Doesn't make sense
to have probe logic scattered so much around.

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

diff --git a/drivers/net/wireless/ath/ath10k/pci.c b/drivers/net/wireless/ath/ath10k/pci.c
index 78f7c72..b97d497 100644
--- a/drivers/net/wireless/ath/ath10k/pci.c
+++ b/drivers/net/wireless/ath/ath10k/pci.c
@@ -2064,31 +2064,6 @@ static int ath10k_pci_reset_target(struct ath10k *ar)
 	return 0;
 }
 
-static int ath10k_pci_configure(struct ath10k *ar)
-{
-	int ret = 0;
-
-	ret = ath10k_pci_start_intr(ar);
-	if (ret) {
-		ath10k_err("could not start interrupt handling (%d)\n", ret);
-		return ret;
-	}
-
-	ret = ath10k_pci_reset_target(ar);
-	if (ret) {
-		ath10k_pci_stop_intr(ar);
-		return ret;
-	}
-
-	if (ath10k_pci_probe_device(ar)) {
-		ath10k_err("Target probe failed\n");
-		ath10k_pci_stop_intr(ar);
-		return -EIO;
-	}
-
-	return 0;
-}
-
 static void ath10k_pci_device_reset(struct ath10k_pci *ar_pci)
 {
 	struct ath10k *ar = ar_pci->ar;
@@ -2315,18 +2290,32 @@ static int ath10k_pci_probe(struct pci_dev *pdev,
 		goto err_iomap;
 	}
 
-	ret = ath10k_pci_configure(ar);
-	if (ret)
+	ret = ath10k_pci_start_intr(ar);
+	if (ret) {
+		ath10k_err("could not start interrupt handling (%d)\n", ret);
 		goto err_iomap;
+	}
+
+	ret = ath10k_pci_reset_target(ar);
+	if (ret)
+		goto err_intr;
+
+	if (ath10k_pci_probe_device(ar)) {
+		ath10k_err("failed to probe target\n");
+		ret = -EIO;
+		goto err_intr;
+	}
 
 	ret = ath10k_core_register(ar);
 	if (ret) {
-		ath10k_pci_stop_intr(ar);
-		goto err_iomap;
+		ath10k_err("could not register driver core (%d)\n", ret);
+		goto err_intr;
 	}
 
 	return 0;
 
+err_intr:
+	ath10k_pci_stop_intr(ar);
 err_iomap:
 	pci_iounmap(pdev, mem);
 err_master:
-- 
1.7.9.5

[ath9k-devel] [PATCH 5/6] ath10k: remove ath10k_pci_probe_device()

[Michal Kazior]

No need to have probing logic scattered all over
the place.

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

diff --git a/drivers/net/wireless/ath/ath10k/pci.c b/drivers/net/wireless/ath/ath10k/pci.c
index b97d497..05bb324 100644
--- a/drivers/net/wireless/ath/ath10k/pci.c
+++ b/drivers/net/wireless/ath/ath10k/pci.c
@@ -1664,47 +1664,6 @@ static void ath10k_pci_ce_init(struct ath10k *ar)
 				   ath10k_pci_bmi_recv_data);
 }
 
-/*
- * Called from PCI layer whenever a new PCI device is probed.
- * Initializes per-device HIF state.
- */
-static int ath10k_pci_probe_device(struct ath10k *ar)
-{
-	struct ath10k_pci *ar_pci = ath10k_pci_priv(ar);
-	int ret;
-
-	atomic_set(&ar_pci->keep_awake_count, 0);
-	ar_pci->fw_indicator_address = FW_INDICATOR_ADDRESS;
-
-	if (ath10k_target_ps)
-		ath10k_dbg(ATH10K_DBG_PCI, "on-chip power save enabled\n");
-	else {
-		/* Force AWAKE forever */
-		ath10k_dbg(ATH10K_DBG_PCI, "on-chip power save disabled\n");
-		ath10k_do_pci_wake(ar);
-	}
-
-	ath10k_pci_ce_init(ar);
-
-	ret = ath10k_pci_init_config(ar);
-	if (ret) {
-		ath10k_pci_ce_deinit(ar);
-		goto ce_deinit;
-	}
-
-	ret = ath10k_pci_wake_target_cpu(ar);
-	if (ret) {
-		ath10k_err("Unable to wakeup target CPU\n");
-		goto ce_deinit;
-	}
-
-	return 0;
-
-ce_deinit:
-	ath10k_pci_ce_deinit(ar);
-	return ret;
-}
-
 static void ath10k_pci_fw_interrupt_handler(struct ath10k *ar)
 {
 	struct ath10k_pci *ar_pci = ath10k_pci_priv(ar);
@@ -2216,6 +2175,9 @@ static int ath10k_pci_probe(struct pci_dev *pdev,
 	}
 
 	ar_pci->ar = ar;
+	ar_pci->fw_indicator_address = FW_INDICATOR_ADDRESS;
+	atomic_set(&ar_pci->keep_awake_count, 0);
+
 	pci_set_drvdata(pdev, ar);
 
 	/*
@@ -2300,20 +2262,36 @@ static int ath10k_pci_probe(struct pci_dev *pdev,
 	if (ret)
 		goto err_intr;
 
-	if (ath10k_pci_probe_device(ar)) {
-		ath10k_err("failed to probe target\n");
-		ret = -EIO;
-		goto err_intr;
+	if (ath10k_target_ps)
+		ath10k_dbg(ATH10K_DBG_PCI, "on-chip power save enabled\n");
+	else {
+		/* Force AWAKE forever */
+		ath10k_dbg(ATH10K_DBG_PCI, "on-chip power save disabled\n");
+		ath10k_do_pci_wake(ar);
+	}
+
+	ath10k_pci_ce_init(ar);
+
+	ret = ath10k_pci_init_config(ar);
+	if (ret)
+		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_ce;
 	}
 
 	ret = ath10k_core_register(ar);
 	if (ret) {
 		ath10k_err("could not register driver core (%d)\n", ret);
-		goto err_intr;
+		goto err_ce;
 	}
 
 	return 0;
 
+err_ce:
+	ath10k_pci_ce_deinit(ar);
 err_intr:
 	ath10k_pci_stop_intr(ar);
 err_iomap:
-- 
1.7.9.5

[ath9k-devel] [PATCH 6/6] ath10k: simplify pci target reset

[Michal Kazior]

We already have a function that resets the target
and waits until it has awoken.

We can also change the ordering and combine device
reset with target reset altogether.

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

diff --git a/drivers/net/wireless/ath/ath10k/pci.c b/drivers/net/wireless/ath/ath10k/pci.c
index 05bb324..3b722c4 100644
--- a/drivers/net/wireless/ath/ath10k/pci.c
+++ b/drivers/net/wireless/ath/ath10k/pci.c
@@ -2087,40 +2087,6 @@ static void ath10k_pci_dump_features(struct ath10k_pci *ar_pci)
 	}
 }
 
-static int ath10k_pci_bring_up(struct ath10k *ar)
-{
-	struct ath10k_pci *ar_pci = ath10k_pci_priv(ar);
-	u32 fw_indicator;
-
-	/*
-	 * Bring the target up cleanly.
-	 *
-	 * The target may be in an undefined state with an AUX-powered Target
-	 * and a Host in WoW mode. If the Host crashes, loses power, or is
-	 * restarted (without unloading the driver) then the Target is left
-	 * (aux) powered and running. On a subsequent driver load, the Target
-	 * is in an unexpected state. We try to catch that here in order to
-	 * reset the Target and retry the probe.
-	 */
-	ath10k_pci_device_reset(ar_pci);
-
-	iowrite32(PCIE_SOC_WAKE_V_MASK,
-		  ar_pci->mem + PCIE_LOCAL_BASE_ADDRESS +
-		  PCIE_SOC_WAKE_ADDRESS);
-
-	ath10k_pci_wait_for_target_to_awake(ar);
-
-	fw_indicator = ioread32(ar_pci->mem + FW_INDICATOR_ADDRESS);
-	iowrite32(PCIE_SOC_WAKE_RESET,
-		  ar_pci->mem + PCIE_LOCAL_BASE_ADDRESS +
-		  PCIE_SOC_WAKE_ADDRESS);
-
-	if (fw_indicator & FW_IND_INITIALIZED)
-		return -EIO;
-
-	return 0;
-}
-
 static int ath10k_pci_probe(struct pci_dev *pdev,
 			    const struct pci_device_id *pci_dev)
 {
@@ -2246,18 +2212,24 @@ static int ath10k_pci_probe(struct pci_dev *pdev,
 
 	ar_pci->cacheline_sz = dma_get_cache_alignment();
 
-	ret = ath10k_pci_bring_up(ar);
-	if (ret) {
-		ath10k_err("could not bring up target\n");
-		goto err_iomap;
-	}
-
 	ret = ath10k_pci_start_intr(ar);
 	if (ret) {
 		ath10k_err("could not start interrupt handling (%d)\n", ret);
 		goto err_iomap;
 	}
 
+	/*
+	 * Bring the target up cleanly.
+	 *
+	 * The target may be in an undefined state with an AUX-powered Target
+	 * and a Host in WoW mode. If the Host crashes, loses power, or is
+	 * restarted (without unloading the driver) then the Target is left
+	 * (aux) powered and running. On a subsequent driver load, the Target
+	 * is in an unexpected state. We try to catch that here in order to
+	 * reset the Target and retry the probe.
+	 */
+	ath10k_pci_device_reset(ar_pci);
+
 	ret = ath10k_pci_reset_target(ar);
 	if (ret)
 		goto err_intr;
-- 
1.7.9.5

[ath9k-devel] [PATCH 0/6] ath10k: pci cleanup

[Michal Kazior]

On 24/04/13 12:19, Kazior Michal wrote:
> This gets rid of the ugly device/target resetting
> retries. It also tries to simplify pci probing
> logic.

Please do not apply this yet. CUS223 2.0 doesn't seem to like this patchset.


-- Pozdrawiam / Best regards, Michal Kazior.

[ath9k-devel] [PATCH v2 0/6] ath10k: pci cleanup

[Michal Kazior]

This gets rid of the ugly device/target resetting
retries. It also tries to simplify pci probing
logic.

v2: fixes MSI-X and legacy interrupts
    adds/changes a few prints

Michal Kazior (6):
  ath10k: fix pci_set_drvpriv ordering
  ath10k: refactor pci interrupt functions
  ath10k: remove pci probe retrying
  ath10k: remove ath10k_pci_configure()
  ath10k: remove ath10k_pci_probe_device()
  ath10k: simplify pci target reset

 drivers/net/wireless/ath/ath10k/pci.c |  451 ++++++++++++++-------------------
 1 file changed, 185 insertions(+), 266 deletions(-)

-- 
1.7.9.5

[ath9k-devel] [PATCH v2 1/6] ath10k: fix pci_set_drvpriv ordering

[Michal Kazior]

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

diff --git a/drivers/net/wireless/ath/ath10k/pci.c b/drivers/net/wireless/ath/ath10k/pci.c
index 40ca6d3..20e6371 100644
--- a/drivers/net/wireless/ath/ath10k/pci.c
+++ b/drivers/net/wireless/ath/ath10k/pci.c
@@ -2066,7 +2066,6 @@ err_stalled:
 	ath10k_pci_nointrs(ar);
 err_intr:
 	pci_disable_msi(ar_pci->pdev);
-	pci_set_drvdata(ar_pci->pdev, NULL);
 
 	return ret;
 }
@@ -2077,7 +2076,6 @@ static void ath10k_pci_teardown(struct ath10k *ar)
 
 	ath10k_pci_nointrs(ar);
 	pci_disable_msi(ar_pci->pdev);
-	pci_set_drvdata(ar_pci->pdev, NULL);
 }
 
 static void ath10k_pci_device_reset(struct ath10k_pci *ar_pci)
@@ -2315,7 +2313,6 @@ retry:
 	return 0;
 
 err_tgtstate:
-	pci_set_drvdata(pdev, NULL);
 	ath10k_pci_device_reset(ar_pci);
 err_iomap:
 	pci_iounmap(pdev, mem);
@@ -2326,6 +2323,7 @@ err_region:
 err_device:
 	pci_disable_device(pdev);
 err_ar:
+	pci_set_drvdata(pdev, NULL);
 	ath10k_core_destroy(ar);
 err_ar_pci:
 	/* call HIF PCI free here */
-- 
1.7.9.5

[ath9k-devel] [PATCH v2 2/6] ath10k: refactor pci interrupt functions

[Michal Kazior]

This patch splits the interrupt registering
function into separate functions. Hopefully this
is more readable and maintanable.

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

diff --git a/drivers/net/wireless/ath/ath10k/pci.c b/drivers/net/wireless/ath/ath10k/pci.c
index 20e6371..f71267c 100644
--- a/drivers/net/wireless/ath/ath10k/pci.c
+++ b/drivers/net/wireless/ath/ath10k/pci.c
@@ -1845,59 +1845,109 @@ static void ath10k_pci_tasklet(unsigned long data)
 	}
 }
 
-static void ath10k_pci_nointrs(struct ath10k *ar)
+static int ath10k_pci_start_intr_msix(struct ath10k *ar, int num)
 {
-	int i;
 	struct ath10k_pci *ar_pci = ath10k_pci_priv(ar);
+	int ret;
+	int i;
 
-	if (ar_pci->num_msi_intrs > 0) {
-		/* MSI interrupt(s) */
-		for (i = 0; i < ar_pci->num_msi_intrs; i++)
-			free_irq(ar_pci->pdev->irq + i, ar);
-		ar_pci->num_msi_intrs = 0;
-	} else
-		/* Legacy PCI line interrupt */
-		free_irq(ar_pci->pdev->irq, ar);
+	ret = pci_enable_msi_block(ar_pci->pdev, num);
+	if (ret)
+		return ret;
+
+	ret = request_irq(ar_pci->pdev->irq + MSI_ASSIGN_FW,
+			  ath10k_pci_msi_fw_handler,
+			  IRQF_SHARED, "ath10k_pci", ar);
+	if (ret)
+		return ret;
+
+	for (i = MSI_ASSIGN_CE_INITIAL; i <= MSI_ASSIGN_CE_MAX; i++) {
+		ret = request_irq(ar_pci->pdev->irq + i,
+				  ath10k_pci_per_engine_handler,
+				  IRQF_SHARED, "ath10k_pci", ar);
+		if (ret) {
+			ath10k_warn("request_irq(%d) failed %d\n",
+				    ar_pci->pdev->irq + i, ret);
+
+			for (; i >= MSI_ASSIGN_CE_INITIAL; i--)
+				free_irq(ar_pci->pdev->irq, ar);
+
+			pci_disable_msi(ar_pci->pdev);
+			return ret;
+		}
+	}
+
+	ath10k_info("MSI-X interrupt handling (%d intrs)\n", num);
+	return 0;
 }
 
-static int ath10k_pci_reset_target(struct ath10k *ar)
+static int ath10k_pci_start_intr_msi(struct ath10k *ar)
 {
 	struct ath10k_pci *ar_pci = ath10k_pci_priv(ar);
-	int wait_limit = 300; /* 3 sec */
+	int ret;
 
-	while (wait_limit-- &&
-	       !(ioread32(ar_pci->mem + FW_INDICATOR_ADDRESS) &
-		 FW_IND_INITIALIZED)) {
+	ret = pci_enable_msi(ar_pci->pdev);
+	if (ret < 0)
+		return ret;
 
-		if (ar_pci->num_msi_intrs == 0)
-			/* Fix potential race by repeating CORE_BASE writes */
-			iowrite32(PCIE_INTR_FIRMWARE_MASK |
-				  PCIE_INTR_CE_MASK_ALL,
-				  ar_pci->mem + (SOC_CORE_BASE_ADDRESS |
-						 PCIE_INTR_ENABLE_ADDRESS));
-		mdelay(10);
+	ret = request_irq(ar_pci->pdev->irq,
+			  ath10k_pci_interrupt_handler,
+			  IRQF_SHARED, "ath10k_pci", ar);
+	if (ret < 0) {
+		pci_disable_msi(ar_pci->pdev);
+		return ret;
 	}
 
-	if (wait_limit < 0) {
-		ath10k_err("Target stalled\n");
-		iowrite32(PCIE_SOC_WAKE_RESET,
-			  ar_pci->mem + PCIE_LOCAL_BASE_ADDRESS +
-			  PCIE_SOC_WAKE_ADDRESS);
-		return -EIO;
-	}
+	ath10k_info("MSI interrupt handling\n");
+	return 0;
+}
+
+static int ath10k_pci_start_intr_legacy(struct ath10k *ar)
+{
+	struct ath10k_pci *ar_pci = ath10k_pci_priv(ar);
+	int ret;
+
+	ret = request_irq(ar_pci->pdev->irq,
+			  ath10k_pci_interrupt_handler,
+			  IRQF_SHARED, "ath10k_pci", ar);
+	if (ret < 0)
+		return ret;
 
+	/*
+	 * Make sure to wake the Target before enabling Legacy
+	 * Interrupt.
+	 */
+	iowrite32(PCIE_SOC_WAKE_V_MASK,
+		  ar_pci->mem + PCIE_LOCAL_BASE_ADDRESS +
+		  PCIE_SOC_WAKE_ADDRESS);
+
+	ath10k_pci_wait(ar);
+
+	/*
+	 * A potential race occurs here: The CORE_BASE write
+	 * depends on target correctly decoding AXI address but
+	 * host won't know when target writes BAR to CORE_CTRL.
+	 * This write might get lost if target has NOT written BAR.
+	 * For now, fix the race by repeating the write in below
+	 * synchronization checking.
+	 */
+	iowrite32(PCIE_INTR_FIRMWARE_MASK |
+		  PCIE_INTR_CE_MASK_ALL,
+		  ar_pci->mem + (SOC_CORE_BASE_ADDRESS |
+				 PCIE_INTR_ENABLE_ADDRESS));
 	iowrite32(PCIE_SOC_WAKE_RESET,
 		  ar_pci->mem + PCIE_LOCAL_BASE_ADDRESS +
 		  PCIE_SOC_WAKE_ADDRESS);
 
+	ath10k_info("legacy interrupt handling\n");
 	return 0;
 }
 
-static int ath10k_pci_configure(struct ath10k *ar)
+static int ath10k_pci_start_intr(struct ath10k *ar)
 {
 	struct ath10k_pci *ar_pci = ath10k_pci_priv(ar);
-	int ret = 0;
-	int num_msi_desired = MSI_NUM_REQUEST;
+	int num = MSI_NUM_REQUEST;
+	int ret;
 	int i;
 
 	tasklet_init(&ar_pci->intr_tq, ath10k_pci_tasklet, (unsigned long) ar);
@@ -1912,170 +1962,113 @@ static int ath10k_pci_configure(struct ath10k *ar)
 	}
 
 	if (!test_bit(ATH10K_PCI_FEATURE_MSI_X, ar_pci->features))
-		num_msi_desired = 1;
-
-	/*
-	 * Interrupt Management is divided into these scenarios :
-	 * A) We wish to use MSI and Multiple MSI is supported and we
-	 *    are able to obtain the number of MSI interrupts desired
-	 *    (best performance)
-	 * B) We wish to use MSI and Single MSI is supported and we are
-	 *    able to obtain a single MSI interrupt
-	 * C) We don't want to use MSI or MSI is not supported and we
-	 *    are able to obtain a legacy interrupt
-	 * D) Failure
-	 */
+		num = 1;
 
-	ath10k_dbg(ATH10K_DBG_PCI, "MSI set to %d\n", num_msi_desired);
+	if (num > 1) {
+		ret = ath10k_pci_start_intr_msix(ar, num);
+		if (ret == 0)
+			goto exit;
 
-	if (num_msi_desired > 1) {
-		int i;
+		ath10k_warn("MSI-X didn't succeed (%d), trying MSI\n", ret);
+		num = 1;
+	}
 
-		ret = pci_enable_msi_block(ar_pci->pdev, num_msi_desired);
-		if (ret == 0) {
-			ar_pci->num_msi_intrs = num_msi_desired;
-			ret = request_irq(ar_pci->pdev->irq +
-					  MSI_ASSIGN_FW,
-					  ath10k_pci_msi_fw_handler,
-					  IRQF_SHARED,
-					  "ath10k_pci",
-					  ar);
-			if (ret) {
-				ath10k_err("request_irq failed (%d)\n", ret);
-				goto err_intr;
-			}
+	if (num == 1) {
+		ret = ath10k_pci_start_intr_msi(ar);
+		if (ret == 0)
+			goto exit;
 
-			for (i = MSI_ASSIGN_CE_INITIAL;
-			     i <= MSI_ASSIGN_CE_MAX; i++) {
-				ret = request_irq(ar_pci->pdev->irq + i,
-						ath10k_pci_per_engine_handler,
-						IRQF_SHARED,
-						"ath10k_pci",
-						ar);
-				if (ret) {
-					ath10k_err("request_irq failed (%d)\n",
-						   ret);
-					goto err_intr;
-				}
-			}
-		} else if (ret < 0)
-			/*
-			 * Can't get any MSI, try for
-			 * legacy line interrupts.
-			 */
-			num_msi_desired = 0;
-		else
-			/*
-			 * Can't get enough MSI interrupts,
-			 * try for just 1.
-			 */
-			num_msi_desired = 1;
+		ath10k_warn("MSI didn't succeed (%d), trying legacy INTR\n", ret);
+		num = 0;
 	}
 
-	if (num_msi_desired == 1) {
-		/*
-		 * We are here because either the platform only supports
-		 * single MSI or because we couldn't get all the MSI interrupts
-		 * that we wanted so we fall back to a single MSI.
-		 */
-		ath10k_dbg(ATH10K_DBG_PCI, "Falling back for single MSI\n");
+	ret = ath10k_pci_start_intr_legacy(ar);
 
-		if (pci_enable_msi(ar_pci->pdev) < 0) {
-			ath10k_err("single MSI interrupt allocation failed\n");
-			/* Try for legacy PCI line interrupts */
-			num_msi_desired = 0;
-		} else {
-			/*
-			 * Use a single Host-side MSI interrupt handler for
-			 * all interrupts.
-			 */
-			num_msi_desired = 1;
-		}
-	}
+exit:
+	ar_pci->num_msi_intrs = num;
+	ar_pci->ce_count = CE_COUNT;
+	return ret;
+}
 
-	if (num_msi_desired <= 1) {
-		/*
-		 * We are here because we want to multiplex a single host
-		 * interrupt among all Target interrupt sources.
-		 */
-		ret = request_irq(ar_pci->pdev->irq,
-				  ath10k_pci_interrupt_handler,
-				  IRQF_SHARED, "ath10k_pci", ar);
-		if (ret) {
-			ath10k_err("request_irq failed (%d)\n", ret);
-			goto err_intr;
-		}
-	}
+static void ath10k_pci_stop_intr(struct ath10k *ar)
+{
+	int i;
+	struct ath10k_pci *ar_pci = ath10k_pci_priv(ar);
 
-	if (num_msi_desired == 0) {
-		ath10k_dbg(ATH10K_DBG_PCI, "using PCI Legacy Interrupt\n");
+	if (ar_pci->num_msi_intrs > 0) {
+		/* MSI interrupt(s) */
+		for (i = 0; i < ar_pci->num_msi_intrs; i++)
+			free_irq(ar_pci->pdev->irq + i, ar);
+		ar_pci->num_msi_intrs = 0;
+		pci_disable_msi(ar_pci->pdev);
+	} else
+		/* Legacy PCI line interrupt */
+		free_irq(ar_pci->pdev->irq, ar);
+}
 
-		/*
-		 * Make sure to wake the Target before enabling Legacy
-		 * Interrupt.
-		 */
-		iowrite32(PCIE_SOC_WAKE_V_MASK,
-			  ar_pci->mem + PCIE_LOCAL_BASE_ADDRESS +
-			  PCIE_SOC_WAKE_ADDRESS);
+static int ath10k_pci_reset_target(struct ath10k *ar)
+{
+	struct ath10k_pci *ar_pci = ath10k_pci_priv(ar);
+	int wait_limit = 300; /* 3 sec */
+
+	/* Wait for Target to finish initialization before we proceed. */
+	iowrite32(PCIE_SOC_WAKE_V_MASK,
+		  ar_pci->mem + PCIE_LOCAL_BASE_ADDRESS +
+		  PCIE_SOC_WAKE_ADDRESS);
 
-		ath10k_pci_wait(ar);
+	ath10k_pci_wait(ar);
 
-		/*
-		 * A potential race occurs here: The CORE_BASE write
-		 * depends on target correctly decoding AXI address but
-		 * host won't know when target writes BAR to CORE_CTRL.
-		 * This write might get lost if target has NOT written BAR.
-		 * For now, fix the race by repeating the write in below
-		 * synchronization checking.
-		 */
-		iowrite32(PCIE_INTR_FIRMWARE_MASK |
-			  PCIE_INTR_CE_MASK_ALL,
-			  ar_pci->mem + (SOC_CORE_BASE_ADDRESS |
-					 PCIE_INTR_ENABLE_ADDRESS));
+	while (wait_limit-- &&
+	       !(ioread32(ar_pci->mem + FW_INDICATOR_ADDRESS) &
+		 FW_IND_INITIALIZED)) {
+
+		if (ar_pci->num_msi_intrs == 0)
+			/* Fix potential race by repeating CORE_BASE writes */
+			iowrite32(PCIE_INTR_FIRMWARE_MASK |
+				  PCIE_INTR_CE_MASK_ALL,
+				  ar_pci->mem + (SOC_CORE_BASE_ADDRESS |
+						 PCIE_INTR_ENABLE_ADDRESS));
+		mdelay(10);
+	}
+
+	if (wait_limit < 0) {
+		ath10k_err("Target stalled\n");
 		iowrite32(PCIE_SOC_WAKE_RESET,
 			  ar_pci->mem + PCIE_LOCAL_BASE_ADDRESS +
 			  PCIE_SOC_WAKE_ADDRESS);
+		return -EIO;
 	}
 
-	ar_pci->num_msi_intrs = num_msi_desired;
-	ar_pci->ce_count = CE_COUNT;
-
-	/*
-	 * Synchronization point: Wait for Target to finish initialization
-	 * before we proceed.
-	 */
-	iowrite32(PCIE_SOC_WAKE_V_MASK,
+	iowrite32(PCIE_SOC_WAKE_RESET,
 		  ar_pci->mem + PCIE_LOCAL_BASE_ADDRESS +
 		  PCIE_SOC_WAKE_ADDRESS);
 
-	ath10k_pci_wait(ar);
+	return 0;
+}
+
+static int ath10k_pci_configure(struct ath10k *ar)
+{
+	int ret = 0;
+
+	ret = ath10k_pci_start_intr(ar);
+	if (ret) {
+		ath10k_err("could not start interrupt handling (%d)\n", ret);
+		return ret;
+	}
 
 	ret = ath10k_pci_reset_target(ar);
-	if (ret)
-		goto err_stalled;
+	if (ret) {
+		ath10k_pci_stop_intr(ar);
+		return ret;
+	}
 
 	if (ath10k_pci_probe_device(ar)) {
 		ath10k_err("Target probe failed\n");
-		ret = -EIO;
-		goto err_stalled;
+		ath10k_pci_stop_intr(ar);
+		return -EIO;
 	}
 
 	return 0;
-
-err_stalled:
-	ath10k_pci_nointrs(ar);
-err_intr:
-	pci_disable_msi(ar_pci->pdev);
-
-	return ret;
-}
-
-static void ath10k_pci_teardown(struct ath10k *ar)
-{
-	struct ath10k_pci *ar_pci = ath10k_pci_priv(ar);
-
-	ath10k_pci_nointrs(ar);
-	pci_disable_msi(ar_pci->pdev);
 }
 
 static void ath10k_pci_device_reset(struct ath10k_pci *ar_pci)
@@ -2306,7 +2299,7 @@ retry:
 
 	ret = ath10k_core_register(ar);
 	if (ret) {
-		ath10k_pci_teardown(ar);
+		ath10k_pci_stop_intr(ar);
 		goto err_iomap;
 	}
 
@@ -2356,10 +2349,9 @@ static void ath10k_pci_remove(struct pci_dev *pdev)
 	tasklet_kill(&ar_pci->msi_fw_err);
 
 	ath10k_core_unregister(ar);
-	ath10k_pci_nointrs(ar);
+	ath10k_pci_stop_intr(ar);
 	ath10k_pci_device_reset(ar_pci);
 
-	pci_disable_msi(pdev);
 	pci_set_drvdata(pdev, NULL);
 	pci_iounmap(pdev, ar_pci->mem);
 	pci_release_region(pdev, BAR_NUM);
-- 
1.7.9.5

[ath9k-devel] [PATCH v2 3/6] ath10k: remove pci probe retrying

[Michal Kazior]

This is an ugly hack. It is better to simply reset
the target and wait until it wakes up. If that is
not enough the platform may be broken.

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

diff --git a/drivers/net/wireless/ath/ath10k/pci.c b/drivers/net/wireless/ath/ath10k/pci.c
index f71267c..0938942 100644
--- a/drivers/net/wireless/ath/ath10k/pci.c
+++ b/drivers/net/wireless/ath/ath10k/pci.c
@@ -2133,23 +2133,52 @@ static void ath10k_pci_dump_features(struct ath10k_pci *ar_pci)
 	}
 }
 
+static int ath10k_pci_bring_up(struct ath10k *ar)
+{
+	struct ath10k_pci *ar_pci = ath10k_pci_priv(ar);
+	u32 fw_indicator;
+
+	/*
+	 * Bring the target up cleanly.
+	 *
+	 * The target may be in an undefined state with an AUX-powered Target
+	 * and a Host in WoW mode. If the Host crashes, loses power, or is
+	 * restarted (without unloading the driver) then the Target is left
+	 * (aux) powered and running. On a subsequent driver load, the Target
+	 * is in an unexpected state. We try to catch that here in order to
+	 * reset the Target and retry the probe.
+	 */
+	ath10k_pci_device_reset(ar_pci);
+
+	iowrite32(PCIE_SOC_WAKE_V_MASK,
+		  ar_pci->mem + PCIE_LOCAL_BASE_ADDRESS +
+		  PCIE_SOC_WAKE_ADDRESS);
+
+	ath10k_pci_wait(ar);
+
+	fw_indicator = ioread32(ar_pci->mem + FW_INDICATOR_ADDRESS);
+	iowrite32(PCIE_SOC_WAKE_RESET,
+		  ar_pci->mem + PCIE_LOCAL_BASE_ADDRESS +
+		  PCIE_SOC_WAKE_ADDRESS);
+
+	if (fw_indicator & FW_IND_INITIALIZED)
+		return -EIO;
+
+	return 0;
+}
+
 static int ath10k_pci_probe(struct pci_dev *pdev,
 			    const struct pci_device_id *pci_dev)
 {
 	void __iomem *mem;
-	int ret;
+	int ret = 0;
 	u32 hif_type;
-	int probe_again = 0;
 	struct ath10k *ar;
 	struct ath10k_pci *ar_pci;
-	u32 fw_indicator;
 	u32 lcr_val;
-	int retries = 3;
 	u32 target_type;
 
 	ath10k_dbg(ATH10K_DBG_PCI, "%s\n", __func__);
-retry:
-	ret = 0;
 
 	ar_pci = kzalloc(sizeof(*ar_pci), GFP_KERNEL);
 	if (ar_pci == NULL)
@@ -2257,42 +2286,12 @@ retry:
 
 	ar_pci->cacheline_sz = dma_get_cache_alignment();
 
-	/*
-	 * Verify that the Target was started cleanly.
-	 *
-	 * The case where this is most likely is with an AUX-powered
-	 * Target and a Host in WoW mode. If the Host crashes,
-	 * loses power, or is restarted (without unloading the driver)
-	 * then the Target is left (aux) powered and running.  On a
-	 * subsequent driver load, the Target is in an unexpected state.
-	 * We try to catch that here in order to reset the Target and
-	 * retry the probe.
-	 */
-	iowrite32(PCIE_SOC_WAKE_V_MASK,
-		  mem + PCIE_LOCAL_BASE_ADDRESS +
-		  PCIE_SOC_WAKE_ADDRESS);
-	ath10k_pci_wait(ar);
-
-	fw_indicator = ioread32(mem + FW_INDICATOR_ADDRESS);
-	iowrite32(PCIE_SOC_WAKE_RESET,
-		  mem + PCIE_LOCAL_BASE_ADDRESS +
-		  PCIE_SOC_WAKE_ADDRESS);
-
-	if (fw_indicator & FW_IND_INITIALIZED) {
-		probe_again++;
-		ath10k_err("target is in an unknown state. "
-			   "resetting (attempt %d).\n", probe_again);
-		/* ath10k_pci_device_reset, below, will reset the target */
-		ret = -EIO;
-		goto err_tgtstate;
+	ret = ath10k_pci_bring_up(ar);
+	if (ret) {
+		ath10k_err("could not bring up target\n");
+		goto err_iomap;
 	}
 
-	/*
-	 * retries are meant for early hw setup;
-	 * beyond this point it makes no sense to retry
-	 */
-	retries = 0;
-
 	ret = ath10k_pci_configure(ar);
 	if (ret)
 		goto err_iomap;
@@ -2305,8 +2304,6 @@ retry:
 
 	return 0;
 
-err_tgtstate:
-	ath10k_pci_device_reset(ar_pci);
 err_iomap:
 	pci_iounmap(pdev, mem);
 err_master:
@@ -2322,12 +2319,6 @@ err_ar_pci:
 	/* call HIF PCI free here */
 	kfree(ar_pci);
 
-	/*
-	 * FIXME: Get rid of this hack as soon as HW is mature enough.
-	 */
-	if (ret && retries--)
-		goto retry;
-
 	return ret;
 }
 
@@ -2350,7 +2341,6 @@ static void ath10k_pci_remove(struct pci_dev *pdev)
 
 	ath10k_core_unregister(ar);
 	ath10k_pci_stop_intr(ar);
-	ath10k_pci_device_reset(ar_pci);
 
 	pci_set_drvdata(pdev, NULL);
 	pci_iounmap(pdev, ar_pci->mem);
-- 
1.7.9.5

[ath9k-devel] [PATCH v2 4/6] ath10k: remove ath10k_pci_configure()

[Michal Kazior]

This moves the contents of the function directly
into the pci probing function. Doesn't make sense
to have probe logic scattered so much around.

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

diff --git a/drivers/net/wireless/ath/ath10k/pci.c b/drivers/net/wireless/ath/ath10k/pci.c
index 0938942..8f3e371 100644
--- a/drivers/net/wireless/ath/ath10k/pci.c
+++ b/drivers/net/wireless/ath/ath10k/pci.c
@@ -2046,31 +2046,6 @@ static int ath10k_pci_reset_target(struct ath10k *ar)
 	return 0;
 }
 
-static int ath10k_pci_configure(struct ath10k *ar)
-{
-	int ret = 0;
-
-	ret = ath10k_pci_start_intr(ar);
-	if (ret) {
-		ath10k_err("could not start interrupt handling (%d)\n", ret);
-		return ret;
-	}
-
-	ret = ath10k_pci_reset_target(ar);
-	if (ret) {
-		ath10k_pci_stop_intr(ar);
-		return ret;
-	}
-
-	if (ath10k_pci_probe_device(ar)) {
-		ath10k_err("Target probe failed\n");
-		ath10k_pci_stop_intr(ar);
-		return -EIO;
-	}
-
-	return 0;
-}
-
 static void ath10k_pci_device_reset(struct ath10k_pci *ar_pci)
 {
 	struct ath10k *ar = ar_pci->ar;
@@ -2292,18 +2267,32 @@ static int ath10k_pci_probe(struct pci_dev *pdev,
 		goto err_iomap;
 	}
 
-	ret = ath10k_pci_configure(ar);
-	if (ret)
+	ret = ath10k_pci_start_intr(ar);
+	if (ret) {
+		ath10k_err("could not start interrupt handling (%d)\n", ret);
 		goto err_iomap;
+	}
+
+	ret = ath10k_pci_reset_target(ar);
+	if (ret)
+		goto err_intr;
+
+	if (ath10k_pci_probe_device(ar)) {
+		ath10k_err("failed to probe target\n");
+		ret = -EIO;
+		goto err_intr;
+	}
 
 	ret = ath10k_core_register(ar);
 	if (ret) {
-		ath10k_pci_stop_intr(ar);
-		goto err_iomap;
+		ath10k_err("could not register driver core (%d)\n", ret);
+		goto err_intr;
 	}
 
 	return 0;
 
+err_intr:
+	ath10k_pci_stop_intr(ar);
 err_iomap:
 	pci_iounmap(pdev, mem);
 err_master:
-- 
1.7.9.5

[ath9k-devel] [PATCH v2 5/6] ath10k: remove ath10k_pci_probe_device()

[Michal Kazior]

No need to have probing logic scattered all over
the place.

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

diff --git a/drivers/net/wireless/ath/ath10k/pci.c b/drivers/net/wireless/ath/ath10k/pci.c
index 8f3e371..4413f6b 100644
--- a/drivers/net/wireless/ath/ath10k/pci.c
+++ b/drivers/net/wireless/ath/ath10k/pci.c
@@ -1650,47 +1650,6 @@ static void ath10k_pci_ce_init(struct ath10k *ar)
 				   ath10k_pci_bmi_recv_data);
 }
 
-/*
- * Called from PCI layer whenever a new PCI device is probed.
- * Initializes per-device HIF state.
- */
-static int ath10k_pci_probe_device(struct ath10k *ar)
-{
-	struct ath10k_pci *ar_pci = ath10k_pci_priv(ar);
-	int ret;
-
-	atomic_set(&ar_pci->keep_awake_count, 0);
-	ar_pci->fw_indicator_address = FW_INDICATOR_ADDRESS;
-
-	if (ath10k_target_ps)
-		ath10k_dbg(ATH10K_DBG_PCI, "on-chip power save enabled\n");
-	else {
-		/* Force AWAKE forever */
-		ath10k_dbg(ATH10K_DBG_PCI, "on-chip power save disabled\n");
-		ath10k_do_pci_wake(ar);
-	}
-
-	ath10k_pci_ce_init(ar);
-
-	ret = ath10k_pci_init_config(ar);
-	if (ret) {
-		ath10k_pci_ce_deinit(ar);
-		goto ce_deinit;
-	}
-
-	ret = ath10k_pci_wake_target_cpu(ar);
-	if (ret) {
-		ath10k_err("Unable to wakeup target CPU\n");
-		goto ce_deinit;
-	}
-
-	return 0;
-
-ce_deinit:
-	ath10k_pci_ce_deinit(ar);
-	return ret;
-}
-
 static void ath10k_pci_fw_interrupt_handler(struct ath10k *ar)
 {
 	struct ath10k_pci *ar_pci = ath10k_pci_priv(ar);
@@ -2193,6 +2152,9 @@ static int ath10k_pci_probe(struct pci_dev *pdev,
 	}
 
 	ar_pci->ar = ar;
+	ar_pci->fw_indicator_address = FW_INDICATOR_ADDRESS;
+	atomic_set(&ar_pci->keep_awake_count, 0);
+
 	pci_set_drvdata(pdev, ar);
 
 	/*
@@ -2277,20 +2239,36 @@ static int ath10k_pci_probe(struct pci_dev *pdev,
 	if (ret)
 		goto err_intr;
 
-	if (ath10k_pci_probe_device(ar)) {
-		ath10k_err("failed to probe target\n");
-		ret = -EIO;
-		goto err_intr;
+	if (ath10k_target_ps)
+		ath10k_dbg(ATH10K_DBG_PCI, "on-chip power save enabled\n");
+	else {
+		/* Force AWAKE forever */
+		ath10k_dbg(ATH10K_DBG_PCI, "on-chip power save disabled\n");
+		ath10k_do_pci_wake(ar);
+	}
+
+	ath10k_pci_ce_init(ar);
+
+	ret = ath10k_pci_init_config(ar);
+	if (ret)
+		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_ce;
 	}
 
 	ret = ath10k_core_register(ar);
 	if (ret) {
 		ath10k_err("could not register driver core (%d)\n", ret);
-		goto err_intr;
+		goto err_ce;
 	}
 
 	return 0;
 
+err_ce:
+	ath10k_pci_ce_deinit(ar);
 err_intr:
 	ath10k_pci_stop_intr(ar);
 err_iomap:
-- 
1.7.9.5

[ath9k-devel] [PATCH v2 6/6] ath10k: simplify pci target reset

[Michal Kazior]

We already have a function that resets the target
and waits until it has awoken.

We can also change the ordering and combine device
reset with target reset altogether.

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

diff --git a/drivers/net/wireless/ath/ath10k/pci.c b/drivers/net/wireless/ath/ath10k/pci.c
index 4413f6b..bcf19e3 100644
--- a/drivers/net/wireless/ath/ath10k/pci.c
+++ b/drivers/net/wireless/ath/ath10k/pci.c
@@ -2067,40 +2067,6 @@ static void ath10k_pci_dump_features(struct ath10k_pci *ar_pci)
 	}
 }
 
-static int ath10k_pci_bring_up(struct ath10k *ar)
-{
-	struct ath10k_pci *ar_pci = ath10k_pci_priv(ar);
-	u32 fw_indicator;
-
-	/*
-	 * Bring the target up cleanly.
-	 *
-	 * The target may be in an undefined state with an AUX-powered Target
-	 * and a Host in WoW mode. If the Host crashes, loses power, or is
-	 * restarted (without unloading the driver) then the Target is left
-	 * (aux) powered and running. On a subsequent driver load, the Target
-	 * is in an unexpected state. We try to catch that here in order to
-	 * reset the Target and retry the probe.
-	 */
-	ath10k_pci_device_reset(ar_pci);
-
-	iowrite32(PCIE_SOC_WAKE_V_MASK,
-		  ar_pci->mem + PCIE_LOCAL_BASE_ADDRESS +
-		  PCIE_SOC_WAKE_ADDRESS);
-
-	ath10k_pci_wait(ar);
-
-	fw_indicator = ioread32(ar_pci->mem + FW_INDICATOR_ADDRESS);
-	iowrite32(PCIE_SOC_WAKE_RESET,
-		  ar_pci->mem + PCIE_LOCAL_BASE_ADDRESS +
-		  PCIE_SOC_WAKE_ADDRESS);
-
-	if (fw_indicator & FW_IND_INITIALIZED)
-		return -EIO;
-
-	return 0;
-}
-
 static int ath10k_pci_probe(struct pci_dev *pdev,
 			    const struct pci_device_id *pci_dev)
 {
@@ -2223,18 +2189,24 @@ static int ath10k_pci_probe(struct pci_dev *pdev,
 
 	ar_pci->cacheline_sz = dma_get_cache_alignment();
 
-	ret = ath10k_pci_bring_up(ar);
-	if (ret) {
-		ath10k_err("could not bring up target\n");
-		goto err_iomap;
-	}
-
 	ret = ath10k_pci_start_intr(ar);
 	if (ret) {
 		ath10k_err("could not start interrupt handling (%d)\n", ret);
 		goto err_iomap;
 	}
 
+	/*
+	 * Bring the target up cleanly.
+	 *
+	 * The target may be in an undefined state with an AUX-powered Target
+	 * and a Host in WoW mode. If the Host crashes, loses power, or is
+	 * restarted (without unloading the driver) then the Target is left
+	 * (aux) powered and running. On a subsequent driver load, the Target
+	 * is in an unexpected state. We try to catch that here in order to
+	 * reset the Target and retry the probe.
+	 */
+	ath10k_pci_device_reset(ar_pci);
+
 	ret = ath10k_pci_reset_target(ar);
 	if (ret)
 		goto err_intr;
-- 
1.7.9.5

[ath9k-devel] [PATCH v2 0/6] ath10k: pci cleanup

[Kalle Valo]

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

> This gets rid of the ugly device/target resetting
> retries. It also tries to simplify pci probing
> logic.
>
> v2: fixes MSI-X and legacy interrupts
>     adds/changes a few prints
>
> Michal Kazior (6):
>   ath10k: fix pci_set_drvpriv ordering
>   ath10k: refactor pci interrupt functions
>   ath10k: remove pci probe retrying
>   ath10k: remove ath10k_pci_configure()
>   ath10k: remove ath10k_pci_probe_device()
>   ath10k: simplify pci target reset

Applying: ath10k: remove pci probe retrying
fatal: sha1 information is lacking or useless (drivers/net/wireless/ath/ath10k/pci.c).
Repository lacks necessary blobs to fall back on 3-way merge.
Cannot fall back to three-way merge.
Patch failed at 0003 ath10k: remove pci probe retrying

I didn't want to start manually fixing the conflict, so can you please
rebase? And please double check how you submit patches, git still
complains about invalid ids. Of course it might be something on my end,
but your patches are the only ones with which I have noticed this
problem.

-- 
Kalle Valo

[ath9k-devel] [PATCH v2 0/6] ath10k: pci cleanup

[Michal Kazior]

On 25/04/13 09:24, Kalle Valo wrote:
> Michal Kazior <michal.kazior@tieto.com> writes:
>
>> This gets rid of the ugly device/target resetting
>> retries. It also tries to simplify pci probing
>> logic.
>>
>> v2: fixes MSI-X and legacy interrupts
>>      adds/changes a few prints
>>
>> Michal Kazior (6):
>>    ath10k: fix pci_set_drvpriv ordering
>>    ath10k: refactor pci interrupt functions
>>    ath10k: remove pci probe retrying
>>    ath10k: remove ath10k_pci_configure()
>>    ath10k: remove ath10k_pci_probe_device()
>>    ath10k: simplify pci target reset
>
> Applying: ath10k: remove pci probe retrying
> fatal: sha1 information is lacking or useless (drivers/net/wireless/ath/ath10k/pci.c).
> Repository lacks necessary blobs to fall back on 3-way merge.
> Cannot fall back to three-way merge.
> Patch failed at 0003 ath10k: remove pci probe retrying
>
> I didn't want to start manually fixing the conflict, so can you please
> rebase? And please double check how you submit patches, git still
> complains about invalid ids. Of course it might be something on my end,
> but your patches are the only ones with which I have noticed this
> problem.

I've made sure to send the v2 with no private patches around. When 
rebasing I had to resolve the conflict on this patch myself too. It was 
in conflict with the hif_type variable removal.

I use this command:

	git send-email --annotate --cover-letter [--subject-prefix=..] 
[--in-reply-to=..] commit_id..commit_id

I also rebase against your github/master branch.



-- Pozdrawiam / Best regards, Michal Kazior.

[ath9k-devel] [PATCH v2 0/6] ath10k: pci cleanup

[Kalle Valo]

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

>> Applying: ath10k: remove pci probe retrying
>> fatal: sha1 information is lacking or useless (drivers/net/wireless/ath/ath10k/pci.c).
>> Repository lacks necessary blobs to fall back on 3-way merge.
>> Cannot fall back to three-way merge.
>> Patch failed at 0003 ath10k: remove pci probe retrying
>>
>> I didn't want to start manually fixing the conflict, so can you please
>> rebase? And please double check how you submit patches, git still
>> complains about invalid ids. Of course it might be something on my end,
>> but your patches are the only ones with which I have noticed this
>> problem.
>
> I've made sure to send the v2 with no private patches around. When
> rebasing I had to resolve the conflict on this patch myself too. It
> was in conflict with the hif_type variable removal.
>
> I use this command:
>
> 	git send-email --annotate --cover-letter [--subject-prefix=..]
> [--in-reply-to=..] commit_id..commit_id

Ok, so you don't use 'git format-patch' at all? I wonder if that makes
the difference here. I would assume not.

> I also rebase against your github/master branch.

What commands do you use to rebase? Have you verified that there are no
private commits, eg. merges?

-- 
Kalle Valo

[ath9k-devel] [PATCH v2 0/6] ath10k: pci cleanup

[Bartosz.Markowski at tieto.com]

>-----Original Message-----
From: ath9k-devel-bounces@lists.ath9k.org [mailto:ath9k-devel-
>bounces at lists.ath9k.org] On Behalf Of Kalle Valo
>Sent: 25 kwietnia 2013 09:56
>To: Kazior Michal
>Cc: ath9k-devel at lists.ath9k.org
>Subject: Re: [ath9k-devel] [PATCH v2 0/6] ath10k: pci cleanup
>
>Michal Kazior <michal.kazior@tieto.com> writes:
>
>> I've made sure to send the v2 with no private patches around. When
>> rebasing I had to resolve the conflict on this patch myself too. It
>> was in conflict with the hif_type variable removal.
>>
>> I use this command:
>>
>> 	git send-email --annotate --cover-letter [--subject-prefix=..]
>> [--in-reply-to=..] commit_id..commit_id
>
>Ok, so you don't use 'git format-patch' at all? I wonder if that makes the
>difference here. I would assume not.

I'm not using either, I base all the patches vs origin/master-github and do:

git send-email --annotate --cover-letter [--subject-prefix=..] [--in-reply-to=..] origin/master-github*

(*master-github branch points to github's origin/master - mirrored in our private repo)

- Bartosz

[ath9k-devel] [PATCH v2 0/6] ath10k: pci cleanup

[Michal Kazior]

On 25/04/13 09:56, Kalle Valo wrote:
> Michal Kazior <michal.kazior@tieto.com> writes:
>
>>> Applying: ath10k: remove pci probe retrying
>>> fatal: sha1 information is lacking or useless (drivers/net/wireless/ath/ath10k/pci.c).
>>> Repository lacks necessary blobs to fall back on 3-way merge.
>>> Cannot fall back to three-way merge.
>>> Patch failed at 0003 ath10k: remove pci probe retrying
>>>
>>> I didn't want to start manually fixing the conflict, so can you please
>>> rebase? And please double check how you submit patches, git still
>>> complains about invalid ids. Of course it might be something on my end,
>>> but your patches are the only ones with which I have noticed this
>>> problem.
>>
>> I've made sure to send the v2 with no private patches around. When
>> rebasing I had to resolve the conflict on this patch myself too. It
>> was in conflict with the hif_type variable removal.
>>
>> I use this command:
>>
>> 	git send-email --annotate --cover-letter [--subject-prefix=..]
>> [--in-reply-to=..] commit_id..commit_id
>
> Ok, so you don't use 'git format-patch' at all? I wonder if that makes
> the difference here. I would assume not.
>
>> I also rebase against your github/master branch.
>
> What commands do you use to rebase? Have you verified that there are no
> private commits, eg. merges?

; git rebase github/master
(github is my remote name for your github ath10k repository)

If I have some private commits around I do a temporary branch, rebase 
interactively and drop those commits.
; git rebase -i github/master

I then do `git send-email` with `github/master..` so I'd probably see 
more patches than I should (after being dropped into editor for 
annotate/cover letter) if there were private commits still around.


-- Pozdrawiam / Best regards, Michal Kazior.

[ath9k-devel] [PATCH v2 0/6] ath10k: pci cleanup

[Kalle Valo]

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

> On 25/04/13 09:56, Kalle Valo wrote:
>> Michal Kazior <michal.kazior@tieto.com> writes:
>>
>>>> Applying: ath10k: remove pci probe retrying
>>>> fatal: sha1 information is lacking or useless (drivers/net/wireless/ath/ath10k/pci.c).
>>>> Repository lacks necessary blobs to fall back on 3-way merge.
>>>> Cannot fall back to three-way merge.
>>>> Patch failed at 0003 ath10k: remove pci probe retrying
>>>>
>>>> I didn't want to start manually fixing the conflict, so can you please
>>>> rebase? And please double check how you submit patches, git still
>>>> complains about invalid ids. Of course it might be something on my end,
>>>> but your patches are the only ones with which I have noticed this
>>>> problem.
>>>
>>> I've made sure to send the v2 with no private patches around. When
>>> rebasing I had to resolve the conflict on this patch myself too. It
>>> was in conflict with the hif_type variable removal.
>>>
>>> I use this command:
>>>
>>> 	git send-email --annotate --cover-letter [--subject-prefix=..]
>>> [--in-reply-to=..] commit_id..commit_id
>>
>> Ok, so you don't use 'git format-patch' at all? I wonder if that makes
>> the difference here. I would assume not.
>>
>>> I also rebase against your github/master branch.
>>
>> What commands do you use to rebase? Have you verified that there are no
>> private commits, eg. merges?
>
> ; git rebase github/master
> (github is my remote name for your github ath10k repository)
>
> If I have some private commits around I do a temporary branch, rebase
> interactively and drop those commits.
> ; git rebase -i github/master

Looks good to me.

> I then do `git send-email` with `github/master..` so I'd probably see
> more patches than I should (after being dropped into editor for
> annotate/cover letter) if there were private commits still around.

Yeah.

This is so weird as with Bartosz' patches I don't have any problems. For
example the last patchset I applied had three patches conflicting and
3-way was always used.

-- 
Kalle Valo

[ath9k-devel] [PATCH v3 0/6] ath10k: pci cleanup

[Michal Kazior]

This gets rid of the ugly device/target resetting
retries. It also tries to simplify pci probing
logic.

v3: rebased on top of latest master branch

Michal Kazior (6):
  ath10k: fix pci_set_drvpriv ordering
  ath10k: refactor pci interrupt functions
  ath10k: remove pci probe retrying
  ath10k: remove ath10k_pci_configure()
  ath10k: remove ath10k_pci_probe_device()
  ath10k: simplify pci target reset

 drivers/net/wireless/ath/ath10k/pci.c |  451 ++++++++++++++-------------------
 1 file changed, 185 insertions(+), 266 deletions(-)

-- 
1.7.9.5

[ath9k-devel] [PATCH v3 1/6] ath10k: fix pci_set_drvpriv ordering

[Michal Kazior]

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

diff --git a/drivers/net/wireless/ath/ath10k/pci.c b/drivers/net/wireless/ath/ath10k/pci.c
index 2e4e85b..14197ec 100644
--- a/drivers/net/wireless/ath/ath10k/pci.c
+++ b/drivers/net/wireless/ath/ath10k/pci.c
@@ -2066,7 +2066,6 @@ err_stalled:
 	ath10k_pci_nointrs(ar);
 err_intr:
 	pci_disable_msi(ar_pci->pdev);
-	pci_set_drvdata(ar_pci->pdev, NULL);
 
 	return ret;
 }
@@ -2077,7 +2076,6 @@ static void ath10k_pci_teardown(struct ath10k *ar)
 
 	ath10k_pci_nointrs(ar);
 	pci_disable_msi(ar_pci->pdev);
-	pci_set_drvdata(ar_pci->pdev, NULL);
 }
 
 static void ath10k_pci_device_reset(struct ath10k_pci *ar_pci)
@@ -2314,7 +2312,6 @@ retry:
 	return 0;
 
 err_tgtstate:
-	pci_set_drvdata(pdev, NULL);
 	ath10k_pci_device_reset(ar_pci);
 err_iomap:
 	pci_iounmap(pdev, mem);
@@ -2325,6 +2322,7 @@ err_region:
 err_device:
 	pci_disable_device(pdev);
 err_ar:
+	pci_set_drvdata(pdev, NULL);
 	ath10k_core_destroy(ar);
 err_ar_pci:
 	/* call HIF PCI free here */
-- 
1.7.9.5

[ath9k-devel] [PATCH v3 1/6] ath10k: fix pci_set_drvpriv ordering

[Kalle Valo]

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

> Signed-off-by: Michal Kazior <michal.kazior@tieto.com>
> ---
>  drivers/net/wireless/ath/ath10k/pci.c |    4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/drivers/net/wireless/ath/ath10k/pci.c b/drivers/net/wireless/ath/ath10k/pci.c
> index 2e4e85b..14197ec 100644
> --- a/drivers/net/wireless/ath/ath10k/pci.c
> +++ b/drivers/net/wireless/ath/ath10k/pci.c

I have been trying to understand why 3-way merge doesn't work with your
patches, but I just don't get it. The object id seems to be valid as
this works for me:

$ git show 2e4e85b | head -3
/*
 * Copyright (c) 2005-2011 Atheros Communications Inc.
 * Copyright (c) 2011-2013 Qualcomm Atheros, Inc.

So does git-am fail when parsing the email or what? Next I tried to
compare your and Bartosz' emails and I didn't find anything which would
explain the problem. The only difference I was able to find was that
Bartosz uses git 1.7.10 and you use 1.7.9.5, but that shouldn't make any
difference as the patch looks ok otherwise.

FWIW, I use git 1.7.9.5 from Ubuntu 12.04 64 bit.

Quite a problem we have :/

-- 
Kalle Valo

[ath9k-devel] [PATCH v3 1/6] ath10k: fix pci_set_drvpriv ordering

[Michal Kazior]

On 26/04/13 08:45, Kalle Valo wrote:
> Michal Kazior <michal.kazior@tieto.com> writes:
>
>> Signed-off-by: Michal Kazior <michal.kazior@tieto.com>
>> ---
>>   drivers/net/wireless/ath/ath10k/pci.c |    4 +---
>>   1 file changed, 1 insertion(+), 3 deletions(-)
>>
>> diff --git a/drivers/net/wireless/ath/ath10k/pci.c b/drivers/net/wireless/ath/ath10k/pci.c
>> index 2e4e85b..14197ec 100644
>> --- a/drivers/net/wireless/ath/ath10k/pci.c
>> +++ b/drivers/net/wireless/ath/ath10k/pci.c
>
> I have been trying to understand why 3-way merge doesn't work with your
> patches, but I just don't get it. The object id seems to be valid as
> this works for me:
>
> $ git show 2e4e85b | head -3
> /*
>   * Copyright (c) 2005-2011 Atheros Communications Inc.
>   * Copyright (c) 2011-2013 Qualcomm Atheros, Inc.
>
> So does git-am fail when parsing the email or what? Next I tried to
> compare your and Bartosz' emails and I didn't find anything which would
> explain the problem. The only difference I was able to find was that
> Bartosz uses git 1.7.10 and you use 1.7.9.5, but that shouldn't make any
> difference as the patch looks ok otherwise.
>
> FWIW, I use git 1.7.9.5 from Ubuntu 12.04 64 bit.
>
> Quite a problem we have :/

Perhaps this has something to do with newline style? \n vs \r\n ?



-- Pozdrawiam / Best regards, Michal Kazior.

[ath9k-devel] [PATCH v3 2/6] ath10k: refactor pci interrupt functions

[Michal Kazior]

This patch splits the interrupt registering
function into separate functions. Hopefully this
is more readable and maintanable.

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

diff --git a/drivers/net/wireless/ath/ath10k/pci.c b/drivers/net/wireless/ath/ath10k/pci.c
index 14197ec..28f0f95 100644
--- a/drivers/net/wireless/ath/ath10k/pci.c
+++ b/drivers/net/wireless/ath/ath10k/pci.c
@@ -1845,59 +1845,109 @@ static void ath10k_pci_tasklet(unsigned long data)
 	}
 }
 
-static void ath10k_pci_nointrs(struct ath10k *ar)
+static int ath10k_pci_start_intr_msix(struct ath10k *ar, int num)
 {
-	int i;
 	struct ath10k_pci *ar_pci = ath10k_pci_priv(ar);
+	int ret;
+	int i;
 
-	if (ar_pci->num_msi_intrs > 0) {
-		/* MSI interrupt(s) */
-		for (i = 0; i < ar_pci->num_msi_intrs; i++)
-			free_irq(ar_pci->pdev->irq + i, ar);
-		ar_pci->num_msi_intrs = 0;
-	} else
-		/* Legacy PCI line interrupt */
-		free_irq(ar_pci->pdev->irq, ar);
+	ret = pci_enable_msi_block(ar_pci->pdev, num);
+	if (ret)
+		return ret;
+
+	ret = request_irq(ar_pci->pdev->irq + MSI_ASSIGN_FW,
+			  ath10k_pci_msi_fw_handler,
+			  IRQF_SHARED, "ath10k_pci", ar);
+	if (ret)
+		return ret;
+
+	for (i = MSI_ASSIGN_CE_INITIAL; i <= MSI_ASSIGN_CE_MAX; i++) {
+		ret = request_irq(ar_pci->pdev->irq + i,
+				  ath10k_pci_per_engine_handler,
+				  IRQF_SHARED, "ath10k_pci", ar);
+		if (ret) {
+			ath10k_warn("request_irq(%d) failed %d\n",
+				    ar_pci->pdev->irq + i, ret);
+
+			for (; i >= MSI_ASSIGN_CE_INITIAL; i--)
+				free_irq(ar_pci->pdev->irq, ar);
+
+			pci_disable_msi(ar_pci->pdev);
+			return ret;
+		}
+	}
+
+	ath10k_info("MSI-X interrupt handling (%d intrs)\n", num);
+	return 0;
 }
 
-static int ath10k_pci_reset_target(struct ath10k *ar)
+static int ath10k_pci_start_intr_msi(struct ath10k *ar)
 {
 	struct ath10k_pci *ar_pci = ath10k_pci_priv(ar);
-	int wait_limit = 300; /* 3 sec */
+	int ret;
 
-	while (wait_limit-- &&
-	       !(ioread32(ar_pci->mem + FW_INDICATOR_ADDRESS) &
-		 FW_IND_INITIALIZED)) {
+	ret = pci_enable_msi(ar_pci->pdev);
+	if (ret < 0)
+		return ret;
 
-		if (ar_pci->num_msi_intrs == 0)
-			/* Fix potential race by repeating CORE_BASE writes */
-			iowrite32(PCIE_INTR_FIRMWARE_MASK |
-				  PCIE_INTR_CE_MASK_ALL,
-				  ar_pci->mem + (SOC_CORE_BASE_ADDRESS |
-						 PCIE_INTR_ENABLE_ADDRESS));
-		mdelay(10);
+	ret = request_irq(ar_pci->pdev->irq,
+			  ath10k_pci_interrupt_handler,
+			  IRQF_SHARED, "ath10k_pci", ar);
+	if (ret < 0) {
+		pci_disable_msi(ar_pci->pdev);
+		return ret;
 	}
 
-	if (wait_limit < 0) {
-		ath10k_err("Target stalled\n");
-		iowrite32(PCIE_SOC_WAKE_RESET,
-			  ar_pci->mem + PCIE_LOCAL_BASE_ADDRESS +
-			  PCIE_SOC_WAKE_ADDRESS);
-		return -EIO;
-	}
+	ath10k_info("MSI interrupt handling\n");
+	return 0;
+}
+
+static int ath10k_pci_start_intr_legacy(struct ath10k *ar)
+{
+	struct ath10k_pci *ar_pci = ath10k_pci_priv(ar);
+	int ret;
+
+	ret = request_irq(ar_pci->pdev->irq,
+			  ath10k_pci_interrupt_handler,
+			  IRQF_SHARED, "ath10k_pci", ar);
+	if (ret < 0)
+		return ret;
 
+	/*
+	 * Make sure to wake the Target before enabling Legacy
+	 * Interrupt.
+	 */
+	iowrite32(PCIE_SOC_WAKE_V_MASK,
+		  ar_pci->mem + PCIE_LOCAL_BASE_ADDRESS +
+		  PCIE_SOC_WAKE_ADDRESS);
+
+	ath10k_pci_wait(ar);
+
+	/*
+	 * A potential race occurs here: The CORE_BASE write
+	 * depends on target correctly decoding AXI address but
+	 * host won't know when target writes BAR to CORE_CTRL.
+	 * This write might get lost if target has NOT written BAR.
+	 * For now, fix the race by repeating the write in below
+	 * synchronization checking.
+	 */
+	iowrite32(PCIE_INTR_FIRMWARE_MASK |
+		  PCIE_INTR_CE_MASK_ALL,
+		  ar_pci->mem + (SOC_CORE_BASE_ADDRESS |
+				 PCIE_INTR_ENABLE_ADDRESS));
 	iowrite32(PCIE_SOC_WAKE_RESET,
 		  ar_pci->mem + PCIE_LOCAL_BASE_ADDRESS +
 		  PCIE_SOC_WAKE_ADDRESS);
 
+	ath10k_info("legacy interrupt handling\n");
 	return 0;
 }
 
-static int ath10k_pci_configure(struct ath10k *ar)
+static int ath10k_pci_start_intr(struct ath10k *ar)
 {
 	struct ath10k_pci *ar_pci = ath10k_pci_priv(ar);
-	int ret = 0;
-	int num_msi_desired = MSI_NUM_REQUEST;
+	int num = MSI_NUM_REQUEST;
+	int ret;
 	int i;
 
 	tasklet_init(&ar_pci->intr_tq, ath10k_pci_tasklet, (unsigned long) ar);
@@ -1912,170 +1962,113 @@ static int ath10k_pci_configure(struct ath10k *ar)
 	}
 
 	if (!test_bit(ATH10K_PCI_FEATURE_MSI_X, ar_pci->features))
-		num_msi_desired = 1;
-
-	/*
-	 * Interrupt Management is divided into these scenarios :
-	 * A) We wish to use MSI and Multiple MSI is supported and we
-	 *    are able to obtain the number of MSI interrupts desired
-	 *    (best performance)
-	 * B) We wish to use MSI and Single MSI is supported and we are
-	 *    able to obtain a single MSI interrupt
-	 * C) We don't want to use MSI or MSI is not supported and we
-	 *    are able to obtain a legacy interrupt
-	 * D) Failure
-	 */
+		num = 1;
 
-	ath10k_dbg(ATH10K_DBG_PCI, "MSI set to %d\n", num_msi_desired);
+	if (num > 1) {
+		ret = ath10k_pci_start_intr_msix(ar, num);
+		if (ret == 0)
+			goto exit;
 
-	if (num_msi_desired > 1) {
-		int i;
+		ath10k_warn("MSI-X didn't succeed (%d), trying MSI\n", ret);
+		num = 1;
+	}
 
-		ret = pci_enable_msi_block(ar_pci->pdev, num_msi_desired);
-		if (ret == 0) {
-			ar_pci->num_msi_intrs = num_msi_desired;
-			ret = request_irq(ar_pci->pdev->irq +
-					  MSI_ASSIGN_FW,
-					  ath10k_pci_msi_fw_handler,
-					  IRQF_SHARED,
-					  "ath10k_pci",
-					  ar);
-			if (ret) {
-				ath10k_err("request_irq failed (%d)\n", ret);
-				goto err_intr;
-			}
+	if (num == 1) {
+		ret = ath10k_pci_start_intr_msi(ar);
+		if (ret == 0)
+			goto exit;
 
-			for (i = MSI_ASSIGN_CE_INITIAL;
-			     i <= MSI_ASSIGN_CE_MAX; i++) {
-				ret = request_irq(ar_pci->pdev->irq + i,
-						ath10k_pci_per_engine_handler,
-						IRQF_SHARED,
-						"ath10k_pci",
-						ar);
-				if (ret) {
-					ath10k_err("request_irq failed (%d)\n",
-						   ret);
-					goto err_intr;
-				}
-			}
-		} else if (ret < 0)
-			/*
-			 * Can't get any MSI, try for
-			 * legacy line interrupts.
-			 */
-			num_msi_desired = 0;
-		else
-			/*
-			 * Can't get enough MSI interrupts,
-			 * try for just 1.
-			 */
-			num_msi_desired = 1;
+		ath10k_warn("MSI didn't succeed (%d), trying legacy INTR\n", ret);
+		num = 0;
 	}
 
-	if (num_msi_desired == 1) {
-		/*
-		 * We are here because either the platform only supports
-		 * single MSI or because we couldn't get all the MSI interrupts
-		 * that we wanted so we fall back to a single MSI.
-		 */
-		ath10k_dbg(ATH10K_DBG_PCI, "Falling back for single MSI\n");
+	ret = ath10k_pci_start_intr_legacy(ar);
 
-		if (pci_enable_msi(ar_pci->pdev) < 0) {
-			ath10k_err("single MSI interrupt allocation failed\n");
-			/* Try for legacy PCI line interrupts */
-			num_msi_desired = 0;
-		} else {
-			/*
-			 * Use a single Host-side MSI interrupt handler for
-			 * all interrupts.
-			 */
-			num_msi_desired = 1;
-		}
-	}
+exit:
+	ar_pci->num_msi_intrs = num;
+	ar_pci->ce_count = CE_COUNT;
+	return ret;
+}
 
-	if (num_msi_desired <= 1) {
-		/*
-		 * We are here because we want to multiplex a single host
-		 * interrupt among all Target interrupt sources.
-		 */
-		ret = request_irq(ar_pci->pdev->irq,
-				  ath10k_pci_interrupt_handler,
-				  IRQF_SHARED, "ath10k_pci", ar);
-		if (ret) {
-			ath10k_err("request_irq failed (%d)\n", ret);
-			goto err_intr;
-		}
-	}
+static void ath10k_pci_stop_intr(struct ath10k *ar)
+{
+	int i;
+	struct ath10k_pci *ar_pci = ath10k_pci_priv(ar);
 
-	if (num_msi_desired == 0) {
-		ath10k_dbg(ATH10K_DBG_PCI, "using PCI Legacy Interrupt\n");
+	if (ar_pci->num_msi_intrs > 0) {
+		/* MSI interrupt(s) */
+		for (i = 0; i < ar_pci->num_msi_intrs; i++)
+			free_irq(ar_pci->pdev->irq + i, ar);
+		ar_pci->num_msi_intrs = 0;
+		pci_disable_msi(ar_pci->pdev);
+	} else
+		/* Legacy PCI line interrupt */
+		free_irq(ar_pci->pdev->irq, ar);
+}
 
-		/*
-		 * Make sure to wake the Target before enabling Legacy
-		 * Interrupt.
-		 */
-		iowrite32(PCIE_SOC_WAKE_V_MASK,
-			  ar_pci->mem + PCIE_LOCAL_BASE_ADDRESS +
-			  PCIE_SOC_WAKE_ADDRESS);
+static int ath10k_pci_reset_target(struct ath10k *ar)
+{
+	struct ath10k_pci *ar_pci = ath10k_pci_priv(ar);
+	int wait_limit = 300; /* 3 sec */
+
+	/* Wait for Target to finish initialization before we proceed. */
+	iowrite32(PCIE_SOC_WAKE_V_MASK,
+		  ar_pci->mem + PCIE_LOCAL_BASE_ADDRESS +
+		  PCIE_SOC_WAKE_ADDRESS);
 
-		ath10k_pci_wait(ar);
+	ath10k_pci_wait(ar);
 
-		/*
-		 * A potential race occurs here: The CORE_BASE write
-		 * depends on target correctly decoding AXI address but
-		 * host won't know when target writes BAR to CORE_CTRL.
-		 * This write might get lost if target has NOT written BAR.
-		 * For now, fix the race by repeating the write in below
-		 * synchronization checking.
-		 */
-		iowrite32(PCIE_INTR_FIRMWARE_MASK |
-			  PCIE_INTR_CE_MASK_ALL,
-			  ar_pci->mem + (SOC_CORE_BASE_ADDRESS |
-					 PCIE_INTR_ENABLE_ADDRESS));
+	while (wait_limit-- &&
+	       !(ioread32(ar_pci->mem + FW_INDICATOR_ADDRESS) &
+		 FW_IND_INITIALIZED)) {
+
+		if (ar_pci->num_msi_intrs == 0)
+			/* Fix potential race by repeating CORE_BASE writes */
+			iowrite32(PCIE_INTR_FIRMWARE_MASK |
+				  PCIE_INTR_CE_MASK_ALL,
+				  ar_pci->mem + (SOC_CORE_BASE_ADDRESS |
+						 PCIE_INTR_ENABLE_ADDRESS));
+		mdelay(10);
+	}
+
+	if (wait_limit < 0) {
+		ath10k_err("Target stalled\n");
 		iowrite32(PCIE_SOC_WAKE_RESET,
 			  ar_pci->mem + PCIE_LOCAL_BASE_ADDRESS +
 			  PCIE_SOC_WAKE_ADDRESS);
+		return -EIO;
 	}
 
-	ar_pci->num_msi_intrs = num_msi_desired;
-	ar_pci->ce_count = CE_COUNT;
-
-	/*
-	 * Synchronization point: Wait for Target to finish initialization
-	 * before we proceed.
-	 */
-	iowrite32(PCIE_SOC_WAKE_V_MASK,
+	iowrite32(PCIE_SOC_WAKE_RESET,
 		  ar_pci->mem + PCIE_LOCAL_BASE_ADDRESS +
 		  PCIE_SOC_WAKE_ADDRESS);
 
-	ath10k_pci_wait(ar);
+	return 0;
+}
+
+static int ath10k_pci_configure(struct ath10k *ar)
+{
+	int ret = 0;
+
+	ret = ath10k_pci_start_intr(ar);
+	if (ret) {
+		ath10k_err("could not start interrupt handling (%d)\n", ret);
+		return ret;
+	}
 
 	ret = ath10k_pci_reset_target(ar);
-	if (ret)
-		goto err_stalled;
+	if (ret) {
+		ath10k_pci_stop_intr(ar);
+		return ret;
+	}
 
 	if (ath10k_pci_probe_device(ar)) {
 		ath10k_err("Target probe failed\n");
-		ret = -EIO;
-		goto err_stalled;
+		ath10k_pci_stop_intr(ar);
+		return -EIO;
 	}
 
 	return 0;
-
-err_stalled:
-	ath10k_pci_nointrs(ar);
-err_intr:
-	pci_disable_msi(ar_pci->pdev);
-
-	return ret;
-}
-
-static void ath10k_pci_teardown(struct ath10k *ar)
-{
-	struct ath10k_pci *ar_pci = ath10k_pci_priv(ar);
-
-	ath10k_pci_nointrs(ar);
-	pci_disable_msi(ar_pci->pdev);
 }
 
 static void ath10k_pci_device_reset(struct ath10k_pci *ar_pci)
@@ -2305,7 +2298,7 @@ retry:
 
 	ret = ath10k_core_register(ar);
 	if (ret) {
-		ath10k_pci_teardown(ar);
+		ath10k_pci_stop_intr(ar);
 		goto err_iomap;
 	}
 
@@ -2355,10 +2348,9 @@ static void ath10k_pci_remove(struct pci_dev *pdev)
 	tasklet_kill(&ar_pci->msi_fw_err);
 
 	ath10k_core_unregister(ar);
-	ath10k_pci_nointrs(ar);
+	ath10k_pci_stop_intr(ar);
 	ath10k_pci_device_reset(ar_pci);
 
-	pci_disable_msi(pdev);
 	pci_set_drvdata(pdev, NULL);
 	pci_iounmap(pdev, ar_pci->mem);
 	pci_release_region(pdev, BAR_NUM);
-- 
1.7.9.5

[ath9k-devel] [PATCH v3 3/6] ath10k: remove pci probe retrying

[Michal Kazior]

This is an ugly hack. It is better to simply reset
the target and wait until it wakes up. If that is
not enough the platform may be broken.

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

diff --git a/drivers/net/wireless/ath/ath10k/pci.c b/drivers/net/wireless/ath/ath10k/pci.c
index 28f0f95..725438b 100644
--- a/drivers/net/wireless/ath/ath10k/pci.c
+++ b/drivers/net/wireless/ath/ath10k/pci.c
@@ -2133,22 +2133,51 @@ static void ath10k_pci_dump_features(struct ath10k_pci *ar_pci)
 	}
 }
 
+static int ath10k_pci_bring_up(struct ath10k *ar)
+{
+	struct ath10k_pci *ar_pci = ath10k_pci_priv(ar);
+	u32 fw_indicator;
+
+	/*
+	 * Bring the target up cleanly.
+	 *
+	 * The target may be in an undefined state with an AUX-powered Target
+	 * and a Host in WoW mode. If the Host crashes, loses power, or is
+	 * restarted (without unloading the driver) then the Target is left
+	 * (aux) powered and running. On a subsequent driver load, the Target
+	 * is in an unexpected state. We try to catch that here in order to
+	 * reset the Target and retry the probe.
+	 */
+	ath10k_pci_device_reset(ar_pci);
+
+	iowrite32(PCIE_SOC_WAKE_V_MASK,
+		  ar_pci->mem + PCIE_LOCAL_BASE_ADDRESS +
+		  PCIE_SOC_WAKE_ADDRESS);
+
+	ath10k_pci_wait(ar);
+
+	fw_indicator = ioread32(ar_pci->mem + FW_INDICATOR_ADDRESS);
+	iowrite32(PCIE_SOC_WAKE_RESET,
+		  ar_pci->mem + PCIE_LOCAL_BASE_ADDRESS +
+		  PCIE_SOC_WAKE_ADDRESS);
+
+	if (fw_indicator & FW_IND_INITIALIZED)
+		return -EIO;
+
+	return 0;
+}
+
 static int ath10k_pci_probe(struct pci_dev *pdev,
 			    const struct pci_device_id *pci_dev)
 {
 	void __iomem *mem;
-	int ret;
-	int probe_again = 0;
+	int ret = 0;
 	struct ath10k *ar;
 	struct ath10k_pci *ar_pci;
-	u32 fw_indicator;
 	u32 lcr_val;
-	int retries = 3;
 	u32 target_type;
 
 	ath10k_dbg(ATH10K_DBG_PCI, "%s\n", __func__);
-retry:
-	ret = 0;
 
 	ar_pci = kzalloc(sizeof(*ar_pci), GFP_KERNEL);
 	if (ar_pci == NULL)
@@ -2256,42 +2285,12 @@ retry:
 
 	ar_pci->cacheline_sz = dma_get_cache_alignment();
 
-	/*
-	 * Verify that the Target was started cleanly.
-	 *
-	 * The case where this is most likely is with an AUX-powered
-	 * Target and a Host in WoW mode. If the Host crashes,
-	 * loses power, or is restarted (without unloading the driver)
-	 * then the Target is left (aux) powered and running.  On a
-	 * subsequent driver load, the Target is in an unexpected state.
-	 * We try to catch that here in order to reset the Target and
-	 * retry the probe.
-	 */
-	iowrite32(PCIE_SOC_WAKE_V_MASK,
-		  mem + PCIE_LOCAL_BASE_ADDRESS +
-		  PCIE_SOC_WAKE_ADDRESS);
-	ath10k_pci_wait(ar);
-
-	fw_indicator = ioread32(mem + FW_INDICATOR_ADDRESS);
-	iowrite32(PCIE_SOC_WAKE_RESET,
-		  mem + PCIE_LOCAL_BASE_ADDRESS +
-		  PCIE_SOC_WAKE_ADDRESS);
-
-	if (fw_indicator & FW_IND_INITIALIZED) {
-		probe_again++;
-		ath10k_err("target is in an unknown state. "
-			   "resetting (attempt %d).\n", probe_again);
-		/* ath10k_pci_device_reset, below, will reset the target */
-		ret = -EIO;
-		goto err_tgtstate;
+	ret = ath10k_pci_bring_up(ar);
+	if (ret) {
+		ath10k_err("could not bring up target\n");
+		goto err_iomap;
 	}
 
-	/*
-	 * retries are meant for early hw setup;
-	 * beyond this point it makes no sense to retry
-	 */
-	retries = 0;
-
 	ret = ath10k_pci_configure(ar);
 	if (ret)
 		goto err_iomap;
@@ -2304,8 +2303,6 @@ retry:
 
 	return 0;
 
-err_tgtstate:
-	ath10k_pci_device_reset(ar_pci);
 err_iomap:
 	pci_iounmap(pdev, mem);
 err_master:
@@ -2321,12 +2318,6 @@ err_ar_pci:
 	/* call HIF PCI free here */
 	kfree(ar_pci);
 
-	/*
-	 * FIXME: Get rid of this hack as soon as HW is mature enough.
-	 */
-	if (ret && retries--)
-		goto retry;
-
 	return ret;
 }
 
@@ -2349,7 +2340,6 @@ static void ath10k_pci_remove(struct pci_dev *pdev)
 
 	ath10k_core_unregister(ar);
 	ath10k_pci_stop_intr(ar);
-	ath10k_pci_device_reset(ar_pci);
 
 	pci_set_drvdata(pdev, NULL);
 	pci_iounmap(pdev, ar_pci->mem);
-- 
1.7.9.5

[ath9k-devel] [PATCH v3 4/6] ath10k: remove ath10k_pci_configure()

[Michal Kazior]

This moves the contents of the function directly
into the pci probing function. Doesn't make sense
to have probe logic scattered so much around.

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

diff --git a/drivers/net/wireless/ath/ath10k/pci.c b/drivers/net/wireless/ath/ath10k/pci.c
index 725438b..aca01a9 100644
--- a/drivers/net/wireless/ath/ath10k/pci.c
+++ b/drivers/net/wireless/ath/ath10k/pci.c
@@ -2046,31 +2046,6 @@ static int ath10k_pci_reset_target(struct ath10k *ar)
 	return 0;
 }
 
-static int ath10k_pci_configure(struct ath10k *ar)
-{
-	int ret = 0;
-
-	ret = ath10k_pci_start_intr(ar);
-	if (ret) {
-		ath10k_err("could not start interrupt handling (%d)\n", ret);
-		return ret;
-	}
-
-	ret = ath10k_pci_reset_target(ar);
-	if (ret) {
-		ath10k_pci_stop_intr(ar);
-		return ret;
-	}
-
-	if (ath10k_pci_probe_device(ar)) {
-		ath10k_err("Target probe failed\n");
-		ath10k_pci_stop_intr(ar);
-		return -EIO;
-	}
-
-	return 0;
-}
-
 static void ath10k_pci_device_reset(struct ath10k_pci *ar_pci)
 {
 	struct ath10k *ar = ar_pci->ar;
@@ -2291,18 +2266,32 @@ static int ath10k_pci_probe(struct pci_dev *pdev,
 		goto err_iomap;
 	}
 
-	ret = ath10k_pci_configure(ar);
-	if (ret)
+	ret = ath10k_pci_start_intr(ar);
+	if (ret) {
+		ath10k_err("could not start interrupt handling (%d)\n", ret);
 		goto err_iomap;
+	}
+
+	ret = ath10k_pci_reset_target(ar);
+	if (ret)
+		goto err_intr;
+
+	if (ath10k_pci_probe_device(ar)) {
+		ath10k_err("failed to probe target\n");
+		ret = -EIO;
+		goto err_intr;
+	}
 
 	ret = ath10k_core_register(ar);
 	if (ret) {
-		ath10k_pci_stop_intr(ar);
-		goto err_iomap;
+		ath10k_err("could not register driver core (%d)\n", ret);
+		goto err_intr;
 	}
 
 	return 0;
 
+err_intr:
+	ath10k_pci_stop_intr(ar);
 err_iomap:
 	pci_iounmap(pdev, mem);
 err_master:
-- 
1.7.9.5

[ath9k-devel] [PATCH v3 5/6] ath10k: remove ath10k_pci_probe_device()

[Michal Kazior]

No need to have probing logic scattered all over
the place.

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

diff --git a/drivers/net/wireless/ath/ath10k/pci.c b/drivers/net/wireless/ath/ath10k/pci.c
index aca01a9..1661120 100644
--- a/drivers/net/wireless/ath/ath10k/pci.c
+++ b/drivers/net/wireless/ath/ath10k/pci.c
@@ -1650,47 +1650,6 @@ static void ath10k_pci_ce_init(struct ath10k *ar)
 				   ath10k_pci_bmi_recv_data);
 }
 
-/*
- * Called from PCI layer whenever a new PCI device is probed.
- * Initializes per-device HIF state.
- */
-static int ath10k_pci_probe_device(struct ath10k *ar)
-{
-	struct ath10k_pci *ar_pci = ath10k_pci_priv(ar);
-	int ret;
-
-	atomic_set(&ar_pci->keep_awake_count, 0);
-	ar_pci->fw_indicator_address = FW_INDICATOR_ADDRESS;
-
-	if (ath10k_target_ps)
-		ath10k_dbg(ATH10K_DBG_PCI, "on-chip power save enabled\n");
-	else {
-		/* Force AWAKE forever */
-		ath10k_dbg(ATH10K_DBG_PCI, "on-chip power save disabled\n");
-		ath10k_do_pci_wake(ar);
-	}
-
-	ath10k_pci_ce_init(ar);
-
-	ret = ath10k_pci_init_config(ar);
-	if (ret) {
-		ath10k_pci_ce_deinit(ar);
-		goto ce_deinit;
-	}
-
-	ret = ath10k_pci_wake_target_cpu(ar);
-	if (ret) {
-		ath10k_err("Unable to wakeup target CPU\n");
-		goto ce_deinit;
-	}
-
-	return 0;
-
-ce_deinit:
-	ath10k_pci_ce_deinit(ar);
-	return ret;
-}
-
 static void ath10k_pci_fw_interrupt_handler(struct ath10k *ar)
 {
 	struct ath10k_pci *ar_pci = ath10k_pci_priv(ar);
@@ -2192,6 +2151,9 @@ static int ath10k_pci_probe(struct pci_dev *pdev,
 	}
 
 	ar_pci->ar = ar;
+	ar_pci->fw_indicator_address = FW_INDICATOR_ADDRESS;
+	atomic_set(&ar_pci->keep_awake_count, 0);
+
 	pci_set_drvdata(pdev, ar);
 
 	/*
@@ -2276,20 +2238,36 @@ static int ath10k_pci_probe(struct pci_dev *pdev,
 	if (ret)
 		goto err_intr;
 
-	if (ath10k_pci_probe_device(ar)) {
-		ath10k_err("failed to probe target\n");
-		ret = -EIO;
-		goto err_intr;
+	if (ath10k_target_ps)
+		ath10k_dbg(ATH10K_DBG_PCI, "on-chip power save enabled\n");
+	else {
+		/* Force AWAKE forever */
+		ath10k_dbg(ATH10K_DBG_PCI, "on-chip power save disabled\n");
+		ath10k_do_pci_wake(ar);
+	}
+
+	ath10k_pci_ce_init(ar);
+
+	ret = ath10k_pci_init_config(ar);
+	if (ret)
+		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_ce;
 	}
 
 	ret = ath10k_core_register(ar);
 	if (ret) {
 		ath10k_err("could not register driver core (%d)\n", ret);
-		goto err_intr;
+		goto err_ce;
 	}
 
 	return 0;
 
+err_ce:
+	ath10k_pci_ce_deinit(ar);
 err_intr:
 	ath10k_pci_stop_intr(ar);
 err_iomap:
-- 
1.7.9.5

[ath9k-devel] [PATCH v3 6/6] ath10k: simplify pci target reset

[Michal Kazior]

We already have a function that resets the target
and waits until it has awoken.

We can also change the ordering and combine device
reset with target reset altogether.

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

diff --git a/drivers/net/wireless/ath/ath10k/pci.c b/drivers/net/wireless/ath/ath10k/pci.c
index 1661120..d768223 100644
--- a/drivers/net/wireless/ath/ath10k/pci.c
+++ b/drivers/net/wireless/ath/ath10k/pci.c
@@ -2067,40 +2067,6 @@ static void ath10k_pci_dump_features(struct ath10k_pci *ar_pci)
 	}
 }
 
-static int ath10k_pci_bring_up(struct ath10k *ar)
-{
-	struct ath10k_pci *ar_pci = ath10k_pci_priv(ar);
-	u32 fw_indicator;
-
-	/*
-	 * Bring the target up cleanly.
-	 *
-	 * The target may be in an undefined state with an AUX-powered Target
-	 * and a Host in WoW mode. If the Host crashes, loses power, or is
-	 * restarted (without unloading the driver) then the Target is left
-	 * (aux) powered and running. On a subsequent driver load, the Target
-	 * is in an unexpected state. We try to catch that here in order to
-	 * reset the Target and retry the probe.
-	 */
-	ath10k_pci_device_reset(ar_pci);
-
-	iowrite32(PCIE_SOC_WAKE_V_MASK,
-		  ar_pci->mem + PCIE_LOCAL_BASE_ADDRESS +
-		  PCIE_SOC_WAKE_ADDRESS);
-
-	ath10k_pci_wait(ar);
-
-	fw_indicator = ioread32(ar_pci->mem + FW_INDICATOR_ADDRESS);
-	iowrite32(PCIE_SOC_WAKE_RESET,
-		  ar_pci->mem + PCIE_LOCAL_BASE_ADDRESS +
-		  PCIE_SOC_WAKE_ADDRESS);
-
-	if (fw_indicator & FW_IND_INITIALIZED)
-		return -EIO;
-
-	return 0;
-}
-
 static int ath10k_pci_probe(struct pci_dev *pdev,
 			    const struct pci_device_id *pci_dev)
 {
@@ -2222,18 +2188,24 @@ static int ath10k_pci_probe(struct pci_dev *pdev,
 
 	ar_pci->cacheline_sz = dma_get_cache_alignment();
 
-	ret = ath10k_pci_bring_up(ar);
-	if (ret) {
-		ath10k_err("could not bring up target\n");
-		goto err_iomap;
-	}
-
 	ret = ath10k_pci_start_intr(ar);
 	if (ret) {
 		ath10k_err("could not start interrupt handling (%d)\n", ret);
 		goto err_iomap;
 	}
 
+	/*
+	 * Bring the target up cleanly.
+	 *
+	 * The target may be in an undefined state with an AUX-powered Target
+	 * and a Host in WoW mode. If the Host crashes, loses power, or is
+	 * restarted (without unloading the driver) then the Target is left
+	 * (aux) powered and running. On a subsequent driver load, the Target
+	 * is in an unexpected state. We try to catch that here in order to
+	 * reset the Target and retry the probe.
+	 */
+	ath10k_pci_device_reset(ar_pci);
+
 	ret = ath10k_pci_reset_target(ar);
 	if (ret)
 		goto err_intr;
-- 
1.7.9.5

[ath9k-devel] [PATCH v3 0/6] ath10k: pci cleanup

[Kalle Valo]

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

> This gets rid of the ugly device/target resetting
> retries. It also tries to simplify pci probing
> logic.
>
> v3: rebased on top of latest master branch
>
> Michal Kazior (6):
>   ath10k: fix pci_set_drvpriv ordering
>   ath10k: refactor pci interrupt functions
>   ath10k: remove pci probe retrying
>   ath10k: remove ath10k_pci_configure()
>   ath10k: remove ath10k_pci_probe_device()
>   ath10k: simplify pci target reset

Thanks, applied. I had conflicts in patches 3 and 5, please check them
carefully. Still sha1 ids didn't work and I had to manually fix the
conflicts.

-- 
Kalle Valo