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

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

[Chris Chiu]

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;
-- 
2.11.0

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

[Chris Chiu]

On Thu, Jun 27, 2019 at 5:52 PM Chris Chiu <chiu@endlessm.com> 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;
> --
> 2.11.0
>

Gentle ping. Cheers.

Chris

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

[Daniel Drake]

Hi Chris,

On Thu, Jun 27, 2019 at 5:53 PM Chris Chiu <chiu@endlessm.com> 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>

Really nice work finding this!

I know that after this change, you plan to bring over the btcoexist
code from the vendor driver (or at least the minimum required code)
for a more complete fix, but I'm curious how you found these magic
register values and how they compare to the values used by the vendor
driver with btcoexist?

What's PTA? A type of firmware-implemented btcoexist that works for
scanning but doesn't work when a BT connection is actually
established?

> 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);

The comment above this still says "WLAN action by PTA" and the vendor
driver has:
        //set wlan_act control by PTA
        pBtCoexist->fBtcWrite1Byte(pBtCoexist, 0x76e, 0x4);

but then also:
            //set wlan_act control by PTA
            pBtCoexist->fBtcWrite1Byte(pBtCoexist, 0x76e, 0xc);

So this change seems to be at least consistent with ambiguity of the
vendor driver, do you have any understanding of the extra bit that is
now set here?

It's not easy to follow the code flow of the vendor driver to see what
actually happens, have you checked that, does it end up using the 0xc
value?

> -        * 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);

I don't quite follow the comment here. Why are there 2 values listed
for each possibility, what do you mean by inverse? You say the
register settings were incorrect, but the previous value was 0x00
which you now document as "antenna switch to wifi" which sounds like
it was already correct?

Which value does the vendor driver use?

>         /*
>          * 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;

At a glance I can't see which code this corresponds to in the vendor
driver, can you point that out?

Thanks
Daniel

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

[Chris Chiu]

On Mon, Jul 1, 2019 at 4:28 PM Daniel Drake <drake@endlessm.com> wrote:
>
> Hi Chris,
>
> On Thu, Jun 27, 2019 at 5:53 PM Chris Chiu <chiu@endlessm.com> 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>
>
> Really nice work finding this!
>
> I know that after this change, you plan to bring over the btcoexist
> code from the vendor driver (or at least the minimum required code)
> for a more complete fix, but I'm curious how you found these magic
> register values and how they compare to the values used by the vendor
> driver with btcoexist?
>
> What's PTA? A type of firmware-implemented btcoexist that works for
> scanning but doesn't work when a BT connection is actually
> established?
>

When the vendor driver invokes rtw_btcoex_HAL_Initialize, which will then
call halbtc8723b1ant_SetAntPath to configure the registers in this patch.
From the code, the registers will have different register settings per the
antenna position and the phase. If the driver is in the InitHwConfig phase,
the register value is identical to what rtl8xxxu driver does in enable_rf().
However, the vendor driver will do halbtc8723b1ant_PsTdma() twice by
halbtc8723b1ant_ActionWifiNotConnected() with the type argument 8 for
PTA control about 200ms after InitHwConfig. The _ActionWifiNotConnected
is invoked by the BTCOEXIST. I keep seeing the halbtc8723b1ant_PsTdma
with type 8 been called every 2 seconds.

I don't know what PTA is. I presume it's the mechanism in FW for the automatic
antenna selecting instead of manual control. Given the phenomenon that wifi
signal still stays low even without bluetooth driver loaded, I believe
setting the
registers as in halbtc8723b1ant_SetAntPath with BTC_ANT_PATH_PTA also
makes sense.



> > 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);
>
> The comment above this still says "WLAN action by PTA" and the vendor
> driver has:
>         //set wlan_act control by PTA
>         pBtCoexist->fBtcWrite1Byte(pBtCoexist, 0x76e, 0x4);
>
> but then also:
>             //set wlan_act control by PTA
>             pBtCoexist->fBtcWrite1Byte(pBtCoexist, 0x76e, 0xc);
>
> So this change seems to be at least consistent with ambiguity of the
> vendor driver, do you have any understanding of the extra bit that is
> now set here?
>
I think the precise expression for 0x04 is "set wlan act to always low",
it's configured for wifi only.

> It's not easy to follow the code flow of the vendor driver to see what
> actually happens, have you checked that, does it end up using the 0xc
> value?
>

Yes, it ends up with 0x0c not matter what antenna position type is. Unless
it's configured wifi only.

> > -        * 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);
>
> I don't quite follow the comment here. Why are there 2 values listed
> for each possibility, what do you mean by inverse? You say the
> register settings were incorrect, but the previous value was 0x00
> which you now document as "antenna switch to wifi" which sounds like
> it was already correct?
>
> Which value does the vendor driver use?
>
The first column means the value for normal antenna installation, wifi
on the main port. The second column is the value for inverse antenna
installation. So if I want to manually switch the antenna for BT use,
and the antenna installation is inverse, I need to set to 0x280. So 0x80
means I want to switch to PTA and the antenna installation in inverse.

The vendor driver's code about this is also in halbtc8723b1ant_SetAntPath.

> >         /*
> >          * 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;
>
> At a glance I can't see which code this corresponds to in the vendor
> driver, can you point that out?
>
> Thanks
> Daniel

It's in rtl8723bu_hal_init and the comment says "Check if MAC has already
power on". In vendor driver, it's just for output messages but in rtl8xxxu, it
will determine whether if the llt_init() and tx related registers
being correctly
initialized. I sometimes hit the problem of connection failure after boot and
it's because the macpower is falsely true.

Chris

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

[Jes Sorensen]

On 7/1/19 4:27 AM, Daniel Drake wrote:
> Hi Chris,
> 
> On Thu, Jun 27, 2019 at 5:53 PM Chris Chiu <chiu@endlessm.com> 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>
> 
> Really nice work finding this!
> 
> I know that after this change, you plan to bring over the btcoexist
> code from the vendor driver (or at least the minimum required code)
> for a more complete fix, but I'm curious how you found these magic
> register values and how they compare to the values used by the vendor
> driver with btcoexist?

We definitely don't want to bring over the vendor code, since it's a
pile of spaghetti, but we probably need to get something sorted. This
went down the drain when the bluetooth driver was added without taking
it into account - long after this driver was merged.

Cheers,
Jes

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

[Jes Sorensen]

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

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

[Chris Chiu]

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))

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.

Chris

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

[Daniel Drake]

On Tue, Jul 2, 2019 at 8:42 PM Jes Sorensen <jes.sorensen@gmail.com> wrote:
> We definitely don't want to bring over the vendor code, since it's a
> pile of spaghetti, but we probably need to get something sorted. This
> went down the drain when the bluetooth driver was added without taking
> it into account - long after this driver was merged.

Yeah, I didn't mean bring over quite so literally.. Chris is studying
it and figuring out the neatest way to reimplement the required bits.

As for the relationship with bluetooth.. actually the bug that Chris
is working on here is that the rtl8xxxu wifi signal is totally
unusable *until* the bluetooth driver is loaded.
Once the bluetooth driver is loaded, at the point of bluetooth
firmware upload, the rtl8xxxu signal magiaclly strength becomes good.
I think this is consistent with other rtl8xxxu problem reports that we
saw lying around, although they had not been diagnosed in so much
detail.
The rtl8723bu vendor driver does not suffer this problem, it works
fine with or without the bluetooth driver in place.

Daniel

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

[Daniel Drake]

On Tue, Jul 2, 2019 at 4:01 PM Chris Chiu <chiu@endlessm.com> wrote:
> When the vendor driver invokes rtw_btcoex_HAL_Initialize, which will then
> call halbtc8723b1ant_SetAntPath to configure the registers in this patch.
> From the code, the registers will have different register settings per the
> antenna position and the phase. If the driver is in the InitHwConfig phase,
> the register value is identical to what rtl8xxxu driver does in enable_rf().
> However, the vendor driver will do halbtc8723b1ant_PsTdma() twice by
> halbtc8723b1ant_ActionWifiNotConnected() with the type argument 8 for
> PTA control about 200ms after InitHwConfig. The _ActionWifiNotConnected
> is invoked by the BTCOEXIST. I keep seeing the halbtc8723b1ant_PsTdma
> with type 8 been called every 2 seconds.

I see. So this is a measured step towards consistency with the vendor
driver. Maybe you can mention these details in the commit message.

> Yes, it ends up with 0x0c not matter what antenna position type is. Unless
> it's configured wifi only.

Also worth mentioning in the commit message then, that the 0xc
ACT_CONTROL value is effectively what the working vendor driver uses.

> > > -        * 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);
> >
> > I don't quite follow the comment here. Why are there 2 values listed
> > for each possibility, what do you mean by inverse? You say the
> > register settings were incorrect, but the previous value was 0x00
> > which you now document as "antenna switch to wifi" which sounds like
> > it was already correct?
> >
> > Which value does the vendor driver use?
> >
> The first column means the value for normal antenna installation, wifi
> on the main port. The second column is the value for inverse antenna
> installation. So if I want to manually switch the antenna for BT use,
> and the antenna installation is inverse, I need to set to 0x280. So 0x80
> means I want to switch to PTA and the antenna installation in inverse.

Still not quite clear what you mean by "inverse" here, but maybe I
just don't know anything about antennas. Is it that an antenna
connector has two pins and this one swaps the meaning of each pin?

Does the new value of 0x80 reflect what the vendor driver does in practice?

Thanks
Daniel

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

[Jes Sorensen]

On 7/3/19 3:42 AM, Daniel Drake wrote:
> On Tue, Jul 2, 2019 at 8:42 PM Jes Sorensen <jes.sorensen@gmail.com> wrote:
>> We definitely don't want to bring over the vendor code, since it's a
>> pile of spaghetti, but we probably need to get something sorted. This
>> went down the drain when the bluetooth driver was added without taking
>> it into account - long after this driver was merged.
> 
> Yeah, I didn't mean bring over quite so literally.. Chris is studying
> it and figuring out the neatest way to reimplement the required bits.
> 
> As for the relationship with bluetooth.. actually the bug that Chris
> is working on here is that the rtl8xxxu wifi signal is totally
> unusable *until* the bluetooth driver is loaded.

So this is not my experience at all from when I wrote the code. The
8723bu dongle I used for it came up just fine.

> Once the bluetooth driver is loaded, at the point of bluetooth
> firmware upload, the rtl8xxxu signal magiaclly strength becomes good.
> I think this is consistent with other rtl8xxxu problem reports that we
> saw lying around, although they had not been diagnosed in so much
> detail.

See this is the very opposite of what I have experienced. The bluetooth
driver ruins the signal when it's loaded with my dongle.

> The rtl8723bu vendor driver does not suffer this problem, it works
> fine with or without the bluetooth driver in place.

My point is this seems to be very dongle dependent :( We have to be
careful not breaking it for some users while fixing it for others.

Cheers,
Jes

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

[Jes Sorensen]

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

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

[Daniel Drake]

On Wed, Jul 3, 2019 at 8:59 PM Jes Sorensen <jes.sorensen@gmail.com> wrote:
> My point is this seems to be very dongle dependent :( We have to be
> careful not breaking it for some users while fixing it for others.

Do you still have your device?

Once we get to the point when you are happy with Chris's two patches
here on a code review level, we'll reach out to other driver
contributors plus people who previously complained about these types
of problems, and see if we can get some wider testing.

Larry, do you have these devices, can you help with testing too?

Thanks
Daniel

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

[Jes Sorensen]

On 7/4/19 11:44 PM, Daniel Drake wrote:
> On Wed, Jul 3, 2019 at 8:59 PM Jes Sorensen <jes.sorensen@gmail.com> wrote:
>> My point is this seems to be very dongle dependent :( We have to be
>> careful not breaking it for some users while fixing it for others.
> 
> Do you still have your device?
> 
> Once we get to the point when you are happy with Chris's two patches
> here on a code review level, we'll reach out to other driver
> contributors plus people who previously complained about these types
> of problems, and see if we can get some wider testing.

I should have them, but I won't have access to them for another 2.5
weeks unfortunately.

Cheers,
Jes

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

[Larry Finger]

On 7/4/19 10:44 PM, Daniel Drake wrote:
> On Wed, Jul 3, 2019 at 8:59 PM Jes Sorensen <jes.sorensen@gmail.com> wrote:
>> My point is this seems to be very dongle dependent :( We have to be
>> careful not breaking it for some users while fixing it for others.
> 
> Do you still have your device?
> 
> Once we get to the point when you are happy with Chris's two patches
> here on a code review level, we'll reach out to other driver
> contributors plus people who previously complained about these types
> of problems, and see if we can get some wider testing.
> 
> Larry, do you have these devices, can you help with testing too?

I have some of the devices, and I can help with the testing.

Larry