2014-08-22 12:33:24

by Michal Kazior

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

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



2014-08-22 12:33:29

by Michal Kazior

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

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 <[email protected]>
---

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


2014-08-22 12:33:33

by Michal Kazior

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

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 <[email protected]>
---

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


2014-08-22 12:33:26

by Michal Kazior

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

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 <[email protected]>
---
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


2014-08-22 12:42:06

by Michal Kazior

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

On 22 August 2014 14:23, Michal Kazior <[email protected]> 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Ƃ

2014-08-25 08:30:22

by Kalle Valo

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

Michal Kazior <[email protected]> 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

2014-08-22 12:33:28

by Michal Kazior

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

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 <[email protected]>
---
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


2014-08-22 12:33:30

by Michal Kazior

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

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 <[email protected]>
---
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


2014-08-22 12:33:31

by Michal Kazior

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

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 <[email protected]>
---
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