Re: [PATCH v2] Bluetooth: Add new quirk for broken extended create connection for ATS2851

[Данил <danstiv404@gmail.com>]

Hi!
Any news? I'm also interested in getting this patch into the kernel.

27.08.2024 3:21, Markus Uhr пишет:
> On Mon, 12 Aug 2024 11:17:20 -0400
> Luiz Augusto von Dentz <luiz.dentz@gmail.com> wrote:
>
>> On Sun, Aug 11, 2024 at 5:05 PM Markus Uhr <uhrmar@gmail.com> wrote:
>>> I'm interested in getting this patch merged. Was this just
>>> forgotten, or is there still work to be done before it can be
>>> merged?
>> It might need to be resend since it is no longer listed in patchwork.
> This is a resubmission of the patch, rebased on current bluetooth-next
> master.
>
> In addition to the original change, this revision also disables
> 'enhanced connection complete' when the quirk is active. This
> eliminates an ugly kernel error log
>
>     Bluetooth: hci0: Opcode 0x200e failed: -16
>
> when the controller connects successfully.
>
>
> Best,
> Markus
>
> -- >8 --
> Subject: [PATCH v3] Bluetooth: Add new quirk for broken extended create connection for ATS2851
>
> The controller based on ATS2851 advertises support for the "LE Extended
> Create Connection" command, but it does not actually implement it. This
> issue is blocking the pairing process from beginning.
>
> To resolve this, add the quirk HCI_QUIRK_BROKEN_EXT_CREATE_CONN.
> This will avoid the unsupported command and instead send a regular "LE
> Create Connection" command.
>
> < 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)
>
> Signed-off-by: Markus Uhr <uhrmar@gmail.com>
> ---
>   drivers/bluetooth/btusb.c        | 1 +
>   include/net/bluetooth/hci.h      | 7 +++++++
>   include/net/bluetooth/hci_core.h | 9 ++++++---
>   net/bluetooth/hci_sync.c         | 4 ++++
>   4 files changed, 18 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
> index 36a869a57..31f39e68a 100644
> --- a/drivers/bluetooth/btusb.c
> +++ b/drivers/bluetooth/btusb.c
> @@ -3883,6 +3883,7 @@ static int btusb_probe(struct usb_interface *intf,
>   		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);
>   		set_bit(HCI_QUIRK_BROKEN_READ_ENC_KEY_SIZE, &hdev->quirks);
>   	}
>   
> diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
> index bab1e3d74..4c942db4a 100644
> --- a/include/net/bluetooth/hci.h
> +++ b/include/net/bluetooth/hci.h
> @@ -328,6 +328,13 @@ enum {
>   	 */
>   	HCI_QUIRK_BROKEN_READ_ENC_KEY_SIZE,
>   
> +	/*
> +	 * When this quirk is set, the HCI_OP_LE_EXT_CREATE_CONN command is
> +	 * disabled. This is required for the Actions Semiconductor ATS2851
> +	 * based controllers, which erroneously claims to support it.
> +	 */
> +	HCI_QUIRK_BROKEN_EXT_CREATE_CONN,
> +
>   	/*
>   	 * When this quirk is set, the reserved bits of Primary/Secondary_PHY
>   	 * inside the LE Extended Advertising Report events are discarded.
> diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
> index e449dba69..a7a03ab13 100644
> --- a/include/net/bluetooth/hci_core.h
> +++ b/include/net/bluetooth/hci_core.h
> @@ -1876,7 +1876,8 @@ void hci_conn_del_sysfs(struct hci_conn *conn);
>   			   !test_bit(HCI_QUIRK_BROKEN_EXT_SCAN, &(dev)->quirks))
>   
>   /* Use ext create connection if command is supported */
> -#define use_ext_conn(dev) ((dev)->commands[37] & 0x80)
> +#define use_ext_conn(dev) (((dev)->commands[37] & 0x80) && \
> +			   !test_bit(HCI_QUIRK_BROKEN_EXT_CREATE_CONN, &(dev)->quirks))
>   
>   /* Extended advertising support */
>   #define ext_adv_capable(dev) (((dev)->le_features[1] & HCI_LE_EXT_ADV))
> @@ -1890,8 +1891,10 @@ void hci_conn_del_sysfs(struct hci_conn *conn);
>    * C24: Mandatory if the LE Controller supports Connection State and either
>    * LE Feature (LL Privacy) or LE Feature (Extended Advertising) is supported
>    */
> -#define use_enhanced_conn_complete(dev) (ll_privacy_capable(dev) || \
> -					 ext_adv_capable(dev))
> +#define use_enhanced_conn_complete(dev) ((ll_privacy_capable(dev) || \
> +					 ext_adv_capable(dev)) && \
> +					 !test_bit(HCI_QUIRK_BROKEN_EXT_CREATE_CONN, \
> +						 &(dev)->quirks))
>   
>   /* Periodic advertising support */
>   #define per_adv_capable(dev) (((dev)->le_features[1] & HCI_LE_PERIODIC_ADV))
> diff --git a/net/bluetooth/hci_sync.c b/net/bluetooth/hci_sync.c
> index e79cd40bd..160f260a7 100644
> --- a/net/bluetooth/hci_sync.c
> +++ b/net/bluetooth/hci_sync.c
> @@ -4800,6 +4800,9 @@ static const struct {
>   	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."),
>   	HCI_QUIRK_BROKEN(LE_CODED,
>   			 "HCI LE Coded PHY feature bit is set, "
>   			 "but its usage is not supported.")
> @@ -6427,6 +6430,7 @@ static int hci_le_create_conn_sync(struct hci_dev *hdev, void *data)
>   	if (err)
>   		goto done;
>   
> +	/* Send command LE Extended Create Connection if supported */
>   	if (use_ext_conn(hdev)) {
>   		err = hci_le_ext_create_conn_sync(hdev, conn, own_addr_type);
>   		goto done;