[PATCH 1/2] Bluetooth: btintel: Fix spurious error on FW download error recovery

[PATCH 1/2] Bluetooth: btintel: Fix spurious error on FW download error recovery

[Ameen Al-Asady]

When firmware download fails because the controller is running an
operational image whose version does not match the firmware file on
disk (the normal case for a firmware upgrade across a warm reboot,
since the operational image survives in controller RAM),
btintel_reset_to_bootloader() sends the Intel Reset vendor command to
force the controller back into bootloader mode for the download retry.

The controller acts on Intel Reset by resetting and re-enumerating on
the bus, which can happen before the command complete event is
delivered. In that case __hci_cmd_sync() fails with -ENODEV and the
driver logs a misleading error even though recovery is proceeding
exactly as intended:

  Bluetooth: hci0: Found device firmware: intel/ibt-0041-0041.sfi
  Bluetooth: hci0: Boot Address: 0x100800
  Bluetooth: hci0: Firmware Version: 202-5.26
  usb 1-6: USB disconnect, device number 4
  Bluetooth: hci0: FW download error recovery failed (-19)
  usb 1-6: new full-speed USB device number 5 using xhci_hcd
  ...
  Bluetooth: hci0: Firmware loaded in 2827613 usecs

Treat -ENODEV as the expected outcome of the reset rather than a
recovery failure, and keep the error message for genuine failures.

Signed-off-by: Ameen Al-Asady <ameenaladdin@gmail.com>
---
 drivers/bluetooth/btintel.c | 18 ++++++++++++++----
 1 file changed, 14 insertions(+), 4 deletions(-)

diff --git a/drivers/bluetooth/btintel.c b/drivers/bluetooth/btintel.c
index 5e9cac090..472542bff 100644
--- a/drivers/bluetooth/btintel.c
+++ b/drivers/bluetooth/btintel.c
@@ -1363,12 +1363,22 @@ static void btintel_reset_to_bootloader(struct hci_dev *hdev)
 	skb = __hci_cmd_sync(hdev, BTINTEL_HCI_OP_RESET, sizeof(params),
 			     &params, HCI_INIT_TIMEOUT);
 	if (IS_ERR(skb)) {
-		bt_dev_err(hdev, "FW download error recovery failed (%ld)",
-			   PTR_ERR(skb));
-		return;
+		/* The controller acts on Intel Reset by resetting and
+		 * re-enumerating on the bus, which can happen before the
+		 * command complete event is delivered. The resulting
+		 * -ENODEV means the reset took effect, not that recovery
+		 * failed.
+		 */
+		if (PTR_ERR(skb) != -ENODEV) {
+			bt_dev_err(hdev,
+				   "FW download error recovery failed (%ld)",
+				   PTR_ERR(skb));
+			return;
+		}
+	} else {
+		kfree_skb(skb);
 	}
 	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.

base-commit: eab19cb7618e5c50fd1f8c1efdc4e66e16575975
-- 
2.55.0

[PATCH 2/2] Bluetooth: hci_sync: Skip MSFT/AOSP init when controller init failed

[Ameen Al-Asady]

hci_dev_init_sync() calls msft_do_open() and aosp_do_open()
unconditionally, even when setup or the HCI init sequence has already
failed. Both send vendor commands with __hci_cmd_sync(), so on a dead
or uninitialized controller each of them blocks the power-on sequence
for a full HCI_CMD_TIMEOUT and logs a spurious error.

This is easy to trigger with an Intel USB controller that needs a
firmware upgrade: btintel resets the controller to bootloader mode,
the device drops off the bus and hdev->setup() fails with -EINVAL,
after which msft_do_open() still spends two more seconds waiting for
a response that cannot arrive:

  usb 1-6: USB disconnect, device number 4
  Bluetooth: hci0: FW download error recovery failed (-19)
  Bluetooth: hci0: Failed to read MSFT supported features (-110)
  usb 1-6: new full-speed USB device number 5 using xhci_hcd

Only initialize the MSFT and AOSP extensions when the controller
initialized successfully.

Signed-off-by: Ameen Al-Asady <ameenaladdin@gmail.com>
---
 net/bluetooth/hci_sync.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/bluetooth/hci_sync.c b/net/bluetooth/hci_sync.c
index b4ca34abe..38158835c 100644
--- a/net/bluetooth/hci_sync.c
+++ b/net/bluetooth/hci_sync.c
@@ -5173,7 +5173,7 @@ static int hci_dev_init_sync(struct hci_dev *hdev)
 	    hci_dev_test_flag(hdev, HCI_VENDOR_DIAG) && hdev->set_diag)
 		ret = hdev->set_diag(hdev, true);
 
-	if (!hci_dev_test_flag(hdev, HCI_USER_CHANNEL)) {
+	if (!ret && !hci_dev_test_flag(hdev, HCI_USER_CHANNEL)) {
 		msft_do_open(hdev);
 		aosp_do_open(hdev);
 	}
-- 
2.55.0

RE: [1/2] Bluetooth: btintel: Fix spurious error on FW download error recovery

[bluez.test.bot]

[-- Attachment #1: Type: text/plain, Size: 2389 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=1126920

---Test result---

Test Summary:
CheckPatch                    PASS      1.72 seconds
VerifyFixes                   PASS      0.11 seconds
VerifySignedoff               PASS      0.11 seconds
GitLint                       PASS      0.57 seconds
SubjectPrefix                 PASS      0.21 seconds
BuildKernel                   PASS      25.29 seconds
CheckAllWarning               PASS      28.08 seconds
CheckSparse                   PASS      26.83 seconds
BuildKernel32                 PASS      25.24 seconds
CheckKernelLLVM               SKIP      0.00 seconds
TestRunnerSetup               PASS      456.79 seconds
TestRunner_l2cap-tester       PASS      57.69 seconds
TestRunner_iso-tester         PASS      80.99 seconds
TestRunner_bnep-tester        PASS      18.81 seconds
TestRunner_mgmt-tester        FAIL      206.36 seconds
TestRunner_rfcomm-tester      PASS      24.99 seconds
TestRunner_sco-tester         PASS      30.51 seconds
TestRunner_ioctl-tester       PASS      25.57 seconds
TestRunner_mesh-tester        FAIL      24.92 seconds
TestRunner_smp-tester         PASS      22.74 seconds
TestRunner_userchan-tester    PASS      19.71 seconds
TestRunner_6lowpan-tester     PASS      22.64 seconds
IncrementalBuild              PASS      27.18 seconds

Details
##############################
Test: CheckKernelLLVM - SKIP
Desc: Build kernel with LLVM + context analysis
Output:
Clang not found
##############################
Test: TestRunner_mgmt-tester - FAIL
Desc: Run mgmt-tester with test-runner
Output:
Total: 494, Passed: 489 (99.0%), Failed: 1, Not Run: 4

Failed Test Cases
Read Exp Feature - Success                           Failed       0.237 seconds
##############################
Test: TestRunner_mesh-tester - FAIL
Desc: Run mesh-tester with test-runner
Output:
Total: 10, Passed: 8 (80.0%), Failed: 2, Not Run: 0

Failed Test Cases
Mesh - Send cancel - 1                               Timed out    1.901 seconds
Mesh - Send cancel - 2                               Timed out    1.989 seconds


https://github.com/bluez/bluetooth-next/pull/432

---
Regards,
Linux Bluetooth

Re: [PATCH 1/2] Bluetooth: btintel: Fix spurious error on FW download error recovery

[Paul Menzel]

Dear Ameen,


Thank you for your patch.

Am 13.07.26 um 23:55 schrieb Ameen Al-Asady:
> When firmware download fails because the controller is running an

“fail” is ambiguous to me. Maybe “errors out”.

> operational image whose version does not match the firmware file on
> disk (the normal case for a firmware upgrade across a warm reboot,
> since the operational image survives in controller RAM),
> btintel_reset_to_bootloader() sends the Intel Reset vendor command to
> force the controller back into bootloader mode for the download retry.
> 
> The controller acts on Intel Reset by resetting and re-enumerating on
> the bus, which can happen before the command complete event is
> delivered. In that case __hci_cmd_sync() fails with -ENODEV and the
> driver logs a misleading error even though recovery is proceeding
> exactly as intended:
> 
>    Bluetooth: hci0: Found device firmware: intel/ibt-0041-0041.sfi
>    Bluetooth: hci0: Boot Address: 0x100800
>    Bluetooth: hci0: Firmware Version: 202-5.26
>    usb 1-6: USB disconnect, device number 4
>    Bluetooth: hci0: FW download error recovery failed (-19)
>    usb 1-6: new full-speed USB device number 5 using xhci_hcd
>    ...
>    Bluetooth: hci0: Firmware loaded in 2827613 usecs

Any hint how to match the firmware versions, or add a sentence what 
firmware is on the controller and which one is downloaded?

> Treat -ENODEV as the expected outcome of the reset rather than a
> recovery failure, and keep the error message for genuine failures.

Please mention the system you tested this on.

> Signed-off-by: Ameen Al-Asady <ameenaladdin@gmail.com>
> ---
>   drivers/bluetooth/btintel.c | 18 ++++++++++++++----
>   1 file changed, 14 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/bluetooth/btintel.c b/drivers/bluetooth/btintel.c
> index 5e9cac090..472542bff 100644
> --- a/drivers/bluetooth/btintel.c
> +++ b/drivers/bluetooth/btintel.c
> @@ -1363,12 +1363,22 @@ static void btintel_reset_to_bootloader(struct hci_dev *hdev)
>   	skb = __hci_cmd_sync(hdev, BTINTEL_HCI_OP_RESET, sizeof(params),
>   			     &params, HCI_INIT_TIMEOUT);
>   	if (IS_ERR(skb)) {
> -		bt_dev_err(hdev, "FW download error recovery failed (%ld)",
> -			   PTR_ERR(skb));
> -		return;
> +		/* The controller acts on Intel Reset by resetting and
> +		 * re-enumerating on the bus, which can happen before the
> +		 * command complete event is delivered. The resulting
> +		 * -ENODEV means the reset took effect, not that recovery
> +		 * failed.
> +		 */
> +		if (PTR_ERR(skb) != -ENODEV) {
> +			bt_dev_err(hdev,
> +				   "FW download error recovery failed (%ld)",
> +				   PTR_ERR(skb));
> +			return;
> +		}
> +	} else {
> +		kfree_skb(skb);
>   	}
>   	bt_dev_info(hdev, "Intel reset sent to retry FW download");
> -	kfree_skb(skb);

Excuse my ignorance, but moving kfree_skb to a new else branch is not 
totally clear to me.

>   
>   	/* Current Intel BT controllers(ThP/JfP) hold the USB reset
>   	 * lines for 2ms when it receives Intel Reset in bootloader mode.
> 
> base-commit: eab19cb7618e5c50fd1f8c1efdc4e66e16575975


Kind regards,

Paul

Re: [PATCH 1/2] Bluetooth: btintel: Fix spurious error on FW download error recovery

[Ameen Al-Asady]

Hi Paul,

Thanks for the review.

> "fail" is ambiguous to me. Maybe "errors out".

Reworded in v2.

> Any hint how to match the firmware versions, or add a sentence what
> firmware is on the controller and which one is downloaded?

Added the firmware build lines to the log excerpt in v2. The
controller was running build 81755 (2024.33) and the file on disk
contained build 82122 (2026.5).

> Please mention the system you tested this on.

Done. Tested on an Intel AX210 (USB 8087:0032) on a 7.1.3 kernel by
rebinding btusb with a non-matching firmware file on disk, in both
the downgrade and upgrade direction. Both runs took the -ENODEV path,
logged the info message instead of the error and completed the
download after re-enumeration.

> Excuse my ignorance, but moving kfree_skb to a new else branch is
> not totally clear to me.

When __hci_cmd_sync() fails it returns an error pointer instead of a
real skb, so there is nothing to free. kfree_skb() must only run in
the success case, which is now the else branch. The old code had the
same behavior through the early return.

v2 sent.

Regards,
Ameen