On 3/5/25 4:03 AM, Manish Pandey wrote:
This patch introduces support for dumping testbus registers,
enhancing the debugging capabilities for UFS-QCOM drivers.
Signed-off-by: Manish Pandey <quic_mapa@xxxxxxxxxxx>
---
drivers/ufs/host/ufs-qcom.c | 73 +++++++++++++++++++++++++++++++++++++
1 file changed, 73 insertions(+)
diff --git a/drivers/ufs/host/ufs-qcom.c b/drivers/ufs/host/ufs-qcom.c
index 7daee416eb8b..c8f95519b580 100644
--- a/drivers/ufs/host/ufs-qcom.c
+++ b/drivers/ufs/host/ufs-qcom.c
@@ -1566,6 +1566,75 @@ int ufs_qcom_testbus_config(struct ufs_qcom_host *host)
return 0;
}
+static void ufs_qcom_dump_testbus(struct ufs_hba *hba)
+{
+ struct ufs_qcom_host *host = ufshcd_get_variant(hba);
+ u32 *testbus = NULL;
+ int i, j, nminor = 0, testbus_len = 0;
+ char *prefix;
Shouldn't the declarations be ordered from longest to shortest for new
code?
Has it been considered to annotate the 'testbus' declaration with __free
and to remove the kfree(testbus) call? See also <linux/cleanup.h>
+ switch (j) {
+ case TSTBUS_UAWM:
+ prefix = "TSTBUS_UAWM ";
+ break;
+ case TSTBUS_UARM:
+ prefix = "TSTBUS_UARM ";
+ break;
+ case TSTBUS_TXUC:
+ prefix = "TSTBUS_TXUC ";
+ break;
+ case TSTBUS_RXUC:
+ prefix = "TSTBUS_RXUC ";
+ break;
+ case TSTBUS_DFC:
+ prefix = "TSTBUS_DFC ";
+ break;
+ case TSTBUS_TRLUT:
+ prefix = "TSTBUS_TRLUT ";
+ break;
+ case TSTBUS_TMRLUT:
+ prefix = "TSTBUS_TMRLUT ";
+ break;
+ case TSTBUS_OCSC:
+ prefix = "TSTBUS_OCSC ";
+ break;
+ case TSTBUS_UTP_HCI:
+ prefix = "TSTBUS_UTP_HCI ";
+ break;
+ case TSTBUS_COMBINED:
+ prefix = "TSTBUS_COMBINED ";
+ break;
+ case TSTBUS_WRAPPER:
+ prefix = "TSTBUS_WRAPPER ";
+ break;
+ case TSTBUS_UNIPRO:
+ nminor = 256;
+ prefix = "TSTBUS_UNIPRO ";
+ break;
+ default:
+ break;
+ }
Has it been considered to convert the above switch-statement into an
array lookup?
@@ -1682,6 +1751,10 @@ static void ufs_qcom_dump_dbg_regs(struct ufs_hba *hba)
ufs_qcom_dump_mcq_hci_regs(hba);
usleep_range(1000, 1100);
}
+ ufshcd_dump_regs(hba, UFS_TEST_BUS, 4, "UFS_TEST_BUS ");
+ usleep_range(1000, 1100);
+ ufs_qcom_dump_testbus(hba);
+ usleep_range(1000, 1100);
}
}
Please add a comment that explains why the usleep_range() calls are
present.
Thanks,
Bart.
