2024-02-13 15:49:48

by K, Kiran

[permalink] [raw]
Subject: [PATCH v1] Bluetooth: btintel: Print Firmware Sequencer information

Firmware sequencer(FSEQ) is a common code shared across Bluetooth
and Wifi. Printing FSEQ will help to debug if there is any mismatch
between Bluetooth and Wifi FSEQ.

Signed-off-by: Kiran K <[email protected]>
---
drivers/bluetooth/btintel.c | 106 ++++++++++++++++++++++++++++++++++++
1 file changed, 106 insertions(+)

diff --git a/drivers/bluetooth/btintel.c b/drivers/bluetooth/btintel.c
index e5b043d96207..0d067ee39408 100644
--- a/drivers/bluetooth/btintel.c
+++ b/drivers/bluetooth/btintel.c
@@ -2670,6 +2670,111 @@ static void btintel_set_msft_opcode(struct hci_dev *hdev, u8 hw_variant)
}
}

+static void btintel_print_fseq_info(struct hci_dev *hdev)
+{
+ struct sk_buff *skb;
+ u8 *p;
+ const char *str;
+
+ skb = __hci_cmd_sync(hdev, 0xfcb3, 0, NULL, HCI_CMD_TIMEOUT);
+ if (IS_ERR(skb)) {
+ bt_dev_dbg(hdev, "Reading fseq status command failed (%ld)",
+ PTR_ERR(skb));
+ return;
+ }
+
+ if (skb->len < (sizeof(u32) * 16 + 2)) {
+ bt_dev_dbg(hdev, "Malformed packet");
+ kfree_skb(skb);
+ return;
+ }
+
+ if (skb->data[0]) {
+ bt_dev_dbg(hdev, "Failed to get fseq status (0x%2.2x)",
+ skb->data[0]);
+ kfree_skb(skb);
+ return;
+ }
+
+ p = skb->data;
+ /* skip status */
+ p = p + 1;
+
+ switch (*p) {
+ case 0:
+ str = "Success";
+ break;
+ case 1:
+ str = "Fatal error";
+ break;
+ case 2:
+ str = "Sem acq error";
+ break;
+ default:
+ str = "Unknown error";
+ break;
+ }
+
+ bt_dev_info(hdev, "Fseq status: %s (0x%2.2x)", str, *p);
+ if (*p)
+ return;
+ p = p + 1;
+ bt_dev_dbg(hdev, "Reason: 0x%8.8x", get_unaligned_le32(p));
+
+ p = p + 4;
+ bt_dev_dbg(hdev, "Global version: 0x%8.8x", get_unaligned_le32(p));
+
+ p = p + 4;
+ bt_dev_dbg(hdev, "Installed version: 0x%8.8x", get_unaligned_le32(p));
+
+ p = p + 4;
+ bt_dev_info(hdev, "Fseq executed: %2.2u.%2.2u.%2.2u.%2.2u", p[0], p[1],
+ p[2], p[3]);
+
+ p = p + 4;
+ bt_dev_info(hdev, "Fseq BT Top: %2.2u.%2.2u.%2.2u.%2.2u", p[0], p[1],
+ p[2], p[3]);
+
+ p = p + 4;
+ bt_dev_dbg(hdev, "Fseq Top init version: 0x%8.8x", get_unaligned_le32(p));
+
+ p = p + 4;
+ bt_dev_dbg(hdev, "Fseq Cnvio init version: 0x%8.8x", get_unaligned_le32(p));
+
+ p = p + 4;
+ bt_dev_dbg(hdev, "Fseq MBX Wifi file version: 0x%8.8x", get_unaligned_le32(p));
+
+ p = p + 4;
+ bt_dev_dbg(hdev, "Fseq BT version: 0x%8.8x", get_unaligned_le32(p));
+
+ p = p + 4;
+ bt_dev_dbg(hdev, "Fseq Top reset address: 0x%8.8x", get_unaligned_le32(p));
+
+ p = p + 4;
+ bt_dev_dbg(hdev, "Fseq MBX timeout: 0x%8.8x", get_unaligned_le32(p));
+
+ p = p + 4;
+ bt_dev_dbg(hdev, "Fseq MBX ack: 0x%8.8x", get_unaligned_le32(p));
+
+ p = p + 4;
+ bt_dev_dbg(hdev, "Fseq CNVi id: 0x%8.8x", get_unaligned_le32(p));
+
+ p = p + 4;
+ bt_dev_dbg(hdev, "Fseq CNVr id: 0x%8.8x", get_unaligned_le32(p));
+
+ p = p + 4;
+ bt_dev_dbg(hdev, "Fseq Error handle: 0x%8.8x", get_unaligned_le32(p));
+
+ p = p + 4;
+ bt_dev_dbg(hdev, "Fseq Magic noalive indication: 0x%8.8x", get_unaligned_le32(p));
+
+ p = p + 4;
+ bt_dev_dbg(hdev, "Fseq OTP version: 0x%8.8x", get_unaligned_le32(p));
+
+ p = p + 4;
+ bt_dev_dbg(hdev, "Fseq MBX otp version: 0x%8.8x", get_unaligned_le32(p));
+}
+
static int btintel_setup_combined(struct hci_dev *hdev)
{
const u8 param[1] = { 0xFF };
@@ -2902,6 +3007,7 @@ static int btintel_setup_combined(struct hci_dev *hdev)

err = btintel_bootloader_setup_tlv(hdev, &ver_tlv);
btintel_register_devcoredump_support(hdev);
+ btintel_print_fseq_info(hdev);
break;
default:
bt_dev_err(hdev, "Unsupported Intel hw variant (%u)",
--
2.34.1



2024-02-13 16:06:35

by Paul Menzel

[permalink] [raw]
Subject: Re: [PATCH v1] Bluetooth: btintel: Print Firmware Sequencer information

Dear Kiran,


Thank you for your patch.

Am 13.02.24 um 17:01 schrieb Kiran K:
> Firmware sequencer(FSEQ) is a common code shared across Bluetooth

Please add a space before (.

> and Wifi. Printing FSEQ will help to debug if there is any mismatch
> between Bluetooth and Wifi FSEQ.

Please give an example output, and document the system, you tested this on.

> Signed-off-by: Kiran K <[email protected]>
> ---
> drivers/bluetooth/btintel.c | 106 ++++++++++++++++++++++++++++++++++++
> 1 file changed, 106 insertions(+)
>
> diff --git a/drivers/bluetooth/btintel.c b/drivers/bluetooth/btintel.c
> index e5b043d96207..0d067ee39408 100644
> --- a/drivers/bluetooth/btintel.c
> +++ b/drivers/bluetooth/btintel.c
> @@ -2670,6 +2670,111 @@ static void btintel_set_msft_opcode(struct hci_dev *hdev, u8 hw_variant)
> }
> }
>
> +static void btintel_print_fseq_info(struct hci_dev *hdev)
> +{
> + struct sk_buff *skb;
> + u8 *p;
> + const char *str;
> +
> + skb = __hci_cmd_sync(hdev, 0xfcb3, 0, NULL, HCI_CMD_TIMEOUT);
> + if (IS_ERR(skb)) {
> + bt_dev_dbg(hdev, "Reading fseq status command failed (%ld)",
> + PTR_ERR(skb));
> + return;
> + }
> +
> + if (skb->len < (sizeof(u32) * 16 + 2)) {
> + bt_dev_dbg(hdev, "Malformed packet");

Please print out the length values.

> + kfree_skb(skb);
> + return;
> + }
> +
> + if (skb->data[0]) {
> + bt_dev_dbg(hdev, "Failed to get fseq status (0x%2.2x)",
> + skb->data[0]);
> + kfree_skb(skb);
> + return;
> + }
> +
> + p = skb->data;
> + /* skip status */
> + p = p + 1;
> +
> + switch (*p) {
> + case 0:
> + str = "Success";
> + break;
> + case 1:
> + str = "Fatal error";
> + break;
> + case 2:
> + str = "Sem acq error";

Maybe elaborate here?

> + break;
> + default:
> + str = "Unknown error";
> + break;
> + }
> +
> + bt_dev_info(hdev, "Fseq status: %s (0x%2.2x)", str, *p);
> + if (*p)
> + return;

Should non-success levels have a different log level?

> + p = p + 1;
> + bt_dev_dbg(hdev, "Reason: 0x%8.8x", get_unaligned_le32(p));
> +
> + p = p + 4;
> + bt_dev_dbg(hdev, "Global version: 0x%8.8x", get_unaligned_le32(p));
> +
> + p = p + 4;
> + bt_dev_dbg(hdev, "Installed version: 0x%8.8x", get_unaligned_le32(p));
> +
> + p = p + 4;
> + bt_dev_info(hdev, "Fseq executed: %2.2u.%2.2u.%2.2u.%2.2u", p[0], p[1],
> + p[2], p[3]);
> +
> + p = p + 4;
> + bt_dev_info(hdev, "Fseq BT Top: %2.2u.%2.2u.%2.2u.%2.2u", p[0], p[1],
> + p[2], p[3]);
> +
> + p = p + 4;
> + bt_dev_dbg(hdev, "Fseq Top init version: 0x%8.8x", get_unaligned_le32(p));
> +
> + p = p + 4;
> + bt_dev_dbg(hdev, "Fseq Cnvio init version: 0x%8.8x", get_unaligned_le32(p));
> +
> + p = p + 4;
> + bt_dev_dbg(hdev, "Fseq MBX Wifi file version: 0x%8.8x", get_unaligned_le32(p));
> +
> + p = p + 4;
> + bt_dev_dbg(hdev, "Fseq BT version: 0x%8.8x", get_unaligned_le32(p));
> +
> + p = p + 4;
> + bt_dev_dbg(hdev, "Fseq Top reset address: 0x%8.8x", get_unaligned_le32(p));
> +
> + p = p + 4;
> + bt_dev_dbg(hdev, "Fseq MBX timeout: 0x%8.8x", get_unaligned_le32(p));
> +
> + p = p + 4;
> + bt_dev_dbg(hdev, "Fseq MBX ack: 0x%8.8x", get_unaligned_le32(p));
> +
> + p = p + 4;
> + bt_dev_dbg(hdev, "Fseq CNVi id: 0x%8.8x", get_unaligned_le32(p));
> +
> + p = p + 4;
> + bt_dev_dbg(hdev, "Fseq CNVr id: 0x%8.8x", get_unaligned_le32(p));
> +
> + p = p + 4;
> + bt_dev_dbg(hdev, "Fseq Error handle: 0x%8.8x", get_unaligned_le32(p));
> +
> + p = p + 4;
> + bt_dev_dbg(hdev, "Fseq Magic noalive indication: 0x%8.8x", get_unaligned_le32(p));
> +
> + p = p + 4;
> + bt_dev_dbg(hdev, "Fseq OTP version: 0x%8.8x", get_unaligned_le32(p));
> +
> + p = p + 4;
> + bt_dev_dbg(hdev, "Fseq MBX otp version: 0x%8.8x", get_unaligned_le32(p));
> +}
> +
> static int btintel_setup_combined(struct hci_dev *hdev)
> {
> const u8 param[1] = { 0xFF };
> @@ -2902,6 +3007,7 @@ static int btintel_setup_combined(struct hci_dev *hdev)
>
> err = btintel_bootloader_setup_tlv(hdev, &ver_tlv);
> btintel_register_devcoredump_support(hdev);
> + btintel_print_fseq_info(hdev);
> break;
> default:
> bt_dev_err(hdev, "Unsupported Intel hw variant (%u)",


Kind regards,

Paul

2024-02-13 16:11:54

by Luiz Augusto von Dentz

[permalink] [raw]
Subject: Re: [PATCH v1] Bluetooth: btintel: Print Firmware Sequencer information

Hi Kiran,

On Tue, Feb 13, 2024 at 10:51 AM Kiran K <[email protected]> wrote:
>
> Firmware sequencer(FSEQ) is a common code shared across Bluetooth
> and Wifi. Printing FSEQ will help to debug if there is any mismatch
> between Bluetooth and Wifi FSEQ.
>
> Signed-off-by: Kiran K <[email protected]>
> ---
> drivers/bluetooth/btintel.c | 106 ++++++++++++++++++++++++++++++++++++
> 1 file changed, 106 insertions(+)
>
> diff --git a/drivers/bluetooth/btintel.c b/drivers/bluetooth/btintel.c
> index e5b043d96207..0d067ee39408 100644
> --- a/drivers/bluetooth/btintel.c
> +++ b/drivers/bluetooth/btintel.c
> @@ -2670,6 +2670,111 @@ static void btintel_set_msft_opcode(struct hci_dev *hdev, u8 hw_variant)
> }
> }
>
> +static void btintel_print_fseq_info(struct hci_dev *hdev)
> +{
> + struct sk_buff *skb;
> + u8 *p;
> + const char *str;
> +
> + skb = __hci_cmd_sync(hdev, 0xfcb3, 0, NULL, HCI_CMD_TIMEOUT);
> + if (IS_ERR(skb)) {
> + bt_dev_dbg(hdev, "Reading fseq status command failed (%ld)",
> + PTR_ERR(skb));
> + return;
> + }
> +
> + if (skb->len < (sizeof(u32) * 16 + 2)) {
> + bt_dev_dbg(hdev, "Malformed packet");
> + kfree_skb(skb);
> + return;
> + }
> +
> + if (skb->data[0]) {
> + bt_dev_dbg(hdev, "Failed to get fseq status (0x%2.2x)",
> + skb->data[0]);
> + kfree_skb(skb);
> + return;
> + }
> +
> + p = skb->data;
> + /* skip status */
> + p = p + 1;

How about we use skb_pull_data instead of accessing these fields with
a pointer cursor?

> + switch (*p) {
> + case 0:
> + str = "Success";
> + break;
> + case 1:
> + str = "Fatal error";
> + break;
> + case 2:
> + str = "Sem acq error";
> + break;
> + default:
> + str = "Unknown error";
> + break;
> + }
> +
> + bt_dev_info(hdev, "Fseq status: %s (0x%2.2x)", str, *p);
> + if (*p)
> + return;
> + p = p + 1;
> + bt_dev_dbg(hdev, "Reason: 0x%8.8x", get_unaligned_le32(p));
> +
> + p = p + 4;
> + bt_dev_dbg(hdev, "Global version: 0x%8.8x", get_unaligned_le32(p));
> +
> + p = p + 4;
> + bt_dev_dbg(hdev, "Installed version: 0x%8.8x", get_unaligned_le32(p));
> +
> + p = p + 4;
> + bt_dev_info(hdev, "Fseq executed: %2.2u.%2.2u.%2.2u.%2.2u", p[0], p[1],
> + p[2], p[3]);
> +
> + p = p + 4;
> + bt_dev_info(hdev, "Fseq BT Top: %2.2u.%2.2u.%2.2u.%2.2u", p[0], p[1],
> + p[2], p[3]);
> +
> + p = p + 4;
> + bt_dev_dbg(hdev, "Fseq Top init version: 0x%8.8x", get_unaligned_le32(p));
> +
> + p = p + 4;
> + bt_dev_dbg(hdev, "Fseq Cnvio init version: 0x%8.8x", get_unaligned_le32(p));
> +
> + p = p + 4;
> + bt_dev_dbg(hdev, "Fseq MBX Wifi file version: 0x%8.8x", get_unaligned_le32(p));
> +
> + p = p + 4;
> + bt_dev_dbg(hdev, "Fseq BT version: 0x%8.8x", get_unaligned_le32(p));
> +
> + p = p + 4;
> + bt_dev_dbg(hdev, "Fseq Top reset address: 0x%8.8x", get_unaligned_le32(p));
> +
> + p = p + 4;
> + bt_dev_dbg(hdev, "Fseq MBX timeout: 0x%8.8x", get_unaligned_le32(p));
> +
> + p = p + 4;
> + bt_dev_dbg(hdev, "Fseq MBX ack: 0x%8.8x", get_unaligned_le32(p));
> +
> + p = p + 4;
> + bt_dev_dbg(hdev, "Fseq CNVi id: 0x%8.8x", get_unaligned_le32(p));
> +
> + p = p + 4;
> + bt_dev_dbg(hdev, "Fseq CNVr id: 0x%8.8x", get_unaligned_le32(p));
> +
> + p = p + 4;
> + bt_dev_dbg(hdev, "Fseq Error handle: 0x%8.8x", get_unaligned_le32(p));
> +
> + p = p + 4;
> + bt_dev_dbg(hdev, "Fseq Magic noalive indication: 0x%8.8x", get_unaligned_le32(p));
> +
> + p = p + 4;
> + bt_dev_dbg(hdev, "Fseq OTP version: 0x%8.8x", get_unaligned_le32(p));
> +
> + p = p + 4;
> + bt_dev_dbg(hdev, "Fseq MBX otp version: 0x%8.8x", get_unaligned_le32(p));
> +}
> +
> static int btintel_setup_combined(struct hci_dev *hdev)
> {
> const u8 param[1] = { 0xFF };
> @@ -2902,6 +3007,7 @@ static int btintel_setup_combined(struct hci_dev *hdev)
>
> err = btintel_bootloader_setup_tlv(hdev, &ver_tlv);
> btintel_register_devcoredump_support(hdev);
> + btintel_print_fseq_info(hdev);
> break;
> default:
> bt_dev_err(hdev, "Unsupported Intel hw variant (%u)",
> --
> 2.34.1
>
>


--
Luiz Augusto von Dentz

2024-02-13 16:36:55

by bluez.test.bot

[permalink] [raw]
Subject: RE: [v1] Bluetooth: btintel: Print Firmware Sequencer information

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

---Test result---

Test Summary:
CheckPatch PASS 0.67 seconds
GitLint PASS 0.29 seconds
SubjectPrefix PASS 0.11 seconds
BuildKernel PASS 27.91 seconds
CheckAllWarning PASS 30.37 seconds
CheckSparse PASS 35.88 seconds
CheckSmatch PASS 98.23 seconds
BuildKernel32 PASS 26.85 seconds
TestRunnerSetup PASS 497.72 seconds
TestRunner_l2cap-tester FAIL 12.19 seconds
TestRunner_iso-tester PASS 28.46 seconds
TestRunner_bnep-tester PASS 4.84 seconds
TestRunner_mgmt-tester FAIL 171.45 seconds
TestRunner_rfcomm-tester PASS 7.39 seconds
TestRunner_sco-tester PASS 14.97 seconds
TestRunner_ioctl-tester PASS 7.80 seconds
TestRunner_mesh-tester PASS 5.92 seconds
TestRunner_smp-tester PASS 6.84 seconds
TestRunner_userchan-tester PASS 4.96 seconds
IncrementalBuild PASS 25.81 seconds

Details
##############################
Test: TestRunner_l2cap-tester - FAIL
Desc: Run l2cap-tester with test-runner
Output:
No test result found
##############################
Test: TestRunner_mgmt-tester - FAIL
Desc: Run mgmt-tester with test-runner
Output:
Total: 492, Passed: 486 (98.8%), Failed: 5, Not Run: 1

Failed Test Cases
LL Privacy - Add Device 4 (2 Devices to AL) Failed 0.115 seconds
LL Privacy - Add Device 5 (2 Devices to RL) Failed 0.119 seconds
LL Privacy - Add Device 6 (RL is full) Failed 0.144 seconds
LL Privacy - Remove Device 2 (Remove from RL) Timed out 2.510 seconds
LL Privacy - Remove Device 4 (Disable Adv) Timed out 1.843 seconds


---
Regards,
Linux Bluetooth

2024-02-21 13:12:35

by K, Kiran

[permalink] [raw]
Subject: RE: [PATCH v1] Bluetooth: btintel: Print Firmware Sequencer information

Hi Paul,

Thanks for your comments.

> -----Original Message-----
> From: Paul Menzel <[email protected]>
> Sent: Tuesday, February 13, 2024 9:35 PM
> To: K, Kiran <[email protected]>
> Cc: Srivatsa, Ravishankar <[email protected]>; Tumkur Narayan,
> Chethan <[email protected]>; linux-
> [email protected]
> Subject: Re: [PATCH v1] Bluetooth: btintel: Print Firmware Sequencer
> information
>
> Dear Kiran,
>
>
> Thank you for your patch.
>
> Am 13.02.24 um 17:01 schrieb Kiran K:
> > Firmware sequencer(FSEQ) is a common code shared across Bluetooth
>
> Please add a space before (.
Ack

> > and Wifi. Printing FSEQ will help to debug if there is any mismatch
> > between Bluetooth and Wifi FSEQ.
>
> Please give an example output, and document the system, you tested this on.

This patch was tested with Typhoon Peak2 controller.

>
> > Signed-off-by: Kiran K <[email protected]>
> > ---
> > drivers/bluetooth/btintel.c | 106
> ++++++++++++++++++++++++++++++++++++
> > 1 file changed, 106 insertions(+)
> >
> > diff --git a/drivers/bluetooth/btintel.c b/drivers/bluetooth/btintel.c
> > index e5b043d96207..0d067ee39408 100644
> > --- a/drivers/bluetooth/btintel.c
> > +++ b/drivers/bluetooth/btintel.c
> > @@ -2670,6 +2670,111 @@ static void btintel_set_msft_opcode(struct
> hci_dev *hdev, u8 hw_variant)
> > }
> > }
> >
> > +static void btintel_print_fseq_info(struct hci_dev *hdev) {
> > + struct sk_buff *skb;
> > + u8 *p;
> > + const char *str;
> > +
> > + skb = __hci_cmd_sync(hdev, 0xfcb3, 0, NULL, HCI_CMD_TIMEOUT);
> > + if (IS_ERR(skb)) {
> > + bt_dev_dbg(hdev, "Reading fseq status command failed
> (%ld)",
> > + PTR_ERR(skb));
> > + return;
> > + }
> > +
> > + if (skb->len < (sizeof(u32) * 16 + 2)) {
> > + bt_dev_dbg(hdev, "Malformed packet");
>
> Please print out the length values.

Sure.

>
> > + kfree_skb(skb);
> > + return;
> > + }
> > +
> > + if (skb->data[0]) {
> > + bt_dev_dbg(hdev, "Failed to get fseq status (0x%2.2x)",
> > + skb->data[0]);
> > + kfree_skb(skb);
> > + return;
> > + }
> > +
> > + p = skb->data;
> > + /* skip status */
> > + p = p + 1;
> > +
> > + switch (*p) {
> > + case 0:
> > + str = "Success";
> > + break;
> > + case 1:
> > + str = "Fatal error";
> > + break;
> > + case 2:
> > + str = "Sem acq error";
>
> Maybe elaborate here?

FSEQ code execution is mutually exclusive between Wifi and Bluetooth. If Bluetooth not able to acquire semaphore, then error code 2 will be reported.
>
> > + break;
> > + default:
> > + str = "Unknown error";
> > + break;
> > + }
> > +
> > + bt_dev_info(hdev, "Fseq status: %s (0x%2.2x)", str, *p);
> > + if (*p)
> > + return;
>
> Should non-success levels have a different log level?

I will add bt_dev_err for non-success case.

>
> > + p = p + 1;
> > + bt_dev_dbg(hdev, "Reason: 0x%8.8x", get_unaligned_le32(p));

Thanks,
Kiran

2024-02-21 13:14:01

by K, Kiran

[permalink] [raw]
Subject: RE: [PATCH v1] Bluetooth: btintel: Print Firmware Sequencer information

Hi Luiz,

> -----Original Message-----
> From: Luiz Augusto von Dentz <[email protected]>
> Sent: Tuesday, February 13, 2024 9:40 PM
> To: K, Kiran <[email protected]>
> Cc: [email protected]; Srivatsa, Ravishankar
> <[email protected]>; Tumkur Narayan, Chethan
> <[email protected]>
> Subject: Re: [PATCH v1] Bluetooth: btintel: Print Firmware Sequencer
> information
>
> Hi Kiran,
>
> On Tue, Feb 13, 2024 at 10:51 AM Kiran K <[email protected]> wrote:
> >
> > Firmware sequencer(FSEQ) is a common code shared across Bluetooth and
> > Wifi. Printing FSEQ will help to debug if there is any mismatch
> > between Bluetooth and Wifi FSEQ.
> >
> > Signed-off-by: Kiran K <[email protected]>
> > ---
> > drivers/bluetooth/btintel.c | 106
> > ++++++++++++++++++++++++++++++++++++
> > 1 file changed, 106 insertions(+)
> >
> > diff --git a/drivers/bluetooth/btintel.c b/drivers/bluetooth/btintel.c
> > index e5b043d96207..0d067ee39408 100644
> > --- a/drivers/bluetooth/btintel.c
> > +++ b/drivers/bluetooth/btintel.c
> > @@ -2670,6 +2670,111 @@ static void btintel_set_msft_opcode(struct
> hci_dev *hdev, u8 hw_variant)
> > }
> > }
> >
> > +static void btintel_print_fseq_info(struct hci_dev *hdev) {
> > + struct sk_buff *skb;
> > + u8 *p;
> > + const char *str;
> > +
> > + skb = __hci_cmd_sync(hdev, 0xfcb3, 0, NULL, HCI_CMD_TIMEOUT);
> > + if (IS_ERR(skb)) {
> > + bt_dev_dbg(hdev, "Reading fseq status command failed (%ld)",
> > + PTR_ERR(skb));
> > + return;
> > + }
> > +
> > + if (skb->len < (sizeof(u32) * 16 + 2)) {
> > + bt_dev_dbg(hdev, "Malformed packet");
> > + kfree_skb(skb);
> > + return;
> > + }
> > +
> > + if (skb->data[0]) {
> > + bt_dev_dbg(hdev, "Failed to get fseq status (0x%2.2x)",
> > + skb->data[0]);
> > + kfree_skb(skb);
> > + return;
> > + }
> > +
> > + p = skb->data;
> > + /* skip status */
> > + p = p + 1;
>
> How about we use skb_pull_data instead of accessing these fields with a
> pointer cursor?

I will fix it in v2 version of patch.

>
> > + switch (*p) {

Thanks,
Kiran