[PATCH 1/6] ath9k: add MSI support and use_msi parameter

[PATCH 1/6] ath9k: add MSI support and use_msi parameter

[AceLan Kao]

Adding MSI support for ath9k devices.
This patch is originally from Qualcomm, but they have no intention of
submitting and maintaining ath9k driver now.
The credit should go to Qualcomm.

Signed-off-by: AceLan Kao <acelan.kao@canonical.com>
---
 drivers/net/wireless/ath/ath9k/hw.c   | 33 ++++++++++++++++++------
 drivers/net/wireless/ath/ath9k/hw.h   |  3 +++
 drivers/net/wireless/ath/ath9k/init.c |  4 +++
 drivers/net/wireless/ath/ath9k/mac.c  | 47 +++++++++++++++++++++++++++++++++++
 drivers/net/wireless/ath/ath9k/pci.c  | 21 +++++++++++++++-
 drivers/net/wireless/ath/ath9k/reg.h  | 15 +++++++++++
 6 files changed, 115 insertions(+), 8 deletions(-)

diff --git a/drivers/net/wireless/ath/ath9k/hw.c b/drivers/net/wireless/ath/ath9k/hw.c
index 8c5c2dd..cd0f023 100644
--- a/drivers/net/wireless/ath/ath9k/hw.c
+++ b/drivers/net/wireless/ath/ath9k/hw.c
@@ -922,6 +922,7 @@ static void ath9k_hw_init_interrupt_masks(struct ath_hw *ah,
 		AR_IMR_RXERR |
 		AR_IMR_RXORN |
 		AR_IMR_BCNMISC;
+	u32 msi_cfg = 0;
 
 	if (AR_SREV_9340(ah) || AR_SREV_9550(ah) || AR_SREV_9531(ah) ||
 	    AR_SREV_9561(ah))
@@ -929,22 +930,30 @@ static void ath9k_hw_init_interrupt_masks(struct ath_hw *ah,
 
 	if (AR_SREV_9300_20_OR_LATER(ah)) {
 		imr_reg |= AR_IMR_RXOK_HP;
-		if (ah->config.rx_intr_mitigation)
+		if (ah->config.rx_intr_mitigation) {
 			imr_reg |= AR_IMR_RXINTM | AR_IMR_RXMINTR;
-		else
+			msi_cfg |= AR_INTCFG_MSI_RXINTM | AR_INTCFG_MSI_RXMINTR;
+		} else {
 			imr_reg |= AR_IMR_RXOK_LP;
-
+			msi_cfg |= AR_INTCFG_MSI_RXOK;
+		}
 	} else {
-		if (ah->config.rx_intr_mitigation)
+		if (ah->config.rx_intr_mitigation) {
 			imr_reg |= AR_IMR_RXINTM | AR_IMR_RXMINTR;
-		else
+			msi_cfg |= AR_INTCFG_MSI_RXINTM | AR_INTCFG_MSI_RXMINTR;
+		} else {
 			imr_reg |= AR_IMR_RXOK;
+			msi_cfg |= AR_INTCFG_MSI_RXOK;
+		}
 	}
 
-	if (ah->config.tx_intr_mitigation)
+	if (ah->config.tx_intr_mitigation) {
 		imr_reg |= AR_IMR_TXINTM | AR_IMR_TXMINTR;
-	else
+		msi_cfg |= AR_INTCFG_MSI_TXINTM | AR_INTCFG_MSI_TXMINTR;
+	} else {
 		imr_reg |= AR_IMR_TXOK;
+		msi_cfg |= AR_INTCFG_MSI_TXOK;
+	}
 
 	ENABLE_REGWRITE_BUFFER(ah);
 
@@ -952,6 +961,16 @@ static void ath9k_hw_init_interrupt_masks(struct ath_hw *ah,
 	ah->imrs2_reg |= AR_IMR_S2_GTT;
 	REG_WRITE(ah, AR_IMR_S2, ah->imrs2_reg);
 
+	if (ah->msi_enabled) {
+		ah->msi_reg = REG_READ(ah, AR_PCIE_MSI);
+		ah->msi_reg |= AR_PCIE_MSI_HW_DBI_WR_EN;
+		ah->msi_reg &= AR_PCIE_MSI_HW_INT_PENDING_ADDR_MSI_64;
+		REG_WRITE(ah, AR_INTCFG, msi_cfg);
+		ath_dbg(ath9k_hw_common(ah), ANY,
+			"value of AR_INTCFG=0x%X, msi_cfg=0x%X\n",
+			REG_READ(ah, AR_INTCFG), msi_cfg);
+	}
+
 	if (!AR_SREV_9100(ah)) {
 		REG_WRITE(ah, AR_INTR_SYNC_CAUSE, 0xFFFFFFFF);
 		REG_WRITE(ah, AR_INTR_SYNC_ENABLE, sync_default);
diff --git a/drivers/net/wireless/ath/ath9k/hw.h b/drivers/net/wireless/ath/ath9k/hw.h
index 4ac7082..0d6c07c7 100644
--- a/drivers/net/wireless/ath/ath9k/hw.h
+++ b/drivers/net/wireless/ath/ath9k/hw.h
@@ -977,6 +977,9 @@ struct ath_hw {
 	bool tpc_enabled;
 	u8 tx_power[Ar5416RateSize];
 	u8 tx_power_stbc[Ar5416RateSize];
+	bool msi_enabled;
+	u32 msi_mask;
+	u32 msi_reg;
 };
 
 struct ath_bus_ops {
diff --git a/drivers/net/wireless/ath/ath9k/init.c b/drivers/net/wireless/ath/ath9k/init.c
index bb79360..b6b7a35 100644
--- a/drivers/net/wireless/ath/ath9k/init.c
+++ b/drivers/net/wireless/ath/ath9k/init.c
@@ -75,6 +75,10 @@ MODULE_PARM_DESC(use_chanctx, "Enable channel context for concurrency");
 
 #endif /* CONFIG_ATH9K_CHANNEL_CONTEXT */
 
+int ath9k_use_msi;
+module_param_named(use_msi, ath9k_use_msi, int, 0444);
+MODULE_PARM_DESC(use_msi, "Use MSI instead of INTx if possible");
+
 bool is_ath9k_unloaded;
 
 #ifdef CONFIG_MAC80211_LEDS
diff --git a/drivers/net/wireless/ath/ath9k/mac.c b/drivers/net/wireless/ath/ath9k/mac.c
index 77c94f9..58d02c1 100644
--- a/drivers/net/wireless/ath/ath9k/mac.c
+++ b/drivers/net/wireless/ath/ath9k/mac.c
@@ -832,6 +832,43 @@ static void __ath9k_hw_enable_interrupts(struct ath_hw *ah)
 	}
 	ath_dbg(common, INTERRUPT, "AR_IMR 0x%x IER 0x%x\n",
 		REG_READ(ah, AR_IMR), REG_READ(ah, AR_IER));
+
+	if (ah->msi_enabled) {
+		u32 _msi_reg = 0;
+		u32 i = 0;
+		u32 msi_pend_addr_mask = AR_PCIE_MSI_HW_INT_PENDING_ADDR_MSI_64;
+
+		ath_dbg(ath9k_hw_common(ah), INTERRUPT,
+			"Enabling MSI, msi_mask=0x%X\n", ah->msi_mask);
+
+		REG_WRITE(ah, AR_INTR_PRIO_ASYNC_ENABLE, ah->msi_mask);
+		REG_WRITE(ah, AR_INTR_PRIO_ASYNC_MASK, ah->msi_mask);
+		ath_dbg(ath9k_hw_common(ah), INTERRUPT,
+			"AR_INTR_PRIO_ASYNC_ENABLE=0x%X, AR_INTR_PRIO_ASYNC_MASK=0x%X\n",
+			REG_READ(ah, AR_INTR_PRIO_ASYNC_ENABLE),
+			REG_READ(ah, AR_INTR_PRIO_ASYNC_MASK));
+
+		if (ah->msi_reg == 0)
+			ah->msi_reg = REG_READ(ah, AR_PCIE_MSI);
+
+		ath_dbg(ath9k_hw_common(ah), INTERRUPT,
+			"AR_PCIE_MSI=0x%X, ah->msi_reg = 0x%X\n",
+			AR_PCIE_MSI, ah->msi_reg);
+
+		i = 0;
+		do {
+			REG_WRITE(ah, AR_PCIE_MSI,
+				  (ah->msi_reg | AR_PCIE_MSI_ENABLE)
+				  & msi_pend_addr_mask);
+			_msi_reg = REG_READ(ah, AR_PCIE_MSI);
+			i++;
+		} while ((_msi_reg & AR_PCIE_MSI_ENABLE) == 0 && i < 200);
+
+		if (i >= 200)
+			ath_err(ath9k_hw_common(ah),
+				"%s: _msi_reg = 0x%X\n",
+				__func__, _msi_reg);
+	}
 }
 
 void ath9k_hw_resume_interrupts(struct ath_hw *ah)
@@ -878,12 +915,21 @@ void ath9k_hw_set_interrupts(struct ath_hw *ah)
 	if (!(ints & ATH9K_INT_GLOBAL))
 		ath9k_hw_disable_interrupts(ah);
 
+	if (ah->msi_enabled) {
+		ath_dbg(common, INTERRUPT, "Clearing AR_INTR_PRIO_ASYNC_ENABLE\n");
+
+		REG_WRITE(ah, AR_INTR_PRIO_ASYNC_ENABLE, 0);
+		REG_READ(ah, AR_INTR_PRIO_ASYNC_ENABLE);
+	}
+
 	ath_dbg(common, INTERRUPT, "New interrupt mask 0x%x\n", ints);
 
 	mask = ints & ATH9K_INT_COMMON;
 	mask2 = 0;
 
+	ah->msi_mask = 0;
 	if (ints & ATH9K_INT_TX) {
+		ah->msi_mask |= AR_INTR_PRIO_TX;
 		if (ah->config.tx_intr_mitigation)
 			mask |= AR_IMR_TXMINTR | AR_IMR_TXINTM;
 		else {
@@ -898,6 +944,7 @@ void ath9k_hw_set_interrupts(struct ath_hw *ah)
 			mask |= AR_IMR_TXEOL;
 	}
 	if (ints & ATH9K_INT_RX) {
+		ah->msi_mask |= AR_INTR_PRIO_RXLP | AR_INTR_PRIO_RXHP;
 		if (AR_SREV_9300_20_OR_LATER(ah)) {
 			mask |= AR_IMR_RXERR | AR_IMR_RXOK_HP;
 			if (ah->config.rx_intr_mitigation) {
diff --git a/drivers/net/wireless/ath/ath9k/pci.c b/drivers/net/wireless/ath/ath9k/pci.c
index 2236063..645f0fb 100644
--- a/drivers/net/wireless/ath/ath9k/pci.c
+++ b/drivers/net/wireless/ath/ath9k/pci.c
@@ -22,6 +22,8 @@
 #include <linux/module.h>
 #include "ath9k.h"
 
+extern int ath9k_use_msi;
+
 static const struct pci_device_id ath_pci_id_table[] = {
 	{ PCI_VDEVICE(ATHEROS, 0x0023) }, /* PCI   */
 	{ PCI_VDEVICE(ATHEROS, 0x0024) }, /* PCI-E */
@@ -889,6 +891,7 @@ static int ath_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id)
 	u32 val;
 	int ret = 0;
 	char hw_name[64];
+	int msi_enabled = 0;
 
 	if (pcim_enable_device(pdev))
 		return -EIO;
@@ -960,7 +963,20 @@ static int ath_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id)
 	sc->mem = pcim_iomap_table(pdev)[0];
 	sc->driver_data = id->driver_data;
 
-	ret = request_irq(pdev->irq, ath_isr, IRQF_SHARED, "ath9k", sc);
+	if (ath9k_use_msi) {
+		if (pci_enable_msi(pdev) == 0) {
+			msi_enabled = 1;
+			dev_err(&pdev->dev, "Using MSI\n");
+		} else {
+			dev_err(&pdev->dev, "Using INTx\n");
+		}
+	}
+
+	if (!msi_enabled)
+		ret = request_irq(pdev->irq, ath_isr, IRQF_SHARED, "ath9k", sc);
+	else
+		ret = request_irq(pdev->irq, ath_isr, 0, "ath9k", sc);
+
 	if (ret) {
 		dev_err(&pdev->dev, "request_irq failed\n");
 		goto err_irq;
@@ -974,6 +990,9 @@ static int ath_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id)
 		goto err_init;
 	}
 
+	sc->sc_ah->msi_enabled = msi_enabled;
+	sc->sc_ah->msi_reg = 0;
+
 	ath9k_hw_name(sc->sc_ah, hw_name, sizeof(hw_name));
 	wiphy_info(hw->wiphy, "%s mem=0x%lx, irq=%d\n",
 		   hw_name, (unsigned long)sc->mem, pdev->irq);
diff --git a/drivers/net/wireless/ath/ath9k/reg.h b/drivers/net/wireless/ath/ath9k/reg.h
index 80ff69f..653e796 100644
--- a/drivers/net/wireless/ath/ath9k/reg.h
+++ b/drivers/net/wireless/ath/ath9k/reg.h
@@ -146,6 +146,14 @@
 #define AR_MACMISC_MISC_OBS_BUS_MSB_S   15
 #define AR_MACMISC_MISC_OBS_BUS_1       1
 
+#define AR_INTCFG               0x005C
+#define AR_INTCFG_MSI_RXOK      0x00000000
+#define AR_INTCFG_MSI_RXINTM    0x00000004
+#define AR_INTCFG_MSI_RXMINTR   0x00000006
+#define AR_INTCFG_MSI_TXOK      0x00000000
+#define AR_INTCFG_MSI_TXINTM    0x00000010
+#define AR_INTCFG_MSI_TXMINTR   0x00000018
+
 #define AR_DATABUF_SIZE		0x0060
 #define AR_DATABUF_SIZE_MASK	0x00000FFF
 
@@ -1256,6 +1264,13 @@ enum {
 #define AR_PCIE_MSI                             (AR_SREV_9340(ah) ? 0x40d8 : \
 						 (AR_SREV_9300_20_OR_LATER(ah) ? 0x40a4 : 0x4094))
 #define AR_PCIE_MSI_ENABLE                       0x00000001
+#define AR_PCIE_MSI_HW_DBI_WR_EN                 0x02000000
+#define AR_PCIE_MSI_HW_INT_PENDING_ADDR          0xFFA0C1FF /* bits 8..11: value must be 0x5060 */
+#define AR_PCIE_MSI_HW_INT_PENDING_ADDR_MSI_64   0xFFA0C9FF /* bits 8..11: value must be 0x5064 */
+
+#define AR_INTR_PRIO_TX               0x00000001
+#define AR_INTR_PRIO_RXLP             0x00000002
+#define AR_INTR_PRIO_RXHP             0x00000004
 
 #define AR_INTR_PRIO_SYNC_ENABLE  (AR_SREV_9340(ah) ? 0x4088 : 0x40c4)
 #define AR_INTR_PRIO_ASYNC_MASK   (AR_SREV_9340(ah) ? 0x408c : 0x40c8)
-- 
2.7.4

[PATCH 2/6] ath9k: add a quirk to set use_msi automatically

[AceLan Kao]

Some platform(BIOS) blocks legacy interrupts (INTx), and only allows MSI
for WLAN device. So adding a quirk to list those machines and set
use_msi automatically.
Adding Dell Inspiron 24-3460 to the quirk.

Signed-off-by: AceLan Kao <acelan.kao@canonical.com>
---
 drivers/net/wireless/ath/ath9k/init.c | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/drivers/net/wireless/ath/ath9k/init.c b/drivers/net/wireless/ath/ath9k/init.c
index b6b7a35..1667949 100644
--- a/drivers/net/wireless/ath/ath9k/init.c
+++ b/drivers/net/wireless/ath/ath9k/init.c
@@ -23,6 +23,7 @@
 #include <linux/of.h>
 #include <linux/of_net.h>
 #include <linux/relay.h>
+#include <linux/dmi.h>
 #include <net/ieee80211_radiotap.h>
 
 #include "ath9k.h"
@@ -96,6 +97,24 @@ static const struct ieee80211_tpt_blink ath9k_tpt_blink[] = {
 };
 #endif
 
+static int __init set_use_msi(const struct dmi_system_id *dmi)
+{
+	ath9k_use_msi = 1;
+	return 1;
+}
+
+static const struct dmi_system_id ath9k_quirks[] __initconst = {
+	{
+		.callback = set_use_msi,
+		.ident = "Dell Inspiron 24-3460",
+		.matches = {
+			DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc."),
+			DMI_MATCH(DMI_PRODUCT_NAME, "Inspiron 24-3460"),
+		},
+	},
+	{}
+};
+
 static void ath9k_deinit_softc(struct ath_softc *sc);
 
 static void ath9k_op_ps_wakeup(struct ath_common *common)
@@ -1104,6 +1123,8 @@ static int __init ath9k_init(void)
 		goto err_pci_exit;
 	}
 
+	dmi_check_system(ath9k_quirks);
+
 	return 0;
 
  err_pci_exit:
-- 
2.7.4

[PATCH 3/6] ath9k: set use_msi=1 on Dell Vostro 3262

[AceLan Kao]

BIOS on Dell Vostro 3262 blocks legacy interrupts (INTx),
and only allows MSI for WLAN device.

Signed-off-by: AceLan Kao <acelan.kao@canonical.com>
---
 drivers/net/wireless/ath/ath9k/init.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/net/wireless/ath/ath9k/init.c b/drivers/net/wireless/ath/ath9k/init.c
index 1667949..6b5d53c 100644
--- a/drivers/net/wireless/ath/ath9k/init.c
+++ b/drivers/net/wireless/ath/ath9k/init.c
@@ -112,6 +112,14 @@ static const struct dmi_system_id ath9k_quirks[] __initconst = {
 			DMI_MATCH(DMI_PRODUCT_NAME, "Inspiron 24-3460"),
 		},
 	},
+	{
+		.callback = set_use_msi,
+		.ident = "Dell Vostro 3262",
+		.matches = {
+			DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc."),
+			DMI_MATCH(DMI_PRODUCT_NAME, "Vostro 3262"),
+		},
+	},
 	{}
 };
 
-- 
2.7.4

[PATCH 4/6] ath9k: set use_msi=1 on Dell Inspiron 3472

[AceLan Kao]

BIOS on Dell Inspiron 3472 blocks legacy interrupts (INTx),
and only allows MSI for WLAN device.

Signed-off-by: AceLan Kao <acelan.kao@canonical.com>
---
 drivers/net/wireless/ath/ath9k/init.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/net/wireless/ath/ath9k/init.c b/drivers/net/wireless/ath/ath9k/init.c
index 6b5d53c..fce9ac7 100644
--- a/drivers/net/wireless/ath/ath9k/init.c
+++ b/drivers/net/wireless/ath/ath9k/init.c
@@ -120,6 +120,14 @@ static const struct dmi_system_id ath9k_quirks[] __initconst = {
 			DMI_MATCH(DMI_PRODUCT_NAME, "Vostro 3262"),
 		},
 	},
+	{
+		.callback = set_use_msi,
+		.ident = "Dell Inspiron 3472",
+		.matches = {
+			DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc."),
+			DMI_MATCH(DMI_PRODUCT_NAME, "Inspiron 3472"),
+		},
+	},
 	{}
 };
 
-- 
2.7.4

[PATCH 5/6] ath9k: set use_msi=1 on Dell Vostro 15-3572

[AceLan Kao]

BIOS on Dell Vostro 15-3572 blocks legacy interrupts (INTx),
and only allows MSI for WLAN device.

Signed-off-by: AceLan Kao <acelan.kao@canonical.com>
---
 drivers/net/wireless/ath/ath9k/init.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/net/wireless/ath/ath9k/init.c b/drivers/net/wireless/ath/ath9k/init.c
index fce9ac7..e1198e1 100644
--- a/drivers/net/wireless/ath/ath9k/init.c
+++ b/drivers/net/wireless/ath/ath9k/init.c
@@ -128,6 +128,14 @@ static const struct dmi_system_id ath9k_quirks[] __initconst = {
 			DMI_MATCH(DMI_PRODUCT_NAME, "Inspiron 3472"),
 		},
 	},
+	{
+		.callback = set_use_msi,
+		.ident = "Dell Vostro 15-3572",
+		.matches = {
+			DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc."),
+			DMI_MATCH(DMI_PRODUCT_NAME, "Vostro 15-3572"),
+		},
+	},
 	{}
 };
 
-- 
2.7.4

[PATCH 6/6] ath9k: set use_msi=1 on Dell Inspiron 14-3473

[AceLan Kao]

BIOS on Dell Inspiron 14-3473 blocks legacy interrupts (INTx),
and only allows MSI for WLAN device.

Signed-off-by: AceLan Kao <acelan.kao@canonical.com>
---
 drivers/net/wireless/ath/ath9k/init.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/net/wireless/ath/ath9k/init.c b/drivers/net/wireless/ath/ath9k/init.c
index e1198e1..d03e1fc 100644
--- a/drivers/net/wireless/ath/ath9k/init.c
+++ b/drivers/net/wireless/ath/ath9k/init.c
@@ -136,6 +136,14 @@ static const struct dmi_system_id ath9k_quirks[] __initconst = {
 			DMI_MATCH(DMI_PRODUCT_NAME, "Vostro 15-3572"),
 		},
 	},
+	{
+		.callback = set_use_msi,
+		.ident = "Dell Inspiron 14-3473",
+		.matches = {
+			DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc."),
+			DMI_MATCH(DMI_PRODUCT_NAME, "Inspiron 14-3473"),
+		},
+	},
 	{}
 };
 
-- 
2.7.4

Re: [PATCH 2/6] ath9k: add a quirk to set use_msi automatically

[Christoph Hellwig]

On Tue, Sep 26, 2017 at 02:41:35PM +0800, AceLan Kao wrote:
> Some platform(BIOS) blocks legacy interrupts (INTx), and only allows MSI
> for WLAN device. So adding a quirk to list those machines and set
> use_msi automatically.
> Adding Dell Inspiron 24-3460 to the quirk.

Huh?  Using MSI should be the default, and skipping MSI should be
a quirk if needed at all (usually it should be autodetected)

Re: [PATCH 2/6] ath9k: add a quirk to set use_msi automatically

[AceLan Kao]

Ath9k is an old driver for old chips, and they work fine with legacy INTx.
But some new platforms are using it, so I think we should list those
new platforms which blocks INTx in the driver.

BTW, new chips use ath10k driver.

2017-09-26 14:44 GMT+08:00 Christoph Hellwig <hch@infradead.org>:
> On Tue, Sep 26, 2017 at 02:41:35PM +0800, AceLan Kao wrote:
>> Some platform(BIOS) blocks legacy interrupts (INTx), and only allows MSI
>> for WLAN device. So adding a quirk to list those machines and set
>> use_msi automatically.
>> Adding Dell Inspiron 24-3460 to the quirk.
>
> Huh?  Using MSI should be the default, and skipping MSI should be
> a quirk if needed at all (usually it should be autodetected)

Re: [PATCH 2/6] ath9k: add a quirk to set use_msi automatically

[Christoph Hellwig]

On Tue, Sep 26, 2017 at 03:26:51PM +0800, AceLan Kao wrote:
> Ath9k is an old driver for old chips, and they work fine with legacy INTx.
> But some new platforms are using it, so I think we should list those
> new platforms which blocks INTx in the driver.

And unless they are broken and need a quirk they should work even better
with MSI if the device advertises MSI support in the PCI capabilities.

Re: [PATCH 2/6] ath9k: add a quirk to set use_msi automatically

[Kalle Valo]

Christoph Hellwig <hch@infradead.org> writes:

> On Tue, Sep 26, 2017 at 03:26:51PM +0800, AceLan Kao wrote:
>> Ath9k is an old driver for old chips, and they work fine with legacy INTx.
>> But some new platforms are using it, so I think we should list those
>> new platforms which blocks INTx in the driver.
>
> And unless they are broken and need a quirk they should work even better
> with MSI if the device advertises MSI support in the PCI capabilities.

Daniel Drake (CCed) already found problems with the MSI implementation:

https://lkml.kernel.org/r/20170925041153.26574-1-drake@endlessm.com

-- 
Kalle Valo

Re: [PATCH 2/6] ath9k: add a quirk to set use_msi automatically

[AceLan Kao]

Hi Christoph,

I'm okay with ether ways, but just worrying the regression if we
switch to use MSI by default.

Hi Daniel,

I've tried your patch, but it doesn't work for me.
Wifi can scan AP, but can't get connected.
It keeps showing
[   48.362297] wlp2s0: authenticate with 3c:ce:73:48:0e:40
[   48.374445] wlp2s0: send auth to 3c:ce:73:48:0e:40 (try 1/3)
[   49.824071] wlp2s0: send auth to 3c:ce:73:48:0e:40 (try 2/3)
[   50.848143] wlp2s0: send auth to 3c:ce:73:48:0e:40 (try 3/3)
[   51.840078] wlp2s0: authentication with 3c:ce:73:48:0e:40 timed out
[   52.038561] wlp2s0: authenticate with 3c:ce:73:48:04:80
[   52.058930] wlp2s0: send auth to 3c:ce:73:48:04:80 (try 1/3)
[   52.832063] wlp2s0: send auth to 3c:ce:73:48:04:80 (try 2/3)
[   53.824135] wlp2s0: send auth to 3c:ce:73:48:04:80 (try 3/3)
[   54.816067] wlp2s0: authentication with 3c:ce:73:48:04:80 timed out
[   55.904797] wlp2s0: authenticate with 3c:ce:73:48:04:80
[   55.921771] wlp2s0: send auth to 3c:ce:73:48:04:80 (try 1/3)
[   56.800151] wlp2s0: send auth to 3c:ce:73:48:04:80 (try 2/3)
[   57.824072] wlp2s0: send auth to 3c:ce:73:48:04:80 (try 3/3)

02:00.0 Network controller [0280]: Qualcomm Atheros QCA9565 / AR9565
Wireless Network Adapter [168c:0036] (rev 01)
       Subsystem: Dell QCA9565 / AR9565 Wireless Network Adapter [1028:020e]
       Kernel driver in use: ath9k
       Kernel modules: ath9k

Do you have a chance to see if my patch works on your side?

Best regards,
AceLan Kao.

2017-09-26 22:14 GMT+08:00 Kalle Valo <kvalo@codeaurora.org>:
> Christoph Hellwig <hch@infradead.org> writes:
>
>> On Tue, Sep 26, 2017 at 03:26:51PM +0800, AceLan Kao wrote:
>>> Ath9k is an old driver for old chips, and they work fine with legacy INTx.
>>> But some new platforms are using it, so I think we should list those
>>> new platforms which blocks INTx in the driver.
>>
>> And unless they are broken and need a quirk they should work even better
>> with MSI if the device advertises MSI support in the PCI capabilities.
>
> Daniel Drake (CCed) already found problems with the MSI implementation:
>
> https://lkml.kernel.org/r/20170925041153.26574-1-drake@endlessm.com
>
> --
> Kalle Valo

Re: [PATCH 2/6] ath9k: add a quirk to set use_msi automatically

[Daniel Drake]

Hi AceLan,

On Thu, Sep 28, 2017 at 4:28 PM, AceLan Kao <acelan.kao@canonical.com> wrote:
> Hi Daniel,
>
> I've tried your patch, but it doesn't work for me.
> Wifi can scan AP, but can't get connected.

Can you please clarify which patch(es) you have tried?

This is the base patch which adds the infrastructure to request
specific MSI IRQ vectors:
https://marc.info/?l=linux-wireless&m=150631274108016&w=2

This is the ath9k MSI patch which makes use of that:
https://github.com/endlessm/linux/commit/739c7a924db8f4434a9617657

If you were already able to use ath9k MSI interrupts without specific
consideration for which MSI vector numbers were used, these are the
possible explanations that spring to mind:

1. You got lucky and it picked a vector number that is 4-aligned. You
can check this in the "lspci -vvv" output. You'll see something like:
Capabilities: [50] MSI: Enable+ Count=1/4 Maskable+ 64bit+
        Address: 00000000fee0300c  Data: 4142
The lower number is the vector number. In my example here 0x42 (66) is
not 4-aligned so the failure condition will be hit.

2. You are using interrupt remapping, which I suspect may provide a
high likelihood of MSI interrupt vectors being 4-aligned. See if
/proc/interrupts shows the IRQ type as IR-PCI-MSI
Unfortunately interrupt remapping is not available here,
https://lists.linuxfoundation.org/pipermail/iommu/2017-August/023717.html

3. My assumption that all ath9k hardware corrupts the MSI vector
number could wrong. However we've seen this on different wifi modules
in laptops produced by different OEMs and ODMs, so it seems to be a
somewhat widespread problem at least.

4. My assumption that ath9k hardware is corrupting the MSI vector
number could be wrong; maybe another component is to blame, could it
be a BIOS issue? Admittedly I don't really know how I can debug the
layers inbetween seeing the MSI Message Data value disagree with the
vector number being handled inside do_IRQ().

Daniel

Re: [PATCH 2/6] ath9k: add a quirk to set use_msi automatically

[AceLan Kao]

Hi all,

Please drop my patches, Qualcomm is working internally and will submit
the MSI patch by themselves.
Thanks.

Hi Daniel,

I'll try your patches tomorrow.

Best regards,
AceLan Kao.

2017-10-02 12:21 GMT+08:00 Daniel Drake <drake@endlessm.com>:
> Hi AceLan,
>
> On Thu, Sep 28, 2017 at 4:28 PM, AceLan Kao <acelan.kao@canonical.com> wrote:
>> Hi Daniel,
>>
>> I've tried your patch, but it doesn't work for me.
>> Wifi can scan AP, but can't get connected.
>
> Can you please clarify which patch(es) you have tried?
>
> This is the base patch which adds the infrastructure to request
> specific MSI IRQ vectors:
> https://marc.info/?l=linux-wireless&m=150631274108016&w=2
>
> This is the ath9k MSI patch which makes use of that:
> https://github.com/endlessm/linux/commit/739c7a924db8f4434a9617657
>
> If you were already able to use ath9k MSI interrupts without specific
> consideration for which MSI vector numbers were used, these are the
> possible explanations that spring to mind:
>
> 1. You got lucky and it picked a vector number that is 4-aligned. You
> can check this in the "lspci -vvv" output. You'll see something like:
> Capabilities: [50] MSI: Enable+ Count=1/4 Maskable+ 64bit+
>         Address: 00000000fee0300c  Data: 4142
> The lower number is the vector number. In my example here 0x42 (66) is
> not 4-aligned so the failure condition will be hit.
>
> 2. You are using interrupt remapping, which I suspect may provide a
> high likelihood of MSI interrupt vectors being 4-aligned. See if
> /proc/interrupts shows the IRQ type as IR-PCI-MSI
> Unfortunately interrupt remapping is not available here,
> https://lists.linuxfoundation.org/pipermail/iommu/2017-August/023717.html
>
> 3. My assumption that all ath9k hardware corrupts the MSI vector
> number could wrong. However we've seen this on different wifi modules
> in laptops produced by different OEMs and ODMs, so it seems to be a
> somewhat widespread problem at least.
>
> 4. My assumption that ath9k hardware is corrupting the MSI vector
> number could be wrong; maybe another component is to blame, could it
> be a BIOS issue? Admittedly I don't really know how I can debug the
> layers inbetween seeing the MSI Message Data value disagree with the
> vector number being handled inside do_IRQ().
>
> Daniel

Re: [PATCH 2/6] ath9k: add a quirk to set use_msi automatically

[AceLan Kao]

Hi Daniel,

After applied the 2 commits you mentioned in the email, ath9k works.

https://marc.info/?l=linux-wireless&m=150631274108016&w=2
https://github.com/endlessm/linux/commit/739c7a924db8f4434a9617657

Best regards,
AceLan Kao.

2017-10-05 14:39 GMT+08:00 AceLan Kao <acelan.kao@canonical.com>:
> Hi all,
>
> Please drop my patches, Qualcomm is working internally and will submit
> the MSI patch by themselves.
> Thanks.
>
> Hi Daniel,
>
> I'll try your patches tomorrow.
>
> Best regards,
> AceLan Kao.
>
> 2017-10-02 12:21 GMT+08:00 Daniel Drake <drake@endlessm.com>:
>> Hi AceLan,
>>
>> On Thu, Sep 28, 2017 at 4:28 PM, AceLan Kao <acelan.kao@canonical.com> wrote:
>>> Hi Daniel,
>>>
>>> I've tried your patch, but it doesn't work for me.
>>> Wifi can scan AP, but can't get connected.
>>
>> Can you please clarify which patch(es) you have tried?
>>
>> This is the base patch which adds the infrastructure to request
>> specific MSI IRQ vectors:
>> https://marc.info/?l=linux-wireless&m=150631274108016&w=2
>>
>> This is the ath9k MSI patch which makes use of that:
>> https://github.com/endlessm/linux/commit/739c7a924db8f4434a9617657
>>
>> If you were already able to use ath9k MSI interrupts without specific
>> consideration for which MSI vector numbers were used, these are the
>> possible explanations that spring to mind:
>>
>> 1. You got lucky and it picked a vector number that is 4-aligned. You
>> can check this in the "lspci -vvv" output. You'll see something like:
>> Capabilities: [50] MSI: Enable+ Count=1/4 Maskable+ 64bit+
>>         Address: 00000000fee0300c  Data: 4142
>> The lower number is the vector number. In my example here 0x42 (66) is
>> not 4-aligned so the failure condition will be hit.
>>
>> 2. You are using interrupt remapping, which I suspect may provide a
>> high likelihood of MSI interrupt vectors being 4-aligned. See if
>> /proc/interrupts shows the IRQ type as IR-PCI-MSI
>> Unfortunately interrupt remapping is not available here,
>> https://lists.linuxfoundation.org/pipermail/iommu/2017-August/023717.html
>>
>> 3. My assumption that all ath9k hardware corrupts the MSI vector
>> number could wrong. However we've seen this on different wifi modules
>> in laptops produced by different OEMs and ODMs, so it seems to be a
>> somewhat widespread problem at least.
>>
>> 4. My assumption that ath9k hardware is corrupting the MSI vector
>> number could be wrong; maybe another component is to blame, could it
>> be a BIOS issue? Admittedly I don't really know how I can debug the
>> layers inbetween seeing the MSI Message Data value disagree with the
>> vector number being handled inside do_IRQ().
>>
>> Daniel

Re: [PATCH 2/6] ath9k: add a quirk to set use_msi automatically

[Daniel Drake]

On Fri, Oct 13, 2017 at 9:12 AM, AceLan Kao <acelan.kao@canonical.com> wrote:
> Hi Daniel,
>
> After applied the 2 commits you mentioned in the email, ath9k works.
>
> https://marc.info/?l=linux-wireless&m=150631274108016&w=2
> https://github.com/endlessm/linux/commit/739c7a924db8f4434a9617657

Thanks for testing. However the approach was basically rejected in this thread:
  [PATCH] PCI MSI: allow alignment restrictions on vector allocation
  https://marc.info/?t=150631283200001&r=1&w=2

So we still need an upstream solution.

I am curious what Qualcomm have to say about their hardware corrupting
the MSI Message Data value. Is there any news on them submitting the
MSI support patch?

Separately we have the option of seeing if Intel can help us unblock
the legacy interrupt (assuming it was simply blocked by the BIOS), or
adding an interrupt-polling fallback path to ath9k.

Daniel