[PATCH] ath9k: turn on btcoex_enable as default

[PATCH] ath9k: turn on btcoex_enable as default

[Kai-Heng Feng]

Without btcoex_enable, WiFi activies make both WiFi and Bluetooth
unstable if there's a bluetooth connection.

Enable this option when bt_ant_diversity is disabled.

BugLink: https://bugs.launchpad.net/bugs/1746164
Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com>
---
 drivers/net/wireless/ath/ath9k/init.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/net/wireless/ath/ath9k/init.c b/drivers/net/wireless/ath/ath9k/init.c
index e479fae5aab9..f8f6b091a077 100644
--- a/drivers/net/wireless/ath/ath9k/init.c
+++ b/drivers/net/wireless/ath/ath9k/init.c
@@ -56,7 +56,7 @@ static int ath9k_led_active_high = -1;
 module_param_named(led_active_high, ath9k_led_active_high, int, 0444);
 MODULE_PARM_DESC(led_active_high, "Invert LED polarity");
 
-static int ath9k_btcoex_enable;
+static int ath9k_btcoex_enable = 1;
 module_param_named(btcoex_enable, ath9k_btcoex_enable, int, 0444);
 MODULE_PARM_DESC(btcoex_enable, "Enable wifi-BT coexistence");
 
@@ -693,7 +693,6 @@ static int ath9k_init_softc(u16 devid, struct ath_softc *sc,
 	common->hw = sc->hw;
 	common->priv = sc;
 	common->debug_mask = ath9k_debug;
-	common->btcoex_enabled = ath9k_btcoex_enable == 1;
 	common->disable_ani = false;
 
 	/*
@@ -715,14 +714,17 @@ static int ath9k_init_softc(u16 devid, struct ath_softc *sc,
 	/*
 	 * Enable WLAN/BT RX Antenna diversity only when:
 	 *
-	 * - BTCOEX is disabled.
 	 * - the user manually requests the feature.
 	 * - the HW cap is set using the platform data.
 	 */
-	if (!common->btcoex_enabled && ath9k_bt_ant_diversity &&
+	if (ath9k_bt_ant_diversity &&
 	    (pCap->hw_caps & ATH9K_HW_CAP_BT_ANT_DIV))
 		common->bt_ant_diversity = 1;
 
+	/* Enable btcoex when ant_diversity is disabled */
+	if (!common->bt_ant_diversity && ath9k_btcoex_enable)
+		common->btcoex_enabled = 1;
+
 	spin_lock_init(&common->cc_lock);
 	spin_lock_init(&sc->intr_lock);
 	spin_lock_init(&sc->sc_serial_rw);
-- 
2.15.1

Re: [PATCH] ath9k: turn on btcoex_enable as default

[Felix Fietkau]

On 2018-02-08 06:28, Kai-Heng Feng wrote:
> Without btcoex_enable, WiFi activies make both WiFi and Bluetooth
> unstable if there's a bluetooth connection.
> 
> Enable this option when bt_ant_diversity is disabled.
> 
> BugLink: https://bugs.launchpad.net/bugs/1746164
> Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com>
I think this might cause regressions on devices that don't have
bluetooth. This probably either needs more EEPROM checks, or something
to selectively enable it only on affected platforms.

- Felix

Re: [PATCH] ath9k: turn on btcoex_enable as default

[Kai Heng Feng]

Hi Felix,

> On Feb 8, 2018, at 7:02 PM, Felix Fietkau <nbd-Vt+b4OUoWG0@public.gmane.org> wrote:
>
> On 2018-02-08 06:28, Kai-Heng Feng wrote:
>> Without btcoex_enable, WiFi activies make both WiFi and Bluetooth
>> unstable if there's a bluetooth connection.
>>
>> Enable this option when bt_ant_diversity is disabled.
>>
>> BugLink: https://bugs.launchpad.net/bugs/1746164
>> Signed-off-by: Kai-Heng Feng <kai.heng.feng-Z7WLFzj8eWMS+FvcfC7Uqw@public.gmane.org>
> I think this might cause regressions on devices that don't have
> bluetooth. This probably either needs more EEPROM checks, or something
> to selectively enable it only on affected platforms.
>

I think it’s better not to use dmi_match. This issue should affect more  
ath9k.
And bluetooth peripherals are more than ever now, so it would be great to  
use BT out of the box.

Can you take a look at the bug link, maybe there are other things caused  
the erratic behavior that I didn’t notice?

Kai-Heng

> - Felix

Re: [PATCH] ath9k: turn on btcoex_enable as default

[Kalle Valo]

Kai Heng Feng <kai.heng.feng@canonical.com> writes:

> Hi Felix,
>
>> On Feb 8, 2018, at 7:02 PM, Felix Fietkau <nbd@nbd.name> wrote:
>>
>> On 2018-02-08 06:28, Kai-Heng Feng wrote:
>>> Without btcoex_enable, WiFi activies make both WiFi and Bluetooth
>>> unstable if there's a bluetooth connection.
>>>
>>> Enable this option when bt_ant_diversity is disabled.
>>>
>>> BugLink: https://bugs.launchpad.net/bugs/1746164
>>> Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com>
>> I think this might cause regressions on devices that don't have
>> bluetooth. This probably either needs more EEPROM checks, or something
>> to selectively enable it only on affected platforms.
>
> I think it’s better not to use dmi_match. This issue should affect
> more ath9k. And bluetooth peripherals are more than ever now, so it
> would be great to use BT out of the box.

Sure, but we have to make sure that we don't create regressions on
existing systems. For example, did you test this with any system which
don't support btcoex? (just asking, haven't tested this myself)

-- 
Kalle Valo

Re: [PATCH] ath9k: turn on btcoex_enable as default

[Kai Heng Feng]

> On 9 Feb 2018, at 3:16 PM, Kalle Valo <kvalo-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org> wrote:
> Sure, but we have to make sure that we don't create regressions on
> existing systems. For example, did you test this with any system which
> don't support btcoex? (just asking, haven't tested this myself)

No not really, but I will definitely test it.
The only module I have that uses ath9k is Dell’s DW1707.
How do I check if it support btcoex or not?

(I resend the mail because my last mail get changed to HTML by my mail client)

Kai-Heng

> 
> -- 
> Kalle Valo

Re: [PATCH] ath9k: turn on btcoex_enable as default

[Felix Fietkau]

On 2018-02-10 14:56, Kai Heng Feng wrote:
> 
>> On 9 Feb 2018, at 3:16 PM, Kalle Valo <kvalo@codeaurora.org> wrote:
>> Sure, but we have to make sure that we don't create regressions on
>> existing systems. For example, did you test this with any system which
>> don't support btcoex? (just asking, haven't tested this myself)
> 
> No not really, but I will definitely test it.
> The only module I have that uses ath9k is Dell’s DW1707.
> How do I check if it support btcoex or not?
I just reviewed the code again, and I am sure that we cannot merge this
patch. Enabling the btcoex parameter makes the driver enable a whole
bunch of code starting timers, listening to some GPIOs, etc.

On non-btcoex systems, some of those GPIOs might be floating or even
connected to different things, which could cause a lot of undefined
behavior.

This is simply too big a risk, so there absolutely needs to be a
whitelist for systems that need this, otherwise it has to remain
disabled by default.

- Felix

Re: [PATCH] ath9k: turn on btcoex_enable as default

[Kai Heng Feng]

> On 10 Feb 2018, at 10:05 PM, Felix Fietkau <nbd@nbd.name> wrote:
> 
> On 2018-02-10 14:56, Kai Heng Feng wrote:
>> 
>>> On 9 Feb 2018, at 3:16 PM, Kalle Valo <kvalo@codeaurora.org> wrote:
>>> Sure, but we have to make sure that we don't create regressions on
>>> existing systems. For example, did you test this with any system which
>>> don't support btcoex? (just asking, haven't tested this myself)
>> 
>> No not really, but I will definitely test it.
>> The only module I have that uses ath9k is Dell’s DW1707.
>> How do I check if it support btcoex or not?
> I just reviewed the code again, and I am sure that we cannot merge this
> patch. Enabling the btcoex parameter makes the driver enable a whole
> bunch of code starting timers, listening to some GPIOs, etc.
> 
> On non-btcoex systems, some of those GPIOs might be floating or even
> connected to different things, which could cause a lot of undefined
> behavior.
> 
> This is simply too big a risk, so there absolutely needs to be a
> whitelist for systems that need this, otherwise it has to remain
> disabled by default.

So what information can we use to whitelist btcoex chips?
Can we get btcoex support status at ath9k probing?

Kai-Heng

> 
> - Felix

Re: [PATCH] ath9k: turn on btcoex_enable as default

[Kai-Heng Feng]

at 12:15, Kai Heng Feng <kai.heng.feng@canonical.com> wrote:

>
>
>> On 10 Feb 2018, at 10:05 PM, Felix Fietkau <nbd@nbd.name> wrote:
>>
>> On 2018-02-10 14:56, Kai Heng Feng wrote:
>>>> On 9 Feb 2018, at 3:16 PM, Kalle Valo <kvalo@codeaurora.org> wrote:
>>>> Sure, but we have to make sure that we don't create regressions on
>>>> existing systems. For example, did you test this with any system which
>>>> don't support btcoex? (just asking, haven't tested this myself)
>>>
>>> No not really, but I will definitely test it.
>>> The only module I have that uses ath9k is Dell’s DW1707.
>>> How do I check if it support btcoex or not?
>> I just reviewed the code again, and I am sure that we cannot merge this
>> patch. Enabling the btcoex parameter makes the driver enable a whole
>> bunch of code starting timers, listening to some GPIOs, etc.
>>
>> On non-btcoex systems, some of those GPIOs might be floating or even
>> connected to different things, which could cause a lot of undefined
>> behavior.
>>
>> This is simply too big a risk, so there absolutely needs to be a
>> whitelist for systems that need this, otherwise it has to remain
>> disabled by default.
>
> So what information can we use to whitelist btcoex chips?
> Can we get btcoex support status at ath9k probing?

Sorry for bringing this up again.

Is DMI based match an acceptable approach for ath9k?

Kai-Heng

>
> Kai-Heng
>
>> - Felix

Re: [PATCH] ath9k: turn on btcoex_enable as default

[Kalle Valo]

Kai-Heng Feng <kai.heng.feng@canonical.com> writes:

> at 12:15, Kai Heng Feng <kai.heng.feng@canonical.com> wrote:
>
>>
>>
>>> On 10 Feb 2018, at 10:05 PM, Felix Fietkau <nbd@nbd.name> wrote:
>>>
>>> On 2018-02-10 14:56, Kai Heng Feng wrote:
>>>>> On 9 Feb 2018, at 3:16 PM, Kalle Valo <kvalo@codeaurora.org> wrote:
>>>>> Sure, but we have to make sure that we don't create regressions on
>>>>> existing systems. For example, did you test this with any system which
>>>>> don't support btcoex? (just asking, haven't tested this myself)
>>>>
>>>> No not really, but I will definitely test it.
>>>> The only module I have that uses ath9k is Dell’s DW1707.
>>>> How do I check if it support btcoex or not?
>>> I just reviewed the code again, and I am sure that we cannot merge this
>>> patch. Enabling the btcoex parameter makes the driver enable a whole
>>> bunch of code starting timers, listening to some GPIOs, etc.
>>>
>>> On non-btcoex systems, some of those GPIOs might be floating or even
>>> connected to different things, which could cause a lot of undefined
>>> behavior.
>>>
>>> This is simply too big a risk, so there absolutely needs to be a
>>> whitelist for systems that need this, otherwise it has to remain
>>> disabled by default.
>>
>> So what information can we use to whitelist btcoex chips?
>> Can we get btcoex support status at ath9k probing?
>
> Sorry for bringing this up again.
>
> Is DMI based match an acceptable approach for ath9k?

I don't know what Felix thinkgs, but to me using DMI sounds like a good
idea to try, assuming the matches are unique enough and there's no risk
of enabling bt coex on wrong platforms. Should the PCI bus number etc
checked as well in case the user adds more ath9k devices to the
platform?

But of course I need to see the patch to comment more.

-- 
Kalle Valo

Re: [PATCH] ath9k: turn on btcoex_enable as default

[Tom Psyborg]

I keep this setting on all the time and just when i read this mail
again i'm suspicious if the bluetooth could actually have an impact on
wifi reception? I am using AR9462 card and it can transmit at 215Mbps
average, but receives only about 125Mbps (2spatial streams AP, 2.4GHz,
AR9531)

On 23/08/2018, Kalle Valo <kvalo@codeaurora.org> wrote:
> Kai-Heng Feng <kai.heng.feng@canonical.com> writes:
>
>> at 12:15, Kai Heng Feng <kai.heng.feng@canonical.com> wrote:
>>
>>>
>>>
>>>> On 10 Feb 2018, at 10:05 PM, Felix Fietkau <nbd@nbd.name> wrote:
>>>>
>>>> On 2018-02-10 14:56, Kai Heng Feng wrote:
>>>>>> On 9 Feb 2018, at 3:16 PM, Kalle Valo <kvalo@codeaurora.org> wrote:
>>>>>> Sure, but we have to make sure that we don't create regressions on
>>>>>> existing systems. For example, did you test this with any system
>>>>>> which
>>>>>> don't support btcoex? (just asking, haven't tested this myself)
>>>>>
>>>>> No not really, but I will definitely test it.
>>>>> The only module I have that uses ath9k is Dell’s DW1707.
>>>>> How do I check if it support btcoex or not?
>>>> I just reviewed the code again, and I am sure that we cannot merge this
>>>> patch. Enabling the btcoex parameter makes the driver enable a whole
>>>> bunch of code starting timers, listening to some GPIOs, etc.
>>>>
>>>> On non-btcoex systems, some of those GPIOs might be floating or even
>>>> connected to different things, which could cause a lot of undefined
>>>> behavior.
>>>>
>>>> This is simply too big a risk, so there absolutely needs to be a
>>>> whitelist for systems that need this, otherwise it has to remain
>>>> disabled by default.
>>>
>>> So what information can we use to whitelist btcoex chips?
>>> Can we get btcoex support status at ath9k probing?
>>
>> Sorry for bringing this up again.
>>
>> Is DMI based match an acceptable approach for ath9k?
>
> I don't know what Felix thinkgs, but to me using DMI sounds like a good
> idea to try, assuming the matches are unique enough and there's no risk
> of enabling bt coex on wrong platforms. Should the PCI bus number etc
> checked as well in case the user adds more ath9k devices to the
> platform?
>
> But of course I need to see the patch to comment more.
>
> --
> Kalle Valo
>