Re: [PATCH v2] ufs: core: wlun suspend SSU/enter hibern8 fail recovery

[Bart Van Assche <bvanassche@acm.org>]

On 12/1/22 01:43, peter.wang@mediatek.com wrote:
> @@ -9049,6 +9050,20 @@ static int __ufshcd_wl_suspend(struct ufs_hba *hba, enum ufs_pm_op pm_op)
>   
>   		if (!hba->dev_info.b_rpm_dev_flush_capable) {
>   			ret = ufshcd_set_dev_pwr_mode(hba, req_dev_pwr_mode);
> +			if ((ret) && (pm_op != UFS_SHUTDOWN_PM)) {

Please remove the superfluous parentheses from the above statement.

> +				/*
> +				 * If return err in suspend flow, IO will hang.
> +				 * Trigger error handler and break suspend for
> +				 * error recovery.
> +				 */
> +				spin_lock_irqsave(hba->host->host_lock, flags);

__ufshcd_wl_suspend() is allowed to sleep. Please change 
spin_lock_irqsave() into spin_lock_irq().

> +				hba->force_reset = true;
> +				ufshcd_schedule_eh_work(hba);
> +				spin_unlock_irqrestore(hba->host->host_lock,
> +					flags);
> +
> +				ret = -EBUSY;

Why is the value of 'ret' changed into -EBUSY? Can the above code be 
left out?

> @@ -9060,6 +9075,20 @@ static int __ufshcd_wl_suspend(struct ufs_hba *hba, enum ufs_pm_op pm_op)
>   	 */
>   	check_for_bkops = !ufshcd_is_ufs_dev_deepsleep(hba);
>   	ret = ufshcd_link_state_transition(hba, req_link_state, check_for_bkops);
> +	if ((ret) && (pm_op != UFS_SHUTDOWN_PM)) {
> +		/*
> +		 * If return err in suspend flow, IO will hang.
> +		 * Trigger error handler and break suspend for
> +		 * error recovery.
> +		 */
> +		spin_lock_irqsave(hba->host->host_lock, flags);
> +		hba->force_reset = true;
> +		ufshcd_schedule_eh_work(hba);
> +		spin_unlock_irqrestore(hba->host->host_lock,
> +			flags);
> +
> +		ret = -EBUSY;
> +	}

Same comments as above for this code block.

Thanks,

Bart.