From 5c3481e0887ae4545552f2ecb3b7a4c94649a2e4 Mon Sep 17 00:00:00 2001 From: Can Guo Date: Thu, 23 Apr 2020 18:59:33 +0800 Subject: [PATCH 1/4] scsi: ufs: add checks before setting clk-gating states Clock gating features can be turned on/off selectively which means its state information is only important if it is enabled. This change makes sure that we only look at state of clk-gating if it is enabled. Change-Id: I201cb65d4a9712ba7c0f7022d7323cdf6dc144c9 Signed-off-by: Can Guo --- drivers/scsi/ufs/ufshcd.c | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c index d32b4a4dfc6d..f86f7ac777f7 100644 --- a/drivers/scsi/ufs/ufshcd.c +++ b/drivers/scsi/ufs/ufshcd.c @@ -1841,6 +1841,8 @@ static void ufshcd_init_clk_gating(struct ufs_hba *hba) if (!ufshcd_is_clkgating_allowed(hba)) return; + hba->clk_gating.state = CLKS_ON; + hba->clk_gating.delay_ms = 150; INIT_DELAYED_WORK(&hba->clk_gating.gate_work, ufshcd_gate_work); INIT_WORK(&hba->clk_gating.ungate_work, ufshcd_ungate_work); @@ -2529,7 +2531,8 @@ static int ufshcd_queuecommand(struct Scsi_Host *host, struct scsi_cmnd *cmd) clear_bit_unlock(tag, &hba->lrb_in_use); goto out; } - WARN_ON(hba->clk_gating.state != CLKS_ON); + if (ufshcd_is_clkgating_allowed(hba)) + WARN_ON(hba->clk_gating.state != CLKS_ON); lrbp = &hba->lrb[tag]; @@ -8176,8 +8179,11 @@ disable_clks: /* If link is active, device ref_clk can't be switched off */ __ufshcd_setup_clocks(hba, false, true); - hba->clk_gating.state = CLKS_OFF; - trace_ufshcd_clk_gating(dev_name(hba->dev), hba->clk_gating.state); + if (ufshcd_is_clkgating_allowed(hba)) { + hba->clk_gating.state = CLKS_OFF; + trace_ufshcd_clk_gating(dev_name(hba->dev), + hba->clk_gating.state); + } /* Put the host controller in low power mode if possible */ ufshcd_hba_vreg_set_lpm(hba); @@ -8315,6 +8321,11 @@ disable_irq_and_vops_clks: if (hba->clk_scaling.is_allowed) ufshcd_suspend_clkscaling(hba); ufshcd_setup_clocks(hba, false); + if (ufshcd_is_clkgating_allowed(hba)) { + hba->clk_gating.state = CLKS_OFF; + trace_ufshcd_clk_gating(dev_name(hba->dev), + hba->clk_gating.state); + } out: hba->pm_op_in_progress = 0; if (ret) From 4b6fb5312ea34feb1d6eba91d338a76194e42a51 Mon Sep 17 00:00:00 2001 From: Can Guo Date: Wed, 3 Jun 2020 13:10:50 +0800 Subject: [PATCH 2/4] scsi: ufs-qcom: Dump bus test and PHY registers only if sleep is allowed Dumping bus test and PHY registers needs to sleep a bit intermittently as there are too many of them. Skip them if it is called from contexts where sleep is not allowed. Change-Id: Ia6b77a66095b72749e2ee85a2d97fb9608867a0b Signed-off-by: Can Guo --- drivers/scsi/ufs/ufs-qcom.c | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/drivers/scsi/ufs/ufs-qcom.c b/drivers/scsi/ufs/ufs-qcom.c index b4217f45ac9a..62a812c1450d 100644 --- a/drivers/scsi/ufs/ufs-qcom.c +++ b/drivers/scsi/ufs/ufs-qcom.c @@ -2778,14 +2778,17 @@ static void ufs_qcom_dump_dbg_regs(struct ufs_hba *hba) /* sleep a bit intermittently as we are dumping too much data */ ufs_qcom_print_hw_debug_reg_all(hba, NULL, ufs_qcom_dump_regs_wrapper); - usleep_range(1000, 1100); - ufs_qcom_testbus_read(hba); - usleep_range(1000, 1100); - ufs_qcom_print_unipro_testbus(hba); - usleep_range(1000, 1100); - ufs_qcom_print_utp_hci_testbus(hba); - usleep_range(1000, 1100); - ufs_qcom_phy_dbg_register_dump(phy); + + if (in_task()) { + usleep_range(1000, 1100); + ufs_qcom_testbus_read(hba); + usleep_range(1000, 1100); + ufs_qcom_print_unipro_testbus(hba); + usleep_range(1000, 1100); + ufs_qcom_print_utp_hci_testbus(hba); + usleep_range(1000, 1100); + ufs_qcom_phy_dbg_register_dump(phy); + } } /* From 8611eb0c52b00186d7342dec32d6a6157899243b Mon Sep 17 00:00:00 2001 From: Can Guo Date: Wed, 10 Jun 2020 10:15:56 +0800 Subject: [PATCH 3/4] scsi: ufs: Move ufshcd_vops_dbg_register_dump out of ufshcd_print_host_regs If ufshcd_print_host_regs() is called from IRQ context, some registers like test bus and UFS PHY will not be printed by ufshcd_vops_dbg_register_dump() due to sleep is not allowed. Move ufshcd_vops_dbg_register_dump out of ufshcd_print_host_regs and explicitly call ufshcd_vops_dbg_register_dump where we need to. Change-Id: Icde9a71fc578c43af04a5329b04353d87217ce4f Signed-off-by: Can Guo --- drivers/scsi/ufs/ufshcd.c | 24 +++++++++++++++++------- 1 file changed, 17 insertions(+), 7 deletions(-) diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c index f86f7ac777f7..6c7681699f26 100644 --- a/drivers/scsi/ufs/ufshcd.c +++ b/drivers/scsi/ufs/ufshcd.c @@ -418,8 +418,6 @@ static void ufshcd_print_host_regs(struct ufs_hba *hba) ufshcd_print_clk_freqs(hba); - ufshcd_vops_dbg_register_dump(hba); - ufshcd_crypto_debug(hba); } @@ -3912,6 +3910,7 @@ out: ufshcd_print_host_state(hba); ufshcd_print_pwr_info(hba); ufshcd_print_host_regs(hba); + ufshcd_vops_dbg_register_dump(hba); } spin_lock_irqsave(hba->host->host_lock, flags); @@ -4623,6 +4622,7 @@ out: ufshcd_print_host_state(hba); ufshcd_print_pwr_info(hba); ufshcd_print_host_regs(hba); + ufshcd_vops_dbg_register_dump(hba); } return ret; } @@ -4979,8 +4979,10 @@ ufshcd_transfer_rsp_status(struct ufs_hba *hba, struct ufshcd_lrb *lrbp) dev_err(hba->dev, "OCS error from controller = %x for tag %d\n", ocs, lrbp->task_tag); - ufshcd_print_host_regs(hba); ufshcd_print_host_state(hba); + ufshcd_print_pwr_info(hba); + ufshcd_print_host_regs(hba); + ufshcd_vops_dbg_register_dump(hba); break; } /* end of switch */ @@ -5533,6 +5535,11 @@ static void ufshcd_err_handler(struct work_struct *work) if (!ret) goto skip_err_handling; } + + spin_unlock_irqrestore(hba->host->host_lock, flags); + ufshcd_vops_dbg_register_dump(hba); + spin_lock_irqsave(hba->host->host_lock, flags); + if ((hba->saved_err & INT_FATAL_ERRORS) || (hba->saved_err & UFSHCD_UIC_HIBERN8_MASK) || ((hba->saved_err & UIC_ERROR) && @@ -5780,8 +5787,9 @@ static irqreturn_t ufshcd_check_errors(struct ufs_hba *hba) __func__, hba->saved_err, hba->saved_uic_err); - ufshcd_print_host_regs(hba); + ufshcd_print_host_state(hba); ufshcd_print_pwr_info(hba); + ufshcd_print_host_regs(hba); ufshcd_print_tmrs(hba, hba->outstanding_tasks); ufshcd_print_trs(hba, hba->outstanding_reqs, pr_prdt); @@ -6358,10 +6366,11 @@ static int ufshcd_abort(struct scsi_cmnd *cmd) scsi_print_command(hba->lrb[tag].cmd); if (!hba->req_abort_count) { ufshcd_update_reg_hist(&hba->ufs_stats.task_abort, 0); - ufshcd_print_host_regs(hba); + ufshcd_print_trs(hba, 1 << tag, true); ufshcd_print_host_state(hba); ufshcd_print_pwr_info(hba); - ufshcd_print_trs(hba, 1 << tag, true); + ufshcd_print_host_regs(hba); + ufshcd_vops_dbg_register_dump(hba); } else { ufshcd_print_trs(hba, 1 << tag, false); } @@ -8744,8 +8753,9 @@ int ufshcd_init(struct ufs_hba *hba, void __iomem *mmio_base, unsigned int irq) err = ufshcd_hba_enable(hba); if (err) { dev_err(hba->dev, "Host controller enable failed\n"); - ufshcd_print_host_regs(hba); ufshcd_print_host_state(hba); + ufshcd_print_host_regs(hba); + ufshcd_vops_dbg_register_dump(hba); goto out_remove_scsi_host; } From c2825345ea65ee89277f7a276a4b8a6b703a812a Mon Sep 17 00:00:00 2001 From: Can Guo Date: Wed, 10 Jun 2020 11:12:23 +0800 Subject: [PATCH 4/4] scsi: ufs-qcom: Remove unnecessary devm_kfree Resources allocated with devm_* are managed by device driver framework, meaning the resources will be automatically released when the associated device is removed. Besides, having a devm_kfree here would cause a use-after-free scenario: after hba->host is freed up, data inside hba->host shall still be accessed when device driver framework frees up all resources associated with hba->dev. hba->host->rcdev, for example, shall be used by func devm_reset_controller_release() when resources associated with hba->dev are freed up. Change-Id: I90bab41c34450a1c814d863b9f3653228f5eeb95 Signed-off-by: Can Guo --- drivers/scsi/ufs/ufs-qcom.c | 1 - 1 file changed, 1 deletion(-) diff --git a/drivers/scsi/ufs/ufs-qcom.c b/drivers/scsi/ufs/ufs-qcom.c index 62a812c1450d..5b79951bf450 100644 --- a/drivers/scsi/ufs/ufs-qcom.c +++ b/drivers/scsi/ufs/ufs-qcom.c @@ -2311,7 +2311,6 @@ out_disable_vddp: out_phy_exit: phy_exit(host->generic_phy); out_variant_clear: - devm_kfree(dev, host); ufshcd_set_variant(hba, NULL); out: return err;