Bluetooth: btintel: Fix spurious error on FW download error recovery
Subsystems (2)
PATCH 1/2 Bluetooth: btintel: Fix spurious error on FW download error recovery
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.
--- drivers/bluetooth/btintel.c | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-)
drivers/bluetooth/btintel.c +14 -4
@@ -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), ¶ms,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
Signature · 1 line
2.55.0
PATCH 2/2 Bluetooth: hci_sync: Skip MSFT/AOSP init when controller init failed
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.
--- net/bluetooth/hci_sync.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
net/bluetooth/hci_sync.c +1 -1
@@ -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); }
Signature · 1 line
2.55.0
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
Dear Ameen,
Thank you for your patch.
Am 13.07.26 um 23:55 schrieb Ameen Al-Asady:
Quoted 1 line
When firmware download fails because the controller is running an
“fail” is ambiguous to me. Maybe “errors out”.
Quoted 12 lines
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-ENODEVand the driver logs a misleading error even though recovery is proceeding exactly as intended:
Quoted 8 lines
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 usingxhci_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?
Quoted 2 lines
Treat
-ENODEVas 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.
Quoted 23 lines
Signed-off-by Ameen Al-Asady <[email protected]>--- drivers/bluetooth/btintel.c | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-)drivers/bluetooth/btintel.c +7
-3@@ -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), ¶ms,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 ) {
drivers/bluetooth/btintel.c static void btintel_reset_to_bootloader(struct hci_dev *hdev) · line 1371
Quoted 10 lines
drivers/bluetooth/btintel.c +7
-1+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.
Quoted 5 lines
drivers/bluetooth/btintel.c +0
-0/* 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
Hi Paul,
Thanks for the review.
Quoted 1 line
"fail" is ambiguous to me. Maybe "errors out".
Reworded in v2.
Quoted 2 lines
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).
Quoted 1 line
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.
Quoted 2 lines
Excuse my ignorance, but moving
kfree_skbto 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