2019-10-18 10:17:45

by Amit K Bag

[permalink] [raw]
Subject: [PATCH v6] Bluetooth: btusb: Trigger Intel FW download error recovery

Sometimes during FW data download stage, in case of an error is
encountered the controller device could not be recovered. To recover
from such failures send Intel hard Reset to re-trigger FW download in
following error scenarios:

1. Intel Read version command error
2. Firmware download timeout
3. Failure in Intel Soft Reset for switching to operational FW
4. Boot timeout for switching to operaional FW

Signed-off-by: Raghuram Hegde <[email protected]>
Signed-off-by: Chethan T N <[email protected]>
Signed-off-by: Amit K Bag <[email protected]>
---
drivers/bluetooth/btintel.c | 46 +++++++++++++++++++++++++++++++++++++++++++++
drivers/bluetooth/btintel.h | 6 ++++++
drivers/bluetooth/btusb.c | 20 ++++++++++++++++----
3 files changed, 68 insertions(+), 4 deletions(-)

diff --git a/drivers/bluetooth/btintel.c b/drivers/bluetooth/btintel.c
index bb99c8653aab..b69cdf991a9a 100644
--- a/drivers/bluetooth/btintel.c
+++ b/drivers/bluetooth/btintel.c
@@ -709,6 +709,52 @@ int btintel_download_firmware(struct hci_dev *hdev, const struct firmware *fw,
}
EXPORT_SYMBOL_GPL(btintel_download_firmware);

+void btintel_reset_to_bootloader(struct hci_dev *hdev)
+{
+ const struct intel_reset params;
+ struct sk_buff *skb;
+ u32 boot_param;
+
+ /* Send Intel Reset command. This will result in
+ * re-enumeration of BT controller.
+ *
+ * Intel Reset parameter description:
+ * reset_type : 0x00 (Soft reset),
+ * 0x01 (Hard reset)
+ * patch_enable : 0x00 (Do not enable),
+ * 0x01 (Enable)
+ * ddc_reload : 0x00 (Do not reload),
+ * 0x01 (Reload)
+ * boot_option: 0x00 (Current image),
+ * 0x01 (Specified boot address)
+ * boot_param: Boot address
+ *
+ */
+ params.reset_type = 0x01;
+ params.patch_enable = 0x01;
+ params.ddc_reload = 0x01;
+ params.boot_option = 0x00;
+ params.boot_param = cpu_to_le32(0x00000000);
+
+ skb = __hci_cmd_sync(hdev, 0xfc01, sizeof(params),
+ &params, HCI_INIT_TIMEOUT);
+ if (IS_ERR(skb)) {
+ bt_dev_err(hdev, "FW download error recovery failed (%ld)",
+ PTR_ERR(skb));
+ return;
+ }
+ bt_dev_info(hdev, "Intel reset sent to retry FW download");
+ kfree_skb(skb);
+
+ /* Current Intel BT controllers(ThP/JfP) hold the USB reset
+ * lines for 2ms when it receives Intel Reset in bootloader mode.
+ * Whereas, the upcoming Intel BT controllers will hold USB reset
+ * for 150ms. To keep the delay generic, 150ms is chosen here.
+ */
+ msleep(150);
+}
+EXPORT_SYMBOL_GPL(btintel_reset_to_bootloader);
+
MODULE_AUTHOR("Marcel Holtmann <[email protected]>");
MODULE_DESCRIPTION("Bluetooth support for Intel devices ver " VERSION);
MODULE_VERSION(VERSION);
diff --git a/drivers/bluetooth/btintel.h b/drivers/bluetooth/btintel.h
index 3d846190f2bf..d2311156f778 100644
--- a/drivers/bluetooth/btintel.h
+++ b/drivers/bluetooth/btintel.h
@@ -87,6 +87,7 @@ int btintel_read_boot_params(struct hci_dev *hdev,
struct intel_boot_params *params);
int btintel_download_firmware(struct hci_dev *dev, const struct firmware *fw,
u32 *boot_param);
+void btintel_reset_to_bootloader(struct hci_dev *hdev);
#else

static inline int btintel_check_bdaddr(struct hci_dev *hdev)
@@ -181,4 +182,9 @@ static inline int btintel_download_firmware(struct hci_dev *dev,
{
return -EOPNOTSUPP;
}
+
+static inline void btintel_reset_to_bootloader(struct hci_dev *hdev)
+{
+ return -EOPNOTSUPP;
+}
#endif
diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
index 5d7bc3410104..f706fde081ce 100644
--- a/drivers/bluetooth/btusb.c
+++ b/drivers/bluetooth/btusb.c
@@ -2182,8 +2182,11 @@ static int btusb_setup_intel_new(struct hci_dev *hdev)
* loaded.
*/
err = btintel_read_version(hdev, &ver);
- if (err)
+ if (err) {
+ bt_dev_err(hdev, "Intel Read version failed (%d)", err);
+ btintel_reset_to_bootloader(hdev);
return err;
+ }

/* The hardware platform number has a fixed value of 0x37 and
* for now only accept this single value.
@@ -2326,9 +2329,13 @@ static int btusb_setup_intel_new(struct hci_dev *hdev)

/* Start firmware downloading and get boot parameter */
err = btintel_download_firmware(hdev, fw, &boot_param);
- if (err < 0)
+ if (err < 0) {
+ /* When FW download fails, send Intel Reset to retry
+ * FW download.
+ */
+ btintel_reset_to_bootloader(hdev);
goto done;
-
+ }
set_bit(BTUSB_FIRMWARE_LOADED, &data->flags);

bt_dev_info(hdev, "Waiting for firmware download to complete");
@@ -2355,6 +2362,7 @@ static int btusb_setup_intel_new(struct hci_dev *hdev)
if (err) {
bt_dev_err(hdev, "Firmware loading timeout");
err = -ETIMEDOUT;
+ btintel_reset_to_bootloader(hdev);
goto done;
}

@@ -2381,8 +2389,11 @@ static int btusb_setup_intel_new(struct hci_dev *hdev)
set_bit(BTUSB_BOOTING, &data->flags);

err = btintel_send_intel_reset(hdev, boot_param);
- if (err)
+ 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.
@@ -2404,6 +2415,7 @@ static int btusb_setup_intel_new(struct hci_dev *hdev)

if (err) {
bt_dev_err(hdev, "Device boot timeout");
+ btintel_reset_to_bootloader(hdev);
return -ETIMEDOUT;
}

--
2.7.4


2019-10-18 15:44:20

by Marcel Holtmann

[permalink] [raw]
Subject: Re: [PATCH v6] Bluetooth: btusb: Trigger Intel FW download error recovery

Hi Amit,

> Sometimes during FW data download stage, in case of an error is
> encountered the controller device could not be recovered. To recover
> from such failures send Intel hard Reset to re-trigger FW download in
> following error scenarios:
>
> 1. Intel Read version command error
> 2. Firmware download timeout
> 3. Failure in Intel Soft Reset for switching to operational FW
> 4. Boot timeout for switching to operaional FW
>
> Signed-off-by: Raghuram Hegde <[email protected]>
> Signed-off-by: Chethan T N <[email protected]>
> Signed-off-by: Amit K Bag <[email protected]>
> ---
> drivers/bluetooth/btintel.c | 46 +++++++++++++++++++++++++++++++++++++++++++++
> drivers/bluetooth/btintel.h | 6 ++++++
> drivers/bluetooth/btusb.c | 20 ++++++++++++++++----
> 3 files changed, 68 insertions(+), 4 deletions(-)

So this patch has a bunch of errors

Applying: Bluetooth: btusb: Trigger Intel FW download error recovery
.git/rebase-apply/patch:97: trailing whitespace.
btintel_reset_to_bootloader(hdev);
warning: 1 line adds whitespace errors.

CC drivers/bluetooth/btintel.o
drivers/bluetooth/btintel.c: In function ‘btintel_reset_to_bootloader’:
drivers/bluetooth/btintel.c:733:20: error: assignment of member ‘reset_type’ in read-only object
733 | params.reset_type = 0x01;
| ^
drivers/bluetooth/btintel.c:734:22: error: assignment of member ‘patch_enable’ in read-only object
734 | params.patch_enable = 0x01;
| ^
drivers/bluetooth/btintel.c:735:20: error: assignment of member ‘ddc_reload’ in read-only object
735 | params.ddc_reload = 0x01;
| ^
drivers/bluetooth/btintel.c:736:21: error: assignment of member ‘boot_option’ in read-only object
736 | params.boot_option = 0x00;
| ^
drivers/bluetooth/btintel.c:737:20: error: assignment of member ‘boot_param’ in read-only object
737 | params.boot_param = cpu_to_le32(0x00000000);
| ^
drivers/bluetooth/btintel.c:716:6: warning: unused variable ‘boot_param’ [-Wunused-variable]
716 | u32 boot_param;
| ^~~~~~~~~~

I would really encourage to test the patch with git am and compile test it at least since these are obvious bugs.

Anyway, I manually fixed this up and applied to patch to bluetooth-next tree.

Regards

Marcel