On Thu, 2022-12-01 at 10:44 -0800, Bart Van Assche wrote:
> On 12/1/22 01:43, peter.wang@xxxxxxxxxxxx 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.
>
Hi Bart,
Okay, will change next version.
> > + /*
> > + * 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?
>
Because in runtime pm framework, return EBUSY is okay for next time
runtime suspend retry.
Return ETIMEOUT/EIO or other then EBUSY will keep error code in
veriable "runtime_error". (supplier first then consumer resume block by
supplier also get runtime_error too). Finally, lead to IO hang.
> > @@ -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.
Okay, will change next version.
Thanks
BR
Peter
>
