Message ID | 20210804044032.59729-3-hj.tedd.an@gmail.com (mailing list archive) |
---|---|
State | Superseded |
Headers | show |
Series | Bluetooth: btintel: Refactoring setup routines | expand |
Hi Tedd, > -----Original Message----- > From: Tedd Ho-Jeong An <hj.tedd.an@gmail.com> > Sent: Wednesday, August 4, 2021 10:10 AM > To: linux-bluetooth@vger.kernel.org > Cc: An, Tedd <tedd.an@intel.com> > Subject: [PATCH v6 02/12] Bluetooth: btintel: Add combined setup and > shutdown functions > > From: Tedd Ho-Jeong An <tedd.an@intel.com> > > 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 <tedd.an@intel.com> > --- > 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 <marcel@holtmann.org>"); > 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
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 <marcel@holtmann.org>"); 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