Hi On Thu, Oct 29, 2020 at 11:47 AM Sathish Narasimman <nsathish41@xxxxxxxxx> 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. > > Signed-off-by: Sathish Narasimman <sathish.narasimman@xxxxxxxxx> > --- > drivers/bluetooth/btintel.c | 105 +++++++++++++++++++++++++++--------- > drivers/bluetooth/btintel.h | 16 ++++++ > drivers/bluetooth/btusb.c | 41 ++++++++++---- > 3 files changed, 129 insertions(+), 33 deletions(-) > > diff --git a/drivers/bluetooth/btintel.c b/drivers/bluetooth/btintel.c > index 88ce5f0ffc4b..67267afc83e1 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,85 @@ 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_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 structure supports only during the Operation Mode. The best way > + * to differentiate the read_version response is to check the length > + * parameter and the 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 firmware 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; > + 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; > diff --git a/drivers/bluetooth/btintel.h b/drivers/bluetooth/btintel.h > index 09346ae308eb..952da44b79de 100644 > --- a/drivers/bluetooth/btintel.h > +++ b/drivers/bluetooth/btintel.h > @@ -132,6 +132,15 @@ 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; > + }; > +} __packed; > + > +#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); > @@ -151,6 +160,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 +258,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) > 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, ¶ms, &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, ¶ms, > + &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, ¶ms, ddcname, > + err = btusb_setup_intel_new_get_fw_name(&bt_ver.ver, ¶ms, 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 > Gentle Reminder Regards Sathish N