Hi Kiran,
> New generation Intel controllers returns controller information
> in TLV format. Adding capability to parse and log it for debug purpose
>
> Signed-off-by: Kiran K <kiran.k@xxxxxxxxx>
> Signed-off-by: Amit K Bag <amit.k.bag@xxxxxxxxx>
> Signed-off-by: Raghuram Hegde <raghuram.hegde@xxxxxxxxx>
> Reviewed-by: Chethan T N <chethan.tumkur.narayan@xxxxxxxxx>
> Reviewed-by: Sathish Narasimman <Sathish.Narasimman@xxxxxxxxx>
> Reviewed-by: Srivatsa Ravishankar <ravishankar.srivatsa@xxxxxxxxx>
> ---
> drivers/bluetooth/btintel.c | 161 ++++++++++++++++++++++++++++++++----
> drivers/bluetooth/btusb.c | 4 +-
> 2 files changed, 148 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/bluetooth/btintel.c b/drivers/bluetooth/btintel.c
> index d0c6576212d7..f0b087d97a83 100644
> --- a/drivers/bluetooth/btintel.c
> +++ b/drivers/bluetooth/btintel.c
> @@ -209,27 +209,59 @@ void btintel_version_info(struct hci_dev *hdev, const struct btintel_version *ve
> {
> const char *variant;
> const struct intel_version *ver;
> + const struct intel_version_tlv *ver_tlv;
>
> - if (version->is_tlv_supported)
> - return;
> + if (!version->is_tlv_supported) {
> + ver = &version->intel_version;
> +
> + switch (ver->fw_variant) {
> + case 0x06:
> + variant = "Bootloader";
> + break;
The break; has to be indented with the variant =.
> + case 0x23:
> + variant = "Firmware";
> + break;
> + default:
> + goto done;
> + }
>
> - ver = &version->intel_version;
> + bt_dev_info(hdev, "%s revision %u.%u build %u week %u %u",
> + variant, ver->fw_revision >> 4,
> + ver->fw_revision & 0x0f, ver->fw_build_num,
> + ver->fw_build_ww, 2000 + ver->fw_build_yy);
> + goto done;
> + }
> +
> + ver_tlv = &version->intel_version_tlv;
>
> - switch (ver->fw_variant) {
> - case 0x06:
> + switch (ver_tlv->img_type) {
> + case 0x01:
> variant = "Bootloader";
> - break;
> - case 0x23:
> + bt_dev_info(hdev, "Device revision is %u", ver_tlv->dev_rev_id);
> + bt_dev_info(hdev, "Secure boot is %s",
> + ver_tlv->secure_boot ? "enabled" : "disabled");
> + bt_dev_info(hdev, "OTP lock is %s",
> + ver_tlv->otp_lock ? "enabled" : "disabled");
> + bt_dev_info(hdev, "API lock is %s",
> + ver_tlv->api_lock ? "enabled" : "disabled");
> + bt_dev_info(hdev, "Debug lock is %s",
> + ver_tlv->debug_lock ? "enabled" : "disabled");
> + bt_dev_info(hdev, "Minimum firmware build %u week %u %u",
> + ver_tlv->min_fw_build_nn, ver_tlv->min_fw_build_cw,
> + 2000 + ver_tlv->min_fw_build_yy);
> + break;
> + case 0x03:
> variant = "Firmware";
> - break;
> + break;
> default:
> - return;
> + goto done;
> }
>
> - bt_dev_info(hdev, "%s revision %u.%u build %u week %u %u",
> - variant, ver->fw_revision >> 4, ver->fw_revision & 0x0f,
> - ver->fw_build_num, ver->fw_build_ww,
> - 2000 + ver->fw_build_yy);
> + bt_dev_info(hdev, "%s timestamp %u.%u buildtype %u build %u", variant,
> + 2000 + (ver_tlv->timestamp >> 8), ver_tlv->timestamp & 0xff,
> + ver_tlv->build_type, ver_tlv->build_num);
> +done:
> + return;
> }
> EXPORT_SYMBOL_GPL(btintel_version_info);
>
> @@ -346,6 +378,8 @@ int btintel_read_version(struct hci_dev *hdev, struct btintel_version *version)
> {
> struct sk_buff *skb;
> u8 *data, param, status, check_tlv;
> + struct intel_version_tlv *ver_tlv;
> + struct intel_tlv *tlv;
>
> if (!version)
> return -EINVAL;
> @@ -373,9 +407,106 @@ int btintel_read_version(struct hci_dev *hdev, struct btintel_version *version)
> if (skb->len == sizeof(version->intel_version) && check_tlv == 0x37) {
> memcpy(&version->intel_version, skb->data, sizeof(version->intel_version));
> version->is_tlv_supported = false;
> - } else {
> - version->is_tlv_supported = true;
> + goto done;
> }
> +
> + bt_dev_info(hdev, "Supports tlv firmware download sequence");
> + version->is_tlv_supported = true;
> + ver_tlv = &version->intel_version_tlv;
> +
> + /* 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
> + * to keep the existing setup flow
> + */
> + while (skb->len) {
> + tlv = (struct intel_tlv *)skb->data;
> + switch (tlv->type) {
> + case INTEL_TLV_CNVI_TOP:
> + ver_tlv->cnvi_top = (tlv->val[3] << 24) |
> + (tlv->val[2] << 16) |
> + (tlv->val[1] << 8) |
> + (tlv->val[0]);
We have get_unaligned functions for that.
> + break;
> + case INTEL_TLV_CNVR_TOP:
> + ver_tlv->cnvr_top = (tlv->val[3] << 24) |
> + (tlv->val[2] << 16) |
> + (tlv->val[1] << 8) |
> + (tlv->val[0]);
> + break;
> + case INTEL_TLV_CNVI_BT:
> + ver_tlv->cnvi_bt = (tlv->val[3] << 24) |
> + (tlv->val[2] << 16) |
> + (tlv->val[1] << 8) |
> + (tlv->val[0]);
> + break;
> + case INTEL_TLV_CNVR_BT:
> + ver_tlv->cnvr_bt = (tlv->val[3] << 24) |
> + (tlv->val[2] << 16) |
> + (tlv->val[1] << 8) |
> + (tlv->val[0]);
> + break;
> + case INTEL_TLV_USB_VENDOR_ID:
> + ver_tlv->usb_vid = (tlv->val[1] << 8) | (tlv->val[0]);
> + break;
> + case INTEL_TLV_USB_PRODUCT_ID:
> + ver_tlv->usb_pid = (tlv->val[1] << 8) | (tlv->val[0]);
> + break;
> + case INTEL_TLV_IMAGE_TYPE:
> + ver_tlv->img_type = tlv->val[0];
> + break;
> + case INTEL_TLV_TIME_STAMP:
> + ver_tlv->timestamp = (tlv->val[1] << 8) | (tlv->val[0]);
> + break;
> + case INTEL_TLV_BUILD_TYPE:
> + ver_tlv->build_type = tlv->val[0];
> + break;
> + case INTEL_TLV_BUILD_NUM:
> + ver_tlv->build_num = (tlv->val[3] << 24) |
> + (tlv->val[2] << 16) |
> + (tlv->val[1] << 8) |
> + (tlv->val[0]);
> + break;
> + case INTEL_TLV_SECURE_BOOT:
> + ver_tlv->secure_boot = tlv->val[0];
> + break;
> + case INTEL_TLV_KEY_FROM_HDR:
> + ver_tlv->key_from_hdr = tlv->val[0];
> + break;
> + case INTEL_TLV_OTP_LOCK:
> + ver_tlv->otp_lock = tlv->val[0];
> + break;
> + case INTEL_TLV_API_LOCK:
> + ver_tlv->api_lock = tlv->val[0];
> + break;
> + case INTEL_TLV_DEBUG_LOCK:
> + ver_tlv->debug_lock = tlv->val[0];
> + break;
> + case INTEL_TLV_MIN_FW:
> + ver_tlv->min_fw_build_nn = tlv->val[0];
> + ver_tlv->min_fw_build_cw = tlv->val[1];
> + ver_tlv->min_fw_build_yy = tlv->val[2];
> + break;
> + case INTEL_TLV_LIMITED_CCE:
> + ver_tlv->limited_cce = tlv->val[0];
> + break;
> + case INTEL_TLV_SBE_TYPE:
> + ver_tlv->sbe_type = tlv->val[0];
> + break;
> + case INTEL_TLV_OTP_BDADDR:
> + memcpy(&ver_tlv->otp_bd_addr, tlv->val, tlv->len);
> + break;
> + default:
> + /* Ignore rest of information */
> + break;
> + }
> + /* consume the current tlv and move to next*/
> + skb_pull(skb, tlv->len + sizeof(*tlv));
> + }
> +done:
> kfree_skb(skb);
> return 0;
> }
> diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
> index 364da6d44ee3..f30b43e15a26 100644
> --- a/drivers/bluetooth/btusb.c
> +++ b/drivers/bluetooth/btusb.c
> @@ -2519,13 +2519,13 @@ static int btusb_setup_intel_new(struct hci_dev *hdev)
> return err;
> }
>
> + btintel_version_info(hdev, &version);
> +
> if (version.is_tlv_supported) {
> bt_dev_err(hdev, "Firmware download in tlv format is not supported");
> return -EOPNOTSUPP;
> }
>
> - btintel_version_info(hdev, &version);
> -
> err = btusb_intel_download_firmware(hdev, &version, ¶ms);
> if (err)
> return err;
Regards
Marcel
