Re: [PATCH] Bluetooth: hci_qca: Add delay for wcn3990 stability

[Jeffrey Hugo]

On Fri, Oct 18, 2019 at 12:03 PM Matthias Kaehlcke <mka@xxxxxxxxxxxx> wrote:
>
> On Thu, Oct 17, 2019 at 02:29:55PM -0700, Jeffrey Hugo wrote:
> > On the msm8998 mtp, the response to the baudrate change command is never
> > received.  On the Lenovo Miix 630, the response to the baudrate change
> > command is corrupted - "Frame reassembly failed (-84)".
> >
> > Adding a 50ms delay before re-enabling flow to receive the baudrate change
> > command response from the wcn3990 addesses both issues, and allows
> > bluetooth to become functional.
>
> From my earlier debugging on sdm845 I don't think this is what happens.
> The problem is that the wcn3990 sends the response to the baudrate change
> command using the new baudrate, while the UART on the SoC still operates
> with the prior speed (for details see 2faa3f15fa2f ("Bluetooth: hci_qca:
> wcn3990: Drop baudrate change vendor event"))
>
> IIRC the 50ms delay causes the HCI core to discard the received data,
> which is why the "Frame reassembly failed" message disappears, not
> because the response was received. In theory commit 78e8fa2972e5
> ("Bluetooth: hci_qca: Deassert RTS while baudrate change command")
> should have fixed those messages, do you know if CTS/RTS are connected
> on the Bluetooth UART of the Lenovo Miix 630?

I was testing with 5.4-rc1 which contains the indicated RTS fix.

Yes, CTS/RTS are connected on the Lenovo Miix 630.

I added debug statements which indicated that data was received,
however it was corrupt, and the packet type did not match what was
expected, hence the frame reassembly errors.

In response to this patch, Balakrishna pointed me to a bug report
which indicated that some of the UART GPIO lines need to have a bias
applied to prevent errant data from floating lines -

https://chromium-review.googlesource.com/c/chromiumos/third_party/kernel/+/1391888

It turns out this fix was never applied to msm8998.  Applying the fix
does cause the the frame reassembly errors to go away, however then
the host SoC never receives the baud rate change response (I increased
the timeout from 2faa3f15fa2f ("Bluetooth: hci_qca: wcn3990: Drop
baudrate change vendor event") to 5 seconds).  As of now, this patch
is still required.

I have no idea why the delay is required, and was hoping that posting
this patch would result in someone else providing some missing pieces
to determine the real root cause.  I suspect that asserting RTS at the
wrong time may cause an issue for the wcn3990, but I have no data nor
documentation to support this guess.  I welcome any further insights
you may have.