Hi Paul,
Thanks for your comments.
> -----Original Message-----
> From: Paul Menzel <pmenzel@xxxxxxxxxxxxx>
> Sent: Tuesday, February 13, 2024 9:35 PM
> To: K, Kiran <kiran.k@xxxxxxxxx>
> Cc: Srivatsa, Ravishankar <ravishankar.srivatsa@xxxxxxxxx>; Tumkur Narayan,
> Chethan <chethan.tumkur.narayan@xxxxxxxxx>; linux-
> bluetooth@xxxxxxxxxxxxxxx
> 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 <kiran.k@xxxxxxxxx>
> > ---
> > 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
