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

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

[Sathish Narasimman]

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

[PATCH 1/3] Bluetooth: btintel: seperated TLV parsing as new function

[Sathish Narasimman]

Create a seperate TLV parse function to read btintel TLV
structure.

Signed-off-by: Sathish Narasimman <sathish.narasimman@intel.com>
---
 drivers/bluetooth/btintel.c | 54 ++++++++++++++++++++-----------------
 1 file changed, 30 insertions(+), 24 deletions(-)

diff --git a/drivers/bluetooth/btintel.c b/drivers/bluetooth/btintel.c
index 88ce5f0ffc4b..cc8e6c4e3205 100644
--- a/drivers/bluetooth/btintel.c
+++ b/drivers/bluetooth/btintel.c
@@ -401,31 +401,9 @@ void btintel_version_info_tlv(struct hci_dev *hdev, struct intel_version_tlv *ve
 }
 EXPORT_SYMBOL_GPL(btintel_version_info_tlv);
 
-int btintel_read_version_tlv(struct hci_dev *hdev, struct intel_version_tlv *version)
+static void btintel_parse_tlv(struct sk_buff *skb,
+			      struct intel_version_tlv *version)
 {
-	struct sk_buff *skb;
-	const u8 param[1] = { 0xFF };
-
-	if (!version)
-		return -EINVAL;
-
-	skb = __hci_cmd_sync(hdev, 0xfc05, 1, param, HCI_CMD_TIMEOUT);
-	if (IS_ERR(skb)) {
-		bt_dev_err(hdev, "Reading Intel version information 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;
-	}
-
-	/* Consume Command Complete Status field */
-	skb_pull(skb, 1);
-
 	/* Event parameters contatin multiple TLVs. Read each of them
 	 * and only keep the required data. Also, it use existing legacy
 	 * version field like hw_platform, hw_variant, and fw_variant
@@ -496,6 +474,34 @@ int btintel_read_version_tlv(struct hci_dev *hdev, struct intel_version_tlv *ver
 		/* consume the current tlv and move to next*/
 		skb_pull(skb, tlv->len + sizeof(*tlv));
 	}
+}
+
+int btintel_read_version_tlv(struct hci_dev *hdev, struct intel_version_tlv *version)
+{
+	struct sk_buff *skb;
+	const u8 param[1] = { 0xFF };
+
+	if (!version)
+		return -EINVAL;
+
+	skb = __hci_cmd_sync(hdev, 0xfc05, 1, param, HCI_CMD_TIMEOUT);
+	if (IS_ERR(skb)) {
+		bt_dev_err(hdev, "Reading Intel version information 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;
+	}
+
+	/* Consume Command Complete Status field */
+	skb_pull(skb, 1);
+
+	btintel_parse_tlv(skb, version);
 
 	kfree_skb(skb);
 	return 0;
-- 
2.17.1

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

[Sathish Narasimman]

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 <sathish.narasimman@intel.com>
---
 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

[PATCH 3/3] Bluetooth: btusb: Use the new btintel read version

[Sathish Narasimman]

Replace the legacy btintel_read_version to btintel_read_verison_new.
The new intel firmware has the support to parse TLV structure during
operational mode. The patch takes care of both legacy and tlv structure.

Signed-off-by: Sathish Narasimman <sathish.narasimman@intel.com>
---
 drivers/bluetooth/btintel.h |  1 +
 drivers/bluetooth/btusb.c   | 41 +++++++++++++++++++++++++++++--------
 2 files changed, 33 insertions(+), 9 deletions(-)

diff --git a/drivers/bluetooth/btintel.h b/drivers/bluetooth/btintel.h
index 08406ef935a3..e3025b90eda5 100644
--- a/drivers/bluetooth/btintel.h
+++ b/drivers/bluetooth/btintel.h
@@ -140,6 +140,7 @@ struct btintel_version {
 	};
 };
 
+#define INTEL_HW_VARIANT(cnvx_bt)	((u8)(((cnvx_bt) & 0x003f0000) >> 16))
 #if IS_ENABLED(CONFIG_BT_INTEL)
 
 int btintel_check_bdaddr(struct hci_dev *hdev);
diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
index 1005b6e8ff74..c63bc8a0c84f 100644
--- a/drivers/bluetooth/btusb.c
+++ b/drivers/bluetooth/btusb.c
@@ -2554,7 +2554,8 @@ static int btusb_intel_download_firmware(struct hci_dev *hdev,
 static int btusb_setup_intel_new(struct hci_dev *hdev)
 {
 	struct btusb_data *data = hci_get_drvdata(hdev);
-	struct intel_version ver;
+	struct btintel_version bt_ver;
+	u8 hw_variant;
 	struct intel_boot_params params;
 	u32 boot_param;
 	char ddcname[64];
@@ -2577,19 +2578,33 @@ static int btusb_setup_intel_new(struct hci_dev *hdev)
 	 * is in bootloader mode or if it already has operational firmware
 	 * loaded.
 	 */
-	err = btintel_read_version(hdev, &ver);
+	err = btintel_read_version_new(hdev, &bt_ver);
 	if (err) {
 		bt_dev_err(hdev, "Intel Read version failed (%d)", err);
 		btintel_reset_to_bootloader(hdev);
 		return err;
 	}
 
-	err = btusb_intel_download_firmware(hdev, &ver, &params, &boot_param);
+	/* If TLV format is supported then it is in Operational Firmware. TLV
+	 * structure is supported only in operational mode in latest Firmware.
+	 */
+	if (bt_ver.tlv_format && bt_ver.ver_tlv.img_type == 0x03) {
+		btintel_version_info_tlv(hdev, &bt_ver.ver_tlv);
+		clear_bit(BTUSB_BOOTLOADER, &data->flags);
+		goto finish;
+	}
+
+	err = btusb_intel_download_firmware(hdev, &bt_ver.ver,  &params,
+					    &boot_param);
 	if (err)
 		return err;
 
-	/* controller is already having an operational firmware */
-	if (ver.fw_variant == 0x23)
+	/* In case if old firmware is used, it should be backward compatible
+	 * to check for operational firmware. only on latest firmware the
+	 * support for TLV structure is added. so check for tlv is not
+	 * required here.
+	 */
+	if (bt_ver.ver.fw_variant == 0x23)
 		goto finish;
 
 	rettime = ktime_get();
@@ -2641,7 +2656,7 @@ static int btusb_setup_intel_new(struct hci_dev *hdev)
 
 	clear_bit(BTUSB_BOOTLOADER, &data->flags);
 
-	err = btusb_setup_intel_new_get_fw_name(&ver, &params, ddcname,
+	err = btusb_setup_intel_new_get_fw_name(&bt_ver.ver, &params, ddcname,
 						sizeof(ddcname), "ddc");
 
 	if (!err) {
@@ -2665,17 +2680,25 @@ static int btusb_setup_intel_new(struct hci_dev *hdev)
 	btintel_set_debug_features(hdev, &features);
 
 	/* Read the Intel version information after loading the FW  */
-	err = btintel_read_version(hdev, &ver);
+	err = btintel_read_version_new(hdev, &bt_ver);
 	if (err)
 		return err;
 
-	btintel_version_info(hdev, &ver);
+	if (bt_ver.tlv_format)
+		btintel_version_info_tlv(hdev, &bt_ver.ver_tlv);
+	else
+		btintel_version_info(hdev, &bt_ver.ver);
 
 finish:
 	/* All Intel controllers that support the Microsoft vendor
 	 * extension are using 0xFC1E for VsMsftOpCode.
 	 */
-	switch (ver.hw_variant) {
+	if (!bt_ver.tlv_format)
+		hw_variant = bt_ver.ver.hw_variant;
+	else
+		hw_variant = INTEL_HW_VARIANT(bt_ver.ver_tlv.cnvi_bt);
+
+	switch (hw_variant) {
 	case 0x12:	/* ThP */
 		hci_set_msft_opcode(hdev, 0xFC1E);
 		break;
-- 
2.17.1

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

[Sathish Narasimman]

hi
gentle reminder

On Thu, Oct 22, 2020 at 1:49 PM Sathish Narasimman <nsathish41@gmail.com> 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
>

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

[Tedd Ho-Jeong An]

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 <sathish.narasimman@intel.com>
> ---
>  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

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

[Narasimman, Sathish]

Hi Tedd

> -----Original Message-----
> From: Tedd Ho-Jeong An <tedd.an@linux.intel.com>
> Sent: Thursday, October 29, 2020 10:58 AM
> To: Sathish Narasimman <nsathish41@gmail.com>
> Cc: linux-bluetooth@vger.kernel.org; Tumkur Narayan, Chethan
> <chethan.tumkur.narayan@intel.com>; Srivatsa, Ravishankar
> <ravishankar.srivatsa@intel.com>; K, Kiran <kiran.k@intel.com>;
> Narasimman, Sathish <sathish.narasimman@intel.com>
> 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 <sathish.narasimman@intel.com>
> > ---
> >  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