Re: [PATCH v9 2/3] scsi: ufs: core: Hold host_sem in ufshcd_wl_poweroff()

[Bart Van Assche]

On 11/9/26 6:03 PM, HOYOUNG SEO wrote:
> -	__ufshcd_wl_suspend(hba, UFS_SHUTDOWN_PM);
> -	return 0;
> +	/* Released by ufshcd_wl_resume() if hibernation is aborted. */
> +	down(&hba->host_sem);
> +	ret = __ufshcd_wl_suspend(hba, UFS_SHUTDOWN_PM);
> +	if (ret)
> +		up(&hba->host_sem);
> +
> +	return ret;

Now that the __ufshcd_wl_suspend() return value is propagated, I think
that the issues reported by Sashiko must be addressed:

- If the WLUN is runtime suspended when .poweroff is called (this
  happens if ufshcd_rpm_ok_for_spm() returned true in
  __ufshcd_suspend_prepare(), e.g. with the default rpm_lvl ==
  spm_lvl), __ufshcd_wl_suspend() triggers error recovery and returns
  -EBUSY. power_down() in kernel/power/hibernate.c interprets -EBUSY
  as a wakeup event and rolls back hibernation. ufshcd_wl_suspend()
  checks pm_runtime_suspended(dev) and ufshcd_wl_shutdown() calls
  ufshcd_rpm_get_sync(). ufshcd_wl_poweroff() does neither.

- For UFS_SHUTDOWN_PM, failures of ufshcd_set_dev_pwr_mode() and
  ufshcd_link_state_transition() are not converted into -EBUSY, so a
  positive SCSI or UIC status can be returned from a dev_pm_ops
  callback.

- hba->is_sys_suspended is not set if .poweroff succeeds.

How about moving the body of ufshcd_wl_suspend() into a helper
function that accepts a pm_op argument and calling that helper from
both ufshcd_wl_suspend() and ufshcd_wl_poweroff()? That would make the
poweroff path identical to the system suspend path except for the
target power mode and link state.

Thanks,

Bart.