Re: [RFC PATCH v1] ufs: relocate turning off device power sources

[Can Guo <cang@codeaurora.org>]

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@samsung.com>
> ---
>  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.