linux-kernel - Re: [PATCH v3 1/2] Bluetooth: btusb: support link statistics telemetry events

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <CABBYNZK7a0XMmhrRBg=6ByuaOD0yq8gSkg87VG3=U2RKfX9FGA@mail.gmail.com>
Date:   Tue, 13 Apr 2021 15:04:19 -0700
From:   Luiz Augusto von Dentz <luiz.dentz@...il.com>
To:     Joseph Hwang <josephsih@...omium.org>
Cc:     "linux-bluetooth@...r.kernel.org" <linux-bluetooth@...r.kernel.org>,
        Marcel Holtmann <marcel@...tmann.org>,
        Pali Rohár <pali@...nel.org>,
        Joseph Hwang <josephsih@...gle.com>,
        ChromeOS Bluetooth Upstreaming 
        <chromeos-bluetooth-upstreaming@...omium.org>,
        Chethan T N <chethan.tumkur.narayan@...el.com>,
        Miao-chen Chou <mcchou@...omium.org>,
        Kiran K <kiran.k@...el.com>,
        Johan Hedberg <johan.hedberg@...il.com>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v3 1/2] Bluetooth: btusb: support link statistics
 telemetry events

Hi Joseph,

On Tue, Apr 13, 2021 at 3:18 AM Joseph Hwang <josephsih@...omium.org> wrote:
>
> From: Chethan T N <chethan.tumkur.narayan@...el.com>
>
> 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 <mcchou@...omium.org>
> Signed-off-by: Chethan T N <chethan.tumkur.narayan@...el.com>
> Signed-off-by: Kiran K <kiran.k@...el.com>
> Signed-off-by: Joseph Hwang <josephsih@...omium.org>
> ---
>
> 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;

Looks like I commented on the wrong version, anyway the comments I
made on v2 also apply here.

>         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
>


-- 
Luiz Augusto von Dentz