* [PATCH v4] bluetooth: Enable erroneous data reporting if wbs is supported
@ 2020-03-05 16:14 Alain Michaud
2020-03-08 7:53 ` Marcel Holtmann
2020-03-10 23:10 ` Luiz Augusto von Dentz
0 siblings, 2 replies; 4+ messages in thread
From: Alain Michaud @ 2020-03-05 16:14 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Alain Michaud
This change introduces a wide band speech setting which allows higher
level clients to query the local controller support for wide band speech
as well as set the setting state when the radio is powered off.
Internally, this setting controls if erroneous data reporting is enabled
on the controller.
Signed-off-by: Alain Michaud <alainm@chromium.org>
---
drivers/bluetooth/btusb.c | 2 +-
include/net/bluetooth/hci.h | 16 +++++++-
include/net/bluetooth/hci_core.h | 1 +
include/net/bluetooth/mgmt.h | 4 +-
net/bluetooth/hci_core.c | 24 ++++++++++++
net/bluetooth/hci_event.c | 39 +++++++++++++++++++
net/bluetooth/mgmt.c | 66 +++++++++++++++++++++++++++++++-
7 files changed, 147 insertions(+), 5 deletions(-)
diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
index 48e78fdc8e83..4b12c34f0b22 100644
--- a/drivers/bluetooth/btusb.c
+++ b/drivers/bluetooth/btusb.c
@@ -3868,7 +3868,7 @@ static int btusb_probe(struct usb_interface *intf,
data->isoc = NULL;
if (id->driver_info & BTUSB_WIDEBAND_SPEECH)
- set_bit(HCI_QUIRK_WIDE_BAND_SPEECH_SUPPORTED, &hdev->quirks);
+ set_bit(HCI_QUIRK_WIDEBAND_SPEECH_SUPPORTED, &hdev->quirks);
if (id->driver_info & BTUSB_DIGIANSWER) {
data->cmdreq_type = USB_TYPE_VENDOR;
diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
index 0b3ebd35681d..4e86f1bb7a87 100644
--- a/include/net/bluetooth/hci.h
+++ b/include/net/bluetooth/hci.h
@@ -213,7 +213,7 @@ enum {
*
* This quirk must be set before hci_register_dev is called.
*/
- HCI_QUIRK_WIDE_BAND_SPEECH_SUPPORTED,
+ HCI_QUIRK_WIDEBAND_SPEECH_SUPPORTED,
};
/* HCI device flags */
@@ -286,6 +286,7 @@ enum {
HCI_FAST_CONNECTABLE,
HCI_BREDR_ENABLED,
HCI_LE_SCAN_INTERRUPTED,
+ HCI_WIDEBAND_SPEECH_ENABLED,
HCI_DUT_MODE,
HCI_VENDOR_DIAG,
@@ -1095,6 +1096,19 @@ struct hci_rp_read_inq_rsp_tx_power {
__s8 tx_power;
} __packed;
+#define HCI_OP_READ_DEF_ERR_DATA_REPORTING 0x0c5a
+ #define ERR_DATA_REPORTING_DISABLED 0x00
+ #define ERR_DATA_REPORTING_ENABLED 0x01
+struct hci_rp_read_def_err_data_reporting {
+ __u8 status;
+ __u8 err_data_reporting;
+} __packed;
+
+#define HCI_OP_WRITE_DEF_ERR_DATA_REPORTING 0x0c5b
+struct hci_cp_write_def_err_data_reporting {
+ __u8 err_data_reporting;
+} __packed;
+
#define HCI_OP_SET_EVENT_MASK_PAGE_2 0x0c63
#define HCI_OP_READ_LOCATION_DATA 0x0c64
diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index dcc0dc6e2624..c498ac113930 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -260,6 +260,7 @@ struct hci_dev {
__u8 stored_num_keys;
__u8 io_capability;
__s8 inq_tx_power;
+ __u8 err_data_reporting;
__u16 page_scan_interval;
__u16 page_scan_window;
__u8 page_scan_type;
diff --git a/include/net/bluetooth/mgmt.h b/include/net/bluetooth/mgmt.h
index f69f88e8e109..f41cd87550dc 100644
--- a/include/net/bluetooth/mgmt.h
+++ b/include/net/bluetooth/mgmt.h
@@ -102,7 +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_SETTING_WIDEBAND_SPEECH 0x00020000
#define MGMT_OP_READ_INFO 0x0004
#define MGMT_READ_INFO_SIZE 0
@@ -672,6 +672,8 @@ struct mgmt_cp_set_blocked_keys {
} __packed;
#define MGMT_OP_SET_BLOCKED_KEYS_SIZE 2
+#define MGMT_OP_SET_WIDEBAND_SPEECH 0x0047
+
#define MGMT_EV_CMD_COMPLETE 0x0001
struct mgmt_ev_cmd_complete {
__le16 opcode;
diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index 4e6d61a95b20..ef1160c1f9d0 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -603,6 +603,9 @@ static int hci_init3_req(struct hci_request *req, unsigned long opt)
if (hdev->commands[8] & 0x01)
hci_req_add(req, HCI_OP_READ_PAGE_SCAN_ACTIVITY, 0, NULL);
+ if (hdev->commands[18] & 0x02)
+ hci_req_add(req, HCI_OP_READ_DEF_ERR_DATA_REPORTING, 0, NULL);
+
/* Some older Broadcom based Bluetooth 1.2 controllers do not
* support the Read Page Scan Type command. Check support for
* this command in the bit mask of supported commands.
@@ -838,6 +841,27 @@ static int hci_init4_req(struct hci_request *req, unsigned long opt)
sizeof(support), &support);
}
+ /* Set erroneous data reporting if supported to the wideband speech
+ * setting value
+ */
+ if (hdev->commands[18] & 0x04) {
+ bool enabled = hci_dev_test_flag(hdev,
+ HCI_WIDEBAND_SPEECH_ENABLED);
+
+ if (enabled !=
+ (hdev->err_data_reporting == ERR_DATA_REPORTING_ENABLED)) {
+ struct hci_cp_write_def_err_data_reporting cp = {};
+
+ cp.err_data_reporting =
+ enabled ?
+ ERR_DATA_REPORTING_ENABLED :
+ ERR_DATA_REPORTING_DISABLED;
+
+ hci_req_add(req, HCI_OP_WRITE_DEF_ERR_DATA_REPORTING,
+ sizeof(cp), &cp);
+ }
+ }
+
/* Set Suggested Default Data Length to maximum if supported */
if (hdev->le_features[0] & HCI_LE_DATA_LEN_EXT) {
struct hci_cp_le_write_def_data_len cp;
diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index 591e7477e925..3772acddda93 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -901,6 +901,37 @@ static void hci_cc_read_inq_rsp_tx_power(struct hci_dev *hdev,
hdev->inq_tx_power = rp->tx_power;
}
+static void hci_cc_read_def_err_data_reporting(struct hci_dev *hdev,
+ struct sk_buff *skb)
+{
+ struct hci_rp_read_def_err_data_reporting *rp = (void *)skb->data;
+
+ BT_DBG("%s status 0x%2.2x", hdev->name, rp->status);
+
+ if (rp->status)
+ return;
+
+ hdev->err_data_reporting = rp->err_data_reporting;
+}
+
+static void hci_cc_write_def_err_data_reporting(struct hci_dev *hdev,
+ struct sk_buff *skb)
+{
+ __u8 status = *((__u8 *)skb->data);
+ struct hci_cp_write_def_err_data_reporting *cp;
+
+ BT_DBG("%s status 0x%2.2x", hdev->name, status);
+
+ if (status)
+ return;
+
+ cp = hci_sent_cmd_data(hdev, HCI_OP_WRITE_DEF_ERR_DATA_REPORTING);
+ if (!cp)
+ return;
+
+ hdev->err_data_reporting = cp->err_data_reporting;
+}
+
static void hci_cc_pin_code_reply(struct hci_dev *hdev, struct sk_buff *skb)
{
struct hci_rp_pin_code_reply *rp = (void *) skb->data;
@@ -3302,6 +3333,14 @@ static void hci_cmd_complete_evt(struct hci_dev *hdev, struct sk_buff *skb,
hci_cc_read_inq_rsp_tx_power(hdev, skb);
break;
+ case HCI_OP_READ_DEF_ERR_DATA_REPORTING:
+ hci_cc_read_def_err_data_reporting(hdev, skb);
+ break;
+
+ case HCI_OP_WRITE_DEF_ERR_DATA_REPORTING:
+ hci_cc_write_def_err_data_reporting(hdev, skb);
+ break;
+
case HCI_OP_PIN_CODE_REPLY:
hci_cc_pin_code_reply(hdev, skb);
break;
diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index 1002c657768a..8fbbb225caf8 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -107,6 +107,7 @@ static const u16 mgmt_commands[] = {
MGMT_OP_READ_EXT_INFO,
MGMT_OP_SET_APPEARANCE,
MGMT_OP_SET_BLOCKED_KEYS,
+ MGMT_OP_SET_WIDEBAND_SPEECH,
};
static const u16 mgmt_events[] = {
@@ -763,9 +764,9 @@ 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,
+ if (test_bit(HCI_QUIRK_WIDEBAND_SPEECH_SUPPORTED,
&hdev->quirks))
- settings |= MGMT_SETTING_WIDE_BAND_SPEECH;
+ settings |= MGMT_SETTING_WIDEBAND_SPEECH;
}
if (lmp_le_capable(hdev)) {
@@ -850,6 +851,9 @@ static u32 get_current_settings(struct hci_dev *hdev)
settings |= MGMT_SETTING_STATIC_ADDRESS;
}
+ if (hci_dev_test_flag(hdev, HCI_WIDEBAND_SPEECH_ENABLED))
+ settings |= MGMT_SETTING_WIDEBAND_SPEECH;
+
return settings;
}
@@ -3593,6 +3597,63 @@ static int set_blocked_keys(struct sock *sk, struct hci_dev *hdev, void *data,
err, NULL, 0);
}
+static int set_wideband_speech(struct sock *sk, struct hci_dev *hdev,
+ void *data, u16 len)
+{
+ struct mgmt_mode *cp = data;
+ int err;
+ bool changed = false;
+
+ BT_DBG("request for %s", hdev->name);
+
+ if (!test_bit(HCI_QUIRK_WIDEBAND_SPEECH_SUPPORTED, &hdev->quirks))
+ return mgmt_cmd_status(sk, hdev->id,
+ MGMT_OP_SET_WIDEBAND_SPEECH,
+ MGMT_STATUS_NOT_SUPPORTED);
+
+ if (cp->val != 0x00 && cp->val != 0x01)
+ return mgmt_cmd_status(sk, hdev->id,
+ MGMT_OP_SET_WIDEBAND_SPEECH,
+ MGMT_STATUS_INVALID_PARAMS);
+
+ hci_dev_lock(hdev);
+
+ if (pending_find(MGMT_OP_SET_WIDEBAND_SPEECH, hdev)) {
+ err = mgmt_cmd_status(sk, hdev->id,
+ MGMT_OP_SET_WIDEBAND_SPEECH,
+ MGMT_STATUS_BUSY);
+ goto unlock;
+ }
+
+ if (hdev_is_powered(hdev) &&
+ !!cp->val != hci_dev_test_flag(hdev,
+ HCI_WIDEBAND_SPEECH_ENABLED)) {
+ err = mgmt_cmd_status(sk, hdev->id,
+ MGMT_OP_SET_WIDEBAND_SPEECH,
+ MGMT_STATUS_REJECTED);
+ goto unlock;
+ }
+
+ if (cp->val)
+ changed = !hci_dev_test_and_set_flag(hdev,
+ HCI_WIDEBAND_SPEECH_ENABLED);
+ else
+ changed = hci_dev_test_and_clear_flag(hdev,
+ HCI_WIDEBAND_SPEECH_ENABLED);
+
+ err = send_settings_rsp(sk, MGMT_OP_SET_WIDEBAND_SPEECH, hdev);
+ if (err < 0)
+ goto unlock;
+
+ if (changed)
+ err = new_settings(hdev, sk);
+
+unlock:
+
+ hci_dev_unlock(hdev);
+ return err;
+}
+
static void read_local_oob_data_complete(struct hci_dev *hdev, u8 status,
u16 opcode, struct sk_buff *skb)
{
@@ -6994,6 +7055,7 @@ static const struct hci_mgmt_handler mgmt_handlers[] = {
{ set_phy_configuration, MGMT_SET_PHY_CONFIGURATION_SIZE },
{ set_blocked_keys, MGMT_OP_SET_BLOCKED_KEYS_SIZE,
HCI_MGMT_VAR_LEN },
+ { set_wideband_speech, MGMT_SETTING_SIZE },
};
void mgmt_index_added(struct hci_dev *hdev)
--
2.25.1.481.gfbce0eb801-goog
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH v4] bluetooth: Enable erroneous data reporting if wbs is supported
2020-03-05 16:14 [PATCH v4] bluetooth: Enable erroneous data reporting if wbs is supported Alain Michaud
@ 2020-03-08 7:53 ` Marcel Holtmann
2020-03-10 23:10 ` Luiz Augusto von Dentz
1 sibling, 0 replies; 4+ messages in thread
From: Marcel Holtmann @ 2020-03-08 7:53 UTC (permalink / raw)
To: Alain Michaud; +Cc: linux-bluetooth
Hi Alain,
> This change introduces a wide band speech setting which allows higher
> level clients to query the local controller support for wide band speech
> as well as set the setting state when the radio is powered off.
> Internally, this setting controls if erroneous data reporting is enabled
> on the controller.
>
> Signed-off-by: Alain Michaud <alainm@chromium.org>
> ---
>
> drivers/bluetooth/btusb.c | 2 +-
> include/net/bluetooth/hci.h | 16 +++++++-
> include/net/bluetooth/hci_core.h | 1 +
> include/net/bluetooth/mgmt.h | 4 +-
> net/bluetooth/hci_core.c | 24 ++++++++++++
> net/bluetooth/hci_event.c | 39 +++++++++++++++++++
> net/bluetooth/mgmt.c | 66 +++++++++++++++++++++++++++++++-
> 7 files changed, 147 insertions(+), 5 deletions(-)
patch has been applied to bluetooth-next tree.
However I fixed a few minor style issues as well.
Regards
Marcel
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v4] bluetooth: Enable erroneous data reporting if wbs is supported
2020-03-05 16:14 [PATCH v4] bluetooth: Enable erroneous data reporting if wbs is supported Alain Michaud
2020-03-08 7:53 ` Marcel Holtmann
@ 2020-03-10 23:10 ` Luiz Augusto von Dentz
2020-03-11 14:10 ` Alain Michaud
1 sibling, 1 reply; 4+ messages in thread
From: Luiz Augusto von Dentz @ 2020-03-10 23:10 UTC (permalink / raw)
To: Alain Michaud; +Cc: linux-bluetooth
Hi Alain,
On Thu, Mar 5, 2020 at 8:16 AM Alain Michaud <alainm@chromium.org> wrote:
>
> This change introduces a wide band speech setting which allows higher
> level clients to query the local controller support for wide band speech
> as well as set the setting state when the radio is powered off.
> Internally, this setting controls if erroneous data reporting is enabled
> on the controller.
>
> Signed-off-by: Alain Michaud <alainm@chromium.org>
> ---
>
> drivers/bluetooth/btusb.c | 2 +-
> include/net/bluetooth/hci.h | 16 +++++++-
> include/net/bluetooth/hci_core.h | 1 +
> include/net/bluetooth/mgmt.h | 4 +-
> net/bluetooth/hci_core.c | 24 ++++++++++++
> net/bluetooth/hci_event.c | 39 +++++++++++++++++++
> net/bluetooth/mgmt.c | 66 +++++++++++++++++++++++++++++++-
> 7 files changed, 147 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
> index 48e78fdc8e83..4b12c34f0b22 100644
> --- a/drivers/bluetooth/btusb.c
> +++ b/drivers/bluetooth/btusb.c
> @@ -3868,7 +3868,7 @@ static int btusb_probe(struct usb_interface *intf,
> data->isoc = NULL;
>
> if (id->driver_info & BTUSB_WIDEBAND_SPEECH)
> - set_bit(HCI_QUIRK_WIDE_BAND_SPEECH_SUPPORTED, &hdev->quirks);
> + set_bit(HCI_QUIRK_WIDEBAND_SPEECH_SUPPORTED, &hdev->quirks);
>
> if (id->driver_info & BTUSB_DIGIANSWER) {
> data->cmdreq_type = USB_TYPE_VENDOR;
> diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
> index 0b3ebd35681d..4e86f1bb7a87 100644
> --- a/include/net/bluetooth/hci.h
> +++ b/include/net/bluetooth/hci.h
> @@ -213,7 +213,7 @@ enum {
> *
> * This quirk must be set before hci_register_dev is called.
> */
> - HCI_QUIRK_WIDE_BAND_SPEECH_SUPPORTED,
> + HCI_QUIRK_WIDEBAND_SPEECH_SUPPORTED,
> };
>
> /* HCI device flags */
> @@ -286,6 +286,7 @@ enum {
> HCI_FAST_CONNECTABLE,
> HCI_BREDR_ENABLED,
> HCI_LE_SCAN_INTERRUPTED,
> + HCI_WIDEBAND_SPEECH_ENABLED,
>
> HCI_DUT_MODE,
> HCI_VENDOR_DIAG,
> @@ -1095,6 +1096,19 @@ struct hci_rp_read_inq_rsp_tx_power {
> __s8 tx_power;
> } __packed;
>
> +#define HCI_OP_READ_DEF_ERR_DATA_REPORTING 0x0c5a
> + #define ERR_DATA_REPORTING_DISABLED 0x00
> + #define ERR_DATA_REPORTING_ENABLED 0x01
> +struct hci_rp_read_def_err_data_reporting {
> + __u8 status;
> + __u8 err_data_reporting;
> +} __packed;
> +
> +#define HCI_OP_WRITE_DEF_ERR_DATA_REPORTING 0x0c5b
> +struct hci_cp_write_def_err_data_reporting {
> + __u8 err_data_reporting;
> +} __packed;
> +
> #define HCI_OP_SET_EVENT_MASK_PAGE_2 0x0c63
>
> #define HCI_OP_READ_LOCATION_DATA 0x0c64
> diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
> index dcc0dc6e2624..c498ac113930 100644
> --- a/include/net/bluetooth/hci_core.h
> +++ b/include/net/bluetooth/hci_core.h
> @@ -260,6 +260,7 @@ struct hci_dev {
> __u8 stored_num_keys;
> __u8 io_capability;
> __s8 inq_tx_power;
> + __u8 err_data_reporting;
> __u16 page_scan_interval;
> __u16 page_scan_window;
> __u8 page_scan_type;
> diff --git a/include/net/bluetooth/mgmt.h b/include/net/bluetooth/mgmt.h
> index f69f88e8e109..f41cd87550dc 100644
> --- a/include/net/bluetooth/mgmt.h
> +++ b/include/net/bluetooth/mgmt.h
> @@ -102,7 +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_SETTING_WIDEBAND_SPEECH 0x00020000
>
> #define MGMT_OP_READ_INFO 0x0004
> #define MGMT_READ_INFO_SIZE 0
> @@ -672,6 +672,8 @@ struct mgmt_cp_set_blocked_keys {
> } __packed;
> #define MGMT_OP_SET_BLOCKED_KEYS_SIZE 2
>
> +#define MGMT_OP_SET_WIDEBAND_SPEECH 0x0047
> +
> #define MGMT_EV_CMD_COMPLETE 0x0001
> struct mgmt_ev_cmd_complete {
> __le16 opcode;
> diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
> index 4e6d61a95b20..ef1160c1f9d0 100644
> --- a/net/bluetooth/hci_core.c
> +++ b/net/bluetooth/hci_core.c
> @@ -603,6 +603,9 @@ static int hci_init3_req(struct hci_request *req, unsigned long opt)
> if (hdev->commands[8] & 0x01)
> hci_req_add(req, HCI_OP_READ_PAGE_SCAN_ACTIVITY, 0, NULL);
>
> + if (hdev->commands[18] & 0x02)
> + hci_req_add(req, HCI_OP_READ_DEF_ERR_DATA_REPORTING, 0, NULL);
Is bit above correct? It looks like this is causing a problem on the emulator:
Basic L2CAP Socket - Success - init
Read Index List callback
Status: 0x00
New hciemu instance created
Unsupported command 0x0c5a
On the emulator we have:
https://git.kernel.org/pub/scm/bluetooth/bluez.git/tree/emulator/btdev.c#n409
btdev->commands[18] |= 0x02; /* Write Inquiry Response TX Power */
I assume the above is correct, I guess it would be the bit at 0x04 like bellow.
> +
> /* Some older Broadcom based Bluetooth 1.2 controllers do not
> * support the Read Page Scan Type command. Check support for
> * this command in the bit mask of supported commands.
> @@ -838,6 +841,27 @@ static int hci_init4_req(struct hci_request *req, unsigned long opt)
> sizeof(support), &support);
> }
>
> + /* Set erroneous data reporting if supported to the wideband speech
> + * setting value
> + */
> + if (hdev->commands[18] & 0x04) {
> + bool enabled = hci_dev_test_flag(hdev,
> + HCI_WIDEBAND_SPEECH_ENABLED);
> +
> + if (enabled !=
> + (hdev->err_data_reporting == ERR_DATA_REPORTING_ENABLED)) {
> + struct hci_cp_write_def_err_data_reporting cp = {};
> +
> + cp.err_data_reporting =
> + enabled ?
> + ERR_DATA_REPORTING_ENABLED :
> + ERR_DATA_REPORTING_DISABLED;
> +
> + hci_req_add(req, HCI_OP_WRITE_DEF_ERR_DATA_REPORTING,
> + sizeof(cp), &cp);
This would be adding it again? Or the first instance was not suppose
to be there?
> + }
> + }
> +
> /* Set Suggested Default Data Length to maximum if supported */
> if (hdev->le_features[0] & HCI_LE_DATA_LEN_EXT) {
> struct hci_cp_le_write_def_data_len cp;
> diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
> index 591e7477e925..3772acddda93 100644
> --- a/net/bluetooth/hci_event.c
> +++ b/net/bluetooth/hci_event.c
> @@ -901,6 +901,37 @@ static void hci_cc_read_inq_rsp_tx_power(struct hci_dev *hdev,
> hdev->inq_tx_power = rp->tx_power;
> }
>
> +static void hci_cc_read_def_err_data_reporting(struct hci_dev *hdev,
> + struct sk_buff *skb)
> +{
> + struct hci_rp_read_def_err_data_reporting *rp = (void *)skb->data;
> +
> + BT_DBG("%s status 0x%2.2x", hdev->name, rp->status);
> +
> + if (rp->status)
> + return;
> +
> + hdev->err_data_reporting = rp->err_data_reporting;
> +}
> +
> +static void hci_cc_write_def_err_data_reporting(struct hci_dev *hdev,
> + struct sk_buff *skb)
> +{
> + __u8 status = *((__u8 *)skb->data);
> + struct hci_cp_write_def_err_data_reporting *cp;
> +
> + BT_DBG("%s status 0x%2.2x", hdev->name, status);
> +
> + if (status)
> + return;
> +
> + cp = hci_sent_cmd_data(hdev, HCI_OP_WRITE_DEF_ERR_DATA_REPORTING);
> + if (!cp)
> + return;
> +
> + hdev->err_data_reporting = cp->err_data_reporting;
> +}
> +
> static void hci_cc_pin_code_reply(struct hci_dev *hdev, struct sk_buff *skb)
> {
> struct hci_rp_pin_code_reply *rp = (void *) skb->data;
> @@ -3302,6 +3333,14 @@ static void hci_cmd_complete_evt(struct hci_dev *hdev, struct sk_buff *skb,
> hci_cc_read_inq_rsp_tx_power(hdev, skb);
> break;
>
> + case HCI_OP_READ_DEF_ERR_DATA_REPORTING:
> + hci_cc_read_def_err_data_reporting(hdev, skb);
> + break;
> +
> + case HCI_OP_WRITE_DEF_ERR_DATA_REPORTING:
> + hci_cc_write_def_err_data_reporting(hdev, skb);
> + break;
> +
> case HCI_OP_PIN_CODE_REPLY:
> hci_cc_pin_code_reply(hdev, skb);
> break;
> diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
> index 1002c657768a..8fbbb225caf8 100644
> --- a/net/bluetooth/mgmt.c
> +++ b/net/bluetooth/mgmt.c
> @@ -107,6 +107,7 @@ static const u16 mgmt_commands[] = {
> MGMT_OP_READ_EXT_INFO,
> MGMT_OP_SET_APPEARANCE,
> MGMT_OP_SET_BLOCKED_KEYS,
> + MGMT_OP_SET_WIDEBAND_SPEECH,
> };
>
> static const u16 mgmt_events[] = {
> @@ -763,9 +764,9 @@ 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,
> + if (test_bit(HCI_QUIRK_WIDEBAND_SPEECH_SUPPORTED,
> &hdev->quirks))
> - settings |= MGMT_SETTING_WIDE_BAND_SPEECH;
> + settings |= MGMT_SETTING_WIDEBAND_SPEECH;
> }
>
> if (lmp_le_capable(hdev)) {
> @@ -850,6 +851,9 @@ static u32 get_current_settings(struct hci_dev *hdev)
> settings |= MGMT_SETTING_STATIC_ADDRESS;
> }
>
> + if (hci_dev_test_flag(hdev, HCI_WIDEBAND_SPEECH_ENABLED))
> + settings |= MGMT_SETTING_WIDEBAND_SPEECH;
> +
> return settings;
> }
>
> @@ -3593,6 +3597,63 @@ static int set_blocked_keys(struct sock *sk, struct hci_dev *hdev, void *data,
> err, NULL, 0);
> }
>
> +static int set_wideband_speech(struct sock *sk, struct hci_dev *hdev,
> + void *data, u16 len)
> +{
> + struct mgmt_mode *cp = data;
> + int err;
> + bool changed = false;
> +
> + BT_DBG("request for %s", hdev->name);
> +
> + if (!test_bit(HCI_QUIRK_WIDEBAND_SPEECH_SUPPORTED, &hdev->quirks))
> + return mgmt_cmd_status(sk, hdev->id,
> + MGMT_OP_SET_WIDEBAND_SPEECH,
> + MGMT_STATUS_NOT_SUPPORTED);
> +
> + if (cp->val != 0x00 && cp->val != 0x01)
> + return mgmt_cmd_status(sk, hdev->id,
> + MGMT_OP_SET_WIDEBAND_SPEECH,
> + MGMT_STATUS_INVALID_PARAMS);
> +
> + hci_dev_lock(hdev);
> +
> + if (pending_find(MGMT_OP_SET_WIDEBAND_SPEECH, hdev)) {
> + err = mgmt_cmd_status(sk, hdev->id,
> + MGMT_OP_SET_WIDEBAND_SPEECH,
> + MGMT_STATUS_BUSY);
> + goto unlock;
> + }
> +
> + if (hdev_is_powered(hdev) &&
> + !!cp->val != hci_dev_test_flag(hdev,
> + HCI_WIDEBAND_SPEECH_ENABLED)) {
> + err = mgmt_cmd_status(sk, hdev->id,
> + MGMT_OP_SET_WIDEBAND_SPEECH,
> + MGMT_STATUS_REJECTED);
> + goto unlock;
> + }
> +
> + if (cp->val)
> + changed = !hci_dev_test_and_set_flag(hdev,
> + HCI_WIDEBAND_SPEECH_ENABLED);
> + else
> + changed = hci_dev_test_and_clear_flag(hdev,
> + HCI_WIDEBAND_SPEECH_ENABLED);
> +
> + err = send_settings_rsp(sk, MGMT_OP_SET_WIDEBAND_SPEECH, hdev);
> + if (err < 0)
> + goto unlock;
> +
> + if (changed)
> + err = new_settings(hdev, sk);
> +
> +unlock:
> +
> + hci_dev_unlock(hdev);
> + return err;
> +}
> +
> static void read_local_oob_data_complete(struct hci_dev *hdev, u8 status,
> u16 opcode, struct sk_buff *skb)
> {
> @@ -6994,6 +7055,7 @@ static const struct hci_mgmt_handler mgmt_handlers[] = {
> { set_phy_configuration, MGMT_SET_PHY_CONFIGURATION_SIZE },
> { set_blocked_keys, MGMT_OP_SET_BLOCKED_KEYS_SIZE,
> HCI_MGMT_VAR_LEN },
> + { set_wideband_speech, MGMT_SETTING_SIZE },
> };
>
> void mgmt_index_added(struct hci_dev *hdev)
> --
> 2.25.1.481.gfbce0eb801-goog
>
--
Luiz Augusto von Dentz
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v4] bluetooth: Enable erroneous data reporting if wbs is supported
2020-03-10 23:10 ` Luiz Augusto von Dentz
@ 2020-03-11 14:10 ` Alain Michaud
0 siblings, 0 replies; 4+ messages in thread
From: Alain Michaud @ 2020-03-11 14:10 UTC (permalink / raw)
To: Luiz Augusto von Dentz; +Cc: Alain Michaud, linux-bluetooth
Hi Luiz,
On Tue, Mar 10, 2020 at 7:10 PM Luiz Augusto von Dentz
<luiz.dentz@gmail.com> wrote:
>
> Hi Alain,
>
> On Thu, Mar 5, 2020 at 8:16 AM Alain Michaud <alainm@chromium.org> wrote:
> >
> > This change introduces a wide band speech setting which allows higher
> > level clients to query the local controller support for wide band speech
> > as well as set the setting state when the radio is powered off.
> > Internally, this setting controls if erroneous data reporting is enabled
> > on the controller.
> >
> > Signed-off-by: Alain Michaud <alainm@chromium.org>
> > ---
> >
> > drivers/bluetooth/btusb.c | 2 +-
> > include/net/bluetooth/hci.h | 16 +++++++-
> > include/net/bluetooth/hci_core.h | 1 +
> > include/net/bluetooth/mgmt.h | 4 +-
> > net/bluetooth/hci_core.c | 24 ++++++++++++
> > net/bluetooth/hci_event.c | 39 +++++++++++++++++++
> > net/bluetooth/mgmt.c | 66 +++++++++++++++++++++++++++++++-
> > 7 files changed, 147 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
> > index 48e78fdc8e83..4b12c34f0b22 100644
> > --- a/drivers/bluetooth/btusb.c
> > +++ b/drivers/bluetooth/btusb.c
> > @@ -3868,7 +3868,7 @@ static int btusb_probe(struct usb_interface *intf,
> > data->isoc = NULL;
> >
> > if (id->driver_info & BTUSB_WIDEBAND_SPEECH)
> > - set_bit(HCI_QUIRK_WIDE_BAND_SPEECH_SUPPORTED, &hdev->quirks);
> > + set_bit(HCI_QUIRK_WIDEBAND_SPEECH_SUPPORTED, &hdev->quirks);
> >
> > if (id->driver_info & BTUSB_DIGIANSWER) {
> > data->cmdreq_type = USB_TYPE_VENDOR;
> > diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
> > index 0b3ebd35681d..4e86f1bb7a87 100644
> > --- a/include/net/bluetooth/hci.h
> > +++ b/include/net/bluetooth/hci.h
> > @@ -213,7 +213,7 @@ enum {
> > *
> > * This quirk must be set before hci_register_dev is called.
> > */
> > - HCI_QUIRK_WIDE_BAND_SPEECH_SUPPORTED,
> > + HCI_QUIRK_WIDEBAND_SPEECH_SUPPORTED,
> > };
> >
> > /* HCI device flags */
> > @@ -286,6 +286,7 @@ enum {
> > HCI_FAST_CONNECTABLE,
> > HCI_BREDR_ENABLED,
> > HCI_LE_SCAN_INTERRUPTED,
> > + HCI_WIDEBAND_SPEECH_ENABLED,
> >
> > HCI_DUT_MODE,
> > HCI_VENDOR_DIAG,
> > @@ -1095,6 +1096,19 @@ struct hci_rp_read_inq_rsp_tx_power {
> > __s8 tx_power;
> > } __packed;
> >
> > +#define HCI_OP_READ_DEF_ERR_DATA_REPORTING 0x0c5a
> > + #define ERR_DATA_REPORTING_DISABLED 0x00
> > + #define ERR_DATA_REPORTING_ENABLED 0x01
> > +struct hci_rp_read_def_err_data_reporting {
> > + __u8 status;
> > + __u8 err_data_reporting;
> > +} __packed;
> > +
> > +#define HCI_OP_WRITE_DEF_ERR_DATA_REPORTING 0x0c5b
> > +struct hci_cp_write_def_err_data_reporting {
> > + __u8 err_data_reporting;
> > +} __packed;
> > +
> > #define HCI_OP_SET_EVENT_MASK_PAGE_2 0x0c63
> >
> > #define HCI_OP_READ_LOCATION_DATA 0x0c64
> > diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
> > index dcc0dc6e2624..c498ac113930 100644
> > --- a/include/net/bluetooth/hci_core.h
> > +++ b/include/net/bluetooth/hci_core.h
> > @@ -260,6 +260,7 @@ struct hci_dev {
> > __u8 stored_num_keys;
> > __u8 io_capability;
> > __s8 inq_tx_power;
> > + __u8 err_data_reporting;
> > __u16 page_scan_interval;
> > __u16 page_scan_window;
> > __u8 page_scan_type;
> > diff --git a/include/net/bluetooth/mgmt.h b/include/net/bluetooth/mgmt.h
> > index f69f88e8e109..f41cd87550dc 100644
> > --- a/include/net/bluetooth/mgmt.h
> > +++ b/include/net/bluetooth/mgmt.h
> > @@ -102,7 +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_SETTING_WIDEBAND_SPEECH 0x00020000
> >
> > #define MGMT_OP_READ_INFO 0x0004
> > #define MGMT_READ_INFO_SIZE 0
> > @@ -672,6 +672,8 @@ struct mgmt_cp_set_blocked_keys {
> > } __packed;
> > #define MGMT_OP_SET_BLOCKED_KEYS_SIZE 2
> >
> > +#define MGMT_OP_SET_WIDEBAND_SPEECH 0x0047
> > +
> > #define MGMT_EV_CMD_COMPLETE 0x0001
> > struct mgmt_ev_cmd_complete {
> > __le16 opcode;
> > diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
> > index 4e6d61a95b20..ef1160c1f9d0 100644
> > --- a/net/bluetooth/hci_core.c
> > +++ b/net/bluetooth/hci_core.c
> > @@ -603,6 +603,9 @@ static int hci_init3_req(struct hci_request *req, unsigned long opt)
> > if (hdev->commands[8] & 0x01)
> > hci_req_add(req, HCI_OP_READ_PAGE_SCAN_ACTIVITY, 0, NULL);
> >
> > + if (hdev->commands[18] & 0x02)
> > + hci_req_add(req, HCI_OP_READ_DEF_ERR_DATA_REPORTING, 0, NULL);
>
> Is bit above correct? It looks like this is causing a problem on the emulator:
>
> Basic L2CAP Socket - Success - init
> Read Index List callback
> Status: 0x00
> New hciemu instance created
> Unsupported command 0x0c5a
>
> On the emulator we have:
> https://git.kernel.org/pub/scm/bluetooth/bluez.git/tree/emulator/btdev.c#n409
>
> btdev->commands[18] |= 0x02; /* Write Inquiry Response TX Power */
>
> I assume the above is correct, I guess it would be the bit at 0x04 like bellow.
>
> > +
> > /* Some older Broadcom based Bluetooth 1.2 controllers do not
> > * support the Read Page Scan Type command. Check support for
> > * this command in the bit mask of supported commands.
> > @@ -838,6 +841,27 @@ static int hci_init4_req(struct hci_request *req, unsigned long opt)
> > sizeof(support), &support);
> > }
> >
> > + /* Set erroneous data reporting if supported to the wideband speech
> > + * setting value
> > + */
> > + if (hdev->commands[18] & 0x04) {
> > + bool enabled = hci_dev_test_flag(hdev,
> > + HCI_WIDEBAND_SPEECH_ENABLED);
> > +
> > + if (enabled !=
> > + (hdev->err_data_reporting == ERR_DATA_REPORTING_ENABLED)) {
> > + struct hci_cp_write_def_err_data_reporting cp = {};
> > +
> > + cp.err_data_reporting =
> > + enabled ?
> > + ERR_DATA_REPORTING_ENABLED :
> > + ERR_DATA_REPORTING_DISABLED;
> > +
> > + hci_req_add(req, HCI_OP_WRITE_DEF_ERR_DATA_REPORTING,
> > + sizeof(cp), &cp);
>
> This would be adding it again? Or the first instance was not suppose
> to be there?
Indeed, I was off by one for the read/write commands, should have been
0x04 and 0x08. on a device that really don't support these commands
(like the emulator?) that could indeed be problematic. I'll send a
patch to fix these.
>
> > + }
> > + }
> > +
> > /* Set Suggested Default Data Length to maximum if supported */
> > if (hdev->le_features[0] & HCI_LE_DATA_LEN_EXT) {
> > struct hci_cp_le_write_def_data_len cp;
> > diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
> > index 591e7477e925..3772acddda93 100644
> > --- a/net/bluetooth/hci_event.c
> > +++ b/net/bluetooth/hci_event.c
> > @@ -901,6 +901,37 @@ static void hci_cc_read_inq_rsp_tx_power(struct hci_dev *hdev,
> > hdev->inq_tx_power = rp->tx_power;
> > }
> >
> > +static void hci_cc_read_def_err_data_reporting(struct hci_dev *hdev,
> > + struct sk_buff *skb)
> > +{
> > + struct hci_rp_read_def_err_data_reporting *rp = (void *)skb->data;
> > +
> > + BT_DBG("%s status 0x%2.2x", hdev->name, rp->status);
> > +
> > + if (rp->status)
> > + return;
> > +
> > + hdev->err_data_reporting = rp->err_data_reporting;
> > +}
> > +
> > +static void hci_cc_write_def_err_data_reporting(struct hci_dev *hdev,
> > + struct sk_buff *skb)
> > +{
> > + __u8 status = *((__u8 *)skb->data);
> > + struct hci_cp_write_def_err_data_reporting *cp;
> > +
> > + BT_DBG("%s status 0x%2.2x", hdev->name, status);
> > +
> > + if (status)
> > + return;
> > +
> > + cp = hci_sent_cmd_data(hdev, HCI_OP_WRITE_DEF_ERR_DATA_REPORTING);
> > + if (!cp)
> > + return;
> > +
> > + hdev->err_data_reporting = cp->err_data_reporting;
> > +}
> > +
> > static void hci_cc_pin_code_reply(struct hci_dev *hdev, struct sk_buff *skb)
> > {
> > struct hci_rp_pin_code_reply *rp = (void *) skb->data;
> > @@ -3302,6 +3333,14 @@ static void hci_cmd_complete_evt(struct hci_dev *hdev, struct sk_buff *skb,
> > hci_cc_read_inq_rsp_tx_power(hdev, skb);
> > break;
> >
> > + case HCI_OP_READ_DEF_ERR_DATA_REPORTING:
> > + hci_cc_read_def_err_data_reporting(hdev, skb);
> > + break;
> > +
> > + case HCI_OP_WRITE_DEF_ERR_DATA_REPORTING:
> > + hci_cc_write_def_err_data_reporting(hdev, skb);
> > + break;
> > +
> > case HCI_OP_PIN_CODE_REPLY:
> > hci_cc_pin_code_reply(hdev, skb);
> > break;
> > diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
> > index 1002c657768a..8fbbb225caf8 100644
> > --- a/net/bluetooth/mgmt.c
> > +++ b/net/bluetooth/mgmt.c
> > @@ -107,6 +107,7 @@ static const u16 mgmt_commands[] = {
> > MGMT_OP_READ_EXT_INFO,
> > MGMT_OP_SET_APPEARANCE,
> > MGMT_OP_SET_BLOCKED_KEYS,
> > + MGMT_OP_SET_WIDEBAND_SPEECH,
> > };
> >
> > static const u16 mgmt_events[] = {
> > @@ -763,9 +764,9 @@ 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,
> > + if (test_bit(HCI_QUIRK_WIDEBAND_SPEECH_SUPPORTED,
> > &hdev->quirks))
> > - settings |= MGMT_SETTING_WIDE_BAND_SPEECH;
> > + settings |= MGMT_SETTING_WIDEBAND_SPEECH;
> > }
> >
> > if (lmp_le_capable(hdev)) {
> > @@ -850,6 +851,9 @@ static u32 get_current_settings(struct hci_dev *hdev)
> > settings |= MGMT_SETTING_STATIC_ADDRESS;
> > }
> >
> > + if (hci_dev_test_flag(hdev, HCI_WIDEBAND_SPEECH_ENABLED))
> > + settings |= MGMT_SETTING_WIDEBAND_SPEECH;
> > +
> > return settings;
> > }
> >
> > @@ -3593,6 +3597,63 @@ static int set_blocked_keys(struct sock *sk, struct hci_dev *hdev, void *data,
> > err, NULL, 0);
> > }
> >
> > +static int set_wideband_speech(struct sock *sk, struct hci_dev *hdev,
> > + void *data, u16 len)
> > +{
> > + struct mgmt_mode *cp = data;
> > + int err;
> > + bool changed = false;
> > +
> > + BT_DBG("request for %s", hdev->name);
> > +
> > + if (!test_bit(HCI_QUIRK_WIDEBAND_SPEECH_SUPPORTED, &hdev->quirks))
> > + return mgmt_cmd_status(sk, hdev->id,
> > + MGMT_OP_SET_WIDEBAND_SPEECH,
> > + MGMT_STATUS_NOT_SUPPORTED);
> > +
> > + if (cp->val != 0x00 && cp->val != 0x01)
> > + return mgmt_cmd_status(sk, hdev->id,
> > + MGMT_OP_SET_WIDEBAND_SPEECH,
> > + MGMT_STATUS_INVALID_PARAMS);
> > +
> > + hci_dev_lock(hdev);
> > +
> > + if (pending_find(MGMT_OP_SET_WIDEBAND_SPEECH, hdev)) {
> > + err = mgmt_cmd_status(sk, hdev->id,
> > + MGMT_OP_SET_WIDEBAND_SPEECH,
> > + MGMT_STATUS_BUSY);
> > + goto unlock;
> > + }
> > +
> > + if (hdev_is_powered(hdev) &&
> > + !!cp->val != hci_dev_test_flag(hdev,
> > + HCI_WIDEBAND_SPEECH_ENABLED)) {
> > + err = mgmt_cmd_status(sk, hdev->id,
> > + MGMT_OP_SET_WIDEBAND_SPEECH,
> > + MGMT_STATUS_REJECTED);
> > + goto unlock;
> > + }
> > +
> > + if (cp->val)
> > + changed = !hci_dev_test_and_set_flag(hdev,
> > + HCI_WIDEBAND_SPEECH_ENABLED);
> > + else
> > + changed = hci_dev_test_and_clear_flag(hdev,
> > + HCI_WIDEBAND_SPEECH_ENABLED);
> > +
> > + err = send_settings_rsp(sk, MGMT_OP_SET_WIDEBAND_SPEECH, hdev);
> > + if (err < 0)
> > + goto unlock;
> > +
> > + if (changed)
> > + err = new_settings(hdev, sk);
> > +
> > +unlock:
> > +
> > + hci_dev_unlock(hdev);
> > + return err;
> > +}
> > +
> > static void read_local_oob_data_complete(struct hci_dev *hdev, u8 status,
> > u16 opcode, struct sk_buff *skb)
> > {
> > @@ -6994,6 +7055,7 @@ static const struct hci_mgmt_handler mgmt_handlers[] = {
> > { set_phy_configuration, MGMT_SET_PHY_CONFIGURATION_SIZE },
> > { set_blocked_keys, MGMT_OP_SET_BLOCKED_KEYS_SIZE,
> > HCI_MGMT_VAR_LEN },
> > + { set_wideband_speech, MGMT_SETTING_SIZE },
> > };
> >
> > void mgmt_index_added(struct hci_dev *hdev)
> > --
> > 2.25.1.481.gfbce0eb801-goog
> >
>
>
> --
> Luiz Augusto von Dentz
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2020-03-11 14:10 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-03-05 16:14 [PATCH v4] bluetooth: Enable erroneous data reporting if wbs is supported Alain Michaud
2020-03-08 7:53 ` Marcel Holtmann
2020-03-10 23:10 ` Luiz Augusto von Dentz
2020-03-11 14:10 ` Alain Michaud
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.