Re: [PATCH v5 2/4] Bluetooth: hci_qca: Add support for QTI Bluetooth chip wcn6855

[Johan Hovold]

On Sun, Mar 12, 2023 at 10:18:48PM -0500, Steev Klimaszewski wrote:
> Hi Johan,
> 
> <SNIP>
> > > > As I mentioned elsewhere, you need to update also this function so that
> > > > wcn6855 can be powered down.
> > >
> > > Sorry, I do have that locally, I just haven't pushed a v6 as I was
> > > looking at Tim's v2 of the qca2066 and was wondering if I should or
> > > shouldn't continue working on my version of the driver?
> >
> > I only skimmed that patch a while ago, but that ones not strictly needed
> > for wcn6855, right? Things seems to work well here with just this series
> > applied.
> 
> Works, but, not quite well, and with the nvm bits from Tim's patch, we
> end up getting closer?  I think that is the best way to put it.  With
> what we currently have, we end up loading hpnv21.bin for our nvm patch
> file, however, we actually want (at least on my Thinkpad X13s) the
> .b8c file from the Windows partition for our nvm patch; With the b8c
> file symlinked to .bin with just my patch set, I am able to connect a
> pair of Air Pods Gen1 to the ThinkPad and play back audio, as well as
> use them for input.  With the .bin file that comes from
> linux-firmware, they will still connect, however, they will randomly
> disconnect, as well as the audio output is all garbled.

Hmm. Ok, but then we need to ask Lenovo and Qualcomm to release the
firmware files we need for the X13s. Until then using your patch and
"hpnv21.bin" at least works to some extent.

I could connect to one bluetooth speaker without noticing any problems,
but I did indeed get some garbled output when connecting to another. I
have not tried the .b8c file yet though, so this could possibly be some
other incompatibility issue.

> I think,
> ideally, we get v6+ in, and then we can figure out what to do about
> the bits that Tim's patch adds.  I've tried them locally, but I'm not
> confident enough in my knowledge to address the issues that are
> brought up in the code review there.

Yes, that seems reasonable. Your patch is more complete in that it adds
supports for managing power. Adding support for more fine grained
loading of "NVM configuration" files could be done on top.

> > > > With power-off handling fixed, this seems to work as quite well on my
> > > > X13s with 6.3-rc1. Nice job!
> > > >
> > > > Btw, apart from the frame reassembly error, I'm also seeing:
> > > >
> > > >         Bluetooth: Received HCI_IBS_WAKE_ACK in tx state 0
> > > >
> > > > during probe.
> > > >
> > > I'm still not sure where the frame reassembly error comes from, and I
> > > don't know how to get more info to figure it out either, if anyone
> > > happens to have any guidance for that, I would love some.
> > > Additionally, it doesn't always happen.  It seems to happen on the
> > > first load of the module, however, running modprobe -r && modprobe in
> > > a loop (with the powerdown properly modified so the log isn't full of
> > > splats),  it doesn't seem to occur every time. Likewise for the
> > > WAKE_ACK.
> >
> > Ok. Looks like the Chromium team tried to suppress these errors when
> > switching line speed by toggling rts, but the frame-assembly error I get
> > appears to happen before that.
> 
> I am still trying to figure it out here as well, but I want to get v6
> out there.

Yeah, I don't think that message during probe should be a show stopper
here.

Johan