Hi Marcel,
>> 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 <raghuram.hegde@xxxxxxxxx>
>> Signed-off-by: Chethan T N <chethan.tumkur.narayan@xxxxxxxxx>
>> Signed-off-by: Amit K Bag <amit.k.bag@xxxxxxxxx>
>> ---
>> drivers/bluetooth/btintel.c | 49
>> +++++++++++++++++++++++++++++++++++++++++++++
>> drivers/bluetooth/btintel.h | 6 ++++++
>> drivers/bluetooth/btusb.c | 20 ++++++++++++++----
>> 3 files changed, 71 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/bluetooth/btintel.c b/drivers/bluetooth/btintel.c
>> index bb99c8653aab..a93aec22d3a6 100644
>> --- a/drivers/bluetooth/btintel.c
>> +++ b/drivers/bluetooth/btintel.c
>> @@ -709,6 +709,55 @@ 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;
>> +
>> +
>> + boot_param = 0x00000000;
>> +
>> + /* 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(boot_param);
>
>params.boot_param = cpu_to_le32(0x00000000);
>
>> +
>> + skb = __hci_cmd_sync(hdev, 0xfc01, sizeof(params),
>> + ¶ms, 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 <marcel@xxxxxxxxxxxx>");
>> 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..47178af7f7fe 100644
>> --- a/drivers/bluetooth/btusb.c
>> +++ b/drivers/bluetooth/btusb.c
>> @@ -1846,8 +1846,11 @@ static int btusb_setup_intel(struct hci_dev *hdev)
>> * firmware variant, revision and build number.
>> */
>> 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;
>> + }
>>
>> 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,
>
>I am bit confused on why you modify the read_version in the legacy Intel setup and not in the new one. Can we focus with this patch on setup_intel_new and you add support for legacy setup in a second patch if that is needed as well.
>
>> @@ -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;
>> }
Sorry by mistake I modify the read_version in the legacy Intel setup. I corrected it in the patch v6 patch and also removed boot_param variable and directly assign the value as well.
Regards,
Amit
