[PATCH v2 0/2] Bluetooth: btmtk: fix subsystem reset error reporting

[PATCH v2 0/2] Bluetooth: btmtk: fix subsystem reset error reporting

[Ismail Tarim]

Two bugs in btmtk_usb_subsys_reset() that make it report success to its
caller when the subsystem reset has actually failed.

Patch 1 covers the case where the post-reset chip id reads back as zero:
the function logs "subsys reset fail." and then returns 0.

Patch 2 restores propagation of the MTK_BT_RST_DONE poll timeout, which
has been silently overwritten since commit 3dcb122b3064 ("Bluetooth:
btusb: mediatek: return error for failed reg access") started assigning
err at the chip id read.

Found while investigating an MT7902 [13d3:3579] whose controller stops
responding and never returns to the USB bus. On that device the failing
path can be reached on demand by requesting a coredump, since btmtk asks
for a reset once the dump completes:

  # echo 1 > /sys/class/bluetooth/hci0/device/coredump

The captured firmware coredump shows the controller asserting inside its
ROM rather than in the downloadable blob, which is consistent with the
failure surviving a firmware update:

  <ASSERT> system/rom/transport/tra_usb3.c #764 - BTSYS, id=0x4 idle

Both patches only correct the error reporting; neither changes the
recovery behaviour or makes that controller recoverable. They are a
prerequisite for anything that wants to react to a failed reset, since
today the failure is not visible to the caller.

Compile tested on x86_64 (W=1, no new warnings); checkpatch --strict clean.

Changes in v2:
- 1/2: shorten the subject so the CI gitlint check passes (was 81 > 80).
- 1/2: document how the failing path can be reached on demand.
- No functional change; the diff is identical to v1.

v1: https://lore.kernel.org/linux-bluetooth/20260815110119.11301-1-ismailtarim7@gmail.com/

Ismail Tarim (2):
  Bluetooth: btmtk: Do not report success when subsys reset fails
  Bluetooth: btmtk: Do not discard the subsystem reset timeout

 drivers/bluetooth/btmtk.c | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)


base-commit: 15ef2f78c49d20d53ec7c0f1c9b40b02e089f2d6
-- 
2.55.0

[PATCH v2 1/2] Bluetooth: btmtk: Do not report success when subsys reset fails

[Ismail Tarim]

btmtk_usb_subsys_reset() validates the subsystem reset by reading the
chip id back. When that read succeeds at the bus level but yields an id
of zero, the reset has demonstrably not taken effect: the function logs
"Can't get device id, subsys reset fail." and then returns the return
value of btmtk_usb_id_get(), which in that case is zero, i.e. success.

btusb_mtk_reset() returns that value unchanged, so its caller cannot
tell a completed reset from a failed one.

Return -ENODEV when the chip id reads back as zero, leaving the existing
MT6639 exemption intact.

Observed on an MT7902 [13d3:3579]. The path can be reached on demand by
asking the controller for a coredump, since btmtk requests a reset once
the dump completes:

  # echo 1 > /sys/class/bluetooth/hci0/device/coredump

  Bluetooth: hci0: Mediatek coredump end
  Bluetooth: hci0: Can't get device id, subsys reset fail.
  usb 3-10: reset high-speed USB device number 5 using xhci_hcd
  usb 3-10: device descriptor read/64, error -110
  usb usb3-port10: attempt power cycle
  usb usb3-port10: unable to enumerate USB device

The same sequence occurs unprompted when the controller firmware asserts
on its own.

Note that this corrects the error reporting only; it does not by itself
make the controller recoverable in the case above.

Fixes: 25b6d7593a3a ("Bluetooth: btmtk: introduce btmtk reset work")
Signed-off-by: Ismail Tarim <ismailtarim7@gmail.com>
---
 drivers/bluetooth/btmtk.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/bluetooth/btmtk.c b/drivers/bluetooth/btmtk.c
index 66b346761043..dc702c0a6034 100644
--- a/drivers/bluetooth/btmtk.c
+++ b/drivers/bluetooth/btmtk.c
@@ -968,8 +968,10 @@ int btmtk_usb_subsys_reset(struct hci_dev *hdev, u32 dev_id)
 	}
 
 	err = btmtk_usb_id_get(hdev, 0x70010200, &val);
-	if (err || (!val && dev_id != 0x6639))
+	if (err || (!val && dev_id != 0x6639)) {
 		bt_dev_err(hdev, "Can't get device id, subsys reset fail.");
+		return err ? err : -ENODEV;
+	}
 
 	return err;
 }
-- 
2.55.0

RE: Bluetooth: btmtk: fix subsystem reset error reporting

[bluez.test.bot]

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

---Test result---

Test Summary:
CheckPatch                    PASS      1.12 seconds
VerifyFixes                   PASS      0.08 seconds
VerifySignedoff               PASS      0.08 seconds
GitLint                       PASS      0.42 seconds
SubjectPrefix                 PASS      0.14 seconds
BuildKernel                   PASS      27.42 seconds
CheckAllWarning               PASS      30.15 seconds
CheckSparse                   PASS      28.90 seconds
BuildKernel32                 PASS      26.56 seconds
CheckKernelLLVM               SKIP      0.00 seconds
TestRunnerSetup               PASS      503.09 seconds
IncrementalBuild              PASS      27.86 seconds

Details
##############################
Test: CheckKernelLLVM - SKIP
Desc: Build kernel with LLVM + context analysis
Output:
Clang not found


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

---
Regards,
Linux Bluetooth

[PATCH v2 2/2] Bluetooth: btmtk: Do not discard the subsystem reset timeout

[Ismail Tarim]

When the MTK_BT_RST_DONE poll times out, btmtk_usb_subsys_reset() logs
"Reset timeout" and keeps the error in err, but err is then overwritten
by the return value of the following btmtk_usb_id_get() call, so the
timeout is never reported to the caller.

Commit 25b6d7593a3a ("Bluetooth: btmtk: introduce btmtk reset work")
discarded the return value of the chip id read, so the function returned
the timeout error as intended. Commit 3dcb122b3064 ("Bluetooth: btusb:
mediatek: return error for failed reg access") started assigning err at
that call and silently dropped it.

Keep the timeout in a separate variable and return it, restoring the
original behaviour without changing the control flow.

Fixes: 3dcb122b3064 ("Bluetooth: btusb: mediatek: return error for failed reg access")
Signed-off-by: Ismail Tarim <ismailtarim7@gmail.com>
---
 drivers/bluetooth/btmtk.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/bluetooth/btmtk.c b/drivers/bluetooth/btmtk.c
index dc702c0a6034..c0ed51567ed4 100644
--- a/drivers/bluetooth/btmtk.c
+++ b/drivers/bluetooth/btmtk.c
@@ -860,6 +860,7 @@ static u32 btmtk_usb_reset_done(struct hci_dev *hdev)
 
 int btmtk_usb_subsys_reset(struct hci_dev *hdev, u32 dev_id)
 {
+	int reset_err = 0;
 	u32 val;
 	int err;
 
@@ -958,8 +959,10 @@ int btmtk_usb_subsys_reset(struct hci_dev *hdev, u32 dev_id)
 
 	err = readx_poll_timeout(btmtk_usb_reset_done, hdev, val,
 				 val & MTK_BT_RST_DONE, 20000, 1000000);
-	if (err < 0)
+	if (err < 0) {
 		bt_dev_err(hdev, "Reset timeout");
+		reset_err = err;
+	}
 
 	if (dev_id == 0x7922) {
 		err = btmtk_usb_uhw_reg_write(hdev, MTK_UDMA_INT_STA_BT, 0x000000FF);
@@ -973,7 +976,7 @@ int btmtk_usb_subsys_reset(struct hci_dev *hdev, u32 dev_id)
 		return err ? err : -ENODEV;
 	}
 
-	return err;
+	return reset_err;
 }
 EXPORT_SYMBOL_GPL(btmtk_usb_subsys_reset);
 
-- 
2.55.0