From: "Asutosh Das (asd)" <asutoshd@codeaurora.org> To: Adrian Hunter <adrian.hunter@intel.com>, cang@codeaurora.org, martin.petersen@oracle.com, linux-scsi@vger.kernel.org Cc: linux-arm-msm@vger.kernel.org, Alim Akhtar <alim.akhtar@samsung.com>, Avri Altman <avri.altman@wdc.com>, "James E.J. Bottomley" <jejb@linux.ibm.com>, Krzysztof Kozlowski <krzk@kernel.org>, Stanley Chu <stanley.chu@mediatek.com>, Andy Gross <agross@kernel.org>, Bjorn Andersson <bjorn.andersson@linaro.org>, Steven Rostedt <rostedt@goodmis.org>, Ingo Molnar <mingo@redhat.com>, Matthias Brugger <matthias.bgg@gmail.com>, Lee Jones <lee.jones@linaro.org>, Bean Huo <beanhuo@micron.com>, Kiwoong Kim <kwmad.kim@samsung.com>, Colin Ian King <colin.king@canonical.com>, Wei Yongjun <weiyongjun1@huawei.com>, Yue Hu <huyue2@yulong.com>, Bart van Assche <bvanassche@acm.org>, "Gustavo A. R. Silva" <gustavoars@kernel.org>, Dinghao Liu <dinghao.liu@zju.edu.cn>, Jaegeuk Kim <jaegeuk@kernel.org>, Satya Tangirala <satyat@google.com>, open list <linux-kernel@vger.kernel.org>, "moderated list:ARM/SAMSUNG S3C, S5P AND EXYNOS ARM ARCHITECTURES" <linux-arm-kernel@lists.infradead.org>, "open list:ARM/SAMSUNG S3C, S5P AND EXYNOS ARM ARCHITECTURES" <linux-samsung-soc@vger.kernel.org>, "moderated list:UNIVERSAL FLASH STORAGE HOST CONTROLLER DRIVER..." <linux-mediatek@lists.infradead.org> Subject: Re: [PATCH v20 1/2] scsi: ufs: Enable power management for wlun Date: Fri, 23 Apr 2021 16:44:26 -0700 [thread overview] Message-ID: <a71d6ac3-0dc6-4fa8-3643-6d3473d08797@codeaurora.org> (raw) In-Reply-To: <973e0bbb-ac2d-7196-2e25-37aee2b77b46@intel.com> On 4/23/2021 1:01 AM, Adrian Hunter wrote: >> >> > Hi Adrian, Thanks for the help. I made the changes and tried to reproduce it. My setup becomes non-responsive and resets. I don't think it's related to this issue though. I hadn't set rpm_lvl and spm_lvl to 0 while testing the other day. Setting those to 0, it proceeds further and now it becomes unresponsive. So I still don't clearly see the issue that you're seeing. However, I'd make the suggested changes and push it in next version. > I think we also need to runtime resume RPMB WLUN before system suspend. > e.g. > > +static int ufshcd_rpmb_rpm_get_sync(struct ufs_hba *hba) > +{ > + return pm_runtime_get_sync(&hba->sdev_rpmb->sdev_gendev); > +} > + > +static int ufshcd_rpmb_rpm_put(struct ufs_hba *hba) > +{ > + return pm_runtime_put(&hba->sdev_rpmb->sdev_gendev); > +} > + > void ufshcd_resume_complete(struct device *dev) > { > struct ufs_hba *hba = dev_get_drvdata(dev); > > + if (hba->rpmb_complete_put) { > + hba->rpmb_complete_put = false; > + ufshcd_rpmb_rpm_put(hba); > + } > if (hba->complete_put) { > hba->complete_put = false; > ufshcd_rpm_put(hba); > @@ -9611,6 +9625,11 @@ int ufshcd_suspend_prepare(struct device *dev) > return ret; > } > hba->complete_put = true; > + > + if (hba->sdev_rpmb) { > + ufshcd_rpmb_rpm_get_sync(hba); > + hba->rpmb_complete_put = true; > + } > return 0; > } > > That also avoids another issue: if RPMB WLUN is runtime suspended at system resume, we have to skip clearing UAC, but SCSI PM will force the runtime status to RPM_ACTIVE after system resume, so the UAC never gets cleared in that case. > > Furthermore, it seems better not to report errors from RPMB resume and instead let the error handler sort it out. > So, with the above change, we can simplify a bit: > > -static int ufshcd_rpmb_runtime_resume(struct device *dev) > -{ > - struct ufs_hba *hba = wlun_dev_to_hba(dev); > - > - if (hba->sdev_rpmb) > - return ufshcd_clear_rpmb_uac(hba); > - return 0; > -} > - > static int ufshcd_rpmb_resume(struct device *dev) > { > struct ufs_hba *hba = wlun_dev_to_hba(dev); > > - if (hba->sdev_rpmb && !pm_runtime_suspended(dev)) > - return ufshcd_clear_rpmb_uac(hba); > + if (hba->sdev_rpmb) > + ufshcd_clear_rpmb_uac(hba); > return 0; > } > > static const struct dev_pm_ops ufs_rpmb_pm_ops = { > - SET_RUNTIME_PM_OPS(NULL, ufshcd_rpmb_runtime_resume, NULL) > + SET_RUNTIME_PM_OPS(NULL, ufshcd_rpmb_resume, NULL) > SET_SYSTEM_SLEEP_PM_OPS(NULL, ufshcd_rpmb_resume) > }; > > -- The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, Linux Foundation Collaborative Project
WARNING: multiple messages have this Message-ID (diff)
From: "Asutosh Das (asd)" <asutoshd@codeaurora.org> To: Adrian Hunter <adrian.hunter@intel.com>, cang@codeaurora.org, martin.petersen@oracle.com, linux-scsi@vger.kernel.org Cc: linux-arm-msm@vger.kernel.org, Alim Akhtar <alim.akhtar@samsung.com>, Avri Altman <avri.altman@wdc.com>, "James E.J. Bottomley" <jejb@linux.ibm.com>, Krzysztof Kozlowski <krzk@kernel.org>, Stanley Chu <stanley.chu@mediatek.com>, Andy Gross <agross@kernel.org>, Bjorn Andersson <bjorn.andersson@linaro.org>, Steven Rostedt <rostedt@goodmis.org>, Ingo Molnar <mingo@redhat.com>, Matthias Brugger <matthias.bgg@gmail.com>, Lee Jones <lee.jones@linaro.org>, Bean Huo <beanhuo@micron.com>, Kiwoong Kim <kwmad.kim@samsung.com>, Colin Ian King <colin.king@canonical.com>, Wei Yongjun <weiyongjun1@huawei.com>, Yue Hu <huyue2@yulong.com>, Bart van Assche <bvanassche@acm.org>, "Gustavo A. R. Silva" <gustavoars@kernel.org>, Dinghao Liu <dinghao.liu@zju.edu.cn>, Jaegeuk Kim <jaegeuk@kernel.org>, Satya Tangirala <satyat@google.com>, open list <linux-kernel@vger.kernel.org>, "moderated list:ARM/SAMSUNG S3C, S5P AND EXYNOS ARM ARCHITECTURES" <linux-arm-kernel@lists.infradead.org>, "open list:ARM/SAMSUNG S3C, S5P AND EXYNOS ARM ARCHITECTURES" <linux-samsung-soc@vger.kernel.org>, "moderated list:UNIVERSAL FLASH STORAGE HOST CONTROLLER DRIVER..." <linux-mediatek@lists.infradead.org> Subject: Re: [PATCH v20 1/2] scsi: ufs: Enable power management for wlun Date: Fri, 23 Apr 2021 16:44:26 -0700 [thread overview] Message-ID: <a71d6ac3-0dc6-4fa8-3643-6d3473d08797@codeaurora.org> (raw) In-Reply-To: <973e0bbb-ac2d-7196-2e25-37aee2b77b46@intel.com> On 4/23/2021 1:01 AM, Adrian Hunter wrote: >> >> > Hi Adrian, Thanks for the help. I made the changes and tried to reproduce it. My setup becomes non-responsive and resets. I don't think it's related to this issue though. I hadn't set rpm_lvl and spm_lvl to 0 while testing the other day. Setting those to 0, it proceeds further and now it becomes unresponsive. So I still don't clearly see the issue that you're seeing. However, I'd make the suggested changes and push it in next version. > I think we also need to runtime resume RPMB WLUN before system suspend. > e.g. > > +static int ufshcd_rpmb_rpm_get_sync(struct ufs_hba *hba) > +{ > + return pm_runtime_get_sync(&hba->sdev_rpmb->sdev_gendev); > +} > + > +static int ufshcd_rpmb_rpm_put(struct ufs_hba *hba) > +{ > + return pm_runtime_put(&hba->sdev_rpmb->sdev_gendev); > +} > + > void ufshcd_resume_complete(struct device *dev) > { > struct ufs_hba *hba = dev_get_drvdata(dev); > > + if (hba->rpmb_complete_put) { > + hba->rpmb_complete_put = false; > + ufshcd_rpmb_rpm_put(hba); > + } > if (hba->complete_put) { > hba->complete_put = false; > ufshcd_rpm_put(hba); > @@ -9611,6 +9625,11 @@ int ufshcd_suspend_prepare(struct device *dev) > return ret; > } > hba->complete_put = true; > + > + if (hba->sdev_rpmb) { > + ufshcd_rpmb_rpm_get_sync(hba); > + hba->rpmb_complete_put = true; > + } > return 0; > } > > That also avoids another issue: if RPMB WLUN is runtime suspended at system resume, we have to skip clearing UAC, but SCSI PM will force the runtime status to RPM_ACTIVE after system resume, so the UAC never gets cleared in that case. > > Furthermore, it seems better not to report errors from RPMB resume and instead let the error handler sort it out. > So, with the above change, we can simplify a bit: > > -static int ufshcd_rpmb_runtime_resume(struct device *dev) > -{ > - struct ufs_hba *hba = wlun_dev_to_hba(dev); > - > - if (hba->sdev_rpmb) > - return ufshcd_clear_rpmb_uac(hba); > - return 0; > -} > - > static int ufshcd_rpmb_resume(struct device *dev) > { > struct ufs_hba *hba = wlun_dev_to_hba(dev); > > - if (hba->sdev_rpmb && !pm_runtime_suspended(dev)) > - return ufshcd_clear_rpmb_uac(hba); > + if (hba->sdev_rpmb) > + ufshcd_clear_rpmb_uac(hba); > return 0; > } > > static const struct dev_pm_ops ufs_rpmb_pm_ops = { > - SET_RUNTIME_PM_OPS(NULL, ufshcd_rpmb_runtime_resume, NULL) > + SET_RUNTIME_PM_OPS(NULL, ufshcd_rpmb_resume, NULL) > SET_SYSTEM_SLEEP_PM_OPS(NULL, ufshcd_rpmb_resume) > }; > > -- The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, Linux Foundation Collaborative Project _______________________________________________ Linux-mediatek mailing list Linux-mediatek@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-mediatek
next prev parent reply other threads:[~2021-04-23 23:44 UTC|newest] Thread overview: 31+ messages / expand[flat|nested] mbox.gz Atom feed top 2021-04-16 19:49 [PATCH v20 0/2] Enable power management for ufs wlun Asutosh Das 2021-04-16 19:49 ` Asutosh Das 2021-04-16 19:49 ` Asutosh Das 2021-04-16 19:49 ` [PATCH v20 1/2] scsi: ufs: Enable power management for wlun Asutosh Das 2021-04-16 19:49 ` Asutosh Das 2021-04-16 19:49 ` Asutosh Das 2021-04-19 18:37 ` Adrian Hunter 2021-04-19 18:37 ` Adrian Hunter 2021-04-19 18:37 ` Adrian Hunter 2021-04-19 21:53 ` Asutosh Das (asd) 2021-04-19 21:53 ` Asutosh Das (asd) 2021-04-20 4:15 ` Adrian Hunter 2021-04-20 4:15 ` Adrian Hunter 2021-04-20 4:15 ` Adrian Hunter 2021-04-20 7:42 ` Adrian Hunter 2021-04-20 7:42 ` Adrian Hunter 2021-04-20 7:42 ` Adrian Hunter 2021-04-22 16:38 ` Asutosh Das (asd) 2021-04-22 16:38 ` Asutosh Das (asd) 2021-04-23 4:23 ` Adrian Hunter 2021-04-23 4:23 ` Adrian Hunter 2021-04-23 4:23 ` Adrian Hunter 2021-04-23 6:18 ` Adrian Hunter 2021-04-23 6:18 ` Adrian Hunter 2021-04-23 6:18 ` Adrian Hunter 2021-04-23 8:01 ` Adrian Hunter 2021-04-23 8:01 ` Adrian Hunter 2021-04-23 8:01 ` Adrian Hunter 2021-04-23 23:44 ` Asutosh Das (asd) [this message] 2021-04-23 23:44 ` Asutosh Das (asd) 2021-04-16 19:49 ` [PATCH v20 2/2] ufs: sysfs: Resume the proper scsi device Asutosh Das
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=a71d6ac3-0dc6-4fa8-3643-6d3473d08797@codeaurora.org \ --to=asutoshd@codeaurora.org \ --cc=adrian.hunter@intel.com \ --cc=agross@kernel.org \ --cc=alim.akhtar@samsung.com \ --cc=avri.altman@wdc.com \ --cc=beanhuo@micron.com \ --cc=bjorn.andersson@linaro.org \ --cc=bvanassche@acm.org \ --cc=cang@codeaurora.org \ --cc=colin.king@canonical.com \ --cc=dinghao.liu@zju.edu.cn \ --cc=gustavoars@kernel.org \ --cc=huyue2@yulong.com \ --cc=jaegeuk@kernel.org \ --cc=jejb@linux.ibm.com \ --cc=krzk@kernel.org \ --cc=kwmad.kim@samsung.com \ --cc=lee.jones@linaro.org \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=linux-arm-msm@vger.kernel.org \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-mediatek@lists.infradead.org \ --cc=linux-samsung-soc@vger.kernel.org \ --cc=linux-scsi@vger.kernel.org \ --cc=martin.petersen@oracle.com \ --cc=matthias.bgg@gmail.com \ --cc=mingo@redhat.com \ --cc=rostedt@goodmis.org \ --cc=satyat@google.com \ --cc=stanley.chu@mediatek.com \ --cc=weiyongjun1@huawei.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: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.