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

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

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.