On 2020-12-19 14:27, Kiwoong Kim wrote:
UFS specification says that while powering off the device,
RST_n signal and REF_CLK signal should be between VSS and VCCQ.
One of ways to make it is to drive both RST_n and REF_CLK to low.
However, the current location of turning off them doesn't
guarantee that. ufshcd_link_state_transition make enter hibern8
but it's not supposed to adjust the level of REF_CLK. Adding
ufshcd_vops_device_reset isn't proper because it just drives the
level of RST_n to low and then to high, not keep low.
In this situation, it could make those levels be
diverged from those proper ranges with actual hardware situations,
especially right when the driver turns off power.
The easist way to make it is just to move the location of turning
off because lowering the levels can be enabled through
the callbacks named suspend and setup_clocks.
Signed-off-by: Kiwoong Kim <kwmad.kim@xxxxxxxxxxx>
---
drivers/scsi/ufs/ufshcd.c | 9 +++++++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index 92d433d..dab9840 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -8633,8 +8633,6 @@ static int ufshcd_suspend(struct ufs_hba *hba,
enum ufs_pm_op pm_op)
if (ret)
goto set_dev_active;
- ufshcd_vreg_set_lpm(hba);
-
disable_clks:
/*
* Call vendor specific suspend callback. As these callbacks may
access
@@ -8662,6 +8660,13 @@ static int ufshcd_suspend(struct ufs_hba *hba,
enum ufs_pm_op pm_op)
hba->clk_gating.state);
}
+ /*
+ * It should follows driving RST_n and REF_CLK
+ * in the range specified in UFS specification
+ */
+ if (!ufshcd_is_ufs_dev_active(hba))
+ ufshcd_vreg_set_lpm(hba);
+
/* Put the host controller in low power mode if possible */
ufshcd_hba_vreg_set_lpm(hba);
goto out;
Ziqi Chen has a change to totally fix the UFS power-on/off spec
violation,
see https://lore.kernel.org/patchwork/patch/1351118/ and he is working
on V2,
can we wait for his change?
Thanks,
Can Guo.
