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 <[email protected]>
---
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
Hi Chris,
On Thu, Jun 27, 2019 at 5:53 PM Chris Chiu <[email protected]> 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 <[email protected]>
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
On Mon, Jul 1, 2019 at 4:28 PM Daniel Drake <[email protected]> wrote:
>
> Hi Chris,
>
> On Thu, Jun 27, 2019 at 5:53 PM Chris Chiu <[email protected]> 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 <[email protected]>
>
> 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
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 <[email protected]>
> ---
> 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
On Tue, Jul 2, 2019 at 8:44 PM Jes Sorensen <[email protected]> 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 <[email protected]>
> > ---
> > 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
On Tue, Jul 2, 2019 at 4:01 PM Chris Chiu <[email protected]> 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
On 7/2/19 11:25 PM, Chris Chiu wrote:
> On Tue, Jul 2, 2019 at 8:44 PM Jes Sorensen <[email protected]> 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 <[email protected]>
>>> ---
>>> 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
On Wed, Jul 3, 2019 at 8:59 PM Jes Sorensen <[email protected]> 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
On 7/4/19 10:44 PM, Daniel Drake wrote:
> On Wed, Jul 3, 2019 at 8:59 PM Jes Sorensen <[email protected]> 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