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

[Peter Wang (王信友)]

On Thu, 2022-12-08 at 08:41 +0200, Adrian Hunter wrote:
> On 8/12/22 08:10, peter.wang at mediatek.com wrote:
> > From: Peter Wang <peter.wang at mediatek.com>
> > 
> > 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 at vger.kernel.org
> > Signed-off-by: Peter Wang <peter.wang at mediatek.com>
> > ---
> >  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);
> }
> 

Hi Adrian,

Okay, will change to static function next version, 
Thank you very much. :)

Peter


> > +
> >  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;
> >  
> 
>