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.
