Enable power management for ufs wlun
Subsystems (5)
v19 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
v18 -> v19:
- Addressed Adrian's comments
- Fixed smatch warnings
v17 -> v18:
- Addressed Adrian's comments
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 | 24 +- drivers/scsi/ufs/ufs_bsg.c | 6 +- drivers/scsi/ufs/ufshcd-pci.c | 36 +- drivers/scsi/ufs/ufshcd.c | 691 ++++++++++++++++++++++++++----------- drivers/scsi/ufs/ufshcd.h | 21 ++ include/trace/events/ufs.h | 20 ++ 14 files changed, 563 insertions(+), 265 deletions(-)
Signature · 1 line
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project.
PATCH v19 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.
This also registers a new scsi driver for rpmb wlun. This new driver is mostly used to clear rpmb uac. With this design, the driver would always be runtime resumed before system suspend.
Fixed smatch warnings:
--- 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 | 691 ++++++++++++++++++++++++++----------- drivers/scsi/ufs/ufshcd.h | 21 ++ include/trace/events/ufs.h | 20 ++ 13 files changed, 551 insertions(+), 253 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); +ufshcd_rpm_get_sync (hba);return 0 ; }static void ufs_debugfs_put_user_access (struct ufs_hba *hba) __releases(&hba->host_sem) { -pm_runtime_put_sync (hba->dev); +ufshcd_rpm_put_sync (hba);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); +ufshcd_rpm_get_sync (hba); 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); +ufshcd_rpm_put_sync (hba); goto out; }
@@ -138,7 +138,7 @@ static int ufs_bsg_request(struct bsg_job *job)break ; } -pm_runtime_put_sync (hba->dev); +ufshcd_rpm_put_sync (hba);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 +482 -209
1,022 lines not on this page · raw message
drivers/scsi/ufs/ufshcd.h +21 -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)
@@ -1309,4 +1315,19 @@ static inline int ufshcd_update_ee_usr_mask(struct ufs_hba *hba, &hba->ee_drv_mask, set, clr); } +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_sync (struct ufs_hba *hba) +{ +return pm_runtime_put_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); +} +#endif /* End of Header */
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.
PATCH v19 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 | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-)
drivers/scsi/ufs/ufs-sysfs.c +12 -12
@@ -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); +ufshcd_rpm_get_sync (hba); res =ufshcd_wb_toggle (hba, wb_enable); -pm_runtime_put_sync (hba->dev); +ufshcd_rpm_put_sync (hba); 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); +ufshcd_rpm_get_sync (hba); ret =ufshcd_read_desc_param (hba, desc_id, desc_index, param_offset, desc_buf, param_size); -pm_runtime_put_sync (hba->dev); +ufshcd_rpm_put_sync (hba);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); \ +ufshcd_rpm_get_sync (hba); \ 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); \ +ufshcd_rpm_put_sync (hba); \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); \ +ufshcd_rpm_get_sync (hba); \ ret =ufshcd_query_flag (hba,UPIU_QUERY_OPCODE_READ_FLAG , \ QUERY_FLAG_IDN##_uname, index, &flag); \ -pm_runtime_put_sync (hba->dev); \ +ufshcd_rpm_put_sync (hba); \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); \ +ufshcd_rpm_get_sync (hba); \ ret =ufshcd_query_attr (hba,UPIU_QUERY_OPCODE_READ_ATTR , \ QUERY_ATTR_IDN##_uname, index, 0, &value); \ -pm_runtime_put_sync (hba->dev); \ +ufshcd_rpm_put_sync (hba); \if (ret) { \ ret = -EINVAL ; \ goto out; \
@@ -964,10 +964,10 @@ static ssize_t dyn_cap_needed_attribute_show(struct device *dev, goto out; } -pm_runtime_get_sync (hba->dev); +ufshcd_rpm_get_sync (hba); 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); +ufshcd_rpm_put_sync (hba);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 v19 1/2] scsi: ufs: Enable power management for wlun
In reply to Asutosh DasQuoted 33 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/0x18
Quoted 9 lines
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.
This also registers a new scsi driver for rpmb wlun. This new driver is mostly used to clear rpmb uac. With this design, the driver would always be runtime resumed before system suspend.
I thought some more about that and I think we can still support allowing runtime suspend to work with system suspend, without too much difficulty. See ufshcd_suspend_prepare() below.
Quoted 9 lines
Fixed smatch warnings:
Reported-by kernel test robot <[email protected]>Reported-by Dan Carpenter <[email protected]>Co-developed-by Can Guo <[email protected]>Signed-off-by Can Guo <[email protected]>Signed-off-by Asutosh Das <[email protected]>---
<SNIP>
Quoted 37 lines
drivers/scsi/ufs/ufshcd.c +4
-18-static int ufshcd_resume (struct ufs_hba *hba,enum ufs_pm_op pm_op) +static int __ufshcd_wl_resume (struct ufs_hba *hba,enum ufs_pm_op pm_op) {int ret; -enum uic_link_state old_link_state; +enum uic_link_state old_link_state = hba->uic_link_state; - hba->pm_op_in_progress =1 ; - old_link_state = hba->uic_link_state; - -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); + hba->pm_op_in_progress = true;/* * Call vendor specific resume callback. As these callbacks may access @@ -8868,7 +8858,7 @@ static int ufshcd_resume(struct ufs_hba *hba, enum ufs_pm_op pm_op)*/ ret =ufshcd_vops_resume (hba, pm_op);if (ret) - goto disable_irq_and_vops_clks; + goto out;/* For DeepSleep, the only supported option is to have the link off */ WARN_ON (ufshcd_is_ufs_dev_deepsleep (hba) && !ufshcd_is_link_off (hba));@@ -8916,42 +8906,219 @@ static int ufshcd_resume(struct ufs_hba *hba, enum ufs_pm_op pm_op)if (hba->ee_usr_mask)
drivers/scsi/ufs/ufshcd.c static int ufshcd_resume(struct ufs_hba *hba, enum ufs_pm_op pm_op) · line 8910
Quoted 10 lines
drivers/scsi/ufs/ufshcd.c +1
-6ufshcd_write_ee_control (hba); - hba->clk_gating.is_suspended = false; -if (ufshcd_is_clkscaling_supported (hba)) -ufshcd_clk_scaling_suspend (hba, false); - -/* Enable Auto-Hibernate if configured */ -ufshcd_auto_hibern8_enable (hba); +ufshcd_resume_clkscaling (hba);
This still doesn't look right. ufshcd_resume_clkscaling() doesn't update hba->clk_scaling.is_allowed whereas ufshcd_clk_scaling_suspend() does.
Quoted 28 lines
drivers/scsi/ufs/ufshcd.c +10
-6if (hba->dev_info.b_rpm_dev_flush_capable) { hba->dev_info.b_rpm_dev_flush_capable = false;cancel_delayed_work (&hba->rpm_dev_flush_recheck_work); } -ufshcd_clear_ua_wluns (hba); - -/* Schedule clock gating in case of no access to UFS device yet */ -ufshcd_release (hba); - +/* Enable Auto-Hibernate if configured */ +ufshcd_auto_hibern8_enable (hba); goto out; set_old_link_state:ufshcd_link_state_transition (hba, old_link_state,0 ); vendor_suspend:ufshcd_vops_suspend (hba, pm_op); -disable_irq_and_vops_clks: +out: +if (ret) +ufshcd_update_evt_hist (hba,UFS_EVT_WL_RES_ERR , (u32 )ret); + hba->clk_gating.is_suspended = false; +ufshcd_release (hba); + hba->pm_op_in_progress = false; +return ret; +}
<SNIP>
Quoted 14 lines
drivers/scsi/ufs/ufshcd.c +14
-0+void ufshcd_resume_complete (struct device *dev) +{ +struct ufs_hba *hba =dev_get_drvdata (dev); + +ufshcd_rpm_put (hba); +} +EXPORT_SYMBOL_GPL (ufshcd_resume_complete); + +int ufshcd_suspend_prepare (struct device *dev) +{ +struct ufs_hba *hba =dev_get_drvdata (dev); + +/* +* SCSI assumes that runtime-pm and system-pm for scsi drivers
drivers/scsi/ufs/ufshcd.c int ufshcd_init(struct ufs_hba *hba, void __iomem *mmio_base, unsigned int irq) · line 9591
Quoted 10 lines
drivers/scsi/ufs/ufshcd.c +10
-0+* 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. +* Refer ufshcd_resume_complete() +*/ +ufshcd_rpm_get_sync (hba); + +return 0 ; +}
I think we can support allowing runtime suspend to work with system suspend. ufshcd_resume_complete() remains the same, and ufshcd_suspend_prepare() is like this:
/*
* 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.
* However UFS doesn't follow that. The rpm-lvl and spm-lvl can be different in
* UFS, so special care is needed.
* Refer also ufshcd_resume_complete()
*/
int ufshcd_suspend_prepare(struct device *dev)
{
struct ufs_hba *hba = dev_get_drvdata(dev);
struct device *ufs_dev = &hba->sdev_ufs_device->sdev_gendev;
enum ufs_dev_pwr_mode spm_pwr_mode;
enum uic_link_state spm_link_state;
unsigned long flags;
bool rpm_state_ok;
/*
* First prevent runtime suspend. Note this does not prevent runtime
* resume e.g. pm_runtime_get_sync() will still do the right thing.
*/
pm_runtime_get_noresume(ufs_dev);
/* Now check if the rpm state is ok to use for spm */
spin_lock_irqsave(&ufs_dev->power.lock, flags);
spm_pwr_mode = ufs_get_pm_lvl_to_dev_pwr_mode(hba->spm_lvl);
spm_link_state = ufs_get_pm_lvl_to_link_pwr_state(hba->spm_lvl);
rpm_state_ok = pm_runtime_suspended(ufs_dev) &&
hba->curr_dev_pwr_mode == spm_pwr_mode &&
hba->uic_link_state == spm_link_state &&
!hba->dev_info.b_rpm_dev_flush_capable;
spin_unlock_irqrestore(&ufs_dev->power.lock, flags);
/* If is isn't, do a runtime resume */
if (!rpm_state_ok)
pm_runtime_resume(ufs_dev);
return 0;
}Re: [PATCH v19 1/2] scsi: ufs: Enable power management for wlun
Quoted 203 lines · Adrian Hunter
Quoted 33 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/0x18Quoted 9 lines
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.
This also registers a new scsi driver for rpmb wlun. This new driver is mostly used to clear rpmb uac. With this design, the driver would always be runtime resumed before system suspend.I thought some more about that and I think we can still support allowing runtime suspend to work with system suspend, without too much difficulty. See
ufshcd_suspend_prepare()below.Quoted 9 lines
Fixed smatch warnings:
Reported-by kernel test robot <[email protected]>Reported-by Dan Carpenter <[email protected]>Co-developed-by Can Guo <[email protected]>Signed-off-by Can Guo <[email protected]>Signed-off-by Asutosh Das <[email protected]>---
<SNIP>
Quoted 37 lines
drivers/scsi/ufs/ufshcd.c +4
-18-static int ufshcd_resume (struct ufs_hba *hba,enum ufs_pm_op pm_op) +static int __ufshcd_wl_resume (struct ufs_hba *hba,enum ufs_pm_op pm_op) {int ret; -enum uic_link_state old_link_state; +enum uic_link_state old_link_state = hba->uic_link_state; - hba->pm_op_in_progress =1 ; - old_link_state = hba->uic_link_state; - -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); + hba->pm_op_in_progress = true;/* * Call vendor specific resume callback. As these callbacks may access @@ -8868,7 +8858,7 @@ static int ufshcd_resume(struct ufs_hba *hba, enum ufs_pm_op pm_op)*/ ret =ufshcd_vops_resume (hba, pm_op);if (ret) - goto disable_irq_and_vops_clks; + goto out;/* For DeepSleep, the only supported option is to have the link off */ WARN_ON (ufshcd_is_ufs_dev_deepsleep (hba) && !ufshcd_is_link_off (hba));@@ -8916,42 +8906,219 @@ static int ufshcd_resume(struct ufs_hba *hba, enum ufs_pm_op pm_op)if (hba->ee_usr_mask)drivers/scsi/ufs/ufshcd.c static int ufshcd_resume(struct ufs_hba *hba, enum ufs_pm_op pm_op) · line 8910
Quoted 10 lines
drivers/scsi/ufs/ufshcd.c +1
-6ufshcd_write_ee_control (hba); - hba->clk_gating.is_suspended = false; -if (ufshcd_is_clkscaling_supported (hba)) -ufshcd_clk_scaling_suspend (hba, false); - -/* Enable Auto-Hibernate if configured */ -ufshcd_auto_hibern8_enable (hba); +ufshcd_resume_clkscaling (hba);This still doesn't look right.
ufshcd_resume_clkscaling()doesn't updatehba->clk_scaling.is_allowedwhereasufshcd_clk_scaling_suspend()does.Quoted 28 lines
drivers/scsi/ufs/ufshcd.c +10
-6if (hba->dev_info.b_rpm_dev_flush_capable) { hba->dev_info.b_rpm_dev_flush_capable = false;cancel_delayed_work (&hba->rpm_dev_flush_recheck_work); } -ufshcd_clear_ua_wluns (hba); - -/* Schedule clock gating in case of no access to UFS device yet */ -ufshcd_release (hba); - +/* Enable Auto-Hibernate if configured */ +ufshcd_auto_hibern8_enable (hba); goto out; set_old_link_state:ufshcd_link_state_transition (hba, old_link_state,0 ); vendor_suspend:ufshcd_vops_suspend (hba, pm_op); -disable_irq_and_vops_clks: +out: +if (ret) +ufshcd_update_evt_hist (hba,UFS_EVT_WL_RES_ERR , (u32 )ret); + hba->clk_gating.is_suspended = false; +ufshcd_release (hba); + hba->pm_op_in_progress = false; +return ret; +}<SNIP>
Quoted 14 lines
drivers/scsi/ufs/ufshcd.c +14
-0+void ufshcd_resume_complete (struct device *dev) +{ +struct ufs_hba *hba =dev_get_drvdata (dev); + +ufshcd_rpm_put (hba); +} +EXPORT_SYMBOL_GPL (ufshcd_resume_complete); + +int ufshcd_suspend_prepare (struct device *dev) +{ +struct ufs_hba *hba =dev_get_drvdata (dev); + +/* +* SCSI assumes that runtime-pm and system-pm for scsi drivers drivers/scsi/ufs/ufshcd.c int ufshcd_init(struct ufs_hba *hba, void __iomem *mmio_base, unsigned int irq) · line 9591
Quoted 10 lines
drivers/scsi/ufs/ufshcd.c +10
-0+* 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. +* Refer ufshcd_resume_complete() +*/ +ufshcd_rpm_get_sync (hba); + +return 0 ; +}I think we can support allowing runtime suspend to work with system suspend.
ufshcd_resume_complete()remains the same, andufshcd_suspend_prepare()is like this:/*
* 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.
* However UFS doesn't follow that. The rpm-lvl and spm-lvl can be different in
* UFS, so special care is needed.
* Refer alsoufshcd_resume_complete()
*/
intufshcd_suspend_prepare(struct device *dev){ struct ufs_hba *hba = dev_get_drvdata(dev); struct device *ufs_dev = &hba->sdev_ufs_device->sdev_gendev; enum ufs_dev_pwr_mode spm_pwr_mode; enum uic_link_state spm_link_state; unsigned long flags; bool rpm_state_ok; /* * First prevent runtime suspend. Note this does not prevent runtime * resume e.g. pm_runtime_get_sync() will still do the right thing. */ pm_runtime_get_noresume(ufs_dev); /* Now check if the rpm state is ok to use for spm */ spin_lock_irqsave(&ufs_dev->power.lock, flags); spm_pwr_mode = ufs_get_pm_lvl_to_dev_pwr_mode(hba->spm_lvl); spm_link_state = ufs_get_pm_lvl_to_link_pwr_state(hba->spm_lvl); rpm_state_ok = pm_runtime_suspended(ufs_dev) && hba->curr_dev_pwr_mode == spm_pwr_mode &&
Quoted 8 lines
hba->uic_link_state == spm_link_state && !hba->dev_info.b_rpm_dev_flush_capable; spin_unlock_irqrestore(&ufs_dev->power.lock, flags); /* If is isn't, do a runtime resume */ if (!rpm_state_ok) pm_runtime_resume(ufs_dev);
But we should return an error if runtime resume
fails.
if (!rpm_state_ok) {
int ret = pm_runtime_resume(ufs_dev);
if (ret < 0 && ret != EACCES) {
pm_runtime_put(ufs_dev);
return ret;
}
}Quoted 4 lines
return 0; }