From: Sathish Narasimman <nsathish41@gmail.com>
To: Bluez mailing list <linux-bluetooth@vger.kernel.org>
Cc: Chethan T N <chethan.tumkur.narayan@intel.com>,
ravishankar.srivatsa@intel.com, kiran.k@intel.com,
Sathish Narasimman <sathish.narasimman@intel.com>
Subject: Re: [PATCH v2] Bluetooth: btintel parse TLV structure
Date: Tue, 3 Nov 2020 07:33:51 +0530 [thread overview]
Message-ID: <CAOVXEJLEgEfC-m_u4ZHYhvWy2=_wQEUExm=6vFovNR6Y=VjGEw@mail.gmail.com> (raw)
In-Reply-To: <20201029062329.29246-1-sathish.narasimman@intel.com>
Hi
On Thu, Oct 29, 2020 at 11:47 AM 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.
>
> Signed-off-by: Sathish Narasimman <sathish.narasimman@intel.com>
> ---
> 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
next prev parent reply other threads:[~2020-11-03 2:04 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-10-29 6:23 [PATCH v2] Bluetooth: btintel parse TLV structure Sathish Narasimman
2020-11-03 2:03 ` Sathish Narasimman [this message]
2020-11-05 7:45 ` [v2] " bluez.test.bot
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='CAOVXEJLEgEfC-m_u4ZHYhvWy2=_wQEUExm=6vFovNR6Y=VjGEw@mail.gmail.com' \
--to=nsathish41@gmail.com \
--cc=chethan.tumkur.narayan@intel.com \
--cc=kiran.k@intel.com \
--cc=linux-bluetooth@vger.kernel.org \
--cc=ravishankar.srivatsa@intel.com \
--cc=sathish.narasimman@intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.