* [PATCH v20 1/2] scsi: ufs: Enable power management for wlun
@ 2021-04-16 19:49 ` Asutosh Das
0 siblings, 0 replies; 31+ messages in thread
From: Asutosh Das @ 2021-04-16 19:49 UTC (permalink / raw)
To: cang, martin.petersen, adrian.hunter, linux-scsi
Cc: Asutosh Das, linux-arm-msm, Alim Akhtar, Avri Altman,
James E.J. Bottomley, Pedro Sousa, Krzysztof Kozlowski,
Stanley Chu, Andy Gross, Bjorn Andersson, Steven Rostedt,
Ingo Molnar, Matthias Brugger, Lee Jones, Bean Huo, Kiwoong Kim,
Colin Ian King, Wei Yongjun, Yue Hu, Bart van Assche,
Gustavo A. R. Silva, Dinghao Liu, Jaegeuk Kim, Satya Tangirala,
open list, moderated list:ARM/SAMSUNG S3C,
S5P AND EXYNOS ARM ARCHITECTURES, open list:ARM/SAMSUNG S3C,
S5P AND EXYNOS ARM ARCHITECTURES,
moderated list:UNIVERSAL FLASH STORAGE HOST CONTROLLER DRIVER...
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.
Fixed smatch warnings:
Reported-by: kernel test robot <lkp@intel.com>
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
Co-developed-by: Can Guo <cang@codeaurora.org>
Signed-off-by: Can Guo <cang@codeaurora.org>
Signed-off-by: Asutosh Das <asutoshd@codeaurora.org>
---
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 | 719 ++++++++++++++++++++++++++-----------
drivers/scsi/ufs/ufshcd.h | 21 ++
include/trace/events/ufs.h | 20 ++
13 files changed, 580 insertions(+), 252 deletions(-)
diff --git a/drivers/scsi/ufs/cdns-pltfrm.c b/drivers/scsi/ufs/cdns-pltfrm.c
index 13d9204..908ff39 100644
--- a/drivers/scsi/ufs/cdns-pltfrm.c
+++ b/drivers/scsi/ufs/cdns-pltfrm.c
@@ -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 = {
diff --git a/drivers/scsi/ufs/tc-dwc-g210-pci.c b/drivers/scsi/ufs/tc-dwc-g210-pci.c
index 67a6a61..ec4589a 100644
--- a/drivers/scsi/ufs/tc-dwc-g210-pci.c
+++ b/drivers/scsi/ufs/tc-dwc-g210-pci.c
@@ -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[] = {
diff --git a/drivers/scsi/ufs/ufs-debugfs.c b/drivers/scsi/ufs/ufs-debugfs.c
index ced9ef4..4e1ff20 100644
--- a/drivers/scsi/ufs/ufs-debugfs.c
+++ b/drivers/scsi/ufs/ufs-debugfs.c
@@ -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);
}
diff --git a/drivers/scsi/ufs/ufs-debugfs.h b/drivers/scsi/ufs/ufs-debugfs.h
index 3ca29d3..97548a3 100644
--- a/drivers/scsi/ufs/ufs-debugfs.h
+++ b/drivers/scsi/ufs/ufs-debugfs.h
@@ -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);
diff --git a/drivers/scsi/ufs/ufs-exynos.c b/drivers/scsi/ufs/ufs-exynos.c
index 70647ea..8b31efb 100644
--- a/drivers/scsi/ufs/ufs-exynos.c
+++ b/drivers/scsi/ufs/ufs-exynos.c
@@ -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 = {
diff --git a/drivers/scsi/ufs/ufs-hisi.c b/drivers/scsi/ufs/ufs-hisi.c
index 0aa5813..a473349 100644
--- a/drivers/scsi/ufs/ufs-hisi.c
+++ b/drivers/scsi/ufs/ufs-hisi.c
@@ -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 = {
diff --git a/drivers/scsi/ufs/ufs-mediatek.c b/drivers/scsi/ufs/ufs-mediatek.c
index a981f26..4862a7b 100644
--- a/drivers/scsi/ufs/ufs-mediatek.c
+++ b/drivers/scsi/ufs/ufs-mediatek.c
@@ -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 = {
diff --git a/drivers/scsi/ufs/ufs-qcom.c b/drivers/scsi/ufs/ufs-qcom.c
index 9b711d6..6651b87 100644
--- a/drivers/scsi/ufs/ufs-qcom.c
+++ b/drivers/scsi/ufs/ufs-qcom.c
@@ -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 = {
diff --git a/drivers/scsi/ufs/ufs_bsg.c b/drivers/scsi/ufs/ufs_bsg.c
index 5b2bc1a..39bf204 100644
--- a/drivers/scsi/ufs/ufs_bsg.c
+++ b/drivers/scsi/ufs/ufs_bsg.c
@@ -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;
diff --git a/drivers/scsi/ufs/ufshcd-pci.c b/drivers/scsi/ufs/ufshcd-pci.c
index 23ee828..e6c334b 100644
--- a/drivers/scsi/ufs/ufshcd-pci.c
+++ b/drivers/scsi/ufs/ufshcd-pci.c
@@ -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[] = {
diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index 0625da7..eb36f753 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -16,6 +16,7 @@
#include <linux/bitfield.h>
#include <linux/blk-pm.h>
#include <linux/blkdev.h>
+#include <scsi/scsi_driver.h>
#include "ufshcd.h"
#include "ufs_quirks.h"
#include "unipro.h"
@@ -77,6 +78,8 @@
/* Polling time to wait for fDeviceInit */
#define FDEVICEINIT_COMPL_TIMEOUT 1500 /* millisecs */
+#define wlun_dev_to_hba(dv) shost_priv(to_scsi_device(dv)->host)
+
#define ufshcd_toggle_vreg(_dev, _vreg, _on) \
({ \
int _ret; \
@@ -1551,7 +1554,7 @@ static ssize_t ufshcd_clkscale_enable_store(struct device *dev,
if (value == hba->clk_scaling.is_enabled)
goto out;
- pm_runtime_get_sync(hba->dev);
+ ufshcd_rpm_get_sync(hba);
ufshcd_hold(hba, false);
hba->clk_scaling.is_enabled = value;
@@ -1567,7 +1570,7 @@ static ssize_t ufshcd_clkscale_enable_store(struct device *dev,
}
ufshcd_release(hba);
- pm_runtime_put_sync(hba->dev);
+ ufshcd_rpm_put_sync(hba);
out:
up(&hba->host_sem);
return err ? err : count;
@@ -2565,6 +2568,17 @@ static inline u16 ufshcd_upiu_wlun_to_scsi_wlun(u8 upiu_wlun_id)
return (upiu_wlun_id & ~UFS_UPIU_WLUN_ID) | SCSI_W_LUN_BASE;
}
+static inline bool is_rpmb_wlun(struct scsi_device *sdev)
+{
+ return sdev->lun == ufshcd_upiu_wlun_to_scsi_wlun(UFS_UPIU_RPMB_WLUN);
+}
+
+static inline bool is_device_wlun(struct scsi_device *sdev)
+{
+ return sdev->lun ==
+ ufshcd_upiu_wlun_to_scsi_wlun(UFS_UPIU_UFS_DEVICE_WLUN);
+}
+
static void ufshcd_init_lrb(struct ufs_hba *hba, struct ufshcd_lrb *lrb, int i)
{
struct utp_transfer_cmd_desc *cmd_descp = hba->ucdl_base_addr;
@@ -4098,12 +4112,13 @@ void ufshcd_auto_hibern8_update(struct ufs_hba *hba, u32 ahit)
}
spin_unlock_irqrestore(hba->host->host_lock, flags);
- if (update && !pm_runtime_suspended(hba->dev)) {
- pm_runtime_get_sync(hba->dev);
+ if (update &&
+ !pm_runtime_suspended(&hba->sdev_ufs_device->sdev_gendev)) {
+ ufshcd_rpm_get_sync(hba);
ufshcd_hold(hba, false);
ufshcd_auto_hibern8_enable(hba);
ufshcd_release(hba);
- pm_runtime_put(hba->dev);
+ ufshcd_rpm_put_sync(hba);
}
}
EXPORT_SYMBOL_GPL(ufshcd_auto_hibern8_update);
@@ -4801,6 +4816,43 @@ static inline void ufshcd_get_lu_power_on_wp_status(struct ufs_hba *hba,
}
/**
+ * ufshcd_setup_links - associate link b/w device wlun and other luns
+ * @sdev: pointer to SCSI device
+ * @hba: pointer to ufs hba
+ */
+static void ufshcd_setup_links(struct ufs_hba *hba, struct scsi_device *sdev)
+{
+ struct device_link *link;
+
+ /*
+ * device wlun is the supplier & rest of the luns are consumers
+ * This ensures that device wlun suspends after all other luns.
+ */
+ if (hba->sdev_ufs_device) {
+ link = device_link_add(&sdev->sdev_gendev,
+ &hba->sdev_ufs_device->sdev_gendev,
+ DL_FLAG_PM_RUNTIME | DL_FLAG_RPM_ACTIVE);
+ if (!link) {
+ dev_err(&sdev->sdev_gendev, "Failed establishing link - %s\n",
+ dev_name(&hba->sdev_ufs_device->sdev_gendev));
+ return;
+ }
+ hba->luns_avail--;
+ /* Ignore REPORT_LUN wlun probing */
+ if (hba->luns_avail == 1) {
+ ufshcd_rpm_put(hba);
+ return;
+ }
+ } else {
+ /*
+ * Device wlun is probed. The assumption is that WLUNs are
+ * scanned before other LUNs.
+ */
+ hba->luns_avail--;
+ }
+}
+
+/**
* ufshcd_slave_alloc - handle initial SCSI device configurations
* @sdev: pointer to SCSI device
*
@@ -4831,6 +4883,8 @@ static int ufshcd_slave_alloc(struct scsi_device *sdev)
ufshcd_get_lu_power_on_wp_status(hba, sdev);
+ ufshcd_setup_links(hba, sdev);
+
return 0;
}
@@ -4862,8 +4916,13 @@ static int ufshcd_slave_configure(struct scsi_device *sdev)
blk_queue_update_dma_pad(q, PRDT_DATA_BYTE_COUNT_PAD - 1);
if (hba->quirks & UFSHCD_QUIRK_ALIGN_SG_WITH_PAGE_SIZE)
blk_queue_update_dma_alignment(q, PAGE_SIZE - 1);
-
- if (ufshcd_is_rpm_autosuspend_allowed(hba))
+ /*
+ * Block runtime-pm until all consumers are added.
+ * Refer ufshcd_setup_links().
+ */
+ if (is_device_wlun(sdev))
+ pm_runtime_get_noresume(&sdev->sdev_gendev);
+ else if (ufshcd_is_rpm_autosuspend_allowed(hba))
sdev->rpm_autosuspend = 1;
ufshcd_crypto_setup_rq_keyslot_manager(hba, q);
@@ -4979,15 +5038,9 @@ ufshcd_transfer_rsp_status(struct ufs_hba *hba, struct ufshcd_lrb *lrbp)
*/
if (!hba->pm_op_in_progress &&
!ufshcd_eh_in_progress(hba) &&
- ufshcd_is_exception_event(lrbp->ucd_rsp_ptr) &&
- schedule_work(&hba->eeh_work)) {
- /*
- * Prevent suspend once eeh_work is scheduled
- * to avoid deadlock between ufshcd_suspend
- * and exception event handler.
- */
- pm_runtime_get_noresume(hba->dev);
- }
+ ufshcd_is_exception_event(lrbp->ucd_rsp_ptr))
+ /* Flushed in suspend */
+ schedule_work(&hba->eeh_work);
break;
case UPIU_TRANSACTION_REJECT_UPIU:
/* TODO: handle Reject UPIU Response */
@@ -5586,8 +5639,8 @@ static void ufshcd_rpm_dev_flush_recheck_work(struct work_struct *work)
* after a certain delay to recheck the threshold by next runtime
* suspend.
*/
- pm_runtime_get_sync(hba->dev);
- pm_runtime_put_sync(hba->dev);
+ ufshcd_rpm_get_sync(hba);
+ ufshcd_rpm_put_sync(hba);
}
/**
@@ -5604,7 +5657,6 @@ static void ufshcd_exception_event_handler(struct work_struct *work)
u32 status = 0;
hba = container_of(work, struct ufs_hba, eeh_work);
- pm_runtime_get_sync(hba->dev);
ufshcd_scsi_block_requests(hba);
err = ufshcd_get_ee_status(hba, &status);
if (err) {
@@ -5621,14 +5673,6 @@ static void ufshcd_exception_event_handler(struct work_struct *work)
ufs_debugfs_exception_event(hba, status);
out:
ufshcd_scsi_unblock_requests(hba);
- /*
- * pm_runtime_get_noresume is called while scheduling
- * eeh_work to avoid suspend racing with exception work.
- * Hence decrement usage counter using pm_runtime_put_noidle
- * to allow suspend on completion of exception event handler.
- */
- pm_runtime_put_noidle(hba->dev);
- pm_runtime_put(hba->dev);
return;
}
@@ -5753,12 +5797,13 @@ static void ufshcd_clk_scaling_suspend(struct ufs_hba *hba, bool suspend)
static void ufshcd_err_handling_prepare(struct ufs_hba *hba)
{
- pm_runtime_get_sync(hba->dev);
- if (pm_runtime_status_suspended(hba->dev) || hba->is_sys_suspended) {
+ ufshcd_rpm_get_sync(hba);
+ if (pm_runtime_status_suspended(&hba->sdev_ufs_device->sdev_gendev) ||
+ hba->is_sys_suspended) {
enum ufs_pm_op pm_op;
/*
- * Don't assume anything of pm_runtime_get_sync(), if
+ * Don't assume anything of resume, if
* resume fails, irq and clocks can be OFF, and powers
* can be OFF or in LPM.
*/
@@ -5794,7 +5839,7 @@ static void ufshcd_err_handling_unprepare(struct ufs_hba *hba)
if (ufshcd_is_clkscaling_supported(hba))
ufshcd_clk_scaling_suspend(hba, false);
ufshcd_clear_ua_wluns(hba);
- pm_runtime_put(hba->dev);
+ ufshcd_rpm_put(hba);
}
static inline bool ufshcd_err_handling_should_stop(struct ufs_hba *hba)
@@ -5815,14 +5860,18 @@ 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);
+
+ /* hba device might have a runtime error otherwise */
+ if (ret)
+ ret = pm_runtime_set_active(hba->dev);
/*
- * If hba device had runtime error, we also need to resume those
- * scsi devices under hba in case any of them has failed to be
- * resumed due to hba runtime resume failure. This is to unblock
+ * If wlun device had runtime error, we also need to resume those
+ * consumer scsi devices in case any of them has failed to be
+ * resumed due to supplier runtime resume failure. This is to unblock
* blk_queue_enter in case there are bios waiting inside it.
*/
if (!ret) {
@@ -7246,7 +7295,6 @@ static int ufshcd_scsi_add_wlus(struct ufs_hba *hba)
hba->sdev_ufs_device = NULL;
goto out;
}
- ufshcd_blk_pm_runtime_init(hba->sdev_ufs_device);
scsi_device_put(hba->sdev_ufs_device);
hba->sdev_rpmb = __scsi_add_device(hba->host, 0, 0,
@@ -7410,6 +7458,9 @@ static int ufs_get_device_desc(struct ufs_hba *hba)
goto out;
}
+ hba->luns_avail = desc_buf[DEVICE_DESC_PARAM_NUM_LU] +
+ desc_buf[DEVICE_DESC_PARAM_NUM_WLU];
+
ufs_fixup_device_setup(hba);
ufshcd_wb_probe(hba, desc_buf);
@@ -7887,6 +7938,7 @@ static int ufshcd_probe_hba(struct ufs_hba *hba, bool async)
ufshcd_set_ufs_dev_active(hba);
ufshcd_force_reset_auto_bkops(hba);
hba->wlun_dev_clr_ua = true;
+ hba->wlun_rpmb_clr_ua = true;
/* Gear up to HS gear if supported */
if (hba->max_pwr_info.is_valid) {
@@ -8472,7 +8524,8 @@ static int ufshcd_set_dev_pwr_mode(struct ufs_hba *hba,
* handling context.
*/
hba->host->eh_noresume = 1;
- ufshcd_clear_ua_wluns(hba);
+ if (hba->wlun_dev_clr_ua)
+ ufshcd_clear_ua_wlun(hba, UFS_UPIU_UFS_DEVICE_WLUN);
cmd[4] = pwr_mode << 4;
@@ -8647,23 +8700,7 @@ static void ufshcd_hba_vreg_set_hpm(struct ufs_hba *hba)
ufshcd_setup_hba_vreg(hba, true);
}
-/**
- * ufshcd_suspend - helper function for suspend operations
- * @hba: per adapter instance
- * @pm_op: desired low power operation type
- *
- * This function will try to put the UFS device and link into low power
- * mode based on the "rpm_lvl" (Runtime PM level) or "spm_lvl"
- * (System PM level).
- *
- * If this function is called during shutdown, it will make sure that
- * both UFS device and UFS link is powered off.
- *
- * NOTE: UFS device & link must be active before we enter in this function.
- *
- * Returns 0 for success and non-zero for failure
- */
-static int ufshcd_suspend(struct ufs_hba *hba, enum ufs_pm_op pm_op)
+static int __ufshcd_wl_suspend(struct ufs_hba *hba, enum ufs_pm_op pm_op)
{
int ret = 0;
int check_for_bkops;
@@ -8671,7 +8708,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;
@@ -8694,17 +8731,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 vops_suspend;
}
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;
}
if (ufshcd_is_runtime_pm(pm_op)) {
@@ -8716,7 +8753,7 @@ static int ufshcd_suspend(struct ufs_hba *hba, enum ufs_pm_op pm_op)
*/
ret = ufshcd_urgent_bkops(hba);
if (ret)
- goto enable_gating;
+ goto enable_scaling;
} else {
/* make sure that auto bkops is disabled */
ufshcd_disable_auto_bkops(hba);
@@ -8744,7 +8781,7 @@ static int ufshcd_suspend(struct ufs_hba *hba, enum ufs_pm_op pm_op)
if (!hba->dev_info.b_rpm_dev_flush_capable) {
ret = ufshcd_set_dev_pwr_mode(hba, req_dev_pwr_mode);
if (ret)
- goto enable_gating;
+ goto enable_scaling;
}
}
@@ -8757,7 +8794,7 @@ static int ufshcd_suspend(struct ufs_hba *hba, enum ufs_pm_op pm_op)
if (ret)
goto set_dev_active;
-disable_clks:
+vops_suspend:
/*
* Call vendor specific suspend callback. As these callbacks may access
* vendor specific host controller register space call them before the
@@ -8766,28 +8803,9 @@ static int ufshcd_suspend(struct ufs_hba *hba, enum ufs_pm_op pm_op)
ret = ufshcd_vops_suspend(hba, pm_op);
if (ret)
goto set_link_active;
- /*
- * 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)) {
- hba->clk_gating.state = CLKS_OFF;
- 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);
goto out;
set_link_active:
- ufshcd_vreg_set_hpm(hba);
/*
* Device hardware reset is required to exit DeepSleep. Also, for
* DeepSleep, the link is off so host reset and restore will be done
@@ -8809,57 +8827,32 @@ static int ufshcd_suspend(struct ufs_hba *hba, enum ufs_pm_op pm_op)
}
if (!ufshcd_set_dev_pwr_mode(hba, UFS_ACTIVE_PWR_MODE))
ufshcd_disable_auto_bkops(hba);
-enable_gating:
+enable_scaling:
if (ufshcd_is_clkscaling_supported(hba))
ufshcd_clk_scaling_suspend(hba, false);
- hba->clk_gating.is_suspended = false;
hba->dev_info.b_rpm_dev_flush_capable = false;
- ufshcd_clear_ua_wluns(hba);
- ufshcd_release(hba);
out:
if (hba->dev_info.b_rpm_dev_flush_capable) {
schedule_delayed_work(&hba->rpm_dev_flush_recheck_work,
msecs_to_jiffies(RPM_DEV_FLUSH_RECHECK_WORK_DELAY_MS));
}
- hba->pm_op_in_progress = 0;
-
- if (ret)
- ufshcd_update_evt_hist(hba, UFS_EVT_SUSPEND_ERR, (u32)ret);
+ if (ret) {
+ ufshcd_update_evt_hist(hba, UFS_EVT_WL_SUSP_ERR, (u32)ret);
+ hba->clk_gating.is_suspended = false;
+ ufshcd_release(hba);
+ }
+ hba->pm_op_in_progress = false;
return ret;
}
-/**
- * ufshcd_resume - helper function for resume operations
- * @hba: per adapter instance
- * @pm_op: runtime PM or system PM
- *
- * This function basically brings the UFS device, UniPro link and controller
- * to active state.
- *
- * Returns 0 for success and non-zero for failure
- */
-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;
-
- 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;
+ enum uic_link_state old_link_state = hba->uic_link_state;
- /* 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 +8861,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 +8909,219 @@ static int ufshcd_resume(struct ufs_hba *hba, enum ufs_pm_op pm_op)
if (hba->ee_usr_mask)
ufshcd_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);
-
if (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;
+}
+
+static int ufshcd_wl_runtime_suspend(struct device *dev)
+{
+ struct scsi_device *sdev = to_scsi_device(dev);
+ struct ufs_hba *hba;
+ int ret;
+ ktime_t start = ktime_get();
+
+ hba = shost_priv(sdev->host);
+
+ ret = __ufshcd_wl_suspend(hba, UFS_RUNTIME_PM);
+ if (ret)
+ dev_err(&sdev->sdev_gendev, "%s failed: %d\n", __func__, ret);
+
+ trace_ufshcd_wl_runtime_suspend(dev_name(dev), ret,
+ ktime_to_us(ktime_sub(ktime_get(), start)),
+ hba->curr_dev_pwr_mode, hba->uic_link_state);
+
+ return ret;
+}
+
+static int ufshcd_wl_runtime_resume(struct device *dev)
+{
+ struct scsi_device *sdev = to_scsi_device(dev);
+ struct ufs_hba *hba;
+ int ret = 0;
+ ktime_t start = ktime_get();
+
+ hba = shost_priv(sdev->host);
+
+ ret = __ufshcd_wl_resume(hba, UFS_RUNTIME_PM);
+ if (ret)
+ dev_err(&sdev->sdev_gendev, "%s failed: %d\n", __func__, ret);
+
+ trace_ufshcd_wl_runtime_resume(dev_name(dev), ret,
+ ktime_to_us(ktime_sub(ktime_get(), start)),
+ hba->curr_dev_pwr_mode, hba->uic_link_state);
+
+ return ret;
+}
+
+#ifdef CONFIG_PM_SLEEP
+static int ufshcd_wl_suspend(struct device *dev)
+{
+ struct scsi_device *sdev = to_scsi_device(dev);
+ struct ufs_hba *hba;
+ int ret = 0;
+ ktime_t start = ktime_get();
+
+ hba = shost_priv(sdev->host);
+ down(&hba->host_sem);
+
+ if (pm_runtime_suspended(dev))
+ goto out;
+
+ ret = __ufshcd_wl_suspend(hba, UFS_SYSTEM_PM);
+ if (ret) {
+ dev_err(&sdev->sdev_gendev, "%s failed: %d\n", __func__, ret);
+ up(&hba->host_sem);
+ }
+
+out:
+ if (!ret)
+ hba->is_sys_suspended = true;
+ trace_ufshcd_wl_suspend(dev_name(dev), ret,
+ ktime_to_us(ktime_sub(ktime_get(), start)),
+ hba->curr_dev_pwr_mode, hba->uic_link_state);
+
+ return ret;
+}
+
+static int ufshcd_wl_resume(struct device *dev)
+{
+ struct scsi_device *sdev = to_scsi_device(dev);
+ struct ufs_hba *hba;
+ int ret = 0;
+ ktime_t start = ktime_get();
+
+ hba = shost_priv(sdev->host);
+
+ if (pm_runtime_suspended(dev))
+ goto out;
+
+ ret = __ufshcd_wl_resume(hba, UFS_SYSTEM_PM);
+ if (ret)
+ dev_err(&sdev->sdev_gendev, "%s failed: %d\n", __func__, ret);
+out:
+ trace_ufshcd_wl_resume(dev_name(dev), ret,
+ ktime_to_us(ktime_sub(ktime_get(), start)),
+ hba->curr_dev_pwr_mode, hba->uic_link_state);
+ if (!ret)
+ hba->is_sys_suspended = false;
+ up(&hba->host_sem);
+ return ret;
+}
+#endif
+
+static void ufshcd_wl_shutdown(struct device *dev)
+{
+ struct scsi_device *sdev = to_scsi_device(dev);
+ struct ufs_hba *hba;
+
+ hba = shost_priv(sdev->host);
+
+ down(&hba->host_sem);
+ hba->shutting_down = true;
+ up(&hba->host_sem);
+
+ /* Turn on everything while shutting down */
+ ufshcd_rpm_get_sync(hba);
+ scsi_device_quiesce(sdev);
+ shost_for_each_device(sdev, hba->host) {
+ if (sdev == hba->sdev_ufs_device)
+ continue;
+ scsi_device_quiesce(sdev);
+ }
+ __ufshcd_wl_suspend(hba, UFS_SHUTDOWN_PM);
+}
+
+/**
+ * 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 int ufshcd_suspend(struct ufs_hba *hba)
+{
+ int ret;
+
+ if (!hba->is_powered)
+ return 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);
+ ret = ufshcd_setup_clocks(hba, false);
+ if (ret) {
+ ufshcd_enable_irq(hba);
+ return ret;
+ }
if (ufshcd_is_clkgating_allowed(hba)) {
hba->clk_gating.state = CLKS_OFF;
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);
+ return ret;
+}
+
+/**
+ * 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;
+
+ if (!hba->is_powered)
+ return 0;
+
+ 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:
- hba->pm_op_in_progress = 0;
if (ret)
ufshcd_update_evt_hist(hba, UFS_EVT_RESUME_ERR, (u32)ret);
return ret;
@@ -8962,6 +9132,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
*/
@@ -8970,41 +9141,14 @@ int ufshcd_system_suspend(struct ufs_hba *hba)
int ret = 0;
ktime_t start = ktime_get();
- down(&hba->host_sem);
-
- if (!hba->is_powered)
- return 0;
-
- 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)
+ if (pm_runtime_suspended(hba->dev))
goto out;
- if (pm_runtime_suspended(hba->dev)) {
- /*
- * UFS device and/or UFS link low power states during runtime
- * 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;
- }
-
- ret = ufshcd_suspend(hba, UFS_SYSTEM_PM);
+ ret = ufshcd_suspend(hba);
out:
trace_ufshcd_system_suspend(dev_name(hba->dev), ret,
ktime_to_us(ktime_sub(ktime_get(), start)),
hba->curr_dev_pwr_mode, hba->uic_link_state);
- if (!ret)
- hba->is_sys_suspended = true;
- else
- up(&hba->host_sem);
return ret;
}
EXPORT_SYMBOL(ufshcd_system_suspend);
@@ -9021,21 +9165,16 @@ int ufshcd_system_resume(struct ufs_hba *hba)
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 (pm_runtime_suspended(hba->dev))
goto out;
- else
- ret = ufshcd_resume(hba, UFS_SYSTEM_PM);
+
+ ret = ufshcd_resume(hba);
+
out:
trace_ufshcd_system_resume(dev_name(hba->dev), ret,
ktime_to_us(ktime_sub(ktime_get(), start)),
hba->curr_dev_pwr_mode, hba->uic_link_state);
- if (!ret)
- hba->is_sys_suspended = false;
- up(&hba->host_sem);
+
return ret;
}
EXPORT_SYMBOL(ufshcd_system_resume);
@@ -9045,19 +9184,17 @@ 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;
+ int ret;
ktime_t start = ktime_get();
- if (!hba->is_powered)
- goto out;
- else
- ret = ufshcd_suspend(hba, UFS_RUNTIME_PM);
-out:
+ ret = ufshcd_suspend(hba);
+
trace_ufshcd_runtime_suspend(dev_name(hba->dev), ret,
ktime_to_us(ktime_sub(ktime_get(), start)),
hba->curr_dev_pwr_mode, hba->uic_link_state);
@@ -9069,33 +9206,19 @@ 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;
+ int ret;
ktime_t start = ktime_get();
- if (!hba->is_powered)
- goto out;
- else
- ret = ufshcd_resume(hba, UFS_RUNTIME_PM);
-out:
+ ret = ufshcd_resume(hba);
+
trace_ufshcd_runtime_resume(dev_name(hba->dev), ret,
ktime_to_us(ktime_sub(ktime_get(), start)),
hba->curr_dev_pwr_mode, hba->uic_link_state);
@@ -9113,30 +9236,20 @@ 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;
-
if (ufshcd_is_ufs_dev_poweroff(hba) && ufshcd_is_link_off(hba))
goto out;
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 +9263,8 @@ EXPORT_SYMBOL(ufshcd_shutdown);
*/
void ufshcd_remove(struct ufs_hba *hba)
{
+ if (hba->sdev_ufs_device)
+ ufshcd_rpm_get_sync(hba);
ufs_bsg_remove(hba);
ufs_sysfs_remove_nodes(hba->dev);
blk_cleanup_queue(hba->tmf_queue);
@@ -9453,15 +9568,203 @@ 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);
+
+ 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);
+ 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;
+
+ /*
+ * 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.
+ * However, if the current_{pwr_mode, link_state} is same as the
+ * desired_{pwr_mode, link_state}, there's no need to rpm resume
+ * the device.
+ * Refer ufshcd_resume_complete()
+ */
+ pm_runtime_get_noresume(ufs_dev);
+
+ 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 (!rpm_state_ok) {
+ int ret = pm_runtime_resume(ufs_dev);
+
+ if (ret < 0 && ret != -EACCES) {
+ pm_runtime_put(ufs_dev);
+ return ret;
+ }
+ }
+ 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();
- return 0;
+
+ 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 ret;
+unregister:
+ scsi_unregister_driver(&ufs_dev_wlun_template.gendrv);
+debugfs_exit:
+ ufs_debugfs_exit();
+ return ret;
}
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);
diff --git a/drivers/scsi/ufs/ufshcd.h b/drivers/scsi/ufs/ufshcd.h
index 5eb66a8..3a8dc0d 100644
--- a/drivers/scsi/ufs/ufshcd.h
+++ b/drivers/scsi/ufs/ufshcd.h
@@ -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 */
diff --git a/include/trace/events/ufs.h b/include/trace/events/ufs.h
index 1cb6f1a..599739e 100644
--- a/include/trace/events/ufs.h
+++ b/include/trace/events/ufs.h
@@ -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,
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 31+ messages in thread* [PATCH v20 1/2] scsi: ufs: Enable power management for wlun
@ 2021-04-16 19:49 ` Asutosh Das
0 siblings, 0 replies; 31+ messages in thread
From: Asutosh Das @ 2021-04-16 19:49 UTC (permalink / raw)
To: cang, martin.petersen, adrian.hunter, linux-scsi
Cc: Asutosh Das, linux-arm-msm, Alim Akhtar, Avri Altman,
James E.J. Bottomley, Pedro Sousa, Krzysztof Kozlowski,
Stanley Chu, Andy Gross, Bjorn Andersson, Steven Rostedt,
Ingo Molnar, Matthias Brugger, Lee Jones, Bean Huo, Kiwoong Kim,
Colin Ian King, Wei Yongjun, Yue Hu, Bart van Assche,
Gustavo A. R. Silva, Dinghao Liu, Jaegeuk Kim, Satya Tangirala,
open list, moderated list:ARM/SAMSUNG S3C,
S5P AND EXYNOS ARM ARCHITECTURES, open list:ARM/SAMSUNG S3C,
S5P AND EXYNOS ARM ARCHITECTURES,
moderated list:UNIVERSAL FLASH STORAGE HOST CONTROLLER DRIVER...
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.
Fixed smatch warnings:
Reported-by: kernel test robot <lkp@intel.com>
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
Co-developed-by: Can Guo <cang@codeaurora.org>
Signed-off-by: Can Guo <cang@codeaurora.org>
Signed-off-by: Asutosh Das <asutoshd@codeaurora.org>
---
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 | 719 ++++++++++++++++++++++++++-----------
drivers/scsi/ufs/ufshcd.h | 21 ++
include/trace/events/ufs.h | 20 ++
13 files changed, 580 insertions(+), 252 deletions(-)
diff --git a/drivers/scsi/ufs/cdns-pltfrm.c b/drivers/scsi/ufs/cdns-pltfrm.c
index 13d9204..908ff39 100644
--- a/drivers/scsi/ufs/cdns-pltfrm.c
+++ b/drivers/scsi/ufs/cdns-pltfrm.c
@@ -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 = {
diff --git a/drivers/scsi/ufs/tc-dwc-g210-pci.c b/drivers/scsi/ufs/tc-dwc-g210-pci.c
index 67a6a61..ec4589a 100644
--- a/drivers/scsi/ufs/tc-dwc-g210-pci.c
+++ b/drivers/scsi/ufs/tc-dwc-g210-pci.c
@@ -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[] = {
diff --git a/drivers/scsi/ufs/ufs-debugfs.c b/drivers/scsi/ufs/ufs-debugfs.c
index ced9ef4..4e1ff20 100644
--- a/drivers/scsi/ufs/ufs-debugfs.c
+++ b/drivers/scsi/ufs/ufs-debugfs.c
@@ -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);
}
diff --git a/drivers/scsi/ufs/ufs-debugfs.h b/drivers/scsi/ufs/ufs-debugfs.h
index 3ca29d3..97548a3 100644
--- a/drivers/scsi/ufs/ufs-debugfs.h
+++ b/drivers/scsi/ufs/ufs-debugfs.h
@@ -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);
diff --git a/drivers/scsi/ufs/ufs-exynos.c b/drivers/scsi/ufs/ufs-exynos.c
index 70647ea..8b31efb 100644
--- a/drivers/scsi/ufs/ufs-exynos.c
+++ b/drivers/scsi/ufs/ufs-exynos.c
@@ -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 = {
diff --git a/drivers/scsi/ufs/ufs-hisi.c b/drivers/scsi/ufs/ufs-hisi.c
index 0aa5813..a473349 100644
--- a/drivers/scsi/ufs/ufs-hisi.c
+++ b/drivers/scsi/ufs/ufs-hisi.c
@@ -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 = {
diff --git a/drivers/scsi/ufs/ufs-mediatek.c b/drivers/scsi/ufs/ufs-mediatek.c
index a981f26..4862a7b 100644
--- a/drivers/scsi/ufs/ufs-mediatek.c
+++ b/drivers/scsi/ufs/ufs-mediatek.c
@@ -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 = {
diff --git a/drivers/scsi/ufs/ufs-qcom.c b/drivers/scsi/ufs/ufs-qcom.c
index 9b711d6..6651b87 100644
--- a/drivers/scsi/ufs/ufs-qcom.c
+++ b/drivers/scsi/ufs/ufs-qcom.c
@@ -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 = {
diff --git a/drivers/scsi/ufs/ufs_bsg.c b/drivers/scsi/ufs/ufs_bsg.c
index 5b2bc1a..39bf204 100644
--- a/drivers/scsi/ufs/ufs_bsg.c
+++ b/drivers/scsi/ufs/ufs_bsg.c
@@ -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;
diff --git a/drivers/scsi/ufs/ufshcd-pci.c b/drivers/scsi/ufs/ufshcd-pci.c
index 23ee828..e6c334b 100644
--- a/drivers/scsi/ufs/ufshcd-pci.c
+++ b/drivers/scsi/ufs/ufshcd-pci.c
@@ -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[] = {
diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index 0625da7..eb36f753 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -16,6 +16,7 @@
#include <linux/bitfield.h>
#include <linux/blk-pm.h>
#include <linux/blkdev.h>
+#include <scsi/scsi_driver.h>
#include "ufshcd.h"
#include "ufs_quirks.h"
#include "unipro.h"
@@ -77,6 +78,8 @@
/* Polling time to wait for fDeviceInit */
#define FDEVICEINIT_COMPL_TIMEOUT 1500 /* millisecs */
+#define wlun_dev_to_hba(dv) shost_priv(to_scsi_device(dv)->host)
+
#define ufshcd_toggle_vreg(_dev, _vreg, _on) \
({ \
int _ret; \
@@ -1551,7 +1554,7 @@ static ssize_t ufshcd_clkscale_enable_store(struct device *dev,
if (value == hba->clk_scaling.is_enabled)
goto out;
- pm_runtime_get_sync(hba->dev);
+ ufshcd_rpm_get_sync(hba);
ufshcd_hold(hba, false);
hba->clk_scaling.is_enabled = value;
@@ -1567,7 +1570,7 @@ static ssize_t ufshcd_clkscale_enable_store(struct device *dev,
}
ufshcd_release(hba);
- pm_runtime_put_sync(hba->dev);
+ ufshcd_rpm_put_sync(hba);
out:
up(&hba->host_sem);
return err ? err : count;
@@ -2565,6 +2568,17 @@ static inline u16 ufshcd_upiu_wlun_to_scsi_wlun(u8 upiu_wlun_id)
return (upiu_wlun_id & ~UFS_UPIU_WLUN_ID) | SCSI_W_LUN_BASE;
}
+static inline bool is_rpmb_wlun(struct scsi_device *sdev)
+{
+ return sdev->lun == ufshcd_upiu_wlun_to_scsi_wlun(UFS_UPIU_RPMB_WLUN);
+}
+
+static inline bool is_device_wlun(struct scsi_device *sdev)
+{
+ return sdev->lun ==
+ ufshcd_upiu_wlun_to_scsi_wlun(UFS_UPIU_UFS_DEVICE_WLUN);
+}
+
static void ufshcd_init_lrb(struct ufs_hba *hba, struct ufshcd_lrb *lrb, int i)
{
struct utp_transfer_cmd_desc *cmd_descp = hba->ucdl_base_addr;
@@ -4098,12 +4112,13 @@ void ufshcd_auto_hibern8_update(struct ufs_hba *hba, u32 ahit)
}
spin_unlock_irqrestore(hba->host->host_lock, flags);
- if (update && !pm_runtime_suspended(hba->dev)) {
- pm_runtime_get_sync(hba->dev);
+ if (update &&
+ !pm_runtime_suspended(&hba->sdev_ufs_device->sdev_gendev)) {
+ ufshcd_rpm_get_sync(hba);
ufshcd_hold(hba, false);
ufshcd_auto_hibern8_enable(hba);
ufshcd_release(hba);
- pm_runtime_put(hba->dev);
+ ufshcd_rpm_put_sync(hba);
}
}
EXPORT_SYMBOL_GPL(ufshcd_auto_hibern8_update);
@@ -4801,6 +4816,43 @@ static inline void ufshcd_get_lu_power_on_wp_status(struct ufs_hba *hba,
}
/**
+ * ufshcd_setup_links - associate link b/w device wlun and other luns
+ * @sdev: pointer to SCSI device
+ * @hba: pointer to ufs hba
+ */
+static void ufshcd_setup_links(struct ufs_hba *hba, struct scsi_device *sdev)
+{
+ struct device_link *link;
+
+ /*
+ * device wlun is the supplier & rest of the luns are consumers
+ * This ensures that device wlun suspends after all other luns.
+ */
+ if (hba->sdev_ufs_device) {
+ link = device_link_add(&sdev->sdev_gendev,
+ &hba->sdev_ufs_device->sdev_gendev,
+ DL_FLAG_PM_RUNTIME | DL_FLAG_RPM_ACTIVE);
+ if (!link) {
+ dev_err(&sdev->sdev_gendev, "Failed establishing link - %s\n",
+ dev_name(&hba->sdev_ufs_device->sdev_gendev));
+ return;
+ }
+ hba->luns_avail--;
+ /* Ignore REPORT_LUN wlun probing */
+ if (hba->luns_avail == 1) {
+ ufshcd_rpm_put(hba);
+ return;
+ }
+ } else {
+ /*
+ * Device wlun is probed. The assumption is that WLUNs are
+ * scanned before other LUNs.
+ */
+ hba->luns_avail--;
+ }
+}
+
+/**
* ufshcd_slave_alloc - handle initial SCSI device configurations
* @sdev: pointer to SCSI device
*
@@ -4831,6 +4883,8 @@ static int ufshcd_slave_alloc(struct scsi_device *sdev)
ufshcd_get_lu_power_on_wp_status(hba, sdev);
+ ufshcd_setup_links(hba, sdev);
+
return 0;
}
@@ -4862,8 +4916,13 @@ static int ufshcd_slave_configure(struct scsi_device *sdev)
blk_queue_update_dma_pad(q, PRDT_DATA_BYTE_COUNT_PAD - 1);
if (hba->quirks & UFSHCD_QUIRK_ALIGN_SG_WITH_PAGE_SIZE)
blk_queue_update_dma_alignment(q, PAGE_SIZE - 1);
-
- if (ufshcd_is_rpm_autosuspend_allowed(hba))
+ /*
+ * Block runtime-pm until all consumers are added.
+ * Refer ufshcd_setup_links().
+ */
+ if (is_device_wlun(sdev))
+ pm_runtime_get_noresume(&sdev->sdev_gendev);
+ else if (ufshcd_is_rpm_autosuspend_allowed(hba))
sdev->rpm_autosuspend = 1;
ufshcd_crypto_setup_rq_keyslot_manager(hba, q);
@@ -4979,15 +5038,9 @@ ufshcd_transfer_rsp_status(struct ufs_hba *hba, struct ufshcd_lrb *lrbp)
*/
if (!hba->pm_op_in_progress &&
!ufshcd_eh_in_progress(hba) &&
- ufshcd_is_exception_event(lrbp->ucd_rsp_ptr) &&
- schedule_work(&hba->eeh_work)) {
- /*
- * Prevent suspend once eeh_work is scheduled
- * to avoid deadlock between ufshcd_suspend
- * and exception event handler.
- */
- pm_runtime_get_noresume(hba->dev);
- }
+ ufshcd_is_exception_event(lrbp->ucd_rsp_ptr))
+ /* Flushed in suspend */
+ schedule_work(&hba->eeh_work);
break;
case UPIU_TRANSACTION_REJECT_UPIU:
/* TODO: handle Reject UPIU Response */
@@ -5586,8 +5639,8 @@ static void ufshcd_rpm_dev_flush_recheck_work(struct work_struct *work)
* after a certain delay to recheck the threshold by next runtime
* suspend.
*/
- pm_runtime_get_sync(hba->dev);
- pm_runtime_put_sync(hba->dev);
+ ufshcd_rpm_get_sync(hba);
+ ufshcd_rpm_put_sync(hba);
}
/**
@@ -5604,7 +5657,6 @@ static void ufshcd_exception_event_handler(struct work_struct *work)
u32 status = 0;
hba = container_of(work, struct ufs_hba, eeh_work);
- pm_runtime_get_sync(hba->dev);
ufshcd_scsi_block_requests(hba);
err = ufshcd_get_ee_status(hba, &status);
if (err) {
@@ -5621,14 +5673,6 @@ static void ufshcd_exception_event_handler(struct work_struct *work)
ufs_debugfs_exception_event(hba, status);
out:
ufshcd_scsi_unblock_requests(hba);
- /*
- * pm_runtime_get_noresume is called while scheduling
- * eeh_work to avoid suspend racing with exception work.
- * Hence decrement usage counter using pm_runtime_put_noidle
- * to allow suspend on completion of exception event handler.
- */
- pm_runtime_put_noidle(hba->dev);
- pm_runtime_put(hba->dev);
return;
}
@@ -5753,12 +5797,13 @@ static void ufshcd_clk_scaling_suspend(struct ufs_hba *hba, bool suspend)
static void ufshcd_err_handling_prepare(struct ufs_hba *hba)
{
- pm_runtime_get_sync(hba->dev);
- if (pm_runtime_status_suspended(hba->dev) || hba->is_sys_suspended) {
+ ufshcd_rpm_get_sync(hba);
+ if (pm_runtime_status_suspended(&hba->sdev_ufs_device->sdev_gendev) ||
+ hba->is_sys_suspended) {
enum ufs_pm_op pm_op;
/*
- * Don't assume anything of pm_runtime_get_sync(), if
+ * Don't assume anything of resume, if
* resume fails, irq and clocks can be OFF, and powers
* can be OFF or in LPM.
*/
@@ -5794,7 +5839,7 @@ static void ufshcd_err_handling_unprepare(struct ufs_hba *hba)
if (ufshcd_is_clkscaling_supported(hba))
ufshcd_clk_scaling_suspend(hba, false);
ufshcd_clear_ua_wluns(hba);
- pm_runtime_put(hba->dev);
+ ufshcd_rpm_put(hba);
}
static inline bool ufshcd_err_handling_should_stop(struct ufs_hba *hba)
@@ -5815,14 +5860,18 @@ 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);
+
+ /* hba device might have a runtime error otherwise */
+ if (ret)
+ ret = pm_runtime_set_active(hba->dev);
/*
- * If hba device had runtime error, we also need to resume those
- * scsi devices under hba in case any of them has failed to be
- * resumed due to hba runtime resume failure. This is to unblock
+ * If wlun device had runtime error, we also need to resume those
+ * consumer scsi devices in case any of them has failed to be
+ * resumed due to supplier runtime resume failure. This is to unblock
* blk_queue_enter in case there are bios waiting inside it.
*/
if (!ret) {
@@ -7246,7 +7295,6 @@ static int ufshcd_scsi_add_wlus(struct ufs_hba *hba)
hba->sdev_ufs_device = NULL;
goto out;
}
- ufshcd_blk_pm_runtime_init(hba->sdev_ufs_device);
scsi_device_put(hba->sdev_ufs_device);
hba->sdev_rpmb = __scsi_add_device(hba->host, 0, 0,
@@ -7410,6 +7458,9 @@ static int ufs_get_device_desc(struct ufs_hba *hba)
goto out;
}
+ hba->luns_avail = desc_buf[DEVICE_DESC_PARAM_NUM_LU] +
+ desc_buf[DEVICE_DESC_PARAM_NUM_WLU];
+
ufs_fixup_device_setup(hba);
ufshcd_wb_probe(hba, desc_buf);
@@ -7887,6 +7938,7 @@ static int ufshcd_probe_hba(struct ufs_hba *hba, bool async)
ufshcd_set_ufs_dev_active(hba);
ufshcd_force_reset_auto_bkops(hba);
hba->wlun_dev_clr_ua = true;
+ hba->wlun_rpmb_clr_ua = true;
/* Gear up to HS gear if supported */
if (hba->max_pwr_info.is_valid) {
@@ -8472,7 +8524,8 @@ static int ufshcd_set_dev_pwr_mode(struct ufs_hba *hba,
* handling context.
*/
hba->host->eh_noresume = 1;
- ufshcd_clear_ua_wluns(hba);
+ if (hba->wlun_dev_clr_ua)
+ ufshcd_clear_ua_wlun(hba, UFS_UPIU_UFS_DEVICE_WLUN);
cmd[4] = pwr_mode << 4;
@@ -8647,23 +8700,7 @@ static void ufshcd_hba_vreg_set_hpm(struct ufs_hba *hba)
ufshcd_setup_hba_vreg(hba, true);
}
-/**
- * ufshcd_suspend - helper function for suspend operations
- * @hba: per adapter instance
- * @pm_op: desired low power operation type
- *
- * This function will try to put the UFS device and link into low power
- * mode based on the "rpm_lvl" (Runtime PM level) or "spm_lvl"
- * (System PM level).
- *
- * If this function is called during shutdown, it will make sure that
- * both UFS device and UFS link is powered off.
- *
- * NOTE: UFS device & link must be active before we enter in this function.
- *
- * Returns 0 for success and non-zero for failure
- */
-static int ufshcd_suspend(struct ufs_hba *hba, enum ufs_pm_op pm_op)
+static int __ufshcd_wl_suspend(struct ufs_hba *hba, enum ufs_pm_op pm_op)
{
int ret = 0;
int check_for_bkops;
@@ -8671,7 +8708,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;
@@ -8694,17 +8731,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 vops_suspend;
}
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;
}
if (ufshcd_is_runtime_pm(pm_op)) {
@@ -8716,7 +8753,7 @@ static int ufshcd_suspend(struct ufs_hba *hba, enum ufs_pm_op pm_op)
*/
ret = ufshcd_urgent_bkops(hba);
if (ret)
- goto enable_gating;
+ goto enable_scaling;
} else {
/* make sure that auto bkops is disabled */
ufshcd_disable_auto_bkops(hba);
@@ -8744,7 +8781,7 @@ static int ufshcd_suspend(struct ufs_hba *hba, enum ufs_pm_op pm_op)
if (!hba->dev_info.b_rpm_dev_flush_capable) {
ret = ufshcd_set_dev_pwr_mode(hba, req_dev_pwr_mode);
if (ret)
- goto enable_gating;
+ goto enable_scaling;
}
}
@@ -8757,7 +8794,7 @@ static int ufshcd_suspend(struct ufs_hba *hba, enum ufs_pm_op pm_op)
if (ret)
goto set_dev_active;
-disable_clks:
+vops_suspend:
/*
* Call vendor specific suspend callback. As these callbacks may access
* vendor specific host controller register space call them before the
@@ -8766,28 +8803,9 @@ static int ufshcd_suspend(struct ufs_hba *hba, enum ufs_pm_op pm_op)
ret = ufshcd_vops_suspend(hba, pm_op);
if (ret)
goto set_link_active;
- /*
- * 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)) {
- hba->clk_gating.state = CLKS_OFF;
- 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);
goto out;
set_link_active:
- ufshcd_vreg_set_hpm(hba);
/*
* Device hardware reset is required to exit DeepSleep. Also, for
* DeepSleep, the link is off so host reset and restore will be done
@@ -8809,57 +8827,32 @@ static int ufshcd_suspend(struct ufs_hba *hba, enum ufs_pm_op pm_op)
}
if (!ufshcd_set_dev_pwr_mode(hba, UFS_ACTIVE_PWR_MODE))
ufshcd_disable_auto_bkops(hba);
-enable_gating:
+enable_scaling:
if (ufshcd_is_clkscaling_supported(hba))
ufshcd_clk_scaling_suspend(hba, false);
- hba->clk_gating.is_suspended = false;
hba->dev_info.b_rpm_dev_flush_capable = false;
- ufshcd_clear_ua_wluns(hba);
- ufshcd_release(hba);
out:
if (hba->dev_info.b_rpm_dev_flush_capable) {
schedule_delayed_work(&hba->rpm_dev_flush_recheck_work,
msecs_to_jiffies(RPM_DEV_FLUSH_RECHECK_WORK_DELAY_MS));
}
- hba->pm_op_in_progress = 0;
-
- if (ret)
- ufshcd_update_evt_hist(hba, UFS_EVT_SUSPEND_ERR, (u32)ret);
+ if (ret) {
+ ufshcd_update_evt_hist(hba, UFS_EVT_WL_SUSP_ERR, (u32)ret);
+ hba->clk_gating.is_suspended = false;
+ ufshcd_release(hba);
+ }
+ hba->pm_op_in_progress = false;
return ret;
}
-/**
- * ufshcd_resume - helper function for resume operations
- * @hba: per adapter instance
- * @pm_op: runtime PM or system PM
- *
- * This function basically brings the UFS device, UniPro link and controller
- * to active state.
- *
- * Returns 0 for success and non-zero for failure
- */
-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;
-
- 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;
+ enum uic_link_state old_link_state = hba->uic_link_state;
- /* 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 +8861,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 +8909,219 @@ static int ufshcd_resume(struct ufs_hba *hba, enum ufs_pm_op pm_op)
if (hba->ee_usr_mask)
ufshcd_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);
-
if (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;
+}
+
+static int ufshcd_wl_runtime_suspend(struct device *dev)
+{
+ struct scsi_device *sdev = to_scsi_device(dev);
+ struct ufs_hba *hba;
+ int ret;
+ ktime_t start = ktime_get();
+
+ hba = shost_priv(sdev->host);
+
+ ret = __ufshcd_wl_suspend(hba, UFS_RUNTIME_PM);
+ if (ret)
+ dev_err(&sdev->sdev_gendev, "%s failed: %d\n", __func__, ret);
+
+ trace_ufshcd_wl_runtime_suspend(dev_name(dev), ret,
+ ktime_to_us(ktime_sub(ktime_get(), start)),
+ hba->curr_dev_pwr_mode, hba->uic_link_state);
+
+ return ret;
+}
+
+static int ufshcd_wl_runtime_resume(struct device *dev)
+{
+ struct scsi_device *sdev = to_scsi_device(dev);
+ struct ufs_hba *hba;
+ int ret = 0;
+ ktime_t start = ktime_get();
+
+ hba = shost_priv(sdev->host);
+
+ ret = __ufshcd_wl_resume(hba, UFS_RUNTIME_PM);
+ if (ret)
+ dev_err(&sdev->sdev_gendev, "%s failed: %d\n", __func__, ret);
+
+ trace_ufshcd_wl_runtime_resume(dev_name(dev), ret,
+ ktime_to_us(ktime_sub(ktime_get(), start)),
+ hba->curr_dev_pwr_mode, hba->uic_link_state);
+
+ return ret;
+}
+
+#ifdef CONFIG_PM_SLEEP
+static int ufshcd_wl_suspend(struct device *dev)
+{
+ struct scsi_device *sdev = to_scsi_device(dev);
+ struct ufs_hba *hba;
+ int ret = 0;
+ ktime_t start = ktime_get();
+
+ hba = shost_priv(sdev->host);
+ down(&hba->host_sem);
+
+ if (pm_runtime_suspended(dev))
+ goto out;
+
+ ret = __ufshcd_wl_suspend(hba, UFS_SYSTEM_PM);
+ if (ret) {
+ dev_err(&sdev->sdev_gendev, "%s failed: %d\n", __func__, ret);
+ up(&hba->host_sem);
+ }
+
+out:
+ if (!ret)
+ hba->is_sys_suspended = true;
+ trace_ufshcd_wl_suspend(dev_name(dev), ret,
+ ktime_to_us(ktime_sub(ktime_get(), start)),
+ hba->curr_dev_pwr_mode, hba->uic_link_state);
+
+ return ret;
+}
+
+static int ufshcd_wl_resume(struct device *dev)
+{
+ struct scsi_device *sdev = to_scsi_device(dev);
+ struct ufs_hba *hba;
+ int ret = 0;
+ ktime_t start = ktime_get();
+
+ hba = shost_priv(sdev->host);
+
+ if (pm_runtime_suspended(dev))
+ goto out;
+
+ ret = __ufshcd_wl_resume(hba, UFS_SYSTEM_PM);
+ if (ret)
+ dev_err(&sdev->sdev_gendev, "%s failed: %d\n", __func__, ret);
+out:
+ trace_ufshcd_wl_resume(dev_name(dev), ret,
+ ktime_to_us(ktime_sub(ktime_get(), start)),
+ hba->curr_dev_pwr_mode, hba->uic_link_state);
+ if (!ret)
+ hba->is_sys_suspended = false;
+ up(&hba->host_sem);
+ return ret;
+}
+#endif
+
+static void ufshcd_wl_shutdown(struct device *dev)
+{
+ struct scsi_device *sdev = to_scsi_device(dev);
+ struct ufs_hba *hba;
+
+ hba = shost_priv(sdev->host);
+
+ down(&hba->host_sem);
+ hba->shutting_down = true;
+ up(&hba->host_sem);
+
+ /* Turn on everything while shutting down */
+ ufshcd_rpm_get_sync(hba);
+ scsi_device_quiesce(sdev);
+ shost_for_each_device(sdev, hba->host) {
+ if (sdev == hba->sdev_ufs_device)
+ continue;
+ scsi_device_quiesce(sdev);
+ }
+ __ufshcd_wl_suspend(hba, UFS_SHUTDOWN_PM);
+}
+
+/**
+ * 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 int ufshcd_suspend(struct ufs_hba *hba)
+{
+ int ret;
+
+ if (!hba->is_powered)
+ return 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);
+ ret = ufshcd_setup_clocks(hba, false);
+ if (ret) {
+ ufshcd_enable_irq(hba);
+ return ret;
+ }
if (ufshcd_is_clkgating_allowed(hba)) {
hba->clk_gating.state = CLKS_OFF;
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);
+ return ret;
+}
+
+/**
+ * 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;
+
+ if (!hba->is_powered)
+ return 0;
+
+ 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:
- hba->pm_op_in_progress = 0;
if (ret)
ufshcd_update_evt_hist(hba, UFS_EVT_RESUME_ERR, (u32)ret);
return ret;
@@ -8962,6 +9132,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
*/
@@ -8970,41 +9141,14 @@ int ufshcd_system_suspend(struct ufs_hba *hba)
int ret = 0;
ktime_t start = ktime_get();
- down(&hba->host_sem);
-
- if (!hba->is_powered)
- return 0;
-
- 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)
+ if (pm_runtime_suspended(hba->dev))
goto out;
- if (pm_runtime_suspended(hba->dev)) {
- /*
- * UFS device and/or UFS link low power states during runtime
- * 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;
- }
-
- ret = ufshcd_suspend(hba, UFS_SYSTEM_PM);
+ ret = ufshcd_suspend(hba);
out:
trace_ufshcd_system_suspend(dev_name(hba->dev), ret,
ktime_to_us(ktime_sub(ktime_get(), start)),
hba->curr_dev_pwr_mode, hba->uic_link_state);
- if (!ret)
- hba->is_sys_suspended = true;
- else
- up(&hba->host_sem);
return ret;
}
EXPORT_SYMBOL(ufshcd_system_suspend);
@@ -9021,21 +9165,16 @@ int ufshcd_system_resume(struct ufs_hba *hba)
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 (pm_runtime_suspended(hba->dev))
goto out;
- else
- ret = ufshcd_resume(hba, UFS_SYSTEM_PM);
+
+ ret = ufshcd_resume(hba);
+
out:
trace_ufshcd_system_resume(dev_name(hba->dev), ret,
ktime_to_us(ktime_sub(ktime_get(), start)),
hba->curr_dev_pwr_mode, hba->uic_link_state);
- if (!ret)
- hba->is_sys_suspended = false;
- up(&hba->host_sem);
+
return ret;
}
EXPORT_SYMBOL(ufshcd_system_resume);
@@ -9045,19 +9184,17 @@ 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;
+ int ret;
ktime_t start = ktime_get();
- if (!hba->is_powered)
- goto out;
- else
- ret = ufshcd_suspend(hba, UFS_RUNTIME_PM);
-out:
+ ret = ufshcd_suspend(hba);
+
trace_ufshcd_runtime_suspend(dev_name(hba->dev), ret,
ktime_to_us(ktime_sub(ktime_get(), start)),
hba->curr_dev_pwr_mode, hba->uic_link_state);
@@ -9069,33 +9206,19 @@ 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;
+ int ret;
ktime_t start = ktime_get();
- if (!hba->is_powered)
- goto out;
- else
- ret = ufshcd_resume(hba, UFS_RUNTIME_PM);
-out:
+ ret = ufshcd_resume(hba);
+
trace_ufshcd_runtime_resume(dev_name(hba->dev), ret,
ktime_to_us(ktime_sub(ktime_get(), start)),
hba->curr_dev_pwr_mode, hba->uic_link_state);
@@ -9113,30 +9236,20 @@ 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;
-
if (ufshcd_is_ufs_dev_poweroff(hba) && ufshcd_is_link_off(hba))
goto out;
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 +9263,8 @@ EXPORT_SYMBOL(ufshcd_shutdown);
*/
void ufshcd_remove(struct ufs_hba *hba)
{
+ if (hba->sdev_ufs_device)
+ ufshcd_rpm_get_sync(hba);
ufs_bsg_remove(hba);
ufs_sysfs_remove_nodes(hba->dev);
blk_cleanup_queue(hba->tmf_queue);
@@ -9453,15 +9568,203 @@ 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);
+
+ 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);
+ 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;
+
+ /*
+ * 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.
+ * However, if the current_{pwr_mode, link_state} is same as the
+ * desired_{pwr_mode, link_state}, there's no need to rpm resume
+ * the device.
+ * Refer ufshcd_resume_complete()
+ */
+ pm_runtime_get_noresume(ufs_dev);
+
+ 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 (!rpm_state_ok) {
+ int ret = pm_runtime_resume(ufs_dev);
+
+ if (ret < 0 && ret != -EACCES) {
+ pm_runtime_put(ufs_dev);
+ return ret;
+ }
+ }
+ 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();
- return 0;
+
+ 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 ret;
+unregister:
+ scsi_unregister_driver(&ufs_dev_wlun_template.gendrv);
+debugfs_exit:
+ ufs_debugfs_exit();
+ return ret;
}
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);
diff --git a/drivers/scsi/ufs/ufshcd.h b/drivers/scsi/ufs/ufshcd.h
index 5eb66a8..3a8dc0d 100644
--- a/drivers/scsi/ufs/ufshcd.h
+++ b/drivers/scsi/ufs/ufshcd.h
@@ -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 */
diff --git a/include/trace/events/ufs.h b/include/trace/events/ufs.h
index 1cb6f1a..599739e 100644
--- a/include/trace/events/ufs.h
+++ b/include/trace/events/ufs.h
@@ -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,
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project.
_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek
^ permalink raw reply related [flat|nested] 31+ messages in thread* Re: [PATCH v20 1/2] scsi: ufs: Enable power management for wlun
2021-04-16 19:49 ` Asutosh Das
(?)
@ 2021-04-19 18:37 ` Adrian Hunter
-1 siblings, 0 replies; 31+ messages in thread
From: Adrian Hunter @ 2021-04-19 18:37 UTC (permalink / raw)
To: Asutosh Das, cang, martin.petersen, linux-scsi
Cc: linux-arm-msm, Alim Akhtar, Avri Altman, James E.J. Bottomley,
Krzysztof Kozlowski, Stanley Chu, Andy Gross, Bjorn Andersson,
Steven Rostedt, Ingo Molnar, Matthias Brugger, Lee Jones,
Bean Huo, Kiwoong Kim, Colin Ian King, Wei Yongjun, Yue Hu,
Bart van Assche, Gustavo A. R. Silva, Dinghao Liu, Jaegeuk Kim,
Satya Tangirala, open list, moderated list:ARM/SAMSUNG S3C,
S5P AND EXYNOS ARM ARCHITECTURES, open list:ARM/SAMSUNG S3C,
S5P AND EXYNOS ARM ARCHITECTURES,
moderated list:UNIVERSAL FLASH STORAGE HOST CONTROLLER DRIVER...
On 16/04/21 10:49 pm, Asutosh Das wrote:
> 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.
>
> Fixed smatch warnings:
> Reported-by: kernel test robot <lkp@intel.com>
> Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
>
> Co-developed-by: Can Guo <cang@codeaurora.org>
> Signed-off-by: Can Guo <cang@codeaurora.org>
> Signed-off-by: Asutosh Das <asutoshd@codeaurora.org>
> ---
I came across 3 issues while testing. See comments below.
<SNIP>
> @@ -5753,12 +5797,13 @@ static void ufshcd_clk_scaling_suspend(struct ufs_hba *hba, bool suspend)
>
> static void ufshcd_err_handling_prepare(struct ufs_hba *hba)
> {
> - pm_runtime_get_sync(hba->dev);
> - if (pm_runtime_status_suspended(hba->dev) || hba->is_sys_suspended) {
> + ufshcd_rpm_get_sync(hba);
hba->sdev_ufs_device could be NULL.
Need to add a check for that in ufshcd_err_handling_should_stop()
> + if (pm_runtime_status_suspended(&hba->sdev_ufs_device->sdev_gendev) ||
> + hba->is_sys_suspended) {
> enum ufs_pm_op pm_op;
>
> /*
> - * Don't assume anything of pm_runtime_get_sync(), if
> + * Don't assume anything of resume, if
> * resume fails, irq and clocks can be OFF, and powers
> * can be OFF or in LPM.
> */
> @@ -5794,7 +5839,7 @@ static void ufshcd_err_handling_unprepare(struct ufs_hba *hba)
> if (ufshcd_is_clkscaling_supported(hba))
> ufshcd_clk_scaling_suspend(hba, false);
> ufshcd_clear_ua_wluns(hba);
ufshcd_clear_ua_wluns() deadlocks trying to clear UFS_UPIU_RPMB_WLUN
if sdev_rpmb is suspended and sdev_ufs_device is suspending.
e.g. ufshcd_wl_suspend() is waiting on host_sem while ufshcd_err_handler()
is running, at which point sdev_rpmb has already suspended.
> - pm_runtime_put(hba->dev);
> + ufshcd_rpm_put(hba);
> }
<SNIP>
> +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);
> + 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;
> +
> + /*
> + * 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.
> + * However, if the current_{pwr_mode, link_state} is same as the
> + * desired_{pwr_mode, link_state}, there's no need to rpm resume
> + * the device.
> + * Refer ufshcd_resume_complete()
> + */
> + pm_runtime_get_noresume(ufs_dev);
> +
> + 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 (!rpm_state_ok) {
> + int ret = pm_runtime_resume(ufs_dev);
> +
> + if (ret < 0 && ret != -EACCES) {
> + pm_runtime_put(ufs_dev);
> + return ret;
> + }
> + }
> + return 0;
> +}
Unfortunately this does not work because SCSI PM forcibly sets
the sdevs to runtime active after system resume. Really we should
change SCSI PM to call the driver's .prepare / .complete then we could
use direct complete, but let's leave that for now and go back to
before, but allowing for errors and !hba->sdev_ufs_device. e.g.
void ufshcd_resume_complete(struct device *dev)
{
struct ufs_hba *hba = dev_get_drvdata(dev);
if (hba->complete_put) {
hba->complete_put = false;
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);
int ret;
if (!hba->sdev_ufs_device)
return 0;
ret = ufshcd_rpm_get_sync(hba);
if (ret < 0 && ret != -EACCES) {
ufshcd_rpm_put(hba);
return ret;
}
hba->complete_put = true;
return 0;
}
EXPORT_SYMBOL_GPL(ufshcd_suspend_prepare);
^ permalink raw reply [flat|nested] 31+ messages in thread* Re: [PATCH v20 1/2] scsi: ufs: Enable power management for wlun
@ 2021-04-19 18:37 ` Adrian Hunter
0 siblings, 0 replies; 31+ messages in thread
From: Adrian Hunter @ 2021-04-19 18:37 UTC (permalink / raw)
To: Asutosh Das, cang, martin.petersen, linux-scsi
Cc: linux-arm-msm, Alim Akhtar, Avri Altman, James E.J. Bottomley,
Krzysztof Kozlowski, Stanley Chu, Andy Gross, Bjorn Andersson,
Steven Rostedt, Ingo Molnar, Matthias Brugger, Lee Jones,
Bean Huo, Kiwoong Kim, Colin Ian King, Wei Yongjun, Yue Hu,
Bart van Assche, Gustavo A. R. Silva, Dinghao Liu, Jaegeuk Kim,
Satya Tangirala, open list, moderated list:ARM/SAMSUNG S3C,
S5P AND EXYNOS ARM ARCHITECTURES, open list:ARM/SAMSUNG S3C,
S5P AND EXYNOS ARM ARCHITECTURES,
moderated list:UNIVERSAL FLASH STORAGE HOST CONTROLLER DRIVER...
On 16/04/21 10:49 pm, Asutosh Das wrote:
> 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.
>
> Fixed smatch warnings:
> Reported-by: kernel test robot <lkp@intel.com>
> Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
>
> Co-developed-by: Can Guo <cang@codeaurora.org>
> Signed-off-by: Can Guo <cang@codeaurora.org>
> Signed-off-by: Asutosh Das <asutoshd@codeaurora.org>
> ---
I came across 3 issues while testing. See comments below.
<SNIP>
> @@ -5753,12 +5797,13 @@ static void ufshcd_clk_scaling_suspend(struct ufs_hba *hba, bool suspend)
>
> static void ufshcd_err_handling_prepare(struct ufs_hba *hba)
> {
> - pm_runtime_get_sync(hba->dev);
> - if (pm_runtime_status_suspended(hba->dev) || hba->is_sys_suspended) {
> + ufshcd_rpm_get_sync(hba);
hba->sdev_ufs_device could be NULL.
Need to add a check for that in ufshcd_err_handling_should_stop()
> + if (pm_runtime_status_suspended(&hba->sdev_ufs_device->sdev_gendev) ||
> + hba->is_sys_suspended) {
> enum ufs_pm_op pm_op;
>
> /*
> - * Don't assume anything of pm_runtime_get_sync(), if
> + * Don't assume anything of resume, if
> * resume fails, irq and clocks can be OFF, and powers
> * can be OFF or in LPM.
> */
> @@ -5794,7 +5839,7 @@ static void ufshcd_err_handling_unprepare(struct ufs_hba *hba)
> if (ufshcd_is_clkscaling_supported(hba))
> ufshcd_clk_scaling_suspend(hba, false);
> ufshcd_clear_ua_wluns(hba);
ufshcd_clear_ua_wluns() deadlocks trying to clear UFS_UPIU_RPMB_WLUN
if sdev_rpmb is suspended and sdev_ufs_device is suspending.
e.g. ufshcd_wl_suspend() is waiting on host_sem while ufshcd_err_handler()
is running, at which point sdev_rpmb has already suspended.
> - pm_runtime_put(hba->dev);
> + ufshcd_rpm_put(hba);
> }
<SNIP>
> +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);
> + 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;
> +
> + /*
> + * 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.
> + * However, if the current_{pwr_mode, link_state} is same as the
> + * desired_{pwr_mode, link_state}, there's no need to rpm resume
> + * the device.
> + * Refer ufshcd_resume_complete()
> + */
> + pm_runtime_get_noresume(ufs_dev);
> +
> + 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 (!rpm_state_ok) {
> + int ret = pm_runtime_resume(ufs_dev);
> +
> + if (ret < 0 && ret != -EACCES) {
> + pm_runtime_put(ufs_dev);
> + return ret;
> + }
> + }
> + return 0;
> +}
Unfortunately this does not work because SCSI PM forcibly sets
the sdevs to runtime active after system resume. Really we should
change SCSI PM to call the driver's .prepare / .complete then we could
use direct complete, but let's leave that for now and go back to
before, but allowing for errors and !hba->sdev_ufs_device. e.g.
void ufshcd_resume_complete(struct device *dev)
{
struct ufs_hba *hba = dev_get_drvdata(dev);
if (hba->complete_put) {
hba->complete_put = false;
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);
int ret;
if (!hba->sdev_ufs_device)
return 0;
ret = ufshcd_rpm_get_sync(hba);
if (ret < 0 && ret != -EACCES) {
ufshcd_rpm_put(hba);
return ret;
}
hba->complete_put = true;
return 0;
}
EXPORT_SYMBOL_GPL(ufshcd_suspend_prepare);
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 31+ messages in thread* Re: [PATCH v20 1/2] scsi: ufs: Enable power management for wlun
@ 2021-04-19 18:37 ` Adrian Hunter
0 siblings, 0 replies; 31+ messages in thread
From: Adrian Hunter @ 2021-04-19 18:37 UTC (permalink / raw)
To: Asutosh Das, cang, martin.petersen, linux-scsi
Cc: linux-arm-msm, Alim Akhtar, Avri Altman, James E.J. Bottomley,
Krzysztof Kozlowski, Stanley Chu, Andy Gross, Bjorn Andersson,
Steven Rostedt, Ingo Molnar, Matthias Brugger, Lee Jones,
Bean Huo, Kiwoong Kim, Colin Ian King, Wei Yongjun, Yue Hu,
Bart van Assche, Gustavo A. R. Silva, Dinghao Liu, Jaegeuk Kim,
Satya Tangirala, open list, moderated list:ARM/SAMSUNG S3C,
S5P AND EXYNOS ARM ARCHITECTURES, open list:ARM/SAMSUNG S3C,
S5P AND EXYNOS ARM ARCHITECTURES,
moderated list:UNIVERSAL FLASH STORAGE HOST CONTROLLER DRIVER...
On 16/04/21 10:49 pm, Asutosh Das wrote:
> 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.
>
> Fixed smatch warnings:
> Reported-by: kernel test robot <lkp@intel.com>
> Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
>
> Co-developed-by: Can Guo <cang@codeaurora.org>
> Signed-off-by: Can Guo <cang@codeaurora.org>
> Signed-off-by: Asutosh Das <asutoshd@codeaurora.org>
> ---
I came across 3 issues while testing. See comments below.
<SNIP>
> @@ -5753,12 +5797,13 @@ static void ufshcd_clk_scaling_suspend(struct ufs_hba *hba, bool suspend)
>
> static void ufshcd_err_handling_prepare(struct ufs_hba *hba)
> {
> - pm_runtime_get_sync(hba->dev);
> - if (pm_runtime_status_suspended(hba->dev) || hba->is_sys_suspended) {
> + ufshcd_rpm_get_sync(hba);
hba->sdev_ufs_device could be NULL.
Need to add a check for that in ufshcd_err_handling_should_stop()
> + if (pm_runtime_status_suspended(&hba->sdev_ufs_device->sdev_gendev) ||
> + hba->is_sys_suspended) {
> enum ufs_pm_op pm_op;
>
> /*
> - * Don't assume anything of pm_runtime_get_sync(), if
> + * Don't assume anything of resume, if
> * resume fails, irq and clocks can be OFF, and powers
> * can be OFF or in LPM.
> */
> @@ -5794,7 +5839,7 @@ static void ufshcd_err_handling_unprepare(struct ufs_hba *hba)
> if (ufshcd_is_clkscaling_supported(hba))
> ufshcd_clk_scaling_suspend(hba, false);
> ufshcd_clear_ua_wluns(hba);
ufshcd_clear_ua_wluns() deadlocks trying to clear UFS_UPIU_RPMB_WLUN
if sdev_rpmb is suspended and sdev_ufs_device is suspending.
e.g. ufshcd_wl_suspend() is waiting on host_sem while ufshcd_err_handler()
is running, at which point sdev_rpmb has already suspended.
> - pm_runtime_put(hba->dev);
> + ufshcd_rpm_put(hba);
> }
<SNIP>
> +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);
> + 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;
> +
> + /*
> + * 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.
> + * However, if the current_{pwr_mode, link_state} is same as the
> + * desired_{pwr_mode, link_state}, there's no need to rpm resume
> + * the device.
> + * Refer ufshcd_resume_complete()
> + */
> + pm_runtime_get_noresume(ufs_dev);
> +
> + 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 (!rpm_state_ok) {
> + int ret = pm_runtime_resume(ufs_dev);
> +
> + if (ret < 0 && ret != -EACCES) {
> + pm_runtime_put(ufs_dev);
> + return ret;
> + }
> + }
> + return 0;
> +}
Unfortunately this does not work because SCSI PM forcibly sets
the sdevs to runtime active after system resume. Really we should
change SCSI PM to call the driver's .prepare / .complete then we could
use direct complete, but let's leave that for now and go back to
before, but allowing for errors and !hba->sdev_ufs_device. e.g.
void ufshcd_resume_complete(struct device *dev)
{
struct ufs_hba *hba = dev_get_drvdata(dev);
if (hba->complete_put) {
hba->complete_put = false;
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);
int ret;
if (!hba->sdev_ufs_device)
return 0;
ret = ufshcd_rpm_get_sync(hba);
if (ret < 0 && ret != -EACCES) {
ufshcd_rpm_put(hba);
return ret;
}
hba->complete_put = true;
return 0;
}
EXPORT_SYMBOL_GPL(ufshcd_suspend_prepare);
_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek
^ permalink raw reply [flat|nested] 31+ messages in thread* Re: [PATCH v20 1/2] scsi: ufs: Enable power management for wlun
2021-04-19 18:37 ` Adrian Hunter
@ 2021-04-19 21:53 ` Asutosh Das (asd)
-1 siblings, 0 replies; 31+ messages in thread
From: Asutosh Das (asd) @ 2021-04-19 21:53 UTC (permalink / raw)
To: Adrian Hunter, cang, martin.petersen, linux-scsi
Cc: linux-arm-msm, Alim Akhtar, Avri Altman, James E.J. Bottomley,
Krzysztof Kozlowski, Stanley Chu, Andy Gross, Bjorn Andersson,
Steven Rostedt, Ingo Molnar, Matthias Brugger, Lee Jones,
Bean Huo, Kiwoong Kim, Colin Ian King, Wei Yongjun, Yue Hu,
Bart van Assche, Gustavo A. R. Silva, Dinghao Liu, Jaegeuk Kim,
Satya Tangirala, open list, moderated list:ARM/SAMSUNG S3C,
S5P AND EXYNOS ARM ARCHITECTURES, open list:ARM/SAMSUNG S3C,
S5P AND EXYNOS ARM ARCHITECTURES,
moderated list:UNIVERSAL FLASH STORAGE HOST CONTROLLER DRIVER...
On 4/19/2021 11:37 AM, Adrian Hunter wrote:
> On 16/04/21 10:49 pm, Asutosh Das wrote:
>>
>> Co-developed-by: Can Guo <cang@codeaurora.org>
>> Signed-off-by: Can Guo <cang@codeaurora.org>
>> Signed-off-by: Asutosh Das <asutoshd@codeaurora.org>
>> ---
>
> I came across 3 issues while testing. See comments below.
>
Hi Adrian
Thanks for the comments.
> <SNIP>
>
>> @@ -5794,7 +5839,7 @@ static void ufshcd_err_handling_unprepare(struct ufs_hba *hba)
>> if (ufshcd_is_clkscaling_supported(hba))
>> ufshcd_clk_scaling_suspend(hba, false);
>> ufshcd_clear_ua_wluns(hba);
>
> ufshcd_clear_ua_wluns() deadlocks trying to clear UFS_UPIU_RPMB_WLUN
> if sdev_rpmb is suspended and sdev_ufs_device is suspending.
> e.g. ufshcd_wl_suspend() is waiting on host_sem while ufshcd_err_handler()
> is running, at which point sdev_rpmb has already suspended.
>
Umm, I didn't understand this deadlock.
When you say, sdev_rpmb is suspended, does it mean runtime_suspended?
sdev_ufs_device is suspending - this can't be runtime_suspending, while
ufshcd_err_handling_unprepare is running.
If you've a call-stack of this deadlock, please can you share it with
me. I'll also try to reproduce this.
I'll address the other comments in the next version.
Thank you!
>> - pm_runtime_put(hba->dev);
>> + ufshcd_rpm_put(hba);
>> }
>
> <SNIP>
>
>> +void ufshcd_resume_complete(struct device *dev)
>> +{
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
Linux Foundation Collaborative Project
^ permalink raw reply [flat|nested] 31+ messages in thread* Re: [PATCH v20 1/2] scsi: ufs: Enable power management for wlun
@ 2021-04-19 21:53 ` Asutosh Das (asd)
0 siblings, 0 replies; 31+ messages in thread
From: Asutosh Das (asd) @ 2021-04-19 21:53 UTC (permalink / raw)
To: Adrian Hunter, cang, martin.petersen, linux-scsi
Cc: linux-arm-msm, Alim Akhtar, Avri Altman, James E.J. Bottomley,
Krzysztof Kozlowski, Stanley Chu, Andy Gross, Bjorn Andersson,
Steven Rostedt, Ingo Molnar, Matthias Brugger, Lee Jones,
Bean Huo, Kiwoong Kim, Colin Ian King, Wei Yongjun, Yue Hu,
Bart van Assche, Gustavo A. R. Silva, Dinghao Liu, Jaegeuk Kim,
Satya Tangirala, open list, moderated list:ARM/SAMSUNG S3C,
S5P AND EXYNOS ARM ARCHITECTURES, open list:ARM/SAMSUNG S3C,
S5P AND EXYNOS ARM ARCHITECTURES,
moderated list:UNIVERSAL FLASH STORAGE HOST CONTROLLER DRIVER...
On 4/19/2021 11:37 AM, Adrian Hunter wrote:
> On 16/04/21 10:49 pm, Asutosh Das wrote:
>>
>> Co-developed-by: Can Guo <cang@codeaurora.org>
>> Signed-off-by: Can Guo <cang@codeaurora.org>
>> Signed-off-by: Asutosh Das <asutoshd@codeaurora.org>
>> ---
>
> I came across 3 issues while testing. See comments below.
>
Hi Adrian
Thanks for the comments.
> <SNIP>
>
>> @@ -5794,7 +5839,7 @@ static void ufshcd_err_handling_unprepare(struct ufs_hba *hba)
>> if (ufshcd_is_clkscaling_supported(hba))
>> ufshcd_clk_scaling_suspend(hba, false);
>> ufshcd_clear_ua_wluns(hba);
>
> ufshcd_clear_ua_wluns() deadlocks trying to clear UFS_UPIU_RPMB_WLUN
> if sdev_rpmb is suspended and sdev_ufs_device is suspending.
> e.g. ufshcd_wl_suspend() is waiting on host_sem while ufshcd_err_handler()
> is running, at which point sdev_rpmb has already suspended.
>
Umm, I didn't understand this deadlock.
When you say, sdev_rpmb is suspended, does it mean runtime_suspended?
sdev_ufs_device is suspending - this can't be runtime_suspending, while
ufshcd_err_handling_unprepare is running.
If you've a call-stack of this deadlock, please can you share it with
me. I'll also try to reproduce this.
I'll address the other comments in the next version.
Thank you!
>> - pm_runtime_put(hba->dev);
>> + ufshcd_rpm_put(hba);
>> }
>
> <SNIP>
>
>> +void ufshcd_resume_complete(struct device *dev)
>> +{
--
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
^ permalink raw reply [flat|nested] 31+ messages in thread* Re: [PATCH v20 1/2] scsi: ufs: Enable power management for wlun
2021-04-19 21:53 ` Asutosh Das (asd)
(?)
@ 2021-04-20 4:15 ` Adrian Hunter
-1 siblings, 0 replies; 31+ messages in thread
From: Adrian Hunter @ 2021-04-20 4:15 UTC (permalink / raw)
To: Asutosh Das (asd), cang, martin.petersen, linux-scsi
Cc: linux-arm-msm, Alim Akhtar, Avri Altman, James E.J. Bottomley,
Krzysztof Kozlowski, Stanley Chu, Andy Gross, Bjorn Andersson,
Steven Rostedt, Ingo Molnar, Matthias Brugger, Lee Jones,
Bean Huo, Kiwoong Kim, Colin Ian King, Wei Yongjun, Yue Hu,
Bart van Assche, Gustavo A. R. Silva, Dinghao Liu, Jaegeuk Kim,
Satya Tangirala, open list, moderated list:ARM/SAMSUNG S3C,
S5P AND EXYNOS ARM ARCHITECTURES, open list:ARM/SAMSUNG S3C,
S5P AND EXYNOS ARM ARCHITECTURES,
moderated list:UNIVERSAL FLASH STORAGE HOST CONTROLLER DRIVER...
On 20/04/21 12:53 am, Asutosh Das (asd) wrote:
> On 4/19/2021 11:37 AM, Adrian Hunter wrote:
>> On 16/04/21 10:49 pm, Asutosh Das wrote:
>>>
>>> Co-developed-by: Can Guo <cang@codeaurora.org>
>>> Signed-off-by: Can Guo <cang@codeaurora.org>
>>> Signed-off-by: Asutosh Das <asutoshd@codeaurora.org>
>>> ---
>>
>> I came across 3 issues while testing. See comments below.
>>
> Hi Adrian
> Thanks for the comments.
>> <SNIP>
>>
>>> @@ -5794,7 +5839,7 @@ static void ufshcd_err_handling_unprepare(struct ufs_hba *hba)
>>> if (ufshcd_is_clkscaling_supported(hba))
>>> ufshcd_clk_scaling_suspend(hba, false);
>>> ufshcd_clear_ua_wluns(hba);
>>
>> ufshcd_clear_ua_wluns() deadlocks trying to clear UFS_UPIU_RPMB_WLUN
>> if sdev_rpmb is suspended and sdev_ufs_device is suspending.
>> e.g. ufshcd_wl_suspend() is waiting on host_sem while ufshcd_err_handler()
>> is running, at which point sdev_rpmb has already suspended.
>>
> Umm, I didn't understand this deadlock.
> When you say, sdev_rpmb is suspended, does it mean runtime_suspended?
> sdev_ufs_device is suspending - this can't be runtime_suspending, while ufshcd_err_handling_unprepare is running.
>
> If you've a call-stack of this deadlock, please can you share it with me. I'll also try to reproduce this.
Yes it is system suspend. sdev_rpmb has suspended, sdev_ufs_device is waiting on host_sem.
ufshcd_err_handler() holds host_sem. ufshcd_clear_ua_wlun(UFS_UPIU_RPMB_WLUN) gets stuck.
I will get some call-stacks.
>
> I'll address the other comments in the next version.
>
>
> Thank you!
>
>>> - pm_runtime_put(hba->dev);
>>> + ufshcd_rpm_put(hba);
>>> }
>>
>> <SNIP>
>>
>>> +void ufshcd_resume_complete(struct device *dev)
>>> +{
>
^ permalink raw reply [flat|nested] 31+ messages in thread* Re: [PATCH v20 1/2] scsi: ufs: Enable power management for wlun
@ 2021-04-20 4:15 ` Adrian Hunter
0 siblings, 0 replies; 31+ messages in thread
From: Adrian Hunter @ 2021-04-20 4:15 UTC (permalink / raw)
To: Asutosh Das (asd), cang, martin.petersen, linux-scsi
Cc: linux-arm-msm, Alim Akhtar, Avri Altman, James E.J. Bottomley,
Krzysztof Kozlowski, Stanley Chu, Andy Gross, Bjorn Andersson,
Steven Rostedt, Ingo Molnar, Matthias Brugger, Lee Jones,
Bean Huo, Kiwoong Kim, Colin Ian King, Wei Yongjun, Yue Hu,
Bart van Assche, Gustavo A. R. Silva, Dinghao Liu, Jaegeuk Kim,
Satya Tangirala, open list, moderated list:ARM/SAMSUNG S3C,
S5P AND EXYNOS ARM ARCHITECTURES, open list:ARM/SAMSUNG S3C,
S5P AND EXYNOS ARM ARCHITECTURES,
moderated list:UNIVERSAL FLASH STORAGE HOST CONTROLLER DRIVER...
On 20/04/21 12:53 am, Asutosh Das (asd) wrote:
> On 4/19/2021 11:37 AM, Adrian Hunter wrote:
>> On 16/04/21 10:49 pm, Asutosh Das wrote:
>>>
>>> Co-developed-by: Can Guo <cang@codeaurora.org>
>>> Signed-off-by: Can Guo <cang@codeaurora.org>
>>> Signed-off-by: Asutosh Das <asutoshd@codeaurora.org>
>>> ---
>>
>> I came across 3 issues while testing. See comments below.
>>
> Hi Adrian
> Thanks for the comments.
>> <SNIP>
>>
>>> @@ -5794,7 +5839,7 @@ static void ufshcd_err_handling_unprepare(struct ufs_hba *hba)
>>> if (ufshcd_is_clkscaling_supported(hba))
>>> ufshcd_clk_scaling_suspend(hba, false);
>>> ufshcd_clear_ua_wluns(hba);
>>
>> ufshcd_clear_ua_wluns() deadlocks trying to clear UFS_UPIU_RPMB_WLUN
>> if sdev_rpmb is suspended and sdev_ufs_device is suspending.
>> e.g. ufshcd_wl_suspend() is waiting on host_sem while ufshcd_err_handler()
>> is running, at which point sdev_rpmb has already suspended.
>>
> Umm, I didn't understand this deadlock.
> When you say, sdev_rpmb is suspended, does it mean runtime_suspended?
> sdev_ufs_device is suspending - this can't be runtime_suspending, while ufshcd_err_handling_unprepare is running.
>
> If you've a call-stack of this deadlock, please can you share it with me. I'll also try to reproduce this.
Yes it is system suspend. sdev_rpmb has suspended, sdev_ufs_device is waiting on host_sem.
ufshcd_err_handler() holds host_sem. ufshcd_clear_ua_wlun(UFS_UPIU_RPMB_WLUN) gets stuck.
I will get some call-stacks.
>
> I'll address the other comments in the next version.
>
>
> Thank you!
>
>>> - pm_runtime_put(hba->dev);
>>> + ufshcd_rpm_put(hba);
>>> }
>>
>> <SNIP>
>>
>>> +void ufshcd_resume_complete(struct device *dev)
>>> +{
>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 31+ messages in thread* Re: [PATCH v20 1/2] scsi: ufs: Enable power management for wlun
@ 2021-04-20 4:15 ` Adrian Hunter
0 siblings, 0 replies; 31+ messages in thread
From: Adrian Hunter @ 2021-04-20 4:15 UTC (permalink / raw)
To: Asutosh Das (asd), cang, martin.petersen, linux-scsi
Cc: linux-arm-msm, Alim Akhtar, Avri Altman, James E.J. Bottomley,
Krzysztof Kozlowski, Stanley Chu, Andy Gross, Bjorn Andersson,
Steven Rostedt, Ingo Molnar, Matthias Brugger, Lee Jones,
Bean Huo, Kiwoong Kim, Colin Ian King, Wei Yongjun, Yue Hu,
Bart van Assche, Gustavo A. R. Silva, Dinghao Liu, Jaegeuk Kim,
Satya Tangirala, open list, moderated list:ARM/SAMSUNG S3C,
S5P AND EXYNOS ARM ARCHITECTURES, open list:ARM/SAMSUNG S3C,
S5P AND EXYNOS ARM ARCHITECTURES,
moderated list:UNIVERSAL FLASH STORAGE HOST CONTROLLER DRIVER...
On 20/04/21 12:53 am, Asutosh Das (asd) wrote:
> On 4/19/2021 11:37 AM, Adrian Hunter wrote:
>> On 16/04/21 10:49 pm, Asutosh Das wrote:
>>>
>>> Co-developed-by: Can Guo <cang@codeaurora.org>
>>> Signed-off-by: Can Guo <cang@codeaurora.org>
>>> Signed-off-by: Asutosh Das <asutoshd@codeaurora.org>
>>> ---
>>
>> I came across 3 issues while testing. See comments below.
>>
> Hi Adrian
> Thanks for the comments.
>> <SNIP>
>>
>>> @@ -5794,7 +5839,7 @@ static void ufshcd_err_handling_unprepare(struct ufs_hba *hba)
>>> if (ufshcd_is_clkscaling_supported(hba))
>>> ufshcd_clk_scaling_suspend(hba, false);
>>> ufshcd_clear_ua_wluns(hba);
>>
>> ufshcd_clear_ua_wluns() deadlocks trying to clear UFS_UPIU_RPMB_WLUN
>> if sdev_rpmb is suspended and sdev_ufs_device is suspending.
>> e.g. ufshcd_wl_suspend() is waiting on host_sem while ufshcd_err_handler()
>> is running, at which point sdev_rpmb has already suspended.
>>
> Umm, I didn't understand this deadlock.
> When you say, sdev_rpmb is suspended, does it mean runtime_suspended?
> sdev_ufs_device is suspending - this can't be runtime_suspending, while ufshcd_err_handling_unprepare is running.
>
> If you've a call-stack of this deadlock, please can you share it with me. I'll also try to reproduce this.
Yes it is system suspend. sdev_rpmb has suspended, sdev_ufs_device is waiting on host_sem.
ufshcd_err_handler() holds host_sem. ufshcd_clear_ua_wlun(UFS_UPIU_RPMB_WLUN) gets stuck.
I will get some call-stacks.
>
> I'll address the other comments in the next version.
>
>
> Thank you!
>
>>> - pm_runtime_put(hba->dev);
>>> + ufshcd_rpm_put(hba);
>>> }
>>
>> <SNIP>
>>
>>> +void ufshcd_resume_complete(struct device *dev)
>>> +{
>
_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek
^ permalink raw reply [flat|nested] 31+ messages in thread* Re: [PATCH v20 1/2] scsi: ufs: Enable power management for wlun
2021-04-20 4:15 ` Adrian Hunter
(?)
@ 2021-04-20 7:42 ` Adrian Hunter
-1 siblings, 0 replies; 31+ messages in thread
From: Adrian Hunter @ 2021-04-20 7:42 UTC (permalink / raw)
To: Asutosh Das (asd), cang, martin.petersen, linux-scsi
Cc: linux-arm-msm, Alim Akhtar, Avri Altman, James E.J. Bottomley,
Krzysztof Kozlowski, Stanley Chu, Andy Gross, Bjorn Andersson,
Steven Rostedt, Ingo Molnar, Matthias Brugger, Lee Jones,
Bean Huo, Kiwoong Kim, Colin Ian King, Wei Yongjun, Yue Hu,
Bart van Assche, Gustavo A. R. Silva, Dinghao Liu, Jaegeuk Kim,
Satya Tangirala, open list, moderated list:ARM/SAMSUNG S3C,
S5P AND EXYNOS ARM ARCHITECTURES, open list:ARM/SAMSUNG S3C,
S5P AND EXYNOS ARM ARCHITECTURES,
moderated list:UNIVERSAL FLASH STORAGE HOST CONTROLLER DRIVER...
On 20/04/21 7:15 am, Adrian Hunter wrote:
> On 20/04/21 12:53 am, Asutosh Das (asd) wrote:
>> On 4/19/2021 11:37 AM, Adrian Hunter wrote:
>>> On 16/04/21 10:49 pm, Asutosh Das wrote:
>>>>
>>>> Co-developed-by: Can Guo <cang@codeaurora.org>
>>>> Signed-off-by: Can Guo <cang@codeaurora.org>
>>>> Signed-off-by: Asutosh Das <asutoshd@codeaurora.org>
>>>> ---
>>>
>>> I came across 3 issues while testing. See comments below.
>>>
>> Hi Adrian
>> Thanks for the comments.
>>> <SNIP>
>>>
>>>> @@ -5794,7 +5839,7 @@ static void ufshcd_err_handling_unprepare(struct ufs_hba *hba)
>>>> if (ufshcd_is_clkscaling_supported(hba))
>>>> ufshcd_clk_scaling_suspend(hba, false);
>>>> ufshcd_clear_ua_wluns(hba);
>>>
>>> ufshcd_clear_ua_wluns() deadlocks trying to clear UFS_UPIU_RPMB_WLUN
>>> if sdev_rpmb is suspended and sdev_ufs_device is suspending.
>>> e.g. ufshcd_wl_suspend() is waiting on host_sem while ufshcd_err_handler()
>>> is running, at which point sdev_rpmb has already suspended.
>>>
>> Umm, I didn't understand this deadlock.
>> When you say, sdev_rpmb is suspended, does it mean runtime_suspended?
>> sdev_ufs_device is suspending - this can't be runtime_suspending, while ufshcd_err_handling_unprepare is running.
>>
>> If you've a call-stack of this deadlock, please can you share it with me. I'll also try to reproduce this.
>
> Yes it is system suspend. sdev_rpmb has suspended, sdev_ufs_device is waiting on host_sem.
> ufshcd_err_handler() holds host_sem. ufshcd_clear_ua_wlun(UFS_UPIU_RPMB_WLUN) gets stuck.
> I will get some call-stacks.
Here are the call stacks
[ 34.094321] Workqueue: ufs_eh_wq_0 ufshcd_err_handler
[ 34.094788] Call Trace:
[ 34.095281] __schedule+0x275/0x6c0
[ 34.095743] schedule+0x41/0xa0
[ 34.096240] blk_queue_enter+0x10d/0x230
[ 34.096693] ? wait_woken+0x70/0x70
[ 34.097167] blk_mq_alloc_request+0x53/0xc0
[ 34.097610] blk_get_request+0x1e/0x60
[ 34.098053] __scsi_execute+0x3c/0x260
[ 34.098529] ufshcd_clear_ua_wlun.cold+0xa6/0x14b
[ 34.098977] ufshcd_clear_ua_wluns.part.0+0x4d/0x92
[ 34.099456] ufshcd_err_handler+0x97a/0x9ff
[ 34.099902] process_one_work+0x1cc/0x360
[ 34.100384] worker_thread+0x45/0x3b0
[ 34.100851] ? process_one_work+0x360/0x360
[ 34.101308] kthread+0xf6/0x130
[ 34.101728] ? kthread_park+0x80/0x80
[ 34.102186] ret_from_fork+0x1f/0x30
[ 34.640751] task:kworker/u10:9 state:D stack:14528 pid: 255 ppid: 2 flags:0x00004000
[ 34.641253] Workqueue: events_unbound async_run_entry_fn
[ 34.641722] Call Trace:
[ 34.642217] __schedule+0x275/0x6c0
[ 34.642683] schedule+0x41/0xa0
[ 34.643179] schedule_timeout+0x18b/0x290
[ 34.643645] ? del_timer_sync+0x30/0x30
[ 34.644131] __down_timeout+0x6b/0xc0
[ 34.644568] ? ufshcd_clkscale_enable_show+0x20/0x20
[ 34.645014] ? async_schedule_node_domain+0x17d/0x190
[ 34.645496] down_timeout+0x42/0x50
[ 34.645947] ufshcd_wl_suspend+0x79/0xa0
[ 34.646432] ? scmd_printk+0x100/0x100
[ 34.646917] scsi_bus_suspend_common+0x56/0xc0
[ 34.647405] ? scsi_bus_freeze+0x10/0x10
[ 34.647858] dpm_run_callback+0x45/0x110
[ 34.648347] __device_suspend+0x117/0x460
[ 34.648788] async_suspend+0x16/0x90
[ 34.649251] async_run_entry_fn+0x26/0x110
[ 34.649676] process_one_work+0x1cc/0x360
[ 34.650137] worker_thread+0x45/0x3b0
[ 34.650563] ? process_one_work+0x360/0x360
[ 34.650994] kthread+0xf6/0x130
[ 34.651455] ? kthread_park+0x80/0x80
[ 34.651882] ret_from_fork+0x1f/0x30
>
>>
>> I'll address the other comments in the next version.
>>
>>
>> Thank you!
>>
>>>> - pm_runtime_put(hba->dev);
>>>> + ufshcd_rpm_put(hba);
>>>> }
>>>
>>> <SNIP>
>>>
>>>> +void ufshcd_resume_complete(struct device *dev)
>>>> +{
>>
>
^ permalink raw reply [flat|nested] 31+ messages in thread* Re: [PATCH v20 1/2] scsi: ufs: Enable power management for wlun
@ 2021-04-20 7:42 ` Adrian Hunter
0 siblings, 0 replies; 31+ messages in thread
From: Adrian Hunter @ 2021-04-20 7:42 UTC (permalink / raw)
To: Asutosh Das (asd), cang, martin.petersen, linux-scsi
Cc: linux-arm-msm, Alim Akhtar, Avri Altman, James E.J. Bottomley,
Krzysztof Kozlowski, Stanley Chu, Andy Gross, Bjorn Andersson,
Steven Rostedt, Ingo Molnar, Matthias Brugger, Lee Jones,
Bean Huo, Kiwoong Kim, Colin Ian King, Wei Yongjun, Yue Hu,
Bart van Assche, Gustavo A. R. Silva, Dinghao Liu, Jaegeuk Kim,
Satya Tangirala, open list, moderated list:ARM/SAMSUNG S3C,
S5P AND EXYNOS ARM ARCHITECTURES, open list:ARM/SAMSUNG S3C,
S5P AND EXYNOS ARM ARCHITECTURES,
moderated list:UNIVERSAL FLASH STORAGE HOST CONTROLLER DRIVER...
On 20/04/21 7:15 am, Adrian Hunter wrote:
> On 20/04/21 12:53 am, Asutosh Das (asd) wrote:
>> On 4/19/2021 11:37 AM, Adrian Hunter wrote:
>>> On 16/04/21 10:49 pm, Asutosh Das wrote:
>>>>
>>>> Co-developed-by: Can Guo <cang@codeaurora.org>
>>>> Signed-off-by: Can Guo <cang@codeaurora.org>
>>>> Signed-off-by: Asutosh Das <asutoshd@codeaurora.org>
>>>> ---
>>>
>>> I came across 3 issues while testing. See comments below.
>>>
>> Hi Adrian
>> Thanks for the comments.
>>> <SNIP>
>>>
>>>> @@ -5794,7 +5839,7 @@ static void ufshcd_err_handling_unprepare(struct ufs_hba *hba)
>>>> if (ufshcd_is_clkscaling_supported(hba))
>>>> ufshcd_clk_scaling_suspend(hba, false);
>>>> ufshcd_clear_ua_wluns(hba);
>>>
>>> ufshcd_clear_ua_wluns() deadlocks trying to clear UFS_UPIU_RPMB_WLUN
>>> if sdev_rpmb is suspended and sdev_ufs_device is suspending.
>>> e.g. ufshcd_wl_suspend() is waiting on host_sem while ufshcd_err_handler()
>>> is running, at which point sdev_rpmb has already suspended.
>>>
>> Umm, I didn't understand this deadlock.
>> When you say, sdev_rpmb is suspended, does it mean runtime_suspended?
>> sdev_ufs_device is suspending - this can't be runtime_suspending, while ufshcd_err_handling_unprepare is running.
>>
>> If you've a call-stack of this deadlock, please can you share it with me. I'll also try to reproduce this.
>
> Yes it is system suspend. sdev_rpmb has suspended, sdev_ufs_device is waiting on host_sem.
> ufshcd_err_handler() holds host_sem. ufshcd_clear_ua_wlun(UFS_UPIU_RPMB_WLUN) gets stuck.
> I will get some call-stacks.
Here are the call stacks
[ 34.094321] Workqueue: ufs_eh_wq_0 ufshcd_err_handler
[ 34.094788] Call Trace:
[ 34.095281] __schedule+0x275/0x6c0
[ 34.095743] schedule+0x41/0xa0
[ 34.096240] blk_queue_enter+0x10d/0x230
[ 34.096693] ? wait_woken+0x70/0x70
[ 34.097167] blk_mq_alloc_request+0x53/0xc0
[ 34.097610] blk_get_request+0x1e/0x60
[ 34.098053] __scsi_execute+0x3c/0x260
[ 34.098529] ufshcd_clear_ua_wlun.cold+0xa6/0x14b
[ 34.098977] ufshcd_clear_ua_wluns.part.0+0x4d/0x92
[ 34.099456] ufshcd_err_handler+0x97a/0x9ff
[ 34.099902] process_one_work+0x1cc/0x360
[ 34.100384] worker_thread+0x45/0x3b0
[ 34.100851] ? process_one_work+0x360/0x360
[ 34.101308] kthread+0xf6/0x130
[ 34.101728] ? kthread_park+0x80/0x80
[ 34.102186] ret_from_fork+0x1f/0x30
[ 34.640751] task:kworker/u10:9 state:D stack:14528 pid: 255 ppid: 2 flags:0x00004000
[ 34.641253] Workqueue: events_unbound async_run_entry_fn
[ 34.641722] Call Trace:
[ 34.642217] __schedule+0x275/0x6c0
[ 34.642683] schedule+0x41/0xa0
[ 34.643179] schedule_timeout+0x18b/0x290
[ 34.643645] ? del_timer_sync+0x30/0x30
[ 34.644131] __down_timeout+0x6b/0xc0
[ 34.644568] ? ufshcd_clkscale_enable_show+0x20/0x20
[ 34.645014] ? async_schedule_node_domain+0x17d/0x190
[ 34.645496] down_timeout+0x42/0x50
[ 34.645947] ufshcd_wl_suspend+0x79/0xa0
[ 34.646432] ? scmd_printk+0x100/0x100
[ 34.646917] scsi_bus_suspend_common+0x56/0xc0
[ 34.647405] ? scsi_bus_freeze+0x10/0x10
[ 34.647858] dpm_run_callback+0x45/0x110
[ 34.648347] __device_suspend+0x117/0x460
[ 34.648788] async_suspend+0x16/0x90
[ 34.649251] async_run_entry_fn+0x26/0x110
[ 34.649676] process_one_work+0x1cc/0x360
[ 34.650137] worker_thread+0x45/0x3b0
[ 34.650563] ? process_one_work+0x360/0x360
[ 34.650994] kthread+0xf6/0x130
[ 34.651455] ? kthread_park+0x80/0x80
[ 34.651882] ret_from_fork+0x1f/0x30
>
>>
>> I'll address the other comments in the next version.
>>
>>
>> Thank you!
>>
>>>> - pm_runtime_put(hba->dev);
>>>> + ufshcd_rpm_put(hba);
>>>> }
>>>
>>> <SNIP>
>>>
>>>> +void ufshcd_resume_complete(struct device *dev)
>>>> +{
>>
>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 31+ messages in thread* Re: [PATCH v20 1/2] scsi: ufs: Enable power management for wlun
@ 2021-04-20 7:42 ` Adrian Hunter
0 siblings, 0 replies; 31+ messages in thread
From: Adrian Hunter @ 2021-04-20 7:42 UTC (permalink / raw)
To: Asutosh Das (asd), cang, martin.petersen, linux-scsi
Cc: linux-arm-msm, Alim Akhtar, Avri Altman, James E.J. Bottomley,
Krzysztof Kozlowski, Stanley Chu, Andy Gross, Bjorn Andersson,
Steven Rostedt, Ingo Molnar, Matthias Brugger, Lee Jones,
Bean Huo, Kiwoong Kim, Colin Ian King, Wei Yongjun, Yue Hu,
Bart van Assche, Gustavo A. R. Silva, Dinghao Liu, Jaegeuk Kim,
Satya Tangirala, open list, moderated list:ARM/SAMSUNG S3C,
S5P AND EXYNOS ARM ARCHITECTURES, open list:ARM/SAMSUNG S3C,
S5P AND EXYNOS ARM ARCHITECTURES,
moderated list:UNIVERSAL FLASH STORAGE HOST CONTROLLER DRIVER...
On 20/04/21 7:15 am, Adrian Hunter wrote:
> On 20/04/21 12:53 am, Asutosh Das (asd) wrote:
>> On 4/19/2021 11:37 AM, Adrian Hunter wrote:
>>> On 16/04/21 10:49 pm, Asutosh Das wrote:
>>>>
>>>> Co-developed-by: Can Guo <cang@codeaurora.org>
>>>> Signed-off-by: Can Guo <cang@codeaurora.org>
>>>> Signed-off-by: Asutosh Das <asutoshd@codeaurora.org>
>>>> ---
>>>
>>> I came across 3 issues while testing. See comments below.
>>>
>> Hi Adrian
>> Thanks for the comments.
>>> <SNIP>
>>>
>>>> @@ -5794,7 +5839,7 @@ static void ufshcd_err_handling_unprepare(struct ufs_hba *hba)
>>>> if (ufshcd_is_clkscaling_supported(hba))
>>>> ufshcd_clk_scaling_suspend(hba, false);
>>>> ufshcd_clear_ua_wluns(hba);
>>>
>>> ufshcd_clear_ua_wluns() deadlocks trying to clear UFS_UPIU_RPMB_WLUN
>>> if sdev_rpmb is suspended and sdev_ufs_device is suspending.
>>> e.g. ufshcd_wl_suspend() is waiting on host_sem while ufshcd_err_handler()
>>> is running, at which point sdev_rpmb has already suspended.
>>>
>> Umm, I didn't understand this deadlock.
>> When you say, sdev_rpmb is suspended, does it mean runtime_suspended?
>> sdev_ufs_device is suspending - this can't be runtime_suspending, while ufshcd_err_handling_unprepare is running.
>>
>> If you've a call-stack of this deadlock, please can you share it with me. I'll also try to reproduce this.
>
> Yes it is system suspend. sdev_rpmb has suspended, sdev_ufs_device is waiting on host_sem.
> ufshcd_err_handler() holds host_sem. ufshcd_clear_ua_wlun(UFS_UPIU_RPMB_WLUN) gets stuck.
> I will get some call-stacks.
Here are the call stacks
[ 34.094321] Workqueue: ufs_eh_wq_0 ufshcd_err_handler
[ 34.094788] Call Trace:
[ 34.095281] __schedule+0x275/0x6c0
[ 34.095743] schedule+0x41/0xa0
[ 34.096240] blk_queue_enter+0x10d/0x230
[ 34.096693] ? wait_woken+0x70/0x70
[ 34.097167] blk_mq_alloc_request+0x53/0xc0
[ 34.097610] blk_get_request+0x1e/0x60
[ 34.098053] __scsi_execute+0x3c/0x260
[ 34.098529] ufshcd_clear_ua_wlun.cold+0xa6/0x14b
[ 34.098977] ufshcd_clear_ua_wluns.part.0+0x4d/0x92
[ 34.099456] ufshcd_err_handler+0x97a/0x9ff
[ 34.099902] process_one_work+0x1cc/0x360
[ 34.100384] worker_thread+0x45/0x3b0
[ 34.100851] ? process_one_work+0x360/0x360
[ 34.101308] kthread+0xf6/0x130
[ 34.101728] ? kthread_park+0x80/0x80
[ 34.102186] ret_from_fork+0x1f/0x30
[ 34.640751] task:kworker/u10:9 state:D stack:14528 pid: 255 ppid: 2 flags:0x00004000
[ 34.641253] Workqueue: events_unbound async_run_entry_fn
[ 34.641722] Call Trace:
[ 34.642217] __schedule+0x275/0x6c0
[ 34.642683] schedule+0x41/0xa0
[ 34.643179] schedule_timeout+0x18b/0x290
[ 34.643645] ? del_timer_sync+0x30/0x30
[ 34.644131] __down_timeout+0x6b/0xc0
[ 34.644568] ? ufshcd_clkscale_enable_show+0x20/0x20
[ 34.645014] ? async_schedule_node_domain+0x17d/0x190
[ 34.645496] down_timeout+0x42/0x50
[ 34.645947] ufshcd_wl_suspend+0x79/0xa0
[ 34.646432] ? scmd_printk+0x100/0x100
[ 34.646917] scsi_bus_suspend_common+0x56/0xc0
[ 34.647405] ? scsi_bus_freeze+0x10/0x10
[ 34.647858] dpm_run_callback+0x45/0x110
[ 34.648347] __device_suspend+0x117/0x460
[ 34.648788] async_suspend+0x16/0x90
[ 34.649251] async_run_entry_fn+0x26/0x110
[ 34.649676] process_one_work+0x1cc/0x360
[ 34.650137] worker_thread+0x45/0x3b0
[ 34.650563] ? process_one_work+0x360/0x360
[ 34.650994] kthread+0xf6/0x130
[ 34.651455] ? kthread_park+0x80/0x80
[ 34.651882] ret_from_fork+0x1f/0x30
>
>>
>> I'll address the other comments in the next version.
>>
>>
>> Thank you!
>>
>>>> - pm_runtime_put(hba->dev);
>>>> + ufshcd_rpm_put(hba);
>>>> }
>>>
>>> <SNIP>
>>>
>>>> +void ufshcd_resume_complete(struct device *dev)
>>>> +{
>>
>
_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek
^ permalink raw reply [flat|nested] 31+ messages in thread* Re: [PATCH v20 1/2] scsi: ufs: Enable power management for wlun
2021-04-20 7:42 ` Adrian Hunter
@ 2021-04-22 16:38 ` Asutosh Das (asd)
-1 siblings, 0 replies; 31+ messages in thread
From: Asutosh Das (asd) @ 2021-04-22 16:38 UTC (permalink / raw)
To: Adrian Hunter, cang, martin.petersen, linux-scsi
Cc: linux-arm-msm, Alim Akhtar, Avri Altman, James E.J. Bottomley,
Krzysztof Kozlowski, Stanley Chu, Andy Gross, Bjorn Andersson,
Steven Rostedt, Ingo Molnar, Matthias Brugger, Lee Jones,
Bean Huo, Kiwoong Kim, Colin Ian King, Wei Yongjun, Yue Hu,
Bart van Assche, Gustavo A. R. Silva, Dinghao Liu, Jaegeuk Kim,
Satya Tangirala, open list, moderated list:ARM/SAMSUNG S3C,
S5P AND EXYNOS ARM ARCHITECTURES, open list:ARM/SAMSUNG S3C,
S5P AND EXYNOS ARM ARCHITECTURES,
moderated list:UNIVERSAL FLASH STORAGE HOST CONTROLLER DRIVER...
On 4/20/2021 12:42 AM, Adrian Hunter wrote:
> On 20/04/21 7:15 am, Adrian Hunter wrote:
>> On 20/04/21 12:53 am, Asutosh Das (asd) wrote:
>>> On 4/19/2021 11:37 AM, Adrian Hunter wrote:
>>>> On 16/04/21 10:49 pm, Asutosh Das wrote:
>>>>>
>>>>> Co-developed-by: Can Guo <cang@codeaurora.org>
>>>>> Signed-off-by: Can Guo <cang@codeaurora.org>
>>>>> Signed-off-by: Asutosh Das <asutoshd@codeaurora.org>
>>>>> ---
>>>>
>>>> I came across 3 issues while testing. See comments below.
>>>>
>>> Hi Adrian
>>> Thanks for the comments.
>>>> <SNIP>
>>>>
>>>>> @@ -5794,7 +5839,7 @@ static void ufshcd_err_handling_unprepare(struct ufs_hba *hba)
>>>>> if (ufshcd_is_clkscaling_supported(hba))
>>>>> ufshcd_clk_scaling_suspend(hba, false);
>>>>> ufshcd_clear_ua_wluns(hba);
>>>>
>>>> ufshcd_clear_ua_wluns() deadlocks trying to clear UFS_UPIU_RPMB_WLUN
>>>> if sdev_rpmb is suspended and sdev_ufs_device is suspending.
>>>> e.g. ufshcd_wl_suspend() is waiting on host_sem while ufshcd_err_handler()
>>>> is running, at which point sdev_rpmb has already suspended.
>>>>
>>> Umm, I didn't understand this deadlock.
>>> When you say, sdev_rpmb is suspended, does it mean runtime_suspended?
>>> sdev_ufs_device is suspending - this can't be runtime_suspending, while ufshcd_err_handling_unprepare is running.
>>>
>>> If you've a call-stack of this deadlock, please can you share it with me. I'll also try to reproduce this.
>>
>> Yes it is system suspend. sdev_rpmb has suspended, sdev_ufs_device is waiting on host_sem.
>> ufshcd_err_handler() holds host_sem. ufshcd_clear_ua_wlun(UFS_UPIU_RPMB_WLUN) gets stuck.
>> I will get some call-stacks.
>
Hi Adrian,
Thanks for the call stacks.
From the current information, I can't say for sure why it'd get stuck
in blk_queue_enter().
I tried reproducing this issue on my setup yesterday but couldn't.
Here's what I did:
1. sdev_rpmb is RPM_SUSPENDED, checked before initiating system suspend
2. sdev_ufs_device is RPM_RESUMED
3. I triggered system suspend (echo mem > /sys/power/state) and
scheduled the error handler from ufshcd_wl_suspend().
4. Waited until error handler ran and then ufshcd_wl_suspend() blocks on
host_sem.
5. The ufshcd_clear_wa_wlun(UFS_UPIU_RPMB_WLUN) went through fine.
Do you've some specific steps to reproduce this or a script, perhaps? If
so, please can you share it with me. I will try again.
My test environment is in 5.10 kernel with Android, I suppose that
should be ok though.
Thanks
-asd
> Here are the call stacks
>
> [ 34.094321] Workqueue: ufs_eh_wq_0 ufshcd_err_handler
> [ 34.094788] Call Trace:
> [ 34.095281] __schedule+0x275/0x6c0
> [ 34.095743] schedule+0x41/0xa0
> [ 34.096240] blk_queue_enter+0x10d/0x230
> [ 34.096693] ? wait_woken+0x70/0x70
> [ 34.097167] blk_mq_alloc_request+0x53/0xc0
> [ 34.097610] blk_get_request+0x1e/0x60
> [ 34.098053] __scsi_execute+0x3c/0x260
> [ 34.098529] ufshcd_clear_ua_wlun.cold+0xa6/0x14b
> [ 34.098977] ufshcd_clear_ua_wluns.part.0+0x4d/0x92
> [ 34.099456] ufshcd_err_handler+0x97a/0x9ff
> [ 34.099902] process_one_work+0x1cc/0x360
> [ 34.100384] worker_thread+0x45/0x3b0
> [ 34.100851] ? process_one_work+0x360/0x360
> [ 34.101308] kthread+0xf6/0x130
> [ 34.101728] ? kthread_park+0x80/0x80
> [ 34.102186] ret_from_fork+0x1f/0x30
>
> [ 34.640751] task:kworker/u10:9 state:D stack:14528 pid: 255 ppid: 2 flags:0x00004000
> [ 34.641253] Workqueue: events_unbound async_run_entry_fn
> [ 34.641722] Call Trace:
> [ 34.642217] __schedule+0x275/0x6c0
> [ 34.642683] schedule+0x41/0xa0
> [ 34.643179] schedule_timeout+0x18b/0x290
> [ 34.643645] ? del_timer_sync+0x30/0x30
> [ 34.644131] __down_timeout+0x6b/0xc0
> [ 34.644568] ? ufshcd_clkscale_enable_show+0x20/0x20
> [ 34.645014] ? async_schedule_node_domain+0x17d/0x190
> [ 34.645496] down_timeout+0x42/0x50
> [ 34.645947] ufshcd_wl_suspend+0x79/0xa0
> [ 34.646432] ? scmd_printk+0x100/0x100
> [ 34.646917] scsi_bus_suspend_common+0x56/0xc0
> [ 34.647405] ? scsi_bus_freeze+0x10/0x10
> [ 34.647858] dpm_run_callback+0x45/0x110
> [ 34.648347] __device_suspend+0x117/0x460
> [ 34.648788] async_suspend+0x16/0x90
> [ 34.649251] async_run_entry_fn+0x26/0x110
> [ 34.649676] process_one_work+0x1cc/0x360
> [ 34.650137] worker_thread+0x45/0x3b0
> [ 34.650563] ? process_one_work+0x360/0x360
> [ 34.650994] kthread+0xf6/0x130
> [ 34.651455] ? kthread_park+0x80/0x80
> [ 34.651882] ret_from_fork+0x1f/0x30
>
>
>
>>
>>>
>>> I'll address the other comments in the next version.
>>>
>>>
>>> Thank you!
>>>
>>>>> - pm_runtime_put(hba->dev);
>>>>> + ufshcd_rpm_put(hba);
>>>>> }
>>>>
>>>> <SNIP>
>>>>
>>>>> +void ufshcd_resume_complete(struct device *dev)
>>>>> +{
>>>
>>
>
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
Linux Foundation Collaborative Project
^ permalink raw reply [flat|nested] 31+ messages in thread* Re: [PATCH v20 1/2] scsi: ufs: Enable power management for wlun
@ 2021-04-22 16:38 ` Asutosh Das (asd)
0 siblings, 0 replies; 31+ messages in thread
From: Asutosh Das (asd) @ 2021-04-22 16:38 UTC (permalink / raw)
To: Adrian Hunter, cang, martin.petersen, linux-scsi
Cc: linux-arm-msm, Alim Akhtar, Avri Altman, James E.J. Bottomley,
Krzysztof Kozlowski, Stanley Chu, Andy Gross, Bjorn Andersson,
Steven Rostedt, Ingo Molnar, Matthias Brugger, Lee Jones,
Bean Huo, Kiwoong Kim, Colin Ian King, Wei Yongjun, Yue Hu,
Bart van Assche, Gustavo A. R. Silva, Dinghao Liu, Jaegeuk Kim,
Satya Tangirala, open list, moderated list:ARM/SAMSUNG S3C,
S5P AND EXYNOS ARM ARCHITECTURES, open list:ARM/SAMSUNG S3C,
S5P AND EXYNOS ARM ARCHITECTURES,
moderated list:UNIVERSAL FLASH STORAGE HOST CONTROLLER DRIVER...
On 4/20/2021 12:42 AM, Adrian Hunter wrote:
> On 20/04/21 7:15 am, Adrian Hunter wrote:
>> On 20/04/21 12:53 am, Asutosh Das (asd) wrote:
>>> On 4/19/2021 11:37 AM, Adrian Hunter wrote:
>>>> On 16/04/21 10:49 pm, Asutosh Das wrote:
>>>>>
>>>>> Co-developed-by: Can Guo <cang@codeaurora.org>
>>>>> Signed-off-by: Can Guo <cang@codeaurora.org>
>>>>> Signed-off-by: Asutosh Das <asutoshd@codeaurora.org>
>>>>> ---
>>>>
>>>> I came across 3 issues while testing. See comments below.
>>>>
>>> Hi Adrian
>>> Thanks for the comments.
>>>> <SNIP>
>>>>
>>>>> @@ -5794,7 +5839,7 @@ static void ufshcd_err_handling_unprepare(struct ufs_hba *hba)
>>>>> if (ufshcd_is_clkscaling_supported(hba))
>>>>> ufshcd_clk_scaling_suspend(hba, false);
>>>>> ufshcd_clear_ua_wluns(hba);
>>>>
>>>> ufshcd_clear_ua_wluns() deadlocks trying to clear UFS_UPIU_RPMB_WLUN
>>>> if sdev_rpmb is suspended and sdev_ufs_device is suspending.
>>>> e.g. ufshcd_wl_suspend() is waiting on host_sem while ufshcd_err_handler()
>>>> is running, at which point sdev_rpmb has already suspended.
>>>>
>>> Umm, I didn't understand this deadlock.
>>> When you say, sdev_rpmb is suspended, does it mean runtime_suspended?
>>> sdev_ufs_device is suspending - this can't be runtime_suspending, while ufshcd_err_handling_unprepare is running.
>>>
>>> If you've a call-stack of this deadlock, please can you share it with me. I'll also try to reproduce this.
>>
>> Yes it is system suspend. sdev_rpmb has suspended, sdev_ufs_device is waiting on host_sem.
>> ufshcd_err_handler() holds host_sem. ufshcd_clear_ua_wlun(UFS_UPIU_RPMB_WLUN) gets stuck.
>> I will get some call-stacks.
>
Hi Adrian,
Thanks for the call stacks.
From the current information, I can't say for sure why it'd get stuck
in blk_queue_enter().
I tried reproducing this issue on my setup yesterday but couldn't.
Here's what I did:
1. sdev_rpmb is RPM_SUSPENDED, checked before initiating system suspend
2. sdev_ufs_device is RPM_RESUMED
3. I triggered system suspend (echo mem > /sys/power/state) and
scheduled the error handler from ufshcd_wl_suspend().
4. Waited until error handler ran and then ufshcd_wl_suspend() blocks on
host_sem.
5. The ufshcd_clear_wa_wlun(UFS_UPIU_RPMB_WLUN) went through fine.
Do you've some specific steps to reproduce this or a script, perhaps? If
so, please can you share it with me. I will try again.
My test environment is in 5.10 kernel with Android, I suppose that
should be ok though.
Thanks
-asd
> Here are the call stacks
>
> [ 34.094321] Workqueue: ufs_eh_wq_0 ufshcd_err_handler
> [ 34.094788] Call Trace:
> [ 34.095281] __schedule+0x275/0x6c0
> [ 34.095743] schedule+0x41/0xa0
> [ 34.096240] blk_queue_enter+0x10d/0x230
> [ 34.096693] ? wait_woken+0x70/0x70
> [ 34.097167] blk_mq_alloc_request+0x53/0xc0
> [ 34.097610] blk_get_request+0x1e/0x60
> [ 34.098053] __scsi_execute+0x3c/0x260
> [ 34.098529] ufshcd_clear_ua_wlun.cold+0xa6/0x14b
> [ 34.098977] ufshcd_clear_ua_wluns.part.0+0x4d/0x92
> [ 34.099456] ufshcd_err_handler+0x97a/0x9ff
> [ 34.099902] process_one_work+0x1cc/0x360
> [ 34.100384] worker_thread+0x45/0x3b0
> [ 34.100851] ? process_one_work+0x360/0x360
> [ 34.101308] kthread+0xf6/0x130
> [ 34.101728] ? kthread_park+0x80/0x80
> [ 34.102186] ret_from_fork+0x1f/0x30
>
> [ 34.640751] task:kworker/u10:9 state:D stack:14528 pid: 255 ppid: 2 flags:0x00004000
> [ 34.641253] Workqueue: events_unbound async_run_entry_fn
> [ 34.641722] Call Trace:
> [ 34.642217] __schedule+0x275/0x6c0
> [ 34.642683] schedule+0x41/0xa0
> [ 34.643179] schedule_timeout+0x18b/0x290
> [ 34.643645] ? del_timer_sync+0x30/0x30
> [ 34.644131] __down_timeout+0x6b/0xc0
> [ 34.644568] ? ufshcd_clkscale_enable_show+0x20/0x20
> [ 34.645014] ? async_schedule_node_domain+0x17d/0x190
> [ 34.645496] down_timeout+0x42/0x50
> [ 34.645947] ufshcd_wl_suspend+0x79/0xa0
> [ 34.646432] ? scmd_printk+0x100/0x100
> [ 34.646917] scsi_bus_suspend_common+0x56/0xc0
> [ 34.647405] ? scsi_bus_freeze+0x10/0x10
> [ 34.647858] dpm_run_callback+0x45/0x110
> [ 34.648347] __device_suspend+0x117/0x460
> [ 34.648788] async_suspend+0x16/0x90
> [ 34.649251] async_run_entry_fn+0x26/0x110
> [ 34.649676] process_one_work+0x1cc/0x360
> [ 34.650137] worker_thread+0x45/0x3b0
> [ 34.650563] ? process_one_work+0x360/0x360
> [ 34.650994] kthread+0xf6/0x130
> [ 34.651455] ? kthread_park+0x80/0x80
> [ 34.651882] ret_from_fork+0x1f/0x30
>
>
>
>>
>>>
>>> I'll address the other comments in the next version.
>>>
>>>
>>> Thank you!
>>>
>>>>> - pm_runtime_put(hba->dev);
>>>>> + ufshcd_rpm_put(hba);
>>>>> }
>>>>
>>>> <SNIP>
>>>>
>>>>> +void ufshcd_resume_complete(struct device *dev)
>>>>> +{
>>>
>>
>
--
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
^ permalink raw reply [flat|nested] 31+ messages in thread* Re: [PATCH v20 1/2] scsi: ufs: Enable power management for wlun
2021-04-22 16:38 ` Asutosh Das (asd)
(?)
@ 2021-04-23 4:23 ` Adrian Hunter
-1 siblings, 0 replies; 31+ messages in thread
From: Adrian Hunter @ 2021-04-23 4:23 UTC (permalink / raw)
To: Asutosh Das (asd), cang, martin.petersen, linux-scsi
Cc: linux-arm-msm, Alim Akhtar, Avri Altman, James E.J. Bottomley,
Krzysztof Kozlowski, Stanley Chu, Andy Gross, Bjorn Andersson,
Steven Rostedt, Ingo Molnar, Matthias Brugger, Lee Jones,
Bean Huo, Kiwoong Kim, Colin Ian King, Wei Yongjun, Yue Hu,
Bart van Assche, Gustavo A. R. Silva, Dinghao Liu, Jaegeuk Kim,
Satya Tangirala, open list, moderated list:ARM/SAMSUNG S3C,
S5P AND EXYNOS ARM ARCHITECTURES, open list:ARM/SAMSUNG S3C,
S5P AND EXYNOS ARM ARCHITECTURES,
moderated list:UNIVERSAL FLASH STORAGE HOST CONTROLLER DRIVER...
On 22/04/21 7:38 pm, Asutosh Das (asd) wrote:
> On 4/20/2021 12:42 AM, Adrian Hunter wrote:
>> On 20/04/21 7:15 am, Adrian Hunter wrote:
>>> On 20/04/21 12:53 am, Asutosh Das (asd) wrote:
>>>> On 4/19/2021 11:37 AM, Adrian Hunter wrote:
>>>>> On 16/04/21 10:49 pm, Asutosh Das wrote:
>>>>>>
>>>>>> Co-developed-by: Can Guo <cang@codeaurora.org>
>>>>>> Signed-off-by: Can Guo <cang@codeaurora.org>
>>>>>> Signed-off-by: Asutosh Das <asutoshd@codeaurora.org>
>>>>>> ---
>>>>>
>>>>> I came across 3 issues while testing. See comments below.
>>>>>
>>>> Hi Adrian
>>>> Thanks for the comments.
>>>>> <SNIP>
>>>>>
>>>>>> @@ -5794,7 +5839,7 @@ static void ufshcd_err_handling_unprepare(struct ufs_hba *hba)
>>>>>> if (ufshcd_is_clkscaling_supported(hba))
>>>>>> ufshcd_clk_scaling_suspend(hba, false);
>>>>>> ufshcd_clear_ua_wluns(hba);
>>>>>
>>>>> ufshcd_clear_ua_wluns() deadlocks trying to clear UFS_UPIU_RPMB_WLUN
>>>>> if sdev_rpmb is suspended and sdev_ufs_device is suspending.
>>>>> e.g. ufshcd_wl_suspend() is waiting on host_sem while ufshcd_err_handler()
>>>>> is running, at which point sdev_rpmb has already suspended.
>>>>>
>>>> Umm, I didn't understand this deadlock.
>>>> When you say, sdev_rpmb is suspended, does it mean runtime_suspended?
>>>> sdev_ufs_device is suspending - this can't be runtime_suspending, while ufshcd_err_handling_unprepare is running.
>>>>
>>>> If you've a call-stack of this deadlock, please can you share it with me. I'll also try to reproduce this.
>>>
>>> Yes it is system suspend. sdev_rpmb has suspended, sdev_ufs_device is waiting on host_sem.
>>> ufshcd_err_handler() holds host_sem. ufshcd_clear_ua_wlun(UFS_UPIU_RPMB_WLUN) gets stuck.
>>> I will get some call-stacks.
>>
> Hi Adrian,
>
> Thanks for the call stacks.
> From the current information, I can't say for sure why it'd get stuck in blk_queue_enter().
I presume SCSI is leaving the RPMB WLUN device runtime suspended and consequently the queue status is RPM_SUSPENDED
>
> I tried reproducing this issue on my setup yesterday but couldn't.
> Here's what I did:
> 1. sdev_rpmb is RPM_SUSPENDED, checked before initiating system suspend
> 2. sdev_ufs_device is RPM_RESUMED
> 3. I triggered system suspend (echo mem > /sys/power/state) and scheduled the error handler from ufshcd_wl_suspend().
> 4. Waited until error handler ran and then ufshcd_wl_suspend() blocks on host_sem.
> 5. The ufshcd_clear_wa_wlun(UFS_UPIU_RPMB_WLUN) went through fine.
>
> Do you've some specific steps to reproduce this or a script, perhaps? If so, please can you share it with me. I will try again.
I was using a device that gives occasional errors, but I will what see I can do.
> My test environment is in 5.10 kernel with Android, I suppose that should be ok though.
>
> Thanks
> -asd
>
>> Here are the call stacks
>>
>> [ 34.094321] Workqueue: ufs_eh_wq_0 ufshcd_err_handler
>> [ 34.094788] Call Trace:
>> [ 34.095281] __schedule+0x275/0x6c0
>> [ 34.095743] schedule+0x41/0xa0
>> [ 34.096240] blk_queue_enter+0x10d/0x230
>> [ 34.096693] ? wait_woken+0x70/0x70
>> [ 34.097167] blk_mq_alloc_request+0x53/0xc0
>> [ 34.097610] blk_get_request+0x1e/0x60
>> [ 34.098053] __scsi_execute+0x3c/0x260
>> [ 34.098529] ufshcd_clear_ua_wlun.cold+0xa6/0x14b
>> [ 34.098977] ufshcd_clear_ua_wluns.part.0+0x4d/0x92
>> [ 34.099456] ufshcd_err_handler+0x97a/0x9ff
>> [ 34.099902] process_one_work+0x1cc/0x360
>> [ 34.100384] worker_thread+0x45/0x3b0
>> [ 34.100851] ? process_one_work+0x360/0x360
>> [ 34.101308] kthread+0xf6/0x130
>> [ 34.101728] ? kthread_park+0x80/0x80
>> [ 34.102186] ret_from_fork+0x1f/0x30
>>
>> [ 34.640751] task:kworker/u10:9 state:D stack:14528 pid: 255 ppid: 2 flags:0x00004000
>> [ 34.641253] Workqueue: events_unbound async_run_entry_fn
>> [ 34.641722] Call Trace:
>> [ 34.642217] __schedule+0x275/0x6c0
>> [ 34.642683] schedule+0x41/0xa0
>> [ 34.643179] schedule_timeout+0x18b/0x290
>> [ 34.643645] ? del_timer_sync+0x30/0x30
>> [ 34.644131] __down_timeout+0x6b/0xc0
>> [ 34.644568] ? ufshcd_clkscale_enable_show+0x20/0x20
>> [ 34.645014] ? async_schedule_node_domain+0x17d/0x190
>> [ 34.645496] down_timeout+0x42/0x50
>> [ 34.645947] ufshcd_wl_suspend+0x79/0xa0
>> [ 34.646432] ? scmd_printk+0x100/0x100
>> [ 34.646917] scsi_bus_suspend_common+0x56/0xc0
>> [ 34.647405] ? scsi_bus_freeze+0x10/0x10
>> [ 34.647858] dpm_run_callback+0x45/0x110
>> [ 34.648347] __device_suspend+0x117/0x460
>> [ 34.648788] async_suspend+0x16/0x90
>> [ 34.649251] async_run_entry_fn+0x26/0x110
>> [ 34.649676] process_one_work+0x1cc/0x360
>> [ 34.650137] worker_thread+0x45/0x3b0
>> [ 34.650563] ? process_one_work+0x360/0x360
>> [ 34.650994] kthread+0xf6/0x130
>> [ 34.651455] ? kthread_park+0x80/0x80
>> [ 34.651882] ret_from_fork+0x1f/0x30
>>
>>
>>
>>>
>>>>
>>>> I'll address the other comments in the next version.
>>>>
>>>>
>>>> Thank you!
>>>>
>>>>>> - pm_runtime_put(hba->dev);
>>>>>> + ufshcd_rpm_put(hba);
>>>>>> }
>>>>>
>>>>> <SNIP>
>>>>>
>>>>>> +void ufshcd_resume_complete(struct device *dev)
>>>>>> +{
>>>>
>>>
>>
>
>
^ permalink raw reply [flat|nested] 31+ messages in thread* Re: [PATCH v20 1/2] scsi: ufs: Enable power management for wlun
@ 2021-04-23 4:23 ` Adrian Hunter
0 siblings, 0 replies; 31+ messages in thread
From: Adrian Hunter @ 2021-04-23 4:23 UTC (permalink / raw)
To: Asutosh Das (asd), cang, martin.petersen, linux-scsi
Cc: linux-arm-msm, Alim Akhtar, Avri Altman, James E.J. Bottomley,
Krzysztof Kozlowski, Stanley Chu, Andy Gross, Bjorn Andersson,
Steven Rostedt, Ingo Molnar, Matthias Brugger, Lee Jones,
Bean Huo, Kiwoong Kim, Colin Ian King, Wei Yongjun, Yue Hu,
Bart van Assche, Gustavo A. R. Silva, Dinghao Liu, Jaegeuk Kim,
Satya Tangirala, open list, moderated list:ARM/SAMSUNG S3C,
S5P AND EXYNOS ARM ARCHITECTURES, open list:ARM/SAMSUNG S3C,
S5P AND EXYNOS ARM ARCHITECTURES,
moderated list:UNIVERSAL FLASH STORAGE HOST CONTROLLER DRIVER...
On 22/04/21 7:38 pm, Asutosh Das (asd) wrote:
> On 4/20/2021 12:42 AM, Adrian Hunter wrote:
>> On 20/04/21 7:15 am, Adrian Hunter wrote:
>>> On 20/04/21 12:53 am, Asutosh Das (asd) wrote:
>>>> On 4/19/2021 11:37 AM, Adrian Hunter wrote:
>>>>> On 16/04/21 10:49 pm, Asutosh Das wrote:
>>>>>>
>>>>>> Co-developed-by: Can Guo <cang@codeaurora.org>
>>>>>> Signed-off-by: Can Guo <cang@codeaurora.org>
>>>>>> Signed-off-by: Asutosh Das <asutoshd@codeaurora.org>
>>>>>> ---
>>>>>
>>>>> I came across 3 issues while testing. See comments below.
>>>>>
>>>> Hi Adrian
>>>> Thanks for the comments.
>>>>> <SNIP>
>>>>>
>>>>>> @@ -5794,7 +5839,7 @@ static void ufshcd_err_handling_unprepare(struct ufs_hba *hba)
>>>>>> if (ufshcd_is_clkscaling_supported(hba))
>>>>>> ufshcd_clk_scaling_suspend(hba, false);
>>>>>> ufshcd_clear_ua_wluns(hba);
>>>>>
>>>>> ufshcd_clear_ua_wluns() deadlocks trying to clear UFS_UPIU_RPMB_WLUN
>>>>> if sdev_rpmb is suspended and sdev_ufs_device is suspending.
>>>>> e.g. ufshcd_wl_suspend() is waiting on host_sem while ufshcd_err_handler()
>>>>> is running, at which point sdev_rpmb has already suspended.
>>>>>
>>>> Umm, I didn't understand this deadlock.
>>>> When you say, sdev_rpmb is suspended, does it mean runtime_suspended?
>>>> sdev_ufs_device is suspending - this can't be runtime_suspending, while ufshcd_err_handling_unprepare is running.
>>>>
>>>> If you've a call-stack of this deadlock, please can you share it with me. I'll also try to reproduce this.
>>>
>>> Yes it is system suspend. sdev_rpmb has suspended, sdev_ufs_device is waiting on host_sem.
>>> ufshcd_err_handler() holds host_sem. ufshcd_clear_ua_wlun(UFS_UPIU_RPMB_WLUN) gets stuck.
>>> I will get some call-stacks.
>>
> Hi Adrian,
>
> Thanks for the call stacks.
> From the current information, I can't say for sure why it'd get stuck in blk_queue_enter().
I presume SCSI is leaving the RPMB WLUN device runtime suspended and consequently the queue status is RPM_SUSPENDED
>
> I tried reproducing this issue on my setup yesterday but couldn't.
> Here's what I did:
> 1. sdev_rpmb is RPM_SUSPENDED, checked before initiating system suspend
> 2. sdev_ufs_device is RPM_RESUMED
> 3. I triggered system suspend (echo mem > /sys/power/state) and scheduled the error handler from ufshcd_wl_suspend().
> 4. Waited until error handler ran and then ufshcd_wl_suspend() blocks on host_sem.
> 5. The ufshcd_clear_wa_wlun(UFS_UPIU_RPMB_WLUN) went through fine.
>
> Do you've some specific steps to reproduce this or a script, perhaps? If so, please can you share it with me. I will try again.
I was using a device that gives occasional errors, but I will what see I can do.
> My test environment is in 5.10 kernel with Android, I suppose that should be ok though.
>
> Thanks
> -asd
>
>> Here are the call stacks
>>
>> [ 34.094321] Workqueue: ufs_eh_wq_0 ufshcd_err_handler
>> [ 34.094788] Call Trace:
>> [ 34.095281] __schedule+0x275/0x6c0
>> [ 34.095743] schedule+0x41/0xa0
>> [ 34.096240] blk_queue_enter+0x10d/0x230
>> [ 34.096693] ? wait_woken+0x70/0x70
>> [ 34.097167] blk_mq_alloc_request+0x53/0xc0
>> [ 34.097610] blk_get_request+0x1e/0x60
>> [ 34.098053] __scsi_execute+0x3c/0x260
>> [ 34.098529] ufshcd_clear_ua_wlun.cold+0xa6/0x14b
>> [ 34.098977] ufshcd_clear_ua_wluns.part.0+0x4d/0x92
>> [ 34.099456] ufshcd_err_handler+0x97a/0x9ff
>> [ 34.099902] process_one_work+0x1cc/0x360
>> [ 34.100384] worker_thread+0x45/0x3b0
>> [ 34.100851] ? process_one_work+0x360/0x360
>> [ 34.101308] kthread+0xf6/0x130
>> [ 34.101728] ? kthread_park+0x80/0x80
>> [ 34.102186] ret_from_fork+0x1f/0x30
>>
>> [ 34.640751] task:kworker/u10:9 state:D stack:14528 pid: 255 ppid: 2 flags:0x00004000
>> [ 34.641253] Workqueue: events_unbound async_run_entry_fn
>> [ 34.641722] Call Trace:
>> [ 34.642217] __schedule+0x275/0x6c0
>> [ 34.642683] schedule+0x41/0xa0
>> [ 34.643179] schedule_timeout+0x18b/0x290
>> [ 34.643645] ? del_timer_sync+0x30/0x30
>> [ 34.644131] __down_timeout+0x6b/0xc0
>> [ 34.644568] ? ufshcd_clkscale_enable_show+0x20/0x20
>> [ 34.645014] ? async_schedule_node_domain+0x17d/0x190
>> [ 34.645496] down_timeout+0x42/0x50
>> [ 34.645947] ufshcd_wl_suspend+0x79/0xa0
>> [ 34.646432] ? scmd_printk+0x100/0x100
>> [ 34.646917] scsi_bus_suspend_common+0x56/0xc0
>> [ 34.647405] ? scsi_bus_freeze+0x10/0x10
>> [ 34.647858] dpm_run_callback+0x45/0x110
>> [ 34.648347] __device_suspend+0x117/0x460
>> [ 34.648788] async_suspend+0x16/0x90
>> [ 34.649251] async_run_entry_fn+0x26/0x110
>> [ 34.649676] process_one_work+0x1cc/0x360
>> [ 34.650137] worker_thread+0x45/0x3b0
>> [ 34.650563] ? process_one_work+0x360/0x360
>> [ 34.650994] kthread+0xf6/0x130
>> [ 34.651455] ? kthread_park+0x80/0x80
>> [ 34.651882] ret_from_fork+0x1f/0x30
>>
>>
>>
>>>
>>>>
>>>> I'll address the other comments in the next version.
>>>>
>>>>
>>>> Thank you!
>>>>
>>>>>> - pm_runtime_put(hba->dev);
>>>>>> + ufshcd_rpm_put(hba);
>>>>>> }
>>>>>
>>>>> <SNIP>
>>>>>
>>>>>> +void ufshcd_resume_complete(struct device *dev)
>>>>>> +{
>>>>
>>>
>>
>
>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 31+ messages in thread* Re: [PATCH v20 1/2] scsi: ufs: Enable power management for wlun
@ 2021-04-23 4:23 ` Adrian Hunter
0 siblings, 0 replies; 31+ messages in thread
From: Adrian Hunter @ 2021-04-23 4:23 UTC (permalink / raw)
To: Asutosh Das (asd), cang, martin.petersen, linux-scsi
Cc: linux-arm-msm, Alim Akhtar, Avri Altman, James E.J. Bottomley,
Krzysztof Kozlowski, Stanley Chu, Andy Gross, Bjorn Andersson,
Steven Rostedt, Ingo Molnar, Matthias Brugger, Lee Jones,
Bean Huo, Kiwoong Kim, Colin Ian King, Wei Yongjun, Yue Hu,
Bart van Assche, Gustavo A. R. Silva, Dinghao Liu, Jaegeuk Kim,
Satya Tangirala, open list, moderated list:ARM/SAMSUNG S3C,
S5P AND EXYNOS ARM ARCHITECTURES, open list:ARM/SAMSUNG S3C,
S5P AND EXYNOS ARM ARCHITECTURES,
moderated list:UNIVERSAL FLASH STORAGE HOST CONTROLLER DRIVER...
On 22/04/21 7:38 pm, Asutosh Das (asd) wrote:
> On 4/20/2021 12:42 AM, Adrian Hunter wrote:
>> On 20/04/21 7:15 am, Adrian Hunter wrote:
>>> On 20/04/21 12:53 am, Asutosh Das (asd) wrote:
>>>> On 4/19/2021 11:37 AM, Adrian Hunter wrote:
>>>>> On 16/04/21 10:49 pm, Asutosh Das wrote:
>>>>>>
>>>>>> Co-developed-by: Can Guo <cang@codeaurora.org>
>>>>>> Signed-off-by: Can Guo <cang@codeaurora.org>
>>>>>> Signed-off-by: Asutosh Das <asutoshd@codeaurora.org>
>>>>>> ---
>>>>>
>>>>> I came across 3 issues while testing. See comments below.
>>>>>
>>>> Hi Adrian
>>>> Thanks for the comments.
>>>>> <SNIP>
>>>>>
>>>>>> @@ -5794,7 +5839,7 @@ static void ufshcd_err_handling_unprepare(struct ufs_hba *hba)
>>>>>> if (ufshcd_is_clkscaling_supported(hba))
>>>>>> ufshcd_clk_scaling_suspend(hba, false);
>>>>>> ufshcd_clear_ua_wluns(hba);
>>>>>
>>>>> ufshcd_clear_ua_wluns() deadlocks trying to clear UFS_UPIU_RPMB_WLUN
>>>>> if sdev_rpmb is suspended and sdev_ufs_device is suspending.
>>>>> e.g. ufshcd_wl_suspend() is waiting on host_sem while ufshcd_err_handler()
>>>>> is running, at which point sdev_rpmb has already suspended.
>>>>>
>>>> Umm, I didn't understand this deadlock.
>>>> When you say, sdev_rpmb is suspended, does it mean runtime_suspended?
>>>> sdev_ufs_device is suspending - this can't be runtime_suspending, while ufshcd_err_handling_unprepare is running.
>>>>
>>>> If you've a call-stack of this deadlock, please can you share it with me. I'll also try to reproduce this.
>>>
>>> Yes it is system suspend. sdev_rpmb has suspended, sdev_ufs_device is waiting on host_sem.
>>> ufshcd_err_handler() holds host_sem. ufshcd_clear_ua_wlun(UFS_UPIU_RPMB_WLUN) gets stuck.
>>> I will get some call-stacks.
>>
> Hi Adrian,
>
> Thanks for the call stacks.
> From the current information, I can't say for sure why it'd get stuck in blk_queue_enter().
I presume SCSI is leaving the RPMB WLUN device runtime suspended and consequently the queue status is RPM_SUSPENDED
>
> I tried reproducing this issue on my setup yesterday but couldn't.
> Here's what I did:
> 1. sdev_rpmb is RPM_SUSPENDED, checked before initiating system suspend
> 2. sdev_ufs_device is RPM_RESUMED
> 3. I triggered system suspend (echo mem > /sys/power/state) and scheduled the error handler from ufshcd_wl_suspend().
> 4. Waited until error handler ran and then ufshcd_wl_suspend() blocks on host_sem.
> 5. The ufshcd_clear_wa_wlun(UFS_UPIU_RPMB_WLUN) went through fine.
>
> Do you've some specific steps to reproduce this or a script, perhaps? If so, please can you share it with me. I will try again.
I was using a device that gives occasional errors, but I will what see I can do.
> My test environment is in 5.10 kernel with Android, I suppose that should be ok though.
>
> Thanks
> -asd
>
>> Here are the call stacks
>>
>> [ 34.094321] Workqueue: ufs_eh_wq_0 ufshcd_err_handler
>> [ 34.094788] Call Trace:
>> [ 34.095281] __schedule+0x275/0x6c0
>> [ 34.095743] schedule+0x41/0xa0
>> [ 34.096240] blk_queue_enter+0x10d/0x230
>> [ 34.096693] ? wait_woken+0x70/0x70
>> [ 34.097167] blk_mq_alloc_request+0x53/0xc0
>> [ 34.097610] blk_get_request+0x1e/0x60
>> [ 34.098053] __scsi_execute+0x3c/0x260
>> [ 34.098529] ufshcd_clear_ua_wlun.cold+0xa6/0x14b
>> [ 34.098977] ufshcd_clear_ua_wluns.part.0+0x4d/0x92
>> [ 34.099456] ufshcd_err_handler+0x97a/0x9ff
>> [ 34.099902] process_one_work+0x1cc/0x360
>> [ 34.100384] worker_thread+0x45/0x3b0
>> [ 34.100851] ? process_one_work+0x360/0x360
>> [ 34.101308] kthread+0xf6/0x130
>> [ 34.101728] ? kthread_park+0x80/0x80
>> [ 34.102186] ret_from_fork+0x1f/0x30
>>
>> [ 34.640751] task:kworker/u10:9 state:D stack:14528 pid: 255 ppid: 2 flags:0x00004000
>> [ 34.641253] Workqueue: events_unbound async_run_entry_fn
>> [ 34.641722] Call Trace:
>> [ 34.642217] __schedule+0x275/0x6c0
>> [ 34.642683] schedule+0x41/0xa0
>> [ 34.643179] schedule_timeout+0x18b/0x290
>> [ 34.643645] ? del_timer_sync+0x30/0x30
>> [ 34.644131] __down_timeout+0x6b/0xc0
>> [ 34.644568] ? ufshcd_clkscale_enable_show+0x20/0x20
>> [ 34.645014] ? async_schedule_node_domain+0x17d/0x190
>> [ 34.645496] down_timeout+0x42/0x50
>> [ 34.645947] ufshcd_wl_suspend+0x79/0xa0
>> [ 34.646432] ? scmd_printk+0x100/0x100
>> [ 34.646917] scsi_bus_suspend_common+0x56/0xc0
>> [ 34.647405] ? scsi_bus_freeze+0x10/0x10
>> [ 34.647858] dpm_run_callback+0x45/0x110
>> [ 34.648347] __device_suspend+0x117/0x460
>> [ 34.648788] async_suspend+0x16/0x90
>> [ 34.649251] async_run_entry_fn+0x26/0x110
>> [ 34.649676] process_one_work+0x1cc/0x360
>> [ 34.650137] worker_thread+0x45/0x3b0
>> [ 34.650563] ? process_one_work+0x360/0x360
>> [ 34.650994] kthread+0xf6/0x130
>> [ 34.651455] ? kthread_park+0x80/0x80
>> [ 34.651882] ret_from_fork+0x1f/0x30
>>
>>
>>
>>>
>>>>
>>>> I'll address the other comments in the next version.
>>>>
>>>>
>>>> Thank you!
>>>>
>>>>>> - pm_runtime_put(hba->dev);
>>>>>> + ufshcd_rpm_put(hba);
>>>>>> }
>>>>>
>>>>> <SNIP>
>>>>>
>>>>>> +void ufshcd_resume_complete(struct device *dev)
>>>>>> +{
>>>>
>>>
>>
>
>
_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek
^ permalink raw reply [flat|nested] 31+ messages in thread* Re: [PATCH v20 1/2] scsi: ufs: Enable power management for wlun
2021-04-23 4:23 ` Adrian Hunter
(?)
@ 2021-04-23 6:18 ` Adrian Hunter
-1 siblings, 0 replies; 31+ messages in thread
From: Adrian Hunter @ 2021-04-23 6:18 UTC (permalink / raw)
To: Asutosh Das (asd), cang, martin.petersen, linux-scsi
Cc: linux-arm-msm, Alim Akhtar, Avri Altman, James E.J. Bottomley,
Krzysztof Kozlowski, Stanley Chu, Andy Gross, Bjorn Andersson,
Steven Rostedt, Ingo Molnar, Matthias Brugger, Lee Jones,
Bean Huo, Kiwoong Kim, Colin Ian King, Wei Yongjun, Yue Hu,
Bart van Assche, Gustavo A. R. Silva, Dinghao Liu, Jaegeuk Kim,
Satya Tangirala, open list, moderated list:ARM/SAMSUNG S3C,
S5P AND EXYNOS ARM ARCHITECTURES, open list:ARM/SAMSUNG S3C,
S5P AND EXYNOS ARM ARCHITECTURES,
moderated list:UNIVERSAL FLASH STORAGE HOST CONTROLLER DRIVER...
[-- Attachment #1: Type: text/plain, Size: 3712 bytes --]
On 23/04/21 7:23 am, Adrian Hunter wrote:
> On 22/04/21 7:38 pm, Asutosh Das (asd) wrote:
>> On 4/20/2021 12:42 AM, Adrian Hunter wrote:
>>> On 20/04/21 7:15 am, Adrian Hunter wrote:
>>>> On 20/04/21 12:53 am, Asutosh Das (asd) wrote:
>>>>> On 4/19/2021 11:37 AM, Adrian Hunter wrote:
>>>>>> On 16/04/21 10:49 pm, Asutosh Das wrote:
>>>>>>>
>>>>>>> Co-developed-by: Can Guo <cang@codeaurora.org>
>>>>>>> Signed-off-by: Can Guo <cang@codeaurora.org>
>>>>>>> Signed-off-by: Asutosh Das <asutoshd@codeaurora.org>
>>>>>>> ---
>>>>>>
>>>>>> I came across 3 issues while testing. See comments below.
>>>>>>
>>>>> Hi Adrian
>>>>> Thanks for the comments.
>>>>>> <SNIP>
>>>>>>
>>>>>>> @@ -5794,7 +5839,7 @@ static void ufshcd_err_handling_unprepare(struct ufs_hba *hba)
>>>>>>> if (ufshcd_is_clkscaling_supported(hba))
>>>>>>> ufshcd_clk_scaling_suspend(hba, false);
>>>>>>> ufshcd_clear_ua_wluns(hba);
>>>>>>
>>>>>> ufshcd_clear_ua_wluns() deadlocks trying to clear UFS_UPIU_RPMB_WLUN
>>>>>> if sdev_rpmb is suspended and sdev_ufs_device is suspending.
>>>>>> e.g. ufshcd_wl_suspend() is waiting on host_sem while ufshcd_err_handler()
>>>>>> is running, at which point sdev_rpmb has already suspended.
>>>>>>
>>>>> Umm, I didn't understand this deadlock.
>>>>> When you say, sdev_rpmb is suspended, does it mean runtime_suspended?
>>>>> sdev_ufs_device is suspending - this can't be runtime_suspending, while ufshcd_err_handling_unprepare is running.
>>>>>
>>>>> If you've a call-stack of this deadlock, please can you share it with me. I'll also try to reproduce this.
>>>>
>>>> Yes it is system suspend. sdev_rpmb has suspended, sdev_ufs_device is waiting on host_sem.
>>>> ufshcd_err_handler() holds host_sem. ufshcd_clear_ua_wlun(UFS_UPIU_RPMB_WLUN) gets stuck.
>>>> I will get some call-stacks.
>>>
>> Hi Adrian,
>>
>> Thanks for the call stacks.
>> From the current information, I can't say for sure why it'd get stuck in blk_queue_enter().
>
> I presume SCSI is leaving the RPMB WLUN device runtime suspended and consequently the queue status is RPM_SUSPENDED
>
>>
>> I tried reproducing this issue on my setup yesterday but couldn't.
>> Here's what I did:
>> 1. sdev_rpmb is RPM_SUSPENDED, checked before initiating system suspend
>> 2. sdev_ufs_device is RPM_RESUMED
>> 3. I triggered system suspend (echo mem > /sys/power/state) and scheduled the error handler from ufshcd_wl_suspend().
>> 4. Waited until error handler ran and then ufshcd_wl_suspend() blocks on host_sem.
>> 5. The ufshcd_clear_wa_wlun(UFS_UPIU_RPMB_WLUN) went through fine.
>>
>> Do you've some specific steps to reproduce this or a script, perhaps? If so, please can you share it with me. I will try again.
>
> I was using a device that gives occasional errors, but I will what see I can do.
I have attached 2 patches to reproduce the issue, and the test script I used.
Note it is using down_timeout() so it doesn't lock up completely.
The kernel messages are:
[ 79.385456] ufshcd_wl_suspend: doing ufshcd_schedule_eh_work
[ 79.385511] ufshcd_wl_suspend: sleeping 1000 ms
[ 79.386916] ufshcd_err_handler: start
[ 79.386979] ufshcd_err_handler: got sem
[ 79.387053] ufshcd_err_handling_unprepare: start
[ 79.387302] ufshcd_clear_ua_wluns: before ufshcd_clear_ua_wlun(hba, UFS_UPIU_RPMB_WLUN)
[ 80.435878] ufshcd_wl_suspend: trying to get sem
[ 85.683876] ufshcd_wl_suspend: failed to get sem
[ 85.683993] ufs_device_wlun 0:0:0:49488: PM: failed to suspend async: error -22
[ 85.686901] ufshcd_clear_ua_wluns: after ufshcd_clear_ua_wlun(hba, UFS_UPIU_RPMB_WLUN), ret 0
[ 85.687025] ufshcd_err_handling_unprepare: finish
[ 85.687090] ufshcd_err_handler: finish
[-- Attachment #2: 0001-changes-suggested-since-v20.patch --]
[-- Type: text/x-patch, Size: 3276 bytes --]
>From c21ea993b28bec894f96c0c7b387f9688b01900f Mon Sep 17 00:00:00 2001
From: Adrian Hunter <adrian.hunter@intel.com>
Date: Tue, 20 Apr 2021 14:33:03 +0300
Subject: [PATCH 1/2] changes suggested since v20
Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
---
drivers/scsi/ufs/ufshcd.c | 50 ++++++++++-----------------------------
drivers/scsi/ufs/ufshcd.h | 1 +
2 files changed, 14 insertions(+), 37 deletions(-)
diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index eb36f753469d..5be5b2472db0 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -5844,7 +5844,7 @@ static void ufshcd_err_handling_unprepare(struct ufs_hba *hba)
static inline bool ufshcd_err_handling_should_stop(struct ufs_hba *hba)
{
- return (!hba->is_powered || hba->shutting_down ||
+ return (!hba->is_powered || hba->shutting_down || !hba->sdev_ufs_device ||
hba->ufshcd_state == UFSHCD_STATE_ERROR ||
(!(hba->saved_err || hba->saved_uic_err || hba->force_reset ||
ufshcd_is_link_broken(hba))));
@@ -9572,51 +9572,27 @@ void ufshcd_resume_complete(struct device *dev)
{
struct ufs_hba *hba = dev_get_drvdata(dev);
- ufshcd_rpm_put(hba);
+ if (hba->complete_put) {
+ hba->complete_put = false;
+ 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);
- 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;
-
- /*
- * 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.
- * However, if the current_{pwr_mode, link_state} is same as the
- * desired_{pwr_mode, link_state}, there's no need to rpm resume
- * the device.
- * Refer ufshcd_resume_complete()
- */
- pm_runtime_get_noresume(ufs_dev);
-
- 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);
+ int ret;
- if (!rpm_state_ok) {
- int ret = pm_runtime_resume(ufs_dev);
+ if (!hba->sdev_ufs_device)
+ return 0;
- if (ret < 0 && ret != -EACCES) {
- pm_runtime_put(ufs_dev);
- return ret;
- }
+ ret = ufshcd_rpm_get_sync(hba);
+ if (ret < 0 && ret != -EACCES) {
+ ufshcd_rpm_put(hba);
+ return ret;
}
+ hba->complete_put = true;
return 0;
}
EXPORT_SYMBOL_GPL(ufshcd_suspend_prepare);
diff --git a/drivers/scsi/ufs/ufshcd.h b/drivers/scsi/ufs/ufshcd.h
index 3a8dc0dfae05..fd5be083e168 100644
--- a/drivers/scsi/ufs/ufshcd.h
+++ b/drivers/scsi/ufs/ufshcd.h
@@ -850,6 +850,7 @@ struct ufs_hba {
u32 debugfs_ee_rate_limit_ms;
#endif
u32 luns_avail;
+ bool complete_put;
};
/* Returns true if clocks can be gated. Otherwise false */
--
2.25.1
[-- Attachment #3: 0002-instrumentation-to-reproduce-issue.patch --]
[-- Type: text/x-patch, Size: 3068 bytes --]
>From 2b9aae51ac5b1f1b13fdab5570dceefbbdc9682b Mon Sep 17 00:00:00 2001
From: Adrian Hunter <adrian.hunter@intel.com>
Date: Fri, 23 Apr 2021 08:56:18 +0300
Subject: [PATCH 2/2] instrumentation to reproduce issue
Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
---
drivers/scsi/ufs/ufshcd.c | 26 ++++++++++++++++++++++----
1 file changed, 22 insertions(+), 4 deletions(-)
diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index 5be5b2472db0..32fc4e3991ff 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -5834,12 +5834,14 @@ static void ufshcd_err_handling_prepare(struct ufs_hba *hba)
static void ufshcd_err_handling_unprepare(struct ufs_hba *hba)
{
+ pr_err("%s: start\n", __func__);
ufshcd_scsi_unblock_requests(hba);
ufshcd_release(hba);
if (ufshcd_is_clkscaling_supported(hba))
ufshcd_clk_scaling_suspend(hba, false);
ufshcd_clear_ua_wluns(hba);
ufshcd_rpm_put(hba);
+ pr_err("%s: finish\n", __func__);
}
static inline bool ufshcd_err_handling_should_stop(struct ufs_hba *hba)
@@ -5919,11 +5921,14 @@ static void ufshcd_err_handler(struct work_struct *work)
int tag;
bool needs_reset = false, needs_restore = false;
+ pr_err("%s: start\n", __func__);
+
hba = container_of(work, struct ufs_hba, eh_work);
down(&hba->host_sem);
+ pr_err("%s: got sem\n", __func__);
spin_lock_irqsave(hba->host->host_lock, flags);
- if (ufshcd_err_handling_should_stop(hba)) {
+ if (0 && ufshcd_err_handling_should_stop(hba)) {
if (hba->ufshcd_state != UFSHCD_STATE_ERROR)
hba->ufshcd_state = UFSHCD_STATE_OPERATIONAL;
spin_unlock_irqrestore(hba->host->host_lock, flags);
@@ -6095,6 +6100,7 @@ static void ufshcd_err_handler(struct work_struct *work)
ufshcd_clear_eh_in_progress(hba);
spin_unlock_irqrestore(hba->host->host_lock, flags);
ufshcd_err_handling_unprepare(hba);
+ pr_err("%s: finish\n", __func__);
up(&hba->host_sem);
}
@@ -7872,12 +7878,15 @@ static int ufshcd_clear_ua_wluns(struct ufs_hba *hba)
{
int ret = 0;
- if (!hba->wlun_dev_clr_ua)
+ if (0 && !hba->wlun_dev_clr_ua)
goto out;
ret = ufshcd_clear_ua_wlun(hba, UFS_UPIU_UFS_DEVICE_WLUN);
- if (!ret)
+ if (!ret) {
+ pr_err("%s: before ufshcd_clear_ua_wlun(hba, UFS_UPIU_RPMB_WLUN)\n", __func__);
ret = ufshcd_clear_ua_wlun(hba, UFS_UPIU_RPMB_WLUN);
+ pr_err("%s: after ufshcd_clear_ua_wlun(hba, UFS_UPIU_RPMB_WLUN), ret %d\n", __func__, ret);
+ }
if (!ret)
hba->wlun_dev_clr_ua = false;
out:
@@ -8983,7 +8992,16 @@ static int ufshcd_wl_suspend(struct device *dev)
ktime_t start = ktime_get();
hba = shost_priv(sdev->host);
- down(&hba->host_sem);
+ pr_err("%s: doing ufshcd_schedule_eh_work\n", __func__);
+ ufshcd_schedule_eh_work(hba);
+ pr_err("%s: sleeping 1000 ms\n", __func__);
+ msleep(1000);
+ pr_err("%s: trying to get sem\n", __func__);
+ if (down_timeout(&hba->host_sem, msecs_to_jiffies(5000))) {
+ pr_err("%s: failed to get sem\n", __func__);
+ return -EINVAL;
+ }
+ pr_err("%s: got sem\n", __func__);
if (pm_runtime_suspended(dev))
goto out;
--
2.25.1
[-- Attachment #4: go.sh --]
[-- Type: application/x-shellscript, Size: 316 bytes --]
^ permalink raw reply related [flat|nested] 31+ messages in thread* Re: [PATCH v20 1/2] scsi: ufs: Enable power management for wlun
@ 2021-04-23 6:18 ` Adrian Hunter
0 siblings, 0 replies; 31+ messages in thread
From: Adrian Hunter @ 2021-04-23 6:18 UTC (permalink / raw)
To: Asutosh Das (asd), cang, martin.petersen, linux-scsi
Cc: linux-arm-msm, Alim Akhtar, Avri Altman, James E.J. Bottomley,
Krzysztof Kozlowski, Stanley Chu, Andy Gross, Bjorn Andersson,
Steven Rostedt, Ingo Molnar, Matthias Brugger, Lee Jones,
Bean Huo, Kiwoong Kim, Colin Ian King, Wei Yongjun, Yue Hu,
Bart van Assche, Gustavo A. R. Silva, Dinghao Liu, Jaegeuk Kim,
Satya Tangirala, open list, moderated list:ARM/SAMSUNG S3C,
S5P AND EXYNOS ARM ARCHITECTURES, open list:ARM/SAMSUNG S3C,
S5P AND EXYNOS ARM ARCHITECTURES,
moderated list:UNIVERSAL FLASH STORAGE HOST CONTROLLER DRIVER...
[-- Attachment #1: Type: text/plain, Size: 3712 bytes --]
On 23/04/21 7:23 am, Adrian Hunter wrote:
> On 22/04/21 7:38 pm, Asutosh Das (asd) wrote:
>> On 4/20/2021 12:42 AM, Adrian Hunter wrote:
>>> On 20/04/21 7:15 am, Adrian Hunter wrote:
>>>> On 20/04/21 12:53 am, Asutosh Das (asd) wrote:
>>>>> On 4/19/2021 11:37 AM, Adrian Hunter wrote:
>>>>>> On 16/04/21 10:49 pm, Asutosh Das wrote:
>>>>>>>
>>>>>>> Co-developed-by: Can Guo <cang@codeaurora.org>
>>>>>>> Signed-off-by: Can Guo <cang@codeaurora.org>
>>>>>>> Signed-off-by: Asutosh Das <asutoshd@codeaurora.org>
>>>>>>> ---
>>>>>>
>>>>>> I came across 3 issues while testing. See comments below.
>>>>>>
>>>>> Hi Adrian
>>>>> Thanks for the comments.
>>>>>> <SNIP>
>>>>>>
>>>>>>> @@ -5794,7 +5839,7 @@ static void ufshcd_err_handling_unprepare(struct ufs_hba *hba)
>>>>>>> if (ufshcd_is_clkscaling_supported(hba))
>>>>>>> ufshcd_clk_scaling_suspend(hba, false);
>>>>>>> ufshcd_clear_ua_wluns(hba);
>>>>>>
>>>>>> ufshcd_clear_ua_wluns() deadlocks trying to clear UFS_UPIU_RPMB_WLUN
>>>>>> if sdev_rpmb is suspended and sdev_ufs_device is suspending.
>>>>>> e.g. ufshcd_wl_suspend() is waiting on host_sem while ufshcd_err_handler()
>>>>>> is running, at which point sdev_rpmb has already suspended.
>>>>>>
>>>>> Umm, I didn't understand this deadlock.
>>>>> When you say, sdev_rpmb is suspended, does it mean runtime_suspended?
>>>>> sdev_ufs_device is suspending - this can't be runtime_suspending, while ufshcd_err_handling_unprepare is running.
>>>>>
>>>>> If you've a call-stack of this deadlock, please can you share it with me. I'll also try to reproduce this.
>>>>
>>>> Yes it is system suspend. sdev_rpmb has suspended, sdev_ufs_device is waiting on host_sem.
>>>> ufshcd_err_handler() holds host_sem. ufshcd_clear_ua_wlun(UFS_UPIU_RPMB_WLUN) gets stuck.
>>>> I will get some call-stacks.
>>>
>> Hi Adrian,
>>
>> Thanks for the call stacks.
>> From the current information, I can't say for sure why it'd get stuck in blk_queue_enter().
>
> I presume SCSI is leaving the RPMB WLUN device runtime suspended and consequently the queue status is RPM_SUSPENDED
>
>>
>> I tried reproducing this issue on my setup yesterday but couldn't.
>> Here's what I did:
>> 1. sdev_rpmb is RPM_SUSPENDED, checked before initiating system suspend
>> 2. sdev_ufs_device is RPM_RESUMED
>> 3. I triggered system suspend (echo mem > /sys/power/state) and scheduled the error handler from ufshcd_wl_suspend().
>> 4. Waited until error handler ran and then ufshcd_wl_suspend() blocks on host_sem.
>> 5. The ufshcd_clear_wa_wlun(UFS_UPIU_RPMB_WLUN) went through fine.
>>
>> Do you've some specific steps to reproduce this or a script, perhaps? If so, please can you share it with me. I will try again.
>
> I was using a device that gives occasional errors, but I will what see I can do.
I have attached 2 patches to reproduce the issue, and the test script I used.
Note it is using down_timeout() so it doesn't lock up completely.
The kernel messages are:
[ 79.385456] ufshcd_wl_suspend: doing ufshcd_schedule_eh_work
[ 79.385511] ufshcd_wl_suspend: sleeping 1000 ms
[ 79.386916] ufshcd_err_handler: start
[ 79.386979] ufshcd_err_handler: got sem
[ 79.387053] ufshcd_err_handling_unprepare: start
[ 79.387302] ufshcd_clear_ua_wluns: before ufshcd_clear_ua_wlun(hba, UFS_UPIU_RPMB_WLUN)
[ 80.435878] ufshcd_wl_suspend: trying to get sem
[ 85.683876] ufshcd_wl_suspend: failed to get sem
[ 85.683993] ufs_device_wlun 0:0:0:49488: PM: failed to suspend async: error -22
[ 85.686901] ufshcd_clear_ua_wluns: after ufshcd_clear_ua_wlun(hba, UFS_UPIU_RPMB_WLUN), ret 0
[ 85.687025] ufshcd_err_handling_unprepare: finish
[ 85.687090] ufshcd_err_handler: finish
[-- Attachment #2: 0001-changes-suggested-since-v20.patch --]
[-- Type: text/x-patch, Size: 3276 bytes --]
>From c21ea993b28bec894f96c0c7b387f9688b01900f Mon Sep 17 00:00:00 2001
From: Adrian Hunter <adrian.hunter@intel.com>
Date: Tue, 20 Apr 2021 14:33:03 +0300
Subject: [PATCH 1/2] changes suggested since v20
Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
---
drivers/scsi/ufs/ufshcd.c | 50 ++++++++++-----------------------------
drivers/scsi/ufs/ufshcd.h | 1 +
2 files changed, 14 insertions(+), 37 deletions(-)
diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index eb36f753469d..5be5b2472db0 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -5844,7 +5844,7 @@ static void ufshcd_err_handling_unprepare(struct ufs_hba *hba)
static inline bool ufshcd_err_handling_should_stop(struct ufs_hba *hba)
{
- return (!hba->is_powered || hba->shutting_down ||
+ return (!hba->is_powered || hba->shutting_down || !hba->sdev_ufs_device ||
hba->ufshcd_state == UFSHCD_STATE_ERROR ||
(!(hba->saved_err || hba->saved_uic_err || hba->force_reset ||
ufshcd_is_link_broken(hba))));
@@ -9572,51 +9572,27 @@ void ufshcd_resume_complete(struct device *dev)
{
struct ufs_hba *hba = dev_get_drvdata(dev);
- ufshcd_rpm_put(hba);
+ if (hba->complete_put) {
+ hba->complete_put = false;
+ 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);
- 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;
-
- /*
- * 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.
- * However, if the current_{pwr_mode, link_state} is same as the
- * desired_{pwr_mode, link_state}, there's no need to rpm resume
- * the device.
- * Refer ufshcd_resume_complete()
- */
- pm_runtime_get_noresume(ufs_dev);
-
- 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);
+ int ret;
- if (!rpm_state_ok) {
- int ret = pm_runtime_resume(ufs_dev);
+ if (!hba->sdev_ufs_device)
+ return 0;
- if (ret < 0 && ret != -EACCES) {
- pm_runtime_put(ufs_dev);
- return ret;
- }
+ ret = ufshcd_rpm_get_sync(hba);
+ if (ret < 0 && ret != -EACCES) {
+ ufshcd_rpm_put(hba);
+ return ret;
}
+ hba->complete_put = true;
return 0;
}
EXPORT_SYMBOL_GPL(ufshcd_suspend_prepare);
diff --git a/drivers/scsi/ufs/ufshcd.h b/drivers/scsi/ufs/ufshcd.h
index 3a8dc0dfae05..fd5be083e168 100644
--- a/drivers/scsi/ufs/ufshcd.h
+++ b/drivers/scsi/ufs/ufshcd.h
@@ -850,6 +850,7 @@ struct ufs_hba {
u32 debugfs_ee_rate_limit_ms;
#endif
u32 luns_avail;
+ bool complete_put;
};
/* Returns true if clocks can be gated. Otherwise false */
--
2.25.1
[-- Attachment #3: 0002-instrumentation-to-reproduce-issue.patch --]
[-- Type: text/x-patch, Size: 3068 bytes --]
>From 2b9aae51ac5b1f1b13fdab5570dceefbbdc9682b Mon Sep 17 00:00:00 2001
From: Adrian Hunter <adrian.hunter@intel.com>
Date: Fri, 23 Apr 2021 08:56:18 +0300
Subject: [PATCH 2/2] instrumentation to reproduce issue
Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
---
drivers/scsi/ufs/ufshcd.c | 26 ++++++++++++++++++++++----
1 file changed, 22 insertions(+), 4 deletions(-)
diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index 5be5b2472db0..32fc4e3991ff 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -5834,12 +5834,14 @@ static void ufshcd_err_handling_prepare(struct ufs_hba *hba)
static void ufshcd_err_handling_unprepare(struct ufs_hba *hba)
{
+ pr_err("%s: start\n", __func__);
ufshcd_scsi_unblock_requests(hba);
ufshcd_release(hba);
if (ufshcd_is_clkscaling_supported(hba))
ufshcd_clk_scaling_suspend(hba, false);
ufshcd_clear_ua_wluns(hba);
ufshcd_rpm_put(hba);
+ pr_err("%s: finish\n", __func__);
}
static inline bool ufshcd_err_handling_should_stop(struct ufs_hba *hba)
@@ -5919,11 +5921,14 @@ static void ufshcd_err_handler(struct work_struct *work)
int tag;
bool needs_reset = false, needs_restore = false;
+ pr_err("%s: start\n", __func__);
+
hba = container_of(work, struct ufs_hba, eh_work);
down(&hba->host_sem);
+ pr_err("%s: got sem\n", __func__);
spin_lock_irqsave(hba->host->host_lock, flags);
- if (ufshcd_err_handling_should_stop(hba)) {
+ if (0 && ufshcd_err_handling_should_stop(hba)) {
if (hba->ufshcd_state != UFSHCD_STATE_ERROR)
hba->ufshcd_state = UFSHCD_STATE_OPERATIONAL;
spin_unlock_irqrestore(hba->host->host_lock, flags);
@@ -6095,6 +6100,7 @@ static void ufshcd_err_handler(struct work_struct *work)
ufshcd_clear_eh_in_progress(hba);
spin_unlock_irqrestore(hba->host->host_lock, flags);
ufshcd_err_handling_unprepare(hba);
+ pr_err("%s: finish\n", __func__);
up(&hba->host_sem);
}
@@ -7872,12 +7878,15 @@ static int ufshcd_clear_ua_wluns(struct ufs_hba *hba)
{
int ret = 0;
- if (!hba->wlun_dev_clr_ua)
+ if (0 && !hba->wlun_dev_clr_ua)
goto out;
ret = ufshcd_clear_ua_wlun(hba, UFS_UPIU_UFS_DEVICE_WLUN);
- if (!ret)
+ if (!ret) {
+ pr_err("%s: before ufshcd_clear_ua_wlun(hba, UFS_UPIU_RPMB_WLUN)\n", __func__);
ret = ufshcd_clear_ua_wlun(hba, UFS_UPIU_RPMB_WLUN);
+ pr_err("%s: after ufshcd_clear_ua_wlun(hba, UFS_UPIU_RPMB_WLUN), ret %d\n", __func__, ret);
+ }
if (!ret)
hba->wlun_dev_clr_ua = false;
out:
@@ -8983,7 +8992,16 @@ static int ufshcd_wl_suspend(struct device *dev)
ktime_t start = ktime_get();
hba = shost_priv(sdev->host);
- down(&hba->host_sem);
+ pr_err("%s: doing ufshcd_schedule_eh_work\n", __func__);
+ ufshcd_schedule_eh_work(hba);
+ pr_err("%s: sleeping 1000 ms\n", __func__);
+ msleep(1000);
+ pr_err("%s: trying to get sem\n", __func__);
+ if (down_timeout(&hba->host_sem, msecs_to_jiffies(5000))) {
+ pr_err("%s: failed to get sem\n", __func__);
+ return -EINVAL;
+ }
+ pr_err("%s: got sem\n", __func__);
if (pm_runtime_suspended(dev))
goto out;
--
2.25.1
[-- Attachment #4: go.sh --]
[-- Type: application/x-shellscript, Size: 316 bytes --]
[-- Attachment #5: Type: text/plain, Size: 176 bytes --]
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 31+ messages in thread* Re: [PATCH v20 1/2] scsi: ufs: Enable power management for wlun
@ 2021-04-23 6:18 ` Adrian Hunter
0 siblings, 0 replies; 31+ messages in thread
From: Adrian Hunter @ 2021-04-23 6:18 UTC (permalink / raw)
To: Asutosh Das (asd), cang, martin.petersen, linux-scsi
Cc: linux-arm-msm, Alim Akhtar, Avri Altman, James E.J. Bottomley,
Krzysztof Kozlowski, Stanley Chu, Andy Gross, Bjorn Andersson,
Steven Rostedt, Ingo Molnar, Matthias Brugger, Lee Jones,
Bean Huo, Kiwoong Kim, Colin Ian King, Wei Yongjun, Yue Hu,
Bart van Assche, Gustavo A. R. Silva, Dinghao Liu, Jaegeuk Kim,
Satya Tangirala, open list, moderated list:ARM/SAMSUNG S3C,
S5P AND EXYNOS ARM ARCHITECTURES, open list:ARM/SAMSUNG S3C,
S5P AND EXYNOS ARM ARCHITECTURES,
moderated list:UNIVERSAL FLASH STORAGE HOST CONTROLLER DRIVER...
[-- Attachment #1: Type: text/plain, Size: 3712 bytes --]
On 23/04/21 7:23 am, Adrian Hunter wrote:
> On 22/04/21 7:38 pm, Asutosh Das (asd) wrote:
>> On 4/20/2021 12:42 AM, Adrian Hunter wrote:
>>> On 20/04/21 7:15 am, Adrian Hunter wrote:
>>>> On 20/04/21 12:53 am, Asutosh Das (asd) wrote:
>>>>> On 4/19/2021 11:37 AM, Adrian Hunter wrote:
>>>>>> On 16/04/21 10:49 pm, Asutosh Das wrote:
>>>>>>>
>>>>>>> Co-developed-by: Can Guo <cang@codeaurora.org>
>>>>>>> Signed-off-by: Can Guo <cang@codeaurora.org>
>>>>>>> Signed-off-by: Asutosh Das <asutoshd@codeaurora.org>
>>>>>>> ---
>>>>>>
>>>>>> I came across 3 issues while testing. See comments below.
>>>>>>
>>>>> Hi Adrian
>>>>> Thanks for the comments.
>>>>>> <SNIP>
>>>>>>
>>>>>>> @@ -5794,7 +5839,7 @@ static void ufshcd_err_handling_unprepare(struct ufs_hba *hba)
>>>>>>> if (ufshcd_is_clkscaling_supported(hba))
>>>>>>> ufshcd_clk_scaling_suspend(hba, false);
>>>>>>> ufshcd_clear_ua_wluns(hba);
>>>>>>
>>>>>> ufshcd_clear_ua_wluns() deadlocks trying to clear UFS_UPIU_RPMB_WLUN
>>>>>> if sdev_rpmb is suspended and sdev_ufs_device is suspending.
>>>>>> e.g. ufshcd_wl_suspend() is waiting on host_sem while ufshcd_err_handler()
>>>>>> is running, at which point sdev_rpmb has already suspended.
>>>>>>
>>>>> Umm, I didn't understand this deadlock.
>>>>> When you say, sdev_rpmb is suspended, does it mean runtime_suspended?
>>>>> sdev_ufs_device is suspending - this can't be runtime_suspending, while ufshcd_err_handling_unprepare is running.
>>>>>
>>>>> If you've a call-stack of this deadlock, please can you share it with me. I'll also try to reproduce this.
>>>>
>>>> Yes it is system suspend. sdev_rpmb has suspended, sdev_ufs_device is waiting on host_sem.
>>>> ufshcd_err_handler() holds host_sem. ufshcd_clear_ua_wlun(UFS_UPIU_RPMB_WLUN) gets stuck.
>>>> I will get some call-stacks.
>>>
>> Hi Adrian,
>>
>> Thanks for the call stacks.
>> From the current information, I can't say for sure why it'd get stuck in blk_queue_enter().
>
> I presume SCSI is leaving the RPMB WLUN device runtime suspended and consequently the queue status is RPM_SUSPENDED
>
>>
>> I tried reproducing this issue on my setup yesterday but couldn't.
>> Here's what I did:
>> 1. sdev_rpmb is RPM_SUSPENDED, checked before initiating system suspend
>> 2. sdev_ufs_device is RPM_RESUMED
>> 3. I triggered system suspend (echo mem > /sys/power/state) and scheduled the error handler from ufshcd_wl_suspend().
>> 4. Waited until error handler ran and then ufshcd_wl_suspend() blocks on host_sem.
>> 5. The ufshcd_clear_wa_wlun(UFS_UPIU_RPMB_WLUN) went through fine.
>>
>> Do you've some specific steps to reproduce this or a script, perhaps? If so, please can you share it with me. I will try again.
>
> I was using a device that gives occasional errors, but I will what see I can do.
I have attached 2 patches to reproduce the issue, and the test script I used.
Note it is using down_timeout() so it doesn't lock up completely.
The kernel messages are:
[ 79.385456] ufshcd_wl_suspend: doing ufshcd_schedule_eh_work
[ 79.385511] ufshcd_wl_suspend: sleeping 1000 ms
[ 79.386916] ufshcd_err_handler: start
[ 79.386979] ufshcd_err_handler: got sem
[ 79.387053] ufshcd_err_handling_unprepare: start
[ 79.387302] ufshcd_clear_ua_wluns: before ufshcd_clear_ua_wlun(hba, UFS_UPIU_RPMB_WLUN)
[ 80.435878] ufshcd_wl_suspend: trying to get sem
[ 85.683876] ufshcd_wl_suspend: failed to get sem
[ 85.683993] ufs_device_wlun 0:0:0:49488: PM: failed to suspend async: error -22
[ 85.686901] ufshcd_clear_ua_wluns: after ufshcd_clear_ua_wlun(hba, UFS_UPIU_RPMB_WLUN), ret 0
[ 85.687025] ufshcd_err_handling_unprepare: finish
[ 85.687090] ufshcd_err_handler: finish
[-- Attachment #2: 0001-changes-suggested-since-v20.patch --]
[-- Type: text/x-patch, Size: 3276 bytes --]
>From c21ea993b28bec894f96c0c7b387f9688b01900f Mon Sep 17 00:00:00 2001
From: Adrian Hunter <adrian.hunter@intel.com>
Date: Tue, 20 Apr 2021 14:33:03 +0300
Subject: [PATCH 1/2] changes suggested since v20
Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
---
drivers/scsi/ufs/ufshcd.c | 50 ++++++++++-----------------------------
drivers/scsi/ufs/ufshcd.h | 1 +
2 files changed, 14 insertions(+), 37 deletions(-)
diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index eb36f753469d..5be5b2472db0 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -5844,7 +5844,7 @@ static void ufshcd_err_handling_unprepare(struct ufs_hba *hba)
static inline bool ufshcd_err_handling_should_stop(struct ufs_hba *hba)
{
- return (!hba->is_powered || hba->shutting_down ||
+ return (!hba->is_powered || hba->shutting_down || !hba->sdev_ufs_device ||
hba->ufshcd_state == UFSHCD_STATE_ERROR ||
(!(hba->saved_err || hba->saved_uic_err || hba->force_reset ||
ufshcd_is_link_broken(hba))));
@@ -9572,51 +9572,27 @@ void ufshcd_resume_complete(struct device *dev)
{
struct ufs_hba *hba = dev_get_drvdata(dev);
- ufshcd_rpm_put(hba);
+ if (hba->complete_put) {
+ hba->complete_put = false;
+ 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);
- 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;
-
- /*
- * 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.
- * However, if the current_{pwr_mode, link_state} is same as the
- * desired_{pwr_mode, link_state}, there's no need to rpm resume
- * the device.
- * Refer ufshcd_resume_complete()
- */
- pm_runtime_get_noresume(ufs_dev);
-
- 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);
+ int ret;
- if (!rpm_state_ok) {
- int ret = pm_runtime_resume(ufs_dev);
+ if (!hba->sdev_ufs_device)
+ return 0;
- if (ret < 0 && ret != -EACCES) {
- pm_runtime_put(ufs_dev);
- return ret;
- }
+ ret = ufshcd_rpm_get_sync(hba);
+ if (ret < 0 && ret != -EACCES) {
+ ufshcd_rpm_put(hba);
+ return ret;
}
+ hba->complete_put = true;
return 0;
}
EXPORT_SYMBOL_GPL(ufshcd_suspend_prepare);
diff --git a/drivers/scsi/ufs/ufshcd.h b/drivers/scsi/ufs/ufshcd.h
index 3a8dc0dfae05..fd5be083e168 100644
--- a/drivers/scsi/ufs/ufshcd.h
+++ b/drivers/scsi/ufs/ufshcd.h
@@ -850,6 +850,7 @@ struct ufs_hba {
u32 debugfs_ee_rate_limit_ms;
#endif
u32 luns_avail;
+ bool complete_put;
};
/* Returns true if clocks can be gated. Otherwise false */
--
2.25.1
[-- Attachment #3: 0002-instrumentation-to-reproduce-issue.patch --]
[-- Type: text/x-patch, Size: 3068 bytes --]
>From 2b9aae51ac5b1f1b13fdab5570dceefbbdc9682b Mon Sep 17 00:00:00 2001
From: Adrian Hunter <adrian.hunter@intel.com>
Date: Fri, 23 Apr 2021 08:56:18 +0300
Subject: [PATCH 2/2] instrumentation to reproduce issue
Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
---
drivers/scsi/ufs/ufshcd.c | 26 ++++++++++++++++++++++----
1 file changed, 22 insertions(+), 4 deletions(-)
diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index 5be5b2472db0..32fc4e3991ff 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -5834,12 +5834,14 @@ static void ufshcd_err_handling_prepare(struct ufs_hba *hba)
static void ufshcd_err_handling_unprepare(struct ufs_hba *hba)
{
+ pr_err("%s: start\n", __func__);
ufshcd_scsi_unblock_requests(hba);
ufshcd_release(hba);
if (ufshcd_is_clkscaling_supported(hba))
ufshcd_clk_scaling_suspend(hba, false);
ufshcd_clear_ua_wluns(hba);
ufshcd_rpm_put(hba);
+ pr_err("%s: finish\n", __func__);
}
static inline bool ufshcd_err_handling_should_stop(struct ufs_hba *hba)
@@ -5919,11 +5921,14 @@ static void ufshcd_err_handler(struct work_struct *work)
int tag;
bool needs_reset = false, needs_restore = false;
+ pr_err("%s: start\n", __func__);
+
hba = container_of(work, struct ufs_hba, eh_work);
down(&hba->host_sem);
+ pr_err("%s: got sem\n", __func__);
spin_lock_irqsave(hba->host->host_lock, flags);
- if (ufshcd_err_handling_should_stop(hba)) {
+ if (0 && ufshcd_err_handling_should_stop(hba)) {
if (hba->ufshcd_state != UFSHCD_STATE_ERROR)
hba->ufshcd_state = UFSHCD_STATE_OPERATIONAL;
spin_unlock_irqrestore(hba->host->host_lock, flags);
@@ -6095,6 +6100,7 @@ static void ufshcd_err_handler(struct work_struct *work)
ufshcd_clear_eh_in_progress(hba);
spin_unlock_irqrestore(hba->host->host_lock, flags);
ufshcd_err_handling_unprepare(hba);
+ pr_err("%s: finish\n", __func__);
up(&hba->host_sem);
}
@@ -7872,12 +7878,15 @@ static int ufshcd_clear_ua_wluns(struct ufs_hba *hba)
{
int ret = 0;
- if (!hba->wlun_dev_clr_ua)
+ if (0 && !hba->wlun_dev_clr_ua)
goto out;
ret = ufshcd_clear_ua_wlun(hba, UFS_UPIU_UFS_DEVICE_WLUN);
- if (!ret)
+ if (!ret) {
+ pr_err("%s: before ufshcd_clear_ua_wlun(hba, UFS_UPIU_RPMB_WLUN)\n", __func__);
ret = ufshcd_clear_ua_wlun(hba, UFS_UPIU_RPMB_WLUN);
+ pr_err("%s: after ufshcd_clear_ua_wlun(hba, UFS_UPIU_RPMB_WLUN), ret %d\n", __func__, ret);
+ }
if (!ret)
hba->wlun_dev_clr_ua = false;
out:
@@ -8983,7 +8992,16 @@ static int ufshcd_wl_suspend(struct device *dev)
ktime_t start = ktime_get();
hba = shost_priv(sdev->host);
- down(&hba->host_sem);
+ pr_err("%s: doing ufshcd_schedule_eh_work\n", __func__);
+ ufshcd_schedule_eh_work(hba);
+ pr_err("%s: sleeping 1000 ms\n", __func__);
+ msleep(1000);
+ pr_err("%s: trying to get sem\n", __func__);
+ if (down_timeout(&hba->host_sem, msecs_to_jiffies(5000))) {
+ pr_err("%s: failed to get sem\n", __func__);
+ return -EINVAL;
+ }
+ pr_err("%s: got sem\n", __func__);
if (pm_runtime_suspended(dev))
goto out;
--
2.25.1
[-- Attachment #4: go.sh --]
[-- Type: application/x-shellscript, Size: 316 bytes --]
[-- Attachment #5: Type: text/plain, Size: 170 bytes --]
_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek
^ permalink raw reply related [flat|nested] 31+ messages in thread* Re: [PATCH v20 1/2] scsi: ufs: Enable power management for wlun
2021-04-23 6:18 ` Adrian Hunter
(?)
@ 2021-04-23 8:01 ` Adrian Hunter
-1 siblings, 0 replies; 31+ messages in thread
From: Adrian Hunter @ 2021-04-23 8:01 UTC (permalink / raw)
To: Asutosh Das (asd), cang, martin.petersen, linux-scsi
Cc: linux-arm-msm, Alim Akhtar, Avri Altman, James E.J. Bottomley,
Krzysztof Kozlowski, Stanley Chu, Andy Gross, Bjorn Andersson,
Steven Rostedt, Ingo Molnar, Matthias Brugger, Lee Jones,
Bean Huo, Kiwoong Kim, Colin Ian King, Wei Yongjun, Yue Hu,
Bart van Assche, Gustavo A. R. Silva, Dinghao Liu, Jaegeuk Kim,
Satya Tangirala, open list, moderated list:ARM/SAMSUNG S3C,
S5P AND EXYNOS ARM ARCHITECTURES, open list:ARM/SAMSUNG S3C,
S5P AND EXYNOS ARM ARCHITECTURES,
moderated list:UNIVERSAL FLASH STORAGE HOST CONTROLLER DRIVER...
On 23/04/21 9:18 am, Adrian Hunter wrote:
> On 23/04/21 7:23 am, Adrian Hunter wrote:
>> On 22/04/21 7:38 pm, Asutosh Das (asd) wrote:
>>> On 4/20/2021 12:42 AM, Adrian Hunter wrote:
>>>> On 20/04/21 7:15 am, Adrian Hunter wrote:
>>>>> On 20/04/21 12:53 am, Asutosh Das (asd) wrote:
>>>>>> On 4/19/2021 11:37 AM, Adrian Hunter wrote:
>>>>>>> On 16/04/21 10:49 pm, Asutosh Das wrote:
>>>>>>>>
>>>>>>>> Co-developed-by: Can Guo <cang@codeaurora.org>
>>>>>>>> Signed-off-by: Can Guo <cang@codeaurora.org>
>>>>>>>> Signed-off-by: Asutosh Das <asutoshd@codeaurora.org>
>>>>>>>> ---
>>>>>>>
>>>>>>> I came across 3 issues while testing. See comments below.
>>>>>>>
>>>>>> Hi Adrian
>>>>>> Thanks for the comments.
>>>>>>> <SNIP>
>>>>>>>
>>>>>>>> @@ -5794,7 +5839,7 @@ static void ufshcd_err_handling_unprepare(struct ufs_hba *hba)
>>>>>>>> if (ufshcd_is_clkscaling_supported(hba))
>>>>>>>> ufshcd_clk_scaling_suspend(hba, false);
>>>>>>>> ufshcd_clear_ua_wluns(hba);
>>>>>>>
>>>>>>> ufshcd_clear_ua_wluns() deadlocks trying to clear UFS_UPIU_RPMB_WLUN
>>>>>>> if sdev_rpmb is suspended and sdev_ufs_device is suspending.
>>>>>>> e.g. ufshcd_wl_suspend() is waiting on host_sem while ufshcd_err_handler()
>>>>>>> is running, at which point sdev_rpmb has already suspended.
>>>>>>>
>>>>>> Umm, I didn't understand this deadlock.
>>>>>> When you say, sdev_rpmb is suspended, does it mean runtime_suspended?
>>>>>> sdev_ufs_device is suspending - this can't be runtime_suspending, while ufshcd_err_handling_unprepare is running.
>>>>>>
>>>>>> If you've a call-stack of this deadlock, please can you share it with me. I'll also try to reproduce this.
>>>>>
>>>>> Yes it is system suspend. sdev_rpmb has suspended, sdev_ufs_device is waiting on host_sem.
>>>>> ufshcd_err_handler() holds host_sem. ufshcd_clear_ua_wlun(UFS_UPIU_RPMB_WLUN) gets stuck.
>>>>> I will get some call-stacks.
>>>>
>>> Hi Adrian,
>>>
>>> Thanks for the call stacks.
>>> From the current information, I can't say for sure why it'd get stuck in blk_queue_enter().
>>
>> I presume SCSI is leaving the RPMB WLUN device runtime suspended and consequently the queue status is RPM_SUSPENDED
>>
>>>
>>> I tried reproducing this issue on my setup yesterday but couldn't.
>>> Here's what I did:
>>> 1. sdev_rpmb is RPM_SUSPENDED, checked before initiating system suspend
>>> 2. sdev_ufs_device is RPM_RESUMED
>>> 3. I triggered system suspend (echo mem > /sys/power/state) and scheduled the error handler from ufshcd_wl_suspend().
>>> 4. Waited until error handler ran and then ufshcd_wl_suspend() blocks on host_sem.
>>> 5. The ufshcd_clear_wa_wlun(UFS_UPIU_RPMB_WLUN) went through fine.
>>>
>>> Do you've some specific steps to reproduce this or a script, perhaps? If so, please can you share it with me. I will try again.
>>
>> I was using a device that gives occasional errors, but I will what see I can do.
>
> I have attached 2 patches to reproduce the issue, and the test script I used.
> Note it is using down_timeout() so it doesn't lock up completely.
> The kernel messages are:
>
> [ 79.385456] ufshcd_wl_suspend: doing ufshcd_schedule_eh_work
> [ 79.385511] ufshcd_wl_suspend: sleeping 1000 ms
> [ 79.386916] ufshcd_err_handler: start
> [ 79.386979] ufshcd_err_handler: got sem
> [ 79.387053] ufshcd_err_handling_unprepare: start
> [ 79.387302] ufshcd_clear_ua_wluns: before ufshcd_clear_ua_wlun(hba, UFS_UPIU_RPMB_WLUN)
> [ 80.435878] ufshcd_wl_suspend: trying to get sem
> [ 85.683876] ufshcd_wl_suspend: failed to get sem
> [ 85.683993] ufs_device_wlun 0:0:0:49488: PM: failed to suspend async: error -22
> [ 85.686901] ufshcd_clear_ua_wluns: after ufshcd_clear_ua_wlun(hba, UFS_UPIU_RPMB_WLUN), ret 0
> [ 85.687025] ufshcd_err_handling_unprepare: finish
> [ 85.687090] ufshcd_err_handler: finish
>
>
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)
};
^ permalink raw reply [flat|nested] 31+ messages in thread* Re: [PATCH v20 1/2] scsi: ufs: Enable power management for wlun
@ 2021-04-23 8:01 ` Adrian Hunter
0 siblings, 0 replies; 31+ messages in thread
From: Adrian Hunter @ 2021-04-23 8:01 UTC (permalink / raw)
To: Asutosh Das (asd), cang, martin.petersen, linux-scsi
Cc: linux-arm-msm, Alim Akhtar, Avri Altman, James E.J. Bottomley,
Krzysztof Kozlowski, Stanley Chu, Andy Gross, Bjorn Andersson,
Steven Rostedt, Ingo Molnar, Matthias Brugger, Lee Jones,
Bean Huo, Kiwoong Kim, Colin Ian King, Wei Yongjun, Yue Hu,
Bart van Assche, Gustavo A. R. Silva, Dinghao Liu, Jaegeuk Kim,
Satya Tangirala, open list, moderated list:ARM/SAMSUNG S3C,
S5P AND EXYNOS ARM ARCHITECTURES, open list:ARM/SAMSUNG S3C,
S5P AND EXYNOS ARM ARCHITECTURES,
moderated list:UNIVERSAL FLASH STORAGE HOST CONTROLLER DRIVER...
On 23/04/21 9:18 am, Adrian Hunter wrote:
> On 23/04/21 7:23 am, Adrian Hunter wrote:
>> On 22/04/21 7:38 pm, Asutosh Das (asd) wrote:
>>> On 4/20/2021 12:42 AM, Adrian Hunter wrote:
>>>> On 20/04/21 7:15 am, Adrian Hunter wrote:
>>>>> On 20/04/21 12:53 am, Asutosh Das (asd) wrote:
>>>>>> On 4/19/2021 11:37 AM, Adrian Hunter wrote:
>>>>>>> On 16/04/21 10:49 pm, Asutosh Das wrote:
>>>>>>>>
>>>>>>>> Co-developed-by: Can Guo <cang@codeaurora.org>
>>>>>>>> Signed-off-by: Can Guo <cang@codeaurora.org>
>>>>>>>> Signed-off-by: Asutosh Das <asutoshd@codeaurora.org>
>>>>>>>> ---
>>>>>>>
>>>>>>> I came across 3 issues while testing. See comments below.
>>>>>>>
>>>>>> Hi Adrian
>>>>>> Thanks for the comments.
>>>>>>> <SNIP>
>>>>>>>
>>>>>>>> @@ -5794,7 +5839,7 @@ static void ufshcd_err_handling_unprepare(struct ufs_hba *hba)
>>>>>>>> if (ufshcd_is_clkscaling_supported(hba))
>>>>>>>> ufshcd_clk_scaling_suspend(hba, false);
>>>>>>>> ufshcd_clear_ua_wluns(hba);
>>>>>>>
>>>>>>> ufshcd_clear_ua_wluns() deadlocks trying to clear UFS_UPIU_RPMB_WLUN
>>>>>>> if sdev_rpmb is suspended and sdev_ufs_device is suspending.
>>>>>>> e.g. ufshcd_wl_suspend() is waiting on host_sem while ufshcd_err_handler()
>>>>>>> is running, at which point sdev_rpmb has already suspended.
>>>>>>>
>>>>>> Umm, I didn't understand this deadlock.
>>>>>> When you say, sdev_rpmb is suspended, does it mean runtime_suspended?
>>>>>> sdev_ufs_device is suspending - this can't be runtime_suspending, while ufshcd_err_handling_unprepare is running.
>>>>>>
>>>>>> If you've a call-stack of this deadlock, please can you share it with me. I'll also try to reproduce this.
>>>>>
>>>>> Yes it is system suspend. sdev_rpmb has suspended, sdev_ufs_device is waiting on host_sem.
>>>>> ufshcd_err_handler() holds host_sem. ufshcd_clear_ua_wlun(UFS_UPIU_RPMB_WLUN) gets stuck.
>>>>> I will get some call-stacks.
>>>>
>>> Hi Adrian,
>>>
>>> Thanks for the call stacks.
>>> From the current information, I can't say for sure why it'd get stuck in blk_queue_enter().
>>
>> I presume SCSI is leaving the RPMB WLUN device runtime suspended and consequently the queue status is RPM_SUSPENDED
>>
>>>
>>> I tried reproducing this issue on my setup yesterday but couldn't.
>>> Here's what I did:
>>> 1. sdev_rpmb is RPM_SUSPENDED, checked before initiating system suspend
>>> 2. sdev_ufs_device is RPM_RESUMED
>>> 3. I triggered system suspend (echo mem > /sys/power/state) and scheduled the error handler from ufshcd_wl_suspend().
>>> 4. Waited until error handler ran and then ufshcd_wl_suspend() blocks on host_sem.
>>> 5. The ufshcd_clear_wa_wlun(UFS_UPIU_RPMB_WLUN) went through fine.
>>>
>>> Do you've some specific steps to reproduce this or a script, perhaps? If so, please can you share it with me. I will try again.
>>
>> I was using a device that gives occasional errors, but I will what see I can do.
>
> I have attached 2 patches to reproduce the issue, and the test script I used.
> Note it is using down_timeout() so it doesn't lock up completely.
> The kernel messages are:
>
> [ 79.385456] ufshcd_wl_suspend: doing ufshcd_schedule_eh_work
> [ 79.385511] ufshcd_wl_suspend: sleeping 1000 ms
> [ 79.386916] ufshcd_err_handler: start
> [ 79.386979] ufshcd_err_handler: got sem
> [ 79.387053] ufshcd_err_handling_unprepare: start
> [ 79.387302] ufshcd_clear_ua_wluns: before ufshcd_clear_ua_wlun(hba, UFS_UPIU_RPMB_WLUN)
> [ 80.435878] ufshcd_wl_suspend: trying to get sem
> [ 85.683876] ufshcd_wl_suspend: failed to get sem
> [ 85.683993] ufs_device_wlun 0:0:0:49488: PM: failed to suspend async: error -22
> [ 85.686901] ufshcd_clear_ua_wluns: after ufshcd_clear_ua_wlun(hba, UFS_UPIU_RPMB_WLUN), ret 0
> [ 85.687025] ufshcd_err_handling_unprepare: finish
> [ 85.687090] ufshcd_err_handler: finish
>
>
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)
};
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 31+ messages in thread* Re: [PATCH v20 1/2] scsi: ufs: Enable power management for wlun
@ 2021-04-23 8:01 ` Adrian Hunter
0 siblings, 0 replies; 31+ messages in thread
From: Adrian Hunter @ 2021-04-23 8:01 UTC (permalink / raw)
To: Asutosh Das (asd), cang, martin.petersen, linux-scsi
Cc: linux-arm-msm, Alim Akhtar, Avri Altman, James E.J. Bottomley,
Krzysztof Kozlowski, Stanley Chu, Andy Gross, Bjorn Andersson,
Steven Rostedt, Ingo Molnar, Matthias Brugger, Lee Jones,
Bean Huo, Kiwoong Kim, Colin Ian King, Wei Yongjun, Yue Hu,
Bart van Assche, Gustavo A. R. Silva, Dinghao Liu, Jaegeuk Kim,
Satya Tangirala, open list, moderated list:ARM/SAMSUNG S3C,
S5P AND EXYNOS ARM ARCHITECTURES, open list:ARM/SAMSUNG S3C,
S5P AND EXYNOS ARM ARCHITECTURES,
moderated list:UNIVERSAL FLASH STORAGE HOST CONTROLLER DRIVER...
On 23/04/21 9:18 am, Adrian Hunter wrote:
> On 23/04/21 7:23 am, Adrian Hunter wrote:
>> On 22/04/21 7:38 pm, Asutosh Das (asd) wrote:
>>> On 4/20/2021 12:42 AM, Adrian Hunter wrote:
>>>> On 20/04/21 7:15 am, Adrian Hunter wrote:
>>>>> On 20/04/21 12:53 am, Asutosh Das (asd) wrote:
>>>>>> On 4/19/2021 11:37 AM, Adrian Hunter wrote:
>>>>>>> On 16/04/21 10:49 pm, Asutosh Das wrote:
>>>>>>>>
>>>>>>>> Co-developed-by: Can Guo <cang@codeaurora.org>
>>>>>>>> Signed-off-by: Can Guo <cang@codeaurora.org>
>>>>>>>> Signed-off-by: Asutosh Das <asutoshd@codeaurora.org>
>>>>>>>> ---
>>>>>>>
>>>>>>> I came across 3 issues while testing. See comments below.
>>>>>>>
>>>>>> Hi Adrian
>>>>>> Thanks for the comments.
>>>>>>> <SNIP>
>>>>>>>
>>>>>>>> @@ -5794,7 +5839,7 @@ static void ufshcd_err_handling_unprepare(struct ufs_hba *hba)
>>>>>>>> if (ufshcd_is_clkscaling_supported(hba))
>>>>>>>> ufshcd_clk_scaling_suspend(hba, false);
>>>>>>>> ufshcd_clear_ua_wluns(hba);
>>>>>>>
>>>>>>> ufshcd_clear_ua_wluns() deadlocks trying to clear UFS_UPIU_RPMB_WLUN
>>>>>>> if sdev_rpmb is suspended and sdev_ufs_device is suspending.
>>>>>>> e.g. ufshcd_wl_suspend() is waiting on host_sem while ufshcd_err_handler()
>>>>>>> is running, at which point sdev_rpmb has already suspended.
>>>>>>>
>>>>>> Umm, I didn't understand this deadlock.
>>>>>> When you say, sdev_rpmb is suspended, does it mean runtime_suspended?
>>>>>> sdev_ufs_device is suspending - this can't be runtime_suspending, while ufshcd_err_handling_unprepare is running.
>>>>>>
>>>>>> If you've a call-stack of this deadlock, please can you share it with me. I'll also try to reproduce this.
>>>>>
>>>>> Yes it is system suspend. sdev_rpmb has suspended, sdev_ufs_device is waiting on host_sem.
>>>>> ufshcd_err_handler() holds host_sem. ufshcd_clear_ua_wlun(UFS_UPIU_RPMB_WLUN) gets stuck.
>>>>> I will get some call-stacks.
>>>>
>>> Hi Adrian,
>>>
>>> Thanks for the call stacks.
>>> From the current information, I can't say for sure why it'd get stuck in blk_queue_enter().
>>
>> I presume SCSI is leaving the RPMB WLUN device runtime suspended and consequently the queue status is RPM_SUSPENDED
>>
>>>
>>> I tried reproducing this issue on my setup yesterday but couldn't.
>>> Here's what I did:
>>> 1. sdev_rpmb is RPM_SUSPENDED, checked before initiating system suspend
>>> 2. sdev_ufs_device is RPM_RESUMED
>>> 3. I triggered system suspend (echo mem > /sys/power/state) and scheduled the error handler from ufshcd_wl_suspend().
>>> 4. Waited until error handler ran and then ufshcd_wl_suspend() blocks on host_sem.
>>> 5. The ufshcd_clear_wa_wlun(UFS_UPIU_RPMB_WLUN) went through fine.
>>>
>>> Do you've some specific steps to reproduce this or a script, perhaps? If so, please can you share it with me. I will try again.
>>
>> I was using a device that gives occasional errors, but I will what see I can do.
>
> I have attached 2 patches to reproduce the issue, and the test script I used.
> Note it is using down_timeout() so it doesn't lock up completely.
> The kernel messages are:
>
> [ 79.385456] ufshcd_wl_suspend: doing ufshcd_schedule_eh_work
> [ 79.385511] ufshcd_wl_suspend: sleeping 1000 ms
> [ 79.386916] ufshcd_err_handler: start
> [ 79.386979] ufshcd_err_handler: got sem
> [ 79.387053] ufshcd_err_handling_unprepare: start
> [ 79.387302] ufshcd_clear_ua_wluns: before ufshcd_clear_ua_wlun(hba, UFS_UPIU_RPMB_WLUN)
> [ 80.435878] ufshcd_wl_suspend: trying to get sem
> [ 85.683876] ufshcd_wl_suspend: failed to get sem
> [ 85.683993] ufs_device_wlun 0:0:0:49488: PM: failed to suspend async: error -22
> [ 85.686901] ufshcd_clear_ua_wluns: after ufshcd_clear_ua_wlun(hba, UFS_UPIU_RPMB_WLUN), ret 0
> [ 85.687025] ufshcd_err_handling_unprepare: finish
> [ 85.687090] ufshcd_err_handler: finish
>
>
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)
};
_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek
^ permalink raw reply [flat|nested] 31+ messages in thread* Re: [PATCH v20 1/2] scsi: ufs: Enable power management for wlun
2021-04-23 8:01 ` Adrian Hunter
@ 2021-04-23 23:44 ` Asutosh Das (asd)
-1 siblings, 0 replies; 31+ messages in thread
From: Asutosh Das (asd) @ 2021-04-23 23:44 UTC (permalink / raw)
To: Adrian Hunter, cang, martin.petersen, linux-scsi
Cc: linux-arm-msm, Alim Akhtar, Avri Altman, James E.J. Bottomley,
Krzysztof Kozlowski, Stanley Chu, Andy Gross, Bjorn Andersson,
Steven Rostedt, Ingo Molnar, Matthias Brugger, Lee Jones,
Bean Huo, Kiwoong Kim, Colin Ian King, Wei Yongjun, Yue Hu,
Bart van Assche, Gustavo A. R. Silva, Dinghao Liu, Jaegeuk Kim,
Satya Tangirala, open list, moderated list:ARM/SAMSUNG S3C,
S5P AND EXYNOS ARM ARCHITECTURES, open list:ARM/SAMSUNG S3C,
S5P AND EXYNOS ARM ARCHITECTURES,
moderated list:UNIVERSAL FLASH STORAGE HOST CONTROLLER DRIVER...
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
^ permalink raw reply [flat|nested] 31+ messages in thread* Re: [PATCH v20 1/2] scsi: ufs: Enable power management for wlun
@ 2021-04-23 23:44 ` Asutosh Das (asd)
0 siblings, 0 replies; 31+ messages in thread
From: Asutosh Das (asd) @ 2021-04-23 23:44 UTC (permalink / raw)
To: Adrian Hunter, cang, martin.petersen, linux-scsi
Cc: linux-arm-msm, Alim Akhtar, Avri Altman, James E.J. Bottomley,
Krzysztof Kozlowski, Stanley Chu, Andy Gross, Bjorn Andersson,
Steven Rostedt, Ingo Molnar, Matthias Brugger, Lee Jones,
Bean Huo, Kiwoong Kim, Colin Ian King, Wei Yongjun, Yue Hu,
Bart van Assche, Gustavo A. R. Silva, Dinghao Liu, Jaegeuk Kim,
Satya Tangirala, open list, moderated list:ARM/SAMSUNG S3C,
S5P AND EXYNOS ARM ARCHITECTURES, open list:ARM/SAMSUNG S3C,
S5P AND EXYNOS ARM ARCHITECTURES,
moderated list:UNIVERSAL FLASH STORAGE HOST CONTROLLER DRIVER...
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
^ permalink raw reply [flat|nested] 31+ messages in thread