Enable power management for ufs wlun
Subsystems (5)
v17 cover Enable power management for ufs wlun
This patch attempts to fix a deadlock in ufs while sending SSU. Recently, blk_queue_enter() added a check to not process requests if the queue is suspended. That leads to a resume of the associated device which is suspended. In ufs, that device is ufs device wlun and it's parent is ufs_hba. This resume tries to resume ufs device wlun which in turn tries to resume ufs_hba, which is already in the process of suspending, thus causing a deadlock.
This patch takes care of:
* Suspending the ufs device lun only after all other luns are suspended
* Sending SSU during ufs device wlun suspend
* Clearing uac for rpmb and ufs device wlun
* Not sending commands to the device during host suspend
v16 -> v17:
- Addressed Adrian's & Daejun's comments
v15 -> v16:
- Brought back the missing changes
* Added scsi_autopm_[get/put] to ufs_debugfs[get/put]_user_access()
* Fix ufshcd_wl_poweroff()
v14 -> v15:
- Rebased on 5.13/scsi-staging
v13 -> v14:
- Addressed Adrian's comments
* Rebased it on top of scsi-next
* Added scsi_autopm_[get/put] to ufs_debugfs[get/put]_user_access()
* Resume the device in ufshcd_remove()
* Unregister ufs_rpmb_wlun before ufs_dev_wlun
* hba->shutting_down moved to ufshcd_wl_shutdown()
v12 -> v13:
- Addressed Adrian's comments
* Paired pm_runtime_get_noresume() with pm_runtime_put()
* no rpm_autosuspend for ufs device wlun
* Moved runtime-pm init functionality to ufshcd_wl_probe()
- Addressed Bart's comments
* Expanded abbrevs in commit message
v11 -> v12:
- Addressed Adrian's comments
* Fixed ahit for Mediatek driver
* Fixed error handling in ufshcd_core_init()
* Tested this patch and the issue is still seen.
v10 -> v11:
- Fixed supplier suspending before consumer race
- Addressed Adrian's comments
* Added proper resume/suspend cb to ufshcd_auto_hibern8_update()
* Cosmetic changes to ufshcd-pci.c
* Cleaned up ufshcd_system_suspend()
* Added ufshcd_debugfs_eh_exit to ufshcd_core_init()
v9 -> v10:
- Addressed Adrian's comments
* Moved suspend/resume vops to __ufshcd_wl_[suspend/resume]()
* Added correct resume in ufs_bsg
v8 -> v9:
- Addressed Adrian's comments
* Moved link transition to __ufshcd_wl_[suspend/resume]()
* Fixed the other minor comments
v7 -> v8:
- Addressed Adrian's comments
* Removed separate autosuspend delay for ufs-device lun
* Fixed the ee handler getting scheduled during pm
* Always runtime resume in suspend_prepare()
* Added CONFIG_PM_SLEEP where needed
v6 -> v7:
* Resume the ufs device before shutting it down
v5 -> v6:
- Addressed Adrian's comments
* Added complete() cb
* Added suspend_prepare() and complete() to all drivers
* Moved suspend_prepare() and complete() to ufshcd
* .poweroff() uses ufhcd_wl_poweroff()
* Removed several forward declarations
* Moved scsi_register_driver() to ufshcd_core_init()
v4 -> v5:
- Addressed Adrian's comments
* Used the rpmb driver contributed by Adrian
* Runtime-resume the ufs device during suspend to honor spm-lvl
* Unregister the scsi_driver in ufshcd_remove()
* Currently shutdown() puts the ufs device to power-down mode so, just removed ufshcd_pci_poweroff()
* Quiesce the scsi device during shutdown instead of remove
v3 RFC -> v4:
- Addressed Bart's comments
* Except that I didn't get any checkpatch failures
- Addressed Avri's comments
- Addressed Adrian's comments
* Added a check for deepsleep power mode
* Removed a couple of forward declarations
* Didn't separate the scsi drivers because in rpmb case it just sends uac in resume and it seemed pretty neat to me.
- Added sysfs changes to resume the devices before accessing
Asutosh Das (2):
scsi: ufs: Enable power management for wlun
ufs: sysfs: Resume the proper scsi device
drivers/scsi/ufs/cdns-pltfrm.c | 2 + drivers/scsi/ufs/tc-dwc-g210-pci.c | 2 + drivers/scsi/ufs/ufs-debugfs.c | 6 +- drivers/scsi/ufs/ufs-debugfs.h | 2 +- drivers/scsi/ufs/ufs-exynos.c | 2 + drivers/scsi/ufs/ufs-hisi.c | 2 + drivers/scsi/ufs/ufs-mediatek.c | 12 +- drivers/scsi/ufs/ufs-qcom.c | 2 + drivers/scsi/ufs/ufs-sysfs.c | 30 +- drivers/scsi/ufs/ufs_bsg.c | 6 +- drivers/scsi/ufs/ufshcd-pci.c | 36 +-- drivers/scsi/ufs/ufshcd.c | 642 ++++++++++++++++++++++++++----------- drivers/scsi/ufs/ufshcd.h | 6 + include/trace/events/ufs.h | 20 ++ 14 files changed, 526 insertions(+), 244 deletions(-)
Signature · 1 line
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project.
PATCH v17 1/2 scsi: ufs: Enable power management for wlun
During runtime-suspend of ufs host, the scsi devices are already suspended and so are the queues associated with them. But the ufs host sends SSU (START_STOP_UNIT) to wlun
during its runtime-suspend.
During the process blk_queue_enter checks if the queue is not in suspended state. If so, it waits for the queue to resume, and never comes out of it.
The commit
(d55d15a33: scsi: block: Do not accept any requests while suspended) adds the check if the queue is in suspended state in blk_queue_enter().
Call trace: __switch_to+0x174/0x2c4 __schedule+0x478/0x764 schedule+0x9c/0xe0 blk_queue_enter+0x158/0x228 blk_mq_alloc_request+0x40/0xa4 blk_get_request+0x2c/0x70 __scsi_execute+0x60/0x1c4 ufshcd_set_dev_pwr_mode+0x124/0x1e4 ufshcd_suspend+0x208/0x83c ufshcd_runtime_suspend+0x40/0x154 ufshcd_pltfrm_runtime_suspend+0x14/0x20 pm_generic_runtime_suspend+0x28/0x3c __rpm_callback+0x80/0x2a4 rpm_suspend+0x308/0x614 rpm_idle+0x158/0x228 pm_runtime_work+0x84/0xac process_one_work+0x1f0/0x470 worker_thread+0x26c/0x4c8 kthread+0x13c/0x320 ret_from_fork+0x10/0x18
Fix this by registering ufs device wlun as a scsi driver and registering it for block runtime-pm. Also make this as a supplier for all other luns. That way, this device wlun suspends after all the consumers and resumes after hba resumes.
--- drivers/scsi/ufs/cdns-pltfrm.c | 2 + drivers/scsi/ufs/tc-dwc-g210-pci.c | 2 + drivers/scsi/ufs/ufs-debugfs.c | 6 +- drivers/scsi/ufs/ufs-debugfs.h | 2 +- drivers/scsi/ufs/ufs-exynos.c | 2 + drivers/scsi/ufs/ufs-hisi.c | 2 + drivers/scsi/ufs/ufs-mediatek.c | 12 +- drivers/scsi/ufs/ufs-qcom.c | 2 + drivers/scsi/ufs/ufs_bsg.c | 6 +- drivers/scsi/ufs/ufshcd-pci.c | 36 +-- drivers/scsi/ufs/ufshcd.c | 642 ++++++++++++++++++++++++++----------- drivers/scsi/ufs/ufshcd.h | 6 + include/trace/events/ufs.h | 20 ++ 13 files changed, 509 insertions(+), 231 deletions(-)
drivers/scsi/ufs/cdns-pltfrm.c +2 -0
@@ -323,6 +323,8 @@ static const struct dev_pm_ops cdns_ufs_dev_pm_ops = { .runtime_suspend = ufshcd_pltfrm_runtime_suspend, .runtime_resume = ufshcd_pltfrm_runtime_resume, .runtime_idle = ufshcd_pltfrm_runtime_idle, + .prepare = ufshcd_suspend_prepare, + .complete = ufshcd_resume_complete, };static struct platform_driver cdns_ufs_pltfrm_driver = {
drivers/scsi/ufs/tc-dwc-g210-pci.c +2 -0
@@ -148,6 +148,8 @@ static const struct dev_pm_ops tc_dwc_g210_pci_pm_ops = { .runtime_suspend = tc_dwc_g210_pci_runtime_suspend, .runtime_resume = tc_dwc_g210_pci_runtime_resume, .runtime_idle = tc_dwc_g210_pci_runtime_idle, + .prepare = ufshcd_suspend_prepare, + .complete = ufshcd_resume_complete, };static const struct pci_device_id tc_dwc_g210_pci_tbl[] = {
drivers/scsi/ufs/ufs-debugfs.c +3 -3
@@ -13,7 +13,7 @@ void __init ufs_debugfs_init(void) ufs_debugfs_root =debugfs_create_dir ("ufshcd" ,NULL ); } -void __exit ufs_debugfs_exit(void) +void ufs_debugfs_exit (void ) {debugfs_remove_recursive (ufs_debugfs_root); }
@@ -60,14 +60,14 @@ __acquires(&hba->host_sem)up (&hba->host_sem);return -EBUSY ; } -pm_runtime_get_sync (hba->dev); +scsi_autopm_get_device (hba->sdev_ufs_device);return 0 ; }static void ufs_debugfs_put_user_access (struct ufs_hba *hba) __releases(&hba->host_sem) { -pm_runtime_put_sync (hba->dev); +scsi_autopm_put_device (hba->sdev_ufs_device);up (&hba->host_sem); }
drivers/scsi/ufs/ufs-debugfs.h +1 -1
@@ -9,7 +9,7 @@ struct ufs_hba;#ifdef CONFIG_DEBUG_FS void __init ufs_debugfs_init (void ); -void __exit ufs_debugfs_exit(void); +void ufs_debugfs_exit (void );void ufs_debugfs_hba_init (struct ufs_hba *hba);void ufs_debugfs_hba_exit (struct ufs_hba *hba);void ufs_debugfs_exception_event (struct ufs_hba *hba,u16 status);
drivers/scsi/ufs/ufs-exynos.c +2 -0
@@ -1267,6 +1267,8 @@ static const struct dev_pm_ops exynos_ufs_pm_ops = { .runtime_suspend = ufshcd_pltfrm_runtime_suspend, .runtime_resume = ufshcd_pltfrm_runtime_resume, .runtime_idle = ufshcd_pltfrm_runtime_idle, + .prepare = ufshcd_suspend_prepare, + .complete = ufshcd_resume_complete, };static struct platform_driver exynos_ufs_pltform = {
drivers/scsi/ufs/ufs-hisi.c +2 -0
@@ -574,6 +574,8 @@ static const struct dev_pm_ops ufs_hisi_pm_ops = { .runtime_suspend = ufshcd_pltfrm_runtime_suspend, .runtime_resume = ufshcd_pltfrm_runtime_resume, .runtime_idle = ufshcd_pltfrm_runtime_idle, + .prepare = ufshcd_suspend_prepare, + .complete = ufshcd_resume_complete, };static struct platform_driver ufs_hisi_pltform = {
drivers/scsi/ufs/ufs-mediatek.c +6 -6
@@ -810,12 +810,10 @@ static int ufs_mtk_post_link(struct ufs_hba *hba)/* enable unipro clock gating feature */ ufs_mtk_cfg_unipro_cg (hba, true); -/* configure auto-hibern8 timer to 10ms */ -if (ufshcd_is_auto_hibern8_supported (hba)) { -ufshcd_auto_hibern8_update (hba, -FIELD_PREP (UFSHCI_AHIBERN8_TIMER_MASK ,10 ) | -FIELD_PREP (UFSHCI_AHIBERN8_SCALE_MASK ,3 )); - } +/* will be configured during probe hba */ +if (ufshcd_is_auto_hibern8_supported (hba)) + hba->ahit =FIELD_PREP (UFSHCI_AHIBERN8_TIMER_MASK ,10 ) | +FIELD_PREP (UFSHCI_AHIBERN8_SCALE_MASK ,3 );ufs_mtk_setup_clk_gating (hba);
@@ -1097,6 +1095,8 @@ static const struct dev_pm_ops ufs_mtk_pm_ops = { .runtime_suspend = ufshcd_pltfrm_runtime_suspend, .runtime_resume = ufshcd_pltfrm_runtime_resume, .runtime_idle = ufshcd_pltfrm_runtime_idle, + .prepare = ufshcd_suspend_prepare, + .complete = ufshcd_resume_complete, };static struct platform_driver ufs_mtk_pltform = {
drivers/scsi/ufs/ufs-qcom.c +2 -0
@@ -1556,6 +1556,8 @@ static const struct dev_pm_ops ufs_qcom_pm_ops = { .runtime_suspend = ufshcd_pltfrm_runtime_suspend, .runtime_resume = ufshcd_pltfrm_runtime_resume, .runtime_idle = ufshcd_pltfrm_runtime_idle, + .prepare = ufshcd_suspend_prepare, + .complete = ufshcd_resume_complete, };static struct platform_driver ufs_qcom_pltform = {
drivers/scsi/ufs/ufs_bsg.c +3 -3
@@ -97,7 +97,7 @@ static int ufs_bsg_request(struct bsg_job *job) bsg_reply->reply_payload_rcv_len =0 ; -pm_runtime_get_sync (hba->dev); +scsi_autopm_get_device (hba->sdev_ufs_device); msgcode = bsg_request->msgcode;switch (msgcode) {
@@ -106,7 +106,7 @@ static int ufs_bsg_request(struct bsg_job *job) ret =ufs_bsg_alloc_desc_buffer (hba, job, &desc_buff, &desc_len, desc_op);if (ret) { -pm_runtime_put_sync (hba->dev); +scsi_autopm_put_device (hba->sdev_ufs_device); goto out; }
@@ -138,7 +138,7 @@ static int ufs_bsg_request(struct bsg_job *job)break ; } -pm_runtime_put_sync (hba->dev); +scsi_autopm_put_device (hba->sdev_ufs_device);if (!desc_buff) goto out;
drivers/scsi/ufs/ufshcd-pci.c +5 -31
@@ -410,29 +410,6 @@ static int ufshcd_pci_resume(struct device *dev) return ufshcd_system_resume(dev_get_drvdata(dev)); } -/** -* ufshcd_pci_poweroff - suspend-to-disk poweroff function -* @dev: pointer to PCI device handle -* -* Returns 0 if successful -* Returns non-zero otherwise -*/ -static int ufshcd_pci_poweroff (struct device *dev) -{ -struct ufs_hba *hba =dev_get_drvdata (dev); -int spm_lvl = hba->spm_lvl; -int ret; - -/* -* For poweroff we need to set the UFS device to PowerDown mode. -* Force spm_lvl to ensure that. -*/ - hba->spm_lvl =5 ; - ret =ufshcd_system_suspend (hba); - hba->spm_lvl = spm_lvl; -return ret; -} -#endif /* !CONFIG_PM_SLEEP */ #ifdef CONFIG_PM
@@ -533,17 +510,14 @@ ufshcd_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id) }static const struct dev_pm_ops ufshcd_pci_pm_ops = { -#ifdef CONFIG_PM_SLEEP - .suspend = ufshcd_pci_suspend, - .resume = ufshcd_pci_resume, - .freeze = ufshcd_pci_suspend, - .thaw = ufshcd_pci_resume, - .poweroff = ufshcd_pci_poweroff, - .restore = ufshcd_pci_resume, -#endif SET_RUNTIME_PM_OPS(ufshcd_pci_runtime_suspend, ufshcd_pci_runtime_resume, ufshcd_pci_runtime_idle) + SET_SYSTEM_SLEEP_PM_OPS(ufshcd_pci_suspend, ufshcd_pci_resume) +#ifdef CONFIG_PM_SLEEP + .prepare = ufshcd_suspend_prepare, + .complete = ufshcd_resume_complete, +#endif };static const struct pci_device_id ufshcd_pci_tbl[] = {
drivers/scsi/ufs/ufshcd.c +455 -187
980 lines not on this page · raw message
drivers/scsi/ufs/ufshcd.h +6 -0
@@ -72,6 +72,8 @@ enum ufs_event_type {UFS_EVT_LINK_STARTUP_FAIL ,UFS_EVT_RESUME_ERR ,UFS_EVT_SUSPEND_ERR , +UFS_EVT_WL_SUSP_ERR , +UFS_EVT_WL_RES_ERR ,/* abnormal events */ UFS_EVT_DEV_RESET,
@@ -807,6 +809,7 @@ struct ufs_hba {struct list_head clk_list_head;bool wlun_dev_clr_ua; +bool wlun_rpmb_clr_ua;/* Number of requests aborts */ int req_abort_count;
@@ -846,6 +849,7 @@ struct ufs_hba {struct delayed_work debugfs_ee_work;u32 debugfs_ee_rate_limit_ms;#endif +u32 luns_avail; };/* Returns true if clocks can be gated. Otherwise false */
@@ -1105,6 +1109,8 @@ int ufshcd_exec_raw_upiu_cmd(struct ufs_hba *hba, enum query_opcode desc_op);int ufshcd_wb_toggle (struct ufs_hba *hba,bool enable); +int ufshcd_suspend_prepare (struct device *dev); +void ufshcd_resume_complete (struct device *dev);/* Wrapper functions for safely calling variant operations */ static inline const char *ufshcd_get_var_name (struct ufs_hba *hba)
include/trace/events/ufs.h +20 -0
@@ -246,6 +246,26 @@ DEFINE_EVENT(ufshcd_template, ufshcd_init, int dev_state, int link_state), TP_ARGS(dev_name, err, usecs, dev_state, link_state)); +DEFINE_EVENT (ufshcd_template, ufshcd_wl_suspend, + TP_PROTO(const char *dev_name, int err, s64 usecs, + int dev_state, int link_state), +TP_ARGS (dev_name, err, usecs, dev_state, link_state)); + +DEFINE_EVENT (ufshcd_template, ufshcd_wl_resume, + TP_PROTO(const char *dev_name, int err, s64 usecs, + int dev_state, int link_state), +TP_ARGS (dev_name, err, usecs, dev_state, link_state)); + +DEFINE_EVENT (ufshcd_template, ufshcd_wl_runtime_suspend, + TP_PROTO(const char *dev_name, int err, s64 usecs, + int dev_state, int link_state), +TP_ARGS (dev_name, err, usecs, dev_state, link_state)); + +DEFINE_EVENT (ufshcd_template, ufshcd_wl_runtime_resume, + TP_PROTO(const char *dev_name, int err, s64 usecs, + int dev_state, int link_state), +TP_ARGS (dev_name, err, usecs, dev_state, link_state)); + TRACE_EVENT(ufshcd_command, TP_PROTO(const char *dev_name, enum ufs_trace_str_t str_t, unsigned int tag, u32 doorbell, int transfer_len, u32 intr,
Signature · 1 line
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project.
RE: [PATCH v17 1/2] scsi: ufs: Enable power management for wlun
Hi Asutosh Das,
Quoted 43 lines
During runtime-suspend of ufs host, the scsi devices are already suspended and so are the queues associated with them. But the ufs host sends SSU (
START_STOP_UNIT) to wlun
during its runtime-suspend.
During the processblk_queue_enterchecks if the queue is not in suspended state. If so, it waits for the queue to resume, and never comes out of it.
The commit
(d55d15a33: scsi: block: Do not accept any requests while suspended) adds the check if the queue is in suspended state inblk_queue_enter().Call trace: __switch_to+0x174/0x2c4 __schedule+0x478/0x764 schedule+0x9c/0xe0 blk_queue_enter+0x158/0x228 blk_mq_alloc_request+0x40/0xa4 blk_get_request+0x2c/0x70 __scsi_execute+0x60/0x1c4 ufshcd_set_dev_pwr_mode+0x124/0x1e4 ufshcd_suspend+0x208/0x83c ufshcd_runtime_suspend+0x40/0x154 ufshcd_pltfrm_runtime_suspend+0x14/0x20 pm_generic_runtime_suspend+0x28/0x3c __rpm_callback+0x80/0x2a4 rpm_suspend+0x308/0x614 rpm_idle+0x158/0x228 pm_runtime_work+0x84/0xac process_one_work+0x1f0/0x470 worker_thread+0x26c/0x4c8 kthread+0x13c/0x320 ret_from_fork+0x10/0x18Fix this by registering ufs device wlun as a scsi driver and registering it for block runtime-pm. Also make this as a supplier for all other luns. That way, this device wlun suspends after all the consumers and resumes after hba resumes.
Co-developed-by Can Guo <[email protected]>Signed-off-by Can Guo <[email protected]>Signed-off-by Asutosh Das <[email protected]>---
Quoted 14 lines
drivers/scsi/ufs/cdns-pltfrm.c | 2 + drivers/scsi/ufs/tc-dwc-g210-pci.c | 2 + drivers/scsi/ufs/ufs-debugfs.c | 6 +- drivers/scsi/ufs/ufs-debugfs.h | 2 +- drivers/scsi/ufs/ufs-exynos.c | 2 + drivers/scsi/ufs/ufs-hisi.c | 2 + drivers/scsi/ufs/ufs-mediatek.c | 12 +- drivers/scsi/ufs/ufs-qcom.c | 2 + drivers/scsi/ufs/ufs_bsg.c | 6 +- drivers/scsi/ufs/ufshcd-pci.c | 36 +-- drivers/scsi/ufs/ufshcd.c | 642 ++++++++++++++++++++++++++----------- drivers/scsi/ufs/ufshcd.h | 6 + include/trace/events/ufs.h | 20 ++ 13 files changed, 509 insertions(+), 231 deletions(-)
In this patch, you changed pm_runtime_{get, put}_sync to scsi_autopm_{get, put}_device. But, scsi_autopm_get_device() calls pm_runtime_put_sync() in case of error of pm_runtime_get_sync(). So, pm_runtime_put_sync() can be called twice if scsi_autopm_get_device has error.
Thanks,
Daejun
Re: [PATCH v17 1/2] scsi: ufs: Enable power management for wlun
Quoted 60 lines · Daejun Park
Hi Asutosh Das,
Quoted 43 lines
During runtime-suspend of ufs host, the scsi devices are already suspended and so are the queues associated with them. But the ufs host sends SSU (
START_STOP_UNIT) to wlun
during its runtime-suspend.
During the processblk_queue_enterchecks if the queue is not in suspended state. If so, it waits for the queue to resume, and never comes out of it.
The commit
(d55d15a33: scsi: block: Do not accept any requests while suspended) adds the check if the queue is in suspended state inblk_queue_enter().Call trace: __switch_to+0x174/0x2c4 __schedule+0x478/0x764 schedule+0x9c/0xe0 blk_queue_enter+0x158/0x228 blk_mq_alloc_request+0x40/0xa4 blk_get_request+0x2c/0x70 __scsi_execute+0x60/0x1c4 ufshcd_set_dev_pwr_mode+0x124/0x1e4 ufshcd_suspend+0x208/0x83c ufshcd_runtime_suspend+0x40/0x154 ufshcd_pltfrm_runtime_suspend+0x14/0x20 pm_generic_runtime_suspend+0x28/0x3c __rpm_callback+0x80/0x2a4 rpm_suspend+0x308/0x614 rpm_idle+0x158/0x228 pm_runtime_work+0x84/0xac process_one_work+0x1f0/0x470 worker_thread+0x26c/0x4c8 kthread+0x13c/0x320 ret_from_fork+0x10/0x18Fix this by registering ufs device wlun as a scsi driver and registering it for block runtime-pm. Also make this as a supplier for all other luns. That way, this device wlun suspends after all the consumers and resumes after hba resumes.
Co-developed-by Can Guo <[email protected]>Signed-off-by Can Guo <[email protected]>Signed-off-by Asutosh Das <[email protected]>---
Quoted 14 lines
drivers/scsi/ufs/cdns-pltfrm.c | 2 + drivers/scsi/ufs/tc-dwc-g210-pci.c | 2 + drivers/scsi/ufs/ufs-debugfs.c | 6 +- drivers/scsi/ufs/ufs-debugfs.h | 2 +- drivers/scsi/ufs/ufs-exynos.c | 2 + drivers/scsi/ufs/ufs-hisi.c | 2 + drivers/scsi/ufs/ufs-mediatek.c | 12 +- drivers/scsi/ufs/ufs-qcom.c | 2 + drivers/scsi/ufs/ufs_bsg.c | 6 +- drivers/scsi/ufs/ufshcd-pci.c | 36 +-- drivers/scsi/ufs/ufshcd.c | 642 ++++++++++++++++++++++++++----------- drivers/scsi/ufs/ufshcd.h | 6 + include/trace/events/ufs.h | 20 ++ 13 files changed, 509 insertions(+), 231 deletions(-)
Quoted 4 lines
In this patch, you changed pm_runtime_{get, put}_sync to scsi_autopm_{get, put}_device. But,
scsi_autopm_get_device()callspm_runtime_put_sync()in case of error ofpm_runtime_get_sync(). So,pm_runtime_put_sync()can be called twice ifscsi_autopm_get_devicehas error.
Also it might be tidy to make wrappers e.g.
static inline int ufshcd_rpm_get_sync(struct ufs_hba *hba)
{
return pm_runtime_get_sync(&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);
}static inline int ufshcd_rpm_put_sync(struct ufs_hba *hba)
{
return pm_runtime_put_sync(&hba->sdev_ufs_device->sdev_gendev);
} And also consider matching: e.g.
pm_runtime_put(hba->dev) to ufshcd_rpm_put(hba) pm_runtime_put_sync(hba->dev) to ufshcd_rpm_put_sync(hba)
Re: [PATCH v17 1/2] scsi: ufs: Enable power management for wlun
Quoted 79 lines · Adrian Hunter
Quoted 60 lines · Daejun Park
Hi Asutosh Das,
Quoted 43 lines
During runtime-suspend of ufs host, the scsi devices are already suspended and so are the queues associated with them. But the ufs host sends SSU (
START_STOP_UNIT) to wlun
during its runtime-suspend.
During the processblk_queue_enterchecks if the queue is not in suspended state. If so, it waits for the queue to resume, and never comes out of it.
The commit
(d55d15a33: scsi: block: Do not accept any requests while suspended) adds the check if the queue is in suspended state inblk_queue_enter().Call trace: __switch_to+0x174/0x2c4 __schedule+0x478/0x764 schedule+0x9c/0xe0 blk_queue_enter+0x158/0x228 blk_mq_alloc_request+0x40/0xa4 blk_get_request+0x2c/0x70 __scsi_execute+0x60/0x1c4 ufshcd_set_dev_pwr_mode+0x124/0x1e4 ufshcd_suspend+0x208/0x83c ufshcd_runtime_suspend+0x40/0x154 ufshcd_pltfrm_runtime_suspend+0x14/0x20 pm_generic_runtime_suspend+0x28/0x3c __rpm_callback+0x80/0x2a4 rpm_suspend+0x308/0x614 rpm_idle+0x158/0x228 pm_runtime_work+0x84/0xac process_one_work+0x1f0/0x470 worker_thread+0x26c/0x4c8 kthread+0x13c/0x320 ret_from_fork+0x10/0x18Fix this by registering ufs device wlun as a scsi driver and registering it for block runtime-pm. Also make this as a supplier for all other luns. That way, this device wlun suspends after all the consumers and resumes after hba resumes.
Co-developed-by Can Guo <[email protected]>Signed-off-by Can Guo <[email protected]>Signed-off-by Asutosh Das <[email protected]>---
Quoted 14 lines
drivers/scsi/ufs/cdns-pltfrm.c | 2 + drivers/scsi/ufs/tc-dwc-g210-pci.c | 2 + drivers/scsi/ufs/ufs-debugfs.c | 6 +- drivers/scsi/ufs/ufs-debugfs.h | 2 +- drivers/scsi/ufs/ufs-exynos.c | 2 + drivers/scsi/ufs/ufs-hisi.c | 2 + drivers/scsi/ufs/ufs-mediatek.c | 12 +- drivers/scsi/ufs/ufs-qcom.c | 2 + drivers/scsi/ufs/ufs_bsg.c | 6 +- drivers/scsi/ufs/ufshcd-pci.c | 36 +-- drivers/scsi/ufs/ufshcd.c | 642 ++++++++++++++++++++++++++----------- drivers/scsi/ufs/ufshcd.h | 6 + include/trace/events/ufs.h | 20 ++ 13 files changed, 509 insertions(+), 231 deletions(-)Quoted 4 lines
In this patch, you changed pm_runtime_{get, put}_sync to scsi_autopm_{get, put}_device. But,
scsi_autopm_get_device()callspm_runtime_put_sync()in case of error ofpm_runtime_get_sync(). So,pm_runtime_put_sync()can be called twice ifscsi_autopm_get_devicehas error.Also it might be tidy to make wrappers e.g.
static inline int
ufshcd_rpm_get_sync(struct ufs_hba *hba){ return pm_runtime_get_sync(&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); }static inline int
ufshcd_rpm_put_sync(struct ufs_hba *hba)
Quoted 11 lines
{ return pm_runtime_put_sync(&hba->sdev_ufs_device->sdev_gendev); }And also consider matching: e.g.
pm_runtime_put(hba->dev) to ufshcd_rpm_put(hba) pm_runtime_put_sync(hba->dev) to ufshcd_rpm_put_sync(hba)
Ok, I'll push the changes shortly.
Signature · 2 lines
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, Linux Foundation Collaborative Project
Re: [PATCH v17 1/2] scsi: ufs: Enable power management for wlun
Quoted 85 lines · Asutosh Das (asd)
Quoted 84 lines · Adrian Hunter
Quoted 60 lines · Daejun Park
Hi Asutosh Das,
Quoted 56 lines
During runtime-suspend of ufs host, the scsi devices are already suspended and so are the queues associated with them. But the ufs host sends SSU (
START_STOP_UNIT) to wlun
during its runtime-suspend.
During the processblk_queue_enterchecks if the queue is not in suspended state. If so, it waits for the queue to resume, and never comes out of it.
The commit
(d55d15a33: scsi: block: Do not accept any requests while suspended) adds the check if the queue is in suspended state inblk_queue_enter().Call trace: __switch_to+0x174/0x2c4 __schedule+0x478/0x764 schedule+0x9c/0xe0 blk_queue_enter+0x158/0x228 blk_mq_alloc_request+0x40/0xa4 blk_get_request+0x2c/0x70 __scsi_execute+0x60/0x1c4 ufshcd_set_dev_pwr_mode+0x124/0x1e4 ufshcd_suspend+0x208/0x83c ufshcd_runtime_suspend+0x40/0x154 ufshcd_pltfrm_runtime_suspend+0x14/0x20 pm_generic_runtime_suspend+0x28/0x3c __rpm_callback+0x80/0x2a4 rpm_suspend+0x308/0x614 rpm_idle+0x158/0x228 pm_runtime_work+0x84/0xac process_one_work+0x1f0/0x470 worker_thread+0x26c/0x4c8 kthread+0x13c/0x320 ret_from_fork+0x10/0x18Fix this by registering ufs device wlun as a scsi driver and registering it for block runtime-pm. Also make this as a supplier for all other luns. That way, this device wlun suspends after all the consumers and resumes after hba resumes.
Co-developed-by Can Guo <[email protected]>Signed-off-by Can Guo <[email protected]>Signed-off-by Asutosh Das <[email protected]>---
drivers/scsi/ufs/cdns-pltfrm.c | 2 +drivers/scsi/ufs/tc-dwc-g210-pci.c| 2 + drivers/scsi/ufs/ufs-debugfs.c | 6 +- drivers/scsi/ufs/ufs-debugfs.h | 2 +- drivers/scsi/ufs/ufs-exynos.c | 2 + drivers/scsi/ufs/ufs-hisi.c | 2 + drivers/scsi/ufs/ufs-mediatek.c | 12 +- drivers/scsi/ufs/ufs-qcom.c | 2 + drivers/scsi/ufs/ufs_bsg.c | 6 +- drivers/scsi/ufs/ufshcd-pci.c | 36 +-- drivers/scsi/ufs/ufshcd.c | 642 ++++++++++++++++++++++++++----------- drivers/scsi/ufs/ufshcd.h | 6 + include/trace/events/ufs.h | 20 ++Quoted 1 line
13 files changed, 509 insertions(+), 231 deletions(-)Quoted 4 lines
In this patch, you changed pm_runtime_{get, put}_sync to scsi_autopm_{get, put}_device. But,
scsi_autopm_get_device()callspm_runtime_put_sync()in case of error ofpm_runtime_get_sync(). So,pm_runtime_put_sync()can be called twice ifscsi_autopm_get_devicehas error.Also it might be tidy to make wrappers e.g.
static inline int
ufshcd_rpm_get_sync(struct ufs_hba *hba){
returnpm_runtime_get_sync(&hba->sdev_ufs_device->sdev_gendev); }
static inline intufshcd_rpm_put(struct ufs_hba *hba){
returnpm_runtime_put(&hba->sdev_ufs_device->sdev_gendev); }static inline int
ufshcd_rpm_put_sync(struct ufs_hba *hba){
returnpm_runtime_put_sync(&hba->sdev_ufs_device->sdev_gendev); }And also consider matching: e.g.
Quoted 8 lines
Quoted 5 lines
pm_runtime_put(hba->dev) to
ufshcd_rpm_put(hba)pm_runtime_put_sync(hba->dev) toufshcd_rpm_put_sync(hba)Ok, I'll push the changes shortly.
I think I will have some more comments, so you could wait if you want.
Re: [PATCH v17 1/2] scsi: ufs: Enable power management for wlun
In reply to Asutosh DasQuoted 43 lines · Asutosh Das
During runtime-suspend of ufs host, the scsi devices are already suspended and so are the queues associated with them. But the ufs host sends SSU (
START_STOP_UNIT) to wlun
during its runtime-suspend.
During the processblk_queue_enterchecks if the queue is not in suspended state. If so, it waits for the queue to resume, and never comes out of it.
The commit
(d55d15a33: scsi: block: Do not accept any requests while suspended) adds the check if the queue is in suspended state inblk_queue_enter().Call trace: __switch_to+0x174/0x2c4 __schedule+0x478/0x764 schedule+0x9c/0xe0 blk_queue_enter+0x158/0x228 blk_mq_alloc_request+0x40/0xa4 blk_get_request+0x2c/0x70 __scsi_execute+0x60/0x1c4 ufshcd_set_dev_pwr_mode+0x124/0x1e4 ufshcd_suspend+0x208/0x83c ufshcd_runtime_suspend+0x40/0x154 ufshcd_pltfrm_runtime_suspend+0x14/0x20 pm_generic_runtime_suspend+0x28/0x3c __rpm_callback+0x80/0x2a4 rpm_suspend+0x308/0x614 rpm_idle+0x158/0x228 pm_runtime_work+0x84/0xac process_one_work+0x1f0/0x470 worker_thread+0x26c/0x4c8 kthread+0x13c/0x320 ret_from_fork+0x10/0x18Fix this by registering ufs device wlun as a scsi driver and registering it for block runtime-pm. Also make this as a supplier for all other luns. That way, this device wlun suspends after all the consumers and resumes after hba resumes.
Co-developed-by Can Guo <[email protected]>Signed-off-by Can Guo <[email protected]>Signed-off-by Asutosh Das <[email protected]>---
<SNIP>
drivers/scsi/ufs/ufshcd.c static void ufshcd_recover_pm_error(struct ufs_hba *hba) · line 5863
Quoted 10 lines
drivers/scsi/ufs/ufshcd.c +2
-2@@ -5815,14 +5857,14 @@ static void ufshcd_recover_pm_error(struct ufs_hba *hba) hba->is_sys_suspended = false;/* -* Set RPM status of hba device to RPM_ACTIVE, +* Set RPM status of wlun device to RPM_ACTIVE, * this also clears its runtime error. */ - ret =pm_runtime_set_active (hba->dev); + ret =pm_runtime_set_active (&hba->sdev_ufs_device->sdev_gendev);
It may be that it was hba->dev with a runtime error. So, also need here:
if (ret) ret = pm_runtime_set_active(hba->dev);
Quoted 10 lines
drivers/scsi/ufs/ufshcd.c +1
-1@@ -8671,7 +8701,7 @@ static int ufshcd_suspend(struct ufs_hba *hba, enum ufs_pm_op pm_op)enum ufs_dev_pwr_mode req_dev_pwr_mode;enum uic_link_state req_link_state; - hba->pm_op_in_progress =1 ; + hba->pm_op_in_progress = true;if (!ufshcd_is_shutdown_pm (pm_op)) { pm_lvl =ufshcd_is_runtime_pm (pm_op) ? hba->rpm_lvl : hba->spm_lvl;
drivers/scsi/ufs/ufshcd.c static int ufshcd_suspend(struct ufs_hba *hba, enum ufs_pm_op pm_op) · line 8727
Quoted 5 lines
drivers/scsi/ufs/ufshcd.c +1
-1@@ -8694,17 +8724,17 @@ static int ufshcd_suspend(struct ufs_hba *hba, enum ufs_pm_op pm_op)if (req_dev_pwr_mode ==UFS_ACTIVE_PWR_MODE && req_link_state ==UIC_LINK_ACTIVE_STATE ) { - goto disable_clks; + goto enable_scaling;
Previously "goto disable_clks" would go to the vendor callback ufshcd_vops_suspend(), but now it is skipped.
Quoted 13 lines
drivers/scsi/ufs/ufshcd.c +2
-2}if ((req_dev_pwr_mode == hba->curr_dev_pwr_mode) && (req_link_state == hba->uic_link_state)) - goto enable_gating; + goto enable_scaling;/* UFS device & link must be active before we enter in this function */ if (!ufshcd_is_ufs_dev_active (hba) || !ufshcd_is_link_active (hba)) { ret = -EINVAL ; - goto enable_gating; + goto enable_scaling; }
<SNIP>
drivers/scsi/ufs/ufshcd.c static int ufshcd_resume(struct ufs_hba *hba, enum ufs_pm_op pm_op) · line 9044
Quoted 11 lines
drivers/scsi/ufs/ufshcd.c +11
-0+/** +* ufshcd_suspend - helper function for suspend operations +* @hba: per adapter instance +* +* This function will put disable irqs, turn off clocks +* and set vreg and hba-vreg in lpm mode. +* Also check the description of __ufshcd_wl_suspend(). +*/ +static void ufshcd_suspend (struct ufs_hba *hba) +{ + hba->pm_op_in_progress =1 ;
Seems like pm_op_in_progress wouldn't be needed in this function, nor ufshcd_resume().
You could probably simplify the callers slighly by putting the "if (!hbs->is_powered) return" check here and in ufshcd_resume().
Quoted 61 lines
drivers/scsi/ufs/ufshcd.c +43
-0+ +/* +* Disable the host irq as host controller as there won't be any +* host controller transaction expected till resume. +*/ ufshcd_disable_irq (hba);ufshcd_setup_clocks (hba, false);if (ufshcd_is_clkgating_allowed (hba)) {@@ -8948,6 +9054,43 @@ static int ufshcd_resume(struct ufs_hba *hba, enum ufs_pm_op pm_op)trace_ufshcd_clk_gating (dev_name (hba->dev), hba->clk_gating.state); } + +ufshcd_vreg_set_lpm (hba); +/* Put the host controller in low power mode if possible */ +ufshcd_hba_vreg_set_lpm (hba); + hba->pm_op_in_progress =0 ; +} + +/** +* ufshcd_resume - helper function for resume operations +* @hba: per adapter instance +* +* This function basically turns on the regulators, clocks and +* irqs of the hba. +* Also check the description of __ufshcd_wl_resume(). +* +* Returns 0 for success and non-zero for failure +*/ +static int ufshcd_resume (struct ufs_hba *hba) +{ +int ret; + + hba->pm_op_in_progress =1 ; + +ufshcd_hba_vreg_set_hpm (hba); + ret =ufshcd_vreg_set_hpm (hba); +if (ret) + goto out; + +/* Make sure clocks are enabled before accessing controller */ + ret =ufshcd_setup_clocks (hba, true); +if (ret) + goto disable_vreg; + +/* enable the host irq as host controller would be active soon */ +ufshcd_enable_irq (hba); + goto out; + disable_vreg:ufshcd_vreg_set_lpm (hba); out:@@ -8962,6 +9105,7 @@ static int ufshcd_resume(struct ufs_hba *hba, enum ufs_pm_op pm_op)* @hba: per adapter instance * * Check the description of ufshcd_suspend() function for more details. +* Also check the description of __ufshcd_wl_suspend(). * * Returns 0 for success and non-zero for failure */
drivers/scsi/ufs/ufshcd.c int ufshcd_system_suspend(struct ufs_hba *hba) · line 9131
Quoted 6 lines
drivers/scsi/ufs/ufshcd.c +0
-0@@ -8982,21 +9126,7 @@ int ufshcd_system_suspend(struct ufs_hba *hba)if ((ufs_get_pm_lvl_to_dev_pwr_mode (hba->spm_lvl) == hba->curr_dev_pwr_mode) && (ufs_get_pm_lvl_to_link_pwr_state (hba->spm_lvl) == hba->uic_link_state) && !hba->dev_info.b_rpm_dev_flush_capable) goto out;
The logic above can be removed.
Quoted 4 lines
drivers/scsi/ufs/ufshcd.c +0
-3-if (pm_runtime_suspended (hba->dev)) { -/* -* UFS device and/or UFS link low power states during runtime
drivers/scsi/ufs/ufshcd.c int ufshcd_system_suspend(struct ufs_hba *hba) · line 8997
Quoted 10 lines
drivers/scsi/ufs/ufshcd.c +0
-10-* suspend seems to be different than what is expected during -* system suspend. Hence runtime resume the devic & link and -* let the system suspend low power states to take effect. -* TODO: If resume takes longer time, we might have optimize -* it in future by not resuming everything if possible. -*/ - ret =ufshcd_runtime_resume (hba); -if (ret) - goto out; - }
System suspend/resume and runtime suspend/resume are identical for hba->dev, so the logic becomes:
if (pm_runtime_suspended(hba->dev)) goto out;
Quoted 9 lines
drivers/scsi/ufs/ufshcd.c +1
-2- - ret =ufshcd_suspend (hba,UFS_SYSTEM_PM ); +ufshcd_suspend (hba);out:
drivers/scsi/ufs/ufshcd.c +0
-0trace_ufshcd_system_suspend(dev_name(hba->dev), ret, ktime_to_us(ktime_sub(ktime_get(), start)),@@ -9018,25 +9148,20 @@ EXPORT_SYMBOL(ufshcd_system_suspend);int ufshcd_system_resume (struct ufs_hba *hba)
drivers/scsi/ufs/ufshcd.c EXPORT_SYMBOL(ufshcd_system_suspend); · line 9153
Quoted 10 lines
drivers/scsi/ufs/ufshcd.c +1
-6{ -int ret =0 ;ktime_t start =ktime_get (); -if (!hba->is_powered ||pm_runtime_suspended (hba->dev)) -/* -* Let the runtime resume take care of resuming -* if runtime suspended. -*/ +if (!hba->is_powered)
System suspend/resume and runtime suspend/resume are identical for hba->dev, so the original logic is fine here.
Quoted 4 lines
drivers/scsi/ufs/ufshcd.c +1
-1goto out; else - ret =ufshcd_resume (hba,UFS_SYSTEM_PM ); +ufshcd_resume (hba );
drivers/scsi/ufs/ufshcd.c EXPORT_SYMBOL(ufshcd_system_suspend); · line 9163
Quoted 10 lines
drivers/scsi/ufs/ufshcd.c +3
-3out: -trace_ufshcd_system_resume (dev_name (hba->dev), ret, +trace_ufshcd_system_resume (dev_name (hba->dev),0 ,ktime_to_us (ktime_sub (ktime_get (), start)), hba->curr_dev_pwr_mode, hba->uic_link_state); -if (!ret) - hba->is_sys_suspended = false; + + hba->is_sys_suspended = false;up (&hba->host_sem);
It seems likely that hba->host_sem down / up should be in ufshcd_wl_suspend() / ufshcd_wl_resume() respectively, instead of here.
Ditto hba->is_sys_suspended
Quoted 119 lines
drivers/scsi/ufs/ufshcd.c +15
-32-return ret; +return 0 ; }EXPORT_SYMBOL (ufshcd_system_resume);@@ -9045,23 +9170,23 @@ EXPORT_SYMBOL(ufshcd_system_resume);* @hba: per adapter instance * * Check the description of ufshcd_suspend() function for more details. +* Also check the description of __ufshcd_wl_suspend(). * * Returns 0 for success and non-zero for failure */ int ufshcd_runtime_suspend (struct ufs_hba *hba) { -int ret =0 ;ktime_t start =ktime_get ();if (!hba->is_powered) goto out;else - ret =ufshcd_suspend (hba,UFS_RUNTIME_PM ); +ufshcd_suspend (hba); out: -trace_ufshcd_runtime_suspend (dev_name (hba->dev), ret, +trace_ufshcd_runtime_suspend (dev_name (hba->dev),0 ,ktime_to_us (ktime_sub (ktime_get (), start)), hba->curr_dev_pwr_mode, hba->uic_link_state); -return ret; +return 0 ; }EXPORT_SYMBOL (ufshcd_runtime_suspend);@@ -9069,37 +9194,25 @@ EXPORT_SYMBOL(ufshcd_runtime_suspend);* ufshcd_runtime_resume - runtime resume routine * @hba: per adapter instance * -* This function basically brings the UFS device, UniPro link and controller +* This function basically brings controller * to active state. Following operations are done in this function: * * 1. Turn on all the controller related clocks -* 2. Bring the UniPro link out of Hibernate state -* 3. If UFS device is in sleep state, turn ON VCC rail and bring the UFS device -* to active state. -* 4. If auto-bkops is enabled on the device, disable it. -* -* So following would be the possible power state after this function return -* successfully: -* S1: UFS device in Active state with VCC rail ON -* UniPro link in Active state -* All the UFS/UniPro controller clocks are ON -* -* Returns 0 for success and non-zero for failure +* 2. Turn ON VCC rail */ int ufshcd_runtime_resume (struct ufs_hba *hba) { -int ret =0 ;ktime_t start =ktime_get ();if (!hba->is_powered) goto out;else - ret =ufshcd_resume (hba,UFS_RUNTIME_PM ); +ufshcd_resume (hba); out: -trace_ufshcd_runtime_resume (dev_name (hba->dev), ret, +trace_ufshcd_runtime_resume (dev_name (hba->dev),0 ,ktime_to_us (ktime_sub (ktime_get (), start)), hba->curr_dev_pwr_mode, hba->uic_link_state); -return ret; +return 0 ; }EXPORT_SYMBOL (ufshcd_runtime_resume);@@ -9113,18 +9226,13 @@ EXPORT_SYMBOL(ufshcd_runtime_idle);* ufshcd_shutdown - shutdown routine * @hba: per adapter instance * -* This function would power off both UFS device and UFS link. +* This function would turn off both UFS device and UFS hba +* regulators. It would also disable clocks. * * Returns 0 always to allow force shutdown even in case of errors. */ int ufshcd_shutdown (struct ufs_hba *hba) { -int ret =0 ; - -down (&hba->host_sem); - hba->shutting_down = true; -up (&hba->host_sem); -if (!hba->is_powered) goto out;@@ -9133,10 +9241,8 @@ int ufshcd_shutdown(struct ufs_hba *hba)pm_runtime_get_sync (hba->dev); - ret =ufshcd_suspend (hba,UFS_SHUTDOWN_PM ); +ufshcd_suspend (hba); out: -if (ret) -dev_err (hba->dev,"%s failed, err %d\n" , __func__, ret); hba->is_powered = false;/* allow force shutdown even in case of errors */ return 0 ;@@ -9150,6 +9256,8 @@ EXPORT_SYMBOL(ufshcd_shutdown);*/ void ufshcd_remove (struct ufs_hba *hba) { +if (hba->sdev_ufs_device) +scsi_autopm_get_device (hba->sdev_ufs_device);ufs_bsg_remove (hba);ufs_sysfs_remove_nodes (hba->dev);blk_cleanup_queue (hba->tmf_queue);
drivers/scsi/ufs/ufshcd.c int ufshcd_init(struct ufs_hba *hba, void __iomem *mmio_base, unsigned int irq) · line 9568
Quoted 8 lines
drivers/scsi/ufs/ufshcd.c +5
-0@@ -9453,15 +9561,175 @@ int ufshcd_init(struct ufs_hba *hba, void __iomem *mmio_base, unsigned int irq) }EXPORT_SYMBOL_GPL (ufshcd_init); +void ufshcd_resume_complete (struct device *dev) +{ +struct ufs_hba *hba =dev_get_drvdata (dev); + +pm_runtime_put_noidle (&hba->sdev_ufs_device->sdev_gendev);
pm_runtime_put() instead of pm_runtime_put_noidle() would be more correct here.
Quoted 8 lines
drivers/scsi/ufs/ufshcd.c +8
-0+} +EXPORT_SYMBOL_GPL (ufshcd_resume_complete); + +int ufshcd_suspend_prepare (struct device *dev) +{ +struct ufs_hba *hba =dev_get_drvdata (dev); + +/*
drivers/scsi/ufs/ufshcd.c int ufshcd_init(struct ufs_hba *hba, void __iomem *mmio_base, unsigned int irq) · line 9586
Quoted 10 lines
drivers/scsi/ufs/ufshcd.c +10
-0+* SCSI assumes that runtime-pm and system-pm for scsi drivers +* are same. And it doesn't wake up the device for system-suspend +* if it's runtime suspended. But ufs doesn't follow that. +* The rpm-lvl and spm-lvl can be different in ufs. +* Force it to honor system-suspend. +*/ +scsi_autopm_get_device (hba->sdev_ufs_device); +/* Refer ufshcd_resume_complete() */ +pm_runtime_get_noresume (&hba->sdev_ufs_device->sdev_gendev); +scsi_autopm_put_device (hba->sdev_ufs_device);
This could simply be:
pm_runtime_get_sync(&hba->sdev_ufs_device->sdev_gendev);
or new wrapper.
Quoted 129 lines
drivers/scsi/ufs/ufshcd.c +126
-0+return 0 ; +} +EXPORT_SYMBOL_GPL (ufshcd_suspend_prepare); + +#ifdef CONFIG_PM_SLEEP +static int ufshcd_wl_poweroff (struct device *dev) +{ +struct scsi_device *sdev =to_scsi_device (dev); +struct ufs_hba *hba =shost_priv (sdev->host); + +__ufshcd_wl_suspend (hba,UFS_SHUTDOWN_PM ); +return 0 ; +} +#endif + +static int ufshcd_wl_probe (struct device *dev) +{ +struct scsi_device *sdev =to_scsi_device (dev); + +if (!is_device_wlun (sdev)) +return -ENODEV ; + +blk_pm_runtime_init (sdev->request_queue, dev); +pm_runtime_set_autosuspend_delay (dev,0 ); +pm_runtime_allow (dev); + +return 0 ; +} + +static int ufshcd_wl_remove (struct device *dev) +{ +pm_runtime_forbid (dev); +return 0 ; +} + +static const struct dev_pm_ops ufshcd_wl_pm_ops = { +#ifdef CONFIG_PM_SLEEP + .suspend = ufshcd_wl_suspend, + .resume = ufshcd_wl_resume, + .freeze = ufshcd_wl_suspend, + .thaw = ufshcd_wl_resume, + .poweroff = ufshcd_wl_poweroff, + .restore = ufshcd_wl_resume, +#endif +SET_RUNTIME_PM_OPS (ufshcd_wl_runtime_suspend, ufshcd_wl_runtime_resume,NULL ) +}; + +/** +* ufs_dev_wlun_template - describes ufs device wlun +* ufs-device wlun - used to send pm commands +* All luns are consumers of ufs-device wlun. +* +* Currently, no sd driver is present for wluns. +* Hence the no specific pm operations are performed. +* With ufs design, SSU should be sent to ufs-device wlun. +* Hence register a scsi driver for ufs wluns only. +*/ +static struct scsi_driver ufs_dev_wlun_template = { + .gendrv = { + .name ="ufs_device_wlun" , + .owner =THIS_MODULE , + .probe = ufshcd_wl_probe, + .remove = ufshcd_wl_remove, + .pm = &ufshcd_wl_pm_ops, + .shutdown = ufshcd_wl_shutdown, + }, +}; + +static int ufshcd_rpmb_probe (struct device *dev) +{ +return is_rpmb_wlun (to_scsi_device (dev)) ?0 : -ENODEV ; +} + +static inline int ufshcd_clear_rpmb_uac (struct ufs_hba *hba) +{ +int ret =0 ; + +if (!hba->wlun_rpmb_clr_ua) +return 0 ; + ret =ufshcd_clear_ua_wlun (hba,UFS_UPIU_RPMB_WLUN ); +if (!ret) + hba->wlun_rpmb_clr_ua =0 ; +return ret; +} + +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); +return 0 ; +} + +static const struct dev_pm_ops ufs_rpmb_pm_ops = { + SET_RUNTIME_PM_OPS(NULL, ufshcd_rpmb_runtime_resume, NULL) +SET_SYSTEM_SLEEP_PM_OPS (NULL , ufshcd_rpmb_resume) +}; + +/** +* Describes the ufs rpmb wlun. +* Used only to send uac. +*/ +static struct scsi_driver ufs_rpmb_wlun_template = { + .gendrv = { + .name ="ufs_rpmb_wlun" , + .owner =THIS_MODULE , + .probe = ufshcd_rpmb_probe, + .pm = &ufs_rpmb_pm_ops, + }, +}; +static int __init ufshcd_core_init (void ) { +int ret; +ufs_debugfs_init (); + + ret =scsi_register_driver (&ufs_dev_wlun_template.gendrv); +if (ret) {
drivers/scsi/ufs/ufshcd.c int ufshcd_init(struct ufs_hba *hba, void __iomem *mmio_base, unsigned int irq) · line 9725
Quoted 10 lines
drivers/scsi/ufs/ufshcd.c +9
-0+ufs_debugfs_exit (); +return ret; + } + ret =scsi_register_driver (&ufs_rpmb_wlun_template.gendrv); +if (ret) { +ufs_debugfs_exit (); +scsi_unregister_driver (&ufs_dev_wlun_template.gendrv); +return ret; + }return 0 ;
A separated error path is preferred i.e.
ret = scsi_register_driver(&ufs_dev_wlun_template.gendrv); if (ret) goto debugfs_exit; ret = scsi_register_driver(&ufs_rpmb_wlun_template.gendrv); if (ret) goto unregister;
return 0;
unregister:
scsi_unregister_driver(&ufs_dev_wlun_template.gendrv); debugfs_exit: ufs_debugfs_exit(); return ret;
Quoted 80 lines
drivers/scsi/ufs/ufshcd.c +2
-0} static void __exit ufshcd_core_exit(void) {ufs_debugfs_exit (); +scsi_unregister_driver (&ufs_rpmb_wlun_template.gendrv); +scsi_unregister_driver (&ufs_dev_wlun_template.gendrv); }module_init (ufshcd_core_init);drivers/scsi/ufs/ufshcd.h +6
-0@@ -72,6 +72,8 @@ enum ufs_event_type {UFS_EVT_LINK_STARTUP_FAIL ,UFS_EVT_RESUME_ERR ,UFS_EVT_SUSPEND_ERR , +UFS_EVT_WL_SUSP_ERR , +UFS_EVT_WL_RES_ERR ,/* abnormal events */ UFS_EVT_DEV_RESET,@@ -807,6 +809,7 @@ struct ufs_hba {struct list_head clk_list_head;bool wlun_dev_clr_ua; +bool wlun_rpmb_clr_ua;/* Number of requests aborts */ int req_abort_count;@@ -846,6 +849,7 @@ struct ufs_hba {struct delayed_work debugfs_ee_work;u32 debugfs_ee_rate_limit_ms;#endif +u32 luns_avail; };/* Returns true if clocks can be gated. Otherwise false */ @@ -1105,6 +1109,8 @@ int ufshcd_exec_raw_upiu_cmd(struct ufs_hba *hba, enum query_opcode desc_op);int ufshcd_wb_toggle (struct ufs_hba *hba,bool enable); +int ufshcd_suspend_prepare (struct device *dev); +void ufshcd_resume_complete (struct device *dev);/* Wrapper functions for safely calling variant operations */ static inline const char *ufshcd_get_var_name (struct ufs_hba *hba)include/trace/events/ufs.h +20
-0@@ -246,6 +246,26 @@ DEFINE_EVENT(ufshcd_template, ufshcd_init, int dev_state, int link_state), TP_ARGS(dev_name, err, usecs, dev_state, link_state)); +DEFINE_EVENT (ufshcd_template, ufshcd_wl_suspend, + TP_PROTO(const char *dev_name, int err, s64 usecs, + int dev_state, int link_state), +TP_ARGS (dev_name, err, usecs, dev_state, link_state)); + +DEFINE_EVENT (ufshcd_template, ufshcd_wl_resume, + TP_PROTO(const char *dev_name, int err, s64 usecs, + int dev_state, int link_state), +TP_ARGS (dev_name, err, usecs, dev_state, link_state)); + +DEFINE_EVENT (ufshcd_template, ufshcd_wl_runtime_suspend, + TP_PROTO(const char *dev_name, int err, s64 usecs, + int dev_state, int link_state), +TP_ARGS (dev_name, err, usecs, dev_state, link_state)); + +DEFINE_EVENT (ufshcd_template, ufshcd_wl_runtime_resume, + TP_PROTO(const char *dev_name, int err, s64 usecs, + int dev_state, int link_state), +TP_ARGS (dev_name, err, usecs, dev_state, link_state)); + TRACE_EVENT(ufshcd_command, TP_PROTO(const char *dev_name, enum ufs_trace_str_t str_t, unsigned int tag, u32 doorbell, int transfer_len, u32 intr,
PATCH v17 2/2 ufs: sysfs: Resume the proper scsi device
In reply to Asutosh DasResumes the actual scsi device the unit descriptor of which is being accessed instead of the hba alone.
--- drivers/scsi/ufs/ufs-sysfs.c | 30 +++++++++++++++++------------- 1 file changed, 17 insertions(+), 13 deletions(-)
drivers/scsi/ufs/ufs-sysfs.c +17 -13
@@ -245,9 +245,9 @@ static ssize_t wb_on_store(struct device *dev, struct device_attribute *attr, goto out; } -pm_runtime_get_sync (hba->dev); +scsi_autopm_get_device (hba->sdev_ufs_device); res =ufshcd_wb_toggle (hba, wb_enable); -pm_runtime_put_sync (hba->dev); +scsi_autopm_put_device (hba->sdev_ufs_device); out:up (&hba->host_sem);return res <0 ? res : count;
@@ -297,10 +297,10 @@ static ssize_t ufs_sysfs_read_desc_param(struct ufs_hba *hba, goto out; } -pm_runtime_get_sync (hba->dev); +scsi_autopm_get_device (hba->sdev_ufs_device); ret =ufshcd_read_desc_param (hba, desc_id, desc_index, param_offset, desc_buf, param_size); -pm_runtime_put_sync (hba->dev); +scsi_autopm_put_device (hba->sdev_ufs_device);if (ret) { ret = -EINVAL ; goto out;
@@ -678,7 +678,7 @@ static ssize_t _name##_show(struct device *dev, \up (&hba->host_sem); \return -ENOMEM ; \ } \ -pm_runtime_get_sync (hba->dev); \ +scsi_autopm_get_device (hba->sdev_ufs_device); \ ret =ufshcd_query_descriptor_retry (hba, \UPIU_QUERY_OPCODE_READ_DESC ,QUERY_DESC_IDN_DEVICE , \0 ,0 , desc_buf, &desc_len); \
@@ -695,7 +695,7 @@ static ssize_t _name##_show(struct device *dev, \ goto out; \ ret =sysfs_emit (buf,"%s\n" , desc_buf); \ out: \ -pm_runtime_put_sync (hba->dev); \ +scsi_autopm_put_device (hba->sdev_ufs_device); \kfree (desc_buf); \up (&hba->host_sem); \return ret; \
@@ -744,10 +744,10 @@ static ssize_t _name##_show(struct device *dev, \ } \ if (ufshcd_is_wb_flags(QUERY_FLAG_IDN##_uname)) \ index =ufshcd_wb_get_query_index (hba); \ -pm_runtime_get_sync (hba->dev); \ +scsi_autopm_get_device (hba->sdev_ufs_device); \ ret =ufshcd_query_flag (hba,UPIU_QUERY_OPCODE_READ_FLAG , \ QUERY_FLAG_IDN##_uname, index, &flag); \ -pm_runtime_put_sync (hba->dev); \ +scsi_autopm_put_device (hba->sdev_ufs_device); \if (ret) { \ ret = -EINVAL ; \ goto out; \
@@ -813,10 +813,10 @@ static ssize_t _name##_show(struct device *dev, \ } \ if (ufshcd_is_wb_attrs(QUERY_ATTR_IDN##_uname)) \ index =ufshcd_wb_get_query_index (hba); \ -pm_runtime_get_sync (hba->dev); \ +scsi_autopm_get_device (hba->sdev_ufs_device); \ ret =ufshcd_query_attr (hba,UPIU_QUERY_OPCODE_READ_ATTR , \ QUERY_ATTR_IDN##_uname, index, 0, &value); \ -pm_runtime_put_sync (hba->dev); \ +scsi_autopm_put_device (hba->sdev_ufs_device); \if (ret) { \ ret = -EINVAL ; \ goto out; \
@@ -899,11 +899,15 @@ static ssize_t _pname##_show(struct device *dev, \struct scsi_device *sdev =to_scsi_device (dev); \struct ufs_hba *hba =shost_priv (sdev->host); \u8 lun =ufshcd_scsi_to_upiu_lun (sdev->lun); \ +int ret; \if (!ufs_is_valid_unit_desc_lun (&hba->dev_info, lun, \ _duname##_DESC_PARAM##_puname)) \return -EINVAL ; \ - return ufs_sysfs_read_desc_param(hba, QUERY_DESC_IDN_##_duname, \ +scsi_autopm_get_device (sdev); \ + ret = ufs_sysfs_read_desc_param(hba, QUERY_DESC_IDN_##_duname, \ lun, _duname##_DESC_PARAM##_puname, buf, _size); \ +scsi_autopm_put_device (sdev); \ +return ret; \ } \static DEVICE_ATTR_RO (_pname)
@@ -964,10 +968,10 @@ static ssize_t dyn_cap_needed_attribute_show(struct device *dev, goto out; } -pm_runtime_get_sync (hba->dev); +scsi_autopm_get_device (hba->sdev_ufs_device); ret =ufshcd_query_attr (hba,UPIU_QUERY_OPCODE_READ_ATTR ,QUERY_ATTR_IDN_DYN_CAP_NEEDED , lun,0 , &value); -pm_runtime_put_sync (hba->dev); +scsi_autopm_put_device (hba->sdev_ufs_device);if (ret) { ret = -EINVAL ; goto out;
Signature · 1 line
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project.
Re: [PATCH v17 2/2] ufs: sysfs: Resume the proper scsi device
Quoted 13 lines · Asutosh Das
Resumes the actual scsi device the unit descriptor of which is being accessed instead of the hba alone.
Reviewed-by Can Guo <[email protected]>Signed-off-by Asutosh Das <[email protected]>--- drivers/scsi/ufs/ufs-sysfs.c | 30 +++++++++++++++++------------- 1 file changed, 17 insertions(+), 13 deletions(-)drivers/scsi/ufs/ufs-sysfs.c +0
-0
<SNIP>
Quoted 4 lines
drivers/scsi/ufs/ufs-sysfs.c +0
-0@@ -899,11 +899,15 @@ static ssize_t _pname##_show(struct device *dev, \struct scsi_device *sdev =to_scsi_device (dev); \struct ufs_hba *hba =shost_priv (sdev->host); \u8 lun =ufshcd_scsi_to_upiu_lun (sdev->lun); \
drivers/scsi/ufs/ufs-sysfs.c static ssize_t _pname##_show(struct device *dev, \ · line 910
Quoted 10 lines
drivers/scsi/ufs/ufs-sysfs.c +5
-1+int ret; \if (!ufs_is_valid_unit_desc_lun (&hba->dev_info, lun, \ _duname##_DESC_PARAM##_puname)) \return -EINVAL ; \ - return ufs_sysfs_read_desc_param(hba, QUERY_DESC_IDN_##_duname, \ +scsi_autopm_get_device (sdev); \ + ret = ufs_sysfs_read_desc_param(hba, QUERY_DESC_IDN_##_duname, \ lun, _duname##_DESC_PARAM##_puname, buf, _size); \ +scsi_autopm_put_device (sdev); \ +return ret; \
I am not sure why this change is needed. It it is needed, please add a comment explaining.