Re: [PATCH 1/7] Bluetooth: add a quirk for controllers that cannot enable Secure Connections

[Yang Li <yang.li@amlogic.com>]

Hi,
>
> [ EXTERNAL EMAIL ]
>
> From: Christian Stefan Kövér-Draxl <ckoever@outlook.de>
>
> The Amlogic W1 (W155S2) lists BR/EDR Secure Connections among its
> extended features and then fails HCI_OP_WRITE_SC_SUPPORT with an error.
> That failure is fatal to hci_dev_open_sync(), so the adapter can never
> be powered on: every mgmt Set Powered returns Invalid Parameters and
> bluetoothd gives up on the controller.
>
> Add HCI_QUIRK_BROKEN_WRITE_SC_SUPPORT for controllers whose feature bit
> cannot be taken at face value, skip the command in both places it is
> issued, and set the quirk in hci_aml.
This appears to be an anomaly with the firmware binary. Could you 
provide the FW version in use? It is logged during Bluetooth driver 
initialization.
> Signed-off-by: Christian Stefan Kövér-Draxl <ckoever@outlook.de>
> ---
>   drivers/bluetooth/hci_aml.c |  5 +++++
>   include/net/bluetooth/hci.h | 14 ++++++++++++++
>   net/bluetooth/hci_sync.c    |  8 ++++++--
>   3 files changed, 25 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/bluetooth/hci_aml.c b/drivers/bluetooth/hci_aml.c
> index 067fbf278b44..990e701da4be 100644
> --- a/drivers/bluetooth/hci_aml.c
> +++ b/drivers/bluetooth/hci_aml.c
> @@ -598,6 +598,11 @@ static int aml_setup(struct hci_uart *hu)
>          /* Setup bdaddr */
>          hdev->set_bdaddr = aml_set_bdaddr;
>
> +       /* The controller lists BR/EDR Secure Connections among its extended
> +        * features and then rejects the command that turns host support on.
> +        */
> +       hci_set_quirk(hdev, HCI_QUIRK_BROKEN_WRITE_SC_SUPPORT);
> +
>          err = aml_power_on(amldev);
>          if (err)
>                  return err;
> diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
> index 1641d879dbda..bcbe76c84d1b 100644
> --- a/include/net/bluetooth/hci.h
> +++ b/include/net/bluetooth/hci.h
> @@ -375,6 +375,20 @@ enum {
>           */
>          HCI_QUIRK_BROKEN_READ_PAGE_SCAN_TYPE,
>
> +       /*
> +        * When this quirk is set, the HCI_OP_WRITE_SC_SUPPORT command is
> +        * not supported.
> +        *
> +        * This quirk is intended for controllers that list BR/EDR Secure
> +        * Connections among their extended features and then reject the
> +        * command that turns host support on. Without it the failure takes
> +        * the whole of hci_dev_open_sync() with it and the controller can
> +        * never be powered on.
> +        *
> +        * This quirk must be set before hci_register_dev is called.
> +        */
> +       HCI_QUIRK_BROKEN_WRITE_SC_SUPPORT,
> +
>          __HCI_NUM_QUIRKS,
>   };
>
> diff --git a/net/bluetooth/hci_sync.c b/net/bluetooth/hci_sync.c
> index a92a81846e9d..f1dfef81ca96 100644
> --- a/net/bluetooth/hci_sync.c
> +++ b/net/bluetooth/hci_sync.c
> @@ -3419,7 +3419,8 @@ int hci_write_sc_support_sync(struct hci_dev *hdev, u8 val)
>   {
>          int err;
>
> -       if (!bredr_sc_enabled(hdev) || lmp_host_sc_capable(hdev))
> +       if (!bredr_sc_enabled(hdev) || lmp_host_sc_capable(hdev) ||
> +           hci_test_quirk(hdev, HCI_QUIRK_BROKEN_WRITE_SC_SUPPORT))
>                  return 0;
>
>          err = __hci_cmd_sync_status(hdev, HCI_OP_WRITE_SC_SUPPORT,
> @@ -5040,7 +5041,8 @@ static int hci_write_sc_support_1_sync(struct hci_dev *hdev)
>          u8 support = 0x01;
>
>          if (!hci_dev_test_flag(hdev, HCI_SSP_ENABLED) ||
> -           !bredr_sc_enabled(hdev))
> +           !bredr_sc_enabled(hdev) ||
> +           hci_test_quirk(hdev, HCI_QUIRK_BROKEN_WRITE_SC_SUPPORT))
>                  return 0;
>
>          return __hci_cmd_sync_status(hdev, HCI_OP_WRITE_SC_SUPPORT,
> @@ -5254,6 +5256,8 @@ static const struct {
>                           "HCI WRITE AUTH PAYLOAD TIMEOUT command leads "
>                           "to unexpected SMP errors when pairing "
>                           "and will not be used."),
> +       HCI_QUIRK_BROKEN(WRITE_SC_SUPPORT,
> +                        "HCI BR/EDR Secure Connections is advertised, but cannot be enabled."),
>          HCI_QUIRK_BROKEN(LE_CODED,
>                           "HCI LE Coded PHY feature bit is set, "
>                           "but its usage is not supported.")
> --
> 2.56.0
>