* [PATCH v5 01/11] Bluetooth: Add support hdev to allocate private data
2021-07-29 18:35 [PATCH v5 00/11] Bluetooth: btintel: Refactoring setup routines Tedd Ho-Jeong An
@ 2021-07-29 18:35 ` Tedd Ho-Jeong An
2021-07-29 19:13 ` Bluetooth: btintel: Refactoring setup routines bluez.test.bot
2021-07-29 18:35 ` [PATCH v5 02/11] Bluetooth: btintel: Add combined setup and shutdown functions Tedd Ho-Jeong An
` (9 subsequent siblings)
10 siblings, 1 reply; 26+ messages in thread
From: Tedd Ho-Jeong An @ 2021-07-29 18:35 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Tedd Ho-Jeong An
From: Tedd Ho-Jeong An <tedd.an@intel.com>
This patch adds support hdev to allocate extra size for private data.
The size of private data is specified in the hdev_alloc_size(priv_size)
and the allocated buffer can be accessed with hci_get_priv(hdev).
Signed-off-by: Tedd Ho-Jeong An <tedd.an@intel.com>
---
include/net/bluetooth/hci_core.h | 13 ++++++++++++-
net/bluetooth/hci_core.c | 13 ++++++++++---
2 files changed, 22 insertions(+), 4 deletions(-)
diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index a53e94459ecd..7ef0e5644e87 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -1223,10 +1223,21 @@ static inline void hci_set_drvdata(struct hci_dev *hdev, void *data)
dev_set_drvdata(&hdev->dev, data);
}
+static inline void *hci_get_priv(struct hci_dev *hdev)
+{
+ return (char *)hdev + sizeof(*hdev);
+}
+
struct hci_dev *hci_dev_get(int index);
struct hci_dev *hci_get_route(bdaddr_t *dst, bdaddr_t *src, u8 src_type);
-struct hci_dev *hci_alloc_dev(void);
+struct hci_dev *hci_alloc_dev_priv(int sizeof_priv);
+
+static inline struct hci_dev *hci_alloc_dev(void)
+{
+ return hci_alloc_dev_priv(0);
+}
+
void hci_free_dev(struct hci_dev *hdev);
int hci_register_dev(struct hci_dev *hdev);
void hci_unregister_dev(struct hci_dev *hdev);
diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index 2560ed2f144d..ae6f5f26db6a 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -3751,11 +3751,18 @@ static int hci_suspend_notifier(struct notifier_block *nb, unsigned long action,
}
/* Alloc HCI device */
-struct hci_dev *hci_alloc_dev(void)
+struct hci_dev *hci_alloc_dev_priv(int sizeof_priv)
{
struct hci_dev *hdev;
+ unsigned int alloc_size;
- hdev = kzalloc(sizeof(*hdev), GFP_KERNEL);
+ alloc_size = sizeof(*hdev);
+ if (sizeof_priv) {
+ /* Fixme: May need ALIGN-ment? */
+ alloc_size += sizeof_priv;
+ }
+
+ hdev = kzalloc(alloc_size, GFP_KERNEL);
if (!hdev)
return NULL;
@@ -3869,7 +3876,7 @@ struct hci_dev *hci_alloc_dev(void)
return hdev;
}
-EXPORT_SYMBOL(hci_alloc_dev);
+EXPORT_SYMBOL(hci_alloc_dev_priv);
/* Free HCI device */
void hci_free_dev(struct hci_dev *hdev)
--
2.25.1
^ permalink raw reply related [flat|nested] 26+ messages in thread* RE: Bluetooth: btintel: Refactoring setup routines
2021-07-29 18:35 ` [PATCH v5 01/11] Bluetooth: Add support hdev to allocate private data Tedd Ho-Jeong An
@ 2021-07-29 19:13 ` bluez.test.bot
0 siblings, 0 replies; 26+ messages in thread
From: bluez.test.bot @ 2021-07-29 19:13 UTC (permalink / raw)
To: linux-bluetooth, hj.tedd.an
[-- Attachment #1: Type: text/plain, Size: 2727 bytes --]
This is automated email and please do not reply to this email!
Dear submitter,
Thank you for submitting the patches to the linux bluetooth mailing list.
This is a CI test results with your patch series:
PW Link:https://patchwork.kernel.org/project/bluetooth/list/?series=523593
---Test result---
Test Summary:
CheckPatch PASS 7.11 seconds
GitLint PASS 1.06 seconds
BuildKernel PASS 505.33 seconds
TestRunner: Setup PASS 332.83 seconds
TestRunner: l2cap-tester PASS 2.51 seconds
TestRunner: bnep-tester PASS 1.91 seconds
TestRunner: mgmt-tester PASS 29.83 seconds
TestRunner: rfcomm-tester PASS 2.07 seconds
TestRunner: sco-tester PASS 2.03 seconds
TestRunner: smp-tester FAIL 2.09 seconds
TestRunner: userchan-tester PASS 1.94 seconds
Details
##############################
Test: CheckPatch - PASS - 7.11 seconds
Run checkpatch.pl script with rule in .checkpatch.conf
##############################
Test: GitLint - PASS - 1.06 seconds
Run gitlint with rule in .gitlint
##############################
Test: BuildKernel - PASS - 505.33 seconds
Build Kernel with minimal configuration supports Bluetooth
##############################
Test: TestRunner: Setup - PASS - 332.83 seconds
Setup environment for running Test Runner
##############################
Test: TestRunner: l2cap-tester - PASS - 2.51 seconds
Run test-runner with l2cap-tester
Total: 40, Passed: 40 (100.0%), Failed: 0, Not Run: 0
##############################
Test: TestRunner: bnep-tester - PASS - 1.91 seconds
Run test-runner with bnep-tester
Total: 1, Passed: 1 (100.0%), Failed: 0, Not Run: 0
##############################
Test: TestRunner: mgmt-tester - PASS - 29.83 seconds
Run test-runner with mgmt-tester
Total: 448, Passed: 445 (99.3%), Failed: 0, Not Run: 3
##############################
Test: TestRunner: rfcomm-tester - PASS - 2.07 seconds
Run test-runner with rfcomm-tester
Total: 9, Passed: 9 (100.0%), Failed: 0, Not Run: 0
##############################
Test: TestRunner: sco-tester - PASS - 2.03 seconds
Run test-runner with sco-tester
Total: 8, Passed: 8 (100.0%), Failed: 0, Not Run: 0
##############################
Test: TestRunner: smp-tester - FAIL - 2.09 seconds
Run test-runner with smp-tester
Total: 8, Passed: 7 (87.5%), Failed: 1, Not Run: 0
Failed Test Cases
SMP Client - SC Request 2 Failed 0.019 seconds
##############################
Test: TestRunner: userchan-tester - PASS - 1.94 seconds
Run test-runner with userchan-tester
Total: 3, Passed: 3 (100.0%), Failed: 0, Not Run: 0
---
Regards,
Linux Bluetooth
[-- Attachment #2: l2cap-tester.log --]
[-- Type: application/octet-stream, Size: 44386 bytes --]
[-- Attachment #3: bnep-tester.log --]
[-- Type: application/octet-stream, Size: 3593 bytes --]
[-- Attachment #4: mgmt-tester.log --]
[-- Type: application/octet-stream, Size: 616863 bytes --]
[-- Attachment #5: rfcomm-tester.log --]
[-- Type: application/octet-stream, Size: 11713 bytes --]
[-- Attachment #6: sco-tester.log --]
[-- Type: application/octet-stream, Size: 9948 bytes --]
[-- Attachment #7: smp-tester.log --]
[-- Type: application/octet-stream, Size: 11741 bytes --]
[-- Attachment #8: userchan-tester.log --]
[-- Type: application/octet-stream, Size: 5490 bytes --]
^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH v5 02/11] Bluetooth: btintel: Add combined setup and shutdown functions
2021-07-29 18:35 [PATCH v5 00/11] Bluetooth: btintel: Refactoring setup routines Tedd Ho-Jeong An
2021-07-29 18:35 ` [PATCH v5 01/11] Bluetooth: Add support hdev to allocate private data Tedd Ho-Jeong An
@ 2021-07-29 18:35 ` Tedd Ho-Jeong An
2021-07-29 19:21 ` Marcel Holtmann
2021-07-29 18:35 ` [PATCH v5 03/11] Bluetooth: btintel: Refactoring setup routine for legacy ROM sku Tedd Ho-Jeong An
` (8 subsequent siblings)
10 siblings, 1 reply; 26+ messages in thread
From: Tedd Ho-Jeong An @ 2021-07-29 18:35 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Tedd Ho-Jeong An
From: Tedd Ho-Jeong An <tedd.an@intel.com>
There are multiple setup and shutdown functions for Intel device and the
setup function to use 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, for HCI_Intel_Read_Version command, the
command parameter and response are changed even though OCF remains
same. Luckly the legacy devices still can handle the command without
error even if it has a extra parameter, so it uses the new command
format to support both legacy and new (tlv based) format.
Signed-off-by: Tedd Ho-Jeong An <tedd.an@intel.com>
---
drivers/bluetooth/btintel.c | 196 ++++++++++++++++++++++++++++++++++++
drivers/bluetooth/btintel.h | 12 +++
2 files changed, 208 insertions(+)
diff --git a/drivers/bluetooth/btintel.c b/drivers/bluetooth/btintel.c
index e44b6993cf91..a23304435814 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,98 @@ 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;
+
+ tlv = (struct intel_tlv *)skb->data;
+ 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 +1372,102 @@ int btintel_set_debug_features(struct hci_dev *hdev,
}
EXPORT_SYMBOL_GPL(btintel_set_debug_features);
+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->data[1] == 0x37 && skb->len == sizeof(ver)) {
+ bt_dev_dbg(hdev, "Read the legacy Intel version information");
+
+ memcpy(&ver, skb->data, sizeof(ver));
+
+ /* Display version information */
+ btintel_version_info(hdev, &ver);
+
+ /* Identify the device type based on the read version */
+ 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 */
+ btintel_parse_version_tlv(hdev, &ver_tlv, skb);
+
+ /* 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;
+}
+EXPORT_SYMBOL_GPL(btintel_setup_combined);
+
+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;
+}
+EXPORT_SYMBOL_GPL(btintel_shutdown_combined);
+
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..68ffa84fa87a 100644
--- a/drivers/bluetooth/btintel.h
+++ b/drivers/bluetooth/btintel.h
@@ -165,6 +165,8 @@ int btintel_read_boot_params(struct hci_dev *hdev,
struct intel_boot_params *params);
int btintel_download_firmware(struct hci_dev *dev, struct intel_version *ver,
const struct firmware *fw, u32 *boot_param);
+int btintel_setup_combined(struct hci_dev *hdev);
+int btintel_shutdown_combined(struct hci_dev *hdev);
int btintel_download_firmware_newgen(struct hci_dev *hdev,
struct intel_version_tlv *ver,
const struct firmware *fw,
@@ -283,6 +285,16 @@ static inline int btintel_download_firmware(struct hci_dev *dev,
return -EOPNOTSUPP;
}
+static inline int btintel_setup_combined(struct hci_dev *hdev)
+{
+ return -EOPNOTSUPP;
+}
+
+static inline int btintel_shutdown_combined(struct hci_dev *hdev)
+{
+ return -EOPNOTSUPP;
+}
+
static inline int btintel_download_firmware_newgen(struct hci_dev *hdev,
const struct firmware *fw,
u32 *boot_param,
--
2.25.1
^ permalink raw reply related [flat|nested] 26+ messages in thread* Re: [PATCH v5 02/11] Bluetooth: btintel: Add combined setup and shutdown functions
2021-07-29 18:35 ` [PATCH v5 02/11] Bluetooth: btintel: Add combined setup and shutdown functions Tedd Ho-Jeong An
@ 2021-07-29 19:21 ` Marcel Holtmann
0 siblings, 0 replies; 26+ messages in thread
From: Marcel Holtmann @ 2021-07-29 19:21 UTC (permalink / raw)
To: Tedd Ho-Jeong An; +Cc: linux-bluetooth, Tedd Ho-Jeong An
Hi Tedd,
> There are multiple setup and shutdown functions for Intel device and the
> setup function to use 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, for HCI_Intel_Read_Version command, the
> command parameter and response are changed even though OCF remains
> same. Luckly the legacy devices still can handle the command without
> error even if it has a extra parameter, so it uses the new command
> format to support both legacy and new (tlv based) format.
>
> Signed-off-by: Tedd Ho-Jeong An <tedd.an@intel.com>
> ---
> drivers/bluetooth/btintel.c | 196 ++++++++++++++++++++++++++++++++++++
> drivers/bluetooth/btintel.h | 12 +++
> 2 files changed, 208 insertions(+)
>
> diff --git a/drivers/bluetooth/btintel.c b/drivers/bluetooth/btintel.c
> index e44b6993cf91..a23304435814 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,98 @@ 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;
> +
> + tlv = (struct intel_tlv *)skb->data;
> + 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 +1372,102 @@ int btintel_set_debug_features(struct hci_dev *hdev,
> }
> EXPORT_SYMBOL_GPL(btintel_set_debug_features);
>
> +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->data[1] == 0x37 && skb->len == sizeof(ver)) {
while the command status is guaranteed, everything after that is not. So you might want to do the length check first. And then look for 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);
> +
> + /* Identify the device type based on the read version */
Initially I added some comments on that we filter on hw_variant on purpose to enable forward compatibility. It would be good to keep having such a statement.
> + 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;
> + }
> +
Now this eliminates the hard limit to a given hw_platform like we had before.
So I think you really need to be careful to not assume it is a TLV based system. We need to have some extra checks here before we continue into the TLV world.
> + /* For TLV type device, parse the tlv data */
> + btintel_parse_version_tlv(hdev, &ver_tlv, skb);
> +
> + /* 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;
> +}
> +EXPORT_SYMBOL_GPL(btintel_setup_combined);
> +
> +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;
> +}
> +EXPORT_SYMBOL_GPL(btintel_shutdown_combined);
> +
> 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..68ffa84fa87a 100644
> --- a/drivers/bluetooth/btintel.h
> +++ b/drivers/bluetooth/btintel.h
> @@ -165,6 +165,8 @@ int btintel_read_boot_params(struct hci_dev *hdev,
> struct intel_boot_params *params);
> int btintel_download_firmware(struct hci_dev *dev, struct intel_version *ver,
> const struct firmware *fw, u32 *boot_param);
> +int btintel_setup_combined(struct hci_dev *hdev);
> +int btintel_shutdown_combined(struct hci_dev *hdev);
> int btintel_download_firmware_newgen(struct hci_dev *hdev,
> struct intel_version_tlv *ver,
> const struct firmware *fw,
> @@ -283,6 +285,16 @@ static inline int btintel_download_firmware(struct hci_dev *dev,
> return -EOPNOTSUPP;
> }
>
> +static inline int btintel_setup_combined(struct hci_dev *hdev)
> +{
> + return -EOPNOTSUPP;
> +}
> +
> +static inline int btintel_shutdown_combined(struct hci_dev *hdev)
> +{
> + return -EOPNOTSUPP;
> +}
> +
> static inline int btintel_download_firmware_newgen(struct hci_dev *hdev,
> const struct firmware *fw,
> u32 *boot_param,
Regards
Marcel
^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH v5 03/11] Bluetooth: btintel: Refactoring setup routine for legacy ROM sku
2021-07-29 18:35 [PATCH v5 00/11] Bluetooth: btintel: Refactoring setup routines Tedd Ho-Jeong An
2021-07-29 18:35 ` [PATCH v5 01/11] Bluetooth: Add support hdev to allocate private data Tedd Ho-Jeong An
2021-07-29 18:35 ` [PATCH v5 02/11] Bluetooth: btintel: Add combined setup and shutdown functions Tedd Ho-Jeong An
@ 2021-07-29 18:35 ` Tedd Ho-Jeong An
2021-07-29 19:25 ` Marcel Holtmann
2021-07-29 18:35 ` [PATCH v5 04/11] Bluetooth: btintel: Add btintel data struct Tedd Ho-Jeong An
` (7 subsequent siblings)
10 siblings, 1 reply; 26+ messages in thread
From: Tedd Ho-Jeong An @ 2021-07-29 18:35 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Tedd Ho-Jeong An
From: Tedd Ho-Jeong An <tedd.an@intel.com>
This patch refactors the setup routines for legacy ROM product into
combined setup, and move the related functions from btusb to btintel.
Signed-off-by: Tedd Ho-Jeong An <tedd.an@intel.com>
---
drivers/bluetooth/btintel.c | 287 +++++++++++++++++++++++++++-
drivers/bluetooth/btusb.c | 362 +-----------------------------------
2 files changed, 294 insertions(+), 355 deletions(-)
diff --git a/drivers/bluetooth/btintel.c b/drivers/bluetooth/btintel.c
index a23304435814..cfc097694b53 100644
--- a/drivers/bluetooth/btintel.c
+++ b/drivers/bluetooth/btintel.c
@@ -1372,6 +1372,291 @@ int btintel_set_debug_features(struct hci_dev *hdev,
}
EXPORT_SYMBOL_GPL(btintel_set_debug_features);
+static const struct firmware *btintel_legacy_rom_get_fw(struct hci_dev *hdev,
+ struct intel_version *ver)
+{
+ const struct firmware *fw;
+ char fwname[64];
+ int ret;
+
+ snprintf(fwname, sizeof(fwname),
+ "intel/ibt-hw-%x.%x.%x-fw-%x.%x.%x.%x.%x.bseq",
+ ver->hw_platform, ver->hw_variant, ver->hw_revision,
+ ver->fw_variant, ver->fw_revision, ver->fw_build_num,
+ ver->fw_build_ww, ver->fw_build_yy);
+
+ ret = request_firmware(&fw, fwname, &hdev->dev);
+ if (ret < 0) {
+ if (ret == -EINVAL) {
+ bt_dev_err(hdev, "Intel firmware file request failed (%d)",
+ ret);
+ return NULL;
+ }
+
+ bt_dev_err(hdev, "failed to open Intel firmware file: %s (%d)",
+ fwname, ret);
+
+ /* If the correct firmware patch file is not found, use the
+ * default firmware patch file instead
+ */
+ snprintf(fwname, sizeof(fwname), "intel/ibt-hw-%x.%x.bseq",
+ ver->hw_platform, ver->hw_variant);
+ if (request_firmware(&fw, fwname, &hdev->dev) < 0) {
+ bt_dev_err(hdev, "failed to open default fw file: %s",
+ fwname);
+ return NULL;
+ }
+ }
+
+ bt_dev_info(hdev, "Intel Bluetooth firmware file: %s", fwname);
+
+ return fw;
+}
+
+static int btintel_legacy_rom_patching(struct hci_dev *hdev,
+ const struct firmware *fw,
+ const u8 **fw_ptr, int *disable_patch)
+{
+ struct sk_buff *skb;
+ struct hci_command_hdr *cmd;
+ const u8 *cmd_param;
+ struct hci_event_hdr *evt = NULL;
+ const u8 *evt_param = NULL;
+ int remain = fw->size - (*fw_ptr - fw->data);
+
+ /* The first byte indicates the types of the patch command or event.
+ * 0x01 means HCI command and 0x02 is HCI event. If the first bytes
+ * in the current firmware buffer doesn't start with 0x01 or
+ * the size of remain buffer is smaller than HCI command header,
+ * the firmware file is corrupted and it should stop the patching
+ * process.
+ */
+ if (remain > HCI_COMMAND_HDR_SIZE && *fw_ptr[0] != 0x01) {
+ bt_dev_err(hdev, "Intel fw corrupted: invalid cmd read");
+ return -EINVAL;
+ }
+ (*fw_ptr)++;
+ remain--;
+
+ cmd = (struct hci_command_hdr *)(*fw_ptr);
+ *fw_ptr += sizeof(*cmd);
+ remain -= sizeof(*cmd);
+
+ /* Ensure that the remain firmware data is long enough than the length
+ * of command parameter. If not, the firmware file is corrupted.
+ */
+ if (remain < cmd->plen) {
+ bt_dev_err(hdev, "Intel fw corrupted: invalid cmd len");
+ return -EFAULT;
+ }
+
+ /* If there is a command that loads a patch in the firmware
+ * file, then enable the patch upon success, otherwise just
+ * disable the manufacturer mode, for example patch activation
+ * is not required when the default firmware patch file is used
+ * because there are no patch data to load.
+ */
+ if (*disable_patch && le16_to_cpu(cmd->opcode) == 0xfc8e)
+ *disable_patch = 0;
+
+ cmd_param = *fw_ptr;
+ *fw_ptr += cmd->plen;
+ remain -= cmd->plen;
+
+ /* This reads the expected events when the above command is sent to the
+ * device. Some vendor commands expects more than one events, for
+ * example command status event followed by vendor specific event.
+ * For this case, it only keeps the last expected event. so the command
+ * can be sent with __hci_cmd_sync_ev() which returns the sk_buff of
+ * last expected event.
+ */
+ while (remain > HCI_EVENT_HDR_SIZE && *fw_ptr[0] == 0x02) {
+ (*fw_ptr)++;
+ remain--;
+
+ evt = (struct hci_event_hdr *)(*fw_ptr);
+ *fw_ptr += sizeof(*evt);
+ remain -= sizeof(*evt);
+
+ if (remain < evt->plen) {
+ bt_dev_err(hdev, "Intel fw corrupted: invalid evt len");
+ return -EFAULT;
+ }
+
+ evt_param = *fw_ptr;
+ *fw_ptr += evt->plen;
+ remain -= evt->plen;
+ }
+
+ /* Every HCI commands in the firmware file has its correspond event.
+ * If event is not found or remain is smaller than zero, the firmware
+ * file is corrupted.
+ */
+ if (!evt || !evt_param || remain < 0) {
+ bt_dev_err(hdev, "Intel fw corrupted: invalid evt read");
+ return -EFAULT;
+ }
+
+ skb = __hci_cmd_sync_ev(hdev, le16_to_cpu(cmd->opcode), cmd->plen,
+ cmd_param, evt->evt, HCI_INIT_TIMEOUT);
+ if (IS_ERR(skb)) {
+ bt_dev_err(hdev, "sending Intel patch command (0x%4.4x) failed (%ld)",
+ cmd->opcode, PTR_ERR(skb));
+ return PTR_ERR(skb);
+ }
+
+ /* It ensures that the returned event matches the event data read from
+ * the firmware file. At fist, it checks the length and then
+ * the contents of the event.
+ */
+ if (skb->len != evt->plen) {
+ bt_dev_err(hdev, "mismatch event length (opcode 0x%4.4x)",
+ le16_to_cpu(cmd->opcode));
+ kfree_skb(skb);
+ return -EFAULT;
+ }
+
+ if (memcmp(skb->data, evt_param, evt->plen)) {
+ bt_dev_err(hdev, "mismatch event parameter (opcode 0x%4.4x)",
+ le16_to_cpu(cmd->opcode));
+ kfree_skb(skb);
+ return -EFAULT;
+ }
+ kfree_skb(skb);
+
+ return 0;
+}
+
+static int btintel_legacy_rom_setup(struct hci_dev *hdev,
+ struct intel_version *ver)
+{
+ const struct firmware *fw;
+ const u8 *fw_ptr;
+ int disable_patch, err;
+ struct intel_version new_ver;
+
+ BT_DBG("%s", hdev->name);
+
+ /* fw_patch_num indicates the version of patch the device currently
+ * have. If there is no patch data in the device, it is always 0x00.
+ * So, if it is other than 0x00, no need to patch the device again.
+ */
+ if (ver->fw_patch_num) {
+ bt_dev_info(hdev,
+ "Intel device is already patched. patch num: %02x",
+ ver->fw_patch_num);
+ goto complete;
+ }
+
+ /* Opens the firmware patch file based on the firmware version read
+ * from the controller. If it fails to open the matching firmware
+ * patch file, it tries to open the default firmware patch file.
+ * If no patch file is found, allow the device to operate without
+ * a patch.
+ */
+ fw = btintel_legacy_rom_get_fw(hdev, ver);
+ if (!fw)
+ goto complete;
+ fw_ptr = fw->data;
+
+ /* Enable the manufacturer mode of the controller.
+ * Only while this mode is enabled, the driver can download the
+ * firmware patch data and configuration parameters.
+ */
+ err = btintel_enter_mfg(hdev);
+ if (err) {
+ release_firmware(fw);
+ return err;
+ }
+
+ disable_patch = 1;
+
+ /* The firmware data file consists of list of Intel specific HCI
+ * commands and its expected events. The first byte indicates the
+ * type of the message, either HCI command or HCI event.
+ *
+ * It reads the command and its expected event from the firmware file,
+ * and send to the controller. Once __hci_cmd_sync_ev() returns,
+ * the returned event is compared with the event read from the firmware
+ * file and it will continue until all the messages are downloaded to
+ * the controller.
+ *
+ * Once the firmware patching is completed successfully,
+ * the manufacturer mode is disabled with reset and activating the
+ * downloaded patch.
+ *
+ * If the firmware patching fails, the manufacturer mode is
+ * disabled with reset and deactivating the patch.
+ *
+ * If the default patch file is used, no reset is done when disabling
+ * the manufacturer.
+ */
+ while (fw->size > fw_ptr - fw->data) {
+ int ret;
+
+ ret = btintel_legacy_rom_patching(hdev, fw, &fw_ptr,
+ &disable_patch);
+ if (ret < 0)
+ goto exit_mfg_deactivate;
+ }
+
+ release_firmware(fw);
+
+ if (disable_patch)
+ goto exit_mfg_disable;
+
+ /* Patching completed successfully and disable the manufacturer mode
+ * with reset and activate the downloaded firmware patches.
+ */
+ err = btintel_exit_mfg(hdev, true, true);
+ if (err)
+ return err;
+
+ /* Need build number for downloaded fw patches in
+ * every power-on boot
+ */
+ err = btintel_read_version(hdev, &new_ver);
+ if (err)
+ return err;
+
+ bt_dev_info(hdev, "Intel BT fw patch 0x%02x completed & activated",
+ new_ver.fw_patch_num);
+
+ goto complete;
+
+exit_mfg_disable:
+ /* Disable the manufacturer mode without reset */
+ err = btintel_exit_mfg(hdev, false, false);
+ if (err)
+ return err;
+
+ bt_dev_info(hdev, "Intel firmware patch completed");
+
+ goto complete;
+
+exit_mfg_deactivate:
+ release_firmware(fw);
+
+ /* Patching failed. Disable the manufacturer mode with reset and
+ * deactivate the downloaded firmware patches.
+ */
+ err = btintel_exit_mfg(hdev, true, false);
+ if (err)
+ return err;
+
+ bt_dev_info(hdev, "Intel firmware patch completed and deactivated");
+
+complete:
+ /* Set the event mask for Intel specific vendor events. This enables
+ * a few extra events that are useful during general operation.
+ */
+ btintel_set_event_mask_mfg(hdev, false);
+
+ btintel_check_bdaddr(hdev);
+
+ return 0;
+}
+
int btintel_setup_combined(struct hci_dev *hdev)
{
const u8 param[1] = { 0xFF };
@@ -1417,7 +1702,7 @@ int btintel_setup_combined(struct hci_dev *hdev)
case 0x07: /* WP */
case 0x08: /* StP */
/* Legacy ROM product */
- /* TODO: call setup routine for legacy rom product */
+ err = btintel_legacy_rom_setup(hdev, &ver);
break;
case 0x0b: /* SfP */
case 0x0c: /* WsP */
diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
index 1876a960b3dc..42f7176a6c70 100644
--- a/drivers/bluetooth/btusb.c
+++ b/drivers/bluetooth/btusb.c
@@ -61,6 +61,7 @@ static struct usb_driver btusb_driver;
#define BTUSB_VALID_LE_STATES 0x800000
#define BTUSB_QCA_WCN6855 0x1000000
#define BTUSB_INTEL_NEWGEN 0x2000000
+#define BTUSB_INTEL_COMBINED 0x4000000
static const struct usb_device_id btusb_table[] = {
/* Generic Bluetooth USB device */
@@ -375,11 +376,11 @@ static const struct usb_device_id blacklist_table[] = {
BTUSB_WIDEBAND_SPEECH |
BTUSB_VALID_LE_STATES },
{ USB_DEVICE(0x8087, 0x07da), .driver_info = BTUSB_CSR },
- { USB_DEVICE(0x8087, 0x07dc), .driver_info = BTUSB_INTEL },
- { USB_DEVICE(0x8087, 0x0a2a), .driver_info = BTUSB_INTEL },
+ { USB_DEVICE(0x8087, 0x07dc), .driver_info = BTUSB_INTEL_COMBINED },
+ { USB_DEVICE(0x8087, 0x0a2a), .driver_info = BTUSB_INTEL_COMBINED },
{ USB_DEVICE(0x8087, 0x0a2b), .driver_info = BTUSB_INTEL_NEW |
BTUSB_WIDEBAND_SPEECH },
- { USB_DEVICE(0x8087, 0x0aa7), .driver_info = BTUSB_INTEL |
+ { USB_DEVICE(0x8087, 0x0aa7), .driver_info = BTUSB_INTEL_COMBINED |
BTUSB_WIDEBAND_SPEECH },
{ USB_DEVICE(0x8087, 0x0aaa), .driver_info = BTUSB_INTEL_NEW |
BTUSB_WIDEBAND_SPEECH |
@@ -1962,319 +1963,6 @@ static int btusb_setup_csr(struct hci_dev *hdev)
return 0;
}
-static const struct firmware *btusb_setup_intel_get_fw(struct hci_dev *hdev,
- struct intel_version *ver)
-{
- const struct firmware *fw;
- char fwname[64];
- int ret;
-
- snprintf(fwname, sizeof(fwname),
- "intel/ibt-hw-%x.%x.%x-fw-%x.%x.%x.%x.%x.bseq",
- ver->hw_platform, ver->hw_variant, ver->hw_revision,
- ver->fw_variant, ver->fw_revision, ver->fw_build_num,
- ver->fw_build_ww, ver->fw_build_yy);
-
- ret = request_firmware(&fw, fwname, &hdev->dev);
- if (ret < 0) {
- if (ret == -EINVAL) {
- bt_dev_err(hdev, "Intel firmware file request failed (%d)",
- ret);
- return NULL;
- }
-
- bt_dev_err(hdev, "failed to open Intel firmware file: %s (%d)",
- fwname, ret);
-
- /* If the correct firmware patch file is not found, use the
- * default firmware patch file instead
- */
- snprintf(fwname, sizeof(fwname), "intel/ibt-hw-%x.%x.bseq",
- ver->hw_platform, ver->hw_variant);
- if (request_firmware(&fw, fwname, &hdev->dev) < 0) {
- bt_dev_err(hdev, "failed to open default fw file: %s",
- fwname);
- return NULL;
- }
- }
-
- bt_dev_info(hdev, "Intel Bluetooth firmware file: %s", fwname);
-
- return fw;
-}
-
-static int btusb_setup_intel_patching(struct hci_dev *hdev,
- const struct firmware *fw,
- const u8 **fw_ptr, int *disable_patch)
-{
- struct sk_buff *skb;
- struct hci_command_hdr *cmd;
- const u8 *cmd_param;
- struct hci_event_hdr *evt = NULL;
- const u8 *evt_param = NULL;
- int remain = fw->size - (*fw_ptr - fw->data);
-
- /* The first byte indicates the types of the patch command or event.
- * 0x01 means HCI command and 0x02 is HCI event. If the first bytes
- * in the current firmware buffer doesn't start with 0x01 or
- * the size of remain buffer is smaller than HCI command header,
- * the firmware file is corrupted and it should stop the patching
- * process.
- */
- if (remain > HCI_COMMAND_HDR_SIZE && *fw_ptr[0] != 0x01) {
- bt_dev_err(hdev, "Intel fw corrupted: invalid cmd read");
- return -EINVAL;
- }
- (*fw_ptr)++;
- remain--;
-
- cmd = (struct hci_command_hdr *)(*fw_ptr);
- *fw_ptr += sizeof(*cmd);
- remain -= sizeof(*cmd);
-
- /* Ensure that the remain firmware data is long enough than the length
- * of command parameter. If not, the firmware file is corrupted.
- */
- if (remain < cmd->plen) {
- bt_dev_err(hdev, "Intel fw corrupted: invalid cmd len");
- return -EFAULT;
- }
-
- /* If there is a command that loads a patch in the firmware
- * file, then enable the patch upon success, otherwise just
- * disable the manufacturer mode, for example patch activation
- * is not required when the default firmware patch file is used
- * because there are no patch data to load.
- */
- if (*disable_patch && le16_to_cpu(cmd->opcode) == 0xfc8e)
- *disable_patch = 0;
-
- cmd_param = *fw_ptr;
- *fw_ptr += cmd->plen;
- remain -= cmd->plen;
-
- /* This reads the expected events when the above command is sent to the
- * device. Some vendor commands expects more than one events, for
- * example command status event followed by vendor specific event.
- * For this case, it only keeps the last expected event. so the command
- * can be sent with __hci_cmd_sync_ev() which returns the sk_buff of
- * last expected event.
- */
- while (remain > HCI_EVENT_HDR_SIZE && *fw_ptr[0] == 0x02) {
- (*fw_ptr)++;
- remain--;
-
- evt = (struct hci_event_hdr *)(*fw_ptr);
- *fw_ptr += sizeof(*evt);
- remain -= sizeof(*evt);
-
- if (remain < evt->plen) {
- bt_dev_err(hdev, "Intel fw corrupted: invalid evt len");
- return -EFAULT;
- }
-
- evt_param = *fw_ptr;
- *fw_ptr += evt->plen;
- remain -= evt->plen;
- }
-
- /* Every HCI commands in the firmware file has its correspond event.
- * If event is not found or remain is smaller than zero, the firmware
- * file is corrupted.
- */
- if (!evt || !evt_param || remain < 0) {
- bt_dev_err(hdev, "Intel fw corrupted: invalid evt read");
- return -EFAULT;
- }
-
- skb = __hci_cmd_sync_ev(hdev, le16_to_cpu(cmd->opcode), cmd->plen,
- cmd_param, evt->evt, HCI_INIT_TIMEOUT);
- if (IS_ERR(skb)) {
- bt_dev_err(hdev, "sending Intel patch command (0x%4.4x) failed (%ld)",
- cmd->opcode, PTR_ERR(skb));
- return PTR_ERR(skb);
- }
-
- /* It ensures that the returned event matches the event data read from
- * the firmware file. At fist, it checks the length and then
- * the contents of the event.
- */
- if (skb->len != evt->plen) {
- bt_dev_err(hdev, "mismatch event length (opcode 0x%4.4x)",
- le16_to_cpu(cmd->opcode));
- kfree_skb(skb);
- return -EFAULT;
- }
-
- if (memcmp(skb->data, evt_param, evt->plen)) {
- bt_dev_err(hdev, "mismatch event parameter (opcode 0x%4.4x)",
- le16_to_cpu(cmd->opcode));
- kfree_skb(skb);
- return -EFAULT;
- }
- kfree_skb(skb);
-
- return 0;
-}
-
-static int btusb_setup_intel(struct hci_dev *hdev)
-{
- struct sk_buff *skb;
- const struct firmware *fw;
- const u8 *fw_ptr;
- int disable_patch, err;
- struct intel_version ver;
-
- BT_DBG("%s", hdev->name);
-
- /* The controller has a bug with the first HCI command sent to it
- * returning number of completed commands as zero. This would stall the
- * command processing in the Bluetooth core.
- *
- * As a workaround, send HCI Reset command first which will reset the
- * number of completed commands and allow normal command processing
- * from now on.
- */
- skb = __hci_cmd_sync(hdev, HCI_OP_RESET, 0, NULL, HCI_INIT_TIMEOUT);
- if (IS_ERR(skb)) {
- bt_dev_err(hdev, "sending initial HCI reset command failed (%ld)",
- PTR_ERR(skb));
- return PTR_ERR(skb);
- }
- kfree_skb(skb);
-
- /* Read Intel specific controller version first to allow selection of
- * which firmware file to load.
- *
- * The returned information are hardware variant and revision plus
- * firmware variant, revision and build number.
- */
- err = btintel_read_version(hdev, &ver);
- if (err)
- return err;
-
- bt_dev_info(hdev, "read Intel version: %02x%02x%02x%02x%02x%02x%02x%02x%02x",
- ver.hw_platform, ver.hw_variant, ver.hw_revision,
- ver.fw_variant, ver.fw_revision, ver.fw_build_num,
- ver.fw_build_ww, ver.fw_build_yy, ver.fw_patch_num);
-
- /* fw_patch_num indicates the version of patch the device currently
- * have. If there is no patch data in the device, it is always 0x00.
- * So, if it is other than 0x00, no need to patch the device again.
- */
- if (ver.fw_patch_num) {
- bt_dev_info(hdev, "Intel device is already patched. "
- "patch num: %02x", ver.fw_patch_num);
- goto complete;
- }
-
- /* Opens the firmware patch file based on the firmware version read
- * from the controller. If it fails to open the matching firmware
- * patch file, it tries to open the default firmware patch file.
- * If no patch file is found, allow the device to operate without
- * a patch.
- */
- fw = btusb_setup_intel_get_fw(hdev, &ver);
- if (!fw)
- goto complete;
- fw_ptr = fw->data;
-
- /* Enable the manufacturer mode of the controller.
- * Only while this mode is enabled, the driver can download the
- * firmware patch data and configuration parameters.
- */
- err = btintel_enter_mfg(hdev);
- if (err) {
- release_firmware(fw);
- return err;
- }
-
- disable_patch = 1;
-
- /* The firmware data file consists of list of Intel specific HCI
- * commands and its expected events. The first byte indicates the
- * type of the message, either HCI command or HCI event.
- *
- * It reads the command and its expected event from the firmware file,
- * and send to the controller. Once __hci_cmd_sync_ev() returns,
- * the returned event is compared with the event read from the firmware
- * file and it will continue until all the messages are downloaded to
- * the controller.
- *
- * Once the firmware patching is completed successfully,
- * the manufacturer mode is disabled with reset and activating the
- * downloaded patch.
- *
- * If the firmware patching fails, the manufacturer mode is
- * disabled with reset and deactivating the patch.
- *
- * If the default patch file is used, no reset is done when disabling
- * the manufacturer.
- */
- while (fw->size > fw_ptr - fw->data) {
- int ret;
-
- ret = btusb_setup_intel_patching(hdev, fw, &fw_ptr,
- &disable_patch);
- if (ret < 0)
- goto exit_mfg_deactivate;
- }
-
- release_firmware(fw);
-
- if (disable_patch)
- goto exit_mfg_disable;
-
- /* Patching completed successfully and disable the manufacturer mode
- * with reset and activate the downloaded firmware patches.
- */
- err = btintel_exit_mfg(hdev, true, true);
- if (err)
- return err;
-
- /* Need build number for downloaded fw patches in
- * every power-on boot
- */
- err = btintel_read_version(hdev, &ver);
- if (err)
- return err;
- bt_dev_info(hdev, "Intel BT fw patch 0x%02x completed & activated",
- ver.fw_patch_num);
-
- goto complete;
-
-exit_mfg_disable:
- /* Disable the manufacturer mode without reset */
- err = btintel_exit_mfg(hdev, false, false);
- if (err)
- return err;
-
- bt_dev_info(hdev, "Intel firmware patch completed");
-
- goto complete;
-
-exit_mfg_deactivate:
- release_firmware(fw);
-
- /* Patching failed. Disable the manufacturer mode with reset and
- * deactivate the downloaded firmware patches.
- */
- err = btintel_exit_mfg(hdev, true, false);
- if (err)
- return err;
-
- bt_dev_info(hdev, "Intel firmware patch completed and deactivated");
-
-complete:
- /* Set the event mask for Intel specific vendor events. This enables
- * a few extra events that are useful during general operation.
- */
- btintel_set_event_mask_mfg(hdev, false);
-
- btintel_check_bdaddr(hdev);
- return 0;
-}
-
static int inject_cmd_complete(struct hci_dev *hdev, __u16 opcode)
{
struct sk_buff *skb;
@@ -3042,41 +2730,6 @@ static int btusb_setup_intel_newgen(struct hci_dev *hdev)
return 0;
}
-static int btusb_shutdown_intel(struct hci_dev *hdev)
-{
- struct sk_buff *skb;
- long ret;
-
- /* In the shutdown sequence where Bluetooth is turned off followed
- * by WiFi being turned off, turning WiFi back on causes issue with
- * the RF calibration.
- *
- * To ensure that any RF activity has been stopped, issue HCI Reset
- * command to clear all ongoing activity including advertising,
- * scanning etc.
- */
- skb = __hci_cmd_sync(hdev, HCI_OP_RESET, 0, NULL, HCI_INIT_TIMEOUT);
- if (IS_ERR(skb)) {
- ret = PTR_ERR(skb);
- bt_dev_err(hdev, "HCI reset during shutdown failed");
- return ret;
- }
- kfree_skb(skb);
-
- /* Some platforms have an issue with BT LED when the interface is
- * down or BT radio is turned off, which takes 5 seconds to BT LED
- * goes off. This command turns off the BT LED immediately.
- */
- skb = __hci_cmd_sync(hdev, 0xfc3f, 0, NULL, HCI_INIT_TIMEOUT);
- if (IS_ERR(skb)) {
- ret = PTR_ERR(skb);
- bt_dev_err(hdev, "turning off Intel device LED failed");
- return ret;
- }
- kfree_skb(skb);
-
- return 0;
-}
static int btusb_shutdown_intel_new(struct hci_dev *hdev)
{
@@ -4649,10 +4302,11 @@ static int btusb_probe(struct usb_interface *intf,
data->diag = usb_ifnum_to_if(data->udev, ifnum_base + 2);
}
- if (id->driver_info & BTUSB_INTEL) {
+ /* Combined Intel Device setup to support multiple setup routine */
+ if (id->driver_info & BTUSB_INTEL_COMBINED) {
hdev->manufacturer = 2;
- hdev->setup = btusb_setup_intel;
- hdev->shutdown = btusb_shutdown_intel;
+ hdev->setup = btintel_setup_combined;
+ hdev->shutdown = btintel_shutdown_combined;
hdev->set_diag = btintel_set_diag_mfg;
hdev->set_bdaddr = btintel_set_bdaddr;
hdev->cmd_timeout = btusb_intel_cmd_timeout;
--
2.25.1
^ permalink raw reply related [flat|nested] 26+ messages in thread* Re: [PATCH v5 03/11] Bluetooth: btintel: Refactoring setup routine for legacy ROM sku
2021-07-29 18:35 ` [PATCH v5 03/11] Bluetooth: btintel: Refactoring setup routine for legacy ROM sku Tedd Ho-Jeong An
@ 2021-07-29 19:25 ` Marcel Holtmann
0 siblings, 0 replies; 26+ messages in thread
From: Marcel Holtmann @ 2021-07-29 19:25 UTC (permalink / raw)
To: Tedd Ho-Jeong An; +Cc: linux-bluetooth, Tedd Ho-Jeong An
Hi Tedd,
> This patch refactors the setup routines for legacy ROM product into
> combined setup, and move the related functions from btusb to btintel.
>
> Signed-off-by: Tedd Ho-Jeong An <tedd.an@intel.com>
> ---
> drivers/bluetooth/btintel.c | 287 +++++++++++++++++++++++++++-
> drivers/bluetooth/btusb.c | 362 +-----------------------------------
> 2 files changed, 294 insertions(+), 355 deletions(-)
>
> diff --git a/drivers/bluetooth/btintel.c b/drivers/bluetooth/btintel.c
> index a23304435814..cfc097694b53 100644
> --- a/drivers/bluetooth/btintel.c
> +++ b/drivers/bluetooth/btintel.c
> @@ -1372,6 +1372,291 @@ int btintel_set_debug_features(struct hci_dev *hdev,
> }
> EXPORT_SYMBOL_GPL(btintel_set_debug_features);
>
> +static const struct firmware *btintel_legacy_rom_get_fw(struct hci_dev *hdev,
> + struct intel_version *ver)
> +{
> + const struct firmware *fw;
> + char fwname[64];
> + int ret;
> +
> + snprintf(fwname, sizeof(fwname),
> + "intel/ibt-hw-%x.%x.%x-fw-%x.%x.%x.%x.%x.bseq",
> + ver->hw_platform, ver->hw_variant, ver->hw_revision,
> + ver->fw_variant, ver->fw_revision, ver->fw_build_num,
> + ver->fw_build_ww, ver->fw_build_yy);
> +
> + ret = request_firmware(&fw, fwname, &hdev->dev);
> + if (ret < 0) {
> + if (ret == -EINVAL) {
> + bt_dev_err(hdev, "Intel firmware file request failed (%d)",
> + ret);
> + return NULL;
> + }
> +
> + bt_dev_err(hdev, "failed to open Intel firmware file: %s (%d)",
> + fwname, ret);
> +
> + /* If the correct firmware patch file is not found, use the
> + * default firmware patch file instead
> + */
> + snprintf(fwname, sizeof(fwname), "intel/ibt-hw-%x.%x.bseq",
> + ver->hw_platform, ver->hw_variant);
> + if (request_firmware(&fw, fwname, &hdev->dev) < 0) {
> + bt_dev_err(hdev, "failed to open default fw file: %s",
> + fwname);
> + return NULL;
> + }
> + }
> +
> + bt_dev_info(hdev, "Intel Bluetooth firmware file: %s", fwname);
> +
> + return fw;
> +}
> +
> +static int btintel_legacy_rom_patching(struct hci_dev *hdev,
> + const struct firmware *fw,
> + const u8 **fw_ptr, int *disable_patch)
> +{
> + struct sk_buff *skb;
> + struct hci_command_hdr *cmd;
> + const u8 *cmd_param;
> + struct hci_event_hdr *evt = NULL;
> + const u8 *evt_param = NULL;
> + int remain = fw->size - (*fw_ptr - fw->data);
> +
> + /* The first byte indicates the types of the patch command or event.
> + * 0x01 means HCI command and 0x02 is HCI event. If the first bytes
> + * in the current firmware buffer doesn't start with 0x01 or
> + * the size of remain buffer is smaller than HCI command header,
> + * the firmware file is corrupted and it should stop the patching
> + * process.
> + */
> + if (remain > HCI_COMMAND_HDR_SIZE && *fw_ptr[0] != 0x01) {
> + bt_dev_err(hdev, "Intel fw corrupted: invalid cmd read");
> + return -EINVAL;
> + }
> + (*fw_ptr)++;
> + remain--;
> +
> + cmd = (struct hci_command_hdr *)(*fw_ptr);
> + *fw_ptr += sizeof(*cmd);
> + remain -= sizeof(*cmd);
> +
> + /* Ensure that the remain firmware data is long enough than the length
> + * of command parameter. If not, the firmware file is corrupted.
> + */
> + if (remain < cmd->plen) {
> + bt_dev_err(hdev, "Intel fw corrupted: invalid cmd len");
> + return -EFAULT;
> + }
> +
> + /* If there is a command that loads a patch in the firmware
> + * file, then enable the patch upon success, otherwise just
> + * disable the manufacturer mode, for example patch activation
> + * is not required when the default firmware patch file is used
> + * because there are no patch data to load.
> + */
> + if (*disable_patch && le16_to_cpu(cmd->opcode) == 0xfc8e)
> + *disable_patch = 0;
> +
> + cmd_param = *fw_ptr;
> + *fw_ptr += cmd->plen;
> + remain -= cmd->plen;
> +
> + /* This reads the expected events when the above command is sent to the
> + * device. Some vendor commands expects more than one events, for
> + * example command status event followed by vendor specific event.
> + * For this case, it only keeps the last expected event. so the command
> + * can be sent with __hci_cmd_sync_ev() which returns the sk_buff of
> + * last expected event.
> + */
> + while (remain > HCI_EVENT_HDR_SIZE && *fw_ptr[0] == 0x02) {
> + (*fw_ptr)++;
> + remain--;
> +
> + evt = (struct hci_event_hdr *)(*fw_ptr);
> + *fw_ptr += sizeof(*evt);
> + remain -= sizeof(*evt);
> +
> + if (remain < evt->plen) {
> + bt_dev_err(hdev, "Intel fw corrupted: invalid evt len");
> + return -EFAULT;
> + }
> +
> + evt_param = *fw_ptr;
> + *fw_ptr += evt->plen;
> + remain -= evt->plen;
> + }
> +
> + /* Every HCI commands in the firmware file has its correspond event.
> + * If event is not found or remain is smaller than zero, the firmware
> + * file is corrupted.
> + */
> + if (!evt || !evt_param || remain < 0) {
> + bt_dev_err(hdev, "Intel fw corrupted: invalid evt read");
> + return -EFAULT;
> + }
> +
> + skb = __hci_cmd_sync_ev(hdev, le16_to_cpu(cmd->opcode), cmd->plen,
> + cmd_param, evt->evt, HCI_INIT_TIMEOUT);
> + if (IS_ERR(skb)) {
> + bt_dev_err(hdev, "sending Intel patch command (0x%4.4x) failed (%ld)",
> + cmd->opcode, PTR_ERR(skb));
> + return PTR_ERR(skb);
> + }
> +
> + /* It ensures that the returned event matches the event data read from
> + * the firmware file. At fist, it checks the length and then
> + * the contents of the event.
> + */
> + if (skb->len != evt->plen) {
> + bt_dev_err(hdev, "mismatch event length (opcode 0x%4.4x)",
> + le16_to_cpu(cmd->opcode));
> + kfree_skb(skb);
> + return -EFAULT;
> + }
> +
> + if (memcmp(skb->data, evt_param, evt->plen)) {
> + bt_dev_err(hdev, "mismatch event parameter (opcode 0x%4.4x)",
> + le16_to_cpu(cmd->opcode));
> + kfree_skb(skb);
> + return -EFAULT;
> + }
> + kfree_skb(skb);
> +
> + return 0;
> +}
> +
> +static int btintel_legacy_rom_setup(struct hci_dev *hdev,
> + struct intel_version *ver)
> +{
> + const struct firmware *fw;
> + const u8 *fw_ptr;
> + int disable_patch, err;
> + struct intel_version new_ver;
> +
> + BT_DBG("%s", hdev->name);
> +
> + /* fw_patch_num indicates the version of patch the device currently
> + * have. If there is no patch data in the device, it is always 0x00.
> + * So, if it is other than 0x00, no need to patch the device again.
> + */
> + if (ver->fw_patch_num) {
> + bt_dev_info(hdev,
> + "Intel device is already patched. patch num: %02x",
> + ver->fw_patch_num);
> + goto complete;
> + }
> +
> + /* Opens the firmware patch file based on the firmware version read
> + * from the controller. If it fails to open the matching firmware
> + * patch file, it tries to open the default firmware patch file.
> + * If no patch file is found, allow the device to operate without
> + * a patch.
> + */
> + fw = btintel_legacy_rom_get_fw(hdev, ver);
> + if (!fw)
> + goto complete;
> + fw_ptr = fw->data;
> +
> + /* Enable the manufacturer mode of the controller.
> + * Only while this mode is enabled, the driver can download the
> + * firmware patch data and configuration parameters.
> + */
> + err = btintel_enter_mfg(hdev);
> + if (err) {
> + release_firmware(fw);
> + return err;
> + }
> +
> + disable_patch = 1;
> +
> + /* The firmware data file consists of list of Intel specific HCI
> + * commands and its expected events. The first byte indicates the
> + * type of the message, either HCI command or HCI event.
> + *
> + * It reads the command and its expected event from the firmware file,
> + * and send to the controller. Once __hci_cmd_sync_ev() returns,
> + * the returned event is compared with the event read from the firmware
> + * file and it will continue until all the messages are downloaded to
> + * the controller.
> + *
> + * Once the firmware patching is completed successfully,
> + * the manufacturer mode is disabled with reset and activating the
> + * downloaded patch.
> + *
> + * If the firmware patching fails, the manufacturer mode is
> + * disabled with reset and deactivating the patch.
> + *
> + * If the default patch file is used, no reset is done when disabling
> + * the manufacturer.
> + */
> + while (fw->size > fw_ptr - fw->data) {
> + int ret;
> +
> + ret = btintel_legacy_rom_patching(hdev, fw, &fw_ptr,
> + &disable_patch);
> + if (ret < 0)
> + goto exit_mfg_deactivate;
> + }
> +
> + release_firmware(fw);
> +
> + if (disable_patch)
> + goto exit_mfg_disable;
> +
> + /* Patching completed successfully and disable the manufacturer mode
> + * with reset and activate the downloaded firmware patches.
> + */
> + err = btintel_exit_mfg(hdev, true, true);
> + if (err)
> + return err;
> +
> + /* Need build number for downloaded fw patches in
> + * every power-on boot
> + */
> + err = btintel_read_version(hdev, &new_ver);
> + if (err)
> + return err;
> +
> + bt_dev_info(hdev, "Intel BT fw patch 0x%02x completed & activated",
> + new_ver.fw_patch_num);
> +
> + goto complete;
> +
> +exit_mfg_disable:
> + /* Disable the manufacturer mode without reset */
> + err = btintel_exit_mfg(hdev, false, false);
> + if (err)
> + return err;
> +
> + bt_dev_info(hdev, "Intel firmware patch completed");
> +
> + goto complete;
> +
> +exit_mfg_deactivate:
> + release_firmware(fw);
> +
> + /* Patching failed. Disable the manufacturer mode with reset and
> + * deactivate the downloaded firmware patches.
> + */
> + err = btintel_exit_mfg(hdev, true, false);
> + if (err)
> + return err;
> +
> + bt_dev_info(hdev, "Intel firmware patch completed and deactivated");
> +
> +complete:
> + /* Set the event mask for Intel specific vendor events. This enables
> + * a few extra events that are useful during general operation.
> + */
> + btintel_set_event_mask_mfg(hdev, false);
> +
> + btintel_check_bdaddr(hdev);
> +
> + return 0;
> +}
> +
> int btintel_setup_combined(struct hci_dev *hdev)
> {
> const u8 param[1] = { 0xFF };
> @@ -1417,7 +1702,7 @@ int btintel_setup_combined(struct hci_dev *hdev)
> case 0x07: /* WP */
> case 0x08: /* StP */
> /* Legacy ROM product */
> - /* TODO: call setup routine for legacy rom product */
> + err = btintel_legacy_rom_setup(hdev, &ver);
> break;
> case 0x0b: /* SfP */
> case 0x0c: /* WsP */
> diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
> index 1876a960b3dc..42f7176a6c70 100644
> --- a/drivers/bluetooth/btusb.c
> +++ b/drivers/bluetooth/btusb.c
> @@ -61,6 +61,7 @@ static struct usb_driver btusb_driver;
> #define BTUSB_VALID_LE_STATES 0x800000
> #define BTUSB_QCA_WCN6855 0x1000000
> #define BTUSB_INTEL_NEWGEN 0x2000000
> +#define BTUSB_INTEL_COMBINED 0x4000000
just rename BTUSB_INTEL into BTUSB_INTEL_COMBINED. Since the BTUSB_INTEL is now an orphan.
>
> static const struct usb_device_id btusb_table[] = {
> /* Generic Bluetooth USB device */
> @@ -375,11 +376,11 @@ static const struct usb_device_id blacklist_table[] = {
> BTUSB_WIDEBAND_SPEECH |
> BTUSB_VALID_LE_STATES },
> { USB_DEVICE(0x8087, 0x07da), .driver_info = BTUSB_CSR },
> - { USB_DEVICE(0x8087, 0x07dc), .driver_info = BTUSB_INTEL },
> - { USB_DEVICE(0x8087, 0x0a2a), .driver_info = BTUSB_INTEL },
> + { USB_DEVICE(0x8087, 0x07dc), .driver_info = BTUSB_INTEL_COMBINED },
> + { USB_DEVICE(0x8087, 0x0a2a), .driver_info = BTUSB_INTEL_COMBINED },
> { USB_DEVICE(0x8087, 0x0a2b), .driver_info = BTUSB_INTEL_NEW |
> BTUSB_WIDEBAND_SPEECH },
> - { USB_DEVICE(0x8087, 0x0aa7), .driver_info = BTUSB_INTEL |
> + { USB_DEVICE(0x8087, 0x0aa7), .driver_info = BTUSB_INTEL_COMBINED |
> BTUSB_WIDEBAND_SPEECH },
> { USB_DEVICE(0x8087, 0x0aaa), .driver_info = BTUSB_INTEL_NEW |
> BTUSB_WIDEBAND_SPEECH |
> @@ -1962,319 +1963,6 @@ static int btusb_setup_csr(struct hci_dev *hdev)
> return 0;
> }
Regards
Marcel
^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH v5 04/11] Bluetooth: btintel: Add btintel data struct
2021-07-29 18:35 [PATCH v5 00/11] Bluetooth: btintel: Refactoring setup routines Tedd Ho-Jeong An
` (2 preceding siblings ...)
2021-07-29 18:35 ` [PATCH v5 03/11] Bluetooth: btintel: Refactoring setup routine for legacy ROM sku Tedd Ho-Jeong An
@ 2021-07-29 18:35 ` Tedd Ho-Jeong An
2021-07-29 19:31 ` Marcel Holtmann
2021-07-29 18:35 ` [PATCH v5 05/11] Bluetooth: btintel: Fix the first HCI command not work with ROM device Tedd Ho-Jeong An
` (6 subsequent siblings)
10 siblings, 1 reply; 26+ messages in thread
From: Tedd Ho-Jeong An @ 2021-07-29 18:35 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Tedd Ho-Jeong An
From: Tedd Ho-Jeong An <tedd.an@intel.com>
This patch adds a data structure for btintel for btintel object, and the
definition of bootloder states.
Signed-off-by: Tedd Ho-Jeong An <tedd.an@intel.com>
---
drivers/bluetooth/btintel.c | 8 ++++++++
drivers/bluetooth/btintel.h | 15 +++++++++++++++
drivers/bluetooth/btusb.c | 6 ++++--
3 files changed, 27 insertions(+), 2 deletions(-)
diff --git a/drivers/bluetooth/btintel.c b/drivers/bluetooth/btintel.c
index cfc097694b53..bf0ad05b80fe 100644
--- a/drivers/bluetooth/btintel.c
+++ b/drivers/bluetooth/btintel.c
@@ -1753,6 +1753,14 @@ int btintel_shutdown_combined(struct hci_dev *hdev)
}
EXPORT_SYMBOL_GPL(btintel_shutdown_combined);
+void btintel_set_flags(struct hci_dev *hdev, unsigned int flag)
+{
+ struct btintel_data *intel = hci_get_priv(hdev);
+
+ set_bit(flag, &intel->flags);
+}
+EXPORT_SYMBOL_GPL(btintel_set_flags);
+
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 68ffa84fa87a..df7aa30142b4 100644
--- a/drivers/bluetooth/btintel.h
+++ b/drivers/bluetooth/btintel.h
@@ -138,6 +138,16 @@ struct intel_debug_features {
#define INTEL_CNVX_TOP_STEP(cnvx_top) (((cnvx_top) & 0x0f000000) >> 24)
#define INTEL_CNVX_TOP_PACK_SWAB(t, s) __swab16(((__u16)(((t) << 4) | (s))))
+#define INTEL_BOOTLOADER 0
+#define INTEL_DOWNLOADING 1
+#define INTEL_FIRMWARE_LOADED 2
+#define INTEL_FIRMWARE_FAILED 3
+#define INTEL_BOOTING 4
+
+struct btintel_data {
+ unsigned long flags;
+};
+
#if IS_ENABLED(CONFIG_BT_INTEL)
int btintel_check_bdaddr(struct hci_dev *hdev);
@@ -167,6 +177,7 @@ int btintel_download_firmware(struct hci_dev *dev, struct intel_version *ver,
const struct firmware *fw, u32 *boot_param);
int btintel_setup_combined(struct hci_dev *hdev);
int btintel_shutdown_combined(struct hci_dev *hdev);
+void btintel_set_flags(struct hci_dev *hdev, unsigned int flag);
int btintel_download_firmware_newgen(struct hci_dev *hdev,
struct intel_version_tlv *ver,
const struct firmware *fw,
@@ -295,6 +306,10 @@ static inline int btintel_shutdown_combined(struct hci_dev *hdev)
return -EOPNOTSUPP;
}
+static inline void btintel_set_flags(struct hci_dev *hdev, unsigned int flag)
+{
+}
+
static inline int btintel_download_firmware_newgen(struct hci_dev *hdev,
const struct firmware *fw,
u32 *boot_param,
diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
index 42f7176a6c70..8c54ab03ee63 100644
--- a/drivers/bluetooth/btusb.c
+++ b/drivers/bluetooth/btusb.c
@@ -4133,7 +4133,7 @@ static int btusb_probe(struct usb_interface *intf,
struct btusb_data *data;
struct hci_dev *hdev;
unsigned ifnum_base;
- int i, err;
+ int i, err, priv_size;
BT_DBG("intf %p id %p", intf, id);
@@ -4219,6 +4219,8 @@ static int btusb_probe(struct usb_interface *intf,
init_usb_anchor(&data->ctrl_anchor);
spin_lock_init(&data->rxlock);
+ priv_size = 0;
+
if (id->driver_info & BTUSB_INTEL_NEW) {
data->recv_event = btusb_recv_event_intel;
data->recv_bulk = btusb_recv_bulk_intel;
@@ -4228,7 +4230,7 @@ static int btusb_probe(struct usb_interface *intf,
data->recv_bulk = btusb_recv_bulk;
}
- hdev = hci_alloc_dev();
+ hdev = hci_alloc_dev_priv(priv_size);
if (!hdev)
return -ENOMEM;
--
2.25.1
^ permalink raw reply related [flat|nested] 26+ messages in thread* Re: [PATCH v5 04/11] Bluetooth: btintel: Add btintel data struct
2021-07-29 18:35 ` [PATCH v5 04/11] Bluetooth: btintel: Add btintel data struct Tedd Ho-Jeong An
@ 2021-07-29 19:31 ` Marcel Holtmann
0 siblings, 0 replies; 26+ messages in thread
From: Marcel Holtmann @ 2021-07-29 19:31 UTC (permalink / raw)
To: Tedd Ho-Jeong An; +Cc: linux-bluetooth, Tedd Ho-Jeong An
Hi Tedd,
> This patch adds a data structure for btintel for btintel object, and the
> definition of bootloder states.
>
> Signed-off-by: Tedd Ho-Jeong An <tedd.an@intel.com>
> ---
> drivers/bluetooth/btintel.c | 8 ++++++++
> drivers/bluetooth/btintel.h | 15 +++++++++++++++
> drivers/bluetooth/btusb.c | 6 ++++--
> 3 files changed, 27 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/bluetooth/btintel.c b/drivers/bluetooth/btintel.c
> index cfc097694b53..bf0ad05b80fe 100644
> --- a/drivers/bluetooth/btintel.c
> +++ b/drivers/bluetooth/btintel.c
> @@ -1753,6 +1753,14 @@ int btintel_shutdown_combined(struct hci_dev *hdev)
> }
> EXPORT_SYMBOL_GPL(btintel_shutdown_combined);
>
> +void btintel_set_flags(struct hci_dev *hdev, unsigned int flag)
> +{
> + struct btintel_data *intel = hci_get_priv(hdev);
> +
> + set_bit(flag, &intel->flags);
> +}
> +EXPORT_SYMBOL_GPL(btintel_set_flags);
> +
> 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 68ffa84fa87a..df7aa30142b4 100644
> --- a/drivers/bluetooth/btintel.h
> +++ b/drivers/bluetooth/btintel.h
> @@ -138,6 +138,16 @@ struct intel_debug_features {
> #define INTEL_CNVX_TOP_STEP(cnvx_top) (((cnvx_top) & 0x0f000000) >> 24)
> #define INTEL_CNVX_TOP_PACK_SWAB(t, s) __swab16(((__u16)(((t) << 4) | (s))))
>
> +#define INTEL_BOOTLOADER 0
> +#define INTEL_DOWNLOADING 1
> +#define INTEL_FIRMWARE_LOADED 2
> +#define INTEL_FIRMWARE_FAILED 3
> +#define INTEL_BOOTING 4
> +
> +struct btintel_data {
> + unsigned long flags;
So I don’t know how many flags we have for this to matter. But maybe DECLARE_BITMAP(flags, __NUM); might be a good idea.
We also do things like #define hci_dev_set_flag(hdev, nr) set_bit((nr), (hdev)->dev_flags) and it looks like using a define is a better approach than a function.
#define btintel_set_flag(hdev, nr) do { \
struct btintel_data *intel = hci_get_priv((hdev)); \
set_bit(nr, &intel->flags); \
} while (0)
> +};
> +
> #if IS_ENABLED(CONFIG_BT_INTEL)
>
> int btintel_check_bdaddr(struct hci_dev *hdev);
> @@ -167,6 +177,7 @@ int btintel_download_firmware(struct hci_dev *dev, struct intel_version *ver,
> const struct firmware *fw, u32 *boot_param);
> int btintel_setup_combined(struct hci_dev *hdev);
> int btintel_shutdown_combined(struct hci_dev *hdev);
> +void btintel_set_flags(struct hci_dev *hdev, unsigned int flag);
> int btintel_download_firmware_newgen(struct hci_dev *hdev,
> struct intel_version_tlv *ver,
> const struct firmware *fw,
> @@ -295,6 +306,10 @@ static inline int btintel_shutdown_combined(struct hci_dev *hdev)
> return -EOPNOTSUPP;
> }
>
> +static inline void btintel_set_flags(struct hci_dev *hdev, unsigned int flag)
> +{
> +}
> +
> static inline int btintel_download_firmware_newgen(struct hci_dev *hdev,
> const struct firmware *fw,
> u32 *boot_param,
> diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
> index 42f7176a6c70..8c54ab03ee63 100644
> --- a/drivers/bluetooth/btusb.c
> +++ b/drivers/bluetooth/btusb.c
> @@ -4133,7 +4133,7 @@ static int btusb_probe(struct usb_interface *intf,
> struct btusb_data *data;
> struct hci_dev *hdev;
> unsigned ifnum_base;
> - int i, err;
> + int i, err, priv_size;
>
> BT_DBG("intf %p id %p", intf, id);
>
> @@ -4219,6 +4219,8 @@ static int btusb_probe(struct usb_interface *intf,
> init_usb_anchor(&data->ctrl_anchor);
> spin_lock_init(&data->rxlock);
>
> + priv_size = 0;
> +
> if (id->driver_info & BTUSB_INTEL_NEW) {
> data->recv_event = btusb_recv_event_intel;
> data->recv_bulk = btusb_recv_bulk_intel;
> @@ -4228,7 +4230,7 @@ static int btusb_probe(struct usb_interface *intf,
> data->recv_bulk = btusb_recv_bulk;
> }
>
But your are not setting the size in case of BTUSB_INTEL_COMBINED.
> - hdev = hci_alloc_dev();
> + hdev = hci_alloc_dev_priv(priv_size);
> if (!hdev)
> return -ENOMEM;
Regards
Marcel
^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH v5 05/11] Bluetooth: btintel: Fix the first HCI command not work with ROM device
2021-07-29 18:35 [PATCH v5 00/11] Bluetooth: btintel: Refactoring setup routines Tedd Ho-Jeong An
` (3 preceding siblings ...)
2021-07-29 18:35 ` [PATCH v5 04/11] Bluetooth: btintel: Add btintel data struct Tedd Ho-Jeong An
@ 2021-07-29 18:35 ` Tedd Ho-Jeong An
2021-07-29 19:35 ` Marcel Holtmann
2021-07-29 18:35 ` [PATCH v5 06/11] Bluetooth: btintel: Fix the LED is not turning off immediately Tedd Ho-Jeong An
` (5 subsequent siblings)
10 siblings, 1 reply; 26+ messages in thread
From: Tedd Ho-Jeong An @ 2021-07-29 18:35 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Tedd Ho-Jeong An
From: Tedd Ho-Jeong An <tedd.an@intel.com>
The some legacy ROM controllers have a bug with the first HCI command
sent to it returning number of completed commands as zero, which would
stall the command processing in the Bluetooth core.
As a workaround, send HCI Rest command first which will reset the
controller to fix the issue.
Signed-off-by: Tedd Ho-Jeong An <tedd.an@intel.com>
---
drivers/bluetooth/btintel.c | 21 +++++++++++++++++++++
drivers/bluetooth/btintel.h | 1 +
drivers/bluetooth/btusb.c | 16 ++++++++++++++--
3 files changed, 36 insertions(+), 2 deletions(-)
diff --git a/drivers/bluetooth/btintel.c b/drivers/bluetooth/btintel.c
index bf0ad05b80fe..65ecf2ae9a10 100644
--- a/drivers/bluetooth/btintel.c
+++ b/drivers/bluetooth/btintel.c
@@ -1659,6 +1659,7 @@ static int btintel_legacy_rom_setup(struct hci_dev *hdev,
int btintel_setup_combined(struct hci_dev *hdev)
{
+ struct btintel_data *intel = hci_get_priv(hdev);
const u8 param[1] = { 0xFF };
struct intel_version ver;
struct intel_version_tlv ver_tlv;
@@ -1667,6 +1668,26 @@ int btintel_setup_combined(struct hci_dev *hdev)
BT_DBG("%s", hdev->name);
+ /* The some controllers have a bug with the first HCI command sent to it
+ * returning number of completed commands as zero. This would stall the
+ * command processing in the Bluetooth core.
+ *
+ * As a workaround, send HCI Reset command first which will reset the
+ * number of completed commands and allow normal command processing
+ * from now on.
+ */
+ if (test_bit(INTEL_BROKEN_READ_VERSION, &intel->flags)) {
+ skb = __hci_cmd_sync(hdev, HCI_OP_RESET, 0, NULL,
+ HCI_INIT_TIMEOUT);
+ if (IS_ERR(skb)) {
+ bt_dev_err(hdev,
+ "sending initial HCI reset failed (%ld)",
+ PTR_ERR(skb));
+ return PTR_ERR(skb);
+ }
+ kfree_skb(skb);
+ }
+
/* 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
diff --git a/drivers/bluetooth/btintel.h b/drivers/bluetooth/btintel.h
index df7aa30142b4..29b678364a79 100644
--- a/drivers/bluetooth/btintel.h
+++ b/drivers/bluetooth/btintel.h
@@ -143,6 +143,7 @@ struct intel_debug_features {
#define INTEL_FIRMWARE_LOADED 2
#define INTEL_FIRMWARE_FAILED 3
#define INTEL_BOOTING 4
+#define INTEL_BROKEN_READ_VERSION 5
struct btintel_data {
unsigned long flags;
diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
index 8c54ab03ee63..a64473c525eb 100644
--- a/drivers/bluetooth/btusb.c
+++ b/drivers/bluetooth/btusb.c
@@ -62,6 +62,7 @@ static struct usb_driver btusb_driver;
#define BTUSB_QCA_WCN6855 0x1000000
#define BTUSB_INTEL_NEWGEN 0x2000000
#define BTUSB_INTEL_COMBINED 0x4000000
+#define BTUSB_INTEL_BROKEN_READ_VERSION 0x8000000
static const struct usb_device_id btusb_table[] = {
/* Generic Bluetooth USB device */
@@ -376,11 +377,14 @@ static const struct usb_device_id blacklist_table[] = {
BTUSB_WIDEBAND_SPEECH |
BTUSB_VALID_LE_STATES },
{ USB_DEVICE(0x8087, 0x07da), .driver_info = BTUSB_CSR },
- { USB_DEVICE(0x8087, 0x07dc), .driver_info = BTUSB_INTEL_COMBINED },
- { USB_DEVICE(0x8087, 0x0a2a), .driver_info = BTUSB_INTEL_COMBINED },
+ { USB_DEVICE(0x8087, 0x07dc), .driver_info = BTUSB_INTEL_COMBINED |
+ BTUSB_INTEL_BROKEN_READ_VERSION },
+ { USB_DEVICE(0x8087, 0x0a2a), .driver_info = BTUSB_INTEL_COMBINED |
+ BTUSB_INTEL_BROKEN_READ_VERSION },
{ USB_DEVICE(0x8087, 0x0a2b), .driver_info = BTUSB_INTEL_NEW |
BTUSB_WIDEBAND_SPEECH },
{ USB_DEVICE(0x8087, 0x0aa7), .driver_info = BTUSB_INTEL_COMBINED |
+ BTUSB_INTEL_BROKEN_READ_VERSION |
BTUSB_WIDEBAND_SPEECH },
{ USB_DEVICE(0x8087, 0x0aaa), .driver_info = BTUSB_INTEL_NEW |
BTUSB_WIDEBAND_SPEECH |
@@ -4221,6 +4225,11 @@ static int btusb_probe(struct usb_interface *intf,
priv_size = 0;
+ if (id->driver_info & BTUSB_INTEL_COMBINED) {
+ /* Allocate extra space for Intel device */
+ priv_size += sizeof(struct btintel_data);
+ }
+
if (id->driver_info & BTUSB_INTEL_NEW) {
data->recv_event = btusb_recv_event_intel;
data->recv_bulk = btusb_recv_bulk_intel;
@@ -4315,6 +4324,9 @@ static int btusb_probe(struct usb_interface *intf,
set_bit(HCI_QUIRK_STRICT_DUPLICATE_FILTER, &hdev->quirks);
set_bit(HCI_QUIRK_SIMULTANEOUS_DISCOVERY, &hdev->quirks);
set_bit(HCI_QUIRK_NON_PERSISTENT_DIAG, &hdev->quirks);
+
+ if (id->driver_info & BTUSB_INTEL_BROKEN_READ_VERSION)
+ btintel_set_flags(hdev, INTEL_BROKEN_READ_VERSION);
}
if (id->driver_info & BTUSB_INTEL_NEW) {
--
2.25.1
^ permalink raw reply related [flat|nested] 26+ messages in thread* Re: [PATCH v5 05/11] Bluetooth: btintel: Fix the first HCI command not work with ROM device
2021-07-29 18:35 ` [PATCH v5 05/11] Bluetooth: btintel: Fix the first HCI command not work with ROM device Tedd Ho-Jeong An
@ 2021-07-29 19:35 ` Marcel Holtmann
2021-07-29 21:59 ` Tedd Ho-Jeong An
0 siblings, 1 reply; 26+ messages in thread
From: Marcel Holtmann @ 2021-07-29 19:35 UTC (permalink / raw)
To: Tedd Ho-Jeong An; +Cc: linux-bluetooth, Tedd Ho-Jeong An
Hi Tedd,
> The some legacy ROM controllers have a bug with the first HCI command
> sent to it returning number of completed commands as zero, which would
> stall the command processing in the Bluetooth core.
>
> As a workaround, send HCI Rest command first which will reset the
> controller to fix the issue.
>
> Signed-off-by: Tedd Ho-Jeong An <tedd.an@intel.com>
> ---
> drivers/bluetooth/btintel.c | 21 +++++++++++++++++++++
> drivers/bluetooth/btintel.h | 1 +
> drivers/bluetooth/btusb.c | 16 ++++++++++++++--
> 3 files changed, 36 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/bluetooth/btintel.c b/drivers/bluetooth/btintel.c
> index bf0ad05b80fe..65ecf2ae9a10 100644
> --- a/drivers/bluetooth/btintel.c
> +++ b/drivers/bluetooth/btintel.c
> @@ -1659,6 +1659,7 @@ static int btintel_legacy_rom_setup(struct hci_dev *hdev,
>
> int btintel_setup_combined(struct hci_dev *hdev)
> {
> + struct btintel_data *intel = hci_get_priv(hdev);
> const u8 param[1] = { 0xFF };
> struct intel_version ver;
> struct intel_version_tlv ver_tlv;
> @@ -1667,6 +1668,26 @@ int btintel_setup_combined(struct hci_dev *hdev)
>
> BT_DBG("%s", hdev->name);
>
> + /* The some controllers have a bug with the first HCI command sent to it
> + * returning number of completed commands as zero. This would stall the
> + * command processing in the Bluetooth core.
> + *
> + * As a workaround, send HCI Reset command first which will reset the
> + * number of completed commands and allow normal command processing
> + * from now on.
> + */
> + if (test_bit(INTEL_BROKEN_READ_VERSION, &intel->flags)) {
> + skb = __hci_cmd_sync(hdev, HCI_OP_RESET, 0, NULL,
> + HCI_INIT_TIMEOUT);
> + if (IS_ERR(skb)) {
> + bt_dev_err(hdev,
> + "sending initial HCI reset failed (%ld)",
> + PTR_ERR(skb));
> + return PTR_ERR(skb);
> + }
> + kfree_skb(skb);
> + }
> +
> /* 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
> diff --git a/drivers/bluetooth/btintel.h b/drivers/bluetooth/btintel.h
> index df7aa30142b4..29b678364a79 100644
> --- a/drivers/bluetooth/btintel.h
> +++ b/drivers/bluetooth/btintel.h
> @@ -143,6 +143,7 @@ struct intel_debug_features {
> #define INTEL_FIRMWARE_LOADED 2
> #define INTEL_FIRMWARE_FAILED 3
> #define INTEL_BOOTING 4
> +#define INTEL_BROKEN_READ_VERSION 5
>
> struct btintel_data {
> unsigned long flags;
> diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
> index 8c54ab03ee63..a64473c525eb 100644
> --- a/drivers/bluetooth/btusb.c
> +++ b/drivers/bluetooth/btusb.c
> @@ -62,6 +62,7 @@ static struct usb_driver btusb_driver;
> #define BTUSB_QCA_WCN6855 0x1000000
> #define BTUSB_INTEL_NEWGEN 0x2000000
> #define BTUSB_INTEL_COMBINED 0x4000000
> +#define BTUSB_INTEL_BROKEN_READ_VERSION 0x8000000
>
> static const struct usb_device_id btusb_table[] = {
> /* Generic Bluetooth USB device */
> @@ -376,11 +377,14 @@ static const struct usb_device_id blacklist_table[] = {
> BTUSB_WIDEBAND_SPEECH |
> BTUSB_VALID_LE_STATES },
> { USB_DEVICE(0x8087, 0x07da), .driver_info = BTUSB_CSR },
> - { USB_DEVICE(0x8087, 0x07dc), .driver_info = BTUSB_INTEL_COMBINED },
> - { USB_DEVICE(0x8087, 0x0a2a), .driver_info = BTUSB_INTEL_COMBINED },
> + { USB_DEVICE(0x8087, 0x07dc), .driver_info = BTUSB_INTEL_COMBINED |
> + BTUSB_INTEL_BROKEN_READ_VERSION },
> + { USB_DEVICE(0x8087, 0x0a2a), .driver_info = BTUSB_INTEL_COMBINED |
> + BTUSB_INTEL_BROKEN_READ_VERSION },
> { USB_DEVICE(0x8087, 0x0a2b), .driver_info = BTUSB_INTEL_NEW |
> BTUSB_WIDEBAND_SPEECH },
> { USB_DEVICE(0x8087, 0x0aa7), .driver_info = BTUSB_INTEL_COMBINED |
> + BTUSB_INTEL_BROKEN_READ_VERSION |
> BTUSB_WIDEBAND_SPEECH },
> { USB_DEVICE(0x8087, 0x0aaa), .driver_info = BTUSB_INTEL_NEW |
> BTUSB_WIDEBAND_SPEECH |
can you check that all 3 have this problem? Don’t we ever produced a ROM where this is fixed?
> @@ -4221,6 +4225,11 @@ static int btusb_probe(struct usb_interface *intf,
>
> priv_size = 0;
>
> + if (id->driver_info & BTUSB_INTEL_COMBINED) {
> + /* Allocate extra space for Intel device */
> + priv_size += sizeof(struct btintel_data);
> + }
> +
This needs to be in the previous patch.
Regards
Marcel
^ permalink raw reply [flat|nested] 26+ messages in thread* Re: [PATCH v5 05/11] Bluetooth: btintel: Fix the first HCI command not work with ROM device
2021-07-29 19:35 ` Marcel Holtmann
@ 2021-07-29 21:59 ` Tedd Ho-Jeong An
2021-07-30 6:40 ` Marcel Holtmann
0 siblings, 1 reply; 26+ messages in thread
From: Tedd Ho-Jeong An @ 2021-07-29 21:59 UTC (permalink / raw)
To: Marcel Holtmann; +Cc: linux-bluetooth
Hi Marcel,
On Thu, 2021-07-29 at 21:35 +0200, Marcel Holtmann wrote:
> Hi Tedd,
>
> > The some legacy ROM controllers have a bug with the first HCI command
> > sent to it returning number of completed commands as zero, which would
> > stall the command processing in the Bluetooth core.
> >
> > As a workaround, send HCI Rest command first which will reset the
> > controller to fix the issue.
> >
> > Signed-off-by: Tedd Ho-Jeong An <tedd.an@intel.com>
> > ---
> > drivers/bluetooth/btintel.c | 21 +++++++++++++++++++++
> > drivers/bluetooth/btintel.h | 1 +
> > drivers/bluetooth/btusb.c | 16 ++++++++++++++--
> > 3 files changed, 36 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/bluetooth/btintel.c b/drivers/bluetooth/btintel.c
> > index bf0ad05b80fe..65ecf2ae9a10 100644
> > --- a/drivers/bluetooth/btintel.c
> > +++ b/drivers/bluetooth/btintel.c
> > @@ -1659,6 +1659,7 @@ static int btintel_legacy_rom_setup(struct hci_dev *hdev,
> >
> > int btintel_setup_combined(struct hci_dev *hdev)
> > {
> > + struct btintel_data *intel = hci_get_priv(hdev);
> > const u8 param[1] = { 0xFF };
> > struct intel_version ver;
> > struct intel_version_tlv ver_tlv;
> > @@ -1667,6 +1668,26 @@ int btintel_setup_combined(struct hci_dev *hdev)
> >
> > BT_DBG("%s", hdev->name);
> >
> > + /* The some controllers have a bug with the first HCI command sent to it
> > + * returning number of completed commands as zero. This would stall the
> > + * command processing in the Bluetooth core.
> > + *
> > + * As a workaround, send HCI Reset command first which will reset the
> > + * number of completed commands and allow normal command processing
> > + * from now on.
> > + */
> > + if (test_bit(INTEL_BROKEN_READ_VERSION, &intel->flags)) {
> > + skb = __hci_cmd_sync(hdev, HCI_OP_RESET, 0, NULL,
> > + HCI_INIT_TIMEOUT);
> > + if (IS_ERR(skb)) {
> > + bt_dev_err(hdev,
> > + "sending initial HCI reset failed (%ld)",
> > + PTR_ERR(skb));
> > + return PTR_ERR(skb);
> > + }
> > + kfree_skb(skb);
> > + }
> > +
> > /* 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
> > diff --git a/drivers/bluetooth/btintel.h b/drivers/bluetooth/btintel.h
> > index df7aa30142b4..29b678364a79 100644
> > --- a/drivers/bluetooth/btintel.h
> > +++ b/drivers/bluetooth/btintel.h
> > @@ -143,6 +143,7 @@ struct intel_debug_features {
> > #define INTEL_FIRMWARE_LOADED 2
> > #define INTEL_FIRMWARE_FAILED 3
> > #define INTEL_BOOTING 4
> > +#define INTEL_BROKEN_READ_VERSION 5
> >
> > struct btintel_data {
> > unsigned long flags;
> > diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
> > index 8c54ab03ee63..a64473c525eb 100644
> > --- a/drivers/bluetooth/btusb.c
> > +++ b/drivers/bluetooth/btusb.c
> > @@ -62,6 +62,7 @@ static struct usb_driver btusb_driver;
> > #define BTUSB_QCA_WCN6855 0x1000000
> > #define BTUSB_INTEL_NEWGEN 0x2000000
> > #define BTUSB_INTEL_COMBINED 0x4000000
> > +#define BTUSB_INTEL_BROKEN_READ_VERSION 0x8000000
> >
> > static const struct usb_device_id btusb_table[] = {
> > /* Generic Bluetooth USB device */
> > @@ -376,11 +377,14 @@ static const struct usb_device_id blacklist_table[] = {
> > BTUSB_WIDEBAND_SPEECH |
> > BTUSB_VALID_LE_STATES },
> > { USB_DEVICE(0x8087, 0x07da), .driver_info = BTUSB_CSR },
> > - { USB_DEVICE(0x8087, 0x07dc), .driver_info = BTUSB_INTEL_COMBINED },
> > - { USB_DEVICE(0x8087, 0x0a2a), .driver_info = BTUSB_INTEL_COMBINED },
> > + { USB_DEVICE(0x8087, 0x07dc), .driver_info = BTUSB_INTEL_COMBINED |
> > + BTUSB_INTEL_BROKEN_READ_VERSION },
> > + { USB_DEVICE(0x8087, 0x0a2a), .driver_info = BTUSB_INTEL_COMBINED |
> > + BTUSB_INTEL_BROKEN_READ_VERSION },
> > { USB_DEVICE(0x8087, 0x0a2b), .driver_info = BTUSB_INTEL_NEW |
> > BTUSB_WIDEBAND_SPEECH },
> > { USB_DEVICE(0x8087, 0x0aa7), .driver_info = BTUSB_INTEL_COMBINED |
> > + BTUSB_INTEL_BROKEN_READ_VERSION |
> > BTUSB_WIDEBAND_SPEECH },
> > { USB_DEVICE(0x8087, 0x0aaa), .driver_info = BTUSB_INTEL_NEW |
> > BTUSB_WIDEBAND_SPEECH |
>
> can you check that all 3 have this problem? Don’t we ever produced a ROM where this is fixed?
It looks like the early version of ROM (WP2) causes the problem. StP and SdP don't have the problem.
I will update accordingly.
WP2 - I only had mini-PCIe form factor card and it is broken.
< HCI Command: Intel Read Version (0x3f|0x0005) plen 1 #1 [hci0] 9.212217
Requested Type:
All Supported Types(0xff)
> HCI Event: Command Complete (0x0e) plen 13 #2 [hci0] 9.213338
Intel Read Version (0x3f|0x0005) ncmd 0
Status: Success (0x00)
Hardware platform: 0x37
Hardware variant: 0x07
Hardware revision: 1.0
Firmware variant: 0x01
Firmware revision: 8.0
Firmware build: 2-3.2013
Firmware patch: 0
StP - OK.
< HCI Command: Intel Read Version (0x3f|0x0005) plen 1 #3 [hci0] 108.053455
Requested Type:
All Supported Types(0xff)
> HCI Event: Command Complete (0x0e) plen 13 #4 [hci0] 108.054034
Intel Read Version (0x3f|0x0005) ncmd 1
Status: Success (0x00)
Hardware platform: 0x37
Hardware variant: 0x08
Hardware revision: 1.0
Firmware variant: 0x01
Firmware revision: 1.0
Firmware build: 3-17.2014
Firmware patch: 0
SdP - OK.
< HCI Command: Intel Read Version (0x3f|0x0005) plen 1 #400 [hci0] 173.911992
Requested Type:
All Supported Types(0xff)
> HCI Event: Command Complete (0x0e) plen 13 #401 [hci0] 173.912576
Intel Read Version (0x3f|0x0005) ncmd 1
Status: Success (0x00)
Hardware platform: 0x37
Hardware variant: 0x08
Hardware revision: 1.0
Firmware variant: 0x22
Firmware revision: 5.0
Firmware build: 25-20.2015
Firmware patch: 0
>
> > @@ -4221,6 +4225,11 @@ static int btusb_probe(struct usb_interface *intf,
> >
> > priv_size = 0;
> >
> > + if (id->driver_info & BTUSB_INTEL_COMBINED) {
> > + /* Allocate extra space for Intel device */
> > + priv_size += sizeof(struct btintel_data);
> > + }
> > +
>
> This needs to be in the previous patch.
>
> Regards
>
> Marcel
>
^ permalink raw reply [flat|nested] 26+ messages in thread* Re: [PATCH v5 05/11] Bluetooth: btintel: Fix the first HCI command not work with ROM device
2021-07-29 21:59 ` Tedd Ho-Jeong An
@ 2021-07-30 6:40 ` Marcel Holtmann
0 siblings, 0 replies; 26+ messages in thread
From: Marcel Holtmann @ 2021-07-30 6:40 UTC (permalink / raw)
To: Tedd Ho-Jeong An; +Cc: linux-bluetooth
Hi Tedd,
>>> The some legacy ROM controllers have a bug with the first HCI command
>>> sent to it returning number of completed commands as zero, which would
>>> stall the command processing in the Bluetooth core.
>>>
>>> As a workaround, send HCI Rest command first which will reset the
>>> controller to fix the issue.
>>>
>>> Signed-off-by: Tedd Ho-Jeong An <tedd.an@intel.com>
>>> ---
>>> drivers/bluetooth/btintel.c | 21 +++++++++++++++++++++
>>> drivers/bluetooth/btintel.h | 1 +
>>> drivers/bluetooth/btusb.c | 16 ++++++++++++++--
>>> 3 files changed, 36 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/bluetooth/btintel.c b/drivers/bluetooth/btintel.c
>>> index bf0ad05b80fe..65ecf2ae9a10 100644
>>> --- a/drivers/bluetooth/btintel.c
>>> +++ b/drivers/bluetooth/btintel.c
>>> @@ -1659,6 +1659,7 @@ static int btintel_legacy_rom_setup(struct hci_dev *hdev,
>>>
>>> int btintel_setup_combined(struct hci_dev *hdev)
>>> {
>>> + struct btintel_data *intel = hci_get_priv(hdev);
>>> const u8 param[1] = { 0xFF };
>>> struct intel_version ver;
>>> struct intel_version_tlv ver_tlv;
>>> @@ -1667,6 +1668,26 @@ int btintel_setup_combined(struct hci_dev *hdev)
>>>
>>> BT_DBG("%s", hdev->name);
>>>
>>> + /* The some controllers have a bug with the first HCI command sent to it
>>> + * returning number of completed commands as zero. This would stall the
>>> + * command processing in the Bluetooth core.
>>> + *
>>> + * As a workaround, send HCI Reset command first which will reset the
>>> + * number of completed commands and allow normal command processing
>>> + * from now on.
>>> + */
>>> + if (test_bit(INTEL_BROKEN_READ_VERSION, &intel->flags)) {
>>> + skb = __hci_cmd_sync(hdev, HCI_OP_RESET, 0, NULL,
>>> + HCI_INIT_TIMEOUT);
>>> + if (IS_ERR(skb)) {
>>> + bt_dev_err(hdev,
>>> + "sending initial HCI reset failed (%ld)",
>>> + PTR_ERR(skb));
>>> + return PTR_ERR(skb);
>>> + }
>>> + kfree_skb(skb);
>>> + }
>>> +
>>> /* 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
>>> diff --git a/drivers/bluetooth/btintel.h b/drivers/bluetooth/btintel.h
>>> index df7aa30142b4..29b678364a79 100644
>>> --- a/drivers/bluetooth/btintel.h
>>> +++ b/drivers/bluetooth/btintel.h
>>> @@ -143,6 +143,7 @@ struct intel_debug_features {
>>> #define INTEL_FIRMWARE_LOADED 2
>>> #define INTEL_FIRMWARE_FAILED 3
>>> #define INTEL_BOOTING 4
>>> +#define INTEL_BROKEN_READ_VERSION 5
>>>
>>> struct btintel_data {
>>> unsigned long flags;
>>> diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
>>> index 8c54ab03ee63..a64473c525eb 100644
>>> --- a/drivers/bluetooth/btusb.c
>>> +++ b/drivers/bluetooth/btusb.c
>>> @@ -62,6 +62,7 @@ static struct usb_driver btusb_driver;
>>> #define BTUSB_QCA_WCN6855 0x1000000
>>> #define BTUSB_INTEL_NEWGEN 0x2000000
>>> #define BTUSB_INTEL_COMBINED 0x4000000
>>> +#define BTUSB_INTEL_BROKEN_READ_VERSION 0x8000000
>>>
>>> static const struct usb_device_id btusb_table[] = {
>>> /* Generic Bluetooth USB device */
>>> @@ -376,11 +377,14 @@ static const struct usb_device_id blacklist_table[] = {
>>> BTUSB_WIDEBAND_SPEECH |
>>> BTUSB_VALID_LE_STATES },
>>> { USB_DEVICE(0x8087, 0x07da), .driver_info = BTUSB_CSR },
>>> - { USB_DEVICE(0x8087, 0x07dc), .driver_info = BTUSB_INTEL_COMBINED },
>>> - { USB_DEVICE(0x8087, 0x0a2a), .driver_info = BTUSB_INTEL_COMBINED },
>>> + { USB_DEVICE(0x8087, 0x07dc), .driver_info = BTUSB_INTEL_COMBINED |
>>> + BTUSB_INTEL_BROKEN_READ_VERSION },
>>> + { USB_DEVICE(0x8087, 0x0a2a), .driver_info = BTUSB_INTEL_COMBINED |
>>> + BTUSB_INTEL_BROKEN_READ_VERSION },
>>> { USB_DEVICE(0x8087, 0x0a2b), .driver_info = BTUSB_INTEL_NEW |
>>> BTUSB_WIDEBAND_SPEECH },
>>> { USB_DEVICE(0x8087, 0x0aa7), .driver_info = BTUSB_INTEL_COMBINED |
>>> + BTUSB_INTEL_BROKEN_READ_VERSION |
>>> BTUSB_WIDEBAND_SPEECH },
>>> { USB_DEVICE(0x8087, 0x0aaa), .driver_info = BTUSB_INTEL_NEW |
>>> BTUSB_WIDEBAND_SPEECH |
>>
>> can you check that all 3 have this problem? Don’t we ever produced a ROM where this is fixed?
>
> It looks like the early version of ROM (WP2) causes the problem. StP and SdP don't have the problem.
> I will update accordingly.
then we should get away with having BTUSB_INTEL_COMBINED (which we can later rename back into BTUSB_INTEL) and one additional BTUSB_INTEL_BROKEN_INITIAL_NCMD. Everything else we can do internally in the Intel vendor specific handling.
>
> WP2 - I only had mini-PCIe form factor card and it is broken.
> < HCI Command: Intel Read Version (0x3f|0x0005) plen 1 #1 [hci0] 9.212217
> Requested Type:
> All Supported Types(0xff)
>> HCI Event: Command Complete (0x0e) plen 13 #2 [hci0] 9.213338
> Intel Read Version (0x3f|0x0005) ncmd 0
> Status: Success (0x00)
> Hardware platform: 0x37
> Hardware variant: 0x07
> Hardware revision: 1.0
> Firmware variant: 0x01
> Firmware revision: 8.0
> Firmware build: 2-3.2013
> Firmware patch: 0
>
> StP - OK.
> < HCI Command: Intel Read Version (0x3f|0x0005) plen 1 #3 [hci0] 108.053455
> Requested Type:
> All Supported Types(0xff)
>> HCI Event: Command Complete (0x0e) plen 13 #4 [hci0] 108.054034
> Intel Read Version (0x3f|0x0005) ncmd 1
> Status: Success (0x00)
> Hardware platform: 0x37
> Hardware variant: 0x08
> Hardware revision: 1.0
> Firmware variant: 0x01
> Firmware revision: 1.0
> Firmware build: 3-17.2014
> Firmware patch: 0
>
> SdP - OK.
> < HCI Command: Intel Read Version (0x3f|0x0005) plen 1 #400 [hci0] 173.911992
> Requested Type:
> All Supported Types(0xff)
>> HCI Event: Command Complete (0x0e) plen 13 #401 [hci0] 173.912576
> Intel Read Version (0x3f|0x0005) ncmd 1
> Status: Success (0x00)
> Hardware platform: 0x37
> Hardware variant: 0x08
> Hardware revision: 1.0
> Firmware variant: 0x22
> Firmware revision: 5.0
> Firmware build: 25-20.2015
> Firmware patch: 0
We should add a document in bluez/doc/intel-variants.txt that documents our own hardware and which variants and firmware combinations we have tested this with. It is good for revision history and if users complain or have slight variations of the SKUs.
Regards
Marcel
^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH v5 06/11] Bluetooth: btintel: Fix the LED is not turning off immediately
2021-07-29 18:35 [PATCH v5 00/11] Bluetooth: btintel: Refactoring setup routines Tedd Ho-Jeong An
` (4 preceding siblings ...)
2021-07-29 18:35 ` [PATCH v5 05/11] Bluetooth: btintel: Fix the first HCI command not work with ROM device Tedd Ho-Jeong An
@ 2021-07-29 18:35 ` Tedd Ho-Jeong An
2021-07-29 19:37 ` Marcel Holtmann
2021-07-29 18:35 ` [PATCH v5 07/11] Bluetooth: btintel: Add combined set_diag functions Tedd Ho-Jeong An
` (4 subsequent siblings)
10 siblings, 1 reply; 26+ messages in thread
From: Tedd Ho-Jeong An @ 2021-07-29 18:35 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Tedd Ho-Jeong An
From: Tedd Ho-Jeong An <tedd.an@intel.com>
Some platforms have an issue with BT LED when the interface is
down or BT radio is turned off, which takes 5 seconds to BT LED
goes off. This command turns off the BT LED immediately.
This patch sends the Intel vendor command to turn off the LED.
Signed-off-by: Tedd Ho-Jeong An <tedd.an@intel.com>
---
drivers/bluetooth/btintel.c | 17 +++++++++++++++++
drivers/bluetooth/btintel.h | 1 +
drivers/bluetooth/btusb.c | 11 +++++++++--
3 files changed, 27 insertions(+), 2 deletions(-)
diff --git a/drivers/bluetooth/btintel.c b/drivers/bluetooth/btintel.c
index 65ecf2ae9a10..4e72d806387c 100644
--- a/drivers/bluetooth/btintel.c
+++ b/drivers/bluetooth/btintel.c
@@ -1757,7 +1757,9 @@ EXPORT_SYMBOL_GPL(btintel_setup_combined);
int btintel_shutdown_combined(struct hci_dev *hdev)
{
+ struct btintel_data *intel = hci_get_priv(hdev);
struct sk_buff *skb;
+ int ret;
/* Send HCI Reset to the controller to stop any BT activity which
* were triggered. This will help to save power and maintain the
@@ -1770,6 +1772,21 @@ int btintel_shutdown_combined(struct hci_dev *hdev)
}
kfree_skb(skb);
+
+ /* Some platforms have an issue with BT LED when the interface is
+ * down or BT radio is turned off, which takes 5 seconds to BT LED
+ * goes off. This command turns off the BT LED immediately.
+ */
+ if (test_bit(INTEL_BROKEN_LED, &intel->flags)) {
+ skb = __hci_cmd_sync(hdev, 0xfc3f, 0, NULL, HCI_INIT_TIMEOUT);
+ if (IS_ERR(skb)) {
+ ret = PTR_ERR(skb);
+ bt_dev_err(hdev, "turning off Intel device LED failed");
+ return ret;
+ }
+ kfree_skb(skb);
+ }
+
return 0;
}
EXPORT_SYMBOL_GPL(btintel_shutdown_combined);
diff --git a/drivers/bluetooth/btintel.h b/drivers/bluetooth/btintel.h
index 29b678364a79..4a35762c3220 100644
--- a/drivers/bluetooth/btintel.h
+++ b/drivers/bluetooth/btintel.h
@@ -144,6 +144,7 @@ struct intel_debug_features {
#define INTEL_FIRMWARE_FAILED 3
#define INTEL_BOOTING 4
#define INTEL_BROKEN_READ_VERSION 5
+#define INTEL_BROKEN_LED 6
struct btintel_data {
unsigned long flags;
diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
index a64473c525eb..8f87c600d026 100644
--- a/drivers/bluetooth/btusb.c
+++ b/drivers/bluetooth/btusb.c
@@ -63,6 +63,7 @@ static struct usb_driver btusb_driver;
#define BTUSB_INTEL_NEWGEN 0x2000000
#define BTUSB_INTEL_COMBINED 0x4000000
#define BTUSB_INTEL_BROKEN_READ_VERSION 0x8000000
+#define BTUSB_INTEL_BROKEN_LED 0x10000000
static const struct usb_device_id btusb_table[] = {
/* Generic Bluetooth USB device */
@@ -378,13 +379,16 @@ static const struct usb_device_id blacklist_table[] = {
BTUSB_VALID_LE_STATES },
{ USB_DEVICE(0x8087, 0x07da), .driver_info = BTUSB_CSR },
{ USB_DEVICE(0x8087, 0x07dc), .driver_info = BTUSB_INTEL_COMBINED |
- BTUSB_INTEL_BROKEN_READ_VERSION },
+ BTUSB_INTEL_BROKEN_READ_VERSION |
+ BTUSB_INTEL_BROKEN_LED },
{ USB_DEVICE(0x8087, 0x0a2a), .driver_info = BTUSB_INTEL_COMBINED |
- BTUSB_INTEL_BROKEN_READ_VERSION },
+ BTUSB_INTEL_BROKEN_READ_VERSION |
+ BTUSB_INTEL_BROKEN_LED },
{ USB_DEVICE(0x8087, 0x0a2b), .driver_info = BTUSB_INTEL_NEW |
BTUSB_WIDEBAND_SPEECH },
{ USB_DEVICE(0x8087, 0x0aa7), .driver_info = BTUSB_INTEL_COMBINED |
BTUSB_INTEL_BROKEN_READ_VERSION |
+ BTUSB_INTEL_BROKEN_LED |
BTUSB_WIDEBAND_SPEECH },
{ USB_DEVICE(0x8087, 0x0aaa), .driver_info = BTUSB_INTEL_NEW |
BTUSB_WIDEBAND_SPEECH |
@@ -4327,6 +4331,9 @@ static int btusb_probe(struct usb_interface *intf,
if (id->driver_info & BTUSB_INTEL_BROKEN_READ_VERSION)
btintel_set_flags(hdev, INTEL_BROKEN_READ_VERSION);
+
+ if (id->driver_info & BTUSB_INTEL_BROKEN_LED)
+ btintel_set_flags(hdev, INTEL_BROKEN_LED);
}
if (id->driver_info & BTUSB_INTEL_NEW) {
--
2.25.1
^ permalink raw reply related [flat|nested] 26+ messages in thread* Re: [PATCH v5 06/11] Bluetooth: btintel: Fix the LED is not turning off immediately
2021-07-29 18:35 ` [PATCH v5 06/11] Bluetooth: btintel: Fix the LED is not turning off immediately Tedd Ho-Jeong An
@ 2021-07-29 19:37 ` Marcel Holtmann
0 siblings, 0 replies; 26+ messages in thread
From: Marcel Holtmann @ 2021-07-29 19:37 UTC (permalink / raw)
To: Tedd Ho-Jeong An; +Cc: linux-bluetooth, Tedd Ho-Jeong An
Hi Tedd,
> Some platforms have an issue with BT LED when the interface is
> down or BT radio is turned off, which takes 5 seconds to BT LED
> goes off. This command turns off the BT LED immediately.
>
> This patch sends the Intel vendor command to turn off the LED.
>
> Signed-off-by: Tedd Ho-Jeong An <tedd.an@intel.com>
> ---
> drivers/bluetooth/btintel.c | 17 +++++++++++++++++
> drivers/bluetooth/btintel.h | 1 +
> drivers/bluetooth/btusb.c | 11 +++++++++--
> 3 files changed, 27 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/bluetooth/btintel.c b/drivers/bluetooth/btintel.c
> index 65ecf2ae9a10..4e72d806387c 100644
> --- a/drivers/bluetooth/btintel.c
> +++ b/drivers/bluetooth/btintel.c
> @@ -1757,7 +1757,9 @@ EXPORT_SYMBOL_GPL(btintel_setup_combined);
>
> int btintel_shutdown_combined(struct hci_dev *hdev)
> {
> + struct btintel_data *intel = hci_get_priv(hdev);
> struct sk_buff *skb;
> + int ret;
>
> /* Send HCI Reset to the controller to stop any BT activity which
> * were triggered. This will help to save power and maintain the
> @@ -1770,6 +1772,21 @@ int btintel_shutdown_combined(struct hci_dev *hdev)
> }
> kfree_skb(skb);
>
> +
> + /* Some platforms have an issue with BT LED when the interface is
> + * down or BT radio is turned off, which takes 5 seconds to BT LED
> + * goes off. This command turns off the BT LED immediately.
> + */
> + if (test_bit(INTEL_BROKEN_LED, &intel->flags)) {
> + skb = __hci_cmd_sync(hdev, 0xfc3f, 0, NULL, HCI_INIT_TIMEOUT);
> + if (IS_ERR(skb)) {
> + ret = PTR_ERR(skb);
> + bt_dev_err(hdev, "turning off Intel device LED failed");
> + return ret;
> + }
> + kfree_skb(skb);
> + }
> +
> return 0;
> }
> EXPORT_SYMBOL_GPL(btintel_shutdown_combined);
> diff --git a/drivers/bluetooth/btintel.h b/drivers/bluetooth/btintel.h
> index 29b678364a79..4a35762c3220 100644
> --- a/drivers/bluetooth/btintel.h
> +++ b/drivers/bluetooth/btintel.h
> @@ -144,6 +144,7 @@ struct intel_debug_features {
> #define INTEL_FIRMWARE_FAILED 3
> #define INTEL_BOOTING 4
> #define INTEL_BROKEN_READ_VERSION 5
> +#define INTEL_BROKEN_LED 6
>
> struct btintel_data {
> unsigned long flags;
> diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
> index a64473c525eb..8f87c600d026 100644
> --- a/drivers/bluetooth/btusb.c
> +++ b/drivers/bluetooth/btusb.c
> @@ -63,6 +63,7 @@ static struct usb_driver btusb_driver;
> #define BTUSB_INTEL_NEWGEN 0x2000000
> #define BTUSB_INTEL_COMBINED 0x4000000
> #define BTUSB_INTEL_BROKEN_READ_VERSION 0x8000000
> +#define BTUSB_INTEL_BROKEN_LED 0x10000000
>
> static const struct usb_device_id btusb_table[] = {
> /* Generic Bluetooth USB device */
> @@ -378,13 +379,16 @@ static const struct usb_device_id blacklist_table[] = {
> BTUSB_VALID_LE_STATES },
> { USB_DEVICE(0x8087, 0x07da), .driver_info = BTUSB_CSR },
> { USB_DEVICE(0x8087, 0x07dc), .driver_info = BTUSB_INTEL_COMBINED |
> - BTUSB_INTEL_BROKEN_READ_VERSION },
> + BTUSB_INTEL_BROKEN_READ_VERSION |
> + BTUSB_INTEL_BROKEN_LED },
> { USB_DEVICE(0x8087, 0x0a2a), .driver_info = BTUSB_INTEL_COMBINED |
> - BTUSB_INTEL_BROKEN_READ_VERSION },
> + BTUSB_INTEL_BROKEN_READ_VERSION |
> + BTUSB_INTEL_BROKEN_LED },
> { USB_DEVICE(0x8087, 0x0a2b), .driver_info = BTUSB_INTEL_NEW |
> BTUSB_WIDEBAND_SPEECH },
> { USB_DEVICE(0x8087, 0x0aa7), .driver_info = BTUSB_INTEL_COMBINED |
> BTUSB_INTEL_BROKEN_READ_VERSION |
> + BTUSB_INTEL_BROKEN_LED |
> BTUSB_WIDEBAND_SPEECH },
> { USB_DEVICE(0x8087, 0x0aaa), .driver_info = BTUSB_INTEL_NEW |
> BTUSB_WIDEBAND_SPEECH |
this is something you need to learn from the HCI_Intel_Read_Version information. This should not bleed into the USB driver.
Regards
Marcel
^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH v5 07/11] Bluetooth: btintel: Add combined set_diag functions
2021-07-29 18:35 [PATCH v5 00/11] Bluetooth: btintel: Refactoring setup routines Tedd Ho-Jeong An
` (5 preceding siblings ...)
2021-07-29 18:35 ` [PATCH v5 06/11] Bluetooth: btintel: Fix the LED is not turning off immediately Tedd Ho-Jeong An
@ 2021-07-29 18:35 ` Tedd Ho-Jeong An
2021-07-29 18:35 ` [PATCH v5 08/11] Bluetooth: btintel: Refactoring setup routine for legacy bootloader Tedd Ho-Jeong An
` (3 subsequent siblings)
10 siblings, 0 replies; 26+ messages in thread
From: Tedd Ho-Jeong An @ 2021-07-29 18:35 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Tedd Ho-Jeong An
From: Tedd Ho-Jeong An <tedd.an@intel.com>
This patch adds a combined set_diag functions.
It also changes the btintel_set_diag_mfg() to static since it is no
longer used by others.
Signed-off-by: Tedd Ho-Jeong An <tedd.an@intel.com>
---
drivers/bluetooth/btintel.c | 23 +++++++++++++++++++++--
drivers/bluetooth/btintel.h | 5 +++--
drivers/bluetooth/btusb.c | 2 +-
3 files changed, 25 insertions(+), 5 deletions(-)
diff --git a/drivers/bluetooth/btintel.c b/drivers/bluetooth/btintel.c
index 4e72d806387c..24b79f449527 100644
--- a/drivers/bluetooth/btintel.c
+++ b/drivers/bluetooth/btintel.c
@@ -164,7 +164,7 @@ int btintel_set_diag(struct hci_dev *hdev, bool enable)
}
EXPORT_SYMBOL_GPL(btintel_set_diag);
-int btintel_set_diag_mfg(struct hci_dev *hdev, bool enable)
+static int btintel_set_diag_mfg(struct hci_dev *hdev, bool enable)
{
int err, ret;
@@ -180,7 +180,25 @@ int btintel_set_diag_mfg(struct hci_dev *hdev, bool enable)
return ret;
}
-EXPORT_SYMBOL_GPL(btintel_set_diag_mfg);
+
+int btintel_set_diag_combined(struct hci_dev *hdev, bool enable)
+{
+ struct btintel_data *intel = hci_get_priv(hdev);
+ int ret;
+
+ /* Legacy ROM device needs to be in the manufacturer mode to apply
+ * diagnostic setting
+ *
+ * This flag is set after reading the Intel version.
+ */
+ if (test_bit(INTEL_ROM_LEGACY, &intel->flags))
+ ret = btintel_set_diag_mfg(hdev, enable);
+ else
+ ret = btintel_set_diag(hdev, enable);
+
+ return ret;
+}
+EXPORT_SYMBOL_GPL(btintel_set_diag_combined);
void btintel_hw_error(struct hci_dev *hdev, u8 code)
{
@@ -1723,6 +1741,7 @@ int btintel_setup_combined(struct hci_dev *hdev)
case 0x07: /* WP */
case 0x08: /* StP */
/* Legacy ROM product */
+ set_bit(INTEL_ROM_LEGACY, &intel->flags);
err = btintel_legacy_rom_setup(hdev, &ver);
break;
case 0x0b: /* SfP */
diff --git a/drivers/bluetooth/btintel.h b/drivers/bluetooth/btintel.h
index 4a35762c3220..abc438b9c62e 100644
--- a/drivers/bluetooth/btintel.h
+++ b/drivers/bluetooth/btintel.h
@@ -145,6 +145,7 @@ struct intel_debug_features {
#define INTEL_BOOTING 4
#define INTEL_BROKEN_READ_VERSION 5
#define INTEL_BROKEN_LED 6
+#define INTEL_ROM_LEGACY 7
struct btintel_data {
unsigned long flags;
@@ -157,7 +158,7 @@ int btintel_enter_mfg(struct hci_dev *hdev);
int btintel_exit_mfg(struct hci_dev *hdev, bool reset, bool patched);
int btintel_set_bdaddr(struct hci_dev *hdev, const bdaddr_t *bdaddr);
int btintel_set_diag(struct hci_dev *hdev, bool enable);
-int btintel_set_diag_mfg(struct hci_dev *hdev, bool enable);
+int btintel_set_diag_combined(struct hci_dev *hdev, bool enable);
void btintel_hw_error(struct hci_dev *hdev, u8 code);
int btintel_version_info(struct hci_dev *hdev, struct intel_version *ver);
@@ -217,7 +218,7 @@ static inline int btintel_set_diag(struct hci_dev *hdev, bool enable)
return -EOPNOTSUPP;
}
-static inline int btintel_set_diag_mfg(struct hci_dev *hdev, bool enable)
+static inline int btintel_set_diag_combined(struct hci_dev *hdev, bool enable)
{
return -EOPNOTSUPP;
}
diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
index 8f87c600d026..01ec2a2d0a1a 100644
--- a/drivers/bluetooth/btusb.c
+++ b/drivers/bluetooth/btusb.c
@@ -4322,7 +4322,7 @@ static int btusb_probe(struct usb_interface *intf,
hdev->manufacturer = 2;
hdev->setup = btintel_setup_combined;
hdev->shutdown = btintel_shutdown_combined;
- hdev->set_diag = btintel_set_diag_mfg;
+ hdev->set_diag = btintel_set_diag_combined;
hdev->set_bdaddr = btintel_set_bdaddr;
hdev->cmd_timeout = btusb_intel_cmd_timeout;
set_bit(HCI_QUIRK_STRICT_DUPLICATE_FILTER, &hdev->quirks);
--
2.25.1
^ permalink raw reply related [flat|nested] 26+ messages in thread* [PATCH v5 08/11] Bluetooth: btintel: Refactoring setup routine for legacy bootloader
2021-07-29 18:35 [PATCH v5 00/11] Bluetooth: btintel: Refactoring setup routines Tedd Ho-Jeong An
` (6 preceding siblings ...)
2021-07-29 18:35 ` [PATCH v5 07/11] Bluetooth: btintel: Add combined set_diag functions Tedd Ho-Jeong An
@ 2021-07-29 18:35 ` Tedd Ho-Jeong An
2021-07-29 19:45 ` Marcel Holtmann
2021-07-29 18:35 ` [PATCH v5 09/11] Bluetooth: btintel: Refactoring setup routine for TLV based booloader Tedd Ho-Jeong An
` (2 subsequent siblings)
10 siblings, 1 reply; 26+ messages in thread
From: Tedd Ho-Jeong An @ 2021-07-29 18:35 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Tedd Ho-Jeong An
From: Tedd Ho-Jeong An <tedd.an@intel.com>
This patch refactors the setup routines for legacy bootloader devices
into combined setup, and move the related functions from btusb to
btintel.
Signed-off-by: Tedd Ho-Jeong An <tedd.an@intel.com>
---
drivers/bluetooth/btintel.c | 419 +++++++++++++++++++++++++++++++++++-
drivers/bluetooth/btintel.h | 12 ++
drivers/bluetooth/btusb.c | 377 +++++---------------------------
3 files changed, 479 insertions(+), 329 deletions(-)
diff --git a/drivers/bluetooth/btintel.c b/drivers/bluetooth/btintel.c
index 24b79f449527..944203cae8e0 100644
--- a/drivers/bluetooth/btintel.c
+++ b/drivers/bluetooth/btintel.c
@@ -1675,6 +1675,392 @@ static int btintel_legacy_rom_setup(struct hci_dev *hdev,
return 0;
}
+static int btintel_download_wait(struct hci_dev *hdev, ktime_t calltime, int msec)
+{
+ struct btintel_data *intel = hci_get_priv(hdev);
+ ktime_t delta, rettime;
+ unsigned long long duration;
+ int err;
+
+ set_bit(INTEL_FIRMWARE_LOADED, &intel->flags);
+
+ bt_dev_info(hdev, "Waiting for firmware download to complete");
+
+ err = wait_on_bit_timeout(&intel->flags, INTEL_DOWNLOADING,
+ TASK_INTERRUPTIBLE,
+ msecs_to_jiffies(msec));
+ if (err == -EINTR) {
+ bt_dev_err(hdev, "Firmware loading interrupted");
+ return err;
+ }
+
+ if (err) {
+ bt_dev_err(hdev, "Firmware loading timeout");
+ return -ETIMEDOUT;
+ }
+
+ if (test_bit(INTEL_FIRMWARE_FAILED, &intel->flags)) {
+ bt_dev_err(hdev, "Firmware loading failed");
+ return -ENOEXEC;
+ }
+
+ rettime = ktime_get();
+ delta = ktime_sub(rettime, calltime);
+ duration = (unsigned long long)ktime_to_ns(delta) >> 10;
+
+ bt_dev_info(hdev, "Firmware loaded in %llu usecs", duration);
+
+ return 0;
+}
+
+static int btintel_boot_wait(struct hci_dev *hdev, ktime_t calltime, int msec)
+{
+ struct btintel_data *intel = hci_get_priv(hdev);
+ ktime_t delta, rettime;
+ unsigned long long duration;
+ int err;
+
+ bt_dev_info(hdev, "Waiting for device to boot");
+
+ err = wait_on_bit_timeout(&intel->flags, INTEL_BOOTING,
+ TASK_INTERRUPTIBLE, msecs_to_jiffies(msec));
+ if (err == -EINTR) {
+ bt_dev_err(hdev, "Device boot interrupted");
+ return -EINTR;
+ }
+
+ if (err) {
+ bt_dev_err(hdev, "Device boot timeout");
+ return -ETIMEDOUT;
+ }
+
+ rettime = ktime_get();
+ delta = ktime_sub(rettime, calltime);
+ duration = (unsigned long long) ktime_to_ns(delta) >> 10;
+
+ bt_dev_info(hdev, "Device booted in %llu usecs", duration);
+
+ return 0;
+}
+
+static int btintel_boot(struct hci_dev *hdev, u32 boot_addr)
+{
+ struct btintel_data *intel = hci_get_priv(hdev);
+ ktime_t calltime;
+ int err;
+
+ calltime = ktime_get();
+
+ set_bit(INTEL_BOOTING, &intel->flags);
+
+ err = btintel_send_intel_reset(hdev, boot_addr);
+ if (err) {
+ bt_dev_err(hdev, "Intel Soft Reset failed (%d)", err);
+ btintel_reset_to_bootloader(hdev);
+ return err;
+ }
+
+ /* The bootloader will not indicate when the device is ready. This
+ * is done by the operational firmware sending bootup notification.
+ *
+ * Booting into operational firmware should not take longer than
+ * 1 second. However if that happens, then just fail the setup
+ * since something went wrong.
+ */
+ err = btintel_boot_wait(hdev, calltime, 1000);
+ if (err == -ETIMEDOUT)
+ btintel_reset_to_bootloader(hdev);
+
+ return err;
+}
+
+static int btintel_get_fw_name(struct intel_version *ver,
+ struct intel_boot_params *params,
+ char *fw_name, size_t len,
+ const char *suffix)
+{
+ switch (ver->hw_variant) {
+ case 0x0b: /* SfP */
+ case 0x0c: /* WsP */
+ snprintf(fw_name, len, "intel/ibt-%u-%u.%s",
+ le16_to_cpu(ver->hw_variant),
+ le16_to_cpu(params->dev_revid),
+ suffix);
+ break;
+ case 0x11: /* JfP */
+ case 0x12: /* ThP */
+ case 0x13: /* HrP */
+ case 0x14: /* CcP */
+ snprintf(fw_name, len, "intel/ibt-%u-%u-%u.%s",
+ le16_to_cpu(ver->hw_variant),
+ le16_to_cpu(ver->hw_revision),
+ le16_to_cpu(ver->fw_revision),
+ suffix);
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ return 0;
+}
+
+static int btintel_download_fw(struct hci_dev *hdev,
+ struct intel_version *ver,
+ struct intel_boot_params *params,
+ u32 *boot_param)
+{
+ const struct firmware *fw;
+ char fwname[64];
+ int err;
+ struct btintel_data *intel = hci_get_priv(hdev);
+ ktime_t calltime;
+
+ if (!ver || !params)
+ return -EINVAL;
+
+ /* The firmware variant determines if the device is in bootloader
+ * mode or is running operational firmware. The value 0x06 identifies
+ * the bootloader and the value 0x23 identifies the operational
+ * firmware.
+ *
+ * When the operational firmware is already present, then only
+ * the check for valid Bluetooth device address is needed. This
+ * determines if the device will be added as configured or
+ * unconfigured controller.
+ *
+ * It is not possible to use the Secure Boot Parameters in this
+ * case since that command is only available in bootloader mode.
+ */
+ if (ver->fw_variant == 0x23) {
+ clear_bit(INTEL_BOOTLOADER, &intel->flags);
+ btintel_check_bdaddr(hdev);
+
+ /* SfP and WsP don't seem to update the firmware version on file
+ * so version checking is currently possible.
+ */
+ switch (ver->hw_variant) {
+ case 0x0b: /* SfP */
+ case 0x0c: /* WsP */
+ return 0;
+ }
+
+ /* Proceed to download to check if the version matches */
+ goto download;
+ }
+
+ /* Read the secure boot parameters to identify the operating
+ * details of the bootloader.
+ */
+ err = btintel_read_boot_params(hdev, params);
+ if (err)
+ return err;
+
+ /* It is required that every single firmware fragment is acknowledged
+ * with a command complete event. If the boot parameters indicate
+ * that this bootloader does not send them, then abort the setup.
+ */
+ if (params->limited_cce != 0x00) {
+ bt_dev_err(hdev, "Unsupported Intel firmware loading method (%u)",
+ params->limited_cce);
+ return -EINVAL;
+ }
+
+ /* If the OTP has no valid Bluetooth device address, then there will
+ * also be no valid address for the operational firmware.
+ */
+ if (!bacmp(¶ms->otp_bdaddr, BDADDR_ANY)) {
+ bt_dev_info(hdev, "No device address configured");
+ set_bit(HCI_QUIRK_INVALID_BDADDR, &hdev->quirks);
+ }
+
+download:
+ /* With this Intel bootloader only the hardware variant and device
+ * revision information are used to select the right firmware for SfP
+ * and WsP.
+ *
+ * The firmware filename is ibt-<hw_variant>-<dev_revid>.sfi.
+ *
+ * Currently the supported hardware variants are:
+ * 11 (0x0b) for iBT3.0 (LnP/SfP)
+ * 12 (0x0c) for iBT3.5 (WsP)
+ *
+ * For ThP/JfP and for future SKU's, the FW name varies based on HW
+ * variant, HW revision and FW revision, as these are dependent on CNVi
+ * and RF Combination.
+ *
+ * 17 (0x11) for iBT3.5 (JfP)
+ * 18 (0x12) for iBT3.5 (ThP)
+ *
+ * The firmware file name for these will be
+ * ibt-<hw_variant>-<hw_revision>-<fw_revision>.sfi.
+ *
+ */
+ err = btintel_get_fw_name(ver, params, fwname, sizeof(fwname), "sfi");
+ if (err < 0) {
+ if (!test_bit(INTEL_BOOTLOADER, &intel->flags)) {
+ /* Firmware has already been loaded */
+ set_bit(INTEL_FIRMWARE_LOADED, &intel->flags);
+ return 0;
+ }
+
+ bt_dev_err(hdev, "Unsupported Intel firmware naming");
+ return -EINVAL;
+ }
+
+ err = firmware_request_nowarn(&fw, fwname, &hdev->dev);
+ if (err < 0) {
+ if (!test_bit(INTEL_BOOTLOADER, &intel->flags)) {
+ /* Firmware has already been loaded */
+ set_bit(INTEL_FIRMWARE_LOADED, &intel->flags);
+ return 0;
+ }
+
+ bt_dev_err(hdev, "Failed to load Intel firmware file %s (%d)",
+ fwname, err);
+ return err;
+ }
+
+ bt_dev_info(hdev, "Found device firmware: %s", fwname);
+
+ if (fw->size < 644) {
+ bt_dev_err(hdev, "Invalid size of firmware file (%zu)",
+ fw->size);
+ err = -EBADF;
+ goto done;
+ }
+
+ calltime = ktime_get();
+
+ set_bit(INTEL_DOWNLOADING, &intel->flags);
+
+ /* Start firmware downloading and get boot parameter */
+ err = btintel_download_firmware(hdev, ver, fw, boot_param);
+ if (err < 0) {
+ if (err == -EALREADY) {
+ /* Firmware has already been loaded */
+ set_bit(INTEL_FIRMWARE_LOADED, &intel->flags);
+ err = 0;
+ goto done;
+ }
+
+ /* When FW download fails, send Intel Reset to retry
+ * FW download.
+ */
+ btintel_reset_to_bootloader(hdev);
+ goto done;
+ }
+
+ /* Before switching the device into operational mode and with that
+ * booting the loaded firmware, wait for the bootloader notification
+ * that all fragments have been successfully received.
+ *
+ * When the event processing receives the notification, then the
+ * INTEL_DOWNLOADING flag will be cleared.
+ *
+ * The firmware loading should not take longer than 5 seconds
+ * and thus just timeout if that happens and fail the setup
+ * of this device.
+ */
+ err = btintel_download_wait(hdev, calltime, 5000);
+ if (err == -ETIMEDOUT)
+ btintel_reset_to_bootloader(hdev);
+
+done:
+ release_firmware(fw);
+ return err;
+}
+
+static int btintel_bootloader_setup(struct hci_dev *hdev,
+ struct intel_version *ver)
+{
+ struct btintel_data *intel = hci_get_priv(hdev);
+ struct intel_version new_ver;
+ struct intel_boot_params params;
+ u32 boot_param;
+ char ddcname[64];
+ int err;
+ struct intel_debug_features features;
+
+ BT_DBG("%s", hdev->name);
+
+ /* Set the default boot parameter to 0x0 and it is updated to
+ * SKU specific boot parameter after reading Intel_Write_Boot_Params
+ * command while downloading the firmware.
+ */
+ boot_param = 0x00000000;
+
+ set_bit(INTEL_BOOTLOADER, &intel->flags);
+
+ err = btintel_download_fw(hdev, ver, ¶ms, &boot_param);
+ if (err)
+ return err;
+
+ /* controller is already having an operational firmware */
+ if (ver->fw_variant == 0x23)
+ goto finish;
+
+ err = btintel_boot(hdev, boot_param);
+ if (err)
+ return err;
+
+ clear_bit(INTEL_BOOTLOADER, &intel->flags);
+
+ err = btintel_get_fw_name(ver, ¶ms, ddcname,
+ sizeof(ddcname), "ddc");
+
+ if (err < 0) {
+ bt_dev_err(hdev, "Unsupported Intel firmware naming");
+ } else {
+ /* Once the device is running in operational mode, it needs to
+ * apply the device configuration (DDC) parameters.
+ *
+ * The device can work without DDC parameters, so even if it
+ * fails to load the file, no need to fail the setup.
+ */
+ btintel_load_ddc_config(hdev, ddcname);
+ }
+
+ /* Read the Intel supported features and if new exception formats
+ * supported, need to load the additional DDC config to enable.
+ */
+ err = btintel_read_debug_features(hdev, &features);
+ if (!err) {
+ /* Set DDC mask for available debug features */
+ btintel_set_debug_features(hdev, &features);
+ }
+
+ /* Read the Intel version information after loading the FW */
+ err = btintel_read_version(hdev, &new_ver);
+ if (err)
+ return err;
+
+ btintel_version_info(hdev, &new_ver);
+
+finish:
+ /* All Intel controllers that support the Microsoft vendor
+ * extension are using 0xFC1E for VsMsftOpCode.
+ */
+ switch (ver->hw_variant) {
+ case 0x11: /* JfP */
+ case 0x12: /* ThP */
+ case 0x13: /* HrP */
+ case 0x14: /* CcP */
+ hci_set_msft_opcode(hdev, 0xFC1E);
+ break;
+ }
+
+ /* Set the event mask for Intel specific vendor events. This enables
+ * a few extra events that are useful during general operation. It
+ * does not enable any debugging related events.
+ *
+ * The device will function correctly without these events enabled
+ * and thus no need to fail the setup.
+ */
+ btintel_set_event_mask(hdev, false);
+
+ return 0;
+}
+
int btintel_setup_combined(struct hci_dev *hdev)
{
struct btintel_data *intel = hci_get_priv(hdev);
@@ -1750,7 +2136,7 @@ int btintel_setup_combined(struct hci_dev *hdev)
case 0x12: /* ThP */
case 0x13: /* HrP */
case 0x14: /* CcP */
- /* TODO: call setup routine for bootloader product */
+ err = btintel_bootloader_setup(hdev, &ver);
break;
default:
bt_dev_err(hdev, "Unsupported Intel hw variant (%u)",
@@ -1818,6 +2204,37 @@ void btintel_set_flags(struct hci_dev *hdev, unsigned int flag)
}
EXPORT_SYMBOL_GPL(btintel_set_flags);
+void btintel_bootup(struct hci_dev *hdev, const void *ptr, unsigned int len)
+{
+ struct btintel_data *intel = hci_get_priv(hdev);
+ const struct intel_bootup *evt = ptr;
+
+ if (len != sizeof(*evt))
+ return;
+
+ if (test_and_clear_bit(INTEL_BOOTING, &intel->flags))
+ wake_up_bit(&intel->flags, INTEL_BOOTING);
+}
+EXPORT_SYMBOL_GPL(btintel_bootup);
+
+void btintel_secure_send_result(struct hci_dev *hdev,
+ const void *ptr, unsigned int len)
+{
+ struct btintel_data *intel = hci_get_priv(hdev);
+ const struct intel_secure_send_result *evt = ptr;
+
+ if (len != sizeof(*evt))
+ return;
+
+ if (evt->result)
+ set_bit(INTEL_FIRMWARE_FAILED, &intel->flags);
+
+ if (test_and_clear_bit(INTEL_DOWNLOADING, &intel->flags) &&
+ test_bit(INTEL_FIRMWARE_LOADED, &intel->flags))
+ wake_up_bit(&intel->flags, INTEL_DOWNLOADING);
+}
+EXPORT_SYMBOL_GPL(btintel_secure_send_result);
+
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 abc438b9c62e..f02ccd7e76fb 100644
--- a/drivers/bluetooth/btintel.h
+++ b/drivers/bluetooth/btintel.h
@@ -191,6 +191,9 @@ 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);
+void btintel_bootup(struct hci_dev *hdev, const void *ptr, unsigned int len);
+void btintel_secure_send_result(struct hci_dev *hdev,
+ const void *ptr, unsigned int len);
#else
static inline int btintel_check_bdaddr(struct hci_dev *hdev)
@@ -337,4 +340,13 @@ static inline int btintel_set_debug_features(struct hci_dev *hdev,
return -EOPNOTSUPP;
}
+static inline void btintel_bootup(struct hci_dev *hdev,
+ const void *ptr, unsigned int len)
+{
+}
+
+static inline void btintel_secure_send_result(struct hci_dev *hdev,
+ const void *ptr, unsigned int len)
+{
+}
#endif
diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
index 01ec2a2d0a1a..4b1e6504f187 100644
--- a/drivers/bluetooth/btusb.c
+++ b/drivers/bluetooth/btusb.c
@@ -365,12 +365,12 @@ static const struct usb_device_id blacklist_table[] = {
{ USB_DEVICE(0x1286, 0x204e), .driver_info = BTUSB_MARVELL },
/* Intel Bluetooth devices */
- { USB_DEVICE(0x8087, 0x0025), .driver_info = BTUSB_INTEL_NEW |
+ { USB_DEVICE(0x8087, 0x0025), .driver_info = BTUSB_INTEL_COMBINED |
BTUSB_WIDEBAND_SPEECH |
BTUSB_VALID_LE_STATES },
- { USB_DEVICE(0x8087, 0x0026), .driver_info = BTUSB_INTEL_NEW |
+ { USB_DEVICE(0x8087, 0x0026), .driver_info = BTUSB_INTEL_COMBINED |
BTUSB_WIDEBAND_SPEECH },
- { USB_DEVICE(0x8087, 0x0029), .driver_info = BTUSB_INTEL_NEW |
+ { USB_DEVICE(0x8087, 0x0029), .driver_info = BTUSB_INTEL_COMBINED |
BTUSB_WIDEBAND_SPEECH },
{ USB_DEVICE(0x8087, 0x0032), .driver_info = BTUSB_INTEL_NEWGEN |
BTUSB_WIDEBAND_SPEECH},
@@ -384,13 +384,13 @@ static const struct usb_device_id blacklist_table[] = {
{ USB_DEVICE(0x8087, 0x0a2a), .driver_info = BTUSB_INTEL_COMBINED |
BTUSB_INTEL_BROKEN_READ_VERSION |
BTUSB_INTEL_BROKEN_LED },
- { USB_DEVICE(0x8087, 0x0a2b), .driver_info = BTUSB_INTEL_NEW |
+ { USB_DEVICE(0x8087, 0x0a2b), .driver_info = BTUSB_INTEL_COMBINED |
BTUSB_WIDEBAND_SPEECH },
{ USB_DEVICE(0x8087, 0x0aa7), .driver_info = BTUSB_INTEL_COMBINED |
BTUSB_INTEL_BROKEN_READ_VERSION |
BTUSB_INTEL_BROKEN_LED |
BTUSB_WIDEBAND_SPEECH },
- { USB_DEVICE(0x8087, 0x0aaa), .driver_info = BTUSB_INTEL_NEW |
+ { USB_DEVICE(0x8087, 0x0aaa), .driver_info = BTUSB_INTEL_COMBINED |
BTUSB_WIDEBAND_SPEECH |
BTUSB_VALID_LE_STATES },
@@ -1999,11 +1999,14 @@ static int inject_cmd_complete(struct hci_dev *hdev, __u16 opcode)
static int btusb_recv_bulk_intel(struct btusb_data *data, void *buffer,
int count)
{
+ struct btintel_data *intel = hci_get_priv(data->hdev);
+
/* When the device is in bootloader mode, then it can send
* events via the bulk endpoint. These events are treated the
* same way as the ones received from the interrupt endpoint.
*/
- if (test_bit(BTUSB_BOOTLOADER, &data->flags))
+ if (test_bit(BTUSB_BOOTLOADER, &data->flags) ||
+ test_bit(INTEL_BOOTLOADER, &intel->flags))
return btusb_recv_intr(data, buffer, count);
return btusb_recv_bulk(data, buffer, count);
@@ -2040,6 +2043,7 @@ static void btusb_intel_secure_send_result(struct btusb_data *data,
static int btusb_recv_event_intel(struct hci_dev *hdev, struct sk_buff *skb)
{
struct btusb_data *data = hci_get_drvdata(hdev);
+ struct btintel_data *intel = hci_get_priv(hdev);
if (test_bit(BTUSB_BOOTLOADER, &data->flags)) {
struct hci_event_hdr *hdr = (void *)skb->data;
@@ -2069,19 +2073,49 @@ static int btusb_recv_event_intel(struct hci_dev *hdev, struct sk_buff *skb)
}
}
+ if (test_bit(INTEL_BOOTLOADER, &intel->flags)) {
+ struct hci_event_hdr *hdr = (void *)skb->data;
+
+ if (skb->len > HCI_EVENT_HDR_SIZE && hdr->evt == 0xff &&
+ hdr->plen > 0) {
+ const void *ptr = skb->data + HCI_EVENT_HDR_SIZE + 1;
+ unsigned int len = skb->len - HCI_EVENT_HDR_SIZE - 1;
+
+ switch (skb->data[2]) {
+ case 0x02:
+ /* When switching to the operational firmware
+ * the device sends a vendor specific event
+ * indicating that the bootup completed.
+ */
+ btintel_bootup(hdev, ptr, len);
+ break;
+ case 0x06:
+ /* When the firmware loading completes the
+ * device sends out a vendor specific event
+ * indicating the result of the firmware
+ * loading.
+ */
+ btintel_secure_send_result(hdev, ptr, len);
+ break;
+ }
+ }
+ }
+
return hci_recv_frame(hdev, skb);
}
static int btusb_send_frame_intel(struct hci_dev *hdev, struct sk_buff *skb)
{
struct btusb_data *data = hci_get_drvdata(hdev);
+ struct btintel_data *intel = hci_get_priv(hdev);
struct urb *urb;
BT_DBG("%s", hdev->name);
switch (hci_skb_pkt_type(skb)) {
case HCI_COMMAND_PKT:
- if (test_bit(BTUSB_BOOTLOADER, &data->flags)) {
+ if (test_bit(BTUSB_BOOTLOADER, &data->flags) ||
+ test_bit(INTEL_BOOTLOADER, &intel->flags)) {
struct hci_command_hdr *cmd = (void *)skb->data;
__u16 opcode = le16_to_cpu(cmd->opcode);
@@ -2133,36 +2167,6 @@ static int btusb_send_frame_intel(struct hci_dev *hdev, struct sk_buff *skb)
return -EILSEQ;
}
-static int btusb_setup_intel_new_get_fw_name(struct intel_version *ver,
- struct intel_boot_params *params,
- char *fw_name, size_t len,
- const char *suffix)
-{
- switch (ver->hw_variant) {
- case 0x0b: /* SfP */
- case 0x0c: /* WsP */
- snprintf(fw_name, len, "intel/ibt-%u-%u.%s",
- le16_to_cpu(ver->hw_variant),
- le16_to_cpu(params->dev_revid),
- suffix);
- break;
- case 0x11: /* JfP */
- case 0x12: /* ThP */
- case 0x13: /* HrP */
- case 0x14: /* CcP */
- snprintf(fw_name, len, "intel/ibt-%u-%u-%u.%s",
- le16_to_cpu(ver->hw_variant),
- le16_to_cpu(ver->hw_revision),
- le16_to_cpu(ver->fw_revision),
- suffix);
- break;
- default:
- return -EINVAL;
- }
-
- return 0;
-}
-
static void btusb_setup_intel_newgen_get_fw_name(const struct intel_version_tlv *ver_tlv,
char *fw_name, size_t len,
const char *suffix)
@@ -2322,173 +2326,6 @@ static int btusb_intel_download_firmware_newgen(struct hci_dev *hdev,
return err;
}
-static int btusb_intel_download_firmware(struct hci_dev *hdev,
- struct intel_version *ver,
- struct intel_boot_params *params,
- u32 *boot_param)
-{
- const struct firmware *fw;
- char fwname[64];
- int err;
- struct btusb_data *data = hci_get_drvdata(hdev);
- ktime_t calltime;
-
- if (!ver || !params)
- return -EINVAL;
-
- /* The firmware variant determines if the device is in bootloader
- * mode or is running operational firmware. The value 0x06 identifies
- * the bootloader and the value 0x23 identifies the operational
- * firmware.
- *
- * When the operational firmware is already present, then only
- * the check for valid Bluetooth device address is needed. This
- * determines if the device will be added as configured or
- * unconfigured controller.
- *
- * It is not possible to use the Secure Boot Parameters in this
- * case since that command is only available in bootloader mode.
- */
- if (ver->fw_variant == 0x23) {
- clear_bit(BTUSB_BOOTLOADER, &data->flags);
- btintel_check_bdaddr(hdev);
-
- /* SfP and WsP don't seem to update the firmware version on file
- * so version checking is currently possible.
- */
- switch (ver->hw_variant) {
- case 0x0b: /* SfP */
- case 0x0c: /* WsP */
- return 0;
- }
-
- /* Proceed to download to check if the version matches */
- goto download;
- }
-
- /* Read the secure boot parameters to identify the operating
- * details of the bootloader.
- */
- err = btintel_read_boot_params(hdev, params);
- if (err)
- return err;
-
- /* It is required that every single firmware fragment is acknowledged
- * with a command complete event. If the boot parameters indicate
- * that this bootloader does not send them, then abort the setup.
- */
- if (params->limited_cce != 0x00) {
- bt_dev_err(hdev, "Unsupported Intel firmware loading method (%u)",
- params->limited_cce);
- return -EINVAL;
- }
-
- /* If the OTP has no valid Bluetooth device address, then there will
- * also be no valid address for the operational firmware.
- */
- if (!bacmp(¶ms->otp_bdaddr, BDADDR_ANY)) {
- bt_dev_info(hdev, "No device address configured");
- set_bit(HCI_QUIRK_INVALID_BDADDR, &hdev->quirks);
- }
-
-download:
- /* With this Intel bootloader only the hardware variant and device
- * revision information are used to select the right firmware for SfP
- * and WsP.
- *
- * The firmware filename is ibt-<hw_variant>-<dev_revid>.sfi.
- *
- * Currently the supported hardware variants are:
- * 11 (0x0b) for iBT3.0 (LnP/SfP)
- * 12 (0x0c) for iBT3.5 (WsP)
- *
- * For ThP/JfP and for future SKU's, the FW name varies based on HW
- * variant, HW revision and FW revision, as these are dependent on CNVi
- * and RF Combination.
- *
- * 17 (0x11) for iBT3.5 (JfP)
- * 18 (0x12) for iBT3.5 (ThP)
- *
- * The firmware file name for these will be
- * ibt-<hw_variant>-<hw_revision>-<fw_revision>.sfi.
- *
- */
- err = btusb_setup_intel_new_get_fw_name(ver, params, fwname,
- sizeof(fwname), "sfi");
- if (err < 0) {
- if (!test_bit(BTUSB_BOOTLOADER, &data->flags)) {
- /* Firmware has already been loaded */
- set_bit(BTUSB_FIRMWARE_LOADED, &data->flags);
- return 0;
- }
-
- bt_dev_err(hdev, "Unsupported Intel firmware naming");
- return -EINVAL;
- }
-
- err = firmware_request_nowarn(&fw, fwname, &hdev->dev);
- if (err < 0) {
- if (!test_bit(BTUSB_BOOTLOADER, &data->flags)) {
- /* Firmware has already been loaded */
- set_bit(BTUSB_FIRMWARE_LOADED, &data->flags);
- return 0;
- }
-
- bt_dev_err(hdev, "Failed to load Intel firmware file %s (%d)",
- fwname, err);
- return err;
- }
-
- bt_dev_info(hdev, "Found device firmware: %s", fwname);
-
- if (fw->size < 644) {
- bt_dev_err(hdev, "Invalid size of firmware file (%zu)",
- fw->size);
- err = -EBADF;
- goto done;
- }
-
- calltime = ktime_get();
-
- set_bit(BTUSB_DOWNLOADING, &data->flags);
-
- /* Start firmware downloading and get boot parameter */
- err = btintel_download_firmware(hdev, ver, fw, boot_param);
- if (err < 0) {
- if (err == -EALREADY) {
- /* Firmware has already been loaded */
- set_bit(BTUSB_FIRMWARE_LOADED, &data->flags);
- err = 0;
- goto done;
- }
-
- /* When FW download fails, send Intel Reset to retry
- * FW download.
- */
- btintel_reset_to_bootloader(hdev);
- goto done;
- }
-
- /* Before switching the device into operational mode and with that
- * booting the loaded firmware, wait for the bootloader notification
- * that all fragments have been successfully received.
- *
- * When the event processing receives the notification, then the
- * BTUSB_DOWNLOADING flag will be cleared.
- *
- * The firmware loading should not take longer than 5 seconds
- * and thus just timeout if that happens and fail the setup
- * of this device.
- */
- err = btusb_download_wait(hdev, calltime, 5000);
- if (err == -ETIMEDOUT)
- btintel_reset_to_bootloader(hdev);
-
-done:
- release_firmware(fw);
- return err;
-}
-
static int btusb_boot_wait(struct hci_dev *hdev, ktime_t calltime, int msec)
{
struct btusb_data *data = hci_get_drvdata(hdev);
@@ -2551,109 +2388,6 @@ static int btusb_intel_boot(struct hci_dev *hdev, u32 boot_addr)
return err;
}
-static int btusb_setup_intel_new(struct hci_dev *hdev)
-{
- struct btusb_data *data = hci_get_drvdata(hdev);
- struct intel_version ver;
- struct intel_boot_params params;
- u32 boot_param;
- char ddcname[64];
- int err;
- struct intel_debug_features features;
-
- BT_DBG("%s", hdev->name);
-
- /* Set the default boot parameter to 0x0 and it is updated to
- * SKU specific boot parameter after reading Intel_Write_Boot_Params
- * command while downloading the firmware.
- */
- boot_param = 0x00000000;
-
- /* Read the Intel version information to determine if the device
- * is in bootloader mode or if it already has operational firmware
- * loaded.
- */
- err = btintel_read_version(hdev, &ver);
- if (err) {
- bt_dev_err(hdev, "Intel Read version failed (%d)", err);
- btintel_reset_to_bootloader(hdev);
- return err;
- }
-
- err = btintel_version_info(hdev, &ver);
- if (err)
- return err;
-
- err = btusb_intel_download_firmware(hdev, &ver, ¶ms, &boot_param);
- if (err)
- return err;
-
- /* controller is already having an operational firmware */
- if (ver.fw_variant == 0x23)
- goto finish;
-
- err = btusb_intel_boot(hdev, boot_param);
- if (err)
- return err;
-
- clear_bit(BTUSB_BOOTLOADER, &data->flags);
-
- err = btusb_setup_intel_new_get_fw_name(&ver, ¶ms, ddcname,
- sizeof(ddcname), "ddc");
-
- if (err < 0) {
- bt_dev_err(hdev, "Unsupported Intel firmware naming");
- } else {
- /* Once the device is running in operational mode, it needs to
- * apply the device configuration (DDC) parameters.
- *
- * The device can work without DDC parameters, so even if it
- * fails to load the file, no need to fail the setup.
- */
- btintel_load_ddc_config(hdev, ddcname);
- }
-
- /* Read the Intel supported features and if new exception formats
- * supported, need to load the additional DDC config to enable.
- */
- err = btintel_read_debug_features(hdev, &features);
- if (!err) {
- /* Set DDC mask for available debug features */
- btintel_set_debug_features(hdev, &features);
- }
-
- /* Read the Intel version information after loading the FW */
- err = btintel_read_version(hdev, &ver);
- if (err)
- return err;
-
- btintel_version_info(hdev, &ver);
-
-finish:
- /* All Intel controllers that support the Microsoft vendor
- * extension are using 0xFC1E for VsMsftOpCode.
- */
- switch (ver.hw_variant) {
- case 0x11: /* JfP */
- case 0x12: /* ThP */
- case 0x13: /* HrP */
- case 0x14: /* CcP */
- hci_set_msft_opcode(hdev, 0xFC1E);
- break;
- }
-
- /* Set the event mask for Intel specific vendor events. This enables
- * a few extra events that are useful during general operation. It
- * does not enable any debugging related events.
- *
- * The device will function correctly without these events enabled
- * and thus no need to fail the setup.
- */
- btintel_set_event_mask(hdev, false);
-
- return 0;
-}
-
static int btusb_setup_intel_newgen(struct hci_dev *hdev)
{
struct btusb_data *data = hci_get_drvdata(hdev);
@@ -4229,18 +3963,17 @@ static int btusb_probe(struct usb_interface *intf,
priv_size = 0;
- if (id->driver_info & BTUSB_INTEL_COMBINED) {
+ data->recv_event = hci_recv_frame;
+ data->recv_bulk = btusb_recv_bulk;
+
+ if (id->driver_info & BTUSB_INTEL_COMBINED ||
+ id->driver_info & BTUSB_INTEL_NEW) {
/* Allocate extra space for Intel device */
priv_size += sizeof(struct btintel_data);
- }
- if (id->driver_info & BTUSB_INTEL_NEW) {
+ /* Override the rx handlers */
data->recv_event = btusb_recv_event_intel;
data->recv_bulk = btusb_recv_bulk_intel;
- set_bit(BTUSB_BOOTLOADER, &data->flags);
- } else {
- data->recv_event = hci_recv_frame;
- data->recv_bulk = btusb_recv_bulk;
}
hdev = hci_alloc_dev_priv(priv_size);
@@ -4320,8 +4053,10 @@ static int btusb_probe(struct usb_interface *intf,
/* Combined Intel Device setup to support multiple setup routine */
if (id->driver_info & BTUSB_INTEL_COMBINED) {
hdev->manufacturer = 2;
+ hdev->send = btusb_send_frame_intel;
hdev->setup = btintel_setup_combined;
hdev->shutdown = btintel_shutdown_combined;
+ hdev->hw_error = btintel_hw_error;
hdev->set_diag = btintel_set_diag_combined;
hdev->set_bdaddr = btintel_set_bdaddr;
hdev->cmd_timeout = btusb_intel_cmd_timeout;
@@ -4336,20 +4071,6 @@ static int btusb_probe(struct usb_interface *intf,
btintel_set_flags(hdev, INTEL_BROKEN_LED);
}
- if (id->driver_info & BTUSB_INTEL_NEW) {
- hdev->manufacturer = 2;
- hdev->send = btusb_send_frame_intel;
- hdev->setup = btusb_setup_intel_new;
- hdev->shutdown = btusb_shutdown_intel_new;
- hdev->hw_error = btintel_hw_error;
- hdev->set_diag = btintel_set_diag;
- hdev->set_bdaddr = btintel_set_bdaddr;
- hdev->cmd_timeout = btusb_intel_cmd_timeout;
- set_bit(HCI_QUIRK_STRICT_DUPLICATE_FILTER, &hdev->quirks);
- set_bit(HCI_QUIRK_SIMULTANEOUS_DISCOVERY, &hdev->quirks);
- set_bit(HCI_QUIRK_NON_PERSISTENT_DIAG, &hdev->quirks);
- }
-
if (id->driver_info & BTUSB_INTEL_NEWGEN) {
hdev->manufacturer = 2;
hdev->send = btusb_send_frame_intel;
--
2.25.1
^ permalink raw reply related [flat|nested] 26+ messages in thread* Re: [PATCH v5 08/11] Bluetooth: btintel: Refactoring setup routine for legacy bootloader
2021-07-29 18:35 ` [PATCH v5 08/11] Bluetooth: btintel: Refactoring setup routine for legacy bootloader Tedd Ho-Jeong An
@ 2021-07-29 19:45 ` Marcel Holtmann
0 siblings, 0 replies; 26+ messages in thread
From: Marcel Holtmann @ 2021-07-29 19:45 UTC (permalink / raw)
To: Tedd Ho-Jeong An; +Cc: linux-bluetooth, Tedd Ho-Jeong An
Hi Tedd,
> This patch refactors the setup routines for legacy bootloader devices
> into combined setup, and move the related functions from btusb to
> btintel.
>
> Signed-off-by: Tedd Ho-Jeong An <tedd.an@intel.com>
> ---
> drivers/bluetooth/btintel.c | 419 +++++++++++++++++++++++++++++++++++-
> drivers/bluetooth/btintel.h | 12 ++
> drivers/bluetooth/btusb.c | 377 +++++---------------------------
> 3 files changed, 479 insertions(+), 329 deletions(-)
>
> diff --git a/drivers/bluetooth/btintel.c b/drivers/bluetooth/btintel.c
> index 24b79f449527..944203cae8e0 100644
> --- a/drivers/bluetooth/btintel.c
> +++ b/drivers/bluetooth/btintel.c
> @@ -1675,6 +1675,392 @@ static int btintel_legacy_rom_setup(struct hci_dev *hdev,
> return 0;
> }
>
> +static int btintel_download_wait(struct hci_dev *hdev, ktime_t calltime, int msec)
> +{
> + struct btintel_data *intel = hci_get_priv(hdev);
> + ktime_t delta, rettime;
> + unsigned long long duration;
> + int err;
> +
> + set_bit(INTEL_FIRMWARE_LOADED, &intel->flags);
> +
> + bt_dev_info(hdev, "Waiting for firmware download to complete");
> +
> + err = wait_on_bit_timeout(&intel->flags, INTEL_DOWNLOADING,
> + TASK_INTERRUPTIBLE,
> + msecs_to_jiffies(msec));
> + if (err == -EINTR) {
> + bt_dev_err(hdev, "Firmware loading interrupted");
> + return err;
> + }
> +
> + if (err) {
> + bt_dev_err(hdev, "Firmware loading timeout");
> + return -ETIMEDOUT;
> + }
> +
> + if (test_bit(INTEL_FIRMWARE_FAILED, &intel->flags)) {
> + bt_dev_err(hdev, "Firmware loading failed");
> + return -ENOEXEC;
> + }
> +
> + rettime = ktime_get();
> + delta = ktime_sub(rettime, calltime);
> + duration = (unsigned long long)ktime_to_ns(delta) >> 10;
> +
> + bt_dev_info(hdev, "Firmware loaded in %llu usecs", duration);
> +
> + return 0;
> +}
> +
> +static int btintel_boot_wait(struct hci_dev *hdev, ktime_t calltime, int msec)
> +{
> + struct btintel_data *intel = hci_get_priv(hdev);
> + ktime_t delta, rettime;
> + unsigned long long duration;
> + int err;
> +
> + bt_dev_info(hdev, "Waiting for device to boot");
> +
> + err = wait_on_bit_timeout(&intel->flags, INTEL_BOOTING,
> + TASK_INTERRUPTIBLE, msecs_to_jiffies(msec));
> + if (err == -EINTR) {
> + bt_dev_err(hdev, "Device boot interrupted");
> + return -EINTR;
> + }
> +
> + if (err) {
> + bt_dev_err(hdev, "Device boot timeout");
> + return -ETIMEDOUT;
> + }
> +
> + rettime = ktime_get();
> + delta = ktime_sub(rettime, calltime);
> + duration = (unsigned long long) ktime_to_ns(delta) >> 10;
> +
> + bt_dev_info(hdev, "Device booted in %llu usecs", duration);
> +
> + return 0;
> +}
> +
> +static int btintel_boot(struct hci_dev *hdev, u32 boot_addr)
> +{
> + struct btintel_data *intel = hci_get_priv(hdev);
> + ktime_t calltime;
> + int err;
> +
> + calltime = ktime_get();
> +
> + set_bit(INTEL_BOOTING, &intel->flags);
> +
> + err = btintel_send_intel_reset(hdev, boot_addr);
> + if (err) {
> + bt_dev_err(hdev, "Intel Soft Reset failed (%d)", err);
> + btintel_reset_to_bootloader(hdev);
> + return err;
> + }
> +
> + /* The bootloader will not indicate when the device is ready. This
> + * is done by the operational firmware sending bootup notification.
> + *
> + * Booting into operational firmware should not take longer than
> + * 1 second. However if that happens, then just fail the setup
> + * since something went wrong.
> + */
> + err = btintel_boot_wait(hdev, calltime, 1000);
> + if (err == -ETIMEDOUT)
> + btintel_reset_to_bootloader(hdev);
> +
> + return err;
> +}
> +
> +static int btintel_get_fw_name(struct intel_version *ver,
> + struct intel_boot_params *params,
> + char *fw_name, size_t len,
> + const char *suffix)
> +{
> + switch (ver->hw_variant) {
> + case 0x0b: /* SfP */
> + case 0x0c: /* WsP */
> + snprintf(fw_name, len, "intel/ibt-%u-%u.%s",
> + le16_to_cpu(ver->hw_variant),
> + le16_to_cpu(params->dev_revid),
> + suffix);
> + break;
> + case 0x11: /* JfP */
> + case 0x12: /* ThP */
> + case 0x13: /* HrP */
> + case 0x14: /* CcP */
> + snprintf(fw_name, len, "intel/ibt-%u-%u-%u.%s",
> + le16_to_cpu(ver->hw_variant),
> + le16_to_cpu(ver->hw_revision),
> + le16_to_cpu(ver->fw_revision),
> + suffix);
> + break;
> + default:
> + return -EINVAL;
> + }
> +
> + return 0;
> +}
> +
> +static int btintel_download_fw(struct hci_dev *hdev,
> + struct intel_version *ver,
> + struct intel_boot_params *params,
> + u32 *boot_param)
> +{
> + const struct firmware *fw;
> + char fwname[64];
> + int err;
> + struct btintel_data *intel = hci_get_priv(hdev);
> + ktime_t calltime;
> +
> + if (!ver || !params)
> + return -EINVAL;
> +
> + /* The firmware variant determines if the device is in bootloader
> + * mode or is running operational firmware. The value 0x06 identifies
> + * the bootloader and the value 0x23 identifies the operational
> + * firmware.
> + *
> + * When the operational firmware is already present, then only
> + * the check for valid Bluetooth device address is needed. This
> + * determines if the device will be added as configured or
> + * unconfigured controller.
> + *
> + * It is not possible to use the Secure Boot Parameters in this
> + * case since that command is only available in bootloader mode.
> + */
> + if (ver->fw_variant == 0x23) {
> + clear_bit(INTEL_BOOTLOADER, &intel->flags);
> + btintel_check_bdaddr(hdev);
> +
> + /* SfP and WsP don't seem to update the firmware version on file
> + * so version checking is currently possible.
> + */
> + switch (ver->hw_variant) {
> + case 0x0b: /* SfP */
> + case 0x0c: /* WsP */
> + return 0;
> + }
> +
> + /* Proceed to download to check if the version matches */
> + goto download;
> + }
> +
> + /* Read the secure boot parameters to identify the operating
> + * details of the bootloader.
> + */
> + err = btintel_read_boot_params(hdev, params);
> + if (err)
> + return err;
> +
> + /* It is required that every single firmware fragment is acknowledged
> + * with a command complete event. If the boot parameters indicate
> + * that this bootloader does not send them, then abort the setup.
> + */
> + if (params->limited_cce != 0x00) {
> + bt_dev_err(hdev, "Unsupported Intel firmware loading method (%u)",
> + params->limited_cce);
> + return -EINVAL;
> + }
> +
> + /* If the OTP has no valid Bluetooth device address, then there will
> + * also be no valid address for the operational firmware.
> + */
> + if (!bacmp(¶ms->otp_bdaddr, BDADDR_ANY)) {
> + bt_dev_info(hdev, "No device address configured");
> + set_bit(HCI_QUIRK_INVALID_BDADDR, &hdev->quirks);
> + }
> +
> +download:
> + /* With this Intel bootloader only the hardware variant and device
> + * revision information are used to select the right firmware for SfP
> + * and WsP.
> + *
> + * The firmware filename is ibt-<hw_variant>-<dev_revid>.sfi.
> + *
> + * Currently the supported hardware variants are:
> + * 11 (0x0b) for iBT3.0 (LnP/SfP)
> + * 12 (0x0c) for iBT3.5 (WsP)
> + *
> + * For ThP/JfP and for future SKU's, the FW name varies based on HW
> + * variant, HW revision and FW revision, as these are dependent on CNVi
> + * and RF Combination.
> + *
> + * 17 (0x11) for iBT3.5 (JfP)
> + * 18 (0x12) for iBT3.5 (ThP)
> + *
> + * The firmware file name for these will be
> + * ibt-<hw_variant>-<hw_revision>-<fw_revision>.sfi.
> + *
> + */
> + err = btintel_get_fw_name(ver, params, fwname, sizeof(fwname), "sfi");
> + if (err < 0) {
> + if (!test_bit(INTEL_BOOTLOADER, &intel->flags)) {
> + /* Firmware has already been loaded */
> + set_bit(INTEL_FIRMWARE_LOADED, &intel->flags);
> + return 0;
> + }
> +
> + bt_dev_err(hdev, "Unsupported Intel firmware naming");
> + return -EINVAL;
> + }
> +
> + err = firmware_request_nowarn(&fw, fwname, &hdev->dev);
> + if (err < 0) {
> + if (!test_bit(INTEL_BOOTLOADER, &intel->flags)) {
> + /* Firmware has already been loaded */
> + set_bit(INTEL_FIRMWARE_LOADED, &intel->flags);
> + return 0;
> + }
> +
> + bt_dev_err(hdev, "Failed to load Intel firmware file %s (%d)",
> + fwname, err);
> + return err;
> + }
> +
> + bt_dev_info(hdev, "Found device firmware: %s", fwname);
> +
> + if (fw->size < 644) {
> + bt_dev_err(hdev, "Invalid size of firmware file (%zu)",
> + fw->size);
> + err = -EBADF;
> + goto done;
> + }
> +
> + calltime = ktime_get();
> +
> + set_bit(INTEL_DOWNLOADING, &intel->flags);
> +
> + /* Start firmware downloading and get boot parameter */
> + err = btintel_download_firmware(hdev, ver, fw, boot_param);
> + if (err < 0) {
> + if (err == -EALREADY) {
> + /* Firmware has already been loaded */
> + set_bit(INTEL_FIRMWARE_LOADED, &intel->flags);
> + err = 0;
> + goto done;
> + }
> +
> + /* When FW download fails, send Intel Reset to retry
> + * FW download.
> + */
> + btintel_reset_to_bootloader(hdev);
> + goto done;
> + }
> +
> + /* Before switching the device into operational mode and with that
> + * booting the loaded firmware, wait for the bootloader notification
> + * that all fragments have been successfully received.
> + *
> + * When the event processing receives the notification, then the
> + * INTEL_DOWNLOADING flag will be cleared.
> + *
> + * The firmware loading should not take longer than 5 seconds
> + * and thus just timeout if that happens and fail the setup
> + * of this device.
> + */
> + err = btintel_download_wait(hdev, calltime, 5000);
> + if (err == -ETIMEDOUT)
> + btintel_reset_to_bootloader(hdev);
> +
> +done:
> + release_firmware(fw);
> + return err;
> +}
> +
> +static int btintel_bootloader_setup(struct hci_dev *hdev,
> + struct intel_version *ver)
> +{
> + struct btintel_data *intel = hci_get_priv(hdev);
> + struct intel_version new_ver;
> + struct intel_boot_params params;
> + u32 boot_param;
> + char ddcname[64];
> + int err;
> + struct intel_debug_features features;
> +
> + BT_DBG("%s", hdev->name);
> +
> + /* Set the default boot parameter to 0x0 and it is updated to
> + * SKU specific boot parameter after reading Intel_Write_Boot_Params
> + * command while downloading the firmware.
> + */
> + boot_param = 0x00000000;
> +
> + set_bit(INTEL_BOOTLOADER, &intel->flags);
> +
> + err = btintel_download_fw(hdev, ver, ¶ms, &boot_param);
> + if (err)
> + return err;
> +
> + /* controller is already having an operational firmware */
> + if (ver->fw_variant == 0x23)
> + goto finish;
> +
> + err = btintel_boot(hdev, boot_param);
> + if (err)
> + return err;
> +
> + clear_bit(INTEL_BOOTLOADER, &intel->flags);
> +
> + err = btintel_get_fw_name(ver, ¶ms, ddcname,
> + sizeof(ddcname), "ddc");
> +
> + if (err < 0) {
> + bt_dev_err(hdev, "Unsupported Intel firmware naming");
> + } else {
> + /* Once the device is running in operational mode, it needs to
> + * apply the device configuration (DDC) parameters.
> + *
> + * The device can work without DDC parameters, so even if it
> + * fails to load the file, no need to fail the setup.
> + */
> + btintel_load_ddc_config(hdev, ddcname);
> + }
> +
> + /* Read the Intel supported features and if new exception formats
> + * supported, need to load the additional DDC config to enable.
> + */
> + err = btintel_read_debug_features(hdev, &features);
> + if (!err) {
> + /* Set DDC mask for available debug features */
> + btintel_set_debug_features(hdev, &features);
> + }
> +
> + /* Read the Intel version information after loading the FW */
> + err = btintel_read_version(hdev, &new_ver);
> + if (err)
> + return err;
> +
> + btintel_version_info(hdev, &new_ver);
> +
> +finish:
> + /* All Intel controllers that support the Microsoft vendor
> + * extension are using 0xFC1E for VsMsftOpCode.
> + */
> + switch (ver->hw_variant) {
> + case 0x11: /* JfP */
> + case 0x12: /* ThP */
> + case 0x13: /* HrP */
> + case 0x14: /* CcP */
> + hci_set_msft_opcode(hdev, 0xFC1E);
> + break;
> + }
> +
> + /* Set the event mask for Intel specific vendor events. This enables
> + * a few extra events that are useful during general operation. It
> + * does not enable any debugging related events.
> + *
> + * The device will function correctly without these events enabled
> + * and thus no need to fail the setup.
> + */
> + btintel_set_event_mask(hdev, false);
> +
> + return 0;
> +}
> +
> int btintel_setup_combined(struct hci_dev *hdev)
> {
> struct btintel_data *intel = hci_get_priv(hdev);
> @@ -1750,7 +2136,7 @@ int btintel_setup_combined(struct hci_dev *hdev)
> case 0x12: /* ThP */
> case 0x13: /* HrP */
> case 0x14: /* CcP */
> - /* TODO: call setup routine for bootloader product */
> + err = btintel_bootloader_setup(hdev, &ver);
> break;
> default:
> bt_dev_err(hdev, "Unsupported Intel hw variant (%u)",
> @@ -1818,6 +2204,37 @@ void btintel_set_flags(struct hci_dev *hdev, unsigned int flag)
> }
> EXPORT_SYMBOL_GPL(btintel_set_flags);
>
> +void btintel_bootup(struct hci_dev *hdev, const void *ptr, unsigned int len)
> +{
> + struct btintel_data *intel = hci_get_priv(hdev);
> + const struct intel_bootup *evt = ptr;
> +
> + if (len != sizeof(*evt))
> + return;
> +
> + if (test_and_clear_bit(INTEL_BOOTING, &intel->flags))
> + wake_up_bit(&intel->flags, INTEL_BOOTING);
> +}
> +EXPORT_SYMBOL_GPL(btintel_bootup);
> +
> +void btintel_secure_send_result(struct hci_dev *hdev,
> + const void *ptr, unsigned int len)
> +{
> + struct btintel_data *intel = hci_get_priv(hdev);
> + const struct intel_secure_send_result *evt = ptr;
> +
> + if (len != sizeof(*evt))
> + return;
> +
> + if (evt->result)
> + set_bit(INTEL_FIRMWARE_FAILED, &intel->flags);
> +
> + if (test_and_clear_bit(INTEL_DOWNLOADING, &intel->flags) &&
> + test_bit(INTEL_FIRMWARE_LOADED, &intel->flags))
> + wake_up_bit(&intel->flags, INTEL_DOWNLOADING);
> +}
> +EXPORT_SYMBOL_GPL(btintel_secure_send_result);
> +
> 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 abc438b9c62e..f02ccd7e76fb 100644
> --- a/drivers/bluetooth/btintel.h
> +++ b/drivers/bluetooth/btintel.h
> @@ -191,6 +191,9 @@ 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);
> +void btintel_bootup(struct hci_dev *hdev, const void *ptr, unsigned int len);
> +void btintel_secure_send_result(struct hci_dev *hdev,
> + const void *ptr, unsigned int len);
> #else
>
> static inline int btintel_check_bdaddr(struct hci_dev *hdev)
> @@ -337,4 +340,13 @@ static inline int btintel_set_debug_features(struct hci_dev *hdev,
> return -EOPNOTSUPP;
> }
>
> +static inline void btintel_bootup(struct hci_dev *hdev,
> + const void *ptr, unsigned int len)
> +{
> +}
> +
> +static inline void btintel_secure_send_result(struct hci_dev *hdev,
> + const void *ptr, unsigned int len)
> +{
> +}
> #endif
> diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
> index 01ec2a2d0a1a..4b1e6504f187 100644
> --- a/drivers/bluetooth/btusb.c
> +++ b/drivers/bluetooth/btusb.c
> @@ -365,12 +365,12 @@ static const struct usb_device_id blacklist_table[] = {
> { USB_DEVICE(0x1286, 0x204e), .driver_info = BTUSB_MARVELL },
>
> /* Intel Bluetooth devices */
> - { USB_DEVICE(0x8087, 0x0025), .driver_info = BTUSB_INTEL_NEW |
> + { USB_DEVICE(0x8087, 0x0025), .driver_info = BTUSB_INTEL_COMBINED |
> BTUSB_WIDEBAND_SPEECH |
> BTUSB_VALID_LE_STATES },
> - { USB_DEVICE(0x8087, 0x0026), .driver_info = BTUSB_INTEL_NEW |
> + { USB_DEVICE(0x8087, 0x0026), .driver_info = BTUSB_INTEL_COMBINED |
> BTUSB_WIDEBAND_SPEECH },
> - { USB_DEVICE(0x8087, 0x0029), .driver_info = BTUSB_INTEL_NEW |
> + { USB_DEVICE(0x8087, 0x0029), .driver_info = BTUSB_INTEL_COMBINED |
> BTUSB_WIDEBAND_SPEECH },
> { USB_DEVICE(0x8087, 0x0032), .driver_info = BTUSB_INTEL_NEWGEN |
> BTUSB_WIDEBAND_SPEECH},
> @@ -384,13 +384,13 @@ static const struct usb_device_id blacklist_table[] = {
> { USB_DEVICE(0x8087, 0x0a2a), .driver_info = BTUSB_INTEL_COMBINED |
> BTUSB_INTEL_BROKEN_READ_VERSION |
> BTUSB_INTEL_BROKEN_LED },
> - { USB_DEVICE(0x8087, 0x0a2b), .driver_info = BTUSB_INTEL_NEW |
> + { USB_DEVICE(0x8087, 0x0a2b), .driver_info = BTUSB_INTEL_COMBINED |
> BTUSB_WIDEBAND_SPEECH },
> { USB_DEVICE(0x8087, 0x0aa7), .driver_info = BTUSB_INTEL_COMBINED |
> BTUSB_INTEL_BROKEN_READ_VERSION |
> BTUSB_INTEL_BROKEN_LED |
> BTUSB_WIDEBAND_SPEECH },
> - { USB_DEVICE(0x8087, 0x0aaa), .driver_info = BTUSB_INTEL_NEW |
> + { USB_DEVICE(0x8087, 0x0aaa), .driver_info = BTUSB_INTEL_COMBINED |
> BTUSB_WIDEBAND_SPEECH |
> BTUSB_VALID_LE_STATES },
while this has nothing to do with this patch, however we need to get rid of the other BTUSB_ flags here. Here we should only have BTUSB_INTEL_COMBINED and something that we need to know before we send the HCI_Intel_Read_Version command. Everything else should be handled internally in btintel.c vendor functions.
So the setup routine should set HCI_QUIRK_WIDEBAND_SPEECH_SUPPORTED and HCI_QUIRK_VALID_LE_STATES if supported. Same as with MSFT extensions and anything else we might want to support.
My wish is that you add a new VID/PID here with BTUSB_INTEL_COMBINED and then only modify one place in btintel.c to add the hw_variant to the list. I don’t want to add flags and features in different locations.
Regards
Marcel
^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH v5 09/11] Bluetooth: btintel: Refactoring setup routine for TLV based booloader
2021-07-29 18:35 [PATCH v5 00/11] Bluetooth: btintel: Refactoring setup routines Tedd Ho-Jeong An
` (7 preceding siblings ...)
2021-07-29 18:35 ` [PATCH v5 08/11] Bluetooth: btintel: Refactoring setup routine for legacy bootloader Tedd Ho-Jeong An
@ 2021-07-29 18:35 ` Tedd Ho-Jeong An
2021-07-29 18:35 ` [PATCH v5 10/11] Bluetooth: btintel: Clean the exported function to static Tedd Ho-Jeong An
2021-07-29 18:36 ` [PATCH v5 11/11] Bluetooth: btintel: Fix the legacy bootloader returns tlv based version Tedd Ho-Jeong An
10 siblings, 0 replies; 26+ messages in thread
From: Tedd Ho-Jeong An @ 2021-07-29 18:35 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Tedd Ho-Jeong An
From: Tedd Ho-Jeong An <tedd.an@intel.com>
This patch refactors the setup routines for the TLV based bootloader
devices into combined setup, and move the related functions from btubs
to btintel.
Signed-off-by: Tedd Ho-Jeong An <tedd.an@intel.com>
---
drivers/bluetooth/btintel.c | 289 +++++++++++++++++--------
drivers/bluetooth/btintel.h | 13 --
drivers/bluetooth/btusb.c | 413 +-----------------------------------
3 files changed, 204 insertions(+), 511 deletions(-)
diff --git a/drivers/bluetooth/btintel.c b/drivers/bluetooth/btintel.c
index 944203cae8e0..dc5e2b255605 100644
--- a/drivers/bluetooth/btintel.c
+++ b/drivers/bluetooth/btintel.c
@@ -623,90 +623,7 @@ int btintel_read_version_tlv(struct hci_dev *hdev, struct intel_version_tlv *ver
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
- * to keep the existing setup flow
- */
- while (skb->len) {
- struct intel_tlv *tlv;
-
- tlv = (struct intel_tlv *)skb->data;
- 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));
- }
+ btintel_parse_version_tlv(hdev, version, skb);
kfree_skb(skb);
return 0;
@@ -1184,10 +1101,10 @@ int btintel_download_firmware(struct hci_dev *hdev,
}
EXPORT_SYMBOL_GPL(btintel_download_firmware);
-int btintel_download_firmware_newgen(struct hci_dev *hdev,
- struct intel_version_tlv *ver,
- const struct firmware *fw, u32 *boot_param,
- u8 hw_variant, u8 sbe_type)
+static int btintel_download_fw_tlv(struct hci_dev *hdev,
+ struct intel_version_tlv *ver,
+ const struct firmware *fw, u32 *boot_param,
+ u8 hw_variant, u8 sbe_type)
{
int err;
u32 css_header_ver;
@@ -1284,7 +1201,6 @@ int btintel_download_firmware_newgen(struct hci_dev *hdev,
}
return 0;
}
-EXPORT_SYMBOL_GPL(btintel_download_firmware_newgen);
void btintel_reset_to_bootloader(struct hci_dev *hdev)
{
@@ -2061,6 +1977,199 @@ static int btintel_bootloader_setup(struct hci_dev *hdev,
return 0;
}
+static void btintel_get_fw_name_tlv(const struct intel_version_tlv *ver,
+ char *fw_name, size_t len,
+ const char *suffix)
+{
+ /* The firmware file name for new generation controllers will be
+ * ibt-<cnvi_top type+cnvi_top step>-<cnvr_top type+cnvr_top step>
+ */
+ snprintf(fw_name, len, "intel/ibt-%04x-%04x.%s",
+ INTEL_CNVX_TOP_PACK_SWAB(INTEL_CNVX_TOP_TYPE(ver->cnvi_top),
+ INTEL_CNVX_TOP_STEP(ver->cnvi_top)),
+ INTEL_CNVX_TOP_PACK_SWAB(INTEL_CNVX_TOP_TYPE(ver->cnvr_top),
+ INTEL_CNVX_TOP_STEP(ver->cnvr_top)),
+ suffix);
+}
+
+static int btintel_prepare_fw_download_tlv(struct hci_dev *hdev,
+ struct intel_version_tlv *ver,
+ u32 *boot_param)
+{
+ struct btintel_data *intel = hci_get_priv(hdev);
+ const struct firmware *fw;
+ char fwname[64];
+ int err;
+ ktime_t calltime;
+
+ if (!ver || !boot_param)
+ return -EINVAL;
+
+ /* The firmware variant determines if the device is in bootloader
+ * mode or is running operational firmware. The value 0x03 identifies
+ * the bootloader and the value 0x23 identifies the operational
+ * firmware.
+ *
+ * When the operational firmware is already present, then only
+ * the check for valid Bluetooth device address is needed. This
+ * determines if the device will be added as configured or
+ * unconfigured controller.
+ *
+ * It is not possible to use the Secure Boot Parameters in this
+ * case since that command is only available in bootloader mode.
+ */
+ if (ver->img_type == 0x03) {
+ clear_bit(INTEL_BOOTLOADER, &intel->flags);
+ btintel_check_bdaddr(hdev);
+ }
+
+ /* If the OTP has no valid Bluetooth device address, then there will
+ * also be no valid address for the operational firmware.
+ */
+ if (!bacmp(&ver->otp_bd_addr, BDADDR_ANY)) {
+ bt_dev_info(hdev, "No device address configured");
+ set_bit(HCI_QUIRK_INVALID_BDADDR, &hdev->quirks);
+ }
+
+ btintel_get_fw_name_tlv(ver, fwname, sizeof(fwname), "sfi");
+ err = firmware_request_nowarn(&fw, fwname, &hdev->dev);
+ if (err < 0) {
+ if (!test_bit(INTEL_BOOTLOADER, &intel->flags)) {
+ /* Firmware has already been loaded */
+ set_bit(INTEL_FIRMWARE_LOADED, &intel->flags);
+ return 0;
+ }
+
+ bt_dev_err(hdev, "Failed to load Intel firmware file %s (%d)",
+ fwname, err);
+
+ return err;
+ }
+
+ bt_dev_info(hdev, "Found device firmware: %s", fwname);
+
+ if (fw->size < 644) {
+ bt_dev_err(hdev, "Invalid size of firmware file (%zu)",
+ fw->size);
+ err = -EBADF;
+ goto done;
+ }
+
+ calltime = ktime_get();
+
+ set_bit(INTEL_DOWNLOADING, &intel->flags);
+
+ /* Start firmware downloading and get boot parameter */
+ err = btintel_download_fw_tlv(hdev, ver, fw, boot_param,
+ INTEL_HW_VARIANT(ver->cnvi_bt),
+ ver->sbe_type);
+ if (err < 0) {
+ if (err == -EALREADY) {
+ /* Firmware has already been loaded */
+ set_bit(INTEL_FIRMWARE_LOADED, &intel->flags);
+ err = 0;
+ goto done;
+ }
+
+ /* When FW download fails, send Intel Reset to retry
+ * FW download.
+ */
+ btintel_reset_to_bootloader(hdev);
+ goto done;
+ }
+
+ /* Before switching the device into operational mode and with that
+ * booting the loaded firmware, wait for the bootloader notification
+ * that all fragments have been successfully received.
+ *
+ * When the event processing receives the notification, then the
+ * BTUSB_DOWNLOADING flag will be cleared.
+ *
+ * The firmware loading should not take longer than 5 seconds
+ * and thus just timeout if that happens and fail the setup
+ * of this device.
+ */
+ err = btintel_download_wait(hdev, calltime, 5000);
+ if (err == -ETIMEDOUT)
+ btintel_reset_to_bootloader(hdev);
+
+done:
+ release_firmware(fw);
+ return err;
+}
+
+static int btintel_bootloader_setup_tlv(struct hci_dev *hdev,
+ struct intel_version_tlv *ver)
+{
+ struct btintel_data *intel = hci_get_priv(hdev);
+ u32 boot_param;
+ char ddcname[64];
+ int err;
+ struct intel_debug_features features;
+ struct intel_version_tlv new_ver;
+
+ bt_dev_dbg(hdev, "");
+
+ /* Set the default boot parameter to 0x0 and it is updated to
+ * SKU specific boot parameter after reading Intel_Write_Boot_Params
+ * command while downloading the firmware.
+ */
+ boot_param = 0x00000000;
+
+ set_bit(INTEL_BOOTLOADER, &intel->flags);
+
+ err = btintel_prepare_fw_download_tlv(hdev, ver, &boot_param);
+ if (err)
+ return err;
+
+ /* check if controller is already having an operational firmware */
+ if (ver->img_type == 0x03)
+ goto finish;
+
+ err = btintel_boot(hdev, boot_param);
+ if (err)
+ return err;
+
+ clear_bit(INTEL_BOOTLOADER, &intel->flags);
+
+ btintel_get_fw_name_tlv(ver, ddcname, sizeof(ddcname), "ddc");
+ /* Once the device is running in operational mode, it needs to
+ * apply the device configuration (DDC) parameters.
+ *
+ * The device can work without DDC parameters, so even if it
+ * fails to load the file, no need to fail the setup.
+ */
+ btintel_load_ddc_config(hdev, ddcname);
+
+ /* Read the Intel supported features and if new exception formats
+ * supported, need to load the additional DDC config to enable.
+ */
+ err = btintel_read_debug_features(hdev, &features);
+ if (!err) {
+ /* Set DDC mask for available debug features */
+ btintel_set_debug_features(hdev, &features);
+ }
+
+ /* Read the Intel version information after loading the FW */
+ err = btintel_read_version_tlv(hdev, &new_ver);
+ if (err)
+ return err;
+
+ btintel_version_info_tlv(hdev, &new_ver);
+
+finish:
+ /* Set the event mask for Intel specific vendor events. This enables
+ * a few extra events that are useful during general operation. It
+ * does not enable any debugging related events.
+ *
+ * The device will function correctly without these events enabled
+ * and thus no need to fail the setup.
+ */
+ btintel_set_event_mask(hdev, false);
+
+ return 0;
+}
+
int btintel_setup_combined(struct hci_dev *hdev)
{
struct btintel_data *intel = hci_get_priv(hdev);
@@ -2154,7 +2263,7 @@ int btintel_setup_combined(struct hci_dev *hdev)
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 */
+ err = btintel_bootloader_setup_tlv(hdev, &ver_tlv);
return err;
}
diff --git a/drivers/bluetooth/btintel.h b/drivers/bluetooth/btintel.h
index f02ccd7e76fb..696656fd7e99 100644
--- a/drivers/bluetooth/btintel.h
+++ b/drivers/bluetooth/btintel.h
@@ -181,11 +181,6 @@ int btintel_download_firmware(struct hci_dev *dev, struct intel_version *ver,
int btintel_setup_combined(struct hci_dev *hdev);
int btintel_shutdown_combined(struct hci_dev *hdev);
void btintel_set_flags(struct hci_dev *hdev, unsigned int flag);
-int btintel_download_firmware_newgen(struct hci_dev *hdev,
- struct intel_version_tlv *ver,
- const struct firmware *fw,
- u32 *boot_param, u8 hw_variant,
- u8 sbe_type);
void btintel_reset_to_bootloader(struct hci_dev *hdev);
int btintel_read_debug_features(struct hci_dev *hdev,
struct intel_debug_features *features);
@@ -316,14 +311,6 @@ static inline void btintel_set_flags(struct hci_dev *hdev, unsigned int flag)
{
}
-static inline int btintel_download_firmware_newgen(struct hci_dev *hdev,
- const struct firmware *fw,
- u32 *boot_param,
- u8 hw_variant, u8 sbe_type)
-{
- return -EOPNOTSUPP;
-}
-
static inline void btintel_reset_to_bootloader(struct hci_dev *hdev)
{
}
diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
index 4b1e6504f187..1e82fbad162d 100644
--- a/drivers/bluetooth/btusb.c
+++ b/drivers/bluetooth/btusb.c
@@ -372,9 +372,9 @@ static const struct usb_device_id blacklist_table[] = {
BTUSB_WIDEBAND_SPEECH },
{ USB_DEVICE(0x8087, 0x0029), .driver_info = BTUSB_INTEL_COMBINED |
BTUSB_WIDEBAND_SPEECH },
- { USB_DEVICE(0x8087, 0x0032), .driver_info = BTUSB_INTEL_NEWGEN |
+ { USB_DEVICE(0x8087, 0x0032), .driver_info = BTUSB_INTEL_COMBINED |
BTUSB_WIDEBAND_SPEECH},
- { USB_DEVICE(0x8087, 0x0033), .driver_info = BTUSB_INTEL_NEWGEN |
+ { USB_DEVICE(0x8087, 0x0033), .driver_info = BTUSB_INTEL_COMBINED |
BTUSB_WIDEBAND_SPEECH |
BTUSB_VALID_LE_STATES },
{ USB_DEVICE(0x8087, 0x07da), .driver_info = BTUSB_CSR },
@@ -2005,74 +2005,16 @@ static int btusb_recv_bulk_intel(struct btusb_data *data, void *buffer,
* events via the bulk endpoint. These events are treated the
* same way as the ones received from the interrupt endpoint.
*/
- if (test_bit(BTUSB_BOOTLOADER, &data->flags) ||
- test_bit(INTEL_BOOTLOADER, &intel->flags))
+ if (test_bit(INTEL_BOOTLOADER, &intel->flags))
return btusb_recv_intr(data, buffer, count);
return btusb_recv_bulk(data, buffer, count);
}
-static void btusb_intel_bootup(struct btusb_data *data, const void *ptr,
- unsigned int len)
-{
- const struct intel_bootup *evt = ptr;
-
- if (len != sizeof(*evt))
- return;
-
- if (test_and_clear_bit(BTUSB_BOOTING, &data->flags))
- wake_up_bit(&data->flags, BTUSB_BOOTING);
-}
-
-static void btusb_intel_secure_send_result(struct btusb_data *data,
- const void *ptr, unsigned int len)
-{
- const struct intel_secure_send_result *evt = ptr;
-
- if (len != sizeof(*evt))
- return;
-
- if (evt->result)
- set_bit(BTUSB_FIRMWARE_FAILED, &data->flags);
-
- if (test_and_clear_bit(BTUSB_DOWNLOADING, &data->flags) &&
- test_bit(BTUSB_FIRMWARE_LOADED, &data->flags))
- wake_up_bit(&data->flags, BTUSB_DOWNLOADING);
-}
-
static int btusb_recv_event_intel(struct hci_dev *hdev, struct sk_buff *skb)
{
- struct btusb_data *data = hci_get_drvdata(hdev);
struct btintel_data *intel = hci_get_priv(hdev);
- if (test_bit(BTUSB_BOOTLOADER, &data->flags)) {
- struct hci_event_hdr *hdr = (void *)skb->data;
-
- if (skb->len > HCI_EVENT_HDR_SIZE && hdr->evt == 0xff &&
- hdr->plen > 0) {
- const void *ptr = skb->data + HCI_EVENT_HDR_SIZE + 1;
- unsigned int len = skb->len - HCI_EVENT_HDR_SIZE - 1;
-
- switch (skb->data[2]) {
- case 0x02:
- /* When switching to the operational firmware
- * the device sends a vendor specific event
- * indicating that the bootup completed.
- */
- btusb_intel_bootup(data, ptr, len);
- break;
- case 0x06:
- /* When the firmware loading completes the
- * device sends out a vendor specific event
- * indicating the result of the firmware
- * loading.
- */
- btusb_intel_secure_send_result(data, ptr, len);
- break;
- }
- }
- }
-
if (test_bit(INTEL_BOOTLOADER, &intel->flags)) {
struct hci_event_hdr *hdr = (void *)skb->data;
@@ -2106,7 +2048,6 @@ static int btusb_recv_event_intel(struct hci_dev *hdev, struct sk_buff *skb)
static int btusb_send_frame_intel(struct hci_dev *hdev, struct sk_buff *skb)
{
- struct btusb_data *data = hci_get_drvdata(hdev);
struct btintel_data *intel = hci_get_priv(hdev);
struct urb *urb;
@@ -2114,8 +2055,7 @@ static int btusb_send_frame_intel(struct hci_dev *hdev, struct sk_buff *skb)
switch (hci_skb_pkt_type(skb)) {
case HCI_COMMAND_PKT:
- if (test_bit(BTUSB_BOOTLOADER, &data->flags) ||
- test_bit(INTEL_BOOTLOADER, &intel->flags)) {
+ if (test_bit(INTEL_BOOTLOADER, &intel->flags)) {
struct hci_command_hdr *cmd = (void *)skb->data;
__u16 opcode = le16_to_cpu(cmd->opcode);
@@ -2167,330 +2107,6 @@ static int btusb_send_frame_intel(struct hci_dev *hdev, struct sk_buff *skb)
return -EILSEQ;
}
-static void btusb_setup_intel_newgen_get_fw_name(const struct intel_version_tlv *ver_tlv,
- char *fw_name, size_t len,
- const char *suffix)
-{
- /* The firmware file name for new generation controllers will be
- * ibt-<cnvi_top type+cnvi_top step>-<cnvr_top type+cnvr_top step>
- */
- snprintf(fw_name, len, "intel/ibt-%04x-%04x.%s",
- INTEL_CNVX_TOP_PACK_SWAB(INTEL_CNVX_TOP_TYPE(ver_tlv->cnvi_top),
- INTEL_CNVX_TOP_STEP(ver_tlv->cnvi_top)),
- INTEL_CNVX_TOP_PACK_SWAB(INTEL_CNVX_TOP_TYPE(ver_tlv->cnvr_top),
- INTEL_CNVX_TOP_STEP(ver_tlv->cnvr_top)),
- suffix);
-}
-
-static int btusb_download_wait(struct hci_dev *hdev, ktime_t calltime, int msec)
-{
- struct btusb_data *data = hci_get_drvdata(hdev);
- ktime_t delta, rettime;
- unsigned long long duration;
- int err;
-
- set_bit(BTUSB_FIRMWARE_LOADED, &data->flags);
-
- bt_dev_info(hdev, "Waiting for firmware download to complete");
-
- err = wait_on_bit_timeout(&data->flags, BTUSB_DOWNLOADING,
- TASK_INTERRUPTIBLE,
- msecs_to_jiffies(msec));
- if (err == -EINTR) {
- bt_dev_err(hdev, "Firmware loading interrupted");
- return err;
- }
-
- if (err) {
- bt_dev_err(hdev, "Firmware loading timeout");
- return -ETIMEDOUT;
- }
-
- if (test_bit(BTUSB_FIRMWARE_FAILED, &data->flags)) {
- bt_dev_err(hdev, "Firmware loading failed");
- return -ENOEXEC;
- }
-
- rettime = ktime_get();
- delta = ktime_sub(rettime, calltime);
- duration = (unsigned long long)ktime_to_ns(delta) >> 10;
-
- bt_dev_info(hdev, "Firmware loaded in %llu usecs", duration);
-
- return 0;
-}
-
-static int btusb_intel_download_firmware_newgen(struct hci_dev *hdev,
- struct intel_version_tlv *ver,
- u32 *boot_param)
-{
- const struct firmware *fw;
- char fwname[64];
- int err;
- struct btusb_data *data = hci_get_drvdata(hdev);
- ktime_t calltime;
-
- if (!ver || !boot_param)
- return -EINVAL;
-
- /* The firmware variant determines if the device is in bootloader
- * mode or is running operational firmware. The value 0x03 identifies
- * the bootloader and the value 0x23 identifies the operational
- * firmware.
- *
- * When the operational firmware is already present, then only
- * the check for valid Bluetooth device address is needed. This
- * determines if the device will be added as configured or
- * unconfigured controller.
- *
- * It is not possible to use the Secure Boot Parameters in this
- * case since that command is only available in bootloader mode.
- */
- if (ver->img_type == 0x03) {
- clear_bit(BTUSB_BOOTLOADER, &data->flags);
- btintel_check_bdaddr(hdev);
- }
-
- /* If the OTP has no valid Bluetooth device address, then there will
- * also be no valid address for the operational firmware.
- */
- if (!bacmp(&ver->otp_bd_addr, BDADDR_ANY)) {
- bt_dev_info(hdev, "No device address configured");
- set_bit(HCI_QUIRK_INVALID_BDADDR, &hdev->quirks);
- }
-
- btusb_setup_intel_newgen_get_fw_name(ver, fwname, sizeof(fwname), "sfi");
- err = firmware_request_nowarn(&fw, fwname, &hdev->dev);
- if (err < 0) {
- if (!test_bit(BTUSB_BOOTLOADER, &data->flags)) {
- /* Firmware has already been loaded */
- set_bit(BTUSB_FIRMWARE_LOADED, &data->flags);
- return 0;
- }
-
- bt_dev_err(hdev, "Failed to load Intel firmware file %s (%d)",
- fwname, err);
-
- return err;
- }
-
- bt_dev_info(hdev, "Found device firmware: %s", fwname);
-
- if (fw->size < 644) {
- bt_dev_err(hdev, "Invalid size of firmware file (%zu)",
- fw->size);
- err = -EBADF;
- goto done;
- }
-
- calltime = ktime_get();
-
- set_bit(BTUSB_DOWNLOADING, &data->flags);
-
- /* Start firmware downloading and get boot parameter */
- err = btintel_download_firmware_newgen(hdev, ver, fw, boot_param,
- INTEL_HW_VARIANT(ver->cnvi_bt),
- ver->sbe_type);
- if (err < 0) {
- if (err == -EALREADY) {
- /* Firmware has already been loaded */
- set_bit(BTUSB_FIRMWARE_LOADED, &data->flags);
- err = 0;
- goto done;
- }
-
- /* When FW download fails, send Intel Reset to retry
- * FW download.
- */
- btintel_reset_to_bootloader(hdev);
- goto done;
- }
-
- /* Before switching the device into operational mode and with that
- * booting the loaded firmware, wait for the bootloader notification
- * that all fragments have been successfully received.
- *
- * When the event processing receives the notification, then the
- * BTUSB_DOWNLOADING flag will be cleared.
- *
- * The firmware loading should not take longer than 5 seconds
- * and thus just timeout if that happens and fail the setup
- * of this device.
- */
- err = btusb_download_wait(hdev, calltime, 5000);
- if (err == -ETIMEDOUT)
- btintel_reset_to_bootloader(hdev);
-
-done:
- release_firmware(fw);
- return err;
-}
-
-static int btusb_boot_wait(struct hci_dev *hdev, ktime_t calltime, int msec)
-{
- struct btusb_data *data = hci_get_drvdata(hdev);
- ktime_t delta, rettime;
- unsigned long long duration;
- int err;
-
- bt_dev_info(hdev, "Waiting for device to boot");
-
- err = wait_on_bit_timeout(&data->flags, BTUSB_BOOTING,
- TASK_INTERRUPTIBLE,
- msecs_to_jiffies(msec));
- if (err == -EINTR) {
- bt_dev_err(hdev, "Device boot interrupted");
- return -EINTR;
- }
-
- if (err) {
- bt_dev_err(hdev, "Device boot timeout");
- return -ETIMEDOUT;
- }
-
- rettime = ktime_get();
- delta = ktime_sub(rettime, calltime);
- duration = (unsigned long long) ktime_to_ns(delta) >> 10;
-
- bt_dev_info(hdev, "Device booted in %llu usecs", duration);
-
- return 0;
-}
-
-static int btusb_intel_boot(struct hci_dev *hdev, u32 boot_addr)
-{
- struct btusb_data *data = hci_get_drvdata(hdev);
- ktime_t calltime;
- int err;
-
- calltime = ktime_get();
-
- set_bit(BTUSB_BOOTING, &data->flags);
-
- err = btintel_send_intel_reset(hdev, boot_addr);
- if (err) {
- bt_dev_err(hdev, "Intel Soft Reset failed (%d)", err);
- btintel_reset_to_bootloader(hdev);
- return err;
- }
-
- /* The bootloader will not indicate when the device is ready. This
- * is done by the operational firmware sending bootup notification.
- *
- * Booting into operational firmware should not take longer than
- * 1 second. However if that happens, then just fail the setup
- * since something went wrong.
- */
- err = btusb_boot_wait(hdev, calltime, 1000);
- if (err == -ETIMEDOUT)
- btintel_reset_to_bootloader(hdev);
-
- return err;
-}
-
-static int btusb_setup_intel_newgen(struct hci_dev *hdev)
-{
- struct btusb_data *data = hci_get_drvdata(hdev);
- u32 boot_param;
- char ddcname[64];
- int err;
- struct intel_debug_features features;
- struct intel_version_tlv version;
-
- bt_dev_dbg(hdev, "");
-
- /* Set the default boot parameter to 0x0 and it is updated to
- * SKU specific boot parameter after reading Intel_Write_Boot_Params
- * command while downloading the firmware.
- */
- boot_param = 0x00000000;
-
- /* Read the Intel version information to determine if the device
- * is in bootloader mode or if it already has operational firmware
- * loaded.
- */
- err = btintel_read_version_tlv(hdev, &version);
- if (err) {
- bt_dev_err(hdev, "Intel Read version failed (%d)", err);
- btintel_reset_to_bootloader(hdev);
- return err;
- }
-
- err = btintel_version_info_tlv(hdev, &version);
- if (err)
- return err;
-
- err = btusb_intel_download_firmware_newgen(hdev, &version, &boot_param);
- if (err)
- return err;
-
- /* check if controller is already having an operational firmware */
- if (version.img_type == 0x03)
- goto finish;
-
- err = btusb_intel_boot(hdev, boot_param);
- if (err)
- return err;
-
- clear_bit(BTUSB_BOOTLOADER, &data->flags);
-
- btusb_setup_intel_newgen_get_fw_name(&version, ddcname, sizeof(ddcname),
- "ddc");
- /* Once the device is running in operational mode, it needs to
- * apply the device configuration (DDC) parameters.
- *
- * The device can work without DDC parameters, so even if it
- * fails to load the file, no need to fail the setup.
- */
- btintel_load_ddc_config(hdev, ddcname);
-
- /* Read the Intel supported features and if new exception formats
- * supported, need to load the additional DDC config to enable.
- */
- err = btintel_read_debug_features(hdev, &features);
- if (!err) {
- /* Set DDC mask for available debug features */
- btintel_set_debug_features(hdev, &features);
- }
-
- /* Read the Intel version information after loading the FW */
- err = btintel_read_version_tlv(hdev, &version);
- if (err)
- return err;
-
- btintel_version_info_tlv(hdev, &version);
-
-finish:
- /* Set the event mask for Intel specific vendor events. This enables
- * a few extra events that are useful during general operation. It
- * does not enable any debugging related events.
- *
- * The device will function correctly without these events enabled
- * and thus no need to fail the setup.
- */
- btintel_set_event_mask(hdev, false);
-
- return 0;
-}
-
-static int btusb_shutdown_intel_new(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;
-}
-
#define FIRMWARE_MT7663 "mediatek/mt7663pr2h.bin"
#define FIRMWARE_MT7668 "mediatek/mt7668pr2h.bin"
@@ -3966,8 +3582,7 @@ static int btusb_probe(struct usb_interface *intf,
data->recv_event = hci_recv_frame;
data->recv_bulk = btusb_recv_bulk;
- if (id->driver_info & BTUSB_INTEL_COMBINED ||
- id->driver_info & BTUSB_INTEL_NEW) {
+ if (id->driver_info & BTUSB_INTEL_COMBINED) {
/* Allocate extra space for Intel device */
priv_size += sizeof(struct btintel_data);
@@ -4071,24 +3686,6 @@ static int btusb_probe(struct usb_interface *intf,
btintel_set_flags(hdev, INTEL_BROKEN_LED);
}
- if (id->driver_info & BTUSB_INTEL_NEWGEN) {
- hdev->manufacturer = 2;
- hdev->send = btusb_send_frame_intel;
- hdev->setup = btusb_setup_intel_newgen;
- hdev->shutdown = btusb_shutdown_intel_new;
- hdev->hw_error = btintel_hw_error;
- hdev->set_diag = btintel_set_diag;
- hdev->set_bdaddr = btintel_set_bdaddr;
- hdev->cmd_timeout = btusb_intel_cmd_timeout;
- set_bit(HCI_QUIRK_STRICT_DUPLICATE_FILTER, &hdev->quirks);
- set_bit(HCI_QUIRK_SIMULTANEOUS_DISCOVERY, &hdev->quirks);
- set_bit(HCI_QUIRK_NON_PERSISTENT_DIAG, &hdev->quirks);
-
- data->recv_event = btusb_recv_event_intel;
- data->recv_bulk = btusb_recv_bulk_intel;
- set_bit(BTUSB_BOOTLOADER, &data->flags);
- }
-
if (id->driver_info & BTUSB_MARVELL)
hdev->set_bdaddr = btusb_set_bdaddr_marvell;
--
2.25.1
^ permalink raw reply related [flat|nested] 26+ messages in thread* [PATCH v5 10/11] Bluetooth: btintel: Clean the exported function to static
2021-07-29 18:35 [PATCH v5 00/11] Bluetooth: btintel: Refactoring setup routines Tedd Ho-Jeong An
` (8 preceding siblings ...)
2021-07-29 18:35 ` [PATCH v5 09/11] Bluetooth: btintel: Refactoring setup routine for TLV based booloader Tedd Ho-Jeong An
@ 2021-07-29 18:35 ` Tedd Ho-Jeong An
2021-07-29 18:36 ` [PATCH v5 11/11] Bluetooth: btintel: Fix the legacy bootloader returns tlv based version Tedd Ho-Jeong An
10 siblings, 0 replies; 26+ messages in thread
From: Tedd Ho-Jeong An @ 2021-07-29 18:35 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Tedd Ho-Jeong An
From: Tedd Ho-Jeong An <tedd.an@intel.com>
This patch changes the exported functions to static if they are no
longer used by others.
Signed-off-by: Tedd Ho-Jeong An <tedd.an@intel.com>
---
drivers/bluetooth/btintel.c | 65 +++++++++++++++++--------------------
drivers/bluetooth/btintel.h | 50 ----------------------------
2 files changed, 30 insertions(+), 85 deletions(-)
diff --git a/drivers/bluetooth/btintel.c b/drivers/bluetooth/btintel.c
index dc5e2b255605..57b64d744f0a 100644
--- a/drivers/bluetooth/btintel.c
+++ b/drivers/bluetooth/btintel.c
@@ -131,6 +131,26 @@ int btintel_set_bdaddr(struct hci_dev *hdev, const bdaddr_t *bdaddr)
}
EXPORT_SYMBOL_GPL(btintel_set_bdaddr);
+static int btintel_set_event_mask(struct hci_dev *hdev, bool debug)
+{
+ u8 mask[8] = { 0x87, 0x0c, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00 };
+ struct sk_buff *skb;
+ int err;
+
+ if (debug)
+ mask[1] |= 0x62;
+
+ skb = __hci_cmd_sync(hdev, 0xfc52, 8, mask, HCI_INIT_TIMEOUT);
+ if (IS_ERR(skb)) {
+ err = PTR_ERR(skb);
+ bt_dev_err(hdev, "Setting Intel event mask failed (%d)", err);
+ return err;
+ }
+ kfree_skb(skb);
+
+ return 0;
+}
+
int btintel_set_diag(struct hci_dev *hdev, bool enable)
{
struct sk_buff *skb;
@@ -296,8 +316,8 @@ int btintel_version_info(struct hci_dev *hdev, struct intel_version *ver)
}
EXPORT_SYMBOL_GPL(btintel_version_info);
-int btintel_secure_send(struct hci_dev *hdev, u8 fragment_type, u32 plen,
- const void *param)
+static int btintel_secure_send(struct hci_dev *hdev, u8 fragment_type, u32 plen,
+ const void *param)
{
while (plen > 0) {
struct sk_buff *skb;
@@ -319,7 +339,6 @@ int btintel_secure_send(struct hci_dev *hdev, u8 fragment_type, u32 plen,
return 0;
}
-EXPORT_SYMBOL_GPL(btintel_secure_send);
int btintel_load_ddc_config(struct hci_dev *hdev, const char *ddc_name)
{
@@ -366,27 +385,6 @@ int btintel_load_ddc_config(struct hci_dev *hdev, const char *ddc_name)
}
EXPORT_SYMBOL_GPL(btintel_load_ddc_config);
-int btintel_set_event_mask(struct hci_dev *hdev, bool debug)
-{
- u8 mask[8] = { 0x87, 0x0c, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00 };
- struct sk_buff *skb;
- int err;
-
- if (debug)
- mask[1] |= 0x62;
-
- skb = __hci_cmd_sync(hdev, 0xfc52, 8, mask, HCI_INIT_TIMEOUT);
- if (IS_ERR(skb)) {
- err = PTR_ERR(skb);
- bt_dev_err(hdev, "Setting Intel event mask failed (%d)", err);
- return err;
- }
- kfree_skb(skb);
-
- return 0;
-}
-EXPORT_SYMBOL_GPL(btintel_set_event_mask);
-
int btintel_set_event_mask_mfg(struct hci_dev *hdev, bool debug)
{
int err, ret;
@@ -430,7 +428,8 @@ int btintel_read_version(struct hci_dev *hdev, struct intel_version *ver)
}
EXPORT_SYMBOL_GPL(btintel_read_version);
-int btintel_version_info_tlv(struct hci_dev *hdev, struct intel_version_tlv *version)
+static int btintel_version_info_tlv(struct hci_dev *hdev,
+ struct intel_version_tlv *version)
{
const char *variant;
@@ -507,7 +506,6 @@ int btintel_version_info_tlv(struct hci_dev *hdev, struct intel_version_tlv *ver
return 0;
}
-EXPORT_SYMBOL_GPL(btintel_version_info_tlv);
static int btintel_parse_version_tlv(struct hci_dev *hdev,
struct intel_version_tlv *version,
@@ -601,7 +599,8 @@ static int btintel_parse_version_tlv(struct hci_dev *hdev,
return 0;
}
-int btintel_read_version_tlv(struct hci_dev *hdev, struct intel_version_tlv *version)
+static int btintel_read_version_tlv(struct hci_dev *hdev,
+ struct intel_version_tlv *version)
{
struct sk_buff *skb;
const u8 param[1] = { 0xFF };
@@ -628,7 +627,6 @@ int btintel_read_version_tlv(struct hci_dev *hdev, struct intel_version_tlv *ver
kfree_skb(skb);
return 0;
}
-EXPORT_SYMBOL_GPL(btintel_read_version_tlv);
/* ------- REGMAP IBT SUPPORT ------- */
@@ -1202,7 +1200,7 @@ static int btintel_download_fw_tlv(struct hci_dev *hdev,
return 0;
}
-void btintel_reset_to_bootloader(struct hci_dev *hdev)
+static void btintel_reset_to_bootloader(struct hci_dev *hdev)
{
struct intel_reset params;
struct sk_buff *skb;
@@ -1245,10 +1243,9 @@ void btintel_reset_to_bootloader(struct hci_dev *hdev)
*/
msleep(150);
}
-EXPORT_SYMBOL_GPL(btintel_reset_to_bootloader);
-int btintel_read_debug_features(struct hci_dev *hdev,
- struct intel_debug_features *features)
+static int btintel_read_debug_features(struct hci_dev *hdev,
+ struct intel_debug_features *features)
{
struct sk_buff *skb;
u8 page_no = 1;
@@ -1277,9 +1274,8 @@ int btintel_read_debug_features(struct hci_dev *hdev,
kfree_skb(skb);
return 0;
}
-EXPORT_SYMBOL_GPL(btintel_read_debug_features);
-int btintel_set_debug_features(struct hci_dev *hdev,
+static int btintel_set_debug_features(struct hci_dev *hdev,
const struct intel_debug_features *features)
{
u8 mask[11] = { 0x0a, 0x92, 0x02, 0x07, 0x00, 0x00, 0x00, 0x00,
@@ -1304,7 +1300,6 @@ int btintel_set_debug_features(struct hci_dev *hdev,
kfree_skb(skb);
return 0;
}
-EXPORT_SYMBOL_GPL(btintel_set_debug_features);
static const struct firmware *btintel_legacy_rom_get_fw(struct hci_dev *hdev,
struct intel_version *ver)
diff --git a/drivers/bluetooth/btintel.h b/drivers/bluetooth/btintel.h
index 696656fd7e99..71af25cd1672 100644
--- a/drivers/bluetooth/btintel.h
+++ b/drivers/bluetooth/btintel.h
@@ -162,15 +162,9 @@ int btintel_set_diag_combined(struct hci_dev *hdev, bool enable);
void btintel_hw_error(struct hci_dev *hdev, u8 code);
int btintel_version_info(struct hci_dev *hdev, struct intel_version *ver);
-int btintel_version_info_tlv(struct hci_dev *hdev, struct intel_version_tlv *version);
-int btintel_secure_send(struct hci_dev *hdev, u8 fragment_type, u32 plen,
- const void *param);
int btintel_load_ddc_config(struct hci_dev *hdev, const char *ddc_name);
-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);
-
struct regmap *btintel_regmap_init(struct hci_dev *hdev, u16 opcode_read,
u16 opcode_write);
int btintel_send_intel_reset(struct hci_dev *hdev, u32 boot_param);
@@ -181,11 +175,6 @@ int btintel_download_firmware(struct hci_dev *dev, struct intel_version *ver,
int btintel_setup_combined(struct hci_dev *hdev);
int btintel_shutdown_combined(struct hci_dev *hdev);
void btintel_set_flags(struct hci_dev *hdev, unsigned int flag);
-void btintel_reset_to_bootloader(struct hci_dev *hdev);
-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);
void btintel_bootup(struct hci_dev *hdev, const void *ptr, unsigned int len);
void btintel_secure_send_result(struct hci_dev *hdev,
const void *ptr, unsigned int len);
@@ -231,29 +220,12 @@ static inline int btintel_version_info(struct hci_dev *hdev,
return -EOPNOTSUPP;
}
-static inline int btintel_version_info_tlv(struct hci_dev *hdev,
- struct intel_version_tlv *version)
-{
- return -EOPNOTSUPP;
-}
-
-static inline int btintel_secure_send(struct hci_dev *hdev, u8 fragment_type,
- u32 plen, const void *param)
-{
- return -EOPNOTSUPP;
-}
-
static inline int btintel_load_ddc_config(struct hci_dev *hdev,
const char *ddc_name)
{
return -EOPNOTSUPP;
}
-static inline int btintel_set_event_mask(struct hci_dev *hdev, bool debug)
-{
- return -EOPNOTSUPP;
-}
-
static inline int btintel_set_event_mask_mfg(struct hci_dev *hdev, bool debug)
{
return -EOPNOTSUPP;
@@ -265,12 +237,6 @@ static inline int btintel_read_version(struct hci_dev *hdev,
return -EOPNOTSUPP;
}
-static inline int btintel_read_version_tlv(struct hci_dev *hdev,
- struct intel_version_tlv *ver)
-{
- return -EOPNOTSUPP;
-}
-
static inline struct regmap *btintel_regmap_init(struct hci_dev *hdev,
u16 opcode_read,
u16 opcode_write)
@@ -311,22 +277,6 @@ static inline void btintel_set_flags(struct hci_dev *hdev, unsigned int flag)
{
}
-static inline void btintel_reset_to_bootloader(struct hci_dev *hdev)
-{
-}
-
-static inline int btintel_read_debug_features(struct hci_dev *hdev,
- struct intel_debug_features *features)
-{
- return -EOPNOTSUPP;
-}
-
-static inline int btintel_set_debug_features(struct hci_dev *hdev,
- const struct intel_debug_features *features)
-{
- return -EOPNOTSUPP;
-}
-
static inline void btintel_bootup(struct hci_dev *hdev,
const void *ptr, unsigned int len)
{
--
2.25.1
^ permalink raw reply related [flat|nested] 26+ messages in thread* [PATCH v5 11/11] Bluetooth: btintel: Fix the legacy bootloader returns tlv based version
2021-07-29 18:35 [PATCH v5 00/11] Bluetooth: btintel: Refactoring setup routines Tedd Ho-Jeong An
` (9 preceding siblings ...)
2021-07-29 18:35 ` [PATCH v5 10/11] Bluetooth: btintel: Clean the exported function to static Tedd Ho-Jeong An
@ 2021-07-29 18:36 ` Tedd Ho-Jeong An
2021-07-29 19:40 ` Marcel Holtmann
10 siblings, 1 reply; 26+ messages in thread
From: Tedd Ho-Jeong An @ 2021-07-29 18:36 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Tedd Ho-Jeong An
From: Tedd Ho-Jeong An <tedd.an@intel.com>
From the ThP, it supports both old and TLV based HCI_Intel_Read_Version
command after downloading the operational firmware.
Starting from th JfP, it supports both old and TLV based
HCI_Intel_Read_Version command in the operational firmware and it causes
the setup() uses the TLV based setup instead of legacy setup.
So, as a workaround, this patch checks the fw variant from the TLV based
version and it uses the legacy HCI_Intel_Read_Version command to get the
legacy version information and run the legacy bootloader setup with it.
Signed-off-by: Tedd Ho-Jeong An <tedd.an@intel.com>
---
drivers/bluetooth/btintel.c | 25 ++++++++++++++++++++++++-
1 file changed, 24 insertions(+), 1 deletion(-)
diff --git a/drivers/bluetooth/btintel.c b/drivers/bluetooth/btintel.c
index 57b64d744f0a..f11882d10da7 100644
--- a/drivers/bluetooth/btintel.c
+++ b/drivers/bluetooth/btintel.c
@@ -2254,10 +2254,33 @@ int btintel_setup_combined(struct hci_dev *hdev)
/* For TLV type device, parse the tlv data */
btintel_parse_version_tlv(hdev, &ver_tlv, skb);
+ /* Some legacy bootloader devices from JfP supports both old and TLV
+ * based HCI_Intel_Read_Version command. But we don't want to use the
+ * TLV based setup routines for those old bootloader device.
+ * Also, it is not easy to convert TLV based version to the legacy
+ * version format.
+ *
+ * So, as a workaround for those devices, use the legacy
+ * HCI_Intel_Read_Version to get the version information and run the
+ * legacy bootloader setup.
+ */
+ switch (INTEL_HW_VARIANT(ver_tlv.cnvi_bt)) {
+ case 0x11: /* JfP */
+ case 0x12: /* ThP */
+ case 0x13: /* HrP */
+ case 0x14: /* CcP */
+ err = btintel_read_version(hdev, &ver);
+ if (err)
+ return err;
+ return btintel_bootloader_setup(hdev, &ver);
+ default:
+ /* Nothing to do here */
+ break;
+ }
+
/* Display version information of TLV type */
btintel_version_info_tlv(hdev, &ver_tlv);
- /* TODO: Need to filter the device for new generation */
err = btintel_bootloader_setup_tlv(hdev, &ver_tlv);
return err;
--
2.25.1
^ permalink raw reply related [flat|nested] 26+ messages in thread* Re: [PATCH v5 11/11] Bluetooth: btintel: Fix the legacy bootloader returns tlv based version
2021-07-29 18:36 ` [PATCH v5 11/11] Bluetooth: btintel: Fix the legacy bootloader returns tlv based version Tedd Ho-Jeong An
@ 2021-07-29 19:40 ` Marcel Holtmann
2021-07-29 20:02 ` Tedd Ho-Jeong An
0 siblings, 1 reply; 26+ messages in thread
From: Marcel Holtmann @ 2021-07-29 19:40 UTC (permalink / raw)
To: Tedd Ho-Jeong An; +Cc: linux-bluetooth, Tedd Ho-Jeong An
Hi Tedd,
> From the ThP, it supports both old and TLV based HCI_Intel_Read_Version
> command after downloading the operational firmware.
> Starting from th JfP, it supports both old and TLV based
> HCI_Intel_Read_Version command in the operational firmware and it causes
> the setup() uses the TLV based setup instead of legacy setup.
>
> So, as a workaround, this patch checks the fw variant from the TLV based
> version and it uses the legacy HCI_Intel_Read_Version command to get the
> legacy version information and run the legacy bootloader setup with it.
>
> Signed-off-by: Tedd Ho-Jeong An <tedd.an@intel.com>
> ---
> drivers/bluetooth/btintel.c | 25 ++++++++++++++++++++++++-
> 1 file changed, 24 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/bluetooth/btintel.c b/drivers/bluetooth/btintel.c
> index 57b64d744f0a..f11882d10da7 100644
> --- a/drivers/bluetooth/btintel.c
> +++ b/drivers/bluetooth/btintel.c
> @@ -2254,10 +2254,33 @@ int btintel_setup_combined(struct hci_dev *hdev)
> /* For TLV type device, parse the tlv data */
> btintel_parse_version_tlv(hdev, &ver_tlv, skb);
>
> + /* Some legacy bootloader devices from JfP supports both old and TLV
> + * based HCI_Intel_Read_Version command. But we don't want to use the
> + * TLV based setup routines for those old bootloader device.
> + * Also, it is not easy to convert TLV based version to the legacy
> + * version format.
> + *
> + * So, as a workaround for those devices, use the legacy
> + * HCI_Intel_Read_Version to get the version information and run the
> + * legacy bootloader setup.
> + */
why is that again? If possible we should convert them. What information is missing from the TLV format that we need?
Regards
Marcel
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v5 11/11] Bluetooth: btintel: Fix the legacy bootloader returns tlv based version
2021-07-29 19:40 ` Marcel Holtmann
@ 2021-07-29 20:02 ` Tedd Ho-Jeong An
0 siblings, 0 replies; 26+ messages in thread
From: Tedd Ho-Jeong An @ 2021-07-29 20:02 UTC (permalink / raw)
To: Marcel Holtmann; +Cc: linux-bluetooth
Hi Marcel,
On Thu, 2021-07-29 at 21:40 +0200, Marcel Holtmann wrote:
> Hi Tedd,
>
> > From the ThP, it supports both old and TLV based HCI_Intel_Read_Version
> > command after downloading the operational firmware.
> > Starting from th JfP, it supports both old and TLV based
> > HCI_Intel_Read_Version command in the operational firmware and it causes
> > the setup() uses the TLV based setup instead of legacy setup.
> >
> > So, as a workaround, this patch checks the fw variant from the TLV based
> > version and it uses the legacy HCI_Intel_Read_Version command to get the
> > legacy version information and run the legacy bootloader setup with it.
> >
> > Signed-off-by: Tedd Ho-Jeong An <tedd.an@intel.com>
> > ---
> > drivers/bluetooth/btintel.c | 25 ++++++++++++++++++++++++-
> > 1 file changed, 24 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/bluetooth/btintel.c b/drivers/bluetooth/btintel.c
> > index 57b64d744f0a..f11882d10da7 100644
> > --- a/drivers/bluetooth/btintel.c
> > +++ b/drivers/bluetooth/btintel.c
> > @@ -2254,10 +2254,33 @@ int btintel_setup_combined(struct hci_dev *hdev)
> > /* For TLV type device, parse the tlv data */
> > btintel_parse_version_tlv(hdev, &ver_tlv, skb);
> >
> > + /* Some legacy bootloader devices from JfP supports both old and TLV
> > + * based HCI_Intel_Read_Version command. But we don't want to use the
> > + * TLV based setup routines for those old bootloader device.
> > + * Also, it is not easy to convert TLV based version to the legacy
> > + * version format.
> > + *
> > + * So, as a workaround for those devices, use the legacy
> > + * HCI_Intel_Read_Version to get the version information and run the
> > + * legacy bootloader setup.
> > + */
>
> why is that again? If possible we should convert them. What information is missing from the TLV
> format that we need?
>
It needs hw_variant, hw_revision, and fw_revision in order to get the firmware file for legacy
bootloader setup. But new TLV format doesn't provide those values.
Let me check with FW team if those values are wrapped in TLV somehow.
> Regards
>
> Marcel
>
Regards,
Tedd
^ permalink raw reply [flat|nested] 26+ messages in thread