Message ID | 1606194312-25378-1-git-send-email-cang@codeaurora.org |
---|---|
State | New |
Headers | show |
Series | scsi: ufs: Don't disable core_clk_unipro if the link is active | expand |
Hi Stanley, On 2020-11-24 13:20, Stanley Chu wrote: > Hi Can, > > On Mon, 2020-11-23 at 21:05 -0800, Can Guo wrote: >> If we want to disable clocks but still keep the link active, both >> ref_clk >> and core_clk_unipro should be skipped. >> > > "core_clk_unipro" seems used by ufs-qcom only and not defined in the > UFS > platform binding document: ufshcd_pltfrm.txt. Agree. > > Could you please add the definition first and then it would be > reasonable to be used in common driver? > > Or, how about add a flag in struct ufs_clk_info indicating if this > clock > needs be ON to keep the link active? The flag could be set properly by > vendor initialization functions. In this way, we can also remove the > hard-coded "ref_clk" in __ufshcd_setup_clocks(). This seems better, I will upload next version to incorporate the idea after it is tested. Thanks, Can Guo. > > Thanks. > Stanley Chu > >> Signed-off-by: Can Guo <cang@codeaurora.org> >> >> diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c >> index a7857f6..69c2e91 100644 >> --- a/drivers/scsi/ufs/ufshcd.c >> +++ b/drivers/scsi/ufs/ufshcd.c >> @@ -222,7 +222,7 @@ static int ufshcd_clear_tm_cmd(struct ufs_hba >> *hba, int tag); >> static void ufshcd_hba_exit(struct ufs_hba *hba); >> static int ufshcd_probe_hba(struct ufs_hba *hba, bool async); >> static int __ufshcd_setup_clocks(struct ufs_hba *hba, bool on, >> - bool skip_ref_clk); >> + bool keep_link_active); >> static int ufshcd_setup_clocks(struct ufs_hba *hba, bool on); >> static int ufshcd_uic_hibern8_enter(struct ufs_hba *hba); >> static inline void ufshcd_add_delay_before_dme_cmd(struct ufs_hba >> *hba); >> @@ -1710,7 +1710,6 @@ static void ufshcd_gate_work(struct work_struct >> *work) >> if (!ufshcd_is_link_active(hba)) >> ufshcd_setup_clocks(hba, false); >> else >> - /* If link is active, device ref_clk can't be switched off */ >> __ufshcd_setup_clocks(hba, false, true); >> >> /* >> @@ -7991,7 +7990,7 @@ static int ufshcd_init_hba_vreg(struct ufs_hba >> *hba) >> } >> >> static int __ufshcd_setup_clocks(struct ufs_hba *hba, bool on, >> - bool skip_ref_clk) >> + bool keep_link_active) >> { >> int ret = 0; >> struct ufs_clk_info *clki; >> @@ -8009,7 +8008,13 @@ static int __ufshcd_setup_clocks(struct ufs_hba >> *hba, bool on, >> >> list_for_each_entry(clki, head, list) { >> if (!IS_ERR_OR_NULL(clki->clk)) { >> - if (skip_ref_clk && !strcmp(clki->name, "ref_clk")) >> + /* >> + * To keep link active, ref_clk and core_clk_unipro >> + * should be kept ON. >> + */ >> + if (keep_link_active && >> + (!strcmp(clki->name, "ref_clk") || >> + !strcmp(clki->name, "core_clk_unipro"))) >> continue; >> >> clk_state_changed = on ^ clki->enabled; >> @@ -8580,7 +8585,6 @@ static int ufshcd_suspend(struct ufs_hba *hba, >> enum ufs_pm_op pm_op) >> if (!ufshcd_is_link_active(hba)) >> ufshcd_setup_clocks(hba, false); >> else >> - /* If link is active, device ref_clk can't be switched off */ >> __ufshcd_setup_clocks(hba, false, true); >> >> if (ufshcd_is_clkgating_allowed(hba)) {
diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c index a7857f6..69c2e91 100644 --- a/drivers/scsi/ufs/ufshcd.c +++ b/drivers/scsi/ufs/ufshcd.c @@ -222,7 +222,7 @@ static int ufshcd_clear_tm_cmd(struct ufs_hba *hba, int tag); static void ufshcd_hba_exit(struct ufs_hba *hba); static int ufshcd_probe_hba(struct ufs_hba *hba, bool async); static int __ufshcd_setup_clocks(struct ufs_hba *hba, bool on, - bool skip_ref_clk); + bool keep_link_active); static int ufshcd_setup_clocks(struct ufs_hba *hba, bool on); static int ufshcd_uic_hibern8_enter(struct ufs_hba *hba); static inline void ufshcd_add_delay_before_dme_cmd(struct ufs_hba *hba); @@ -1710,7 +1710,6 @@ static void ufshcd_gate_work(struct work_struct *work) if (!ufshcd_is_link_active(hba)) ufshcd_setup_clocks(hba, false); else - /* If link is active, device ref_clk can't be switched off */ __ufshcd_setup_clocks(hba, false, true); /* @@ -7991,7 +7990,7 @@ static int ufshcd_init_hba_vreg(struct ufs_hba *hba) } static int __ufshcd_setup_clocks(struct ufs_hba *hba, bool on, - bool skip_ref_clk) + bool keep_link_active) { int ret = 0; struct ufs_clk_info *clki; @@ -8009,7 +8008,13 @@ static int __ufshcd_setup_clocks(struct ufs_hba *hba, bool on, list_for_each_entry(clki, head, list) { if (!IS_ERR_OR_NULL(clki->clk)) { - if (skip_ref_clk && !strcmp(clki->name, "ref_clk")) + /* + * To keep link active, ref_clk and core_clk_unipro + * should be kept ON. + */ + if (keep_link_active && + (!strcmp(clki->name, "ref_clk") || + !strcmp(clki->name, "core_clk_unipro"))) continue; clk_state_changed = on ^ clki->enabled; @@ -8580,7 +8585,6 @@ static int ufshcd_suspend(struct ufs_hba *hba, enum ufs_pm_op pm_op) if (!ufshcd_is_link_active(hba)) ufshcd_setup_clocks(hba, false); else - /* If link is active, device ref_clk can't be switched off */ __ufshcd_setup_clocks(hba, false, true); if (ufshcd_is_clkgating_allowed(hba)) {
If we want to disable clocks but still keep the link active, both ref_clk and core_clk_unipro should be skipped. Signed-off-by: Can Guo <cang@codeaurora.org>