On 8/12/22 08:10, peter.wang@xxxxxxxxxxxx wrote:
> From: Peter Wang <peter.wang@xxxxxxxxxxxx>
>
> When SSU/enter hibern8 fail in wlun suspend flow, trigger error
> handler and return busy to break the suspend.
> If not, wlun runtime pm status become error and the consumer will
> stuck in runtime suspend status.
>
> Fixes: b294ff3e3449 ("scsi: ufs: core: Enable power management for wlun")
> Cc: stable@xxxxxxxxxxxxxxx
> Signed-off-by: Peter Wang <peter.wang@xxxxxxxxxxxx>
> ---
> drivers/ufs/core/ufshcd.c | 25 +++++++++++++++++++++++++
> 1 file changed, 25 insertions(+)
>
> diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
> index b1f59a5fe632..c91d58d1486a 100644
> --- a/drivers/ufs/core/ufshcd.c
> +++ b/drivers/ufs/core/ufshcd.c
> @@ -106,6 +106,13 @@
> 16, 4, buf, __len, false); \
> } while (0)
>
> +#define ufshcd_force_error_recovery(hba) do { \
> + spin_lock_irq(hba->host->host_lock); \
> + hba->force_reset = true; \
> + ufshcd_schedule_eh_work(hba); \
> + spin_unlock_irq(hba->host->host_lock); \
> +} while (0)
Thanks for separating it out, but there is no reason to make this
a macro, so it should be a function, because functions offer nicer
structure and less chance of surprises. It need not be an inline
function either because the compiler can determine that, and this
is not a hot path. i.e.
static void ufshcd_force_error_recovery(struct ufs_hba *hba)
{
spin_lock_irq(hba->host->host_lock);
hba->force_reset = true;
ufshcd_schedule_eh_work(hba);
spin_unlock_irq(hba->host->host_lock);
}
> +
> int ufshcd_dump_regs(struct ufs_hba *hba, size_t offset, size_t len,
> const char *prefix)
> {
> @@ -9049,6 +9056,15 @@ 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) {
> + /*
> + * If return err in suspend flow, IO will hang.
> + * Trigger error handler and break suspend for
> + * error recovery.
> + */
> + ufshcd_force_error_recovery(hba);
> + ret = -EBUSY;
> + }
> if (ret)
> goto enable_scaling;
> }
> @@ -9060,6 +9076,15 @@ 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.
> + */
> + ufshcd_force_error_recovery(hba);
> + ret = -EBUSY;
> + }
> if (ret)
> goto set_dev_active;
>
