2014-10-09 08:38:12

by Rafał Miłecki

[permalink] [raw]
Subject: [PATCH][RTF][RFC] brcmsmac: add workaround for old BCM4313 devices with Bluetooth

Signed-off-by: Rafał Miłecki <[email protected]>
---
Please check if it fixes BCM4313 performance problems.
---
drivers/net/wireless/brcm80211/brcmsmac/aiutils.c | 13 +++++++++++++
drivers/net/wireless/brcm80211/brcmsmac/aiutils.h | 3 +++
drivers/net/wireless/brcm80211/brcmsmac/main.c | 6 ++++++
3 files changed, 22 insertions(+)

diff --git a/drivers/net/wireless/brcm80211/brcmsmac/aiutils.c b/drivers/net/wireless/brcm80211/brcmsmac/aiutils.c
index 5336597..3d39176 100644
--- a/drivers/net/wireless/brcm80211/brcmsmac/aiutils.c
+++ b/drivers/net/wireless/brcm80211/brcmsmac/aiutils.c
@@ -310,6 +310,9 @@
/* External PA enable mask */
#define GPIO_CTRL_EPA_EN_MASK 0x40

+/* WL/BT control enable mask */
+#define GPIO_CTRL_5_6_EN_MASK 0x60
+
#define DEFAULT_GPIOTIMERVAL \
((DEFAULT_GPIO_ONTIME << GPIO_ONTIME_SHIFT) | DEFAULT_GPIO_OFFTIME)

@@ -691,6 +694,16 @@ void ai_epa_4313war(struct si_pub *sih)
bcma_set32(cc, CHIPCREGOFFS(gpiocontrol), GPIO_CTRL_EPA_EN_MASK);
}

+/* WL/BT control for 4313 btcombo boards >= P250 */
+void ai_btcombo_p250_4313_war(struct si_pub *sih)
+{
+ struct si_info *sii = container_of(sih, struct si_info, pub);
+ struct bcma_device *cc = sii->icbus->drv_cc.core;
+
+ bcma_set32(cc, CHIPCREGOFFS(gpiocontrol), GPIO_CTRL_5_6_EN_MASK);
+ bcma_set32(cc, CHIPCREGOFFS(gpioouten), GPIO_CTRL_5_6_EN_MASK);
+}
+
/* check if the device is removed */
bool ai_deviceremoved(struct si_pub *sih)
{
diff --git a/drivers/net/wireless/brcm80211/brcmsmac/aiutils.h b/drivers/net/wireless/brcm80211/brcmsmac/aiutils.h
index 2d08c15..7652a0f 100644
--- a/drivers/net/wireless/brcm80211/brcmsmac/aiutils.h
+++ b/drivers/net/wireless/brcm80211/brcmsmac/aiutils.h
@@ -186,6 +186,9 @@ bool ai_deviceremoved(struct si_pub *sih);
/* Enable Ex-PA for 4313 */
void ai_epa_4313war(struct si_pub *sih);

+/* WL/BT control for 4313 btcombo boards >= P250 */
+void ai_btcombo_p250_4313_war(struct si_pub *sih);
+
static inline u32 ai_get_cccaps(struct si_pub *sih)
{
return sih->cccaps;
diff --git a/drivers/net/wireless/brcm80211/brcmsmac/main.c b/drivers/net/wireless/brcm80211/brcmsmac/main.c
index 1b47482..046e832 100644
--- a/drivers/net/wireless/brcm80211/brcmsmac/main.c
+++ b/drivers/net/wireless/brcm80211/brcmsmac/main.c
@@ -4942,6 +4942,12 @@ static void brcms_b_hw_up(struct brcms_hardware *wlc_hw)
&& (wlc_hw->boardflags & BFL_FEM_BT)))
ai_epa_4313war(wlc_hw->sih);
}
+
+ /* TODO: Fix the condition. Only for boards >= P250 */
+ if (ai_get_chip_id(wlc_hw->sih) == BCMA_CHIP_ID_BCM4313 && (wlc_hw->boardflags & BFL_FEM_BT)) {
+ pr_info("Applying BCM4313 WL/BT workaround\n");
+ ai_btcombo_p250_4313_war(wlc_hw->sih);
+ }
}

static int brcms_b_up_prep(struct brcms_hardware *wlc_hw)
--
1.8.4.5



2014-10-09 09:02:41

by Rafał Miłecki

[permalink] [raw]
Subject: Re: [PATCH][RTF][RFC] brcmsmac: add workaround for old BCM4313 devices with Bluetooth

On 9 October 2014 10:37, Rafał Miłecki <[email protected]> wrote:
> + /* TODO: Fix the condition. Only for boards >= P250 */
> + if (ai_get_chip_id(wlc_hw->sih) == BCMA_CHIP_ID_BCM4313 && (wlc_hw->boardflags & BFL_FEM_BT)) {
> + pr_info("Applying BCM4313 WL/BT workaround\n");
> + ai_btcombo_p250_4313_war(wlc_hw->sih);
> + }

This of course have to be checked in some hardware documentation for
the correct condition. We already have some workaround (right above
the newly added code) for boards with boardrev >= 0x1250. So my guess
is the code I added applies to some other cards. The board this patch
is supposed to fix is:
board vendor: 14e4
board type: 608
board revision: 1109
board flags: 402201
board flags2: 884
firmware revision: 262032c

So whatever condition we will need it'll likely need to cover above
case (maybe boardrev == 0x1109?).

--
Rafał

2014-10-09 10:11:54

by Rafał Miłecki

[permalink] [raw]
Subject: Re: [PATCH][RTF][RFC] brcmsmac: add workaround for old BCM4313 devices with Bluetooth

On 9 October 2014 11:46, Arend van Spriel <[email protected]> wrote:
> On 10/09/14 11:02, Rafał Miłecki wrote:
>>
>> On 9 October 2014 10:37, Rafał Miłecki<[email protected]> wrote:
>>>
>>> + /* TODO: Fix the condition. Only for boards>= P250 */
>>> + if (ai_get_chip_id(wlc_hw->sih) == BCMA_CHIP_ID_BCM4313&&
>>> (wlc_hw->boardflags& BFL_FEM_BT)) {
>>> + pr_info("Applying BCM4313 WL/BT workaround\n");
>>> + ai_btcombo_p250_4313_war(wlc_hw->sih);
>>> + }
>>
>>
>> This of course have to be checked in some hardware documentation for
>> the correct condition. We already have some workaround (right above
>> the newly added code) for boards with boardrev>= 0x1250. So my guess
>> is the code I added applies to some other cards. The board this patch
>> is supposed to fix is:
>> board vendor: 14e4
>> board type: 608
>> board revision: 1109
>> board flags: 402201
>> board flags2: 884
>> firmware revision: 262032c
>>
>> So whatever condition we will need it'll likely need to cover above
>> case (maybe boardrev == 0x1109?).
>
>
> Well, there is something fishy going on. The brcmsmac code looks like:
>
> if (bfl & BFL_FEM && chip == 4313) {
> if (!(boardrev >= 0x1250 && bfl & BFL_FEM_BT))
> ai_epa_4313war(wlc_hw->sih);
> }

Ohh, I didn't notice this negation at the beginning... Now meaning of
my functions makes more sense. The old code it only for boardrev <
0x1250 (plus other conditions). This new function has "p250" in its
name, that may mean it's for boardrev >= 0x1250.


> However the boardflags above (0x402201) only has BFL_FEM_BT set so this code
> is never called. I have to ask if !BFL_FEM && BFL_FEM_BT is a valid
> combination.

Yeah, that's fishy. Maybe that new function ai_btcombo_p250_4313_war
should be called if !BFL_FEM && BFL_FEM_BT? But it sounds weird.

--
Rafał

2014-10-09 10:22:29

by Arend van Spriel

[permalink] [raw]
Subject: Re: [PATCH][RTF][RFC] brcmsmac: add workaround for old BCM4313 devices with Bluetooth

On 10/09/14 12:11, Rafał Miłecki wrote:
> On 9 October 2014 11:46, Arend van Spriel<[email protected]> wrote:
>> On 10/09/14 11:02, Rafał Miłecki wrote:
>>>
>>> On 9 October 2014 10:37, Rafał Miłecki<[email protected]> wrote:
>>>>
>>>> + /* TODO: Fix the condition. Only for boards>= P250 */
>>>> + if (ai_get_chip_id(wlc_hw->sih) == BCMA_CHIP_ID_BCM4313&&
>>>> (wlc_hw->boardflags& BFL_FEM_BT)) {
>>>> + pr_info("Applying BCM4313 WL/BT workaround\n");
>>>> + ai_btcombo_p250_4313_war(wlc_hw->sih);
>>>> + }
>>>
>>>
>>> This of course have to be checked in some hardware documentation for
>>> the correct condition. We already have some workaround (right above
>>> the newly added code) for boards with boardrev>= 0x1250. So my guess
>>> is the code I added applies to some other cards. The board this patch
>>> is supposed to fix is:
>>> board vendor: 14e4
>>> board type: 608
>>> board revision: 1109
>>> board flags: 402201
>>> board flags2: 884
>>> firmware revision: 262032c
>>>
>>> So whatever condition we will need it'll likely need to cover above
>>> case (maybe boardrev == 0x1109?).
>>
>>
>> Well, there is something fishy going on. The brcmsmac code looks like:
>>
>> if (bfl& BFL_FEM&& chip == 4313) {
>> if (!(boardrev>= 0x1250&& bfl& BFL_FEM_BT))
>> ai_epa_4313war(wlc_hw->sih);
>> }
>
> Ohh, I didn't notice this negation at the beginning... Now meaning of
> my functions makes more sense. The old code it only for boardrev<
> 0x1250 (plus other conditions). This new function has "p250" in its
> name, that may mean it's for boardrev>= 0x1250.
>
>
>> However the boardflags above (0x402201) only has BFL_FEM_BT set so this code
>> is never called. I have to ask if !BFL_FEM&& BFL_FEM_BT is a valid
>> combination.
>
> Yeah, that's fishy. Maybe that new function ai_btcombo_p250_4313_war
> should be called if !BFL_FEM&& BFL_FEM_BT? But it sounds weird.

I know where the function should be called according our driver and
guess what:

if (bfl& BFL_FEM&& chip == 4313) {
if (!(boardrev>= 0x1250 && bfl& BFL_FEM_BT))
ai_epa_4313war(wlc_hw->sih);
+ else
+ ai_btcombo_p250_4313_war(wlc_hw->sih);
}

I hate this inverse logic as it is so easy to overlook ;-)

Regards,
Arend

2014-10-09 09:46:49

by Arend van Spriel

[permalink] [raw]
Subject: Re: [PATCH][RTF][RFC] brcmsmac: add workaround for old BCM4313 devices with Bluetooth

On 10/09/14 11:02, Rafał Miłecki wrote:
> On 9 October 2014 10:37, Rafał Miłecki<[email protected]> wrote:
>> + /* TODO: Fix the condition. Only for boards>= P250 */
>> + if (ai_get_chip_id(wlc_hw->sih) == BCMA_CHIP_ID_BCM4313&& (wlc_hw->boardflags& BFL_FEM_BT)) {
>> + pr_info("Applying BCM4313 WL/BT workaround\n");
>> + ai_btcombo_p250_4313_war(wlc_hw->sih);
>> + }
>
> This of course have to be checked in some hardware documentation for
> the correct condition. We already have some workaround (right above
> the newly added code) for boards with boardrev>= 0x1250. So my guess
> is the code I added applies to some other cards. The board this patch
> is supposed to fix is:
> board vendor: 14e4
> board type: 608
> board revision: 1109
> board flags: 402201
> board flags2: 884
> firmware revision: 262032c
>
> So whatever condition we will need it'll likely need to cover above
> case (maybe boardrev == 0x1109?).

Well, there is something fishy going on. The brcmsmac code looks like:

if (bfl & BFL_FEM && chip == 4313) {
if (!(boardrev >= 0x1250 && bfl & BFL_FEM_BT))
ai_epa_4313war(wlc_hw->sih);
}

However the boardflags above (0x402201) only has BFL_FEM_BT set so this
code is never called. I have to ask if !BFL_FEM && BFL_FEM_BT is a valid
combination.

Regards,
Arend




2014-10-09 10:37:34

by Rafał Miłecki

[permalink] [raw]
Subject: Re: [PATCH][RTF][RFC] brcmsmac: add workaround for old BCM4313 devices with Bluetooth

On 9 October 2014 12:28, Michael Tokarev <[email protected]> wrote:
> 09.10.2014 14:22, Arend van Spriel wrote:
>> I know where the function should be called according our driver and guess what:
>>
>> if (bfl& BFL_FEM&& chip == 4313) {
>
> Can you fix whitespace there too, and add parens around the & operation, while at it?

I think you didn't realize it's a pseudo-code / reference ;)

2014-10-09 10:46:27

by Arend van Spriel

[permalink] [raw]
Subject: Re: [PATCH][RTF][RFC] brcmsmac: add workaround for old BCM4313 devices with Bluetooth

On 10/09/14 12:37, Rafał Miłecki wrote:
> On 9 October 2014 12:28, Michael Tokarev<[email protected]> wrote:
>> 09.10.2014 14:22, Arend van Spriel wrote:
>>> I know where the function should be called according our driver and guess what:
>>>
>>> if (bfl& BFL_FEM&& chip == 4313) {
>>
>> Can you fix whitespace there too, and add parens around the& operation, while at it?
>
> I think you didn't realize it's a pseudo-code / reference ;)

Indeed or just me being lazy :-p

Regards,
Arend

2014-10-09 10:28:05

by Michael Tokarev

[permalink] [raw]
Subject: Re: [PATCH][RTF][RFC] brcmsmac: add workaround for old BCM4313 devices with Bluetooth

09.10.2014 14:22, Arend van Spriel wrote:
> I know where the function should be called according our driver and guess what:
>
> if (bfl& BFL_FEM&& chip == 4313) {

Can you fix whitespace there too, and add parens around the & operation, while at it?

> if (!(boardrev>= 0x1250 && bfl& BFL_FEM_BT))

The same here.

> ai_epa_4313war(wlc_hw->sih);
> + else
> + ai_btcombo_p250_4313_war(wlc_hw->sih);
> }

I'll try the patch with my 4313 in a few days, hopefully (I'll have to disassemble
my notebook again).

/mjt


2014-10-09 10:46:35

by Rafał Miłecki

[permalink] [raw]
Subject: Re: [PATCH][RTF][RFC] brcmsmac: add workaround for old BCM4313 devices with Bluetooth

On 9 October 2014 12:22, Arend van Spriel <[email protected]> wrote:
> On 10/09/14 12:11, Rafał Miłecki wrote:
>>
>> On 9 October 2014 11:46, Arend van Spriel<[email protected]> wrote:
>>>
>>> On 10/09/14 11:02, Rafał Miłecki wrote:
>>>>
>>>>
>>>> On 9 October 2014 10:37, Rafał Miłecki<[email protected]> wrote:
>>>>>
>>>>>
>>>>> + /* TODO: Fix the condition. Only for boards>= P250 */
>>>>> + if (ai_get_chip_id(wlc_hw->sih) == BCMA_CHIP_ID_BCM4313&&
>>>>> (wlc_hw->boardflags& BFL_FEM_BT)) {
>>>>> + pr_info("Applying BCM4313 WL/BT workaround\n");
>>>>> + ai_btcombo_p250_4313_war(wlc_hw->sih);
>>>>> + }
>>>>
>>>>
>>>>
>>>> This of course have to be checked in some hardware documentation for
>>>> the correct condition. We already have some workaround (right above
>>>> the newly added code) for boards with boardrev>= 0x1250. So my guess
>>>> is the code I added applies to some other cards. The board this patch
>>>> is supposed to fix is:
>>>> board vendor: 14e4
>>>> board type: 608
>>>> board revision: 1109
>>>> board flags: 402201
>>>> board flags2: 884
>>>> firmware revision: 262032c
>>>>
>>>> So whatever condition we will need it'll likely need to cover above
>>>> case (maybe boardrev == 0x1109?).
>>>
>>>
>>>
>>> Well, there is something fishy going on. The brcmsmac code looks like:
>>>
>>> if (bfl& BFL_FEM&& chip == 4313) {
>>> if (!(boardrev>= 0x1250&& bfl& BFL_FEM_BT))
>>> ai_epa_4313war(wlc_hw->sih);
>>> }
>>
>>
>> Ohh, I didn't notice this negation at the beginning... Now meaning of
>> my functions makes more sense. The old code it only for boardrev<
>> 0x1250 (plus other conditions). This new function has "p250" in its
>> name, that may mean it's for boardrev>= 0x1250.
>>
>>
>>> However the boardflags above (0x402201) only has BFL_FEM_BT set so this
>>> code
>>> is never called. I have to ask if !BFL_FEM&& BFL_FEM_BT is a valid
>>> combination.
>>
>>
>> Yeah, that's fishy. Maybe that new function ai_btcombo_p250_4313_war
>> should be called if !BFL_FEM&& BFL_FEM_BT? But it sounds weird.
>
>
> I know where the function should be called according our driver and guess
> what:
>
> if (bfl& BFL_FEM&& chip == 4313) {
> if (!(boardrev>= 0x1250 && bfl& BFL_FEM_BT))
> ai_epa_4313war(wlc_hw->sih);
> + else
> + ai_btcombo_p250_4313_war(wlc_hw->sih);
> }

So if your internal codebase for wl driver also checks for BFL_FEM, it
seems my guess was wrong. There must be something else that matters.

Unfortunately I have only BCM94313HMG2L which doesn't include BT.

--
Rafał