From: Tedd Ho-Jeong An <[email protected]>
There are multiple setup and shutdown functions for Intel device and the
setup function to be used is depends on the USB PID/VID, which makes
difficult to maintain the code and increases the code size.
This patch adds combined setup and shutdown functions to provide a
single entry point for all Intel devices and choose the setup functions
based on the information read with HCI_Intel_Read_Version command.
Starting from TyP device, the command and response parameters for
HCI_Intel_Read_Version command are changed even though OCF remains
same. However, the legacy devices still can handle the command without
error even if it has a extra parameter, so to simplify the flow,
the new command format is used to read the version information for
both legacy and new (tlv based) format.
Also, it also adds a routine to setup the hdev callbacks in btintel.
Signed-off-by: Tedd Ho-Jeong An <[email protected]>
---
drivers/bluetooth/btintel.c | 230 ++++++++++++++++++++++++++++++++++++
drivers/bluetooth/btintel.h | 6 +
2 files changed, 236 insertions(+)
diff --git a/drivers/bluetooth/btintel.c b/drivers/bluetooth/btintel.c
index e44b6993cf91..3d98fc2a64b9 100644
--- a/drivers/bluetooth/btintel.c
+++ b/drivers/bluetooth/btintel.c
@@ -236,6 +236,8 @@ int btintel_version_info(struct hci_dev *hdev, struct intel_version *ver)
* compatibility options when newer hardware variants come along.
*/
switch (ver->hw_variant) {
+ case 0x07: /* WP - Legacy ROM */
+ case 0x08: /* StP - Legacy ROM */
case 0x0b: /* SfP */
case 0x0c: /* WsP */
case 0x11: /* JfP */
@@ -250,9 +252,15 @@ int btintel_version_info(struct hci_dev *hdev, struct intel_version *ver)
}
switch (ver->fw_variant) {
+ case 0x01:
+ variant = "Legacy ROM 2.5";
+ break;
case 0x06:
variant = "Bootloader";
break;
+ case 0x22:
+ variant = "Legacy ROM 2.x";
+ break;
case 0x23:
variant = "Firmware";
break;
@@ -483,6 +491,107 @@ int btintel_version_info_tlv(struct hci_dev *hdev, struct intel_version_tlv *ver
}
EXPORT_SYMBOL_GPL(btintel_version_info_tlv);
+static int btintel_parse_version_tlv(struct hci_dev *hdev,
+ struct intel_version_tlv *version,
+ struct sk_buff *skb)
+{
+ /* 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) {
+ struct intel_tlv *tlv;
+
+ /* Make sure skb has a minimum length of the header */
+ if (skb->len < 2)
+ return -EINVAL;
+
+ tlv = (struct intel_tlv *)skb->data;
+
+ /* Make sure skb has a enough data */
+ if (skb->len < tlv->len + sizeof(*tlv))
+ return -EINVAL;
+
+ switch (tlv->type) {
+ case INTEL_TLV_CNVI_TOP:
+ version->cnvi_top = get_unaligned_le32(tlv->val);
+ break;
+ case INTEL_TLV_CNVR_TOP:
+ version->cnvr_top = get_unaligned_le32(tlv->val);
+ break;
+ case INTEL_TLV_CNVI_BT:
+ version->cnvi_bt = get_unaligned_le32(tlv->val);
+ break;
+ case INTEL_TLV_CNVR_BT:
+ version->cnvr_bt = get_unaligned_le32(tlv->val);
+ break;
+ case INTEL_TLV_DEV_REV_ID:
+ version->dev_rev_id = get_unaligned_le16(tlv->val);
+ break;
+ case INTEL_TLV_IMAGE_TYPE:
+ version->img_type = tlv->val[0];
+ break;
+ case INTEL_TLV_TIME_STAMP:
+ /* If image type is Operational firmware (0x03), then
+ * running FW Calendar Week and Year information can
+ * be extracted from Timestamp information
+ */
+ version->min_fw_build_cw = tlv->val[0];
+ version->min_fw_build_yy = tlv->val[1];
+ version->timestamp = get_unaligned_le16(tlv->val);
+ break;
+ case INTEL_TLV_BUILD_TYPE:
+ version->build_type = tlv->val[0];
+ break;
+ case INTEL_TLV_BUILD_NUM:
+ /* If image type is Operational firmware (0x03), then
+ * running FW build number can be extracted from the
+ * Build information
+ */
+ version->min_fw_build_nn = tlv->val[0];
+ version->build_num = get_unaligned_le32(tlv->val);
+ break;
+ case INTEL_TLV_SECURE_BOOT:
+ version->secure_boot = tlv->val[0];
+ break;
+ case INTEL_TLV_OTP_LOCK:
+ version->otp_lock = tlv->val[0];
+ break;
+ case INTEL_TLV_API_LOCK:
+ version->api_lock = tlv->val[0];
+ break;
+ case INTEL_TLV_DEBUG_LOCK:
+ version->debug_lock = tlv->val[0];
+ break;
+ case INTEL_TLV_MIN_FW:
+ version->min_fw_build_nn = tlv->val[0];
+ version->min_fw_build_cw = tlv->val[1];
+ version->min_fw_build_yy = tlv->val[2];
+ break;
+ case INTEL_TLV_LIMITED_CCE:
+ version->limited_cce = tlv->val[0];
+ break;
+ case INTEL_TLV_SBE_TYPE:
+ version->sbe_type = tlv->val[0];
+ break;
+ case INTEL_TLV_OTP_BDADDR:
+ memcpy(&version->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));
+ }
+
+ return 0;
+}
+
int btintel_read_version_tlv(struct hci_dev *hdev, struct intel_version_tlv *version)
{
struct sk_buff *skb;
@@ -1272,6 +1381,127 @@ int btintel_set_debug_features(struct hci_dev *hdev,
}
EXPORT_SYMBOL_GPL(btintel_set_debug_features);
+static int btintel_setup_combined(struct hci_dev *hdev)
+{
+ const u8 param[1] = { 0xFF };
+ struct intel_version ver;
+ struct intel_version_tlv ver_tlv;
+ struct sk_buff *skb;
+ int err;
+
+ BT_DBG("%s", hdev->name);
+
+ /* Starting from TyP device, the command parameter and response are
+ * changed even though the OCF for HCI_Intel_Read_Version command
+ * remains same. The legacy devices can handle even if the
+ * command has a parameter and returns a correct version information.
+ * So, it uses new format to support both legacy and new format.
+ */
+ skb = __hci_cmd_sync(hdev, 0xfc05, 1, param, HCI_CMD_TIMEOUT);
+ if (IS_ERR(skb)) {
+ bt_dev_err(hdev, "Reading Intel version command failed (%ld)",
+ PTR_ERR(skb));
+ return PTR_ERR(skb);
+ }
+
+ /* Check the status */
+ if (skb->data[0]) {
+ bt_dev_err(hdev, "Intel Read Version command failed (%02x)",
+ skb->data[0]);
+ kfree_skb(skb);
+ return -EIO;
+ }
+
+ /* For Legacy device, check the HW platform value and size */
+ if (skb->len == sizeof(ver) && skb->data[1] == 0x37) {
+ bt_dev_dbg(hdev, "Read the legacy Intel version information");
+
+ memcpy(&ver, skb->data, sizeof(ver));
+
+ /* Display version information */
+ btintel_version_info(hdev, &ver);
+
+ /* Check for supported iBT hardware variants of this firmware
+ * loading method.
+ *
+ * This check has been put in place to ensure correct forward
+ * compatibility options when newer hardware variants come
+ * along.
+ */
+ switch (ver.hw_variant) {
+ case 0x07: /* WP */
+ case 0x08: /* StP */
+ /* Legacy ROM product */
+ /* TODO: call setup routine for legacy rom product */
+ break;
+ case 0x0b: /* SfP */
+ case 0x0c: /* WsP */
+ case 0x11: /* JfP */
+ case 0x12: /* ThP */
+ case 0x13: /* HrP */
+ case 0x14: /* CcP */
+ /* TODO: call setup routine for bootloader product */
+ break;
+ default:
+ bt_dev_err(hdev, "Unsupported Intel hw variant (%u)",
+ ver.hw_variant);
+ return -EINVAL;
+ }
+
+ return err;
+ }
+
+ /* For TLV type device, parse the tlv data */
+ err = btintel_parse_version_tlv(hdev, &ver_tlv, skb);
+ if (err) {
+ bt_dev_err(hdev, "Failed to parse TLV version information");
+ return err;
+ }
+
+ if (INTEL_HW_PLATFORM(ver_tlv.cnvi_bt) != 0x37) {
+ bt_dev_err(hdev, "Unsupported Intel hardware platform (0x%2x)",
+ INTEL_HW_PLATFORM(ver_tlv.cnvi_bt));
+ return -EINVAL;
+ }
+
+ /* Display version information of TLV type */
+ btintel_version_info_tlv(hdev, &ver_tlv);
+
+ /* TODO: Need to filter the device for new generation */
+ /* TODO: call setup routine for tlv based bootloader product */
+
+ return err;
+}
+
+static int btintel_shutdown_combined(struct hci_dev *hdev)
+{
+ struct sk_buff *skb;
+
+ /* Send HCI Reset to the controller to stop any BT activity which
+ * were triggered. This will help to save power and maintain the
+ * sync b/w Host and controller
+ */
+ skb = __hci_cmd_sync(hdev, HCI_OP_RESET, 0, NULL, HCI_INIT_TIMEOUT);
+ if (IS_ERR(skb)) {
+ bt_dev_err(hdev, "HCI reset during shutdown failed");
+ return PTR_ERR(skb);
+ }
+ kfree_skb(skb);
+
+ return 0;
+}
+
+int btintel_configure_setup(struct hci_dev *hdev)
+{
+ /* TODO: Setup hdev callback here */
+ hdev->manufacturer = 2;
+ hdev->setup = btintel_setup_combined;
+ hdev->shutdown = btintel_shutdown_combined;
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(btintel_configure_setup);
+
MODULE_AUTHOR("Marcel Holtmann <[email protected]>");
MODULE_DESCRIPTION("Bluetooth support for Intel devices ver " VERSION);
MODULE_VERSION(VERSION);
diff --git a/drivers/bluetooth/btintel.h b/drivers/bluetooth/btintel.h
index d184064a5e7c..dda890d94a07 100644
--- a/drivers/bluetooth/btintel.h
+++ b/drivers/bluetooth/btintel.h
@@ -175,6 +175,7 @@ int btintel_read_debug_features(struct hci_dev *hdev,
struct intel_debug_features *features);
int btintel_set_debug_features(struct hci_dev *hdev,
const struct intel_debug_features *features);
+int btintel_configure_setup(struct hci_dev *hdev);
#else
static inline int btintel_check_bdaddr(struct hci_dev *hdev)
@@ -307,4 +308,9 @@ static inline int btintel_set_debug_features(struct hci_dev *hdev,
return -EOPNOTSUPP;
}
+static inline int btintel_configure_setup(struct hci_dev *hdev)
+{
+ return -ENODEV;
+}
+
#endif
--
2.25.1
Hi Tedd,
> -----Original Message-----
> From: Tedd Ho-Jeong An <[email protected]>
> Sent: Wednesday, August 4, 2021 10:10 AM
> To: [email protected]
> Cc: An, Tedd <[email protected]>
> Subject: [PATCH v6 02/12] Bluetooth: btintel: Add combined setup and
> shutdown functions
>
> From: Tedd Ho-Jeong An <[email protected]>
>
> There are multiple setup and shutdown functions for Intel device and the
> setup function to be used is depends on the USB PID/VID, which makes
> difficult to maintain the code and increases the code size.
>
> This patch adds combined setup and shutdown functions to provide a single
> entry point for all Intel devices and choose the setup functions based on the
> information read with HCI_Intel_Read_Version command.
>
> Starting from TyP device, the command and response parameters for
> HCI_Intel_Read_Version command are changed even though OCF remains
> same. However, the legacy devices still can handle the command without
> error even if it has a extra parameter, so to simplify the flow, the new
> command format is used to read the version information for both legacy and
> new (tlv based) format.
>
> Also, it also adds a routine to setup the hdev callbacks in btintel.
>
> Signed-off-by: Tedd Ho-Jeong An <[email protected]>
> ---
> drivers/bluetooth/btintel.c | 230 ++++++++++++++++++++++++++++++++++++
> drivers/bluetooth/btintel.h | 6 +
> 2 files changed, 236 insertions(+)
>
> diff --git a/drivers/bluetooth/btintel.c b/drivers/bluetooth/btintel.c index
> e44b6993cf91..3d98fc2a64b9 100644
> --- a/drivers/bluetooth/btintel.c
> +++ b/drivers/bluetooth/btintel.c
> @@ -236,6 +236,8 @@ int btintel_version_info(struct hci_dev *hdev, struct
> intel_version *ver)
> * compatibility options when newer hardware variants come along.
> */
> switch (ver->hw_variant) {
> + case 0x07: /* WP - Legacy ROM */
> + case 0x08: /* StP - Legacy ROM */
> case 0x0b: /* SfP */
> case 0x0c: /* WsP */
> case 0x11: /* JfP */
> @@ -250,9 +252,15 @@ int btintel_version_info(struct hci_dev *hdev, struct
> intel_version *ver)
> }
>
> switch (ver->fw_variant) {
> + case 0x01:
> + variant = "Legacy ROM 2.5";
> + break;
> case 0x06:
> variant = "Bootloader";
> break;
> + case 0x22:
> + variant = "Legacy ROM 2.x";
> + break;
> case 0x23:
> variant = "Firmware";
> break;
> @@ -483,6 +491,107 @@ int btintel_version_info_tlv(struct hci_dev *hdev,
> struct intel_version_tlv *ver }
> EXPORT_SYMBOL_GPL(btintel_version_info_tlv);
>
> +static int btintel_parse_version_tlv(struct hci_dev *hdev,
> + struct intel_version_tlv *version,
> + struct sk_buff *skb)
> +{
> + /* 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) {
> + struct intel_tlv *tlv;
> +
> + /* Make sure skb has a minimum length of the header */
> + if (skb->len < 2)
> + return -EINVAL;
Can we use sizeof(*tlv) instead of 2 ?
> +
> + tlv = (struct intel_tlv *)skb->data;
> +
> + /* Make sure skb has a enough data */
> + if (skb->len < tlv->len + sizeof(*tlv))
> + return -EINVAL;
> +
> + switch (tlv->type) {
> + case INTEL_TLV_CNVI_TOP:
> + version->cnvi_top = get_unaligned_le32(tlv->val);
> + break;
> + case INTEL_TLV_CNVR_TOP:
> + version->cnvr_top = get_unaligned_le32(tlv->val);
> + break;
> + case INTEL_TLV_CNVI_BT:
> + version->cnvi_bt = get_unaligned_le32(tlv->val);
> + break;
> + case INTEL_TLV_CNVR_BT:
> + version->cnvr_bt = get_unaligned_le32(tlv->val);
> + break;
> + case INTEL_TLV_DEV_REV_ID:
> + version->dev_rev_id = get_unaligned_le16(tlv->val);
> + break;
> + case INTEL_TLV_IMAGE_TYPE:
> + version->img_type = tlv->val[0];
> + break;
> + case INTEL_TLV_TIME_STAMP:
> + /* If image type is Operational firmware (0x03), then
> + * running FW Calendar Week and Year information
> can
> + * be extracted from Timestamp information
> + */
> + version->min_fw_build_cw = tlv->val[0];
> + version->min_fw_build_yy = tlv->val[1];
> + version->timestamp = get_unaligned_le16(tlv->val);
> + break;
> + case INTEL_TLV_BUILD_TYPE:
> + version->build_type = tlv->val[0];
> + break;
> + case INTEL_TLV_BUILD_NUM:
> + /* If image type is Operational firmware (0x03), then
> + * running FW build number can be extracted from
> the
> + * Build information
> + */
> + version->min_fw_build_nn = tlv->val[0];
> + version->build_num = get_unaligned_le32(tlv->val);
> + break;
> + case INTEL_TLV_SECURE_BOOT:
> + version->secure_boot = tlv->val[0];
> + break;
> + case INTEL_TLV_OTP_LOCK:
> + version->otp_lock = tlv->val[0];
> + break;
> + case INTEL_TLV_API_LOCK:
> + version->api_lock = tlv->val[0];
> + break;
> + case INTEL_TLV_DEBUG_LOCK:
> + version->debug_lock = tlv->val[0];
> + break;
> + case INTEL_TLV_MIN_FW:
> + version->min_fw_build_nn = tlv->val[0];
> + version->min_fw_build_cw = tlv->val[1];
> + version->min_fw_build_yy = tlv->val[2];
> + break;
> + case INTEL_TLV_LIMITED_CCE:
> + version->limited_cce = tlv->val[0];
> + break;
> + case INTEL_TLV_SBE_TYPE:
> + version->sbe_type = tlv->val[0];
> + break;
> + case INTEL_TLV_OTP_BDADDR:
> + memcpy(&version->otp_bd_addr, tlv->val, tlv->len);
I think we need to restrict copying to only sizeof(version->otp_bd_addr) bytes here.
> + break;
> + default:
> + /* Ignore rest of information */
> + break;
> + }
> + /* consume the current tlv and move to next*/
> + skb_pull(skb, tlv->len + sizeof(*tlv));
> + }
> +
> + return 0;
> +}
> +
> int btintel_read_version_tlv(struct hci_dev *hdev, struct intel_version_tlv
> *version) {
> struct sk_buff *skb;
> @@ -1272,6 +1381,127 @@ int btintel_set_debug_features(struct hci_dev
> *hdev, } EXPORT_SYMBOL_GPL(btintel_set_debug_features);
>
> +static int btintel_setup_combined(struct hci_dev *hdev) {
> + const u8 param[1] = { 0xFF };
> + struct intel_version ver;
> + struct intel_version_tlv ver_tlv;
> + struct sk_buff *skb;
> + int err;
> +
> + BT_DBG("%s", hdev->name);
> +
> + /* Starting from TyP device, the command parameter and response
> are
> + * changed even though the OCF for HCI_Intel_Read_Version
> command
> + * remains same. The legacy devices can handle even if the
> + * command has a parameter and returns a correct version
> information.
> + * So, it uses new format to support both legacy and new format.
> + */
> + skb = __hci_cmd_sync(hdev, 0xfc05, 1, param, HCI_CMD_TIMEOUT);
> + if (IS_ERR(skb)) {
> + bt_dev_err(hdev, "Reading Intel version command failed
> (%ld)",
> + PTR_ERR(skb));
> + return PTR_ERR(skb);
> + }
> +
> + /* Check the status */
> + if (skb->data[0]) {
Need to check for skb->len before accessing data.
> + bt_dev_err(hdev, "Intel Read Version command failed
> (%02x)",
> + skb->data[0]);
> + kfree_skb(skb);
> + return -EIO;
> + }
> +
> + /* For Legacy device, check the HW platform value and size */
> + if (skb->len == sizeof(ver) && skb->data[1] == 0x37) {
> + bt_dev_dbg(hdev, "Read the legacy Intel version
> information");
> +
> + memcpy(&ver, skb->data, sizeof(ver));
> +
> + /* Display version information */
> + btintel_version_info(hdev, &ver);
> +
> + /* Check for supported iBT hardware variants of this
> firmware
> + * loading method.
> + *
> + * This check has been put in place to ensure correct forward
> + * compatibility options when newer hardware variants come
> + * along.
> + */
> + switch (ver.hw_variant) {
> + case 0x07: /* WP */
> + case 0x08: /* StP */
> + /* Legacy ROM product */
> + /* TODO: call setup routine for legacy rom product */
> + break;
> + case 0x0b: /* SfP */
> + case 0x0c: /* WsP */
> + case 0x11: /* JfP */
> + case 0x12: /* ThP */
> + case 0x13: /* HrP */
> + case 0x14: /* CcP */
> + /* TODO: call setup routine for bootloader product
> */
> + break;
> + default:
> + bt_dev_err(hdev, "Unsupported Intel hw variant
> (%u)",
> + ver.hw_variant);
> + return -EINVAL;
Possibility of memory leak here
> + }
> +
> + return err;
Ditto.. plus possibility of returning uninitialized err.
> + }
> +
> + /* For TLV type device, parse the tlv data */
> + err = btintel_parse_version_tlv(hdev, &ver_tlv, skb);
> + if (err) {
> + bt_dev_err(hdev, "Failed to parse TLV version information");
> + return err;
> + }
> +
> + if (INTEL_HW_PLATFORM(ver_tlv.cnvi_bt) != 0x37) {
> + bt_dev_err(hdev, "Unsupported Intel hardware platform
> (0x%2x)",
> + INTEL_HW_PLATFORM(ver_tlv.cnvi_bt));
> + return -EINVAL;
> + }
> +
> + /* Display version information of TLV type */
> + btintel_version_info_tlv(hdev, &ver_tlv);
> +
> + /* TODO: Need to filter the device for new generation */
> + /* TODO: call setup routine for tlv based bootloader product */
> +
> + return err;
> +}
> +
> +static int btintel_shutdown_combined(struct hci_dev *hdev) {
> + struct sk_buff *skb;
> +
> + /* Send HCI Reset to the controller to stop any BT activity which
> + * were triggered. This will help to save power and maintain the
> + * sync b/w Host and controller
> + */
> + skb = __hci_cmd_sync(hdev, HCI_OP_RESET, 0, NULL,
> HCI_INIT_TIMEOUT);
> + if (IS_ERR(skb)) {
> + bt_dev_err(hdev, "HCI reset during shutdown failed");
> + return PTR_ERR(skb);
> + }
> + kfree_skb(skb);
> +
> + return 0;
> +}
> +
> +int btintel_configure_setup(struct hci_dev *hdev) {
> + /* TODO: Setup hdev callback here */
> + hdev->manufacturer = 2;
> + hdev->setup = btintel_setup_combined;
> + hdev->shutdown = btintel_shutdown_combined;
> +
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(btintel_configure_setup);
> +
> MODULE_AUTHOR("Marcel Holtmann <[email protected]>");
> MODULE_DESCRIPTION("Bluetooth support for Intel devices ver " VERSION);
> MODULE_VERSION(VERSION); diff --git a/drivers/bluetooth/btintel.h
> b/drivers/bluetooth/btintel.h index d184064a5e7c..dda890d94a07 100644
> --- a/drivers/bluetooth/btintel.h
> +++ b/drivers/bluetooth/btintel.h
> @@ -175,6 +175,7 @@ int btintel_read_debug_features(struct hci_dev
> *hdev,
> struct intel_debug_features *features); int
> btintel_set_debug_features(struct hci_dev *hdev,
> const struct intel_debug_features *features);
> +int btintel_configure_setup(struct hci_dev *hdev);
> #else
>
> static inline int btintel_check_bdaddr(struct hci_dev *hdev) @@ -307,4
> +308,9 @@ static inline int btintel_set_debug_features(struct hci_dev *hdev,
> return -EOPNOTSUPP;
> }
>
> +static inline int btintel_configure_setup(struct hci_dev *hdev) {
> + return -ENODEV;
> +}
> +
> #endif
> --
> 2.25.1
Thanks,
Kiran