[PATCH v3] Bluetooth: btintel: read ROM debug registers on FW download failure

[PATCH v3] Bluetooth: btintel: read ROM debug registers on FW download failure

[Ravindra]

The bootloader records init sequence progress and debug info in four
core debug registers, useful for debugging FW download failures.

Add btintel_get_rom_debug_info() to issue BTINTEL_GET_ROM_DEBUG_INFO
(0xfcde) and log these registers, and invoke it from every FW download
failure and timeout path: btintel_download_fw() (legacy .sfi flow) and
btintel_prepare_fw_download_tlv() (TLV flow), before resetting to the
bootloader.

Tested by corrupting the operational firmware file to force a
download timeout; the driver logged the debug registers on each
failure and retry, e.g.:

  Bluetooth: hci0: Firmware loading timeout
  Bluetooth: Intel get rom debug info: dr0:0x80000001 dr1:0x00000000 dr2:0x00000001 dr3:0x00df0071

Signed-off-by: Ravindra <ravindra@intel.com>
Reviewed-by: Paul Menzel <pmenzel@molgen.mpg.de>
---
 drivers/bluetooth/btintel.c | 51 +++++++++++++++++++++++++++++++++++--
 drivers/bluetooth/btintel.h |  9 +++++++
 2 files changed, 58 insertions(+), 2 deletions(-)

diff --git a/drivers/bluetooth/btintel.c b/drivers/bluetooth/btintel.c
index e998fd390ad2..fc204c708f35 100644
--- a/drivers/bluetooth/btintel.c
+++ b/drivers/bluetooth/btintel.c
@@ -983,6 +983,41 @@ int btintel_send_intel_reset(struct hci_dev *hdev, u32 boot_param)
 }
 EXPORT_SYMBOL_GPL(btintel_send_intel_reset);
 
+static void btintel_get_rom_debug_info(struct hci_dev *hdev)
+{
+	struct btintel_rp_get_rom_debug_info *rom_debug_info;
+	struct sk_buff *skb;
+
+	skb = __hci_cmd_sync(hdev, BTINTEL_GET_ROM_DEBUG_INFO, 0, NULL,
+			     HCI_CMD_TIMEOUT);
+	if (IS_ERR(skb)) {
+		bt_dev_err(hdev, "Failed to send Intel get rom debug info command (%ld)",
+			   PTR_ERR(skb));
+		return;
+	}
+
+	if (skb->len != sizeof(*rom_debug_info)) {
+		bt_dev_err(hdev,
+			   "Intel get rom debug info size mismatch (%u != %zu)",
+			   skb->len, sizeof(*rom_debug_info));
+		kfree_skb(skb);
+		return;
+	}
+
+	rom_debug_info = (struct btintel_rp_get_rom_debug_info *)skb->data;
+
+	/* A non-zero status here would have already been turned into an
+	 * error by the HCI core and reported via the IS_ERR(skb) check above.
+	 */
+	bt_dev_info(hdev, "Intel get rom debug info: dr0:0x%08x dr1:0x%08x dr2:0x%08x dr3:0x%08x",
+		    le32_to_cpu(rom_debug_info->debug_reg0),
+		    le32_to_cpu(rom_debug_info->debug_reg1),
+		    le32_to_cpu(rom_debug_info->debug_reg2),
+		    le32_to_cpu(rom_debug_info->debug_reg3));
+
+	kfree_skb(skb);
+}
+
 int btintel_read_boot_params(struct hci_dev *hdev,
 			     struct intel_boot_params *params)
 {
@@ -2232,6 +2267,9 @@ download:
 			goto done;
 		}
 
+		/* Collect rom debug info on FW download failure */
+		btintel_get_rom_debug_info(hdev);
+
 		/* When FW download fails, send Intel Reset to retry
 		 * FW download.
 		 */
@@ -2251,8 +2289,11 @@ download:
 	 * of this device.
 	 */
 	err = btintel_download_wait(hdev, calltime, INTEL_SLE_MULTIPLIER * 5000);
-	if (err == -ETIMEDOUT)
+	if (err == -ETIMEDOUT) {
+		/* Collect rom debug info on FW download failure */
+		btintel_get_rom_debug_info(hdev);
 		btintel_reset_to_bootloader(hdev);
+	}
 
 done:
 	release_firmware(fw);
@@ -2492,6 +2533,9 @@ static int btintel_prepare_fw_download_tlv(struct hci_dev *hdev,
 			goto done;
 		}
 
+		/* Collect rom debug info on FW download failure */
+		btintel_get_rom_debug_info(hdev);
+
 		/* When FW download fails, send Intel Reset to retry
 		 * FW download.
 		 */
@@ -2511,8 +2555,11 @@ static int btintel_prepare_fw_download_tlv(struct hci_dev *hdev,
 	 * of this device.
 	 */
 	err = btintel_download_wait(hdev, calltime, INTEL_SLE_MULTIPLIER * 5000);
-	if (err == -ETIMEDOUT)
+	if (err == -ETIMEDOUT) {
+		/* Collect rom debug info on FW download failure */
+		btintel_get_rom_debug_info(hdev);
 		btintel_reset_to_bootloader(hdev);
+	}
 
 done:
 	{
diff --git a/drivers/bluetooth/btintel.h b/drivers/bluetooth/btintel.h
index 586be92b7690..0f7261c102b8 100644
--- a/drivers/bluetooth/btintel.h
+++ b/drivers/bluetooth/btintel.h
@@ -188,6 +188,15 @@ struct hci_ppag_enable_cmd {
 	__le32	ppag_enable_flags;
 } __packed;
 
+#define BTINTEL_GET_ROM_DEBUG_INFO	0xfcde
+struct btintel_rp_get_rom_debug_info {
+	__u8	status;
+	__le32	debug_reg0;
+	__le32	debug_reg1;
+	__le32	debug_reg2;
+	__le32	debug_reg3;
+} __packed;
+
 #define INTEL_TLV_TYPE_ID		0x01
 
 #define INTEL_TLV_SYSTEM_EXCEPTION	0x00
-- 
2.43.0

RE: [v3] Bluetooth: btintel: read ROM debug registers on FW download failure

[bluez.test.bot]

[-- Attachment #1: Type: text/plain, Size: 853 bytes --]

This is an automated email and please do not reply to this email.

Dear Submitter,

Thank you for submitting the patches to the linux bluetooth mailing list.
While preparing the CI tests, the patches you submitted couldn't be applied to the current HEAD of the repository.

----- Output -----

error: patch failed: drivers/bluetooth/btintel.c:2251
error: drivers/bluetooth/btintel.c: patch does not apply
hint: Use 'git am --show-current-patch=diff' to see the failed patch
hint: When you have resolved this problem, run "git am --continue".
hint: If you prefer to skip this patch, run "git am --skip" instead.
hint: To restore the original branch and stop patching, run "git am --abort".
hint: Disable this message with "git config set advice.mergeConflict false"

Please resolve the issue and submit the patches again.


---
Regards,
Linux Bluetooth

[PATCH v3] Bluetooth: btintel: read ROM debug registers on FW download failure

[Ravindra]

The bootloader records init sequence progress and debug info in four
core debug registers, useful for debugging FW download failures.

Add btintel_get_rom_debug_info() to issue BTINTEL_GET_ROM_DEBUG_INFO
(0xfcde) and log these registers, and invoke it from every FW download
failure and timeout path: btintel_download_fw() (legacy .sfi flow) and
btintel_prepare_fw_download_tlv() (TLV flow), before resetting to the
bootloader.

Tested by corrupting the operational firmware file to force a
download timeout; the driver logged the debug registers on each
failure and retry, e.g.:

  Bluetooth: hci0: Firmware loading timeout
  Bluetooth: Intel get rom debug info: dr0:0x80000001 dr1:0x00000000 dr2:0x00000001 dr3:0x00df0071

Signed-off-by: Ravindra <ravindra@intel.com>
Reviewed-by: Paul Menzel <pmenzel@molgen.mpg.de>
---
 drivers/bluetooth/btintel.c | 47 +++++++++++++++++++++++++++++++++++--
 drivers/bluetooth/btintel.h |  9 +++++++
 2 files changed, 54 insertions(+), 2 deletions(-)

diff --git a/drivers/bluetooth/btintel.c b/drivers/bluetooth/btintel.c
index 964d2de30e65..5e6d1ddc99c6 100644
--- a/drivers/bluetooth/btintel.c
+++ b/drivers/bluetooth/btintel.c
@@ -963,6 +963,41 @@ int btintel_send_intel_reset(struct hci_dev *hdev, u32 boot_param)
 }
 EXPORT_SYMBOL_GPL(btintel_send_intel_reset);
 
+static void btintel_get_rom_debug_info(struct hci_dev *hdev)
+{
+	struct btintel_rp_get_rom_debug_info *rom_debug_info;
+	struct sk_buff *skb;
+
+	skb = __hci_cmd_sync(hdev, BTINTEL_GET_ROM_DEBUG_INFO, 0, NULL,
+			     HCI_CMD_TIMEOUT);
+	if (IS_ERR(skb)) {
+		bt_dev_err(hdev, "Failed to send Intel get rom debug info command (%ld)",
+			   PTR_ERR(skb));
+		return;
+	}
+
+	if (skb->len != sizeof(*rom_debug_info)) {
+		bt_dev_err(hdev,
+			   "Intel get rom debug info size mismatch (%u != %zu)",
+			   skb->len, sizeof(*rom_debug_info));
+		kfree_skb(skb);
+		return;
+	}
+
+	rom_debug_info = (struct btintel_rp_get_rom_debug_info *)skb->data;
+
+	/* A non-zero status here would have already been turned into an
+	 * error by the HCI core and reported via the IS_ERR(skb) check above.
+	 */
+	bt_dev_info(hdev, "Intel get rom debug info: dr0:0x%08x dr1:0x%08x dr2:0x%08x dr3:0x%08x",
+		    le32_to_cpu(rom_debug_info->debug_reg0),
+		    le32_to_cpu(rom_debug_info->debug_reg1),
+		    le32_to_cpu(rom_debug_info->debug_reg2),
+		    le32_to_cpu(rom_debug_info->debug_reg3));
+
+	kfree_skb(skb);
+}
+
 int btintel_read_boot_params(struct hci_dev *hdev,
 			     struct intel_boot_params *params)
 {
@@ -2212,6 +2247,8 @@ static int btintel_download_fw(struct hci_dev *hdev,
 			goto done;
 		}
 
+		btintel_get_rom_debug_info(hdev);
+
 		/* When FW download fails, send Intel Reset to retry
 		 * FW download.
 		 */
@@ -2231,8 +2268,10 @@ static int btintel_download_fw(struct hci_dev *hdev,
 	 * of this device.
 	 */
 	err = btintel_download_wait(hdev, calltime, 5000);
-	if (err == -ETIMEDOUT)
+	if (err == -ETIMEDOUT) {
+		btintel_get_rom_debug_info(hdev);
 		btintel_reset_to_bootloader(hdev);
+	}
 
 done:
 	release_firmware(fw);
@@ -2465,6 +2504,8 @@ static int btintel_prepare_fw_download_tlv(struct hci_dev *hdev,
 			goto done;
 		}
 
+		btintel_get_rom_debug_info(hdev);
+
 		/* When FW download fails, send Intel Reset to retry
 		 * FW download.
 		 */
@@ -2484,8 +2525,10 @@ static int btintel_prepare_fw_download_tlv(struct hci_dev *hdev,
 	 * of this device.
 	 */
 	err = btintel_download_wait(hdev, calltime, 5000);
-	if (err == -ETIMEDOUT)
+	if (err == -ETIMEDOUT) {
+		btintel_get_rom_debug_info(hdev);
 		btintel_reset_to_bootloader(hdev);
+	}
 
 done:
 	intel_data->cnvi_bt = ver->cnvi_bt;
diff --git a/drivers/bluetooth/btintel.h b/drivers/bluetooth/btintel.h
index c00f793d9dc8..b3c4c6f87424 100644
--- a/drivers/bluetooth/btintel.h
+++ b/drivers/bluetooth/btintel.h
@@ -178,6 +178,15 @@ struct hci_ppag_enable_cmd {
 	__le32	ppag_enable_flags;
 } __packed;
 
+#define BTINTEL_GET_ROM_DEBUG_INFO	0xfcde
+struct btintel_rp_get_rom_debug_info {
+	__u8	status;
+	__le32	debug_reg0;
+	__le32	debug_reg1;
+	__le32	debug_reg2;
+	__le32	debug_reg3;
+} __packed;
+
 #define INTEL_TLV_TYPE_ID		0x01
 
 #define INTEL_TLV_SYSTEM_EXCEPTION	0x00
-- 
2.43.0

RE: [v3] Bluetooth: btintel: read ROM debug registers on FW download failure

[bluez.test.bot]

[-- Attachment #1: Type: text/plain, Size: 2152 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/series/1170015/

---Test result---

Test Summary:
CheckPatch                    FAIL      1.09 seconds
VerifyFixes                   PASS      0.08 seconds
VerifySignedoff               PASS      0.08 seconds
GitLint                       FAIL      0.24 seconds
SubjectPrefix                 PASS      0.08 seconds
BuildKernel                   PASS      29.91 seconds
CheckAllWarning               PASS      34.05 seconds
CheckSparse                   PASS      37.61 seconds
BuildKernel32                 PASS      30.12 seconds
CheckKernelLLVM               PASS      34.46 seconds
TestRunnerSetup               PASS      793.74 seconds
IncrementalBuild              PASS      28.08 seconds

Details
##############################
Test: CheckPatch - FAIL
Desc: Run checkpatch.pl script
Output:
[v3] Bluetooth: btintel: read ROM debug registers on FW download failure
WARNING: Prefer a maximum 75 chars per line (possible unwrapped commit description?)
#94: 
  Bluetooth: Intel get rom debug info: dr0:0x80000001 dr1:0x00000000 dr2:0x00000001 dr3:0x00df0071

total: 0 errors, 1 warnings, 94 lines checked

NOTE: For some of the reported defects, checkpatch may be able to
      mechanically convert to the typical style using --fix or --fix-inplace.

/home/runner/work/bluetooth-next/bluetooth-next/src/patch/14833867.patch has style problems, please review.

NOTE: Ignored message types: UNKNOWN_COMMIT_ID

NOTE: If any of the errors are false positives, please report
      them to the maintainer, see CHECKPATCH in MAINTAINERS.


##############################
Test: GitLint - FAIL
Desc: Run gitlint
Output:
[v3] Bluetooth: btintel: read ROM debug registers on FW download failure

17: B1 Line exceeds max length (98>80): "  Bluetooth: Intel get rom debug info: dr0:0x80000001 dr1:0x00000000 dr2:0x00000001 dr3:0x00df0071"


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

---
Regards,
Linux Bluetooth