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

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

[Rafał Miłecki]

Signed-off-by: Rafał Miłecki <zajec5@gmail.com>
---
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

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

[Rafał Miłecki]

On 9 October 2014 10:37, Rafał Miłecki <zajec5@gmail.com> 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ł

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

[Arend van Spriel]

On 10/09/14 11:02, Rafał Miłecki wrote:
> On 9 October 2014 10:37, Rafał Miłecki<zajec5@gmail.com>  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

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

[Rafał Miłecki]

On 9 October 2014 11:46, Arend van Spriel <arend@broadcom.com> wrote:
> On 10/09/14 11:02, Rafał Miłecki wrote:
>>
>> On 9 October 2014 10:37, Rafał Miłecki<zajec5@gmail.com>  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ł

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

[Arend van Spriel]

On 10/09/14 12:11, Rafał Miłecki wrote:
> On 9 October 2014 11:46, Arend van Spriel<arend@broadcom.com>  wrote:
>> On 10/09/14 11:02, Rafał Miłecki wrote:
>>>
>>> On 9 October 2014 10:37, Rafał Miłecki<zajec5@gmail.com>   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

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

[Michael Tokarev]

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

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

[Rafał Miłecki]

On 9 October 2014 12:28, Michael Tokarev <mjt@tls.msk.ru> 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 ;)

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

[Arend van Spriel]

On 10/09/14 12:37, Rafał Miłecki wrote:
> On 9 October 2014 12:28, Michael Tokarev<mjt@tls.msk.ru>  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

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

[Rafał Miłecki]

On 9 October 2014 12:22, Arend van Spriel <arend@broadcom.com> wrote:
> On 10/09/14 12:11, Rafał Miłecki wrote:
>>
>> On 9 October 2014 11:46, Arend van Spriel<arend@broadcom.com>  wrote:
>>>
>>> On 10/09/14 11:02, Rafał Miłecki wrote:
>>>>
>>>>
>>>> On 9 October 2014 10:37, Rafał Miłecki<zajec5@gmail.com>   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ł