Bluetooth: hci_sync: Add a new quirk to skip HCI_FLT_CLEAR_ALL
Subsystems (2)
PATCH v4 1/2 Bluetooth: hci_sync: Add a new quirk to skip HCI_FLT_CLEAR_ALL
Some controllers have problems with being sent a command to clear all filtering. While the HCI code does not unconditionally send a clear-all anymore at BR/EDR setup (after the state machine refactor), there might be more ways of hitting these codepaths in the future as the kernel develops.
--- include/net/bluetooth/hci.h | 10 ++++++++++ net/bluetooth/hci_sync.c | 16 ++++++++++++++++ 2 files changed, 26 insertions(+)
include/net/bluetooth/hci.h +10 -0
@@ -255,6 +255,16 @@ enum {* during the hdev->setup vendor callback. */ HCI_QUIRK_BROKEN_READ_TRANSMIT_POWER , + +/* When this quirk is set, HCI_OP_SET_EVENT_FLT requests with +* HCI_FLT_CLEAR_ALL are ignored and event filtering is +* completely avoided. A subset of the CSR controller +* clones struggle with this and instantly lock up. +* +* Note that devices using this must (separately) disable +* runtime suspend, because event filtering takes place there. +*/ + HCI_QUIRK_BROKEN_FILTER_CLEAR_ALL, };/* HCI device flags */
net/bluetooth/hci_sync.c +16 -0
@@ -2812,6 +2812,9 @@ static int hci_set_event_filter_sync(struct hci_dev *hdev, u8 flt_type,if (!hci_dev_test_flag (hdev,HCI_BREDR_ENABLED ))return 0 ; +if (test_bit (HCI_QUIRK_BROKEN_FILTER_CLEAR_ALL , &hdev->quirks)) +return 0 ; +memset (&cp,0 ,sizeof (cp)); cp.flt_type = flt_type;
@@ -2832,6 +2835,13 @@ static int hci_clear_event_filter_sync(struct hci_dev *hdev)if (!hci_dev_test_flag (hdev,HCI_EVENT_FILTER_CONFIGURED ))return 0 ; +/* In theory the state machine should not reach here unless +* a hci_set_event_filter_sync() call succeeds, but we do +* the check both for parity and as a future reminder. +*/ +if (test_bit (HCI_QUIRK_BROKEN_FILTER_CLEAR_ALL , &hdev->quirks)) +return 0 ; +return hci_set_event_filter_sync (hdev,HCI_FLT_CLEAR_ALL ,0x00 ,BDADDR_ANY ,0x00 ); }
@@ -4831,6 +4841,12 @@ static int hci_update_event_filter_sync(struct hci_dev *hdev)if (!hci_dev_test_flag (hdev,HCI_BREDR_ENABLED ))return 0 ; +/* Some fake CSR controllers lock up after setting this type of +* filter, so avoid sending the request altogether. +*/ +if (test_bit (HCI_QUIRK_BROKEN_FILTER_CLEAR_ALL , &hdev->quirks)) +return 0 ; +/* Always clear event filter when starting */ hci_clear_event_filter_sync (hdev);
Signature · 1 line
2.35.1
PATCH v4 2/2 Bluetooth: btusb: Use quirk to skip HCI_FLT_CLEAR_ALL on fake CSR controllers
Another subset of the more recent batch of Chinese clones aren't specs-compliant and seem to lock up whenever they receive a HCI_OP_SET_EVENT_FLT with flt_type set to zero/HCI_FLT_CLEAR_ALL, which on Linux (until the recent HCI state-machine refactor) happened right at BR/EDR setup. As there are other less-straightforward ways of reaching those operations, this patch is still relevant.
So, while all the previous efforts to wrangle the herd of fake CSRs seem to be paying off (and these also get detected as such) we still need to take care of this quirk; testers seem to agree that these dongles tend to work well enough afterwards.
From some cursory USB packet capture on Windows it seems like that driver doesn't appear to use this clear-all functionality at all.
This patch was tested on some really popular AliExpress-style dongles, in my case marked as "V5.0". Chip markings: UG8413, the backside of the PCB says "USB Dangel" (sic).
Here is the hciconfig -a output; for completeness:
hci0: Type: Primary Bus: USB
BD Address: 00:1A:7D:DA:7X:XX ACL MTU: 679:8 SCO MTU: 48:16 UP RUNNING PSCAN ISCAN
Features: 0xbf 0x3e 0x4d 0xfa 0xdb 0x3d 0x7b 0xc7
Packet type: DM1 DM3 DM5 DH1 DH3 DH5 HV1 HV2 HV3
Link policy: RSWITCH SNIFF
Link mode: PERIPHERAL ACCEPT
Name: 'CSR8510 A10.'
Class: 0x7c0104
Service Classes: Rendering, Capturing, Object Transfer, Audio, Telephony Device Class: Computer, Desktop workstation
HCI Version: 4.0 (0x6) Revision: 0x3120
LMP Version: 4.0 (0x6) Subversion: 0x22bb
Manufacturer: Cambridge Silicon Radio (10)
As well as the lsusb -vv -d 0a12:0001:
ID 0a12:0001 Cambridge Silicon Radio, Ltd Bluetooth Dongle (HCI mode) Device Descriptor:
bLength 18
bDescriptorType 1
bcdUSB 2.00
bDeviceClass 224 Wireless
bDeviceSubClass 1 Radio Frequency
bDeviceProtocol 1 Bluetooth
bMaxPacketSize0 64
idVendor 0x0a12 Cambridge Silicon Radio, Ltd
idProduct 0x0001 Bluetooth Dongle (HCI mode)
bcdDevice 88.91
iManufacturer 0 iProduct 2 BT DONGLE10 iSerial 0 bNumConfigurations 1
Also, changed the benign dmesg print that shows up whenever the generic force-suspend fails from bt_dev_err to bt_dev_warn; it's okay and done on a best-effort basis, not a problem if that does not work.
Also, swapped the HCI subver and LMP subver numbers for the Barrot in the comment, which I copied wrong the last time around.
--- drivers/bluetooth/btusb.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)
drivers/bluetooth/btusb.c +4 -2
@@ -2057,6 +2057,8 @@ static int btusb_setup_csr(struct hci_dev *hdev)*/ set_bit (HCI_QUIRK_BROKEN_STORED_LINK_KEY , &hdev->quirks);set_bit (HCI_QUIRK_BROKEN_ERR_DATA_REPORTING , &hdev->quirks); +set_bit (HCI_QUIRK_BROKEN_FILTER_CLEAR_ALL , &hdev->quirks); +set_bit (HCI_QUIRK_NO_SUSPEND_NOTIFIER , &hdev->quirks);/* Clear the reset quirk since this is not an actual * early Bluetooth 1.1 device from CSR.
@@ -2067,7 +2069,7 @@ static int btusb_setup_csr(struct hci_dev *hdev)/* * Special workaround for these BT 4.0 chip clones, and potentially more: * -* - 0x0134: a Barrot 8041a02 (HCI rev: 0x1012 sub: 0x0810) +* - 0x0134: a Barrot 8041a02 (HCI rev: 0x0810 sub: 0x1012) * - 0x7558: IC markings FR3191AHAL 749H15143 (HCI rev/sub-version: 0x0709) * * These controllers are really messed-up.
@@ -2096,7 +2098,7 @@ static int btusb_setup_csr(struct hci_dev *hdev)if (ret >=0 )msleep (200 );else -bt_dev_err (hdev,"CSR: Failed to suspend the device for our Barrot 8041a02 receive-issue workaround" ); +bt_dev_warn (hdev,"CSR: Couldn't suspend the device for our Barrot 8041a02 receive-issue workaround" );pm_runtime_forbid (&data->udev->dev);
Signature · 1 line
2.35.1
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=621166
---Test result---
Test Summary:
CheckPatch PASS 3.37 seconds GitLint FAIL 2.00 seconds SubjectPrefix PASS 1.71 seconds BuildKernel PASS 30.88 seconds BuildKernel32 PASS 27.99 seconds Incremental Build with patchesPASS 43.99 seconds TestRunner: Setup PASS 474.58 seconds TestRunner: l2cap-tester PASS 15.58 seconds TestRunner: bnep-tester PASS 6.06 seconds TestRunner: mgmt-tester PASS 101.13 seconds TestRunner: rfcomm-tester PASS 7.73 seconds TestRunner: sco-tester PASS 7.61 seconds TestRunner: smp-tester PASS 7.62 seconds TestRunner: userchan-tester PASS 6.41 seconds
Details
##############################
Test: GitLint - FAIL - 2.00 seconds
Run gitlint with rule in .gitlint
[v4,2/2] Bluetooth: btusb: Use quirk to skip HCI_FLT_CLEAR_ALL on fake CSR controllers
1: T1 Title exceeds max length (86>80): "[v4,2/2] Bluetooth: btusb: Use quirk to skip HCI_FLT_CLEAR_ALL on fake CSR controllers"
24: B3 Line contains hard tab characters (\t): "hci0: Type: Primary Bus: USB"
25: B3 Line contains hard tab characters (\t): " BD Address: 00:1A:7D:DA:7X:XX ACL MTU: 679:8 SCO MTU: 48:16"
26: B3 Line contains hard tab characters (\t): " UP RUNNING PSCAN ISCAN"
27: B3 Line contains hard tab characters (\t): " Features: 0xbf 0x3e 0x4d 0xfa 0xdb 0x3d 0x7b 0xc7"
28: B3 Line contains hard tab characters (\t): " Packet type: DM1 DM3 DM5 DH1 DH3 DH5 HV1 HV2 HV3"
29: B3 Line contains hard tab characters (\t): " Link policy: RSWITCH SNIFF"
30: B3 Line contains hard tab characters (\t): " Link mode: PERIPHERAL ACCEPT"
31: B3 Line contains hard tab characters (\t): " Name: 'CSR8510 A10.'"
32: B3 Line contains hard tab characters (\t): " Class: 0x7c0104"
33: B3 Line contains hard tab characters (\t): " Service Classes: Rendering, Capturing, Object Transfer, Audio, Telephony"
34: B3 Line contains hard tab characters (\t): " Device Class: Computer, Desktop workstation"
35: B3 Line contains hard tab characters (\t): " HCI Version: 4.0 (0x6) Revision: 0x3120"
36: B3 Line contains hard tab characters (\t): " LMP Version: 4.0 (0x6) Subversion: 0x22bb"
37: B3 Line contains hard tab characters (\t): " Manufacturer: Cambridge Silicon Radio (10)"
---
Regards,
Linux Bluetooth
Hi,
Quoted 9 lines · Ismael Ferreras Morezuelas
Some controllers have problems with being sent a command to clear all filtering. While the HCI code does not unconditionally send a clear-all anymore at BR/EDR setup (after the state machine refactor), there might be more ways of hitting these codepaths in the future as the kernel develops.
Cc Hans de Goede <[email protected]>Signed-off-by Ismael Ferreras Morezuelas <[email protected]>
Thanks, the series looks good to me:
for both patches.
Regards,
Hans
Quoted 67 lines
--- include/net/bluetooth/hci.h | 10 ++++++++++ net/bluetooth/hci_sync.c | 16 ++++++++++++++++ 2 files changed, 26 insertions(+)include/net/bluetooth/hci.h +10
-0@@ -255,6 +255,16 @@ enum {* during the hdev->setup vendor callback. */ HCI_QUIRK_BROKEN_READ_TRANSMIT_POWER , + +/* When this quirk is set, HCI_OP_SET_EVENT_FLT requests with +* HCI_FLT_CLEAR_ALL are ignored and event filtering is +* completely avoided. A subset of the CSR controller +* clones struggle with this and instantly lock up. +* +* Note that devices using this must (separately) disable +* runtime suspend, because event filtering takes place there. +*/ + HCI_QUIRK_BROKEN_FILTER_CLEAR_ALL, };/* HCI device flags */ net/bluetooth/hci_sync.c +16
-0@@ -2812,6 +2812,9 @@ static int hci_set_event_filter_sync(struct hci_dev *hdev, u8 flt_type,if (!hci_dev_test_flag (hdev,HCI_BREDR_ENABLED ))return 0 ; +if (test_bit (HCI_QUIRK_BROKEN_FILTER_CLEAR_ALL , &hdev->quirks)) +return 0 ; +memset (&cp,0 ,sizeof (cp)); cp.flt_type = flt_type;@@ -2832,6 +2835,13 @@ static int hci_clear_event_filter_sync(struct hci_dev *hdev)if (!hci_dev_test_flag (hdev,HCI_EVENT_FILTER_CONFIGURED ))return 0 ; +/* In theory the state machine should not reach here unless +* a hci_set_event_filter_sync() call succeeds, but we do +* the check both for parity and as a future reminder. +*/ +if (test_bit (HCI_QUIRK_BROKEN_FILTER_CLEAR_ALL , &hdev->quirks)) +return 0 ; +return hci_set_event_filter_sync (hdev,HCI_FLT_CLEAR_ALL ,0x00 ,BDADDR_ANY ,0x00 ); }@@ -4831,6 +4841,12 @@ static int hci_update_event_filter_sync(struct hci_dev *hdev)if (!hci_dev_test_flag (hdev,HCI_BREDR_ENABLED ))return 0 ; +/* Some fake CSR controllers lock up after setting this type of +* filter, so avoid sending the request altogether. +*/ +if (test_bit (HCI_QUIRK_BROKEN_FILTER_CLEAR_ALL , &hdev->quirks)) +return 0 ; +/* Always clear event filter when starting */ hci_clear_event_filter_sync (hdev);
Marcel Holtmann
maintainerHi Ismael,
Quoted 6 lines
Some controllers have problems with being sent a command to clear all filtering. While the HCI code does not unconditionally send a clear-all anymore at BR/EDR setup (after the state machine refactor), there might be more ways of hitting these codepaths in the future as the kernel develops.
Quoted 7 lines
Cc Hans de Goede <[email protected]>Signed-off-by Ismael Ferreras Morezuelas <[email protected]>---
include/net/bluetooth/hci.h| 10 ++++++++++net/bluetooth/hci_sync.c| 16 ++++++++++++++++2 files changed, 26 insertions(+)
both patches have been applied to bluetooth-next tree.
Regards
Marcel