diff mbox series

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

Message ID 1506408099-18488-2-git-send-email-acelan.kao@canonical.com
State Awaiting Upstream, archived
Delegated to: David Miller
Headers show
Series [1/6] ath9k: add MSI support and use_msi parameter | expand

Commit Message

AceLan Kao Sept. 26, 2017, 6:41 a.m. UTC
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(+)

Comments

Christoph Hellwig Sept. 26, 2017, 6:44 a.m. UTC | #1
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)
AceLan Kao Sept. 26, 2017, 7:26 a.m. UTC | #2
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)
Christoph Hellwig Sept. 26, 2017, 2:04 p.m. UTC | #3
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.
Kalle Valo Sept. 26, 2017, 2:14 p.m. UTC | #4
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
AceLan Kao Sept. 28, 2017, 8:28 a.m. UTC | #5
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
Daniel Drake Oct. 2, 2017, 4:21 a.m. UTC | #6
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
AceLan Kao Oct. 5, 2017, 6:39 a.m. UTC | #7
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
AceLan Kao Oct. 13, 2017, 1:12 a.m. UTC | #8
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
Daniel Drake Oct. 13, 2017, 1:17 a.m. UTC | #9
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
diff mbox series

Patch

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: