From 921a880827498e501921bf034012a97e554550d2 Mon Sep 17 00:00:00 2001 From: Andrew Halaney Date: Thu, 1 Dec 2022 17:08:07 -0600 Subject: [PATCH 1/4] scsi: ufs: ufs-qcom: Drop unnecessary NULL checks This code path is only called through one function, and the HBA struct is already accessed in ufshcd_vops_dbg_register_dump() prior to calling so there is no way for it to be NULL. Likewise, the print_fn callback is always supplied within this driver and is always provided. Signed-off-by: Andrew Halaney Reviewed-by: Manivannan Sadhasivam Reviewed-by: Asutosh Das Signed-off-by: Martin K. Petersen --- drivers/ufs/host/ufs-qcom.c | 9 --------- 1 file changed, 9 deletions(-) diff --git a/drivers/ufs/host/ufs-qcom.c b/drivers/ufs/host/ufs-qcom.c index 8ad1415e10b6..70e25f9f8ca8 100644 --- a/drivers/ufs/host/ufs-qcom.c +++ b/drivers/ufs/host/ufs-qcom.c @@ -1202,15 +1202,6 @@ static void ufs_qcom_print_hw_debug_reg_all(struct ufs_hba *hba, u32 reg; struct ufs_qcom_host *host; - if (unlikely(!hba)) { - pr_err("%s: hba is NULL\n", __func__); - return; - } - if (unlikely(!print_fn)) { - dev_err(hba->dev, "%s: print_fn is NULL\n", __func__); - return; - } - host = ufshcd_get_variant(hba); if (!(host->dbg_print_en & UFS_QCOM_DBG_PRINT_REGS_EN)) return; From 50a427a00c85995009a134d443949e5128a84191 Mon Sep 17 00:00:00 2001 From: Andrew Halaney Date: Thu, 1 Dec 2022 17:08:08 -0600 Subject: [PATCH 2/4] scsi: ufs: ufs-qcom: Clean up dbg_register_dump The current implementation has abstractions that don't give any benefits. The print_fn callback (and its only callback implementation, ufs_qcom_dump_regs_wrapper()) was only used by ufs_qcom_print_hw_debug_reg_all() and just multiplies len by 4 before calling ufshcd_dump_regs(). ufs_qcom_print_hw_debug_reg_all() is only called by ufs_qcom_dump_dbg_regs(). There's no real gain in those abstractions, so let's just do the work directly in ufs_qcom_dump_dbg_regs() (the dbg_register_dump callback). Signed-off-by: Andrew Halaney Reviewed-by: Manivannan Sadhasivam Reviewed-by: Asutosh Das Signed-off-by: Martin K. Petersen --- drivers/ufs/host/ufs-qcom.c | 106 ++++++++++++++++-------------------- 1 file changed, 47 insertions(+), 59 deletions(-) diff --git a/drivers/ufs/host/ufs-qcom.c b/drivers/ufs/host/ufs-qcom.c index 70e25f9f8ca8..1b0dfbbdcdf3 100644 --- a/drivers/ufs/host/ufs-qcom.c +++ b/drivers/ufs/host/ufs-qcom.c @@ -52,12 +52,6 @@ static struct ufs_qcom_host *rcdev_to_ufs_host(struct reset_controller_dev *rcd) return container_of(rcd, struct ufs_qcom_host, rcdev); } -static void ufs_qcom_dump_regs_wrapper(struct ufs_hba *hba, int offset, int len, - const char *prefix, void *priv) -{ - ufshcd_dump_regs(hba, offset, len * 4, prefix); -} - static int ufs_qcom_host_clk_get(struct device *dev, const char *name, struct clk **clk_out, bool optional) { @@ -1195,58 +1189,6 @@ static int ufs_qcom_clk_scale_notify(struct ufs_hba *hba, return err; } -static void ufs_qcom_print_hw_debug_reg_all(struct ufs_hba *hba, - void *priv, void (*print_fn)(struct ufs_hba *hba, - int offset, int num_regs, const char *str, void *priv)) -{ - u32 reg; - struct ufs_qcom_host *host; - - host = ufshcd_get_variant(hba); - if (!(host->dbg_print_en & UFS_QCOM_DBG_PRINT_REGS_EN)) - return; - - reg = ufs_qcom_get_debug_reg_offset(host, UFS_UFS_DBG_RD_REG_OCSC); - print_fn(hba, reg, 44, "UFS_UFS_DBG_RD_REG_OCSC ", priv); - - reg = ufshcd_readl(hba, REG_UFS_CFG1); - reg |= UTP_DBG_RAMS_EN; - ufshcd_writel(hba, reg, REG_UFS_CFG1); - - reg = ufs_qcom_get_debug_reg_offset(host, UFS_UFS_DBG_RD_EDTL_RAM); - print_fn(hba, reg, 32, "UFS_UFS_DBG_RD_EDTL_RAM ", priv); - - reg = ufs_qcom_get_debug_reg_offset(host, UFS_UFS_DBG_RD_DESC_RAM); - print_fn(hba, reg, 128, "UFS_UFS_DBG_RD_DESC_RAM ", priv); - - reg = ufs_qcom_get_debug_reg_offset(host, UFS_UFS_DBG_RD_PRDT_RAM); - print_fn(hba, reg, 64, "UFS_UFS_DBG_RD_PRDT_RAM ", priv); - - /* clear bit 17 - UTP_DBG_RAMS_EN */ - ufshcd_rmwl(hba, UTP_DBG_RAMS_EN, 0, REG_UFS_CFG1); - - reg = ufs_qcom_get_debug_reg_offset(host, UFS_DBG_RD_REG_UAWM); - print_fn(hba, reg, 4, "UFS_DBG_RD_REG_UAWM ", priv); - - reg = ufs_qcom_get_debug_reg_offset(host, UFS_DBG_RD_REG_UARM); - print_fn(hba, reg, 4, "UFS_DBG_RD_REG_UARM ", priv); - - reg = ufs_qcom_get_debug_reg_offset(host, UFS_DBG_RD_REG_TXUC); - print_fn(hba, reg, 48, "UFS_DBG_RD_REG_TXUC ", priv); - - reg = ufs_qcom_get_debug_reg_offset(host, UFS_DBG_RD_REG_RXUC); - print_fn(hba, reg, 27, "UFS_DBG_RD_REG_RXUC ", priv); - - reg = ufs_qcom_get_debug_reg_offset(host, UFS_DBG_RD_REG_DFC); - print_fn(hba, reg, 19, "UFS_DBG_RD_REG_DFC ", priv); - - reg = ufs_qcom_get_debug_reg_offset(host, UFS_DBG_RD_REG_TRLUT); - print_fn(hba, reg, 34, "UFS_DBG_RD_REG_TRLUT ", priv); - - reg = ufs_qcom_get_debug_reg_offset(host, UFS_DBG_RD_REG_TMRLUT); - print_fn(hba, reg, 9, "UFS_DBG_RD_REG_TMRLUT ", priv); -} - static void ufs_qcom_enable_test_bus(struct ufs_qcom_host *host) { if (host->dbg_print_en & UFS_QCOM_DBG_PRINT_TEST_BUS_EN) { @@ -1365,10 +1307,56 @@ int ufs_qcom_testbus_config(struct ufs_qcom_host *host) static void ufs_qcom_dump_dbg_regs(struct ufs_hba *hba) { + u32 reg; + struct ufs_qcom_host *host; + + host = ufshcd_get_variant(hba); + ufshcd_dump_regs(hba, REG_UFS_SYS1CLK_1US, 16 * 4, "HCI Vendor Specific Registers "); - ufs_qcom_print_hw_debug_reg_all(hba, NULL, ufs_qcom_dump_regs_wrapper); + if (!(host->dbg_print_en & UFS_QCOM_DBG_PRINT_REGS_EN)) + return; + + reg = ufs_qcom_get_debug_reg_offset(host, UFS_UFS_DBG_RD_REG_OCSC); + ufshcd_dump_regs(hba, reg, 44 * 4, "UFS_UFS_DBG_RD_REG_OCSC "); + + reg = ufshcd_readl(hba, REG_UFS_CFG1); + reg |= UTP_DBG_RAMS_EN; + ufshcd_writel(hba, reg, REG_UFS_CFG1); + + reg = ufs_qcom_get_debug_reg_offset(host, UFS_UFS_DBG_RD_EDTL_RAM); + ufshcd_dump_regs(hba, reg, 32 * 4, "UFS_UFS_DBG_RD_EDTL_RAM "); + + reg = ufs_qcom_get_debug_reg_offset(host, UFS_UFS_DBG_RD_DESC_RAM); + ufshcd_dump_regs(hba, reg, 128 * 4, "UFS_UFS_DBG_RD_DESC_RAM "); + + reg = ufs_qcom_get_debug_reg_offset(host, UFS_UFS_DBG_RD_PRDT_RAM); + ufshcd_dump_regs(hba, reg, 64 * 4, "UFS_UFS_DBG_RD_PRDT_RAM "); + + /* clear bit 17 - UTP_DBG_RAMS_EN */ + ufshcd_rmwl(hba, UTP_DBG_RAMS_EN, 0, REG_UFS_CFG1); + + reg = ufs_qcom_get_debug_reg_offset(host, UFS_DBG_RD_REG_UAWM); + ufshcd_dump_regs(hba, reg, 4 * 4, "UFS_DBG_RD_REG_UAWM "); + + reg = ufs_qcom_get_debug_reg_offset(host, UFS_DBG_RD_REG_UARM); + ufshcd_dump_regs(hba, reg, 4 * 4, "UFS_DBG_RD_REG_UARM "); + + reg = ufs_qcom_get_debug_reg_offset(host, UFS_DBG_RD_REG_TXUC); + ufshcd_dump_regs(hba, reg, 48 * 4, "UFS_DBG_RD_REG_TXUC "); + + reg = ufs_qcom_get_debug_reg_offset(host, UFS_DBG_RD_REG_RXUC); + ufshcd_dump_regs(hba, reg, 27 * 4, "UFS_DBG_RD_REG_RXUC "); + + reg = ufs_qcom_get_debug_reg_offset(host, UFS_DBG_RD_REG_DFC); + ufshcd_dump_regs(hba, reg, 19 * 4, "UFS_DBG_RD_REG_DFC "); + + reg = ufs_qcom_get_debug_reg_offset(host, UFS_DBG_RD_REG_TRLUT); + ufshcd_dump_regs(hba, reg, 34 * 4, "UFS_DBG_RD_REG_TRLUT "); + + reg = ufs_qcom_get_debug_reg_offset(host, UFS_DBG_RD_REG_TMRLUT); + ufshcd_dump_regs(hba, reg, 9 * 4, "UFS_DBG_RD_REG_TMRLUT "); } /** From e4ce23fba366e5b0629b13d9c42e68fab7fda002 Mon Sep 17 00:00:00 2001 From: Andrew Halaney Date: Thu, 1 Dec 2022 17:08:09 -0600 Subject: [PATCH 3/4] scsi: ufs: ufs-qcom: Remove usage of dbg_print_en This bitmask is unconditionally set in the current driver, so all conditionals using it can be considered bit rot. Let's take the current default conditional path everywhere and remove dbg_print_en from the driver. Signed-off-by: Andrew Halaney Reviewed-by: Manivannan Sadhasivam Reviewed-by: Asutosh Das Signed-off-by: Martin K. Petersen --- drivers/ufs/host/ufs-qcom.c | 18 +++--------------- drivers/ufs/host/ufs-qcom.h | 11 ----------- 2 files changed, 3 insertions(+), 26 deletions(-) diff --git a/drivers/ufs/host/ufs-qcom.c b/drivers/ufs/host/ufs-qcom.c index 1b0dfbbdcdf3..b1fcff1fad0c 100644 --- a/drivers/ufs/host/ufs-qcom.c +++ b/drivers/ufs/host/ufs-qcom.c @@ -22,9 +22,6 @@ #include #include -#define UFS_QCOM_DEFAULT_DBG_PRINT_EN \ - (UFS_QCOM_DBG_PRINT_REGS_EN | UFS_QCOM_DBG_PRINT_TEST_BUS_EN) - enum { TSTBUS_UAWM, TSTBUS_UARM, @@ -1040,7 +1037,6 @@ static int ufs_qcom_init(struct ufs_hba *hba) if (hba->dev->id < MAX_UFS_QCOM_HOSTS) ufs_qcom_hosts[hba->dev->id] = host; - host->dbg_print_en |= UFS_QCOM_DEFAULT_DBG_PRINT_EN; ufs_qcom_get_default_testbus_cfg(host); err = ufs_qcom_testbus_config(host); if (err) { @@ -1191,14 +1187,9 @@ static int ufs_qcom_clk_scale_notify(struct ufs_hba *hba, static void ufs_qcom_enable_test_bus(struct ufs_qcom_host *host) { - if (host->dbg_print_en & UFS_QCOM_DBG_PRINT_TEST_BUS_EN) { - ufshcd_rmwl(host->hba, UFS_REG_TEST_BUS_EN, - UFS_REG_TEST_BUS_EN, REG_UFS_CFG1); - ufshcd_rmwl(host->hba, TEST_BUS_EN, TEST_BUS_EN, REG_UFS_CFG1); - } else { - ufshcd_rmwl(host->hba, UFS_REG_TEST_BUS_EN, 0, REG_UFS_CFG1); - ufshcd_rmwl(host->hba, TEST_BUS_EN, 0, REG_UFS_CFG1); - } + ufshcd_rmwl(host->hba, UFS_REG_TEST_BUS_EN, + UFS_REG_TEST_BUS_EN, REG_UFS_CFG1); + ufshcd_rmwl(host->hba, TEST_BUS_EN, TEST_BUS_EN, REG_UFS_CFG1); } static void ufs_qcom_get_default_testbus_cfg(struct ufs_qcom_host *host) @@ -1315,9 +1306,6 @@ static void ufs_qcom_dump_dbg_regs(struct ufs_hba *hba) ufshcd_dump_regs(hba, REG_UFS_SYS1CLK_1US, 16 * 4, "HCI Vendor Specific Registers "); - if (!(host->dbg_print_en & UFS_QCOM_DBG_PRINT_REGS_EN)) - return; - reg = ufs_qcom_get_debug_reg_offset(host, UFS_UFS_DBG_RD_REG_OCSC); ufshcd_dump_regs(hba, reg, 44 * 4, "UFS_UFS_DBG_RD_REG_OCSC "); diff --git a/drivers/ufs/host/ufs-qcom.h b/drivers/ufs/host/ufs-qcom.h index 44466a395bb5..e567e4636357 100644 --- a/drivers/ufs/host/ufs-qcom.h +++ b/drivers/ufs/host/ufs-qcom.h @@ -113,15 +113,6 @@ enum { MASK_CLK_NS_REG = 0xFFFC00, }; -/* QCOM UFS debug print bit mask */ -#define UFS_QCOM_DBG_PRINT_REGS_EN BIT(0) -#define UFS_QCOM_DBG_PRINT_ICE_REGS_EN BIT(1) -#define UFS_QCOM_DBG_PRINT_TEST_BUS_EN BIT(2) - -#define UFS_QCOM_DBG_PRINT_ALL \ - (UFS_QCOM_DBG_PRINT_REGS_EN | UFS_QCOM_DBG_PRINT_ICE_REGS_EN | \ - UFS_QCOM_DBG_PRINT_TEST_BUS_EN) - /* QUniPro Vendor specific attributes */ #define PA_VS_CONFIG_REG1 0x9000 #define DME_VS_CORE_CLK_CTRL 0xD002 @@ -212,8 +203,6 @@ struct ufs_qcom_host { u32 dev_ref_clk_en_mask; - /* Bitmask for enabling debug prints */ - u32 dbg_print_en; struct ufs_qcom_testbus testbus; /* Reset control of HCI */ From 1026f7d366785d0a1edae731a3d06e9924c3e71d Mon Sep 17 00:00:00 2001 From: Andrew Halaney Date: Thu, 1 Dec 2022 17:08:10 -0600 Subject: [PATCH 4/4] scsi: ufs: ufs-qcom: Use dev_err() where possible dev_err() statements are better to use than pr_err(), so switch to those. In a similar vein, the check on the dev_req_params pointer here is not needed, the two places this function is called never pass in a NULL pointer, so instead of using dev_err() there just remove it. Signed-off-by: Andrew Halaney Reviewed-by: Manivannan Sadhasivam Reviewed-by: Asutosh Das Signed-off-by: Martin K. Petersen --- drivers/ufs/host/ufs-qcom.c | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/drivers/ufs/host/ufs-qcom.c b/drivers/ufs/host/ufs-qcom.c index b1fcff1fad0c..4350c44a6fc7 100644 --- a/drivers/ufs/host/ufs-qcom.c +++ b/drivers/ufs/host/ufs-qcom.c @@ -680,12 +680,6 @@ static int ufs_qcom_pwr_change_notify(struct ufs_hba *hba, struct ufs_dev_params ufs_qcom_cap; int ret = 0; - if (!dev_req_params) { - pr_err("%s: incoming dev_req_params is NULL\n", __func__); - ret = -EINVAL; - goto out; - } - switch (status) { case PRE_CHANGE: ufshcd_init_pwr_dev_param(&ufs_qcom_cap); @@ -709,7 +703,7 @@ static int ufs_qcom_pwr_change_notify(struct ufs_hba *hba, dev_max_params, dev_req_params); if (ret) { - pr_err("%s: failed to determine capabilities\n", + dev_err(hba->dev, "%s: failed to determine capabilities\n", __func__); goto out; }