[PATCH v1] scsi: ufs: correct ufshcd_shutdown flow

[Bart Van Assche]

On 7/19/22 06:02, peter.wang at mediatek.com wrote:
> From: Peter Wang <peter.wang at mediatek.com>
> 
> After ufshcd_wl_shutdown set device poweroff and link off,
> ufshcd_shutdown not turn off regulators/clocks.
> Correct the flow to wait ufshcd_wl_shutdown done and turn off
> regulators/clocks by polling ufs device/link state 500ms.
> Also remove pm_runtime_get_sync because it is unnecessary.

Please explain in the patch description why the pm_runtime_get_sync() 
call is not necessary.

> diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
> index c7b337480e3e..1c11af48b584 100644
> --- a/drivers/ufs/core/ufshcd.c
> +++ b/drivers/ufs/core/ufshcd.c
> @@ -9461,10 +9461,14 @@ EXPORT_SYMBOL(ufshcd_runtime_resume);
>    */
>   int ufshcd_shutdown(struct ufs_hba *hba)
>   {
> -	if (ufshcd_is_ufs_dev_poweroff(hba) && ufshcd_is_link_off(hba))
> -		goto out;
> +	ktime_t timeout = ktime_add_ms(ktime_get(), 500);

Where does the 500 ms timeout come from?

Additionally, given the large timeout, please use jiffies instead of 
ktime_get().

> -	pm_runtime_get_sync(hba->dev);
> +	/* Wait ufshcd_wl_shutdown clear ufs state, timeout 500 ms */
> +	while (!ufshcd_is_ufs_dev_poweroff(hba) || !ufshcd_is_link_off(hba)) {
> +		if (ktime_after(ktime_get(), timeout))
> +			goto out;
> +		msleep(1);
> +	}

Please explain why this wait loop has been introduced.

Thanks,

Bart.