Re: [PATCH 2/3] Bluetooth: HCI: Add HCI_QUIRK_BROKEN_ENHANCED_SETUP_SCO quirk

Marcel Holtmann
2022-03-04T09:50:45Z

Hi Luiz,

> This adds HCI_QUIRK_BROKEN_ENHANCED_SETUP_SCO quirk which can be used
> to mark HCI_Enhanced_Setup_Synchronous_Connection as broken even if its
> support command bit are set since some controller report it as supported
> but the command don't work properly with some configurations
> (e.g. BT_VOICE_TRANSPARENT/mSBC).
>
> Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
> ---
> include/net/bluetooth/hci.h | 9 +++++++++
> include/net/bluetooth/hci_core.h | 8 ++++++--
> 2 files changed, 15 insertions(+), 2 deletions(-)
>
> diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
> index 35c073d44ec5..a4da339aab07 100644
> --- a/include/net/bluetooth/hci.h
> +++ b/include/net/bluetooth/hci.h
> @@ -255,6 +255,15 @@ enum {
> * during the hdev->setup vendor callback.
> */
> HCI_QUIRK_BROKEN_READ_TRANSMIT_POWER,
> +
> + /*
> + * When this quirk is set, disables the use of
> + * HCI_OP_ENHANCED_SETUP_SYNC_CONN command to setup SCO connections.

it is SCO and eSCO connections.

> + *
> + * This quirk can be set before hci_register_dev is called or
> + * during the hdev->setup vendor callback.
> + */
> + HCI_QUIRK_BROKEN_ENHANCED_SETUP_SCO,
> };

s/_SCO/_SYNC/

>
> /* HCI device flags */
> diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
> index d5377740e99c..7a9795783850 100644
> --- a/include/net/bluetooth/hci_core.h
> +++ b/include/net/bluetooth/hci_core.h
> @@ -1492,8 +1492,12 @@ void hci_conn_del_sysfs(struct hci_conn *conn);
> #define privacy_mode_capable(dev) (use_ll_privacy(dev) && \
> (hdev->commands[39] & 0x04))
>
> -/* Use enhanced synchronous connection if command is supported */
> -#define enhanced_sco_capable(dev) ((dev)->commands[29] & 0x08)
> +/* Use enhanced synchronous connection if command is supported and its quirk
> + * has not been set.
> + */
> +#define enhanced_sco_capable(dev) (((dev)->commands[29] & 0x08) && \
> + !test_bit(HCI_QUIRK_BROKEN_ENHANCED_SETUP_SCO, \
> + &(dev)->quirks))

Hmmm. So we got that naming wrong already. It should have really read enhanced_setup_sync_capable().

And frankly now I would actually rename it in the context that we limit it other than commands/feature bits. So use use_enhanced_setup_sync() in the code now.

Regards

Marcel