On 2021-05-13 03:57, Bart Van Assche wrote:
If the rpm_lvl and spm_lvl sysfs attributes indicate that
ufshcd_suspend()
should keep the link active, re-enable clock gating instead of
disabling
clocks.
Suggested-by: Jaegeuk Kim <jaegeuk@xxxxxxxxxx>
Cc: Can Guo <cang@xxxxxxxxxxxxxx>
Cc: Alim Akhtar <alim.akhtar@xxxxxxxxxxx>
Cc: Avri Altman <avri.altman@xxxxxxx>
Cc: Stanley Chu <stanley.chu@xxxxxxxxxxxx>
Cc: Bean Huo <beanhuo@xxxxxxxxxx>
Cc: Adrian Hunter <adrian.hunter@xxxxxxxxx>
Signed-off-by: Bart Van Assche <bvanassche@xxxxxxx>
---
drivers/scsi/ufs/ufshcd.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index f540b0cc253f..c96e36aab989 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -8690,9 +8690,8 @@ static int ufshcd_suspend(struct ufs_hba *hba,
enum ufs_pm_op pm_op)
ufshcd_clk_scaling_suspend(hba, true);
if (req_dev_pwr_mode == UFS_ACTIVE_PWR_MODE &&
- req_link_state == UIC_LINK_ACTIVE_STATE) {
- goto disable_clks;
- }
+ req_link_state == UIC_LINK_ACTIVE_STATE)
+ goto enable_gating;
if ((req_dev_pwr_mode == hba->curr_dev_pwr_mode) &&
(req_link_state == hba->uic_link_state))
@@ -8754,7 +8753,6 @@ static int ufshcd_suspend(struct ufs_hba *hba,
enum ufs_pm_op pm_op)
if (ret)
goto set_dev_active;
-disable_clks:
/*
* Call vendor specific suspend callback. As these callbacks may
access
* vendor specific host controller register space call them before
the
Hi Bart,
The change is unnecessary, ufshcd_suspend() is indeed keeping link alive
even if
we are disabling clocks. In __ufshcd_setup_clocks(), we have checks on
clock sources
so that we leave certain clock sources ON if the link is alive. And we
have many
of our customers tested the case rpm/spm_lvl == 0, it is working well.
Please check
below changes:
https://lore.kernel.org/patchwork/patch/1345336/
https://lore.kernel.org/patchwork/patch/1345337/
With this change, after suspend (rpm/spm_lvl == 0), leaving clock gating
running is risky:
1. clock gating may run into concurrency with resume.
2. In ufshcd_resume(), we also have a ufshcd_release(), it will cause
unbalanced usage of clock gating.
And it seems quite opposite from what you want - you want to keep link
alive but you are leaving clock gating enabled, then when clock gating
kicks start, it will put the link into hibern8, but not keep link alive.
Thanks,
Can Guo.
