In particular this series :
1. It allows the driver to indicate support for wide band speech.
2. Use a quirk tied to a MGMT local controller settings allowing a
higherlevel HFP implementation to check if the local controller supports
wide band speech.
3. When the controller reports support for wide band speech, enable
erroneous data reporting to allow a higher level PLC to deal with
erroneous or missing packets.
Alain Michaud (3):
bluetooth: Add flag to define wideband speech capability
bluetooth: Support querying for WBS support through MGMT
bluetooth: Enable erroneous data reporting if wbs is supported
drivers/bluetooth/btusb.c | 22 ++++++++++++++++------
include/net/bluetooth/hci.h | 16 ++++++++++++++++
include/net/bluetooth/mgmt.h | 1 +
net/bluetooth/hci_core.c | 33 +++++++++++++++++++++++++++++++++
net/bluetooth/mgmt.c | 4 ++++
5 files changed, 70 insertions(+), 6 deletions(-)
--
2.25.1.481.gfbce0eb801-goog
This change adds a new flag to define a controller's wideband speech
capability. This is required since no reliable over HCI mechanism
exists to query the controller and driver's compatibility with
wideband speech.
Signed-off-by: Alain Michaud <[email protected]>
---
drivers/bluetooth/btusb.c | 19 +++++++++++++------
1 file changed, 13 insertions(+), 6 deletions(-)
diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
index f5924f3e8b8d..1b175ba3085c 100644
--- a/drivers/bluetooth/btusb.c
+++ b/drivers/bluetooth/btusb.c
@@ -57,6 +57,7 @@ static struct usb_driver btusb_driver;
#define BTUSB_IFNUM_2 0x80000
#define BTUSB_CW6622 0x100000
#define BTUSB_MEDIATEK 0x200000
+#define BTUSB_WIDEBAND_SPEECH 0x400000
static const struct usb_device_id btusb_table[] = {
/* Generic Bluetooth USB device */
@@ -333,15 +334,21 @@ static const struct usb_device_id blacklist_table[] = {
{ USB_DEVICE(0x1286, 0x204e), .driver_info = BTUSB_MARVELL },
/* Intel Bluetooth devices */
- { USB_DEVICE(0x8087, 0x0025), .driver_info = BTUSB_INTEL_NEW },
- { USB_DEVICE(0x8087, 0x0026), .driver_info = BTUSB_INTEL_NEW },
- { USB_DEVICE(0x8087, 0x0029), .driver_info = BTUSB_INTEL_NEW },
+ { USB_DEVICE(0x8087, 0x0025), .driver_info = BTUSB_INTEL_NEW |
+ BTUSB_WIDEBAND_SPEECH },
+ { USB_DEVICE(0x8087, 0x0026), .driver_info = BTUSB_INTEL_NEW |
+ BTUSB_WIDEBAND_SPEECH },
+ { USB_DEVICE(0x8087, 0x0029), .driver_info = BTUSB_INTEL_NEW |
+ BTUSB_WIDEBAND_SPEECH },
{ USB_DEVICE(0x8087, 0x07da), .driver_info = BTUSB_CSR },
{ USB_DEVICE(0x8087, 0x07dc), .driver_info = BTUSB_INTEL },
{ USB_DEVICE(0x8087, 0x0a2a), .driver_info = BTUSB_INTEL },
- { USB_DEVICE(0x8087, 0x0a2b), .driver_info = BTUSB_INTEL_NEW },
- { USB_DEVICE(0x8087, 0x0aa7), .driver_info = BTUSB_INTEL },
- { USB_DEVICE(0x8087, 0x0aaa), .driver_info = BTUSB_INTEL_NEW },
+ { USB_DEVICE(0x8087, 0x0a2b), .driver_info = BTUSB_INTEL_NEW |
+ BTUSB_WIDEBAND_SPEECH },
+ { USB_DEVICE(0x8087, 0x0aa7), .driver_info = BTUSB_INTEL |
+ BTUSB_WIDEBAND_SPEECH },
+ { USB_DEVICE(0x8087, 0x0aaa), .driver_info = BTUSB_INTEL_NEW |
+ BTUSB_WIDEBAND_SPEECH },
/* Other Intel Bluetooth devices */
{ USB_VENDOR_AND_INTERFACE_INFO(0x8087, 0xe0, 0x01, 0x01),
--
2.25.1.481.gfbce0eb801-goog
This change will enable erroneous data reporting if wide band speech is
supported by the controller as indicated by the
HCI_QUIRK_WIDE_BAND_SPEECH_SUPPORTED quirk.
Signed-off-by: Alain Michaud <[email protected]>
---
include/net/bluetooth/hci.h | 7 +++++++
net/bluetooth/hci_core.c | 33 +++++++++++++++++++++++++++++++++
2 files changed, 40 insertions(+)
diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
index b214c4358d1c..807fd40bb358 100644
--- a/include/net/bluetooth/hci.h
+++ b/include/net/bluetooth/hci.h
@@ -1095,6 +1095,13 @@ struct hci_rp_read_inq_rsp_tx_power {
__s8 tx_power;
} __packed;
+#define HCI_OP_WRITE_DEFAULT_ERRONEOUS_DATA_REPORTING 0x0c5b
+ #define ERRONEOUS_DATA_REPORTING_DISABLED 0x00
+ #define ERRONEOUS_DATA_REPORTING_ENABLED 0x01
+struct hci_cp_write_default_erroneous_data_reporting {
+ __u8 erroneous_data_reporting;
+} __packed;
+
#define HCI_OP_SET_EVENT_MASK_PAGE_2 0x0c63
#define HCI_OP_READ_LOCATION_DATA 0x0c64
diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index 1ca7508b6ca7..19afb85ad399 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -1382,6 +1382,29 @@ static void hci_dev_get_bd_addr_from_property(struct hci_dev *hdev)
bacpy(&hdev->public_addr, &ba);
}
+static int hci_write_default_erroneous_data_req(struct hci_request *req,
+ unsigned long opt)
+{
+ struct hci_cp_write_default_erroneous_data_reporting cp = {};
+
+ cp.erroneous_data_reporting = (__u8)opt;
+ hci_req_add(req, HCI_OP_WRITE_DEFAULT_ERRONEOUS_DATA_REPORTING,
+ sizeof(cp), &cp);
+ return 0;
+}
+
+static void hci_write_default_erroneous_data(struct hci_dev *hdev,
+ __u8 erroneous_data_reporting)
+{
+ int err = __hci_req_sync(hdev,
+ hci_write_default_erroneous_data_req,
+ erroneous_data_reporting, HCI_CMD_TIMEOUT,
+ NULL);
+ if (err) {
+ BT_ERR("HCI_OP_WRITE_DEFAULT_ERRONEOUS_DATA_REPORTING failed");
+ }
+}
+
static int hci_dev_do_open(struct hci_dev *hdev)
{
int ret = 0;
@@ -1547,6 +1570,16 @@ static int hci_dev_do_open(struct hci_dev *hdev)
set_bit(HCI_UP, &hdev->flags);
hci_sock_dev_event(hdev, HCI_DEV_UP);
hci_leds_update_powered(hdev, true);
+
+ /* If the controller supports wide_band_speech, enable erroneous
+ * data reporting.
+ */
+ if (test_bit(HCI_QUIRK_WIDE_BAND_SPEECH_SUPPORTED,
+ &hdev->quirks)) {
+ hci_write_default_erroneous_data(hdev,
+ ERRONEOUS_DATA_REPORTING_ENABLED);
+ }
+
if (!hci_dev_test_flag(hdev, HCI_SETUP) &&
!hci_dev_test_flag(hdev, HCI_CONFIG) &&
!hci_dev_test_flag(hdev, HCI_UNCONFIGURED) &&
--
2.25.1.481.gfbce0eb801-goog
This patch provides a mechanism for MGMT interface client to query the
capability of the controller to support WBS.
Signed-off-by: Alain Michaud <[email protected]>
---
drivers/bluetooth/btusb.c | 3 +++
include/net/bluetooth/hci.h | 9 +++++++++
include/net/bluetooth/mgmt.h | 1 +
net/bluetooth/mgmt.c | 4 ++++
4 files changed, 17 insertions(+)
diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
index 1b175ba3085c..95613f103d17 100644
--- a/drivers/bluetooth/btusb.c
+++ b/drivers/bluetooth/btusb.c
@@ -3866,6 +3866,9 @@ static int btusb_probe(struct usb_interface *intf,
if (id->driver_info & BTUSB_BROKEN_ISOC)
data->isoc = NULL;
+ if (id->driver_info & BTUSB_WIDEBAND_SPEECH)
+ set_bit(HCI_QUIRK_WIDE_BAND_SPEECH_SUPPORTED, &hdev->quirks);
+
if (id->driver_info & BTUSB_DIGIANSWER) {
data->cmdreq_type = USB_TYPE_VENDOR;
set_bit(HCI_QUIRK_RESET_ON_CLOSE, &hdev->quirks);
diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
index 6293bdd7d862..b214c4358d1c 100644
--- a/include/net/bluetooth/hci.h
+++ b/include/net/bluetooth/hci.h
@@ -205,6 +205,15 @@ enum {
*
*/
HCI_QUIRK_NON_PERSISTENT_SETUP,
+
+ /* When this quirt is set, wide band speech is supported by
+ * the driver since no reliable mechanism exist to report
+ * this from the hardware, a driver flag is use to convey
+ * this support
+ *
+ * This quirk must be set before hci_register_dev is called.
+ */
+ HCI_QUIRK_WIDE_BAND_SPEECH_SUPPORTED,
};
/* HCI device flags */
diff --git a/include/net/bluetooth/mgmt.h b/include/net/bluetooth/mgmt.h
index a90666af05bd..51339584c3a9 100644
--- a/include/net/bluetooth/mgmt.h
+++ b/include/net/bluetooth/mgmt.h
@@ -102,6 +102,7 @@ struct mgmt_rp_read_index_list {
#define MGMT_SETTING_CONFIGURATION 0x00004000
#define MGMT_SETTING_STATIC_ADDRESS 0x00008000
#define MGMT_SETTING_PHY_CONFIGURATION 0x00010000
+#define MGMT_SETTING_WIDE_BAND_SPEECH 0x00020000
#define MGMT_OP_READ_INFO 0x0004
#define MGMT_READ_INFO_SIZE 0
diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index 0dc610faab70..a08691d679a8 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -762,6 +762,10 @@ static u32 get_supported_settings(struct hci_dev *hdev)
if (lmp_sc_capable(hdev))
settings |= MGMT_SETTING_SECURE_CONN;
+
+ if (test_bit(HCI_QUIRK_WIDE_BAND_SPEECH_SUPPORTED,
+ &hdev->quirks))
+ settings |= MGMT_SETTING_WIDE_BAND_SPEECH;
}
if (lmp_le_capable(hdev)) {
--
2.25.1.481.gfbce0eb801-goog
Hi Alain,
> This patch provides a mechanism for MGMT interface client to query the
> capability of the controller to support WBS.
>
> Signed-off-by: Alain Michaud <[email protected]>
> ---
>
> drivers/bluetooth/btusb.c | 3 +++
> include/net/bluetooth/hci.h | 9 +++++++++
> include/net/bluetooth/mgmt.h | 1 +
> net/bluetooth/mgmt.c | 4 ++++
> 4 files changed, 17 insertions(+)
patch has been applied to bluetooth-next tree.
However ..
> +
> + /* When this quirt is set, wide band speech is supported by
> + * the driver since no reliable mechanism exist to report
> + * this from the hardware, a driver flag is use to convey
> + * this support
> + *
> + * This quirk must be set before hci_register_dev is called.
> + */
> + HCI_QUIRK_WIDE_BAND_SPEECH_SUPPORTED,
> };
quirt - a short-handled riding whip with a braided leather lash.
.. this totally made my day and I was tempted to leave this typo in the code, but I fixed it up in the end ;)
Regards
Marcel
Hi Alain,
> This change adds a new flag to define a controller's wideband speech
> capability. This is required since no reliable over HCI mechanism
> exists to query the controller and driver's compatibility with
> wideband speech.
>
> Signed-off-by: Alain Michaud <[email protected]>
> ---
>
> drivers/bluetooth/btusb.c | 19 +++++++++++++------
> 1 file changed, 13 insertions(+), 6 deletions(-)
patch has been applied to bluetooth-next tree.
Regards
Marcel
Hi Alain,
> This change will enable erroneous data reporting if wide band speech is
> supported by the controller as indicated by the
> HCI_QUIRK_WIDE_BAND_SPEECH_SUPPORTED quirk.
>
> Signed-off-by: Alain Michaud <[email protected]>
> ---
>
> include/net/bluetooth/hci.h | 7 +++++++
> net/bluetooth/hci_core.c | 33 +++++++++++++++++++++++++++++++++
> 2 files changed, 40 insertions(+)
>
> diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
> index b214c4358d1c..807fd40bb358 100644
> --- a/include/net/bluetooth/hci.h
> +++ b/include/net/bluetooth/hci.h
> @@ -1095,6 +1095,13 @@ struct hci_rp_read_inq_rsp_tx_power {
> __s8 tx_power;
> } __packed;
>
> +#define HCI_OP_WRITE_DEFAULT_ERRONEOUS_DATA_REPORTING 0x0c5b
> + #define ERRONEOUS_DATA_REPORTING_DISABLED 0x00
> + #define ERRONEOUS_DATA_REPORTING_ENABLED 0x01
> +struct hci_cp_write_default_erroneous_data_reporting {
> + __u8 erroneous_data_reporting;
> +} __packed;
> +
So I realized that in monitor/bt.h I also used the full name, but inside the kernel code it feels way too long and creates harder to read code.
My proposal is HCI_OP_READ_DEF_ERR_DATA_REPORTING and HCI_OP_WRITE_DEF_ERR_DATA_REPORTING.
You might realize that I included the read command here as well. So first, we need to read the current value of the controller if supported and then also start tracking changes to it. You will find examples in hci_event.c and how that is done. Store the current value in hci_dev.
I also realized that we should only advertise Wideband speech support if this command is actually supported. So that you might need to fix as well.
> #define HCI_OP_SET_EVENT_MASK_PAGE_2 0x0c63
>
> #define HCI_OP_READ_LOCATION_DATA 0x0c64
> diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
> index 1ca7508b6ca7..19afb85ad399 100644
> --- a/net/bluetooth/hci_core.c
> +++ b/net/bluetooth/hci_core.c
> @@ -1382,6 +1382,29 @@ static void hci_dev_get_bd_addr_from_property(struct hci_dev *hdev)
> bacpy(&hdev->public_addr, &ba);
> }
>
> +static int hci_write_default_erroneous_data_req(struct hci_request *req,
> + unsigned long opt)
> +{
> + struct hci_cp_write_default_erroneous_data_reporting cp = {};
> +
> + cp.erroneous_data_reporting = (__u8)opt;
> + hci_req_add(req, HCI_OP_WRITE_DEFAULT_ERRONEOUS_DATA_REPORTING,
> + sizeof(cp), &cp);
> + return 0;
> +}
> +
> +static void hci_write_default_erroneous_data(struct hci_dev *hdev,
> + __u8 erroneous_data_reporting)
> +{
> + int err = __hci_req_sync(hdev,
> + hci_write_default_erroneous_data_req,
> + erroneous_data_reporting, HCI_CMD_TIMEOUT,
> + NULL);
> + if (err) {
> + BT_ERR("HCI_OP_WRITE_DEFAULT_ERRONEOUS_DATA_REPORTING failed");
> + }
> +}
> +
> static int hci_dev_do_open(struct hci_dev *hdev)
> {
> int ret = 0;
> @@ -1547,6 +1570,16 @@ static int hci_dev_do_open(struct hci_dev *hdev)
> set_bit(HCI_UP, &hdev->flags);
> hci_sock_dev_event(hdev, HCI_DEV_UP);
> hci_leds_update_powered(hdev, true);
> +
> + /* If the controller supports wide_band_speech, enable erroneous
> + * data reporting.
> + */
> + if (test_bit(HCI_QUIRK_WIDE_BAND_SPEECH_SUPPORTED,
> + &hdev->quirks)) {
> + hci_write_default_erroneous_data(hdev,
> + ERRONEOUS_DATA_REPORTING_ENABLED);
> + }
> +
> if (!hci_dev_test_flag(hdev, HCI_SETUP) &&
> !hci_dev_test_flag(hdev, HCI_CONFIG) &&
> !hci_dev_test_flag(hdev, HCI_UNCONFIGURED) &&
Don’t do this in hci_dev_do_open. You want to read the current value in the init procedure and change it if needed. This is similar to Secure Connections and other values that we only allow to change while the controller is down.
Regards
Marcel