[8/8] ath10k: allow explicit MSI/MSI-X disabling

Commit Message

Michal Kazior Nov. 22, 2013, 1:05 p.m. UTC

This can be useful for testing and debugging.

Two new ath10k_pci module options are now available:

 disable_msix - disable just MSI-X
 disable_msi - disable MSI (along with MSI-X)

Signed-off-by: Michal Kazior <[e-mail]>
---
 drivers/net/wireless/ath/ath10k/pci.c | 43 +++++++++++++++++++++++------------
 1 file changed, 29 insertions(+), 14 deletions(-)

Comments

Kalle Valo Nov. 25, 2013, 12:01 p.m. UTC | #1

Michal Kazior <[e-mail]> writes:

> This can be useful for testing and debugging.
>
> Two new ath10k_pci module options are now available:
>
>  disable_msix - disable just MSI-X
>  disable_msi - disable MSI (along with MSI-X)
>
> Signed-off-by: Michal Kazior <[e-mail]>

This is useful but I think it's a bit too much to have two more options,
it would be simpler to have just one. Few quick ideas how to do this a
bit differently:

irq_mode: 0 = automatic, 2 = MSI, 1 = legacy

Here the problem is of course that we can't force use of MSI if the host
only supports legacy interrupts, but IMHO that's a minor nuisance and in
that case we should just enable legacy interrupts.

disable_msi: 0x1 = disable MSI-X, 0x2 = disable MSI

Basically this is just your booleans converted to a bitfield.

I vote for irq_mode because I think that's easier for the user to
understand. But I'm sure there is a better way to do this.

> +	if (msix_supported && !ath10k_disable_msix && !ath10k_disable_msi) {
> +		ar_pci->num_msi_intrs = MSI_NUM_REQUEST;
> +		ret = pci_enable_msi_block(ar_pci->pdev, ar_pci->num_msi_intrs);
> +		if (ret == 0)
> +			return 0;
> +		if (ret > 0)
> +			pci_disable_msi(ar_pci->pdev);
> +
> +		/* fall-through */
> +	}

If we force some other irq mode than the default it would be good to
have an info print for that.

Patch

diff --git a/drivers/net/wireless/ath/ath10k/pci.c b/drivers/net/wireless/ath/ath10k/pci.c
index 47d9b6e..b89fd7b 100644
--- a/drivers/net/wireless/ath/ath10k/pci.c
+++ b/drivers/net/wireless/ath/ath10k/pci.c
@@ -34,9 +34,18 @@ 
 #include "pci.h"
 
 static unsigned int ath10k_target_ps;
+static bool ath10k_disable_msix;
+static bool ath10k_disable_msi;
+
 module_param(ath10k_target_ps, uint, 0644);
 MODULE_PARM_DESC(ath10k_target_ps, "Enable ath10k Target (SoC) PS option");
 
+module_param_named(disable_msix, ath10k_disable_msix, bool, 0644);
+MODULE_PARM_DESC(disable_msix, "Disable MSI-X support");
+
+module_param_named(disable_msi, ath10k_disable_msi, bool, 0644);
+MODULE_PARM_DESC(disable_msi, "Disable MSI support");
+
 #define QCA988X_2_0_DEVICE_ID	(0x003c)
 
 static DEFINE_PCI_DEVICE_TABLE(ath10k_pci_id_table) = {
@@ -2367,27 +2376,33 @@  static void ath10k_pci_init_irq_tasklets(struct ath10k *ar)
 static int ath10k_pci_init_irq(struct ath10k *ar)
 {
 	struct ath10k_pci *ar_pci = ath10k_pci_priv(ar);
+	bool msix_supported = test_bit(ATH10K_PCI_FEATURE_MSI_X,
+				       ar_pci->features);
 	int ret;
 
 	ath10k_pci_init_irq_tasklets(ar);
 
-	if (!test_bit(ATH10K_PCI_FEATURE_MSI_X, ar_pci->features))
-		goto msi;
-
 	/* Try MSI-X */
-	ar_pci->num_msi_intrs = MSI_NUM_REQUEST;
-	ret = pci_enable_msi_block(ar_pci->pdev, ar_pci->num_msi_intrs);
-	if (ret == 0)
-		return 0;
-	if (ret > 0)
-		pci_disable_msi(ar_pci->pdev);
+	if (msix_supported && !ath10k_disable_msix && !ath10k_disable_msi) {
+		ar_pci->num_msi_intrs = MSI_NUM_REQUEST;
+		ret = pci_enable_msi_block(ar_pci->pdev, ar_pci->num_msi_intrs);
+		if (ret == 0)
+			return 0;
+		if (ret > 0)
+			pci_disable_msi(ar_pci->pdev);
+
+		/* fall-through */
+	}
 
-msi:
 	/* Try MSI */
-	ar_pci->num_msi_intrs = 1;
-	ret = pci_enable_msi(ar_pci->pdev);
-	if (ret == 0)
-		return 0;
+	if (!ath10k_disable_msi) {
+		ar_pci->num_msi_intrs = 1;
+		ret = pci_enable_msi(ar_pci->pdev);
+		if (ret == 0)
+			return 0;
+
+		/* fall-through */
+	}
 
 	/* Try legacy irq
 	 *