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

From: Daniel Drake <drake@endlessm.com>
To: Chris Chiu <chiu@endlessm.com>
Cc: Jes Sorensen <jes.sorensen@gmail.com>,
	Kalle Valo <kvalo@codeaurora.org>,
	David Miller <davem@davemloft.net>,
	linux-wireless <linux-wireless@vger.kernel.org>,
	netdev <netdev@vger.kernel.org>,
	Linux Kernel <linux-kernel@vger.kernel.org>,
	Linux Upstreaming Team <linux@endlessm.com>,
	Larry Finger <Larry.Finger@lwfinger.net>
Subject: Re: [PATCH] rtl8xxxu: Fix wifi low signal strength issue of RTL8723BU
Date: Wed, 3 Jul 2019 15:51:52 +0800	[thread overview]
Message-ID: <CAD8Lp44fjuJ-hf=8Spb9n8gkv-sS0f2O3Xv+E6=MmC_ugKnW-g@mail.gmail.com> (raw)
In-Reply-To: <CAB4CAwcVoWffpK8xR_UbXaGyHh8ZrrX_9vvzjAkWGKXQotpmYA@mail.gmail.com>

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