2023-08-23 01:28:57

by Paul Menzel

[permalink] [raw]
Subject: Re: [PATCH] Bluetooth: HCI: Introduce HCI_QUIRK_BROKEN_LE_CODED

[CC: +Bruno]

Dear Luiz,


Thank you for the patch.

Am 22.08.23 um 21:14 schrieb Luiz Augusto von Dentz:
> From: Luiz Augusto von Dentz <[email protected]>
>
> This introduces HCI_QUIRK_BROKEN_LE_CODED is is used to indicate that

…. It is used …

> LE Coded PHY shall not be used, it is then set for some Intel models
> that claims to support it but when used causes many problems.

that claim to …

> Link: https://github.com/bluez/bluez/issues/577
> Link: https://github.com/bluez/bluez/issues/582
> Link: https://lore.kernel.org/linux-bluetooth/CABBYNZKco-v7wkjHHexxQbgwwSz-S=GZ=dZKbRE1qxT1h4fFbQ@mail.gmail.com/T/#
> Fixes: 288c90224eec ("Bluetooth: Enable all supported LE PHY by default")
> Signed-off-by: Luiz Augusto von Dentz <[email protected]>
> ---
> drivers/bluetooth/btintel.c | 2 ++
> include/net/bluetooth/hci.h | 10 ++++++++++
> include/net/bluetooth/hci_core.h | 4 +++-
> 3 files changed, 15 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/bluetooth/btintel.c b/drivers/bluetooth/btintel.c
> index 9b239ce96fa4..3ed60b2b0340 100644
> --- a/drivers/bluetooth/btintel.c
> +++ b/drivers/bluetooth/btintel.c
> @@ -2776,6 +2776,8 @@ static int btintel_setup_combined(struct hci_dev *hdev)
> case 0x11: /* JfP */
> case 0x12: /* ThP */
> case 0x13: /* HrP */
> + set_bit(HCI_QUIRK_BROKEN_LE_CODED, &hdev->quirks);
> + fallthrough;
> case 0x14: /* CcP */
> set_bit(HCI_QUIRK_VALID_LE_STATES, &hdev->quirks);
> fallthrough;
> diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
> index c58425d4c592..767921d7f5c1 100644
> --- a/include/net/bluetooth/hci.h
> +++ b/include/net/bluetooth/hci.h
> @@ -319,6 +319,16 @@ enum {
> * This quirk must be set before hci_register_dev is called.
> */
> HCI_QUIRK_USE_MSFT_EXT_ADDRESS_FILTER,
> +
> + /*
> + * When this quirk is set, LE Coded PHY is shall not be used. This is

s/is shall/shall/

> + * required for some Intel controllers which erroneously claim to
> + * support it but it causes problems with extended scanning.
> + *
> + * This quirk can be set before hci_register_dev is called or
> + * during the hdev->setup vendor callback.
> + */
> + HCI_QUIRK_BROKEN_LE_CODED,
> };
>
> /* HCI device flags */
> diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
> index 6e2988b11f99..e6359f7346f1 100644
> --- a/include/net/bluetooth/hci_core.h
> +++ b/include/net/bluetooth/hci_core.h
> @@ -1817,7 +1817,9 @@ void hci_conn_del_sysfs(struct hci_conn *conn);
> #define scan_2m(dev) (((dev)->le_tx_def_phys & HCI_LE_SET_PHY_2M) || \
> ((dev)->le_rx_def_phys & HCI_LE_SET_PHY_2M))
>
> -#define le_coded_capable(dev) (((dev)->le_features[1] & HCI_LE_PHY_CODED))
> +#define le_coded_capable(dev) (((dev)->le_features[1] & HCI_LE_PHY_CODED) && \
> + !test_bit(HCI_QUIRK_BROKEN_LE_CODED, \
> + &(dev)->quirks))
>
> #define scan_coded(dev) (((dev)->le_tx_def_phys & HCI_LE_SET_PHY_CODED) || \
> ((dev)->le_rx_def_phys & HCI_LE_SET_PHY_CODED))

Will this be future proof, once firmware for the broken controllers are
fixed?


Kind regards,

Paul