| Message ID | 20230317204620.2809181-1-raul.cheleguini@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Bluetooth: Partial support for Actions Semi ATS2851 based devices | expand |
| Context | Check | Description |
|---|---|---|
| tedd_an/pre-ci_am | success | Success |
| tedd_an/CheckPatch | warning | CHECK: Please don't use multiple blank lines #152: FILE: net/bluetooth/hci_sync.c:4097: + WARNING: quoted string split across lines #160: FILE: net/bluetooth/hci_sync.c:4535: "HCI Enhanced Setup Synchronous Connection command is " + "advertised, but not supported."), WARNING: quoted string split across lines #163: FILE: net/bluetooth/hci_sync.c:4538: + "HCI LE Set Random Private Address Timeout command is " + "advertised, but not supported."), total: 0 errors, 2 warnings, 1 checks, 61 lines checked NOTE: For some of the reported defects, checkpatch may be able to mechanically convert to the typical style using --fix or --fix-inplace. /github/workspace/src/src/13179427.patch has style problems, please review. NOTE: Ignored message types: UNKNOWN_COMMIT_ID NOTE: If any of the errors are false positives, please report them to the maintainer, see CHECKPATCH in MAINTAINERS. |
| tedd_an/GitLint | success | Gitlint PASS |
| tedd_an/SubjectPrefix | success | Gitlint PASS |
| tedd_an/BuildKernel | success | BuildKernel PASS |
| tedd_an/CheckAllWarning | success | CheckAllWarning PASS |
| tedd_an/CheckSparse | success | CheckSparse PASS |
| tedd_an/CheckSmatch | success | CheckSparse PASS |
| tedd_an/BuildKernel32 | success | BuildKernel32 PASS |
| tedd_an/TestRunnerSetup | success | TestRunnerSetup PASS |
| tedd_an/TestRunner_l2cap-tester | success | TestRunner PASS |
| tedd_an/TestRunner_iso-tester | success | TestRunner PASS |
| tedd_an/TestRunner_bnep-tester | success | TestRunner PASS |
| tedd_an/TestRunner_mgmt-tester | success | TestRunner PASS |
| tedd_an/TestRunner_rfcomm-tester | success | TestRunner PASS |
| tedd_an/TestRunner_sco-tester | success | TestRunner PASS |
| tedd_an/TestRunner_ioctl-tester | success | TestRunner PASS |
| tedd_an/TestRunner_mesh-tester | success | TestRunner PASS |
| tedd_an/TestRunner_smp-tester | success | TestRunner PASS |
| tedd_an/TestRunner_userchan-tester | success | TestRunner PASS |
| tedd_an/IncrementalBuild | success | Incremental Build PASS |
Hi Raul, On Fri, Mar 17, 2023 at 1:46 PM Raul Cheleguini <raul.cheleguini@gmail.com> wrote: > > This change enables support to device initialization and to scan > process, by adding two new quirks for the following advertised but > unsupported commands: "Set Random Private Address Timeout" and > "Extended Create Connection". > > It offers partial device support: controller initialization and scan. > The pairing process still needs work. > > At the moment there is little to none available documentation about the > ATS2851 and its based USB dongles. It is known that it works in other > systems via generic drivers, and this is one step towards have it working > in Linux. > > Signed-off-by: Raul Cheleguini <raul.cheleguini@gmail.com> > --- > drivers/bluetooth/btusb.c | 2 ++ > include/net/bluetooth/hci.h | 14 ++++++++++++++ > net/bluetooth/hci_sync.c | 13 +++++++++++-- > 3 files changed, 27 insertions(+), 2 deletions(-) > > diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c > index 7382b021f3df..7bba19061277 100644 > --- a/drivers/bluetooth/btusb.c > +++ b/drivers/bluetooth/btusb.c > @@ -4106,6 +4106,8 @@ static int btusb_probe(struct usb_interface *intf, > set_bit(HCI_QUIRK_BROKEN_ERR_DATA_REPORTING, &hdev->quirks); > set_bit(HCI_QUIRK_BROKEN_READ_TRANSMIT_POWER, &hdev->quirks); > set_bit(HCI_QUIRK_BROKEN_EXT_SCAN, &hdev->quirks); > + set_bit(HCI_QUIRK_BROKEN_SET_RPA_TIMEOUT, &hdev->quirks); > + set_bit(HCI_QUIRK_BROKEN_EXT_CREATE_CONN, &hdev->quirks); > } > > if (!reset) > diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h > index 997107bfc0b1..3ff1681fd2b8 100644 > --- a/include/net/bluetooth/hci.h > +++ b/include/net/bluetooth/hci.h > @@ -301,6 +301,20 @@ enum { > * don't actually support features declared there. > */ > HCI_QUIRK_BROKEN_LOCAL_EXT_FEATURES_PAGE_2, > + > + /* > + * When this quirk is set, the HCI_OP_LE_SET_RPA_TIMEOUT command is > + * disabled. This is required for the Actions Semiconductor ATS2851 > + * controller, which erroneously claim to support it. > + */ > + HCI_QUIRK_BROKEN_SET_RPA_TIMEOUT, > + > + /* > + * When this quirk is set, the HCI_OP_LE_EXT_CREATE_CONN command is > + * disabled. This is required for the Actions Semiconductor ATS2851 > + * controller, which erroneously claim to support it. > + */ > + HCI_QUIRK_BROKEN_EXT_CREATE_CONN, > }; > > /* HCI device flags */ > diff --git a/net/bluetooth/hci_sync.c b/net/bluetooth/hci_sync.c > index 8e5fe73873a8..9b2a0d6d6c1a 100644 > --- a/net/bluetooth/hci_sync.c > +++ b/net/bluetooth/hci_sync.c > @@ -4090,9 +4090,11 @@ static int hci_le_set_rpa_timeout_sync(struct hci_dev *hdev) > { > __le16 timeout = cpu_to_le16(hdev->rpa_timeout); > > - if (!(hdev->commands[35] & 0x04)) > + if (!(hdev->commands[35] & 0x04) || > + test_bit(HCI_QUIRK_BROKEN_SET_RPA_TIMEOUT, &hdev->quirks)) > return 0; This one Im not so sure, if we can't set a timeout then we shouldn't use the controller to rotate the RPA, although it seems we are already doing it when the command bit is not set. > > + > return __hci_cmd_sync_status(hdev, HCI_OP_LE_SET_RPA_TIMEOUT, > sizeof(timeout), &timeout, > HCI_CMD_TIMEOUT); > @@ -4530,6 +4532,12 @@ static const struct { > "HCI Set Event Filter command not supported."), > HCI_QUIRK_BROKEN(ENHANCED_SETUP_SYNC_CONN, > "HCI Enhanced Setup Synchronous Connection command is " > + "advertised, but not supported."), > + HCI_QUIRK_BROKEN(SET_RPA_TIMEOUT, > + "HCI LE Set Random Private Address Timeout command is " > + "advertised, but not supported."), > + HCI_QUIRK_BROKEN(EXT_CREATE_CONN, > + "HCI LE Extended Create Connection command is " > "advertised, but not supported.") > }; > > @@ -6067,7 +6075,8 @@ int hci_le_create_conn_sync(struct hci_dev *hdev, struct hci_conn *conn) > if (err) > goto done; > > - if (use_ext_conn(hdev)) { > + if (use_ext_conn(hdev) && > + !test_bit(HCI_QUIRK_BROKEN_EXT_CREATE_CONN, &hdev->quirks)) { > err = hci_le_ext_create_conn_sync(hdev, conn, own_addr_type); > goto done; > } I guess we can incorporate the new quirk check inside use_ext_conn. > -- > 2.39.2 >
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=731306
---Test result---
Test Summary:
CheckPatch FAIL 1.23 seconds
GitLint PASS 0.25 seconds
SubjectPrefix PASS 0.08 seconds
BuildKernel PASS 31.61 seconds
CheckAllWarning PASS 33.80 seconds
CheckSparse PASS 38.67 seconds
CheckSmatch PASS 107.38 seconds
BuildKernel32 PASS 30.10 seconds
TestRunnerSetup PASS 432.85 seconds
TestRunner_l2cap-tester PASS 16.46 seconds
TestRunner_iso-tester PASS 16.63 seconds
TestRunner_bnep-tester PASS 5.29 seconds
TestRunner_mgmt-tester PASS 106.43 seconds
TestRunner_rfcomm-tester PASS 8.50 seconds
TestRunner_sco-tester PASS 7.81 seconds
TestRunner_ioctl-tester PASS 9.16 seconds
TestRunner_mesh-tester PASS 6.73 seconds
TestRunner_smp-tester PASS 7.70 seconds
TestRunner_userchan-tester PASS 5.58 seconds
IncrementalBuild PASS 27.95 seconds
Details
##############################
Test: CheckPatch - FAIL
Desc: Run checkpatch.pl script
Output:
Bluetooth: Partial support for Actions Semi ATS2851 based devices
CHECK: Please don't use multiple blank lines
#152: FILE: net/bluetooth/hci_sync.c:4097:
+
WARNING: quoted string split across lines
#160: FILE: net/bluetooth/hci_sync.c:4535:
"HCI Enhanced Setup Synchronous Connection command is "
+ "advertised, but not supported."),
WARNING: quoted string split across lines
#163: FILE: net/bluetooth/hci_sync.c:4538:
+ "HCI LE Set Random Private Address Timeout command is "
+ "advertised, but not supported."),
total: 0 errors, 2 warnings, 1 checks, 61 lines checked
NOTE: For some of the reported defects, checkpatch may be able to
mechanically convert to the typical style using --fix or --fix-inplace.
/github/workspace/src/src/13179427.patch has style problems, please review.
NOTE: Ignored message types: UNKNOWN_COMMIT_ID
NOTE: If any of the errors are false positives, please report
them to the maintainer, see CHECKPATCH in MAINTAINERS.
---
Regards,
Linux Bluetooth
On Fri, Mar 17, 2023 at 5:54 PM Luiz Augusto von Dentz <luiz.dentz@gmail.com> wrote: > > Hi Raul, > > On Fri, Mar 17, 2023 at 1:46 PM Raul Cheleguini > <raul.cheleguini@gmail.com> wrote: > > > > This change enables support to device initialization and to scan > > process, by adding two new quirks for the following advertised but > > unsupported commands: "Set Random Private Address Timeout" and > > "Extended Create Connection". > > > > It offers partial device support: controller initialization and scan. > > The pairing process still needs work. > > > > At the moment there is little to none available documentation about the > > ATS2851 and its based USB dongles. It is known that it works in other > > systems via generic drivers, and this is one step towards have it working > > in Linux. > > > > Signed-off-by: Raul Cheleguini <raul.cheleguini@gmail.com> > > --- > > drivers/bluetooth/btusb.c | 2 ++ > > include/net/bluetooth/hci.h | 14 ++++++++++++++ > > net/bluetooth/hci_sync.c | 13 +++++++++++-- > > 3 files changed, 27 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c > > index 7382b021f3df..7bba19061277 100644 > > --- a/drivers/bluetooth/btusb.c > > +++ b/drivers/bluetooth/btusb.c > > @@ -4106,6 +4106,8 @@ static int btusb_probe(struct usb_interface *intf, > > set_bit(HCI_QUIRK_BROKEN_ERR_DATA_REPORTING, &hdev->quirks); > > set_bit(HCI_QUIRK_BROKEN_READ_TRANSMIT_POWER, &hdev->quirks); > > set_bit(HCI_QUIRK_BROKEN_EXT_SCAN, &hdev->quirks); > > + set_bit(HCI_QUIRK_BROKEN_SET_RPA_TIMEOUT, &hdev->quirks); > > + set_bit(HCI_QUIRK_BROKEN_EXT_CREATE_CONN, &hdev->quirks); > > } > > > > if (!reset) > > diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h > > index 997107bfc0b1..3ff1681fd2b8 100644 > > --- a/include/net/bluetooth/hci.h > > +++ b/include/net/bluetooth/hci.h > > @@ -301,6 +301,20 @@ enum { > > * don't actually support features declared there. > > */ > > HCI_QUIRK_BROKEN_LOCAL_EXT_FEATURES_PAGE_2, > > + > > + /* > > + * When this quirk is set, the HCI_OP_LE_SET_RPA_TIMEOUT command is > > + * disabled. This is required for the Actions Semiconductor ATS2851 > > + * controller, which erroneously claim to support it. > > + */ > > + HCI_QUIRK_BROKEN_SET_RPA_TIMEOUT, > > + > > + /* > > + * When this quirk is set, the HCI_OP_LE_EXT_CREATE_CONN command is > > + * disabled. This is required for the Actions Semiconductor ATS2851 > > + * controller, which erroneously claim to support it. > > + */ > > + HCI_QUIRK_BROKEN_EXT_CREATE_CONN, > > }; > > > > /* HCI device flags */ > > diff --git a/net/bluetooth/hci_sync.c b/net/bluetooth/hci_sync.c > > index 8e5fe73873a8..9b2a0d6d6c1a 100644 > > --- a/net/bluetooth/hci_sync.c > > +++ b/net/bluetooth/hci_sync.c > > @@ -4090,9 +4090,11 @@ static int hci_le_set_rpa_timeout_sync(struct hci_dev *hdev) > > { > > __le16 timeout = cpu_to_le16(hdev->rpa_timeout); > > > > - if (!(hdev->commands[35] & 0x04)) > > + if (!(hdev->commands[35] & 0x04) || > > + test_bit(HCI_QUIRK_BROKEN_SET_RPA_TIMEOUT, &hdev->quirks)) > > return 0; > > This one Im not so sure, if we can't set a timeout then we shouldn't > use the controller to rotate the RPA, although it seems we are already > doing it when the command bit is not set. Hi Luiz, this propose is based on two observations: - Another USB dongle I own here (fake CSR) initializes and works without the command. - The ATS2851 dongle initializes and works in other systems (as a generic device) without the command. I noticed there is a default timeout (HCI_DEFAULT_RPA_TIMEOUT) which is set during the hci device allocation. I presume this is used when we are unable to set the timeout. > > > > + > > return __hci_cmd_sync_status(hdev, HCI_OP_LE_SET_RPA_TIMEOUT, > > sizeof(timeout), &timeout, > > HCI_CMD_TIMEOUT); > > @@ -4530,6 +4532,12 @@ static const struct { > > "HCI Set Event Filter command not supported."), > > HCI_QUIRK_BROKEN(ENHANCED_SETUP_SYNC_CONN, > > "HCI Enhanced Setup Synchronous Connection command is " > > + "advertised, but not supported."), > > + HCI_QUIRK_BROKEN(SET_RPA_TIMEOUT, > > + "HCI LE Set Random Private Address Timeout command is " > > + "advertised, but not supported."), > > + HCI_QUIRK_BROKEN(EXT_CREATE_CONN, > > + "HCI LE Extended Create Connection command is " > > "advertised, but not supported.") > > }; > > > > @@ -6067,7 +6075,8 @@ int hci_le_create_conn_sync(struct hci_dev *hdev, struct hci_conn *conn) > > if (err) > > goto done; > > > > - if (use_ext_conn(hdev)) { > > + if (use_ext_conn(hdev) && > > + !test_bit(HCI_QUIRK_BROKEN_EXT_CREATE_CONN, &hdev->quirks)) { > > err = hci_le_ext_create_conn_sync(hdev, conn, own_addr_type); > > goto done; > > } > > I guess we can incorporate the new quirk check inside use_ext_conn. Thanks Luiz, I will resend it with this suggestion. > > -- > > 2.39.2 > > > > > -- > Luiz Augusto von Dentz
diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c index 7382b021f3df..7bba19061277 100644 --- a/drivers/bluetooth/btusb.c +++ b/drivers/bluetooth/btusb.c @@ -4106,6 +4106,8 @@ static int btusb_probe(struct usb_interface *intf, set_bit(HCI_QUIRK_BROKEN_ERR_DATA_REPORTING, &hdev->quirks); set_bit(HCI_QUIRK_BROKEN_READ_TRANSMIT_POWER, &hdev->quirks); set_bit(HCI_QUIRK_BROKEN_EXT_SCAN, &hdev->quirks); + set_bit(HCI_QUIRK_BROKEN_SET_RPA_TIMEOUT, &hdev->quirks); + set_bit(HCI_QUIRK_BROKEN_EXT_CREATE_CONN, &hdev->quirks); } if (!reset) diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h index 997107bfc0b1..3ff1681fd2b8 100644 --- a/include/net/bluetooth/hci.h +++ b/include/net/bluetooth/hci.h @@ -301,6 +301,20 @@ enum { * don't actually support features declared there. */ HCI_QUIRK_BROKEN_LOCAL_EXT_FEATURES_PAGE_2, + + /* + * When this quirk is set, the HCI_OP_LE_SET_RPA_TIMEOUT command is + * disabled. This is required for the Actions Semiconductor ATS2851 + * controller, which erroneously claim to support it. + */ + HCI_QUIRK_BROKEN_SET_RPA_TIMEOUT, + + /* + * When this quirk is set, the HCI_OP_LE_EXT_CREATE_CONN command is + * disabled. This is required for the Actions Semiconductor ATS2851 + * controller, which erroneously claim to support it. + */ + HCI_QUIRK_BROKEN_EXT_CREATE_CONN, }; /* HCI device flags */ diff --git a/net/bluetooth/hci_sync.c b/net/bluetooth/hci_sync.c index 8e5fe73873a8..9b2a0d6d6c1a 100644 --- a/net/bluetooth/hci_sync.c +++ b/net/bluetooth/hci_sync.c @@ -4090,9 +4090,11 @@ static int hci_le_set_rpa_timeout_sync(struct hci_dev *hdev) { __le16 timeout = cpu_to_le16(hdev->rpa_timeout); - if (!(hdev->commands[35] & 0x04)) + if (!(hdev->commands[35] & 0x04) || + test_bit(HCI_QUIRK_BROKEN_SET_RPA_TIMEOUT, &hdev->quirks)) return 0; + return __hci_cmd_sync_status(hdev, HCI_OP_LE_SET_RPA_TIMEOUT, sizeof(timeout), &timeout, HCI_CMD_TIMEOUT); @@ -4530,6 +4532,12 @@ static const struct { "HCI Set Event Filter command not supported."), HCI_QUIRK_BROKEN(ENHANCED_SETUP_SYNC_CONN, "HCI Enhanced Setup Synchronous Connection command is " + "advertised, but not supported."), + HCI_QUIRK_BROKEN(SET_RPA_TIMEOUT, + "HCI LE Set Random Private Address Timeout command is " + "advertised, but not supported."), + HCI_QUIRK_BROKEN(EXT_CREATE_CONN, + "HCI LE Extended Create Connection command is " "advertised, but not supported.") }; @@ -6067,7 +6075,8 @@ int hci_le_create_conn_sync(struct hci_dev *hdev, struct hci_conn *conn) if (err) goto done; - if (use_ext_conn(hdev)) { + if (use_ext_conn(hdev) && + !test_bit(HCI_QUIRK_BROKEN_EXT_CREATE_CONN, &hdev->quirks)) { err = hci_le_ext_create_conn_sync(hdev, conn, own_addr_type); goto done; }
This change enables support to device initialization and to scan process, by adding two new quirks for the following advertised but unsupported commands: "Set Random Private Address Timeout" and "Extended Create Connection". It offers partial device support: controller initialization and scan. The pairing process still needs work. At the moment there is little to none available documentation about the ATS2851 and its based USB dongles. It is known that it works in other systems via generic drivers, and this is one step towards have it working in Linux. Signed-off-by: Raul Cheleguini <raul.cheleguini@gmail.com> --- drivers/bluetooth/btusb.c | 2 ++ include/net/bluetooth/hci.h | 14 ++++++++++++++ net/bluetooth/hci_sync.c | 13 +++++++++++-- 3 files changed, 27 insertions(+), 2 deletions(-)