[PATCH v9 1/5] Bluetooth: btusb: disable Intel link statistics telemetry events

[PATCH v9 1/5] Bluetooth: btusb: disable Intel link statistics telemetry events

[Joseph Hwang]

To avoid the overhead on both the controller and the host, the
Intel link statistics telemetry events are disabled by default.

Reviewed-by: Miao-chen Chou <mcchou@chromium.org>
Signed-off-by: Chethan T N <chethan.tumkur.narayan@intel.com>
Signed-off-by: Kiran K <kiran.k@intel.com>
Signed-off-by: Joseph Hwang <josephsih@chromium.org>
---

Changes in v9:
- This version fixes the compile errors of patch 3/5.

Changes in v8:
- This version adds a new patch which refactors the set_exp_feature
  function with a feature table.
- Swap the patches per the comments on v7.
- Remove the unsuitable debug messages.
- This patch is not changed in this version.

Changes in v7:
- Rebase on Tedd's patches that moved functionality from btusb to
  btintel.

Changes in v6:
- Rebase on the latest commit.

Changes in v5:
- Rebase this patch 1/4 to resolve conflicts.
- There are changes in patches 3/4 and 4/4.

Changes in v4:
- The original 2 patches in Series-version 3 are split into
  2 patches from each patch per reviewers' comments. There are
  A total of 4 patches in this series now.
- The callback function is renamed from hdev->set_vs_dbg_evt to
  hdev->set_quality_report. Note that there are two different
  specifications which will be integrated soon and enabled/disabled
  with the same callback. One is Android Bluetooth Quality Report
  (BQR), and the other Intel link statistics telemetry events here.
  While most Bluetooth controller vendors have supported or are
  supporting the Android specification in their controllers, it looks
  making sense to use set_quality_report as the callback name.
- Similarly, the config option BT_FEATURE_VS_DBG_EVT is renamed as
  BT_FEATURE_QUALITY_REPORT which depends on BT now.
- The BQR is controller specific. There needs to be a valid hdev in the
  first place. This is fixed in set_exp_feature().
- In set_exp_feature(), bluez will only set experimental feature to set
  BQR when the feature is supported. Please refer to bluez CLs.
- Also refer to bluez patches for the decoding support of btmon.

Changes in v3:
- fix the long line in the commit message

Changes in v2:
- take care of intel_newgen as well as intel_new
- fix the long lines in mgmt.c

 drivers/bluetooth/btintel.c | 20 --------------------
 1 file changed, 20 deletions(-)

diff --git a/drivers/bluetooth/btintel.c b/drivers/bluetooth/btintel.c
index f1705b46fc88..0fe093fa5158 100644
--- a/drivers/bluetooth/btintel.c
+++ b/drivers/bluetooth/btintel.c
@@ -1893,7 +1893,6 @@ static int btintel_bootloader_setup(struct hci_dev *hdev,
 	u32 boot_param;
 	char ddcname[64];
 	int err;
-	struct intel_debug_features features;
 
 	BT_DBG("%s", hdev->name);
 
@@ -1934,15 +1933,6 @@ static int btintel_bootloader_setup(struct hci_dev *hdev,
 		btintel_load_ddc_config(hdev, ddcname);
 	}
 
-	/* Read the Intel supported features and if new exception formats
-	 * supported, need to load the additional DDC config to enable.
-	 */
-	err = btintel_read_debug_features(hdev, &features);
-	if (!err) {
-		/* Set DDC mask for available debug features */
-		btintel_set_debug_features(hdev, &features);
-	}
-
 	/* Read the Intel version information after loading the FW  */
 	err = btintel_read_version(hdev, &new_ver);
 	if (err)
@@ -2089,7 +2079,6 @@ static int btintel_bootloader_setup_tlv(struct hci_dev *hdev,
 	u32 boot_param;
 	char ddcname[64];
 	int err;
-	struct intel_debug_features features;
 	struct intel_version_tlv new_ver;
 
 	bt_dev_dbg(hdev, "");
@@ -2125,15 +2114,6 @@ static int btintel_bootloader_setup_tlv(struct hci_dev *hdev,
 	 */
 	btintel_load_ddc_config(hdev, ddcname);
 
-	/* Read the Intel supported features and if new exception formats
-	 * supported, need to load the additional DDC config to enable.
-	 */
-	err = btintel_read_debug_features(hdev, &features);
-	if (!err) {
-		/* Set DDC mask for available debug features */
-		btintel_set_debug_features(hdev, &features);
-	}
-
 	/* Read the Intel version information after loading the FW  */
 	err = btintel_read_version_tlv(hdev, &new_ver);
 	if (err)
-- 
2.33.0.rc1.237.g0d66db33f3-goog

[PATCH v9 2/5] Bluetooth: btintel: support link statistics telemetry events

[Joseph Hwang]

From: Chethan T N <chethan.tumkur.narayan@intel.com>

This patch supports the link statistics telemetry events for
intel controllers

Reviewed-by: Miao-chen Chou <mcchou@chromium.org>
Signed-off-by: Chethan T N <chethan.tumkur.narayan@intel.com>
Signed-off-by: Kiran K <kiran.k@intel.com>
Signed-off-by: Joseph Hwang <josephsih@chromium.org>
---

(no changes since v7)

Changes in v7:
- Rebase on Tedd's patches that moved functionality from btusb to
  btintel.

 drivers/bluetooth/btintel.c | 20 +++++++++++++++++++-
 1 file changed, 19 insertions(+), 1 deletion(-)

diff --git a/drivers/bluetooth/btintel.c b/drivers/bluetooth/btintel.c
index 0fe093fa5158..643e2194ca01 100644
--- a/drivers/bluetooth/btintel.c
+++ b/drivers/bluetooth/btintel.c
@@ -1285,8 +1285,10 @@ static int btintel_read_debug_features(struct hci_dev *hdev,
 static int btintel_set_debug_features(struct hci_dev *hdev,
 			       const struct intel_debug_features *features)
 {
-	u8 mask[11] = { 0x0a, 0x92, 0x02, 0x07, 0x00, 0x00, 0x00, 0x00,
+	u8 mask[11] = { 0x0a, 0x92, 0x02, 0x7f, 0x00, 0x00, 0x00, 0x00,
 			0x00, 0x00, 0x00 };
+	u8 period[5] = { 0x04, 0x91, 0x02, 0x05, 0x00 };
+	u8 trace_enable = 0x02;
 	struct sk_buff *skb;
 
 	if (!features)
@@ -1303,8 +1305,24 @@ static int btintel_set_debug_features(struct hci_dev *hdev,
 			   PTR_ERR(skb));
 		return PTR_ERR(skb);
 	}
+	kfree_skb(skb);
+
+	skb = __hci_cmd_sync(hdev, 0xfc8b, 5, period, HCI_INIT_TIMEOUT);
+	if (IS_ERR(skb)) {
+		bt_dev_err(hdev, "Setting periodicity for link statistics traces failed (%ld)",
+			   PTR_ERR(skb));
+		return PTR_ERR(skb);
+	}
+	kfree_skb(skb);
 
+	skb = __hci_cmd_sync(hdev, 0xfca1, 1, &trace_enable, HCI_INIT_TIMEOUT);
+	if (IS_ERR(skb)) {
+		bt_dev_err(hdev, "Enable tracing of link statistics events failed (%ld)",
+			   PTR_ERR(skb));
+		return PTR_ERR(skb);
+	}
 	kfree_skb(skb);
+
 	return 0;
 }
 
-- 
2.33.0.rc1.237.g0d66db33f3-goog

[PATCH v9 3/5] Bluetooth: refactor set_exp_feature with a feature table

[Joseph Hwang]

This patch refactors the set_exp_feature with a feature table
consisting of UUIDs and the corresponding callback functions.
In this way, a new experimental feature setting function can be
simply added with its UUID and callback function.

When looking at this patch, please use

  git show --patience

which will display the diff much better than the default
diff-algorithm.

Signed-off-by: Joseph Hwang <josephsih@chromium.org>
---

Changes in v9:
- This version fixes the compile errors of this patch.
  The errors were caused by accidentally messing up my
  development environment.

Changes in v8:
- Refactor the set_exp_feature function with a feature table.
- This is a new patch added in v8.

 net/bluetooth/mgmt.c | 248 +++++++++++++++++++++++++------------------
 1 file changed, 142 insertions(+), 106 deletions(-)

diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index 1e21e014efd2..42bd503da20d 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -3806,7 +3806,7 @@ static const u8 rpa_resolution_uuid[16] = {
 static int read_exp_features_info(struct sock *sk, struct hci_dev *hdev,
 				  void *data, u16 data_len)
 {
-	char buf[62];	/* Enough space for 3 features */
+	char buf[62];   /* Enough space for 3 features */
 	struct mgmt_rp_read_exp_features_info *rp = (void *)buf;
 	u16 idx = 0;
 	u32 flags;
@@ -3892,150 +3892,186 @@ static int exp_debug_feature_changed(bool enabled, struct sock *skip)
 }
 #endif
 
-static int set_exp_feature(struct sock *sk, struct hci_dev *hdev,
-			   void *data, u16 data_len)
+#define EXP_FEAT(_uuid, _set_func)	\
+{					\
+	.uuid = _uuid,			\
+	.set_func = _set_func,		\
+}
+
+/* The zero key uuid is special. Multiple exp features are set through it. */
+static int set_zero_key_func(struct sock *sk, struct hci_dev *hdev,
+			     struct mgmt_cp_set_exp_feature *cp, u16 data_len)
 {
-	struct mgmt_cp_set_exp_feature *cp = data;
 	struct mgmt_rp_set_exp_feature rp;
 
-	bt_dev_dbg(hdev, "sock %p", sk);
-
-	if (!memcmp(cp->uuid, ZERO_KEY, 16)) {
-		memset(rp.uuid, 0, 16);
-		rp.flags = cpu_to_le32(0);
+	memset(rp.uuid, 0, 16);
+	rp.flags = cpu_to_le32(0);
 
 #ifdef CONFIG_BT_FEATURE_DEBUG
-		if (!hdev) {
-			bool changed = bt_dbg_get();
+	if (!hdev) {
+		bool changed = bt_dbg_get();
 
-			bt_dbg_set(false);
+		bt_dbg_set(false);
 
-			if (changed)
-				exp_debug_feature_changed(false, sk);
-		}
+		if (changed)
+			exp_debug_feature_changed(false, sk);
+	}
 #endif
 
-		if (hdev && use_ll_privacy(hdev) && !hdev_is_powered(hdev)) {
-			bool changed = hci_dev_test_flag(hdev,
-							 HCI_ENABLE_LL_PRIVACY);
+	if (hdev && use_ll_privacy(hdev) && !hdev_is_powered(hdev)) {
+		bool changed = hci_dev_test_flag(hdev, HCI_ENABLE_LL_PRIVACY);
 
-			hci_dev_clear_flag(hdev, HCI_ENABLE_LL_PRIVACY);
+		hci_dev_clear_flag(hdev, HCI_ENABLE_LL_PRIVACY);
 
-			if (changed)
-				exp_ll_privacy_feature_changed(false, hdev, sk);
-		}
+		if (changed)
+			exp_ll_privacy_feature_changed(false, hdev, sk);
+	}
 
-		hci_sock_set_flag(sk, HCI_MGMT_EXP_FEATURE_EVENTS);
+	hci_sock_set_flag(sk, HCI_MGMT_EXP_FEATURE_EVENTS);
 
-		return mgmt_cmd_complete(sk, hdev ? hdev->id : MGMT_INDEX_NONE,
-					 MGMT_OP_SET_EXP_FEATURE, 0,
-					 &rp, sizeof(rp));
-	}
+	return mgmt_cmd_complete(sk, hdev ? hdev->id : MGMT_INDEX_NONE,
+				 MGMT_OP_SET_EXP_FEATURE, 0,
+				 &rp, sizeof(rp));
+}
 
 #ifdef CONFIG_BT_FEATURE_DEBUG
-	if (!memcmp(cp->uuid, debug_uuid, 16)) {
-		bool val, changed;
-		int err;
+static int set_debug_func(struct sock *sk, struct hci_dev *hdev,
+			  struct mgmt_cp_set_exp_feature *cp, u16 data_len)
+{
+	struct mgmt_rp_set_exp_feature rp;
 
-		/* Command requires to use the non-controller index */
-		if (hdev)
-			return mgmt_cmd_status(sk, hdev->id,
-					       MGMT_OP_SET_EXP_FEATURE,
-					       MGMT_STATUS_INVALID_INDEX);
+	bool val, changed;
+	int err;
 
-		/* Parameters are limited to a single octet */
-		if (data_len != MGMT_SET_EXP_FEATURE_SIZE + 1)
-			return mgmt_cmd_status(sk, MGMT_INDEX_NONE,
-					       MGMT_OP_SET_EXP_FEATURE,
-					       MGMT_STATUS_INVALID_PARAMS);
+	/* Command requires to use the non-controller index */
+	if (hdev)
+		return mgmt_cmd_status(sk, hdev->id,
+				       MGMT_OP_SET_EXP_FEATURE,
+				       MGMT_STATUS_INVALID_INDEX);
 
-		/* Only boolean on/off is supported */
-		if (cp->param[0] != 0x00 && cp->param[0] != 0x01)
-			return mgmt_cmd_status(sk, MGMT_INDEX_NONE,
-					       MGMT_OP_SET_EXP_FEATURE,
-					       MGMT_STATUS_INVALID_PARAMS);
+	/* Parameters are limited to a single octet */
+	if (data_len != MGMT_SET_EXP_FEATURE_SIZE + 1)
+		return mgmt_cmd_status(sk, MGMT_INDEX_NONE,
+				       MGMT_OP_SET_EXP_FEATURE,
+				       MGMT_STATUS_INVALID_PARAMS);
 
-		val = !!cp->param[0];
-		changed = val ? !bt_dbg_get() : bt_dbg_get();
-		bt_dbg_set(val);
+	/* Only boolean on/off is supported */
+	if (cp->param[0] != 0x00 && cp->param[0] != 0x01)
+		return mgmt_cmd_status(sk, MGMT_INDEX_NONE,
+				       MGMT_OP_SET_EXP_FEATURE,
+				       MGMT_STATUS_INVALID_PARAMS);
 
-		memcpy(rp.uuid, debug_uuid, 16);
-		rp.flags = cpu_to_le32(val ? BIT(0) : 0);
+	val = !!cp->param[0];
+	changed = val ? !bt_dbg_get() : bt_dbg_get();
+	bt_dbg_set(val);
 
-		hci_sock_set_flag(sk, HCI_MGMT_EXP_FEATURE_EVENTS);
+	memcpy(rp.uuid, debug_uuid, 16);
+	rp.flags = cpu_to_le32(val ? BIT(0) : 0);
 
-		err = mgmt_cmd_complete(sk, MGMT_INDEX_NONE,
-					MGMT_OP_SET_EXP_FEATURE, 0,
-					&rp, sizeof(rp));
+	hci_sock_set_flag(sk, HCI_MGMT_EXP_FEATURE_EVENTS);
 
-		if (changed)
-			exp_debug_feature_changed(val, sk);
+	err = mgmt_cmd_complete(sk, MGMT_INDEX_NONE,
+				MGMT_OP_SET_EXP_FEATURE, 0,
+				&rp, sizeof(rp));
 
-		return err;
-	}
+	if (changed)
+		exp_debug_feature_changed(val, sk);
+
+	return err;
+}
 #endif
 
-	if (!memcmp(cp->uuid, rpa_resolution_uuid, 16)) {
-		bool val, changed;
-		int err;
-		u32 flags;
+static int set_rpa_resolution_func(struct sock *sk, struct hci_dev *hdev,
+				   struct mgmt_cp_set_exp_feature *cp,
+				   u16 data_len)
+{
+	struct mgmt_rp_set_exp_feature rp;
+	bool val, changed;
+	int err;
+	u32 flags;
+
+	/* Command requires to use the controller index */
+	if (!hdev)
+		return mgmt_cmd_status(sk, MGMT_INDEX_NONE,
+				       MGMT_OP_SET_EXP_FEATURE,
+				       MGMT_STATUS_INVALID_INDEX);
 
-		/* Command requires to use the controller index */
-		if (!hdev)
-			return mgmt_cmd_status(sk, MGMT_INDEX_NONE,
-					       MGMT_OP_SET_EXP_FEATURE,
-					       MGMT_STATUS_INVALID_INDEX);
+	/* Changes can only be made when controller is powered down */
+	if (hdev_is_powered(hdev))
+		return mgmt_cmd_status(sk, hdev->id,
+				       MGMT_OP_SET_EXP_FEATURE,
+				       MGMT_STATUS_REJECTED);
 
-		/* Changes can only be made when controller is powered down */
-		if (hdev_is_powered(hdev))
-			return mgmt_cmd_status(sk, hdev->id,
-					       MGMT_OP_SET_EXP_FEATURE,
-					       MGMT_STATUS_REJECTED);
+	/* Parameters are limited to a single octet */
+	if (data_len != MGMT_SET_EXP_FEATURE_SIZE + 1)
+		return mgmt_cmd_status(sk, hdev->id,
+				       MGMT_OP_SET_EXP_FEATURE,
+				       MGMT_STATUS_INVALID_PARAMS);
 
-		/* Parameters are limited to a single octet */
-		if (data_len != MGMT_SET_EXP_FEATURE_SIZE + 1)
-			return mgmt_cmd_status(sk, hdev->id,
-					       MGMT_OP_SET_EXP_FEATURE,
-					       MGMT_STATUS_INVALID_PARAMS);
+	/* Only boolean on/off is supported */
+	if (cp->param[0] != 0x00 && cp->param[0] != 0x01)
+		return mgmt_cmd_status(sk, hdev->id,
+				       MGMT_OP_SET_EXP_FEATURE,
+				       MGMT_STATUS_INVALID_PARAMS);
 
-		/* Only boolean on/off is supported */
-		if (cp->param[0] != 0x00 && cp->param[0] != 0x01)
-			return mgmt_cmd_status(sk, hdev->id,
-					       MGMT_OP_SET_EXP_FEATURE,
-					       MGMT_STATUS_INVALID_PARAMS);
+	val = !!cp->param[0];
 
-		val = !!cp->param[0];
+	if (val) {
+		changed = !hci_dev_test_flag(hdev, HCI_ENABLE_LL_PRIVACY);
+		hci_dev_set_flag(hdev, HCI_ENABLE_LL_PRIVACY);
+		hci_dev_clear_flag(hdev, HCI_ADVERTISING);
 
-		if (val) {
-			changed = !hci_dev_test_flag(hdev,
-						     HCI_ENABLE_LL_PRIVACY);
-			hci_dev_set_flag(hdev, HCI_ENABLE_LL_PRIVACY);
-			hci_dev_clear_flag(hdev, HCI_ADVERTISING);
+		/* Enable LL privacy + supported settings changed */
+		flags = BIT(0) | BIT(1);
+	} else {
+		changed = hci_dev_test_flag(hdev, HCI_ENABLE_LL_PRIVACY);
+		hci_dev_clear_flag(hdev, HCI_ENABLE_LL_PRIVACY);
 
-			/* Enable LL privacy + supported settings changed */
-			flags = BIT(0) | BIT(1);
-		} else {
-			changed = hci_dev_test_flag(hdev,
-						    HCI_ENABLE_LL_PRIVACY);
-			hci_dev_clear_flag(hdev, HCI_ENABLE_LL_PRIVACY);
+		/* Disable LL privacy + supported settings changed */
+		flags = BIT(1);
+	}
 
-			/* Disable LL privacy + supported settings changed */
-			flags = BIT(1);
-		}
+	memcpy(rp.uuid, rpa_resolution_uuid, 16);
+	rp.flags = cpu_to_le32(flags);
 
-		memcpy(rp.uuid, rpa_resolution_uuid, 16);
-		rp.flags = cpu_to_le32(flags);
+	hci_sock_set_flag(sk, HCI_MGMT_EXP_FEATURE_EVENTS);
 
-		hci_sock_set_flag(sk, HCI_MGMT_EXP_FEATURE_EVENTS);
+	err = mgmt_cmd_complete(sk, hdev->id,
+				MGMT_OP_SET_EXP_FEATURE, 0,
+				&rp, sizeof(rp));
 
-		err = mgmt_cmd_complete(sk, hdev->id,
-					MGMT_OP_SET_EXP_FEATURE, 0,
-					&rp, sizeof(rp));
+	if (changed)
+		exp_ll_privacy_feature_changed(val, hdev, sk);
 
-		if (changed)
-			exp_ll_privacy_feature_changed(val, hdev, sk);
+	return err;
+}
 
-		return err;
+static const struct mgmt_exp_feature {
+	const u8 *uuid;
+	int (*set_func)(struct sock *sk, struct hci_dev *hdev,
+			struct mgmt_cp_set_exp_feature *cp, u16 data_len);
+} exp_features[] = {
+	EXP_FEAT(ZERO_KEY, set_zero_key_func),
+#ifdef CONFIG_BT_FEATURE_DEBUG
+	EXP_FEAT(debug_uuid, set_debug_func),
+#endif
+	EXP_FEAT(rpa_resolution_uuid, set_rpa_resolution_func),
+
+	/* end with a null feature */
+	EXP_FEAT(NULL, NULL)
+};
+
+static int set_exp_feature(struct sock *sk, struct hci_dev *hdev,
+			   void *data, u16 data_len)
+{
+	struct mgmt_cp_set_exp_feature *cp = data;
+	size_t i = 0;
+
+	bt_dev_dbg(hdev, "sock %p", sk);
+
+	for (i = 0; exp_features[i].uuid; i++) {
+		if (!memcmp(cp->uuid, exp_features[i].uuid, 16))
+			return exp_features[i].set_func(sk, hdev, cp, data_len);
 	}
 
 	return mgmt_cmd_status(sk, hdev ? hdev->id : MGMT_INDEX_NONE,
-- 
2.33.0.rc1.237.g0d66db33f3-goog

[PATCH v9 4/5] Bluetooth: Support the quality report events

[Joseph Hwang]

This patch allows a user space process to enable/disable the quality
report events dynamically through the set experimental feature mgmt
interface if CONFIG_BT_FEATURE_QUALITY_REPORT is enabled.

Since the quality report feature needs to invoke the callback function
provided by the driver, i.e., hdev->set_quality_report, a valid
controller index is required.

Reviewed-by: Miao-chen Chou <mcchou@chromium.org>
Signed-off-by: Joseph Hwang <josephsih@chromium.org>
---

(no changes since v8)

Changes in v8:
- Rebase on the previous patch about refactoring set_exp_feature with
  a feature table. A standalone set_quality_report_func is implemented
  instead of adding the logic into set_exp_feature.

Changes in v7:
- Rebase on Tedd's patches that moved functionality from btusb to
  btintel.

Changes in v5:
- Removed CONFIG_BT_FEATURE_QUALITY_REPORT since there was no
  large size impact.

 include/net/bluetooth/hci.h      |   1 +
 include/net/bluetooth/hci_core.h |   2 +
 net/bluetooth/mgmt.c             | 113 ++++++++++++++++++++++++++++++-
 3 files changed, 115 insertions(+), 1 deletion(-)

diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
index b80415011dcd..bb6b7398f490 100644
--- a/include/net/bluetooth/hci.h
+++ b/include/net/bluetooth/hci.h
@@ -330,6 +330,7 @@ enum {
 	HCI_ENABLE_LL_PRIVACY,
 	HCI_CMD_PENDING,
 	HCI_FORCE_NO_MITM,
+	HCI_QUALITY_REPORT,
 
 	__HCI_NUM_FLAGS,
 };
diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index a7d06d7da602..7e9ae36b2582 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -606,6 +606,7 @@ struct hci_dev {
 	int (*set_bdaddr)(struct hci_dev *hdev, const bdaddr_t *bdaddr);
 	void (*cmd_timeout)(struct hci_dev *hdev);
 	bool (*prevent_wake)(struct hci_dev *hdev);
+	int (*set_quality_report)(struct hci_dev *hdev, bool enable);
 };
 
 #define HCI_PHY_HANDLE(handle)	(handle & 0xff)
@@ -759,6 +760,7 @@ extern struct mutex hci_cb_list_lock;
 		hci_dev_clear_flag(hdev, HCI_LE_ADV);		\
 		hci_dev_clear_flag(hdev, HCI_LL_RPA_RESOLUTION);\
 		hci_dev_clear_flag(hdev, HCI_PERIODIC_INQ);	\
+		hci_dev_clear_flag(hdev, HCI_QUALITY_REPORT);	\
 	} while (0)
 
 /* ----- HCI interface to upper protocols ----- */
diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index 42bd503da20d..2910488bcb3b 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -3791,6 +3791,12 @@ static const u8 debug_uuid[16] = {
 };
 #endif
 
+/* 330859bc-7506-492d-9370-9a6f0614037f */
+static const u8 quality_report_uuid[16] = {
+	0x7f, 0x03, 0x14, 0x06, 0x6f, 0x9a, 0x70, 0x93,
+	0x2d, 0x49, 0x06, 0x75, 0xbc, 0x59, 0x08, 0x33,
+};
+
 /* 671b10b5-42c0-4696-9227-eb28d1b049d6 */
 static const u8 simult_central_periph_uuid[16] = {
 	0xd6, 0x49, 0xb0, 0xd1, 0x28, 0xeb, 0x27, 0x92,
@@ -3806,7 +3812,7 @@ static const u8 rpa_resolution_uuid[16] = {
 static int read_exp_features_info(struct sock *sk, struct hci_dev *hdev,
 				  void *data, u16 data_len)
 {
-	char buf[62];   /* Enough space for 3 features */
+	char buf[82];   /* Enough space for 4 features: 2 + 20 * 4 */
 	struct mgmt_rp_read_exp_features_info *rp = (void *)buf;
 	u16 idx = 0;
 	u32 flags;
@@ -3850,6 +3856,24 @@ static int read_exp_features_info(struct sock *sk, struct hci_dev *hdev,
 		idx++;
 	}
 
+	if (hdev) {
+		if (hdev->set_quality_report) {
+			/* BIT(0): indicating if set_quality_report is
+			 * supported by controller.
+			 */
+			flags = BIT(0);
+
+			/* BIT(1): indicating if the feature is enabled. */
+			if (hci_dev_test_flag(hdev, HCI_QUALITY_REPORT))
+				flags |= BIT(1);
+		} else {
+			flags = 0;
+		}
+		memcpy(rp->features[idx].uuid, quality_report_uuid, 16);
+		rp->features[idx].flags = cpu_to_le32(flags);
+		idx++;
+	}
+
 	rp->feature_count = cpu_to_le16(idx);
 
 	/* After reading the experimental features information, enable
@@ -3892,6 +3916,21 @@ static int exp_debug_feature_changed(bool enabled, struct sock *skip)
 }
 #endif
 
+static int exp_quality_report_feature_changed(bool enabled, struct sock *skip)
+{
+	struct mgmt_ev_exp_feature_changed ev;
+
+	BT_INFO("enabled %d", enabled);
+
+	memset(&ev, 0, sizeof(ev));
+	memcpy(ev.uuid, quality_report_uuid, 16);
+	ev.flags = cpu_to_le32(enabled ? BIT(0) : 0);
+
+	return mgmt_limited_event(MGMT_EV_EXP_FEATURE_CHANGED, NULL,
+				  &ev, sizeof(ev),
+				  HCI_MGMT_EXP_FEATURE_EVENTS, skip);
+}
+
 #define EXP_FEAT(_uuid, _set_func)	\
 {					\
 	.uuid = _uuid,			\
@@ -4046,6 +4085,77 @@ static int set_rpa_resolution_func(struct sock *sk, struct hci_dev *hdev,
 	return err;
 }
 
+static int set_quality_report_func(struct sock *sk, struct hci_dev *hdev,
+				   struct mgmt_cp_set_exp_feature *cp,
+				   u16 data_len)
+{
+	struct mgmt_rp_set_exp_feature rp;
+	bool val, changed;
+	int err;
+
+	/* Command requires to use a valid controller index */
+	if (!hdev)
+		return mgmt_cmd_status(sk, MGMT_INDEX_NONE,
+				       MGMT_OP_SET_EXP_FEATURE,
+				       MGMT_STATUS_INVALID_INDEX);
+
+	/* Parameters are limited to a single octet */
+	if (data_len != MGMT_SET_EXP_FEATURE_SIZE + 1)
+		return mgmt_cmd_status(sk, hdev->id,
+				       MGMT_OP_SET_EXP_FEATURE,
+				       MGMT_STATUS_INVALID_PARAMS);
+
+	/* Only boolean on/off is supported */
+	if (cp->param[0] != 0x00 && cp->param[0] != 0x01)
+		return mgmt_cmd_status(sk, hdev->id,
+				       MGMT_OP_SET_EXP_FEATURE,
+				       MGMT_STATUS_INVALID_PARAMS);
+
+	hci_req_sync_lock(hdev);
+
+	val = !!cp->param[0];
+	changed = (val != hci_dev_test_flag(hdev, HCI_QUALITY_REPORT));
+
+	if (!hdev->set_quality_report) {
+		BT_INFO("quality report not supported");
+		err = mgmt_cmd_status(sk, hdev->id,
+				      MGMT_OP_SET_EXP_FEATURE,
+				      MGMT_STATUS_NOT_SUPPORTED);
+		goto unlock_quality_report;
+	}
+
+	if (changed) {
+		err = hdev->set_quality_report(hdev, val);
+		if (err) {
+			BT_ERR("set_quality_report value %d err %d", val, err);
+			err = mgmt_cmd_status(sk, hdev->id,
+					      MGMT_OP_SET_EXP_FEATURE,
+					      MGMT_STATUS_FAILED);
+			goto unlock_quality_report;
+		}
+		if (val)
+			hci_dev_set_flag(hdev, HCI_QUALITY_REPORT);
+		else
+			hci_dev_clear_flag(hdev, HCI_QUALITY_REPORT);
+	}
+
+	BT_INFO("quality report enable %d changed %d", val, changed);
+
+	memcpy(rp.uuid, quality_report_uuid, 16);
+	rp.flags = cpu_to_le32(val ? BIT(0) : 0);
+	hci_sock_set_flag(sk, HCI_MGMT_EXP_FEATURE_EVENTS);
+	err = mgmt_cmd_complete(sk, hdev->id,
+				MGMT_OP_SET_EXP_FEATURE, 0,
+				&rp, sizeof(rp));
+
+	if (changed)
+		exp_quality_report_feature_changed(val, sk);
+
+unlock_quality_report:
+	hci_req_sync_unlock(hdev);
+	return err;
+}
+
 static const struct mgmt_exp_feature {
 	const u8 *uuid;
 	int (*set_func)(struct sock *sk, struct hci_dev *hdev,
@@ -4056,6 +4166,7 @@ static const struct mgmt_exp_feature {
 	EXP_FEAT(debug_uuid, set_debug_func),
 #endif
 	EXP_FEAT(rpa_resolution_uuid, set_rpa_resolution_func),
+	EXP_FEAT(quality_report_uuid, set_quality_report_func),
 
 	/* end with a null feature */
 	EXP_FEAT(NULL, NULL)
-- 
2.33.0.rc1.237.g0d66db33f3-goog

[PATCH v9 5/5] Bluetooth: set quality report callback for Intel

[Joseph Hwang]

This patch sets up set_quality_report callback for Intel to
set and reset the debug features.

Reviewed-by: Miao-chen Chou <mcchou@chromium.org>
Signed-off-by: Joseph Hwang <josephsih@chromium.org>
---

(no changes since v8)

Changes in v8:
- Removed the unsuitable debug messages.

Changes in v7:
- Rebase on Tedd's patches that moved functionality from btusb to
  btintel.

Changes in v5:
- Removed CONFIG_BT_FEATURE_QUALITY_REPORT since there was no
  large size impact.

 drivers/bluetooth/btintel.c | 79 ++++++++++++++++++++++++++++++++++++-
 drivers/bluetooth/btintel.h |  6 +++
 2 files changed, 84 insertions(+), 1 deletion(-)

diff --git a/drivers/bluetooth/btintel.c b/drivers/bluetooth/btintel.c
index 643e2194ca01..778d803159f3 100644
--- a/drivers/bluetooth/btintel.c
+++ b/drivers/bluetooth/btintel.c
@@ -1291,8 +1291,10 @@ static int btintel_set_debug_features(struct hci_dev *hdev,
 	u8 trace_enable = 0x02;
 	struct sk_buff *skb;
 
-	if (!features)
+	if (!features) {
+		bt_dev_warn(hdev, "Debug features not read");
 		return -EINVAL;
+	}
 
 	if (!(features->page1[0] & 0x3f)) {
 		bt_dev_info(hdev, "Telemetry exception format not supported");
@@ -1323,9 +1325,77 @@ static int btintel_set_debug_features(struct hci_dev *hdev,
 	}
 	kfree_skb(skb);
 
+	bt_dev_info(hdev, "set debug features: trace_enable 0x%02x mask 0x%02x",
+		    trace_enable, mask[3]);
+
 	return 0;
 }
 
+static int btintel_reset_debug_features(struct hci_dev *hdev,
+				 const struct intel_debug_features *features)
+{
+	u8 mask[11] = { 0x0a, 0x92, 0x02, 0x00, 0x00, 0x00, 0x00, 0x00,
+			0x00, 0x00, 0x00 };
+	u8 trace_enable = 0x00;
+	struct sk_buff *skb;
+
+	if (!features) {
+		bt_dev_warn(hdev, "Debug features not read");
+		return -EINVAL;
+	}
+
+	if (!(features->page1[0] & 0x3f)) {
+		bt_dev_info(hdev, "Telemetry exception format not supported");
+		return 0;
+	}
+
+	/* Should stop the trace before writing ddc event mask. */
+	skb = __hci_cmd_sync(hdev, 0xfca1, 1, &trace_enable, HCI_INIT_TIMEOUT);
+	if (IS_ERR(skb)) {
+		bt_dev_err(hdev, "Stop tracing of link statistics events failed (%ld)",
+			   PTR_ERR(skb));
+		return PTR_ERR(skb);
+	}
+	kfree_skb(skb);
+
+	skb = __hci_cmd_sync(hdev, 0xfc8b, 11, mask, HCI_INIT_TIMEOUT);
+	if (IS_ERR(skb)) {
+		bt_dev_err(hdev, "Setting Intel telemetry ddc write event mask failed (%ld)",
+			   PTR_ERR(skb));
+		return PTR_ERR(skb);
+	}
+	kfree_skb(skb);
+
+	bt_dev_info(hdev, "reset debug features: trace_enable 0x%02x mask 0x%02x",
+		    trace_enable, mask[3]);
+
+	return 0;
+}
+
+int btintel_set_quality_report(struct hci_dev *hdev, bool enable)
+{
+	struct intel_debug_features features;
+	int err;
+
+	bt_dev_dbg(hdev, "enable %d", enable);
+
+	/* Read the Intel supported features and if new exception formats
+	 * supported, need to load the additional DDC config to enable.
+	 */
+	err = btintel_read_debug_features(hdev, &features);
+	if (err)
+		return err;
+
+	/* Set or reset the debug features. */
+	if (enable)
+		err = btintel_set_debug_features(hdev, &features);
+	else
+		err = btintel_reset_debug_features(hdev, &features);
+
+	return err;
+}
+EXPORT_SYMBOL_GPL(btintel_set_quality_report);
+
 static const struct firmware *btintel_legacy_rom_get_fw(struct hci_dev *hdev,
 					       struct intel_version *ver)
 {
@@ -1951,6 +2021,8 @@ static int btintel_bootloader_setup(struct hci_dev *hdev,
 		btintel_load_ddc_config(hdev, ddcname);
 	}
 
+	hci_dev_clear_flag(hdev, HCI_QUALITY_REPORT);
+
 	/* Read the Intel version information after loading the FW  */
 	err = btintel_read_version(hdev, &new_ver);
 	if (err)
@@ -2132,6 +2204,8 @@ static int btintel_bootloader_setup_tlv(struct hci_dev *hdev,
 	 */
 	btintel_load_ddc_config(hdev, ddcname);
 
+	hci_dev_clear_flag(hdev, HCI_QUALITY_REPORT);
+
 	/* Read the Intel version information after loading the FW  */
 	err = btintel_read_version_tlv(hdev, &new_ver);
 	if (err)
@@ -2230,6 +2304,9 @@ static int btintel_setup_combined(struct hci_dev *hdev)
 	set_bit(HCI_QUIRK_SIMULTANEOUS_DISCOVERY, &hdev->quirks);
 	set_bit(HCI_QUIRK_NON_PERSISTENT_DIAG, &hdev->quirks);
 
+	/* Set up the quality report callback for Intel devices */
+	hdev->set_quality_report = btintel_set_quality_report;
+
 	/* For Legacy device, check the HW platform value and size */
 	if (skb->len == sizeof(ver) && skb->data[1] == 0x37) {
 		bt_dev_dbg(hdev, "Read the legacy Intel version information");
diff --git a/drivers/bluetooth/btintel.h b/drivers/bluetooth/btintel.h
index aa64072bbe68..fe02cb9ac96c 100644
--- a/drivers/bluetooth/btintel.h
+++ b/drivers/bluetooth/btintel.h
@@ -204,6 +204,7 @@ int btintel_configure_setup(struct hci_dev *hdev);
 void btintel_bootup(struct hci_dev *hdev, const void *ptr, unsigned int len);
 void btintel_secure_send_result(struct hci_dev *hdev,
 				const void *ptr, unsigned int len);
+int btintel_set_quality_report(struct hci_dev *hdev, bool enable);
 #else
 
 static inline int btintel_check_bdaddr(struct hci_dev *hdev)
@@ -294,4 +295,9 @@ static inline void btintel_secure_send_result(struct hci_dev *hdev,
 				const void *ptr, unsigned int len)
 {
 }
+
+static inline int btintel_set_quality_report(struct hci_dev *hdev, bool enable)
+{
+	return -ENODEV;
+}
 #endif
-- 
2.33.0.rc1.237.g0d66db33f3-goog

RE: [v9,1/5] Bluetooth: btusb: disable Intel link statistics telemetry events

[bluez.test.bot]

[-- Attachment #1: Type: text/plain, Size: 2826 bytes --]

This is automated email and please do not reply to this email!

Dear submitter,

Thank you for submitting the patches to the linux bluetooth mailing list.
This is a CI test results with your patch series:
PW Link:https://patchwork.kernel.org/project/bluetooth/list/?series=531683

---Test result---

Test Summary:
CheckPatch                    PASS      2.92 seconds
GitLint                       PASS      0.61 seconds
BuildKernel                   PASS      598.41 seconds
TestRunner: Setup             PASS      394.12 seconds
TestRunner: l2cap-tester      PASS      2.89 seconds
TestRunner: bnep-tester       PASS      2.07 seconds
TestRunner: mgmt-tester       FAIL      30.94 seconds
TestRunner: rfcomm-tester     PASS      2.31 seconds
TestRunner: sco-tester        PASS      2.20 seconds
TestRunner: smp-tester        FAIL      2.27 seconds
TestRunner: userchan-tester   PASS      2.09 seconds

Details
##############################
Test: CheckPatch - PASS - 2.92 seconds
Run checkpatch.pl script with rule in .checkpatch.conf


##############################
Test: GitLint - PASS - 0.61 seconds
Run gitlint with rule in .gitlint


##############################
Test: BuildKernel - PASS - 598.41 seconds
Build Kernel with minimal configuration supports Bluetooth


##############################
Test: TestRunner: Setup - PASS - 394.12 seconds
Setup environment for running Test Runner


##############################
Test: TestRunner: l2cap-tester - PASS - 2.89 seconds
Run test-runner with l2cap-tester
Total: 40, Passed: 40 (100.0%), Failed: 0, Not Run: 0

##############################
Test: TestRunner: bnep-tester - PASS - 2.07 seconds
Run test-runner with bnep-tester
Total: 1, Passed: 1 (100.0%), Failed: 0, Not Run: 0

##############################
Test: TestRunner: mgmt-tester - FAIL - 30.94 seconds
Run test-runner with mgmt-tester
Total: 448, Passed: 444 (99.1%), Failed: 1, Not Run: 3

Failed Test Cases
Read Exp Feature - Success                           Failed       0.012 seconds

##############################
Test: TestRunner: rfcomm-tester - PASS - 2.31 seconds
Run test-runner with rfcomm-tester
Total: 9, Passed: 9 (100.0%), Failed: 0, Not Run: 0

##############################
Test: TestRunner: sco-tester - PASS - 2.20 seconds
Run test-runner with sco-tester
Total: 8, Passed: 8 (100.0%), Failed: 0, Not Run: 0

##############################
Test: TestRunner: smp-tester - FAIL - 2.27 seconds
Run test-runner with smp-tester
Total: 8, Passed: 7 (87.5%), Failed: 1, Not Run: 0

Failed Test Cases
SMP Client - SC Request 2                            Failed       0.024 seconds

##############################
Test: TestRunner: userchan-tester - PASS - 2.09 seconds
Run test-runner with userchan-tester
Total: 3, Passed: 3 (100.0%), Failed: 0, Not Run: 0



---
Regards,
Linux Bluetooth


[-- Attachment #2: l2cap-tester.log --]
[-- Type: application/octet-stream, Size: 44386 bytes --]

[-- Attachment #3: bnep-tester.log --]
[-- Type: application/octet-stream, Size: 3593 bytes --]

[-- Attachment #4: mgmt-tester.log --]
[-- Type: application/octet-stream, Size: 616867 bytes --]

[-- Attachment #5: rfcomm-tester.log --]
[-- Type: application/octet-stream, Size: 11713 bytes --]

[-- Attachment #6: sco-tester.log --]
[-- Type: application/octet-stream, Size: 9948 bytes --]

[-- Attachment #7: smp-tester.log --]
[-- Type: application/octet-stream, Size: 11741 bytes --]

[-- Attachment #8: userchan-tester.log --]
[-- Type: application/octet-stream, Size: 5490 bytes --]

Re: [v9,1/5] Bluetooth: btusb: disable Intel link statistics telemetry events

[Luiz Augusto von Dentz]

Hi Joseph,

On Sun, Aug 15, 2021 at 6:25 AM <bluez.test.bot@gmail.com> wrote:
>
> This is automated email and please do not reply to this email!
>
> Dear submitter,
>
> Thank you for submitting the patches to the linux bluetooth mailing list.
> This is a CI test results with your patch series:
> PW Link:https://patchwork.kernel.org/project/bluetooth/list/?series=531683
>
> ---Test result---
>
> Test Summary:
> CheckPatch                    PASS      2.92 seconds
> GitLint                       PASS      0.61 seconds
> BuildKernel                   PASS      598.41 seconds
> TestRunner: Setup             PASS      394.12 seconds
> TestRunner: l2cap-tester      PASS      2.89 seconds
> TestRunner: bnep-tester       PASS      2.07 seconds
> TestRunner: mgmt-tester       FAIL      30.94 seconds
> TestRunner: rfcomm-tester     PASS      2.31 seconds
> TestRunner: sco-tester        PASS      2.20 seconds
> TestRunner: smp-tester        FAIL      2.27 seconds
> TestRunner: userchan-tester   PASS      2.09 seconds
>
> Details
> ##############################
> Test: CheckPatch - PASS - 2.92 seconds
> Run checkpatch.pl script with rule in .checkpatch.conf
>
>
> ##############################
> Test: GitLint - PASS - 0.61 seconds
> Run gitlint with rule in .gitlint
>
>
> ##############################
> Test: BuildKernel - PASS - 598.41 seconds
> Build Kernel with minimal configuration supports Bluetooth
>
>
> ##############################
> Test: TestRunner: Setup - PASS - 394.12 seconds
> Setup environment for running Test Runner
>
>
> ##############################
> Test: TestRunner: l2cap-tester - PASS - 2.89 seconds
> Run test-runner with l2cap-tester
> Total: 40, Passed: 40 (100.0%), Failed: 0, Not Run: 0
>
> ##############################
> Test: TestRunner: bnep-tester - PASS - 2.07 seconds
> Run test-runner with bnep-tester
> Total: 1, Passed: 1 (100.0%), Failed: 0, Not Run: 0
>
> ##############################
> Test: TestRunner: mgmt-tester - FAIL - 30.94 seconds
> Run test-runner with mgmt-tester
> Total: 448, Passed: 444 (99.1%), Failed: 1, Not Run: 3
>
> Failed Test Cases
> Read Exp Feature - Success                           Failed       0.012 seconds

Looks like there is a regression on mgmt-tester:

Read Exp Feature - Success - run
  Sending Read Experimental Features Information (0x0049)
  Test condition added, total 1
  Read Experimental Features Information (0x0049): Success (0x00)
  Invalid cmd response parameter size
Read Exp Feature - Success - test failed

>
> ##############################
> Test: TestRunner: rfcomm-tester - PASS - 2.31 seconds
> Run test-runner with rfcomm-tester
> Total: 9, Passed: 9 (100.0%), Failed: 0, Not Run: 0
>
> ##############################
> Test: TestRunner: sco-tester - PASS - 2.20 seconds
> Run test-runner with sco-tester
> Total: 8, Passed: 8 (100.0%), Failed: 0, Not Run: 0
>
> ##############################
> Test: TestRunner: smp-tester - FAIL - 2.27 seconds
> Run test-runner with smp-tester
> Total: 8, Passed: 7 (87.5%), Failed: 1, Not Run: 0
>
> Failed Test Cases
> SMP Client - SC Request 2                            Failed       0.024 seconds
>
> ##############################
> Test: TestRunner: userchan-tester - PASS - 2.09 seconds
> Run test-runner with userchan-tester
> Total: 3, Passed: 3 (100.0%), Failed: 0, Not Run: 0
>
>
>
> ---
> Regards,
> Linux Bluetooth
>


-- 
Luiz Augusto von Dentz

Re: [v9,1/5] Bluetooth: btusb: disable Intel link statistics telemetry events

[Joseph Hwang]

(Resent this email to contain only plaintext.)

Hi Luiz:

  It seems that mgmt-tester currently uses a fixed feature count.
Every time a new exp feature is added, the mgmt-tester would  be
broken. By checking the kernel, it seems that there are currently 2 or
3 exp features, i.e., debug uuid, simultaneous central peripheral
uuid, and LL privacy uuid. Note that the debug exp feature is guarded
by CONFIG_BT_FEATURE_DEBUG. So I am not sure how the kernel is
configured and made on your test setup.

  If we fix the mgmt-tester in bluez to have 3 or 4 (which one?)
features before merging the kernel changes here that adds a new
quality exp feature, it would not match the existing kernel which has
only 2 or 3 features.

  Do you have any preference about how to fix the mgmt-tester?

  My suggestion is to remove the checking of the feature count from
the mgmt-tester. The feature count changes over time. It is possible
to implement a customized exp_feat_check function which can be more
flexible. If a uuid is found in the MGMT response, its associated
flags are checked against.

  The data currently used in the mgmt-tester:

static const uint8_t read_exp_feat_param_success[] = {
0x02, 0x00, /* Feature Count */
0xd6, 0x49, 0xb0, 0xd1, 0x28, 0xeb, /* UUID - Simultaneous */
0x27, 0x92, 0x96, 0x46, 0xc0, 0x42, /* Central Peripheral */
0xb5, 0x10, 0x1b, 0x67,
0x00, 0x00, 0x00, 0x00, /* Flags */
0x04, 0x00, 0x13, 0xac, 0x42, 0x02, /* UUID - LL Privacy */
0xde, 0xb3, 0xea, 0x11, 0x73, 0xc2,
0x48, 0xa1, 0xc0, 0x15,
0x02, 0x00, 0x00, 0x00, /* Flags */
};


  Please let me know what you think.

Thanks and regards,
Joseph


On Tue, Aug 17, 2021 at 1:49 AM Luiz Augusto von Dentz
<luiz.dentz@gmail.com> wrote:
>
> Hi Joseph,
>
> On Sun, Aug 15, 2021 at 6:25 AM <bluez.test.bot@gmail.com> wrote:
> >
> > This is automated email and please do not reply to this email!
> >
> > Dear submitter,
> >
> > Thank you for submitting the patches to the linux bluetooth mailing list.
> > This is a CI test results with your patch series:
> > PW Link:https://patchwork.kernel.org/project/bluetooth/list/?series=531683
> >
> > ---Test result---
> >
> > Test Summary:
> > CheckPatch                    PASS      2.92 seconds
> > GitLint                       PASS      0.61 seconds
> > BuildKernel                   PASS      598.41 seconds
> > TestRunner: Setup             PASS      394.12 seconds
> > TestRunner: l2cap-tester      PASS      2.89 seconds
> > TestRunner: bnep-tester       PASS      2.07 seconds
> > TestRunner: mgmt-tester       FAIL      30.94 seconds
> > TestRunner: rfcomm-tester     PASS      2.31 seconds
> > TestRunner: sco-tester        PASS      2.20 seconds
> > TestRunner: smp-tester        FAIL      2.27 seconds
> > TestRunner: userchan-tester   PASS      2.09 seconds
> >
> > Details
> > ##############################
> > Test: CheckPatch - PASS - 2.92 seconds
> > Run checkpatch.pl script with rule in .checkpatch.conf
> >
> >
> > ##############################
> > Test: GitLint - PASS - 0.61 seconds
> > Run gitlint with rule in .gitlint
> >
> >
> > ##############################
> > Test: BuildKernel - PASS - 598.41 seconds
> > Build Kernel with minimal configuration supports Bluetooth
> >
> >
> > ##############################
> > Test: TestRunner: Setup - PASS - 394.12 seconds
> > Setup environment for running Test Runner
> >
> >
> > ##############################
> > Test: TestRunner: l2cap-tester - PASS - 2.89 seconds
> > Run test-runner with l2cap-tester
> > Total: 40, Passed: 40 (100.0%), Failed: 0, Not Run: 0
> >
> > ##############################
> > Test: TestRunner: bnep-tester - PASS - 2.07 seconds
> > Run test-runner with bnep-tester
> > Total: 1, Passed: 1 (100.0%), Failed: 0, Not Run: 0
> >
> > ##############################
> > Test: TestRunner: mgmt-tester - FAIL - 30.94 seconds
> > Run test-runner with mgmt-tester
> > Total: 448, Passed: 444 (99.1%), Failed: 1, Not Run: 3
> >
> > Failed Test Cases
> > Read Exp Feature - Success                           Failed       0.012 seconds
>
> Looks like there is a regression on mgmt-tester:
>
> Read Exp Feature - Success - run
>   Sending Read Experimental Features Information (0x0049)
>   Test condition added, total 1
>   Read Experimental Features Information (0x0049): Success (0x00)
>   Invalid cmd response parameter size
> Read Exp Feature - Success - test failed
>
> >
> > ##############################
> > Test: TestRunner: rfcomm-tester - PASS - 2.31 seconds
> > Run test-runner with rfcomm-tester
> > Total: 9, Passed: 9 (100.0%), Failed: 0, Not Run: 0
> >
> > ##############################
> > Test: TestRunner: sco-tester - PASS - 2.20 seconds
> > Run test-runner with sco-tester
> > Total: 8, Passed: 8 (100.0%), Failed: 0, Not Run: 0
> >
> > ##############################
> > Test: TestRunner: smp-tester - FAIL - 2.27 seconds
> > Run test-runner with smp-tester
> > Total: 8, Passed: 7 (87.5%), Failed: 1, Not Run: 0
> >
> > Failed Test Cases
> > SMP Client - SC Request 2                            Failed       0.024 seconds
> >
> > ##############################
> > Test: TestRunner: userchan-tester - PASS - 2.09 seconds
> > Run test-runner with userchan-tester
> > Total: 3, Passed: 3 (100.0%), Failed: 0, Not Run: 0
> >
> >
> >
> > ---
> > Regards,
> > Linux Bluetooth
> >
>
>
> --
> Luiz Augusto von Dentz



-- 

Joseph Shyh-In Hwang
Email: josephsih@google.com

Re: [v9,1/5] Bluetooth: btusb: disable Intel link statistics telemetry events

[Joseph Hwang]

Hi Luiz:

  I am wondering if it is possible to merge these kernel patches
before fixing the mgmt-tester?

  The mgmt-tester failed due to its checking against the fixed exp
feature count and the corresponding exp UUIDs and flags. A more
flexible tester may be required so that the tester would not be broken
whenever a new exp feature is added.

Thanks and regards,
Joseph

On Tue, Aug 17, 2021 at 5:54 PM Joseph Hwang <josephsih@google.com> wrote:
>
> (Resent this email to contain only plaintext.)
>
> Hi Luiz:
>
>   It seems that mgmt-tester currently uses a fixed feature count.
> Every time a new exp feature is added, the mgmt-tester would  be
> broken. By checking the kernel, it seems that there are currently 2 or
> 3 exp features, i.e., debug uuid, simultaneous central peripheral
> uuid, and LL privacy uuid. Note that the debug exp feature is guarded
> by CONFIG_BT_FEATURE_DEBUG. So I am not sure how the kernel is
> configured and made on your test setup.
>
>   If we fix the mgmt-tester in bluez to have 3 or 4 (which one?)
> features before merging the kernel changes here that adds a new
> quality exp feature, it would not match the existing kernel which has
> only 2 or 3 features.
>
>   Do you have any preference about how to fix the mgmt-tester?
>
>   My suggestion is to remove the checking of the feature count from
> the mgmt-tester. The feature count changes over time. It is possible
> to implement a customized exp_feat_check function which can be more
> flexible. If a uuid is found in the MGMT response, its associated
> flags are checked against.
>
>   The data currently used in the mgmt-tester:
>
> static const uint8_t read_exp_feat_param_success[] = {
> 0x02, 0x00, /* Feature Count */
> 0xd6, 0x49, 0xb0, 0xd1, 0x28, 0xeb, /* UUID - Simultaneous */
> 0x27, 0x92, 0x96, 0x46, 0xc0, 0x42, /* Central Peripheral */
> 0xb5, 0x10, 0x1b, 0x67,
> 0x00, 0x00, 0x00, 0x00, /* Flags */
> 0x04, 0x00, 0x13, 0xac, 0x42, 0x02, /* UUID - LL Privacy */
> 0xde, 0xb3, 0xea, 0x11, 0x73, 0xc2,
> 0x48, 0xa1, 0xc0, 0x15,
> 0x02, 0x00, 0x00, 0x00, /* Flags */
> };
>
>
>   Please let me know what you think.
>
> Thanks and regards,
> Joseph
>
>
> On Tue, Aug 17, 2021 at 1:49 AM Luiz Augusto von Dentz
> <luiz.dentz@gmail.com> wrote:
> >
> > Hi Joseph,
> >
> > On Sun, Aug 15, 2021 at 6:25 AM <bluez.test.bot@gmail.com> wrote:
> > >
> > > This is automated email and please do not reply to this email!
> > >
> > > Dear submitter,
> > >
> > > Thank you for submitting the patches to the linux bluetooth mailing list.
> > > This is a CI test results with your patch series:
> > > PW Link:https://patchwork.kernel.org/project/bluetooth/list/?series=531683
> > >
> > > ---Test result---
> > >
> > > Test Summary:
> > > CheckPatch                    PASS      2.92 seconds
> > > GitLint                       PASS      0.61 seconds
> > > BuildKernel                   PASS      598.41 seconds
> > > TestRunner: Setup             PASS      394.12 seconds
> > > TestRunner: l2cap-tester      PASS      2.89 seconds
> > > TestRunner: bnep-tester       PASS      2.07 seconds
> > > TestRunner: mgmt-tester       FAIL      30.94 seconds
> > > TestRunner: rfcomm-tester     PASS      2.31 seconds
> > > TestRunner: sco-tester        PASS      2.20 seconds
> > > TestRunner: smp-tester        FAIL      2.27 seconds
> > > TestRunner: userchan-tester   PASS      2.09 seconds
> > >
> > > Details
> > > ##############################
> > > Test: CheckPatch - PASS - 2.92 seconds
> > > Run checkpatch.pl script with rule in .checkpatch.conf
> > >
> > >
> > > ##############################
> > > Test: GitLint - PASS - 0.61 seconds
> > > Run gitlint with rule in .gitlint
> > >
> > >
> > > ##############################
> > > Test: BuildKernel - PASS - 598.41 seconds
> > > Build Kernel with minimal configuration supports Bluetooth
> > >
> > >
> > > ##############################
> > > Test: TestRunner: Setup - PASS - 394.12 seconds
> > > Setup environment for running Test Runner
> > >
> > >
> > > ##############################
> > > Test: TestRunner: l2cap-tester - PASS - 2.89 seconds
> > > Run test-runner with l2cap-tester
> > > Total: 40, Passed: 40 (100.0%), Failed: 0, Not Run: 0
> > >
> > > ##############################
> > > Test: TestRunner: bnep-tester - PASS - 2.07 seconds
> > > Run test-runner with bnep-tester
> > > Total: 1, Passed: 1 (100.0%), Failed: 0, Not Run: 0
> > >
> > > ##############################
> > > Test: TestRunner: mgmt-tester - FAIL - 30.94 seconds
> > > Run test-runner with mgmt-tester
> > > Total: 448, Passed: 444 (99.1%), Failed: 1, Not Run: 3
> > >
> > > Failed Test Cases
> > > Read Exp Feature - Success                           Failed       0.012 seconds
> >
> > Looks like there is a regression on mgmt-tester:
> >
> > Read Exp Feature - Success - run
> >   Sending Read Experimental Features Information (0x0049)
> >   Test condition added, total 1
> >   Read Experimental Features Information (0x0049): Success (0x00)
> >   Invalid cmd response parameter size
> > Read Exp Feature - Success - test failed
> >
> > >
> > > ##############################
> > > Test: TestRunner: rfcomm-tester - PASS - 2.31 seconds
> > > Run test-runner with rfcomm-tester
> > > Total: 9, Passed: 9 (100.0%), Failed: 0, Not Run: 0
> > >
> > > ##############################
> > > Test: TestRunner: sco-tester - PASS - 2.20 seconds
> > > Run test-runner with sco-tester
> > > Total: 8, Passed: 8 (100.0%), Failed: 0, Not Run: 0
> > >
> > > ##############################
> > > Test: TestRunner: smp-tester - FAIL - 2.27 seconds
> > > Run test-runner with smp-tester
> > > Total: 8, Passed: 7 (87.5%), Failed: 1, Not Run: 0
> > >
> > > Failed Test Cases
> > > SMP Client - SC Request 2                            Failed       0.024 seconds
> > >
> > > ##############################
> > > Test: TestRunner: userchan-tester - PASS - 2.09 seconds
> > > Run test-runner with userchan-tester
> > > Total: 3, Passed: 3 (100.0%), Failed: 0, Not Run: 0
> > >
> > >
> > >
> > > ---
> > > Regards,
> > > Linux Bluetooth
> > >
> >
> >
> > --
> > Luiz Augusto von Dentz
>
>
>
> --
>
> Joseph Shyh-In Hwang
> Email: josephsih@google.com



-- 

Joseph Shyh-In Hwang
Email: josephsih@google.com

Re: [v9,1/5] Bluetooth: btusb: disable Intel link statistics telemetry events

[Luiz Augusto von Dentz]

Hi Joseph,

On Tue, Aug 17, 2021 at 10:49 PM Joseph Hwang <josephsih@google.com> wrote:
>
> Hi Luiz:
>
>   I am wondering if it is possible to merge these kernel patches
> before fixing the mgmt-tester?
>
>   The mgmt-tester failed due to its checking against the fixed exp
> feature count and the corresponding exp UUIDs and flags. A more
> flexible tester may be required so that the tester would not be broken
> whenever a new exp feature is added.

I would prefer to have the mgmt-tester changes applied as well so we
don't have false positives for other patches causing mgmt-tester to
fail, in fact we include CI is useful to validate this changes so we
can check the feature is correctly exposed to userspace and UUID is
properly formatted, etc, it shouldn't be a big change to mgmt-tester
thought if you just include the new UUID later on we can think about
how we could make it more extensible so it doesn't break when a new
feature is added.

> On Tue, Aug 17, 2021 at 5:54 PM Joseph Hwang <josephsih@google.com> wrote:
> >
> > (Resent this email to contain only plaintext.)
> >
> > Hi Luiz:
> >
> >   It seems that mgmt-tester currently uses a fixed feature count.
> > Every time a new exp feature is added, the mgmt-tester would  be
> > broken. By checking the kernel, it seems that there are currently 2 or
> > 3 exp features, i.e., debug uuid, simultaneous central peripheral
> > uuid, and LL privacy uuid. Note that the debug exp feature is guarded
> > by CONFIG_BT_FEATURE_DEBUG. So I am not sure how the kernel is
> > configured and made on your test setup.
> >
> >   If we fix the mgmt-tester in bluez to have 3 or 4 (which one?)
> > features before merging the kernel changes here that adds a new
> > quality exp feature, it would not match the existing kernel which has
> > only 2 or 3 features.
> >
> >   Do you have any preference about how to fix the mgmt-tester?
> >
> >   My suggestion is to remove the checking of the feature count from
> > the mgmt-tester. The feature count changes over time. It is possible
> > to implement a customized exp_feat_check function which can be more
> > flexible. If a uuid is found in the MGMT response, its associated
> > flags are checked against.
> >
> >   The data currently used in the mgmt-tester:
> >
> > static const uint8_t read_exp_feat_param_success[] = {
> > 0x02, 0x00, /* Feature Count */
> > 0xd6, 0x49, 0xb0, 0xd1, 0x28, 0xeb, /* UUID - Simultaneous */
> > 0x27, 0x92, 0x96, 0x46, 0xc0, 0x42, /* Central Peripheral */
> > 0xb5, 0x10, 0x1b, 0x67,
> > 0x00, 0x00, 0x00, 0x00, /* Flags */
> > 0x04, 0x00, 0x13, 0xac, 0x42, 0x02, /* UUID - LL Privacy */
> > 0xde, 0xb3, 0xea, 0x11, 0x73, 0xc2,
> > 0x48, 0xa1, 0xc0, 0x15,
> > 0x02, 0x00, 0x00, 0x00, /* Flags */
> > };
> >
> >
> >   Please let me know what you think.
> >
> > Thanks and regards,
> > Joseph
> >
> >
> > On Tue, Aug 17, 2021 at 1:49 AM Luiz Augusto von Dentz
> > <luiz.dentz@gmail.com> wrote:
> > >
> > > Hi Joseph,
> > >
> > > On Sun, Aug 15, 2021 at 6:25 AM <bluez.test.bot@gmail.com> wrote:
> > > >
> > > > This is automated email and please do not reply to this email!
> > > >
> > > > Dear submitter,
> > > >
> > > > Thank you for submitting the patches to the linux bluetooth mailing list.
> > > > This is a CI test results with your patch series:
> > > > PW Link:https://patchwork.kernel.org/project/bluetooth/list/?series=531683
> > > >
> > > > ---Test result---
> > > >
> > > > Test Summary:
> > > > CheckPatch                    PASS      2.92 seconds
> > > > GitLint                       PASS      0.61 seconds
> > > > BuildKernel                   PASS      598.41 seconds
> > > > TestRunner: Setup             PASS      394.12 seconds
> > > > TestRunner: l2cap-tester      PASS      2.89 seconds
> > > > TestRunner: bnep-tester       PASS      2.07 seconds
> > > > TestRunner: mgmt-tester       FAIL      30.94 seconds
> > > > TestRunner: rfcomm-tester     PASS      2.31 seconds
> > > > TestRunner: sco-tester        PASS      2.20 seconds
> > > > TestRunner: smp-tester        FAIL      2.27 seconds
> > > > TestRunner: userchan-tester   PASS      2.09 seconds
> > > >
> > > > Details
> > > > ##############################
> > > > Test: CheckPatch - PASS - 2.92 seconds
> > > > Run checkpatch.pl script with rule in .checkpatch.conf
> > > >
> > > >
> > > > ##############################
> > > > Test: GitLint - PASS - 0.61 seconds
> > > > Run gitlint with rule in .gitlint
> > > >
> > > >
> > > > ##############################
> > > > Test: BuildKernel - PASS - 598.41 seconds
> > > > Build Kernel with minimal configuration supports Bluetooth
> > > >
> > > >
> > > > ##############################
> > > > Test: TestRunner: Setup - PASS - 394.12 seconds
> > > > Setup environment for running Test Runner
> > > >
> > > >
> > > > ##############################
> > > > Test: TestRunner: l2cap-tester - PASS - 2.89 seconds
> > > > Run test-runner with l2cap-tester
> > > > Total: 40, Passed: 40 (100.0%), Failed: 0, Not Run: 0
> > > >
> > > > ##############################
> > > > Test: TestRunner: bnep-tester - PASS - 2.07 seconds
> > > > Run test-runner with bnep-tester
> > > > Total: 1, Passed: 1 (100.0%), Failed: 0, Not Run: 0
> > > >
> > > > ##############################
> > > > Test: TestRunner: mgmt-tester - FAIL - 30.94 seconds
> > > > Run test-runner with mgmt-tester
> > > > Total: 448, Passed: 444 (99.1%), Failed: 1, Not Run: 3
> > > >
> > > > Failed Test Cases
> > > > Read Exp Feature - Success                           Failed       0.012 seconds
> > >
> > > Looks like there is a regression on mgmt-tester:
> > >
> > > Read Exp Feature - Success - run
> > >   Sending Read Experimental Features Information (0x0049)
> > >   Test condition added, total 1
> > >   Read Experimental Features Information (0x0049): Success (0x00)
> > >   Invalid cmd response parameter size
> > > Read Exp Feature - Success - test failed
> > >
> > > >
> > > > ##############################
> > > > Test: TestRunner: rfcomm-tester - PASS - 2.31 seconds
> > > > Run test-runner with rfcomm-tester
> > > > Total: 9, Passed: 9 (100.0%), Failed: 0, Not Run: 0
> > > >
> > > > ##############################
> > > > Test: TestRunner: sco-tester - PASS - 2.20 seconds
> > > > Run test-runner with sco-tester
> > > > Total: 8, Passed: 8 (100.0%), Failed: 0, Not Run: 0
> > > >
> > > > ##############################
> > > > Test: TestRunner: smp-tester - FAIL - 2.27 seconds
> > > > Run test-runner with smp-tester
> > > > Total: 8, Passed: 7 (87.5%), Failed: 1, Not Run: 0
> > > >
> > > > Failed Test Cases
> > > > SMP Client - SC Request 2                            Failed       0.024 seconds
> > > >
> > > > ##############################
> > > > Test: TestRunner: userchan-tester - PASS - 2.09 seconds
> > > > Run test-runner with userchan-tester
> > > > Total: 3, Passed: 3 (100.0%), Failed: 0, Not Run: 0
> > > >
> > > >
> > > >
> > > > ---
> > > > Regards,
> > > > Linux Bluetooth
> > > >
> > >
> > >
> > > --
> > > Luiz Augusto von Dentz
> >
> >
> >
> > --
> >
> > Joseph Shyh-In Hwang
> > Email: josephsih@google.com
>
>
>
> --
>
> Joseph Shyh-In Hwang
> Email: josephsih@google.com



-- 
Luiz Augusto von Dentz

Re: [v9,1/5] Bluetooth: btusb: disable Intel link statistics telemetry events

[Joseph Hwang]

Hi Luiz:

The mgmt-tester change to add the UUID of the new exp feature is just
sent for review:

https://patchwork.kernel.org/project/bluetooth/patch/20210820131751.BlueZ.v1.1.I165b6fc2b20d80c8d18946434005f0269d92f489@changeid/

  The tester checks the exp features UUIDs and FLAGs. As the FLAG
values depend on how the test is set up. I set the flags to be all
0x00 which passed the mgmt-tester in my local test setup. If that is
not the case upstream, please let me know.

Thanks and regards,
Joseph

On Fri, Aug 20, 2021 at 2:04 AM Luiz Augusto von Dentz
<luiz.dentz@gmail.com> wrote:
>
> Hi Joseph,
>
> On Tue, Aug 17, 2021 at 10:49 PM Joseph Hwang <josephsih@google.com> wrote:
> >
> > Hi Luiz:
> >
> >   I am wondering if it is possible to merge these kernel patches
> > before fixing the mgmt-tester?
> >
> >   The mgmt-tester failed due to its checking against the fixed exp
> > feature count and the corresponding exp UUIDs and flags. A more
> > flexible tester may be required so that the tester would not be broken
> > whenever a new exp feature is added.
>
> I would prefer to have the mgmt-tester changes applied as well so we
> don't have false positives for other patches causing mgmt-tester to
> fail, in fact we include CI is useful to validate this changes so we
> can check the feature is correctly exposed to userspace and UUID is
> properly formatted, etc, it shouldn't be a big change to mgmt-tester
> thought if you just include the new UUID later on we can think about
> how we could make it more extensible so it doesn't break when a new
> feature is added.
>
> > On Tue, Aug 17, 2021 at 5:54 PM Joseph Hwang <josephsih@google.com> wrote:
> > >
> > > (Resent this email to contain only plaintext.)
> > >
> > > Hi Luiz:
> > >
> > >   It seems that mgmt-tester currently uses a fixed feature count.
> > > Every time a new exp feature is added, the mgmt-tester would  be
> > > broken. By checking the kernel, it seems that there are currently 2 or
> > > 3 exp features, i.e., debug uuid, simultaneous central peripheral
> > > uuid, and LL privacy uuid. Note that the debug exp feature is guarded
> > > by CONFIG_BT_FEATURE_DEBUG. So I am not sure how the kernel is
> > > configured and made on your test setup.
> > >
> > >   If we fix the mgmt-tester in bluez to have 3 or 4 (which one?)
> > > features before merging the kernel changes here that adds a new
> > > quality exp feature, it would not match the existing kernel which has
> > > only 2 or 3 features.
> > >
> > >   Do you have any preference about how to fix the mgmt-tester?
> > >
> > >   My suggestion is to remove the checking of the feature count from
> > > the mgmt-tester. The feature count changes over time. It is possible
> > > to implement a customized exp_feat_check function which can be more
> > > flexible. If a uuid is found in the MGMT response, its associated
> > > flags are checked against.
> > >
> > >   The data currently used in the mgmt-tester:
> > >
> > > static const uint8_t read_exp_feat_param_success[] = {
> > > 0x02, 0x00, /* Feature Count */
> > > 0xd6, 0x49, 0xb0, 0xd1, 0x28, 0xeb, /* UUID - Simultaneous */
> > > 0x27, 0x92, 0x96, 0x46, 0xc0, 0x42, /* Central Peripheral */
> > > 0xb5, 0x10, 0x1b, 0x67,
> > > 0x00, 0x00, 0x00, 0x00, /* Flags */
> > > 0x04, 0x00, 0x13, 0xac, 0x42, 0x02, /* UUID - LL Privacy */
> > > 0xde, 0xb3, 0xea, 0x11, 0x73, 0xc2,
> > > 0x48, 0xa1, 0xc0, 0x15,
> > > 0x02, 0x00, 0x00, 0x00, /* Flags */
> > > };
> > >
> > >
> > >   Please let me know what you think.
> > >
> > > Thanks and regards,
> > > Joseph
> > >
> > >
> > > On Tue, Aug 17, 2021 at 1:49 AM Luiz Augusto von Dentz
> > > <luiz.dentz@gmail.com> wrote:
> > > >
> > > > Hi Joseph,
> > > >
> > > > On Sun, Aug 15, 2021 at 6:25 AM <bluez.test.bot@gmail.com> wrote:
> > > > >
> > > > > This is automated email and please do not reply to this email!
> > > > >
> > > > > Dear submitter,
> > > > >
> > > > > Thank you for submitting the patches to the linux bluetooth mailing list.
> > > > > This is a CI test results with your patch series:
> > > > > PW Link:https://patchwork.kernel.org/project/bluetooth/list/?series=531683
> > > > >
> > > > > ---Test result---
> > > > >
> > > > > Test Summary:
> > > > > CheckPatch                    PASS      2.92 seconds
> > > > > GitLint                       PASS      0.61 seconds
> > > > > BuildKernel                   PASS      598.41 seconds
> > > > > TestRunner: Setup             PASS      394.12 seconds
> > > > > TestRunner: l2cap-tester      PASS      2.89 seconds
> > > > > TestRunner: bnep-tester       PASS      2.07 seconds
> > > > > TestRunner: mgmt-tester       FAIL      30.94 seconds
> > > > > TestRunner: rfcomm-tester     PASS      2.31 seconds
> > > > > TestRunner: sco-tester        PASS      2.20 seconds
> > > > > TestRunner: smp-tester        FAIL      2.27 seconds
> > > > > TestRunner: userchan-tester   PASS      2.09 seconds
> > > > >
> > > > > Details
> > > > > ##############################
> > > > > Test: CheckPatch - PASS - 2.92 seconds
> > > > > Run checkpatch.pl script with rule in .checkpatch.conf
> > > > >
> > > > >
> > > > > ##############################
> > > > > Test: GitLint - PASS - 0.61 seconds
> > > > > Run gitlint with rule in .gitlint
> > > > >
> > > > >
> > > > > ##############################
> > > > > Test: BuildKernel - PASS - 598.41 seconds
> > > > > Build Kernel with minimal configuration supports Bluetooth
> > > > >
> > > > >
> > > > > ##############################
> > > > > Test: TestRunner: Setup - PASS - 394.12 seconds
> > > > > Setup environment for running Test Runner
> > > > >
> > > > >
> > > > > ##############################
> > > > > Test: TestRunner: l2cap-tester - PASS - 2.89 seconds
> > > > > Run test-runner with l2cap-tester
> > > > > Total: 40, Passed: 40 (100.0%), Failed: 0, Not Run: 0
> > > > >
> > > > > ##############################
> > > > > Test: TestRunner: bnep-tester - PASS - 2.07 seconds
> > > > > Run test-runner with bnep-tester
> > > > > Total: 1, Passed: 1 (100.0%), Failed: 0, Not Run: 0
> > > > >
> > > > > ##############################
> > > > > Test: TestRunner: mgmt-tester - FAIL - 30.94 seconds
> > > > > Run test-runner with mgmt-tester
> > > > > Total: 448, Passed: 444 (99.1%), Failed: 1, Not Run: 3
> > > > >
> > > > > Failed Test Cases
> > > > > Read Exp Feature - Success                           Failed       0.012 seconds
> > > >
> > > > Looks like there is a regression on mgmt-tester:
> > > >
> > > > Read Exp Feature - Success - run
> > > >   Sending Read Experimental Features Information (0x0049)
> > > >   Test condition added, total 1
> > > >   Read Experimental Features Information (0x0049): Success (0x00)
> > > >   Invalid cmd response parameter size
> > > > Read Exp Feature - Success - test failed
> > > >
> > > > >
> > > > > ##############################
> > > > > Test: TestRunner: rfcomm-tester - PASS - 2.31 seconds
> > > > > Run test-runner with rfcomm-tester
> > > > > Total: 9, Passed: 9 (100.0%), Failed: 0, Not Run: 0
> > > > >
> > > > > ##############################
> > > > > Test: TestRunner: sco-tester - PASS - 2.20 seconds
> > > > > Run test-runner with sco-tester
> > > > > Total: 8, Passed: 8 (100.0%), Failed: 0, Not Run: 0
> > > > >
> > > > > ##############################
> > > > > Test: TestRunner: smp-tester - FAIL - 2.27 seconds
> > > > > Run test-runner with smp-tester
> > > > > Total: 8, Passed: 7 (87.5%), Failed: 1, Not Run: 0
> > > > >
> > > > > Failed Test Cases
> > > > > SMP Client - SC Request 2                            Failed       0.024 seconds
> > > > >
> > > > > ##############################
> > > > > Test: TestRunner: userchan-tester - PASS - 2.09 seconds
> > > > > Run test-runner with userchan-tester
> > > > > Total: 3, Passed: 3 (100.0%), Failed: 0, Not Run: 0
> > > > >
> > > > >
> > > > >
> > > > > ---
> > > > > Regards,
> > > > > Linux Bluetooth
> > > > >
> > > >
> > > >
> > > > --
> > > > Luiz Augusto von Dentz
> > >
> > >
> > >
> > > --
> > >
> > > Joseph Shyh-In Hwang
> > > Email: josephsih@google.com
> >
> >
> >
> > --
> >
> > Joseph Shyh-In Hwang
> > Email: josephsih@google.com
>
>
>
> --
> Luiz Augusto von Dentz



-- 

Joseph Shyh-In Hwang
Email: josephsih@google.com

Re: [v9,1/5] Bluetooth: btusb: disable Intel link statistics telemetry events

[Joseph Hwang]

(Resent in plaintext mode)

Hi Luiz:

  I have fixed the mgmt-tester on the bluez side. The "Read Exp
Feature - Success" test passed on my local test setup.

  Please let me know if there are any issues.

Thanks and regards,
Joseph


On Fri, Aug 20, 2021 at 1:27 PM Joseph Hwang <josephsih@google.com> wrote:
>
> Hi Luiz:
>
> The mgmt-tester change to add the UUID of the new exp feature is just
> sent for review:
>
> https://patchwork.kernel.org/project/bluetooth/patch/20210820131751.BlueZ.v1.1.I165b6fc2b20d80c8d18946434005f0269d92f489@changeid/
>
>   The tester checks the exp features UUIDs and FLAGs. As the FLAG
> values depend on how the test is set up. I set the flags to be all
> 0x00 which passed the mgmt-tester in my local test setup. If that is
> not the case upstream, please let me know.
>
> Thanks and regards,
> Joseph
>
> On Fri, Aug 20, 2021 at 2:04 AM Luiz Augusto von Dentz
> <luiz.dentz@gmail.com> wrote:
> >
> > Hi Joseph,
> >
> > On Tue, Aug 17, 2021 at 10:49 PM Joseph Hwang <josephsih@google.com> wrote:
> > >
> > > Hi Luiz:
> > >
> > >   I am wondering if it is possible to merge these kernel patches
> > > before fixing the mgmt-tester?
> > >
> > >   The mgmt-tester failed due to its checking against the fixed exp
> > > feature count and the corresponding exp UUIDs and flags. A more
> > > flexible tester may be required so that the tester would not be broken
> > > whenever a new exp feature is added.
> >
> > I would prefer to have the mgmt-tester changes applied as well so we
> > don't have false positives for other patches causing mgmt-tester to
> > fail, in fact we include CI is useful to validate this changes so we
> > can check the feature is correctly exposed to userspace and UUID is
> > properly formatted, etc, it shouldn't be a big change to mgmt-tester
> > thought if you just include the new UUID later on we can think about
> > how we could make it more extensible so it doesn't break when a new
> > feature is added.
> >
> > > On Tue, Aug 17, 2021 at 5:54 PM Joseph Hwang <josephsih@google.com> wrote:
> > > >
> > > > (Resent this email to contain only plaintext.)
> > > >
> > > > Hi Luiz:
> > > >
> > > >   It seems that mgmt-tester currently uses a fixed feature count.
> > > > Every time a new exp feature is added, the mgmt-tester would  be
> > > > broken. By checking the kernel, it seems that there are currently 2 or
> > > > 3 exp features, i.e., debug uuid, simultaneous central peripheral
> > > > uuid, and LL privacy uuid. Note that the debug exp feature is guarded
> > > > by CONFIG_BT_FEATURE_DEBUG. So I am not sure how the kernel is
> > > > configured and made on your test setup.
> > > >
> > > >   If we fix the mgmt-tester in bluez to have 3 or 4 (which one?)
> > > > features before merging the kernel changes here that adds a new
> > > > quality exp feature, it would not match the existing kernel which has
> > > > only 2 or 3 features.
> > > >
> > > >   Do you have any preference about how to fix the mgmt-tester?
> > > >
> > > >   My suggestion is to remove the checking of the feature count from
> > > > the mgmt-tester. The feature count changes over time. It is possible
> > > > to implement a customized exp_feat_check function which can be more
> > > > flexible. If a uuid is found in the MGMT response, its associated
> > > > flags are checked against.
> > > >
> > > >   The data currently used in the mgmt-tester:
> > > >
> > > > static const uint8_t read_exp_feat_param_success[] = {
> > > > 0x02, 0x00, /* Feature Count */
> > > > 0xd6, 0x49, 0xb0, 0xd1, 0x28, 0xeb, /* UUID - Simultaneous */
> > > > 0x27, 0x92, 0x96, 0x46, 0xc0, 0x42, /* Central Peripheral */
> > > > 0xb5, 0x10, 0x1b, 0x67,
> > > > 0x00, 0x00, 0x00, 0x00, /* Flags */
> > > > 0x04, 0x00, 0x13, 0xac, 0x42, 0x02, /* UUID - LL Privacy */
> > > > 0xde, 0xb3, 0xea, 0x11, 0x73, 0xc2,
> > > > 0x48, 0xa1, 0xc0, 0x15,
> > > > 0x02, 0x00, 0x00, 0x00, /* Flags */
> > > > };
> > > >
> > > >
> > > >   Please let me know what you think.
> > > >
> > > > Thanks and regards,
> > > > Joseph
> > > >
> > > >
> > > > On Tue, Aug 17, 2021 at 1:49 AM Luiz Augusto von Dentz
> > > > <luiz.dentz@gmail.com> wrote:
> > > > >
> > > > > Hi Joseph,
> > > > >
> > > > > On Sun, Aug 15, 2021 at 6:25 AM <bluez.test.bot@gmail.com> wrote:
> > > > > >
> > > > > > This is automated email and please do not reply to this email!
> > > > > >
> > > > > > Dear submitter,
> > > > > >
> > > > > > Thank you for submitting the patches to the linux bluetooth mailing list.
> > > > > > This is a CI test results with your patch series:
> > > > > > PW Link:https://patchwork.kernel.org/project/bluetooth/list/?series=531683
> > > > > >
> > > > > > ---Test result---
> > > > > >
> > > > > > Test Summary:
> > > > > > CheckPatch                    PASS      2.92 seconds
> > > > > > GitLint                       PASS      0.61 seconds
> > > > > > BuildKernel                   PASS      598.41 seconds
> > > > > > TestRunner: Setup             PASS      394.12 seconds
> > > > > > TestRunner: l2cap-tester      PASS      2.89 seconds
> > > > > > TestRunner: bnep-tester       PASS      2.07 seconds
> > > > > > TestRunner: mgmt-tester       FAIL      30.94 seconds
> > > > > > TestRunner: rfcomm-tester     PASS      2.31 seconds
> > > > > > TestRunner: sco-tester        PASS      2.20 seconds
> > > > > > TestRunner: smp-tester        FAIL      2.27 seconds
> > > > > > TestRunner: userchan-tester   PASS      2.09 seconds
> > > > > >
> > > > > > Details
> > > > > > ##############################
> > > > > > Test: CheckPatch - PASS - 2.92 seconds
> > > > > > Run checkpatch.pl script with rule in .checkpatch.conf
> > > > > >
> > > > > >
> > > > > > ##############################
> > > > > > Test: GitLint - PASS - 0.61 seconds
> > > > > > Run gitlint with rule in .gitlint
> > > > > >
> > > > > >
> > > > > > ##############################
> > > > > > Test: BuildKernel - PASS - 598.41 seconds
> > > > > > Build Kernel with minimal configuration supports Bluetooth
> > > > > >
> > > > > >
> > > > > > ##############################
> > > > > > Test: TestRunner: Setup - PASS - 394.12 seconds
> > > > > > Setup environment for running Test Runner
> > > > > >
> > > > > >
> > > > > > ##############################
> > > > > > Test: TestRunner: l2cap-tester - PASS - 2.89 seconds
> > > > > > Run test-runner with l2cap-tester
> > > > > > Total: 40, Passed: 40 (100.0%), Failed: 0, Not Run: 0
> > > > > >
> > > > > > ##############################
> > > > > > Test: TestRunner: bnep-tester - PASS - 2.07 seconds
> > > > > > Run test-runner with bnep-tester
> > > > > > Total: 1, Passed: 1 (100.0%), Failed: 0, Not Run: 0
> > > > > >
> > > > > > ##############################
> > > > > > Test: TestRunner: mgmt-tester - FAIL - 30.94 seconds
> > > > > > Run test-runner with mgmt-tester
> > > > > > Total: 448, Passed: 444 (99.1%), Failed: 1, Not Run: 3
> > > > > >
> > > > > > Failed Test Cases
> > > > > > Read Exp Feature - Success                           Failed       0.012 seconds
> > > > >
> > > > > Looks like there is a regression on mgmt-tester:
> > > > >
> > > > > Read Exp Feature - Success - run
> > > > >   Sending Read Experimental Features Information (0x0049)
> > > > >   Test condition added, total 1
> > > > >   Read Experimental Features Information (0x0049): Success (0x00)
> > > > >   Invalid cmd response parameter size
> > > > > Read Exp Feature - Success - test failed
> > > > >
> > > > > >
> > > > > > ##############################
> > > > > > Test: TestRunner: rfcomm-tester - PASS - 2.31 seconds
> > > > > > Run test-runner with rfcomm-tester
> > > > > > Total: 9, Passed: 9 (100.0%), Failed: 0, Not Run: 0
> > > > > >
> > > > > > ##############################
> > > > > > Test: TestRunner: sco-tester - PASS - 2.20 seconds
> > > > > > Run test-runner with sco-tester
> > > > > > Total: 8, Passed: 8 (100.0%), Failed: 0, Not Run: 0
> > > > > >
> > > > > > ##############################
> > > > > > Test: TestRunner: smp-tester - FAIL - 2.27 seconds
> > > > > > Run test-runner with smp-tester
> > > > > > Total: 8, Passed: 7 (87.5%), Failed: 1, Not Run: 0
> > > > > >
> > > > > > Failed Test Cases
> > > > > > SMP Client - SC Request 2                            Failed       0.024 seconds
> > > > > >
> > > > > > ##############################
> > > > > > Test: TestRunner: userchan-tester - PASS - 2.09 seconds
> > > > > > Run test-runner with userchan-tester
> > > > > > Total: 3, Passed: 3 (100.0%), Failed: 0, Not Run: 0
> > > > > >
> > > > > >
> > > > > >
> > > > > > ---
> > > > > > Regards,
> > > > > > Linux Bluetooth
> > > > > >
> > > > >
> > > > >
> > > > > --
> > > > > Luiz Augusto von Dentz
> > > >
> > > >
> > > >
> > > > --
> > > >
> > > > Joseph Shyh-In Hwang
> > > > Email: josephsih@google.com
> > >
> > >
> > >
> > > --
> > >
> > > Joseph Shyh-In Hwang
> > > Email: josephsih@google.com
> >
> >
> >
> > --
> > Luiz Augusto von Dentz
>
>
>
> --
>
> Joseph Shyh-In Hwang
> Email: josephsih@google.com



-- 

Joseph Shyh-In Hwang
Email: josephsih@google.com

Re: [v9,1/5] Bluetooth: btusb: disable Intel link statistics telemetry events

[Luiz Augusto von Dentz]

Hi Joseph,

On Wed, Aug 25, 2021 at 5:02 PM Joseph Hwang <josephsih@google.com> wrote:
>
> (Resent in plaintext mode)
>
> Hi Luiz:
>
>   I have fixed the mgmt-tester on the bluez side. The "Read Exp
> Feature - Success" test passed on my local test setup.
>
>   Please let me know if there are any issues.

Applied, thanks.