On Fri, Mar 17, 2023 at 5:54 PM Luiz Augusto von Dentz
<luiz.dentz@xxxxxxxxx> wrote:
>
> Hi Raul,
>
> On Fri, Mar 17, 2023 at 1:46 PM Raul Cheleguini
> <raul.cheleguini@xxxxxxxxx> 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@xxxxxxxxx>
> > ---
> > 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
