2021-04-13 10:19:35

by Joseph Hwang

[permalink] [raw]
Subject: [PATCH v3 1/2] Bluetooth: btusb: support link statistics telemetry events

From: Chethan T N <[email protected]>

This patch supports the link statistics telemetry events for
Intel controllers

To avoid the overhead, this debug feature is disabled by default.

Reviewed-by: Miao-chen Chou <[email protected]>
Signed-off-by: Chethan T N <[email protected]>
Signed-off-by: Kiran K <[email protected]>
Signed-off-by: Joseph Hwang <[email protected]>
---

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 +++++++++++++++++++-
drivers/bluetooth/btusb.c | 18 ------------------
2 files changed, 19 insertions(+), 19 deletions(-)

diff --git a/drivers/bluetooth/btintel.c b/drivers/bluetooth/btintel.c
index e44b6993cf91..de1dbdc01e5a 100644
--- a/drivers/bluetooth/btintel.c
+++ b/drivers/bluetooth/btintel.c
@@ -1248,8 +1248,10 @@ EXPORT_SYMBOL_GPL(btintel_read_debug_features);
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, 0x01, 0x00 };
+ u8 trace_enable = 0x02;
struct sk_buff *skb;

if (!features)
@@ -1266,8 +1268,24 @@ 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;
}
EXPORT_SYMBOL_GPL(btintel_set_debug_features);
diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
index 192cb8c191bc..f29946f15f59 100644
--- a/drivers/bluetooth/btusb.c
+++ b/drivers/bluetooth/btusb.c
@@ -2811,7 +2811,6 @@ static int btusb_setup_intel_new(struct hci_dev *hdev)
u32 boot_param;
char ddcname[64];
int err;
- struct intel_debug_features features;

BT_DBG("%s", hdev->name);

@@ -2865,14 +2864,6 @@ static int btusb_setup_intel_new(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.
- */
- btintel_read_debug_features(hdev, &features);
-
- /* 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, &ver);
if (err)
@@ -2911,7 +2902,6 @@ static int btusb_setup_intel_newgen(struct hci_dev *hdev)
u32 boot_param;
char ddcname[64];
int err;
- struct intel_debug_features features;
struct intel_version_tlv version;

bt_dev_dbg(hdev, "");
@@ -2961,14 +2951,6 @@ static int btusb_setup_intel_newgen(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.
- */
- btintel_read_debug_features(hdev, &features);
-
- /* 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, &version);
if (err)
--
2.31.1.295.g9ea45b61b8-goog


2021-04-14 05:55:31

by Marcel Holtmann

[permalink] [raw]
Subject: Re: [PATCH v3 1/2] Bluetooth: btusb: support link statistics telemetry events

Hi Joseph,

> This patch supports the link statistics telemetry events for
> Intel controllers
>
> To avoid the overhead, this debug feature is disabled by default.
>
> Reviewed-by: Miao-chen Chou <[email protected]>
> Signed-off-by: Chethan T N <[email protected]>
> Signed-off-by: Kiran K <[email protected]>
> Signed-off-by: Joseph Hwang <[email protected]>
> ---
>
> 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 +++++++++++++++++++-
> drivers/bluetooth/btusb.c | 18 ------------------
> 2 files changed, 19 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/bluetooth/btintel.c b/drivers/bluetooth/btintel.c
> index e44b6993cf91..de1dbdc01e5a 100644
> --- a/drivers/bluetooth/btintel.c
> +++ b/drivers/bluetooth/btintel.c
> @@ -1248,8 +1248,10 @@ EXPORT_SYMBOL_GPL(btintel_read_debug_features);
> 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, 0x01, 0x00 };
> + u8 trace_enable = 0x02;
> struct sk_buff *skb;
>
> if (!features)
> @@ -1266,8 +1268,24 @@ 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;
> }
> EXPORT_SYMBOL_GPL(btintel_set_debug_features);
> diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
> index 192cb8c191bc..f29946f15f59 100644
> --- a/drivers/bluetooth/btusb.c
> +++ b/drivers/bluetooth/btusb.c
> @@ -2811,7 +2811,6 @@ static int btusb_setup_intel_new(struct hci_dev *hdev)
> u32 boot_param;
> char ddcname[64];
> int err;
> - struct intel_debug_features features;
>
> BT_DBG("%s", hdev->name);
>
> @@ -2865,14 +2864,6 @@ static int btusb_setup_intel_new(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.
> - */
> - btintel_read_debug_features(hdev, &features);
> -
> - /* 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, &ver);
> if (err)
> @@ -2911,7 +2902,6 @@ static int btusb_setup_intel_newgen(struct hci_dev *hdev)
> u32 boot_param;
> char ddcname[64];
> int err;
> - struct intel_debug_features features;
> struct intel_version_tlv version;
>
> bt_dev_dbg(hdev, "");
> @@ -2961,14 +2951,6 @@ static int btusb_setup_intel_newgen(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.
> - */
> - btintel_read_debug_features(hdev, &features);
> -
> - /* 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, &version);
> if (err)

so I don’t like this kind of removing and adding things at the same time. Please separate these patches properly.

Regards

Marcel