| Message ID | 20221202073532.7884-1-peter.wang@mediatek.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [v3] ufs: core: wlun suspend SSU/enter hibern8 fail recovery | expand |
On 12/1/22 23:35, peter.wang@mediatek.com wrote: > From: Peter Wang <peter.wang@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. > > Signed-off-by: Peter Wang <peter.wang@mediatek.com> > --- > drivers/ufs/core/ufshcd.c | 26 ++++++++++++++++++++++++++ > 1 file changed, 26 insertions(+) > > diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c > index b1f59a5fe632..a0dbf2c410b1 100644 > --- a/drivers/ufs/core/ufshcd.c > +++ b/drivers/ufs/core/ufshcd.c > @@ -9049,6 +9049,19 @@ 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. > + */ > + spin_lock_irq(hba->host->host_lock); > + hba->force_reset = true; > + ufshcd_schedule_eh_work(hba); > + spin_unlock_irq(hba->host->host_lock); > + > + ret = -EBUSY; > + } > if (ret) > goto enable_scaling; > } > @@ -9060,6 +9073,19 @@ 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_irq(hba->host->host_lock); > + hba->force_reset = true; > + ufshcd_schedule_eh_work(hba); > + spin_unlock_irq(hba->host->host_lock); > + > + ret = -EBUSY; > + } > if (ret) > goto set_dev_active; > Please follow the coding style that is used elsewhere in the kernel and remove the superfluous parentheses from this patch. "if (ret && (pm_op != UFS_SHUTDOWN_PM)) {" can be changed into "if (ret && pm_op != UFS_SHUTDOWN_PM) {" and "if ((ret) && (pm_op != UFS_SHUTDOWN_PM)) {" can be changed into "if ((ret) && (pm_op != UFS_SHUTDOWN_PM)) {". Otherwise this patch looks good to me. Thanks, Bart.
On Mon, 2022-12-05 at 10:41 -0800, Bart Van Assche wrote: > On 12/1/22 23:35, peter.wang@mediatek.com wrote: > > From: Peter Wang <peter.wang@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. > > > > Signed-off-by: Peter Wang <peter.wang@mediatek.com> > > --- > > drivers/ufs/core/ufshcd.c | 26 ++++++++++++++++++++++++++ > > 1 file changed, 26 insertions(+) > > > > diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c > > index b1f59a5fe632..a0dbf2c410b1 100644 > > --- a/drivers/ufs/core/ufshcd.c > > +++ b/drivers/ufs/core/ufshcd.c > > @@ -9049,6 +9049,19 @@ 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. > > + */ > > + spin_lock_irq(hba->host->host_lock); > > + hba->force_reset = true; > > + ufshcd_schedule_eh_work(hba); > > + spin_unlock_irq(hba->host->host_lock); > > + > > + ret = -EBUSY; > > + } > > if (ret) > > goto enable_scaling; > > } > > @@ -9060,6 +9073,19 @@ 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_irq(hba->host->host_lock); > > + hba->force_reset = true; > > + ufshcd_schedule_eh_work(hba); > > + spin_unlock_irq(hba->host->host_lock); > > + > > + ret = -EBUSY; > > + } > > if (ret) > > goto set_dev_active; > > > > Please follow the coding style that is used elsewhere in the kernel > and > remove the superfluous parentheses from this patch. "if (ret && > (pm_op != > UFS_SHUTDOWN_PM)) {" can be changed into "if (ret && pm_op != > UFS_SHUTDOWN_PM) {" and "if ((ret) && (pm_op != UFS_SHUTDOWN_PM)) {" > can be > changed into "if ((ret) && (pm_op != UFS_SHUTDOWN_PM)) {". > > Otherwise this patch looks good to me. > > Thanks, Hi Bart, Got it, will remove parentheses next version. Thanks for review. BR Peter > > Bart.
diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c index b1f59a5fe632..a0dbf2c410b1 100644 --- a/drivers/ufs/core/ufshcd.c +++ b/drivers/ufs/core/ufshcd.c @@ -9049,6 +9049,19 @@ 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. + */ + spin_lock_irq(hba->host->host_lock); + hba->force_reset = true; + ufshcd_schedule_eh_work(hba); + spin_unlock_irq(hba->host->host_lock); + + ret = -EBUSY; + } if (ret) goto enable_scaling; } @@ -9060,6 +9073,19 @@ 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_irq(hba->host->host_lock); + hba->force_reset = true; + ufshcd_schedule_eh_work(hba); + spin_unlock_irq(hba->host->host_lock); + + ret = -EBUSY; + } if (ret) goto set_dev_active;