Re: [PATCH] rtl8xxxu: Fix wifi low signal strength issue of RTL8723BU

[Jes Sorensen <jes.sorensen@gmail.com>]

On 7/2/19 11:25 PM, Chris Chiu wrote:
> On Tue, Jul 2, 2019 at 8:44 PM Jes Sorensen <jes.sorensen@gmail.com> wrote:
>>
>> On 6/27/19 5:52 AM, Chris Chiu wrote:
>>> The WiFi tx power of RTL8723BU is extremely low after booting. So
>>> the WiFi scan gives very limited AP list and it always fails to
>>> connect to the selected AP. This module only supports 1x1 antenna
>>> and the antenna is switched to bluetooth due to some incorrect
>>> register settings.
>>>
>>> This commit hand over the antenna control to PTA, the wifi signal
>>> will be back to normal and the bluetooth scan can also work at the
>>> same time. However, the btcoexist still needs to be handled under
>>> different circumstances. If there's a BT connection established,
>>> the wifi still fails to connect until disconneting the BT.
>>>
>>> Signed-off-by: Chris Chiu <chiu@endlessm.com>
>>> ---
>>>  drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8723b.c | 9 ++++++---
>>>  drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c  | 3 ++-
>>>  2 files changed, 8 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8723b.c b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8723b.c
>>> index 3adb1d3d47ac..6c3c70d93ac1 100644
>>> --- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8723b.c
>>> +++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8723b.c
>>> @@ -1525,7 +1525,7 @@ static void rtl8723b_enable_rf(struct rtl8xxxu_priv *priv)
>>>       /*
>>>        * WLAN action by PTA
>>>        */
>>> -     rtl8xxxu_write8(priv, REG_WLAN_ACT_CONTROL_8723B, 0x04);
>>> +     rtl8xxxu_write8(priv, REG_WLAN_ACT_CONTROL_8723B, 0x0c);
>>>
>>>       /*
>>>        * BT select S0/S1 controlled by WiFi
>>> @@ -1568,9 +1568,12 @@ static void rtl8723b_enable_rf(struct rtl8xxxu_priv *priv)
>>>       rtl8xxxu_gen2_h2c_cmd(priv, &h2c, sizeof(h2c.ant_sel_rsv));
>>>
>>>       /*
>>> -      * 0x280, 0x00, 0x200, 0x80 - not clear
>>> +      * Different settings per different antenna position.
>>> +      * Antenna switch to BT: 0x280, 0x00 (inverse)
>>> +      * Antenna switch to WiFi: 0x0, 0x280 (inverse)
>>> +      * Antenna controlled by PTA: 0x200, 0x80 (inverse)
>>>        */
>>> -     rtl8xxxu_write32(priv, REG_S0S1_PATH_SWITCH, 0x00);
>>> +     rtl8xxxu_write32(priv, REG_S0S1_PATH_SWITCH, 0x80);
>>>
>>>       /*
>>>        * Software control, antenna at WiFi side
>>> diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
>>> index 8136e268b4e6..87b2179a769e 100644
>>> --- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
>>> +++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
>>> @@ -3891,12 +3891,13 @@ static int rtl8xxxu_init_device(struct ieee80211_hw *hw)
>>>
>>>       /* Check if MAC is already powered on */
>>>       val8 = rtl8xxxu_read8(priv, REG_CR);
>>> +     val16 = rtl8xxxu_read16(priv, REG_SYS_CLKR);
>>>
>>>       /*
>>>        * Fix 92DU-VC S3 hang with the reason is that secondary mac is not
>>>        * initialized. First MAC returns 0xea, second MAC returns 0x00
>>>        */
>>> -     if (val8 == 0xea)
>>> +     if (val8 == 0xea || !(val16 & BIT(11)))
>>>               macpower = false;
>>>       else
>>>               macpower = true;
>>
>> This part I would like to ask you take a good look at the other chips to
>> make sure you don't break support for 8192cu, 8723au, 8188eu with this.
>>
>> Cheers,
>> Jes
> 
> I checked the vendor code of 8192cu and 8188eu, they don't have this part
> of code to check the REG_CR before power on sequence. I can only find
> similar code in rtl8723be.
> if (tmp_u1b != 0 && tmp_u1b !=0xea)
>     rtlhal->mac_func_enable = true;
> 
> By definition, the BIT(11) of REG_SYS_CLKR in rtl8xxxu_regs.h is
> SYS_CLK_MAC_CLK_ENABLE. It seems to make sense to check this value
> for macpower no matter what chip it is. I think I can make it more
> self-expressive
> as down below.
> 
>  if (val8 == 0xea || !(val16 & SYS_CLK_MAC_CLK_ENABLE))

Yes, please always use the descriptive defines rather than hard coding
the bit numbers.

> And per the comment, this code is for 92DU-VC S3 hang problem and I think an
> OR check for SYS_CLK_MAC_CLK_ENABLE is still safe for this.

Sounds reasonable - keep in mind that some of these bugs may have been
fixed for one chip, and then just copied forward.

Cheers,
Jes