On 2020-12-21 09:44, Kiwoong Kim wrote:
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.
Yes, it does cover your fix as you can tell from V1.
2. What does he plan to post?
Within today.
Thanks,
Can Guo.
What do you think?
Thanks.
Kiwoong Kim
Thanks,
Can Guo.
