From: "Asutosh Das (asd)" <asutoshd@codeaurora.org>
To: Adrian Hunter <adrian.hunter@intel.com>,
"Martin K . Petersen" <martin.petersen@oracle.com>
Cc: "James E . J . Bottomley" <jejb@linux.ibm.com>,
Bean Huo <huobean@gmail.com>, Avri Altman <avri.altman@wdc.com>,
Alim Akhtar <alim.akhtar@samsung.com>,
Can Guo <cang@codeaurora.org>,
Bart Van Assche <bvanassche@acm.org>,
Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>,
Wei Li <liwei213@huawei.com>,
linux-scsi@vger.kernel.org
Subject: Re: [PATCH V5 3/3] scsi: ufs: Let devices remain runtime suspended during system suspend
Date: Wed, 22 Sep 2021 09:15:49 -0700 [thread overview]
Message-ID: <917fe2b8-78ff-de35-c3cb-3251b23ac003@codeaurora.org> (raw)
In-Reply-To: <20210922093842.18025-4-adrian.hunter@intel.com>
On 9/22/2021 2:38 AM, Adrian Hunter wrote:
> If the UFS Device WLUN is runtime suspended and is in the same power
> mode, link state and b_rpm_dev_flush_capable (BKOP or WB buffer flush etc)
> state, then it can remain runtime suspended instead of being runtime
> resumed and then system suspended.
>
> The following patches have cleared the way for that to happen:
> scsi: ufs: Fix runtime PM dependencies getting broken
> scsi: ufs: Fix error handler clear ua deadlock
>
> So amend the logic accordingly.
>
> Note, the ufs-hisi driver uses different RPM and SPM, but it is made
> explicit by a new parameter to suspend prepare.
>
> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
> ---
LGTM.
Reviewed-by: Asutosh Das <asutoshd@codeaurora.org>
> drivers/scsi/ufs/ufs-hisi.c | 8 +++++-
> drivers/scsi/ufs/ufshcd.c | 53 ++++++++++++++++++++++++++++---------
> drivers/scsi/ufs/ufshcd.h | 12 ++++++++-
> 3 files changed, 58 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/scsi/ufs/ufs-hisi.c b/drivers/scsi/ufs/ufs-hisi.c
> index 6b706de8354b..4a08fb35642c 100644
> --- a/drivers/scsi/ufs/ufs-hisi.c
> +++ b/drivers/scsi/ufs/ufs-hisi.c
> @@ -396,6 +396,12 @@ static int ufs_hisi_pwr_change_notify(struct ufs_hba *hba,
> return ret;
> }
>
> +static int ufs_hisi_suspend_prepare(struct device *dev)
> +{
> + /* RPM and SPM are different. Refer ufs_hisi_suspend() */
> + return __ufshcd_suspend_prepare(dev, false);
> +}
> +
> static int ufs_hisi_suspend(struct ufs_hba *hba, enum ufs_pm_op pm_op)
> {
> struct ufs_hisi_host *host = ufshcd_get_variant(hba);
> @@ -574,7 +580,7 @@ static int ufs_hisi_remove(struct platform_device *pdev)
> static const struct dev_pm_ops ufs_hisi_pm_ops = {
> SET_SYSTEM_SLEEP_PM_OPS(ufshcd_system_suspend, ufshcd_system_resume)
> SET_RUNTIME_PM_OPS(ufshcd_runtime_suspend, ufshcd_runtime_resume, NULL)
> - .prepare = ufshcd_suspend_prepare,
> + .prepare = ufs_hisi_suspend_prepare,
> .complete = ufshcd_resume_complete,
> };
>
> diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
> index 39fc6070d2f7..64e81ece1aae 100644
> --- a/drivers/scsi/ufs/ufshcd.c
> +++ b/drivers/scsi/ufs/ufshcd.c
> @@ -9797,14 +9797,30 @@ void ufshcd_resume_complete(struct device *dev)
> ufshcd_rpm_put(hba);
> hba->complete_put = false;
> }
> - if (hba->rpmb_complete_put) {
> - ufshcd_rpmb_rpm_put(hba);
> - hba->rpmb_complete_put = false;
> - }
> }
> EXPORT_SYMBOL_GPL(ufshcd_resume_complete);
>
> -int ufshcd_suspend_prepare(struct device *dev)
> +static bool ufshcd_rpm_ok_for_spm(struct ufs_hba *hba)
> +{
> + struct device *dev = &hba->sdev_ufs_device->sdev_gendev;
> + enum ufs_dev_pwr_mode dev_pwr_mode;
> + enum uic_link_state link_state;
> + unsigned long flags;
> + bool res;
> +
> + spin_lock_irqsave(&dev->power.lock, flags);
> + dev_pwr_mode = ufs_get_pm_lvl_to_dev_pwr_mode(hba->spm_lvl);
> + link_state = ufs_get_pm_lvl_to_link_pwr_state(hba->spm_lvl);
> + res = pm_runtime_suspended(dev) &&
> + hba->curr_dev_pwr_mode == dev_pwr_mode &&
> + hba->uic_link_state == link_state &&
> + !hba->dev_info.b_rpm_dev_flush_capable;
> + spin_unlock_irqrestore(&dev->power.lock, flags);
> +
> + return res;
> +}
> +
> +int __ufshcd_suspend_prepare(struct device *dev, bool rpm_ok_for_spm)
> {
> struct ufs_hba *hba = dev_get_drvdata(dev);
> int ret;
> @@ -9816,19 +9832,30 @@ int ufshcd_suspend_prepare(struct device *dev)
> * Refer ufshcd_resume_complete()
> */
> if (hba->sdev_ufs_device) {
> - ret = ufshcd_rpm_get_sync(hba);
> - if (ret < 0 && ret != -EACCES) {
> - ufshcd_rpm_put(hba);
> - return ret;
> + /* Prevent runtime suspend */
> + ufshcd_rpm_get_noresume(hba);
> + /*
> + * Check if already runtime suspended in same state as system
> + * suspend would be.
> + */
> + if (!rpm_ok_for_spm || !ufshcd_rpm_ok_for_spm(hba)) {
> + /* RPM state is not ok for SPM, so runtime resume */
> + ret = ufshcd_rpm_resume(hba);
> + if (ret < 0 && ret != -EACCES) {
> + ufshcd_rpm_put(hba);
> + return ret;
> + }
> }
> hba->complete_put = true;
> }
> - if (hba->sdev_rpmb) {
> - ufshcd_rpmb_rpm_get_sync(hba);
> - hba->rpmb_complete_put = true;
> - }
> return 0;
> }
> +EXPORT_SYMBOL_GPL(__ufshcd_suspend_prepare);
> +
> +int ufshcd_suspend_prepare(struct device *dev)
> +{
> + return __ufshcd_suspend_prepare(dev, true);
> +}
> EXPORT_SYMBOL_GPL(ufshcd_suspend_prepare);
>
> #ifdef CONFIG_PM_SLEEP
> diff --git a/drivers/scsi/ufs/ufshcd.h b/drivers/scsi/ufs/ufshcd.h
> index 06e743a5937a..c64a92ef15a4 100644
> --- a/drivers/scsi/ufs/ufshcd.h
> +++ b/drivers/scsi/ufs/ufshcd.h
> @@ -929,7 +929,6 @@ struct ufs_hba {
> #endif
> u32 luns_avail;
> bool complete_put;
> - bool rpmb_complete_put;
> };
>
> /* Returns true if clocks can be gated. Otherwise false */
> @@ -1189,6 +1188,7 @@ int ufshcd_exec_raw_upiu_cmd(struct ufs_hba *hba,
>
> int ufshcd_wb_toggle(struct ufs_hba *hba, bool enable);
> int ufshcd_suspend_prepare(struct device *dev);
> +int __ufshcd_suspend_prepare(struct device *dev, bool rpm_ok_for_spm);
> void ufshcd_resume_complete(struct device *dev);
>
> /* Wrapper functions for safely calling variant operations */
> @@ -1397,6 +1397,16 @@ static inline int ufshcd_rpm_put_sync(struct ufs_hba *hba)
> return pm_runtime_put_sync(&hba->sdev_ufs_device->sdev_gendev);
> }
>
> +static inline void ufshcd_rpm_get_noresume(struct ufs_hba *hba)
> +{
> + pm_runtime_get_noresume(&hba->sdev_ufs_device->sdev_gendev);
> +}
> +
> +static inline int ufshcd_rpm_resume(struct ufs_hba *hba)
> +{
> + return pm_runtime_resume(&hba->sdev_ufs_device->sdev_gendev);
> +}
> +
> static inline int ufshcd_rpm_put(struct ufs_hba *hba)
> {
> return pm_runtime_put(&hba->sdev_ufs_device->sdev_gendev);
>
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
Linux Foundation Collaborative Project
prev parent reply other threads:[~2021-09-22 16:16 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-09-22 9:38 [PATCH V5 0/3] scsi: ufs: Let devices remain runtime suspended during system suspend Adrian Hunter
2021-09-22 9:38 ` [PATCH V5 1/3] scsi: ufs: Fix error handler clear ua deadlock Adrian Hunter
[not found] ` <CGME20210928071133epcas2p28e00e20bbebbb5c1920933204f91743b@epcas2p2.samsung.com>
2021-09-28 6:55 ` Kiwoong Kim
2021-09-30 12:44 ` Adrian Hunter
[not found] ` <CGME20210929051745epcas2p1024eb171d57dca361d2d3d522683770d@epcas2p1.samsung.com>
2021-09-29 5:01 ` Kiwoong Kim
2021-09-30 12:43 ` Adrian Hunter
2021-09-22 9:38 ` [PATCH V5 2/3] scsi: ufs: Fix runtime PM dependencies getting broken Adrian Hunter
2021-09-22 9:38 ` [PATCH V5 3/3] scsi: ufs: Let devices remain runtime suspended during system suspend Adrian Hunter
2021-09-22 16:15 ` Asutosh Das (asd) [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=917fe2b8-78ff-de35-c3cb-3251b23ac003@codeaurora.org \
--to=asutoshd@codeaurora.org \
--cc=adrian.hunter@intel.com \
--cc=alim.akhtar@samsung.com \
--cc=avri.altman@wdc.com \
--cc=bvanassche@acm.org \
--cc=cang@codeaurora.org \
--cc=huobean@gmail.com \
--cc=jejb@linux.ibm.com \
--cc=linux-scsi@vger.kernel.org \
--cc=liwei213@huawei.com \
--cc=manivannan.sadhasivam@linaro.org \
--cc=martin.petersen@oracle.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).