| Message ID | 20220307200445.5554-1-swyterzone@gmail.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | d35c9b22957afd7a7784b15f9886a130a3026668 |
| Headers | show |
| Series | [v4,1/2] Bluetooth: hci_sync: Add a new quirk to skip HCI_FLT_CLEAR_ALL | expand |
| Context | Check | Description |
|---|---|---|
| tedd_an/pre-ci_am | success | Success |
| tedd_an/checkpatch | success | Checkpatch PASS |
| tedd_an/gitlint | success | Gitlint PASS |
| tedd_an/subjectprefix | success | PASS |
| tedd_an/buildkernel | success | Build Kernel PASS |
| tedd_an/buildkernel32 | success | Build Kernel32 PASS |
| tedd_an/incremental_build | success | Pass |
| tedd_an/testrunnersetup | success | Test Runner Setup PASS |
| tedd_an/testrunnerl2cap-tester | success | Total: 40, Passed: 40 (100.0%), Failed: 0, Not Run: 0 |
| tedd_an/testrunnerbnep-tester | success | Total: 1, Passed: 1 (100.0%), Failed: 0, Not Run: 0 |
| tedd_an/testrunnermgmt-tester | success | Total: 493, Passed: 493 (100.0%), Failed: 0, Not Run: 0 |
| tedd_an/testrunnerrfcomm-tester | success | Total: 10, Passed: 10 (100.0%), Failed: 0, Not Run: 0 |
| tedd_an/testrunnersco-tester | success | Total: 12, Passed: 12 (100.0%), Failed: 0, Not Run: 0 |
| tedd_an/testrunnersmp-tester | success | Total: 8, Passed: 8 (100.0%), Failed: 0, Not Run: 0 |
| tedd_an/testrunneruserchan-tester | success | Total: 4, Passed: 4 (100.0%), Failed: 0, Not Run: 0 |
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, On 3/7/22 21:04, Ismael Ferreras Morezuelas wrote: > 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: stable@vger.kernel.org > Cc: Hans de Goede <hdegoede@redhat.com> > Signed-off-by: Ismael Ferreras Morezuelas <swyterzone@gmail.com> Thanks, the series looks good to me: Reviewed-by: Hans de Goede <hdegoede@redhat.com> for both patches. Regards, Hans > --- > include/net/bluetooth/hci.h | 10 ++++++++++ > net/bluetooth/hci_sync.c | 16 ++++++++++++++++ > 2 files changed, 26 insertions(+) > > diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h > index 35c073d44ec5..5cb095b09a94 100644 > --- a/include/net/bluetooth/hci.h > +++ b/include/net/bluetooth/hci.h > @@ -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 */ > diff --git a/net/bluetooth/hci_sync.c b/net/bluetooth/hci_sync.c > index e31d1150dc71..c3bdaf2de511 100644 > --- a/net/bluetooth/hci_sync.c > +++ b/net/bluetooth/hci_sync.c > @@ -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); >
Hi Ismael, > 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: stable@vger.kernel.org > Cc: Hans de Goede <hdegoede@redhat.com> > Signed-off-by: Ismael Ferreras Morezuelas <swyterzone@gmail.com> > --- > 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
diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h index 35c073d44ec5..5cb095b09a94 100644 --- a/include/net/bluetooth/hci.h +++ b/include/net/bluetooth/hci.h @@ -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 */ diff --git a/net/bluetooth/hci_sync.c b/net/bluetooth/hci_sync.c index e31d1150dc71..c3bdaf2de511 100644 --- a/net/bluetooth/hci_sync.c +++ b/net/bluetooth/hci_sync.c @@ -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);
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: stable@vger.kernel.org Cc: Hans de Goede <hdegoede@redhat.com> Signed-off-by: Ismael Ferreras Morezuelas <swyterzone@gmail.com> --- include/net/bluetooth/hci.h | 10 ++++++++++ net/bluetooth/hci_sync.c | 16 ++++++++++++++++ 2 files changed, 26 insertions(+)