Re: [RFC PATCH v2] scsi: ufs: set STATE_ERROR when ufshcd_probe_hba() failed

[Can Guo <cang@codeaurora.org>]

Hi Sang Hyun,

On 2020-07-17 15:31, Lee Sang Hyun wrote:
> set STATE_ERR like below to prevent a lockup(IO stuck)
> when ufshcd_probe_hba() returns error.
> 
> Change-Id: I6c85ff290503cc9414d7f5fdd934295497b854ff
> Signed-off-by: Lee Sang Hyun <sh425.lee@samsung.com>
> ---
>  drivers/scsi/ufs/ufshcd.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
> index ad4fc82..37e4105 100644
> --- a/drivers/scsi/ufs/ufshcd.c
> +++ b/drivers/scsi/ufs/ufshcd.c
> @@ -7368,6 +7368,7 @@ static int ufshcd_probe_hba(struct ufs_hba *hba,
> bool async)
>  {
>  	int ret;
>  	ktime_t start = ktime_get();
> +	unsigned long flags;
> 
>  	ret = ufshcd_link_startup(hba);
>  	if (ret)
> @@ -7439,6 +7440,11 @@ static int ufshcd_probe_hba(struct ufs_hba
> *hba, bool async)
>  	ufshcd_auto_hibern8_enable(hba);
> 
>  out:
> +	if (ret) {
> +		spin_lock_irqsave(hba->host->host_lock, flags);
> +		hba->ufshcd_state = UFSHCD_STATE_ERROR;
> +		spin_unlock_irqrestore(hba->host->host_lock, flags);
> +	}
> 
>  	trace_ufshcd_init(dev_name(hba->dev), ret,
>  		ktime_to_us(ktime_sub(ktime_get(), start)),

This change is included in my change
"scsi: ufs: Fix up and simplify error recovery mechanism".

Besides, this change seems not complete because

#1 You are only protecting your changes with spin lock in
ufshcd_probe_hba, what about the other line
"hba->ufshcd_state = UFSHCD_STATE_OPERATIONAL;"?

#2 As you are giving "hba->ufshcd_state = UFSHCD_STATE_ERROR;"
in ufshcd_probe_hba, why keep the same lines in
ufshcd_error_handler and ufshcd_eh_host_reset_handler?

Thanks,

Can Guo.