2020-10-22 09:02:30

by Sathish Narasimman

[permalink] [raw]
Subject: [PATCH 0/3] Bluetooth:btintel parse TLV structure

Latest intel firmware supports TLV structure in operational mode for intel
read version. so made changes accordingly to support both bootloader
and operational mode . These changes are only to specific intel bluetooth
controller for example ThP, CcP.

Sathish Narasimman (3):
Bluetooth: btintel: seperated TLV parsing as new function
Bluetooth: btintel: Introducing new btintel read version
Bluetooth: btusb: Use the new btintel read version

drivers/bluetooth/btintel.c | 105 +++++++++++++++++++++++++++---------
drivers/bluetooth/btintel.h | 16 ++++++
drivers/bluetooth/btusb.c | 41 ++++++++++----
3 files changed, 129 insertions(+), 33 deletions(-)

--
2.17.1


2020-10-22 09:04:45

by Sathish Narasimman

[permalink] [raw]
Subject: [PATCH 2/3] Bluetooth: btintel: Introducing new btintel read version

The new btintel read version supports the latest intel read version
command and also supports the TLV structure parsing. It still
handles the legacy read version

Signed-off-by: Sathish Narasimman <[email protected]>
---
drivers/bluetooth/btintel.c | 51 +++++++++++++++++++++++++++++++++++++
drivers/bluetooth/btintel.h | 15 +++++++++++
2 files changed, 66 insertions(+)

diff --git a/drivers/bluetooth/btintel.c b/drivers/bluetooth/btintel.c
index cc8e6c4e3205..ddd3c4bbdd6f 100644
--- a/drivers/bluetooth/btintel.c
+++ b/drivers/bluetooth/btintel.c
@@ -476,6 +476,57 @@ static void btintel_parse_tlv(struct sk_buff *skb,
}
}

+int btintel_read_version_new(struct hci_dev *hdev, struct btintel_version *ver)
+{
+ struct sk_buff *skb;
+ struct intel_version *version = &ver->ver;
+ const u8 param[1] = { 0xFF };
+
+ skb = __hci_cmd_sync(hdev, 0xfc05, 1, param, HCI_CMD_TIMEOUT);
+ if (IS_ERR(skb)) {
+ bt_dev_err(hdev, "Reading Intel version info failed (%ld)",
+ PTR_ERR(skb));
+ return PTR_ERR(skb);
+ }
+
+ if (skb->data[0]) {
+ bt_dev_err(hdev, "Intel Read Version command failed (%02x)",
+ skb->data[0]);
+ kfree_skb(skb);
+ return -EIO;
+ }
+
+ /* The new Intel read version is backward compatible for Thp and CcP
+ * type cards. when the controller is in bootloader mode the controller
+ * response remains same as old intel_read version. For ThP/CcP cards
+ * TLV stucture supports only during the Operation Mode. The best way
+ * to differentiate the read_version response is to check the length
+ * parameter and first byte of the payload, which is a fixed value.
+ * After the status parameter if the payload starts with 0x37(This is
+ * a fixed value) and length of the payload is 10 then it is identified
+ * as legacy struct intel_version. In the latest firmweare the support
+ * of TLV structure is added during Operational Firmware.
+ */
+ if (skb->len == sizeof(*version) && skb->data[1] == 0x37) {
+ memcpy(version, skb->data, sizeof(*version));
+ ver->tlv_format = false;
+ goto finish;
+ }
+
+ /* Consume Command Complete Status field */
+ skb_pull(skb, 1);
+
+ ver->tlv_format = true;
+
+ bt_dev_info(hdev, "Parsing TLV Supported intel read version");
+ btintel_parse_tlv(skb, &ver->ver_tlv);
+
+finish:
+ kfree_skb(skb);
+ return 0;
+}
+EXPORT_SYMBOL_GPL(btintel_read_version_new);
+
int btintel_read_version_tlv(struct hci_dev *hdev, struct intel_version_tlv *version)
{
struct sk_buff *skb;
diff --git a/drivers/bluetooth/btintel.h b/drivers/bluetooth/btintel.h
index 09346ae308eb..08406ef935a3 100644
--- a/drivers/bluetooth/btintel.h
+++ b/drivers/bluetooth/btintel.h
@@ -132,6 +132,14 @@ struct intel_debug_features {
__u8 page1[16];
} __packed;

+struct btintel_version {
+ bool tlv_format;
+ union {
+ struct intel_version ver; /*Legacy Intel read version*/
+ struct intel_version_tlv ver_tlv;
+ };
+};
+
#if IS_ENABLED(CONFIG_BT_INTEL)

int btintel_check_bdaddr(struct hci_dev *hdev);
@@ -151,6 +159,7 @@ int btintel_set_event_mask(struct hci_dev *hdev, bool debug);
int btintel_set_event_mask_mfg(struct hci_dev *hdev, bool debug);
int btintel_read_version(struct hci_dev *hdev, struct intel_version *ver);
int btintel_read_version_tlv(struct hci_dev *hdev, struct intel_version_tlv *ver);
+int btintel_read_version_new(struct hci_dev *hdev, struct btintel_version *ver);

struct regmap *btintel_regmap_init(struct hci_dev *hdev, u16 opcode_read,
u16 opcode_write);
@@ -248,6 +257,12 @@ static inline int btintel_read_version_tlv(struct hci_dev *hdev,
return -EOPNOTSUPP;
}

+static inline int btintel_read_version_new(struct hci_dev *hdev,
+ struct btintel_version *ver)
+{
+ return -EOPNOTSUPP;
+}
+
static inline struct regmap *btintel_regmap_init(struct hci_dev *hdev,
u16 opcode_read,
u16 opcode_write)
--
2.17.1

2020-10-28 22:02:46

by Sathish Narasimman

[permalink] [raw]
Subject: Re: [PATCH 0/3] Bluetooth:btintel parse TLV structure

hi
gentle reminder

On Thu, Oct 22, 2020 at 1:49 PM Sathish Narasimman <[email protected]> wrote:
>
> Latest intel firmware supports TLV structure in operational mode for intel
> read version. so made changes accordingly to support both bootloader
> and operational mode . These changes are only to specific intel bluetooth
> controller for example ThP, CcP.
>
> Sathish Narasimman (3):
> Bluetooth: btintel: seperated TLV parsing as new function
> Bluetooth: btintel: Introducing new btintel read version
> Bluetooth: btusb: Use the new btintel read version
>
> drivers/bluetooth/btintel.c | 105 +++++++++++++++++++++++++++---------
> drivers/bluetooth/btintel.h | 16 ++++++
> drivers/bluetooth/btusb.c | 41 ++++++++++----
> 3 files changed, 129 insertions(+), 33 deletions(-)
>
> --
> 2.17.1
>

2020-10-29 09:05:56

by Sathish Narasimman

[permalink] [raw]
Subject: RE: [PATCH 2/3] Bluetooth: btintel: Introducing new btintel read version

Hi Tedd

> -----Original Message-----
> From: Tedd Ho-Jeong An <[email protected]>
> Sent: Thursday, October 29, 2020 10:58 AM
> To: Sathish Narasimman <[email protected]>
> Cc: [email protected]; Tumkur Narayan, Chethan
> <[email protected]>; Srivatsa, Ravishankar
> <[email protected]>; K, Kiran <[email protected]>;
> Narasimman, Sathish <[email protected]>
> Subject: Re: [PATCH 2/3] Bluetooth: btintel: Introducing new btintel read
> version
>
> Hi Sathish,
>
> On 2020-10-22 at 13:54:34 +0530, Sathish Narasimman wrote:
> > The new btintel read version supports the latest intel read version
> > command and also supports the TLV structure parsing. It still handles
> > the legacy read version
> >
> > Signed-off-by: Sathish Narasimman <[email protected]>
> > ---
> > drivers/bluetooth/btintel.c | 51
> > +++++++++++++++++++++++++++++++++++++
> > drivers/bluetooth/btintel.h | 15 +++++++++++
> > 2 files changed, 66 insertions(+)
> >
> > diff --git a/drivers/bluetooth/btintel.c b/drivers/bluetooth/btintel.c
> > index cc8e6c4e3205..ddd3c4bbdd6f 100644
> > --- a/drivers/bluetooth/btintel.c
> > +++ b/drivers/bluetooth/btintel.c
> > @@ -476,6 +476,57 @@ static void btintel_parse_tlv(struct sk_buff *skb,
> > }
> > }
> >
> > +int btintel_read_version_new(struct hci_dev *hdev, struct
> > +btintel_version *ver) {
> > + struct sk_buff *skb;
> > + struct intel_version *version = &ver->ver;
> > + const u8 param[1] = { 0xFF };
> > +
> > + skb = __hci_cmd_sync(hdev, 0xfc05, 1, param, HCI_CMD_TIMEOUT);
> > + if (IS_ERR(skb)) {
> > + bt_dev_err(hdev, "Reading Intel version info failed (%ld)",
> > + PTR_ERR(skb));
> > + return PTR_ERR(skb);
> > + }
> > +
> > + if (skb->data[0]) {
> > + bt_dev_err(hdev, "Intel Read Version command failed
> (%02x)",
> > + skb->data[0]);
> > + kfree_skb(skb);
> > + return -EIO;
> > + }
> > +
> > + /* The new Intel read version is backward compatible for Thp and
> CcP
> > + * type cards. when the controller is in bootloader mode the
> controller
> > + * response remains same as old intel_read version. For ThP/CcP
> cards
> > + * TLV stucture supports only during the Operation Mode. The best
> > +way
>
> misspelled structure
>
> > + * to differentiate the read_version response is to check the length
> > + * parameter and first byte of the payload, which is a fixed value.
> > + * After the status parameter if the payload starts with 0x37(This is
> > + * a fixed value) and length of the payload is 10 then it is identified
> > + * as legacy struct intel_version. In the latest firmweare the support
> > + * of TLV structure is added during Operational Firmware.
> > + */
> > + if (skb->len == sizeof(*version) && skb->data[1] == 0x37) {
> > + memcpy(version, skb->data, sizeof(*version));
> > + ver->tlv_format = false;
> > + goto finish;
> > + }
> > +
> > + /* Consume Command Complete Status field */
> > + skb_pull(skb, 1);
> > +
> > + ver->tlv_format = true;
> > +
> > + bt_dev_info(hdev, "Parsing TLV Supported intel read version");
> > + btintel_parse_tlv(skb, &ver->ver_tlv);
> > +
> > +finish:
> > + kfree_skb(skb);
> > + return 0;
> > +}
> > +EXPORT_SYMBOL_GPL(btintel_read_version_new);
> > +
> > int btintel_read_version_tlv(struct hci_dev *hdev, struct
> > intel_version_tlv *version) {
> > struct sk_buff *skb;
> > diff --git a/drivers/bluetooth/btintel.h b/drivers/bluetooth/btintel.h
> > index 09346ae308eb..08406ef935a3 100644
> > --- a/drivers/bluetooth/btintel.h
> > +++ b/drivers/bluetooth/btintel.h
> > @@ -132,6 +132,14 @@ struct intel_debug_features {
> > __u8 page1[16];
> > } __packed;
> >
> > +struct btintel_version {
> > + bool tlv_format;
> > + union {
> > + struct intel_version ver; /*Legacy Intel read version*/
> > + struct intel_version_tlv ver_tlv;
> > + };
> > +};
>
> Add __packed;
>
> > +
> > #if IS_ENABLED(CONFIG_BT_INTEL)
> >
> > int btintel_check_bdaddr(struct hci_dev *hdev); @@ -151,6 +159,7 @@
> > int btintel_set_event_mask(struct hci_dev *hdev, bool debug); int
> > btintel_set_event_mask_mfg(struct hci_dev *hdev, bool debug); int
> > btintel_read_version(struct hci_dev *hdev, struct intel_version *ver);
> > int btintel_read_version_tlv(struct hci_dev *hdev, struct
> > intel_version_tlv *ver);
> > +int btintel_read_version_new(struct hci_dev *hdev, struct
> > +btintel_version *ver);
> >
> > struct regmap *btintel_regmap_init(struct hci_dev *hdev, u16
> opcode_read,
> > u16 opcode_write);
> > @@ -248,6 +257,12 @@ static inline int btintel_read_version_tlv(struct
> hci_dev *hdev,
> > return -EOPNOTSUPP;
> > }
> >
> > +static inline int btintel_read_version_new(struct hci_dev *hdev,
> > + struct btintel_version *ver)
> > +{
> > + return -EOPNOTSUPP;
> > +}
> > +
> > static inline struct regmap *btintel_regmap_init(struct hci_dev *hdev,
> > u16 opcode_read,
> > u16 opcode_write)
> > --
> > 2.17.1
> >
>
> I think you can combine your 3 patches into one patch.

Will correct the changes as mentioned and upload next version combining all the patches.

>
> Regards,
>
> Tedd

Regards
Sathish N

2020-10-29 09:07:08

by Tedd Ho-Jeong An

[permalink] [raw]
Subject: Re: [PATCH 2/3] Bluetooth: btintel: Introducing new btintel read version

Hi Sathish,

On 2020-10-22 at 13:54:34 +0530, Sathish Narasimman wrote:
> The new btintel read version supports the latest intel read version
> command and also supports the TLV structure parsing. It still
> handles the legacy read version
>
> Signed-off-by: Sathish Narasimman <[email protected]>
> ---
> drivers/bluetooth/btintel.c | 51 +++++++++++++++++++++++++++++++++++++
> drivers/bluetooth/btintel.h | 15 +++++++++++
> 2 files changed, 66 insertions(+)
>
> diff --git a/drivers/bluetooth/btintel.c b/drivers/bluetooth/btintel.c
> index cc8e6c4e3205..ddd3c4bbdd6f 100644
> --- a/drivers/bluetooth/btintel.c
> +++ b/drivers/bluetooth/btintel.c
> @@ -476,6 +476,57 @@ static void btintel_parse_tlv(struct sk_buff *skb,
> }
> }
>
> +int btintel_read_version_new(struct hci_dev *hdev, struct btintel_version *ver)
> +{
> + struct sk_buff *skb;
> + struct intel_version *version = &ver->ver;
> + const u8 param[1] = { 0xFF };
> +
> + skb = __hci_cmd_sync(hdev, 0xfc05, 1, param, HCI_CMD_TIMEOUT);
> + if (IS_ERR(skb)) {
> + bt_dev_err(hdev, "Reading Intel version info failed (%ld)",
> + PTR_ERR(skb));
> + return PTR_ERR(skb);
> + }
> +
> + if (skb->data[0]) {
> + bt_dev_err(hdev, "Intel Read Version command failed (%02x)",
> + skb->data[0]);
> + kfree_skb(skb);
> + return -EIO;
> + }
> +
> + /* The new Intel read version is backward compatible for Thp and CcP
> + * type cards. when the controller is in bootloader mode the controller
> + * response remains same as old intel_read version. For ThP/CcP cards
> + * TLV stucture supports only during the Operation Mode. The best way

misspelled structure

> + * to differentiate the read_version response is to check the length
> + * parameter and first byte of the payload, which is a fixed value.
> + * After the status parameter if the payload starts with 0x37(This is
> + * a fixed value) and length of the payload is 10 then it is identified
> + * as legacy struct intel_version. In the latest firmweare the support
> + * of TLV structure is added during Operational Firmware.
> + */
> + if (skb->len == sizeof(*version) && skb->data[1] == 0x37) {
> + memcpy(version, skb->data, sizeof(*version));
> + ver->tlv_format = false;
> + goto finish;
> + }
> +
> + /* Consume Command Complete Status field */
> + skb_pull(skb, 1);
> +
> + ver->tlv_format = true;
> +
> + bt_dev_info(hdev, "Parsing TLV Supported intel read version");
> + btintel_parse_tlv(skb, &ver->ver_tlv);
> +
> +finish:
> + kfree_skb(skb);
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(btintel_read_version_new);
> +
> int btintel_read_version_tlv(struct hci_dev *hdev, struct intel_version_tlv *version)
> {
> struct sk_buff *skb;
> diff --git a/drivers/bluetooth/btintel.h b/drivers/bluetooth/btintel.h
> index 09346ae308eb..08406ef935a3 100644
> --- a/drivers/bluetooth/btintel.h
> +++ b/drivers/bluetooth/btintel.h
> @@ -132,6 +132,14 @@ struct intel_debug_features {
> __u8 page1[16];
> } __packed;
>
> +struct btintel_version {
> + bool tlv_format;
> + union {
> + struct intel_version ver; /*Legacy Intel read version*/
> + struct intel_version_tlv ver_tlv;
> + };
> +};

Add __packed;

> +
> #if IS_ENABLED(CONFIG_BT_INTEL)
>
> int btintel_check_bdaddr(struct hci_dev *hdev);
> @@ -151,6 +159,7 @@ int btintel_set_event_mask(struct hci_dev *hdev, bool debug);
> int btintel_set_event_mask_mfg(struct hci_dev *hdev, bool debug);
> int btintel_read_version(struct hci_dev *hdev, struct intel_version *ver);
> int btintel_read_version_tlv(struct hci_dev *hdev, struct intel_version_tlv *ver);
> +int btintel_read_version_new(struct hci_dev *hdev, struct btintel_version *ver);
>
> struct regmap *btintel_regmap_init(struct hci_dev *hdev, u16 opcode_read,
> u16 opcode_write);
> @@ -248,6 +257,12 @@ static inline int btintel_read_version_tlv(struct hci_dev *hdev,
> return -EOPNOTSUPP;
> }
>
> +static inline int btintel_read_version_new(struct hci_dev *hdev,
> + struct btintel_version *ver)
> +{
> + return -EOPNOTSUPP;
> +}
> +
> static inline struct regmap *btintel_regmap_init(struct hci_dev *hdev,
> u16 opcode_read,
> u16 opcode_write)
> --
> 2.17.1
>

I think you can combine your 3 patches into one patch.

Regards,

Tedd