Re: [PATCH RESEND 2/2] Bluetooth: fix use-bdaddr-property quirk

[Amit Pundir]

On Fri, 7 Jul 2023 at 16:37, Johan Hovold <johan@xxxxxxxxxx> wrote:
>
> On Fri, Jul 07, 2023 at 03:11:11PM +0530, Amit Pundir wrote:
>
> > On Wed, 31 May 2023 at 14:35, Johan Hovold <johan+linaro@xxxxxxxxxx> wrote:
> > >
> > > Devices that lack persistent storage for the device address can indicate
> > > this by setting the HCI_QUIRK_INVALID_BDADDR which causes the controller
> > > to be marked as unconfigured until user space has set a valid address.
> > >
> > > The related HCI_QUIRK_USE_BDADDR_PROPERTY was later added to similarly
> > > indicate that the device lacks a valid address but that one may be
> > > specified in the devicetree.
> > >
> > > As is clear from commit 7a0e5b15ca45 ("Bluetooth: Add quirk for reading
> > > BD_ADDR from fwnode property") that added and documented this quirk and
> > > commits like de79a9df1692 ("Bluetooth: btqcomsmd: use
> > > HCI_QUIRK_USE_BDADDR_PROPERTY"), the device address of controllers with
> > > this flag should be treated as invalid until user space has had a chance
> > > to configure the controller in case the devicetree property is missing.
> > >
> > > As it does not make sense to allow controllers with invalid addresses,
> > > restore the original semantics, which also makes sure that the
> > > implementation is consistent (e.g. get_missing_options() indicates that
> > > the address must be set) and matches the documentation (including
> > > comments in the code, such as, "In case any of them is set, the
> > > controller has to start up as unconfigured.").
>
> > This patch broke Bluetooth on Dragonboard 845c (SDM845) devboard.
> > Reverting this patch fixes the BT breakage and I see the following
> > messages in dmesg:
> >
> > Bluetooth: hci0: setting up wcn399x
> > Bluetooth: hci0: QCA Product ID   :0x0000000a
> > Bluetooth: hci0: QCA SOC Version  :0x40010214
> > Bluetooth: hci0: QCA ROM Version  :0x00000201
> > Bluetooth: hci0: QCA Patch Version:0x00000001
> > Bluetooth: hci0: QCA controller version 0x02140201
> > Bluetooth: hci0: QCA Downloading qca/crbtfw21.tlv
> > Bluetooth: hci0: QCA Downloading qca/crnv21.bin
> > Bluetooth: hci0: QCA setup on UART is completed
>
> That's odd. You should still see the above messages also with this patch
> applied, but you may now need to provide a valid device address before
> being able to use device in case the bootloader has not provided one
> (e.g. using btmgmt).

Sorry for the confusion, I missed the part where I do see these
messages when the kernel module is loaded but the direct firmware
loading fails.

Bluetooth: hci0: setting up wcn399x
Bluetooth: hci0: QCA Product ID   :0x0000000a
Bluetooth: hci0: QCA SOC Version  :0x40010214
Bluetooth: hci0: QCA ROM Version  :0x00000201
Bluetooth: hci0: QCA Patch Version:0x00000001
Bluetooth: hci0: QCA controller version 0x02140201
Bluetooth: hci0: QCA Downloading qca/crbtfw21.tlv
bluetooth hci0: Direct firmware load for qca/crbtfw21.tlv failed with error -2
Bluetooth: hci0: QCA Failed to request file: qca/crbtfw21.tlv (-2)
Bluetooth: hci0: QCA Failed to download patch (-2)
Bluetooth: hci0: QCA preshutdown_cmd failed (-56)

This happens in all the cases (working and non-working BT) because
filesystem is not mounted by that time. I'm running AOSP and all the
kernel modules get loaded from a ramdisk. But in the working case, the
firmware loading kicks in again later in the boot process and BT gets
initiazed..

With this patch, after the first attempt to load the firmware fails,
the firmware loading doesn't kick-in again. Also even if I keep the
firmware in ramdisk then the direct firmware loading from ramdisk
happens but BT still doesn't work
https://bugs.linaro.org/attachment.cgi?id=1148.

>
> Are there any error messages in the log when running with this patch?

I don't see any relevant error message in dmesg. I'll check if I can
find a command line BT debug tool which I can use on AOSP for
debugging. There used to be a few hci command line tools, when I
looked into it a few years ago. Not sure if they are still around and
useful.

Regards,
Amit Pundir

>
> Does
>
>         btmgmt --index 0 public-addr <bdaddr>
>
> work?
>
> Johan