* [PATCH] scsi: ufs: Fix a bug in ufshcd_system_resume()
@ 2020-11-10 8:55 Yang Yang
2020-11-13 5:05 ` Can Guo
0 siblings, 1 reply; 2+ messages in thread
From: Yang Yang @ 2020-11-10 8:55 UTC (permalink / raw)
To: Alim Akhtar, Avri Altman, James E.J. Bottomley,
Martin K. Petersen, Stanley Chu, Can Guo, Bean Huo,
Bart Van Assche, Asutosh Das, Sujit Reddy Thumma, Dolev Raviv,
Subhash Jadavani, linux-scsi, linux-kernel
Cc: onlyfever, yang.yang
During system resume, ufshcd_system_resume() won't resume UFS host if
runtime suspended. After that, scsi_bus_resume() try to set SCSI host's
RPM status to RPM_ACTIVE, this will fail because UFS host's RPM status
is still RPM_SUSPENDED. So fix it.
scsi host0: scsi_runtime_suspend()
ufshcd_runtime_suspend()
scsi host0: scsi_bus_suspend()
ufshcd_system_suspend()
----------------------------------
ufshcd_pltfrm_resume()
scsi host0: scsi_bus_resume()
scsi host0: scsi_bus_resume_common()
scsi host0: pm_runtime_set_active(dev)
scsi host0: runtime PM trying to activate child device host0 but parent
(8800000.ufshc) is not active
Fixes: 57d104c153d3 ("ufs: add UFS power management support")
Signed-off-by: Yang Yang <yang.yang@vivo.com>
---
drivers/scsi/ufs/ufshcd.c | 15 +++++++++------
1 file changed, 9 insertions(+), 6 deletions(-)
diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index b8f573a02713..9e666e1ad58c 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -8767,11 +8767,7 @@ int ufshcd_system_resume(struct ufs_hba *hba)
if (!hba)
return -EINVAL;
- if (!hba->is_powered || pm_runtime_suspended(hba->dev))
- /*
- * Let the runtime resume take care of resuming
- * if runtime suspended.
- */
+ if (!hba->is_powered)
goto out;
else
ret = ufshcd_resume(hba, UFS_SYSTEM_PM);
@@ -8779,8 +8775,15 @@ int ufshcd_system_resume(struct ufs_hba *hba)
trace_ufshcd_system_resume(dev_name(hba->dev), ret,
ktime_to_us(ktime_sub(ktime_get(), start)),
hba->curr_dev_pwr_mode, hba->uic_link_state);
- if (!ret)
+ if (!ret) {
hba->is_sys_suspended = false;
+
+ if (pm_runtime_suspended(hba->dev)) {
+ pm_runtime_disable(hba->dev);
+ pm_runtime_set_active(hba->dev);
+ pm_runtime_enable(hba->dev);
+ }
+ }
return ret;
}
EXPORT_SYMBOL(ufshcd_system_resume);
--
2.17.1
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH] scsi: ufs: Fix a bug in ufshcd_system_resume()
2020-11-10 8:55 [PATCH] scsi: ufs: Fix a bug in ufshcd_system_resume() Yang Yang
@ 2020-11-13 5:05 ` Can Guo
0 siblings, 0 replies; 2+ messages in thread
From: Can Guo @ 2020-11-13 5:05 UTC (permalink / raw)
To: Yang Yang
Cc: Alim Akhtar, Avri Altman, James E.J. Bottomley,
Martin K. Petersen, Stanley Chu, Bean Huo, Bart Van Assche,
Asutosh Das, Sujit Reddy Thumma, Dolev Raviv, Subhash Jadavani,
linux-scsi, linux-kernel, onlyfever
On 2020-11-10 16:55, Yang Yang wrote:
> During system resume, ufshcd_system_resume() won't resume UFS host if
> runtime suspended. After that, scsi_bus_resume() try to set SCSI host's
> RPM status to RPM_ACTIVE, this will fail because UFS host's RPM status
> is still RPM_SUSPENDED. So fix it.
>
> scsi host0: scsi_runtime_suspend()
> ufshcd_runtime_suspend()
> scsi host0: scsi_bus_suspend()
> ufshcd_system_suspend()
> ----------------------------------
> ufshcd_pltfrm_resume()
> scsi host0: scsi_bus_resume()
> scsi host0: scsi_bus_resume_common()
> scsi host0: pm_runtime_set_active(dev)
>
> scsi host0: runtime PM trying to activate child device host0 but
> parent
> (8800000.ufshc) is not active
>
> Fixes: 57d104c153d3 ("ufs: add UFS power management support")
> Signed-off-by: Yang Yang <yang.yang@vivo.com>
> ---
> drivers/scsi/ufs/ufshcd.c | 15 +++++++++------
> 1 file changed, 9 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
> index b8f573a02713..9e666e1ad58c 100644
> --- a/drivers/scsi/ufs/ufshcd.c
> +++ b/drivers/scsi/ufs/ufshcd.c
> @@ -8767,11 +8767,7 @@ int ufshcd_system_resume(struct ufs_hba *hba)
> if (!hba)
> return -EINVAL;
>
> - if (!hba->is_powered || pm_runtime_suspended(hba->dev))
> - /*
> - * Let the runtime resume take care of resuming
> - * if runtime suspended.
> - */
> + if (!hba->is_powered)
> goto out;
> else
> ret = ufshcd_resume(hba, UFS_SYSTEM_PM);
> @@ -8779,8 +8775,15 @@ int ufshcd_system_resume(struct ufs_hba *hba)
> trace_ufshcd_system_resume(dev_name(hba->dev), ret,
> ktime_to_us(ktime_sub(ktime_get(), start)),
> hba->curr_dev_pwr_mode, hba->uic_link_state);
> - if (!ret)
> + if (!ret) {
> hba->is_sys_suspended = false;
> +
> + if (pm_runtime_suspended(hba->dev)) {
> + pm_runtime_disable(hba->dev);
> + pm_runtime_set_active(hba->dev);
> + pm_runtime_enable(hba->dev);
> + }
> + }
> return ret;
> }
> EXPORT_SYMBOL(ufshcd_system_resume);
It is designed like this - if hba is runtime suspend, do not resume it
during system resume, which is the so called deferred resume feature.
This is to leave the runtime PM management to block layer PM, which
is more power efficiency.
> scsi host0: runtime PM trying to activate child device host0 but parent
> (8800000.ufshc) is not active
The log is not harmful or fatal, maybe just annoying. Do you see real
problem with it? If yes, please share it here.
Thanks,
Can Guo.
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2020-11-13 5:05 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-11-10 8:55 [PATCH] scsi: ufs: Fix a bug in ufshcd_system_resume() Yang Yang
2020-11-13 5:05 ` Can Guo
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).