Bluetooth: fix bdaddr quirks
Subsystems (3)
cover Bluetooth: fix bdaddr quirks
These patches fix a couple of issues with the two bdaddr quirks:
The first one allows HCI_QUIRK_INVALID_BDADDR to be used with HCI_QUIRK_NON_PERSISTENT_SETUP.
The second patch restores the original semantics of the HCI_QUIRK_USE_BDADDR_PROPERTY so that the controller is marked as unconfigured when no device address is specified in the devicetree (as the quirk is documented to work).
This specifically makes sure that Qualcomm HCI controllers such as wcn6855 found on the Lenovo X13s are marked as unconfigured until user space has provided a valid address.
Long term, the HCI_QUIRK_USE_BDADDR_PROPERTY should probably be dropped in favour of HCI_QUIRK_INVALID_BDADDR and always checking the devicetree property.
Johan
Johan Hovold (2):
Bluetooth: fix invalid-bdaddr quirk for non-persistent setup
Bluetooth: fix use-bdaddr-property quirk
net/bluetooth/hci_sync.c | 30 +++++++++++------------------- 1 file changed, 11 insertions(+), 19 deletions(-)
Signature · 1 line
2.39.3
PATCH RESEND 1/2 Bluetooth: fix invalid-bdaddr quirk for non-persistent setup
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.
Once configured, the device address must be set on every setup for controllers with HCI_QUIRK_NON_PERSISTENT_SETUP to avoid marking the controller as unconfigured and requiring the address to be set again.
--- net/bluetooth/hci_sync.c | 28 +++++++++++----------------- 1 file changed, 11 insertions(+), 17 deletions(-)
net/bluetooth/hci_sync.c +11 -17
@@ -4618,23 +4618,17 @@ static int hci_dev_setup_sync(struct hci_dev *hdev) invalid_bdaddr =test_bit (HCI_QUIRK_INVALID_BDADDR , &hdev->quirks);if (!ret) { -if (test_bit (HCI_QUIRK_USE_BDADDR_PROPERTY , &hdev->quirks)) { -if (!bacmp (&hdev->public_addr,BDADDR_ANY )) -hci_dev_get_bd_addr_from_property (hdev); - -if (bacmp (&hdev->public_addr,BDADDR_ANY ) && - hdev->set_bdaddr) { - ret = hdev->set_bdaddr (hdev, - &hdev->public_addr); - -/* If setting of the BD_ADDR from the device -* property succeeds, then treat the address -* as valid even if the invalid BD_ADDR -* quirk indicates otherwise. -*/ -if (!ret) - invalid_bdaddr = false; - } +if (test_bit (HCI_QUIRK_USE_BDADDR_PROPERTY , &hdev->quirks) && + !bacmp (&hdev->public_addr,BDADDR_ANY )) +hci_dev_get_bd_addr_from_property (hdev); + +if ((invalid_bdaddr || +test_bit (HCI_QUIRK_USE_BDADDR_PROPERTY , &hdev->quirks)) && +bacmp (&hdev->public_addr,BDADDR_ANY ) && + hdev->set_bdaddr) { + ret = hdev->set_bdaddr (hdev, &hdev->public_addr); +if (!ret) + invalid_bdaddr = false; } }
Signature · 1 line
2.39.3
This is automated email and please do not reply to this email!
Dear submitter,
Thank you for submitting the patches to the linux bluetooth mailing list. This is a CI test results with your patch series:
PW Link:https://patchwork.kernel.org/project/bluetooth/list/?series=752646
---Test result---
Test Summary:
CheckPatch PASS 1.31 seconds GitLint PASS 0.62 seconds SubjectPrefix PASS 0.20 seconds BuildKernel PASS 34.55 seconds CheckAllWarning PASS 37.25 seconds CheckSparse PASS 42.93 seconds CheckSmatch PASS 114.27 seconds BuildKernel32 PASS 33.57 seconds TestRunnerSetup PASS 471.26 seconds TestRunner_l2cap-tester PASS 17.27 seconds TestRunner_iso-tester FAIL 24.23 seconds TestRunner_bnep-tester PASS 5.78 seconds TestRunner_mgmt-tester PASS 117.97 seconds TestRunner_rfcomm-tester PASS 9.13 seconds TestRunner_sco-tester PASS 8.52 seconds TestRunner_ioctl-tester PASS 9.87 seconds TestRunner_mesh-tester PASS 7.24 seconds TestRunner_smp-tester PASS 8.32 seconds TestRunner_userchan-tester PASS 6.05 seconds IncrementalBuild PASS 36.67 seconds
Details
##############################
Test: TestRunner_iso-tester - FAIL
Desc: Run iso-tester with test-runner Output:
Total: 80, Passed: 75 (93.8%), Failed: 5, Not Run: 0
Failed Test Cases
ISO AC 6(i) - Success Failed 0.239 seconds ISO AC 7(i) - Success Failed 0.237 seconds ISO AC 8(i) - Success Failed 0.238 seconds ISO AC 9(i) - Success Failed 0.243 seconds ISO AC 11(i) - Success Failed 0.238 seconds
---
Regards,
Linux Bluetooth
PATCH RESEND 2/2 Bluetooth: fix use-bdaddr-property quirk
In reply to Johan HovoldDevices 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.").
--- net/bluetooth/hci_sync.c | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-)
net/bluetooth/hci_sync.c +3 -5
@@ -4615,16 +4615,14 @@ static int hci_dev_setup_sync(struct hci_dev *hdev)* BD_ADDR invalid before creating the HCI device or in * its setup callback. */ - invalid_bdaddr =test_bit (HCI_QUIRK_INVALID_BDADDR , &hdev->quirks); - + invalid_bdaddr =test_bit (HCI_QUIRK_INVALID_BDADDR , &hdev->quirks) || +test_bit (HCI_QUIRK_USE_BDADDR_PROPERTY , &hdev->quirks);if (!ret) {if (test_bit (HCI_QUIRK_USE_BDADDR_PROPERTY , &hdev->quirks) && !bacmp (&hdev->public_addr,BDADDR_ANY ))hci_dev_get_bd_addr_from_property (hdev); -if ((invalid_bdaddr || -test_bit (HCI_QUIRK_USE_BDADDR_PROPERTY , &hdev->quirks)) && -bacmp (&hdev->public_addr,BDADDR_ANY ) && +if (invalid_bdaddr &&bacmp (&hdev->public_addr,BDADDR_ANY ) && hdev->set_bdaddr) { ret = hdev->set_bdaddr (hdev, &hdev->public_addr);if (!ret)
Signature · 1 line
2.39.3
Re: [PATCH RESEND 2/2] Bluetooth: fix use-bdaddr-property quirk
Quoted 22 lines · Johan Hovold
Devices that lack persistent storage for the device address can indicate this by setting the
HCI_QUIRK_INVALID_BDADDRwhich causes the controller to be marked as unconfigured until user space has set a valid address.The related
HCI_QUIRK_USE_BDADDR_PROPERTYwas 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 readingBD_ADDRfrom fwnode property") that added and documented this quirk and commits likede79a9df1692("Bluetooth: btqcomsmd: useHCI_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.").
Quoted 2 lines
Fixes e668eb1e1578 ("Bluetooth: hci_core: Don't stop BT if the BD address missing in dts")Signed-off-by Johan Hovold <[email protected]>
This patch has been recently merged to linux-next as commit 6ac517d8cf8b ("Bluetooth: fix use-bdaddr-property quirk"). Unfortunately it breaks bluetooth operation on my Raspberry Pi 3b+, 4b+ and Khadas VIM3 based test systems.
Before this patch on Raspberry Pi 4b+:
root@target:~# dmesg | grep hci0 [ 14.459292] Bluetooth: hci0: BCM: chip id 107 [ 14.464283] Bluetooth: hci0: BCM: features 0x2f [ 14.470632] Bluetooth: hci0: BCM4345C0 [ 14.474483] Bluetooth: hci0: BCM4345C0 (003.001.025) build 0000 [ 14.487275] Bluetooth: hci0: BCM4345C0 'brcm/BCM4345C0.hcd' Patch [ 15.347542] Bluetooth: hci0: BCM: features 0x2f [ 15.354588] Bluetooth: hci0: BCM43455 37.4MHz Raspberry Pi 3+-0159 [ 15.361076] Bluetooth: hci0: BCM4345C0 (003.001.025) build 0290 root@target:~# hcitool dev Devices: hci0 DC:A6:32:12:38:D1 root@target:~# root@target:~# hcitool scan Scanning ... 88:57:1D:AB:19:B2 Samsung Family Hub root@target:~# hcitool | head -n1 hcitool - HCI Tool ver 5.50 root@target:~#
After this patch:
root@target:~# dmesg | grep hci0 [ 13.979860] Bluetooth: hci0: BCM: chip id 107 [ 13.984969] Bluetooth: hci0: BCM: features 0x2f [ 13.991444] Bluetooth: hci0: BCM4345C0 [ 13.995300] Bluetooth: hci0: BCM4345C0 (003.001.025) build 0000 [ 14.005131] Bluetooth: hci0: BCM4345C0 'brcm/BCM4345C0.hcd' Patch [ 14.839465] Bluetooth: hci0: BCM: features 0x2f [ 14.846047] Bluetooth: hci0: BCM43455 37.4MHz Raspberry Pi 3+-0159 [ 14.859859] Bluetooth: hci0: BCM4345C0 (003.001.025) build 0290 root@target:~# hcitool dev Devices: root@target:~# hcitool scan Device is not available: No such device root@target:~# hcitool | head -n1 hcitool - HCI Tool ver 5.50 root@target:~#
Reverting $subject on top of linux-next fixes this 'issue'.
Let me know if you need more information about my test systems or to make some other tests.
Quoted 1 line
...
Best regards
Signature · 2 lines
Marek Szyprowski, PhD Samsung R&D Institute Poland
Luiz Augusto von Dentz
maintainerRe: [PATCH RESEND 2/2] Bluetooth: fix use-bdaddr-property quirk
Hi Johan,
Quoted 74 lines · Marek Szyprowski
Quoted 22 lines · Johan Hovold
Devices that lack persistent storage for the device address can indicate this by setting the
HCI_QUIRK_INVALID_BDADDRwhich causes the controller to be marked as unconfigured until user space has set a valid address.The related
HCI_QUIRK_USE_BDADDR_PROPERTYwas 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 readingBD_ADDRfrom fwnode property") that added and documented this quirk and commits likede79a9df1692("Bluetooth: btqcomsmd: useHCI_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.").Quoted 2 lines
Fixes e668eb1e1578 ("Bluetooth: hci_core: Don't stop BT if the BD address missing in dts")Signed-off-by Johan Hovold <[email protected]>This patch has been recently merged to linux-next as commit
6ac517d8cf8b("Bluetooth: fix use-bdaddr-property quirk"). Unfortunately it breaks bluetooth operation on my Raspberry Pi 3b+, 4b+ and Khadas VIM3 based test systems.Before this patch on Raspberry Pi 4b+:
root@target:~# dmesg | grep hci0 [ 14.459292] Bluetooth: hci0: BCM: chip id 107 [ 14.464283] Bluetooth: hci0: BCM: features 0x2f [ 14.470632] Bluetooth: hci0: BCM4345C0 [ 14.474483] Bluetooth: hci0: BCM4345C0 (003.001.025) build 0000 [ 14.487275] Bluetooth: hci0: BCM4345C0 'brcm/BCM4345C0.hcd' Patch [ 15.347542] Bluetooth: hci0: BCM: features 0x2f [ 15.354588] Bluetooth: hci0: BCM43455 37.4MHz Raspberry Pi 3+-0159 [ 15.361076] Bluetooth: hci0: BCM4345C0 (003.001.025) build 0290 root@target:~# hcitool dev Devices: hci0 DC:A6:32:12:38:D1 root@target:~# root@target:~# hcitool scan Scanning ... 88:57:1D:AB:19:B2 Samsung Family Hub root@target:~# hcitool | head -n1 hcitool - HCI Tool ver 5.50 root@target:~#After this patch:
root@target:~# dmesg | grep hci0 [ 13.979860] Bluetooth: hci0: BCM: chip id 107 [ 13.984969] Bluetooth: hci0: BCM: features 0x2f [ 13.991444] Bluetooth: hci0: BCM4345C0 [ 13.995300] Bluetooth: hci0: BCM4345C0 (003.001.025) build 0000 [ 14.005131] Bluetooth: hci0: BCM4345C0 'brcm/BCM4345C0.hcd' Patch [ 14.839465] Bluetooth: hci0: BCM: features 0x2f [ 14.846047] Bluetooth: hci0: BCM43455 37.4MHz Raspberry Pi 3+-0159 [ 14.859859] Bluetooth: hci0: BCM4345C0 (003.001.025) build 0290 root@target:~# hcitool dev Devices: root@target:~# hcitool scan Device is not available: No such device root@target:~# hcitool | head -n1 hcitool - HCI Tool ver 5.50 root@target:~#
Quoted 4 lines
Reverting $subject on top of linux-next fixes this 'issue'.
Let me know if you need more information about my test systems or to make some other tests.
Can you give it a look, looks like different manufacturers have different expectations, anyway we should probably figure out a way to get these controllers working otherwise we will need to revert.
Signature · 1 line
Luiz Augusto von Dentz
Re: [PATCH RESEND 2/2] Bluetooth: fix use-bdaddr-property quirk
In reply to Marek SzyprowskiQuoted 33 lines · Marek Szyprowski
Quoted 22 lines · Johan Hovold
Devices that lack persistent storage for the device address can indicate this by setting the
HCI_QUIRK_INVALID_BDADDRwhich causes the controller to be marked as unconfigured until user space has set a valid address.The related
HCI_QUIRK_USE_BDADDR_PROPERTYwas 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 readingBD_ADDRfrom fwnode property") that added and documented this quirk and commits likede79a9df1692("Bluetooth: btqcomsmd: useHCI_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.").Quoted 2 lines
Fixes e668eb1e1578 ("Bluetooth: hci_core: Don't stop BT if the BD address missing in dts")Signed-off-by Johan Hovold <[email protected]>This patch has been recently merged to linux-next as commit
6ac517d8cf8b("Bluetooth: fix use-bdaddr-property quirk"). Unfortunately it breaks bluetooth operation on my Raspberry Pi 3b+, 4b+ and Khadas VIM3 based test systems.Before this patch on Raspberry Pi 4b+:
Quoted 12 lines
root@target:~# dmesg | grep hci0 [ 14.459292] Bluetooth: hci0: BCM: chip id 107 [ 14.464283] Bluetooth: hci0: BCM: features 0x2f [ 14.470632] Bluetooth: hci0: BCM4345C0 [ 14.474483] Bluetooth: hci0: BCM4345C0 (003.001.025) build 0000 [ 14.487275] Bluetooth: hci0: BCM4345C0 'brcm/BCM4345C0.hcd' Patch [ 15.347542] Bluetooth: hci0: BCM: features 0x2f [ 15.354588] Bluetooth: hci0: BCM43455 37.4MHz Raspberry Pi 3+-0159 [ 15.361076] Bluetooth: hci0: BCM4345C0 (003.001.025) build 0290 root@target:~# hcitool dev Devices: hci0 DC:A6:32:12:38:D1
Thanks for the report and sorry about the breakage.
Turns out that the Broadcom driver has so far been setting the HCI_QUIRK_USE_BDADDR_PROPERTY also for devices such as yours which already have a valid address.
I've sent a fix for the Broadcom driver here:
https://lore.kernel.org/lkml/[email protected]
which should address this. Could you give it a try?
Johan
Re: [PATCH RESEND 2/2] Bluetooth: fix use-bdaddr-property quirk
In reply to Johan HovoldHi Johan,
Quoted 16 lines · Johan Hovold
Devices that lack persistent storage for the device address can indicate this by setting the
HCI_QUIRK_INVALID_BDADDRwhich causes the controller to be marked as unconfigured until user space has set a valid address.The related
HCI_QUIRK_USE_BDADDR_PROPERTYwas 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 readingBD_ADDRfrom fwnode property") that added and documented this quirk and commits likede79a9df1692("Bluetooth: btqcomsmd: useHCI_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.
Quoted 7 lines
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
Regards,
Amit Pundir
Quoted 33 lines
Fixes e668eb1e1578 ("Bluetooth: hci_core: Don't stop BT if the BD address missing in dts")Signed-off-by Johan Hovold <[email protected]>--- net/bluetooth/hci_sync.c | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-)net/bluetooth/hci_sync.c +3
-6@@ -4615,16 +4615,14 @@ static int hci_dev_setup_sync(struct hci_dev *hdev)* BD_ADDR invalid before creating the HCI device or in * its setup callback. */ - invalid_bdaddr =test_bit (HCI_QUIRK_INVALID_BDADDR , &hdev->quirks); - + invalid_bdaddr =test_bit (HCI_QUIRK_INVALID_BDADDR , &hdev->quirks) || +test_bit (HCI_QUIRK_USE_BDADDR_PROPERTY , &hdev->quirks);if (!ret) {if (test_bit (HCI_QUIRK_USE_BDADDR_PROPERTY , &hdev->quirks) && !bacmp (&hdev->public_addr,BDADDR_ANY ))hci_dev_get_bd_addr_from_property (hdev); -if ((invalid_bdaddr || -test_bit (HCI_QUIRK_USE_BDADDR_PROPERTY , &hdev->quirks)) && -bacmp (&hdev->public_addr,BDADDR_ANY ) && +if (invalid_bdaddr &&bacmp (&hdev->public_addr,BDADDR_ANY ) && hdev->set_bdaddr) { ret = hdev->set_bdaddr (hdev, &hdev->public_addr);if (!ret) --2.39.3
Re: [PATCH RESEND 2/2] Bluetooth: fix use-bdaddr-property quirk
Quoted 28 lines · Amit Pundir
Quoted 16 lines · Johan Hovold
Devices that lack persistent storage for the device address can indicate this by setting the
HCI_QUIRK_INVALID_BDADDRwhich causes the controller to be marked as unconfigured until user space has set a valid address.The related
HCI_QUIRK_USE_BDADDR_PROPERTYwas 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 readingBD_ADDRfrom fwnode property") that added and documented this quirk and commits likede79a9df1692("Bluetooth: btqcomsmd: useHCI_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.Quoted 6 lines
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:
Quoted 9 lines
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 version0x02140201
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).
Are there any error messages in the log when running with this patch?
Does
btmgmt --index 0 public-addr <bdaddr>
work?
Johan
Re: [PATCH RESEND 2/2] Bluetooth: fix use-bdaddr-property quirk
Quoted 41 lines · Johan Hovold
Quoted 28 lines · Amit Pundir
Quoted 16 lines · Johan Hovold
Devices that lack persistent storage for the device address can indicate this by setting the
HCI_QUIRK_INVALID_BDADDRwhich causes the controller to be marked as unconfigured until user space has set a valid address.The related
HCI_QUIRK_USE_BDADDR_PROPERTYwas 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 readingBD_ADDRfrom fwnode property") that added and documented this quirk and commits likede79a9df1692("Bluetooth: btqcomsmd: useHCI_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.Quoted 6 lines
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:
Quoted 9 lines
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 version0x02140201
Bluetooth: hci0: QCA Downloading qca/crbtfw21.tlv
Bluetooth: hci0: QCA Downloading qca/crnv21.bin
Bluetooth: hci0: QCA setup on UART is completed
Quoted 4 lines
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.
Quoted 2 lines
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
Quoted 8 lines
Does
btmgmt --index 0 public-addr <bdaddr>work?
Johan
Re: [PATCH RESEND 2/2] Bluetooth: fix use-bdaddr-property quirk
Quoted 29 lines · Amit Pundir
Quoted 4 lines · Johan Hovold
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 version0x02140201
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: QCApreshutdown_cmdfailed (-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..
Quoted 5 lines
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.
So everything appears to work as intended. First, the firmware needs to be in your initramfs if you want to avoid that initial fw load failure.
And after that you need to provide a valid device address as these devices ship without one.
Once you set the address, the firmware should be loaded if it hasn't been already.
Quoted 7 lines
Quoted 1 line
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.
Yeah, I'm not sure how you set the device address with the Android stack, but there must be some way as there are other bluetooth controllers out there which similarly need a valid address before they can be used.
Johan
Re: [PATCH RESEND 2/2] Bluetooth: fix use-bdaddr-property quirk
Quoted 37 lines · Johan Hovold
Quoted 29 lines · Amit Pundir
Quoted 4 lines · Johan Hovold
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 version0x02140201
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: QCApreshutdown_cmdfailed (-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..
Quoted 5 lines
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.
Quoted 8 lines
So everything appears to work as intended. First, the firmware needs to be in your initramfs if you want to avoid that initial fw load failure.
And after that you need to provide a valid device address as these devices ship without one.
Once you set the address, the firmware should be loaded if it hasn't been already.
Thanks a lot for explanation Johan. I just booted up Debian on DB845c and btmgmt works as you pointed out above.
root@linaro-gnome:~#
root@linaro-gnome:~# uname -a Linux linaro-gnome 6.5.0-rc1 #1 SMP PREEMPT Mon Jul 10 15:42:50 IST 2023 aarch64 GNU/Linux root@linaro-gnome:~# root@linaro-gnome:~# btmgmt public-addr 01:02:03:04:05:06 [ 165.708146] Bluetooth: MGMT ver 1.22 hci0 Set Public Address complete, options: [ 165.715275] Bluetooth: hci0: setting up wcn399x root@linaro-gnome:~# [ 165.868755] Bluetooth: hci0: QCA Product ID :0x0000000a [ 165.874272] Bluetooth: hci0: QCA SOC Version :0x40010214 [ 165.879758] Bluetooth: hci0: QCA ROM Version :0x00000201 [ 165.885226] Bluetooth: hci0: QCA Patch Version:0x00000001 [ 165.903506] Bluetooth: hci0: QCA controller version 0x02140201 [ 165.909424] Bluetooth: hci0: QCA Downloading qca/crbtfw21.tlv [ 166.794992] Bluetooth: hci0: QCA Downloading qca/crnv21.bin [ 166.870882] Bluetooth: hci0: QCA setup on UART is completed
Quoted 9 lines
Quoted 8 lines
Quoted 2 lines
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.
Quoted 5 lines
Yeah, I'm not sure how you set the device address with the Android stack, but there must be some way as there are other bluetooth controllers out there which similarly need a valid address before they can be used.
I'll look if I can reuse/simplify "btmgmt public-addr" command on Android or find an equivalent tool to do that.
Regards,
Amit Pundir
Re: [PATCH RESEND 2/2] Bluetooth: fix use-bdaddr-property quirk
Quoted 9 lines · Amit Pundir
Quoted 8 lines · Johan Hovold
Quoted 7 lines · Amit Pundir
Quoted 1 line · Johan Hovold
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.
Quoted 6 lines
Quoted 4 lines
Yeah, I'm not sure how you set the device address with the Android stack, but there must be some way as there are other bluetooth controllers out there which similarly need a valid address before they can be used.
I'll look if I can reuse/simplify "btmgmt public-addr" command on Android or find an equivalent tool to do that.
Please correct me if I'm wrong: the avove to me sounds like you are happy with this approach, even if this is kind of a regression; but likely one that is rare and thus not worth making a fuzz about. In that case I'll remove it from the regression tracking:
#regzbot resolve: minor issue, workaround found
#regzbot ignore-activity
Ciao, Thorsten (wearing his 'the Linux kernel's regression tracker' hat) --
Everything you wanna know about Linux kernel regression tracking:
https://linux-regtracking.leemhuis.info/about/#tldr
If I did something stupid, please tell me, as explained on that page.
Re: [PATCH RESEND 2/2] Bluetooth: fix use-bdaddr-property quirk
Quoted 15 lines · Linux regression tracking (Thorsten Leemhuis)
Quoted 13 lines · Amit Pundir
Quoted 12 lines · Johan Hovold
Quoted 7 lines · Amit Pundir
Quoted 1 line · Johan Hovold
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.
Yeah, I'm not sure how you set the device address with the Android stack, but there must be some way as there are other bluetooth controllers out there which similarly need a valid address before they can be used.
Quoted 7 lines
Quoted 2 lines
I'll look if I can reuse/simplify "btmgmt public-addr" command on Android or find an equivalent tool to do that.
Please correct me if I'm wrong: the avove to me sounds like you are happy with this approach, even if this is kind of a regression; but likely one that is rare and thus not worth making a fuzz about. In that case I'll remove it from the regression tracking:
Hi. Thanks to Stephan, this had been taken care of from the userspace
https://android.googlesource.com/device/linaro/dragonboard/+/f70f12a826af
So please remove it from the kernel regression tracking.
Regards,
Amit Pundir
Quoted 9 lines
#regzbot resolve: minor issue, workaround found
#regzbot ignore-activityCiao, Thorsten (wearing his 'the Linux kernel's regression tracker' hat) --
Everything you wanna know about Linux kernel regression tracking:
https://linux-regtracking.leemhuis.info/about/#tldr
If I did something stupid, please tell me, as explained on that page.
Re: [PATCH RESEND 2/2] Bluetooth: fix use-bdaddr-property quirk
In reply to Amit Pundir[TLDR: I'm adding this report to the list of tracked Linux kernel regressions; the text you find below is based on a few templates paragraphs you might have encountered already in similar form. See link in footer if these mails annoy you.]
Quoted 31 lines · Amit Pundir
Hi Johan,
Quoted 16 lines · Johan Hovold
Devices that lack persistent storage for the device address can indicate this by setting the
HCI_QUIRK_INVALID_BDADDRwhich causes the controller to be marked as unconfigured until user space has set a valid address.The related
HCI_QUIRK_USE_BDADDR_PROPERTYwas 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 readingBD_ADDRfrom fwnode property") that added and documented this quirk and commits likede79a9df1692("Bluetooth: btqcomsmd: useHCI_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.Quoted 7 lines
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:
Quoted 9 lines
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 version0x02140201
Bluetooth: hci0: QCA Downloading qca/crbtfw21.tlv
Bluetooth: hci0: QCA Downloading qca/crnv21.bin
Bluetooth: hci0: QCA setup on UART is completed
Thanks for the report. To be sure the issue doesn't fall through the cracks unnoticed, I'm adding it to regzbot, the Linux kernel regression tracking bot:
#regzbot ^introduced 6945795bc81
#regzbot title Bluetooth: Dragonboard 845c (SDM845) devboard broken #regzbot ignore-activity
This isn't a regression? This issue or a fix for it are already discussed somewhere else? It was fixed already? You want to clarify when the regression started to happen? Or point out I got the title or something else totally wrong? Then just reply and tell me -- ideally while also telling regzbot about it, as explained by the page listed in the footer of this mail.
Developers: When fixing the issue, remember to add 'Link:' tags pointing to the report (the parent of this mail). See page linked in footer for details.
Ciao, Thorsten (wearing his 'the Linux kernel's regression tracker' hat) --
Everything you wanna know about Linux kernel regression tracking:
https://linux-regtracking.leemhuis.info/about/#tldr
That page also explains what to do if mails like this annoy you.