Re: [PATCH v2] Bluetooth: Partial support for Actions Semi ATS2851 based devices

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Paul,

On Wed, Mar 22, 2023 at 5:00 AM Paul Menzel <pmenzel@xxxxxxxxxxxxx> wrote:
>
> Dear Raul,
>
>
> Thank you for your patch. You could make the summary a statement by
> adding a verb (in imperative mood):
>
> Add partial support for Actions Semi ATS2851 based devices
>

Thanks Paul, noted it.

> Am 22.03.23 um 02:24 schrieb Raul Cheleguini:
> > The ATS2851 advertises support for commands "Set Random Private Address
> > Timeout" and "Extended Create Connection" but does not actually implement
> > them and reply with unknown HCI command.
> >
> > The failed first command blocks the device initialization, and the failed
> > second command blocks the start of the pairing process.
> >
> > Add these two quirks to unblock the device initialization and to skip
> > the extended create connection command when start pairing.
>
> … when start*ing* pairing.

Thanks Paul, noted it too.

>
> > v2: Move the extended create connection quirk to use_ext_conn, edit commit
> > description and add btmon logs.
> >
> > < HCI Command: LE Set Resolvable Private... (0x08|0x002e) plen 2
> >          Timeout: 900 seconds
> >> HCI Event: Command Status (0x0f) plen 4
> >        LE Set Resolvable Private Address Timeout (0x08|0x002e) ncmd 1
> >          Status: Unknown HCI Command (0x01)
> >
> > < HCI Command: LE Extended Create Conn.. (0x08|0x0043) plen 26
> >          Filter policy: Accept list is not used (0x00)
> >          Own address type: Public (0x00)
> >          Peer address type: Random (0x01)
> >          Peer address: DD:5E:B9:FE:49:3D (Static)
> >          Initiating PHYs: 0x01
> >          Entry 0: LE 1M
> >            Scan interval: 60.000 msec (0x0060)
> >            Scan window: 60.000 msec (0x0060)
> >            Min connection interval: 30.00 msec (0x0018)
> >            Max connection interval: 50.00 msec (0x0028)
> >            Connection latency: 0 (0x0000)
> >            Supervision timeout: 420 msec (0x002a)
> >            Min connection length: 0.000 msec (0x0000)
> >            Max connection length: 0.000 msec (0x0000)
> >> HCI Event: Command Status (0x0f) plen 4
> >        LE Extended Create Connection (0x08|0x0043) ncmd 1
> >          Status: Unknown HCI Command (0x01)
>
> The commit message summary/title says “partial support”. What is not
> working?

The pairing process is not working yet. There is some problem, related
to SMP I believe, that results in disconnection when pairing in Linux.

To give context, after I altered part of SMP code I managed to get my mouse
paired in Linux, but it needs more work to understand and isolate the problem.
I plan to gather more details and start a new thread about it.

I chose to mark it as partial support to inform the readers that it is
work in progress, but at the same time leave the device support in a little
better state than before. Is it acceptable?

> I’d also split this commit into two. One for each quirk.

Thanks for suggesting this and reviewing everything else Paul.
I will split it in two and improve the commit messages.

> > Signed-off-by: Raul Cheleguini <raul.cheleguini@xxxxxxxxx>
> > ---
> >   drivers/bluetooth/btusb.c        |  2 ++
> >   include/net/bluetooth/hci.h      | 14 ++++++++++++++
> >   include/net/bluetooth/hci_core.h |  3 ++-
> >   net/bluetooth/hci_sync.c         | 10 +++++++++-
> >   4 files changed, 27 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
> > index 7382b021f3df..8656ac491f13 100644
> > --- a/drivers/bluetooth/btusb.c
> > +++ b/drivers/bluetooth/btusb.c
> > @@ -4105,7 +4105,9 @@ static int btusb_probe(struct usb_interface *intf,
> >               /* Support is advertised, but not implemented */
> >               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_SET_RPA_TIMEOUT, &hdev->quirks);
> >               set_bit(HCI_QUIRK_BROKEN_EXT_SCAN, &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.
>
> controller*s* or claim*s*

Thanks Paul.

>
> > +      */
> > +     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,
>
> Ditto.

Thanks Paul.




[Index of Archives]     [Bluez Devel]     [Linux Wireless Networking]     [Linux Wireless Personal Area Networking]     [Linux ATH6KL]     [Linux USB Devel]     [Linux Media Drivers]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Big List of Linux Books]

  Powered by Linux