> 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? Two questions. 1. If his patch covers what I said, it's okay. 2. What does he plan to post? What do you think? Thanks. Kiwoong Kim > > Thanks, > > Can Guo.
