| Message ID | 20221206031109.10609-1-peter.wang@mediatek.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v4] ufs: core: wlun suspend SSU/enter hibern8 fail recovery | expand |
On Tue, Dec 6, 2022 at 11:18 AM <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> Reviewed-by: Stanley Chu <stanley.chu@mediatek.com>
On 12/5/22 19:11, 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. Reviewed-by: Bart Van Assche <bvanassche@acm.org>
On 6/12/22 05:11, 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> Should this have a Fixes or stable tag? Another minor comment below, otherwise: Reviewed-by: Adrian Hunter <adrian.hunter@intel.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..98f105f32877 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); Same 4 lines of code could be separated into a helper function. > + > + ret = -EBUSY; > + } > if (ret) > goto set_dev_active; >
On Wed, 2022-12-07 at 19:17 +0200, Adrian Hunter wrote: > On 6/12/22 05:11, 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> > > Should this have a Fixes or stable tag? > Hi Adrian, Yes, will add Fixes tag. Thanks for reminder. > Another minor comment below, otherwise: > > Reviewed-by: Adrian Hunter <adrian.hunter@intel.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..98f105f32877 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); > > Same 4 lines of code could be separated into a helper function. OK, will change next version. Thanks for review. > > > + > > + ret = -EBUSY; > > + } > > if (ret) > > goto set_dev_active; > > > >
On Wed, 2022-12-07 at 19:17 +0200, Adrian Hunter wrote: > On 6/12/22 05:11, 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> > > Should this have a Fixes or stable tag? > > Another minor comment below, otherwise: > > Reviewed-by: Adrian Hunter <adrian.hunter@intel.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..98f105f32877 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); > > Same 4 lines of code could be separated into a helper function. > > > + > > + ret = -EBUSY; > > + } > > if (ret) > > goto set_dev_active; > > > >
diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c index b1f59a5fe632..98f105f32877 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;