[PATCH v4 0/2] Enable power management for ufs wlun

[PATCH v4 0/2] Enable power management for ufs wlun

[Asutosh Das]

This patch attempts to fix a deadlock in ufs while sending SSU.
Recently, blk_queue_enter() added a check to not process requests if the
queue is suspended. That leads to a resume of the associated device which
is suspended. In ufs, that device is ufs device wlun and it's parent is
ufs_hba. This resume tries to resume ufs device wlun which in turn tries
to resume ufs_hba, which is already in the process of suspending, thus
causing a deadlock.

This patch takes care of:
* Suspending the ufs device lun only after all other luns are suspended
* Sending SSU during ufs device wlun suspend
* Clearing uac for rpmb and ufs device wlun
* Not sending commands to the device during host suspend

v3 RFC -> v4:
- Addressed Bart's comments
  * Except that I didn't get any checkpatch failures
- Addressed Avri's comments
- Addressed Adrian's comments
  * Added a check for deepsleep power mode
  * Removed a couple of forward declarations
  * Didn't separate the scsi drivers because in rpmb case it just sends uac
    in resume and it seemed pretty neat to me.
- Added sysfs changes to resume the devices before accessing

Asutosh Das (2):
  scsi: ufs: Enable power management for wlun
  ufs: sysfs: Resume the proper scsi device

 drivers/scsi/ufs/ufs-sysfs.c |  26 +--
 drivers/scsi/ufs/ufshcd.c    | 455 ++++++++++++++++++++++++++++++++++---------
 drivers/scsi/ufs/ufshcd.h    |   4 +
 include/trace/events/ufs.h   |  20 ++
 4 files changed, 397 insertions(+), 108 deletions(-)

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project.

[PATCH v4 1/2] scsi: ufs: Enable power management for wlun

[Asutosh Das]

During runtime-suspend of ufs host, the scsi devices are
already suspended and so are the queues associated with them.
But the ufs host sends SSU 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.

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/ufshcd.c  | 455 +++++++++++++++++++++++++++++++++++----------
 drivers/scsi/ufs/ufshcd.h  |   4 +
 include/trace/events/ufs.h |  20 ++
 3 files changed, 382 insertions(+), 97 deletions(-)

diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index 45624c7..1d8044a 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"
@@ -251,6 +252,11 @@ static int ufshcd_wb_toggle_flush_during_h8(struct ufs_hba *hba, bool set);
 static inline int ufshcd_wb_toggle_flush(struct ufs_hba *hba, bool enable);
 static void ufshcd_hba_vreg_set_lpm(struct ufs_hba *hba);
 static void ufshcd_hba_vreg_set_hpm(struct ufs_hba *hba);
+static int ufshcd_wl_runtime_suspend(struct device *dev);
+static int ufshcd_wl_runtime_resume(struct device *dev);
+static int ufshcd_wl_suspend(struct device *dev);
+static int ufshcd_wl_resume(struct device *dev);
+static void ufshcd_wl_shutdown(struct device *dev);
 
 static inline bool ufshcd_valid_tag(struct ufs_hba *hba, int tag)
 {
@@ -1556,7 +1562,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);
+	scsi_autopm_get_device(hba->sdev_ufs_device);
 	ufshcd_hold(hba, false);
 
 	hba->clk_scaling.is_enabled = value;
@@ -1572,7 +1578,7 @@ static ssize_t ufshcd_clkscale_enable_store(struct device *dev,
 	}
 
 	ufshcd_release(hba);
-	pm_runtime_put_sync(hba->dev);
+	scsi_autopm_put_device(hba->sdev_ufs_device);
 out:
 	up(&hba->host_sem);
 	return err ? err : count;
@@ -2572,6 +2578,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;
@@ -4808,6 +4825,41 @@ 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);
+		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)
+			return;
+
+		pm_runtime_put_noidle(&hba->sdev_ufs_device->sdev_gendev);
+		pm_runtime_mark_last_busy(&hba->sdev_ufs_device->sdev_gendev);
+	} else {
+		/* device wlun is probed */
+		hba->luns_avail--;
+	}
+}
+
+/**
  * ufshcd_slave_alloc - handle initial SCSI device configurations
  * @sdev: pointer to SCSI device
  *
@@ -4838,6 +4890,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;
 }
 
@@ -4984,16 +5038,9 @@ ufshcd_transfer_rsp_status(struct ufs_hba *hba, struct ufshcd_lrb *lrbp)
 			 * solution could be to abort the system suspend if
 			 * UFS device needs urgent BKOPs.
 			 */
-			if (!hba->pm_op_in_progress &&
-			    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);
-			}
+			if (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 */
@@ -5589,8 +5636,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);
+	scsi_autopm_get_device(hba->sdev_ufs_device);
+	scsi_autopm_put_device(hba->sdev_ufs_device);
 }
 
 /**
@@ -5607,7 +5654,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) {
@@ -5623,14 +5669,6 @@ static void ufshcd_exception_event_handler(struct work_struct *work)
 
 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;
 }
 
@@ -7254,6 +7292,15 @@ static int ufshcd_scsi_add_wlus(struct ufs_hba *hba)
 		goto out;
 	}
 	ufshcd_blk_pm_runtime_init(hba->sdev_ufs_device);
+	/*
+	 * A pm_runtime_put_sync is invoked when this device enables blk_pm_runtime
+	 * & would suspend the device-wlun upon timer expiry.
+	 * But suspending device wlun _may_ put the ufs device in the pre-defined
+	 * low power mode (SSU <rpm_lvl>). Probing of other luns may fail then.
+	 * Don't allow this suspend until all the luns have been probed.
+	 * See pm_runtime_mark_last_busy in ufshcd_setup_links.
+	 */
+	pm_runtime_get_noresume(&hba->sdev_ufs_device->sdev_gendev);
 	scsi_device_put(hba->sdev_ufs_device);
 
 	hba->sdev_rpmb = __scsi_add_device(hba->host, 0, 0,
@@ -7417,6 +7464,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);
@@ -7747,6 +7797,54 @@ static int ufshcd_device_params_init(struct ufs_hba *hba)
 	return ret;
 }
 
+static int ufshcd_wl_probe(struct device *dev)
+{
+	struct scsi_device *sdev = to_scsi_device(dev);
+
+	/*
+	 * Use this driver for both device wlun and rpmb lun
+	 * It's used to send uac to rpmb wlun and ssu to device wlun
+	 */
+	if (sdev && (is_rpmb_wlun(sdev) || is_device_wlun(sdev)))
+		return 0;
+	return -ENODEV;
+}
+
+static int ufshcd_wl_remove(struct device *dev)
+{
+	struct scsi_device *sdev = to_scsi_device(dev);
+
+	scsi_remove_device(sdev);
+	return 0;
+}
+
+static const struct dev_pm_ops ufshcd_wl_pm_ops = {
+	SET_RUNTIME_PM_OPS(ufshcd_wl_runtime_suspend, ufshcd_wl_runtime_resume, NULL)
+	SET_SYSTEM_SLEEP_PM_OPS(ufshcd_wl_suspend, ufshcd_wl_resume)
+};
+
+/**
+ * ufs_wlun - describes 2 wluns:
+ * ufs-device wlun - used to send pm commands
+ * and rpmb wlun - for rpmb commands
+ * All luns including rpmb 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_wlun = {
+	.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,
+	},
+};
+
 /**
  * ufshcd_add_lus - probe and add UFS logical units
  * @hba: per-adapter instance
@@ -7755,6 +7853,12 @@ static int ufshcd_add_lus(struct ufs_hba *hba)
 {
 	int ret;
 
+	ret = scsi_register_driver(&ufs_wlun.gendrv);
+	if (ret) {
+		dev_err(hba->dev, "Register device wlun driver failed: %d\n",
+			ret);
+		return ret;
+	}
 	/* Add required well known logical units to scsi mid layer */
 	ret = ufshcd_scsi_add_wlus(hba);
 	if (ret)
@@ -7964,6 +8068,7 @@ static void ufshcd_async_scan(void *data, async_cookie_t cookie)
 		pm_runtime_put_sync(hba->dev);
 		ufshcd_hba_exit(hba);
 	}
+	hba->init_done = true;
 }
 
 static const struct attribute_group *ufshcd_driver_groups[] = {
@@ -8475,7 +8580,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;
 
@@ -8650,31 +8756,19 @@ 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;
 	enum ufs_pm_level pm_lvl;
 	enum ufs_dev_pwr_mode req_dev_pwr_mode;
 	enum uic_link_state req_link_state;
 
-	hba->pm_op_in_progress = 1;
+	/*
+	 * Is invoked when the device wlun is added to sysfs
+	 * But by then hba->sdev_ufs_device may not be initialized.
+	 */
+	if (!hba->init_done)
+		return 0;
 	if (!ufshcd_is_shutdown_pm(pm_op)) {
 		pm_lvl = ufshcd_is_runtime_pm(pm_op) ?
 			 hba->rpm_lvl : hba->spm_lvl;
@@ -8690,24 +8784,23 @@ static int ufshcd_suspend(struct ufs_hba *hba, enum ufs_pm_op pm_op)
 	 * just gate the clocks.
 	 */
 	ufshcd_hold(hba, false);
-	hba->clk_gating.is_suspended = true;
 
 	if (ufshcd_is_clkscaling_supported(hba))
 		ufshcd_clk_scaling_suspend(hba, true);
 
 	if (req_dev_pwr_mode == UFS_ACTIVE_PWR_MODE &&
 			req_link_state == UIC_LINK_ACTIVE_STATE) {
-		goto disable_clks;
+		goto enable_scaling;
 	}
 
 	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)) {
@@ -8719,7 +8812,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);
@@ -8746,10 +8839,224 @@ 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;
+			if (!ret)
+				goto out;
 		}
 	}
+enable_scaling:
+	if (ufshcd_is_clkscaling_supported(hba))
+		ufshcd_clk_scaling_suspend(hba, false);
+
+	hba->dev_info.b_rpm_dev_flush_capable = false;
+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));
+	}
+	if (ret)
+		ufshcd_update_evt_hist(hba, UFS_EVT_WL_SUSP_ERR, (u32)ret);
+	ufshcd_release(hba);
+	return ret;
+}
+
+static int __ufshcd_wl_resume(struct ufs_hba *hba, enum ufs_pm_op pm_op)
+{
+	int ret = 0;
+
+	if (!hba->init_done)
+		return 0;
+
+	ufshcd_hold(hba, false);
+	if (ufshcd_is_ufs_dev_deepsleep(hba)) {
+		/* The device is in DeepSleep but hba was not suspended */
+		ret = ufshcd_reset_and_restore(hba);
+		if (ret)
+			goto out;
+	}
+
+	if (!ufshcd_is_ufs_dev_active(hba)) {
+		ret = ufshcd_set_dev_pwr_mode(hba, UFS_ACTIVE_PWR_MODE);
+		if (ret)
+			goto out;
+	}
+
+	if (ufshcd_keep_autobkops_enabled_except_suspend(hba))
+		ufshcd_enable_auto_bkops(hba);
+	else
+		/*
+		 * If BKOPs operations are urgently needed at this moment then
+		 * keep auto-bkops enabled or else disable it.
+		 */
+		ufshcd_urgent_bkops(hba);
+
+	if (hba->clk_scaling.is_allowed)
+		ufshcd_resume_clkscaling(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);
+	}
+
+out:
+	if (ret)
+		ufshcd_update_evt_hist(hba, UFS_EVT_WL_RES_ERR, (u32)ret);
+	ufshcd_release(hba);
+	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);
+
+	if (is_rpmb_wlun(sdev))
+		return 0;
+	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;
+	ktime_t start = ktime_get();
+
+	hba = shost_priv(sdev->host);
+
+	if (is_rpmb_wlun(sdev)) {
+		if (hba->sdev_rpmb)
+			return ufshcd_clear_ua_wlun(hba, UFS_UPIU_RPMB_WLUN);
+		return 0;
+	}
+
+	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;
+}
+
+static int ufshcd_wl_suspend(struct device *dev)
+{
+	struct scsi_device *sdev = to_scsi_device(dev);
+	struct ufs_hba *hba;
+	int ret;
+	ktime_t start = ktime_get();
+
+	if (is_rpmb_wlun(sdev))
+		return 0;
+	hba = shost_priv(sdev->host);
+	ret = __ufshcd_wl_suspend(hba, UFS_SYSTEM_PM);
+	if (ret)
+		dev_err(&sdev->sdev_gendev, "%s failed: %d\n", __func__,  ret);
+
+	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;
+	ktime_t start = ktime_get();
+
+	if (pm_runtime_suspended(dev))
+		return 0;
+	hba = shost_priv(sdev->host);
+
+	if (is_rpmb_wlun(sdev)) {
+		if (hba->sdev_rpmb)
+			return ufshcd_clear_ua_wlun(hba, UFS_UPIU_RPMB_WLUN);
+		return 0;
+	}
+
+	ret = __ufshcd_wl_resume(hba, UFS_SYSTEM_PM);
+	if (ret)
+		dev_err(&sdev->sdev_gendev, "%s failed: %d\n", __func__, ret);
+
+	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);
+
+	return ret;
+}
+
+static void ufshcd_wl_shutdown(struct device *dev)
+{
+	struct scsi_device *sdev = to_scsi_device(dev);
+	struct ufs_hba *hba;
+
+	if (is_rpmb_wlun(sdev))
+		return;
+	hba = shost_priv(sdev->host);
+	scsi_device_quiesce(sdev);
+	shost_for_each_device(sdev, hba->host) {
+		if (sdev == hba->sdev_ufs_device)
+			continue;
+		scsi_remove_device(sdev);
+	}
+	__ufshcd_wl_suspend(hba, UFS_SHUTDOWN_PM);
+}
+
+/**
+ * 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 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
+ * link is powered off.
+ *
+ * Returns 0 for success and non-zero for failure
+ */
+static int ufshcd_suspend(struct ufs_hba *hba, enum ufs_pm_op pm_op)
+{
+	int ret = 0;
+	int check_for_bkops;
+	enum ufs_pm_level pm_lvl;
+	enum ufs_dev_pwr_mode req_dev_pwr_mode;
+	enum uic_link_state req_link_state;
+
+	hba->pm_op_in_progress = 1;
+	if (!ufshcd_is_shutdown_pm(pm_op)) {
+		pm_lvl = ufshcd_is_runtime_pm(pm_op) ?
+			 hba->rpm_lvl : hba->spm_lvl;
+		req_dev_pwr_mode = ufs_get_pm_lvl_to_dev_pwr_mode(pm_lvl);
+		req_link_state = ufs_get_pm_lvl_to_link_pwr_state(pm_lvl);
+	} else {
+		req_dev_pwr_mode = UFS_POWERDOWN_PWR_MODE;
+		req_link_state = UIC_LINK_OFF_STATE;
+	}
+
+	/*
+	 * If we can't transition into any of the low power modes
+	 * just gate the clocks.
+	 */
+	ufshcd_hold(hba, false);
+	hba->clk_gating.is_suspended = true;
 
 	/*
 	 * In the case of DeepSleep, the device is expected to remain powered
@@ -8758,9 +9065,8 @@ static int ufshcd_suspend(struct ufs_hba *hba, enum ufs_pm_op pm_op)
 	check_for_bkops = !ufshcd_is_ufs_dev_deepsleep(hba);
 	ret = ufshcd_link_state_transition(hba, req_link_state, check_for_bkops);
 	if (ret)
-		goto set_dev_active;
+		goto enable_gating;
 
-disable_clks:
 	/*
 	 * Call vendor specific suspend callback. As these callbacks may access
 	 * vendor specific host controller register space call them before the
@@ -8790,7 +9096,6 @@ static int ufshcd_suspend(struct ufs_hba *hba, enum ufs_pm_op pm_op)
 	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
@@ -8804,28 +9109,15 @@ static int ufshcd_suspend(struct ufs_hba *hba, enum ufs_pm_op pm_op)
 		ufshcd_set_link_active(hba);
 	else if (ufshcd_is_link_off(hba))
 		ufshcd_host_reset_and_restore(hba);
-set_dev_active:
 	/* Can also get here needing to exit DeepSleep */
 	if (ufshcd_is_ufs_dev_deepsleep(hba)) {
 		ufshcd_device_reset(hba);
 		ufshcd_host_reset_and_restore(hba);
 	}
-	if (!ufshcd_set_dev_pwr_mode(hba, UFS_ACTIVE_PWR_MODE))
-		ufshcd_disable_auto_bkops(hba);
 enable_gating:
-	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)
@@ -8846,10 +9138,8 @@ static int ufshcd_suspend(struct ufs_hba *hba, enum ufs_pm_op pm_op)
 static int ufshcd_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);
@@ -8901,43 +9191,14 @@ static int ufshcd_resume(struct ufs_hba *hba, enum ufs_pm_op pm_op)
 			goto vendor_suspend;
 	}
 
-	if (!ufshcd_is_ufs_dev_active(hba)) {
-		ret = ufshcd_set_dev_pwr_mode(hba, UFS_ACTIVE_PWR_MODE);
-		if (ret)
-			goto set_old_link_state;
-	}
-
-	if (ufshcd_keep_autobkops_enabled_except_suspend(hba))
-		ufshcd_enable_auto_bkops(hba);
-	else
-		/*
-		 * If BKOPs operations are urgently needed at this moment then
-		 * keep auto-bkops enabled or else disable it.
-		 */
-		ufshcd_urgent_bkops(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);
 
 	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:
diff --git a/drivers/scsi/ufs/ufshcd.h b/drivers/scsi/ufs/ufshcd.h
index ee61f82..7662387 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,
@@ -841,6 +843,8 @@ struct ufs_hba {
 #ifdef CONFIG_DEBUG_FS
 	struct dentry *debugfs_root;
 #endif
+	bool init_done;
+	u32 luns_avail;
 };
 
 /* Returns true if clocks can be gated. Otherwise false */
diff --git a/include/trace/events/ufs.h b/include/trace/events/ufs.h
index e151477..d9d233b 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.

Re: [PATCH v4 1/2] scsi: ufs: Enable power management for wlun

[Adrian Hunter]

On 23/02/21 1:04 am, 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 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.

Hi

I have a few more questions and comments.

At a glance it looked to me like the SCSI bus will leave a SCSI device
runtime suspended at system suspend, but UFS device spm_lvl and rpm_lvl need
not be the same.  Do you know how SCSI handles that case?

> 
> 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/ufshcd.c  | 455 +++++++++++++++++++++++++++++++++++----------
>  drivers/scsi/ufs/ufshcd.h  |   4 +
>  include/trace/events/ufs.h |  20 ++
>  3 files changed, 382 insertions(+), 97 deletions(-)
> 
> diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
> index 45624c7..1d8044a 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"
> @@ -251,6 +252,11 @@ static int ufshcd_wb_toggle_flush_during_h8(struct ufs_hba *hba, bool set);
>  static inline int ufshcd_wb_toggle_flush(struct ufs_hba *hba, bool enable);
>  static void ufshcd_hba_vreg_set_lpm(struct ufs_hba *hba);
>  static void ufshcd_hba_vreg_set_hpm(struct ufs_hba *hba);
> +static int ufshcd_wl_runtime_suspend(struct device *dev);
> +static int ufshcd_wl_runtime_resume(struct device *dev);
> +static int ufshcd_wl_suspend(struct device *dev);
> +static int ufshcd_wl_resume(struct device *dev);
> +static void ufshcd_wl_shutdown(struct device *dev);
>  
>  static inline bool ufshcd_valid_tag(struct ufs_hba *hba, int tag)
>  {
> @@ -1556,7 +1562,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);
> +	scsi_autopm_get_device(hba->sdev_ufs_device);
>  	ufshcd_hold(hba, false);
>  
>  	hba->clk_scaling.is_enabled = value;
> @@ -1572,7 +1578,7 @@ static ssize_t ufshcd_clkscale_enable_store(struct device *dev,
>  	}
>  
>  	ufshcd_release(hba);
> -	pm_runtime_put_sync(hba->dev);
> +	scsi_autopm_put_device(hba->sdev_ufs_device);
>  out:
>  	up(&hba->host_sem);
>  	return err ? err : count;
> @@ -2572,6 +2578,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;
> @@ -4808,6 +4825,41 @@ 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);
> +		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)
> +			return;
> +
> +		pm_runtime_put_noidle(&hba->sdev_ufs_device->sdev_gendev);
> +		pm_runtime_mark_last_busy(&hba->sdev_ufs_device->sdev_gendev);
> +	} else {
> +		/* device wlun is probed */
> +		hba->luns_avail--;
> +	}
> +}
> +
> +/**
>   * ufshcd_slave_alloc - handle initial SCSI device configurations
>   * @sdev: pointer to SCSI device
>   *
> @@ -4838,6 +4890,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;
>  }
>  
> @@ -4984,16 +5038,9 @@ ufshcd_transfer_rsp_status(struct ufs_hba *hba, struct ufshcd_lrb *lrbp)
>  			 * solution could be to abort the system suspend if
>  			 * UFS device needs urgent BKOPs.
>  			 */
> -			if (!hba->pm_op_in_progress &&
> -			    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);
> -			}
> +			if (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 */
> @@ -5589,8 +5636,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);
> +	scsi_autopm_get_device(hba->sdev_ufs_device);
> +	scsi_autopm_put_device(hba->sdev_ufs_device);
>  }
>  
>  /**
> @@ -5607,7 +5654,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) {
> @@ -5623,14 +5669,6 @@ static void ufshcd_exception_event_handler(struct work_struct *work)
>  
>  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;
>  }
>  
> @@ -7254,6 +7292,15 @@ static int ufshcd_scsi_add_wlus(struct ufs_hba *hba)
>  		goto out;
>  	}
>  	ufshcd_blk_pm_runtime_init(hba->sdev_ufs_device);
> +	/*
> +	 * A pm_runtime_put_sync is invoked when this device enables blk_pm_runtime
> +	 * & would suspend the device-wlun upon timer expiry.
> +	 * But suspending device wlun _may_ put the ufs device in the pre-defined
> +	 * low power mode (SSU <rpm_lvl>). Probing of other luns may fail then.
> +	 * Don't allow this suspend until all the luns have been probed.
> +	 * See pm_runtime_mark_last_busy in ufshcd_setup_links.
> +	 */
> +	pm_runtime_get_noresume(&hba->sdev_ufs_device->sdev_gendev);
>  	scsi_device_put(hba->sdev_ufs_device);
>  
>  	hba->sdev_rpmb = __scsi_add_device(hba->host, 0, 0,
> @@ -7417,6 +7464,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);
> @@ -7747,6 +7797,54 @@ static int ufshcd_device_params_init(struct ufs_hba *hba)
>  	return ret;
>  }
>  
> +static int ufshcd_wl_probe(struct device *dev)
> +{
> +	struct scsi_device *sdev = to_scsi_device(dev);
> +
> +	/*
> +	 * Use this driver for both device wlun and rpmb lun
> +	 * It's used to send uac to rpmb wlun and ssu to device wlun
> +	 */
> +	if (sdev && (is_rpmb_wlun(sdev) || is_device_wlun(sdev)))
> +		return 0;
> +	return -ENODEV;
> +}
> +
> +static int ufshcd_wl_remove(struct device *dev)
> +{
> +	struct scsi_device *sdev = to_scsi_device(dev);
> +
> +	scsi_remove_device(sdev);

The 'probe' did not do the __scsi_add_device(), so does the 'remove' need to
do scsi_remove_device()?

> +	return 0;
> +}
> +
> +static const struct dev_pm_ops ufshcd_wl_pm_ops = {
> +	SET_RUNTIME_PM_OPS(ufshcd_wl_runtime_suspend, ufshcd_wl_runtime_resume, NULL)
> +	SET_SYSTEM_SLEEP_PM_OPS(ufshcd_wl_suspend, ufshcd_wl_resume)

.poweroff needs to set the power mode to PowerDown irrespective of spm_lvl,
so not exactly the same as ufshcd_wl_suspend.

Also, ufshcd_pci_poweroff() in ufshcd-pci.c should be removed since it won't
work anymore.

> +};
> +
> +/**
> + * ufs_wlun - describes 2 wluns:
> + * ufs-device wlun - used to send pm commands
> + * and rpmb wlun - for rpmb commands
> + * All luns including rpmb 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_wlun = {
> +	.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,
> +	},
> +};

WRT to RPMB, aren't all the callbacks optional, so that would leave only
something like below.  Would that work?

#define wlun_dev_to_hba(dv) shost_priv(to_scsi_device(dv)->host)

static int ufs_rpmb_probe(struct device *dev)
{
    return is_rpmb_wlun(to_scsi_device(dev)) ? 0 : -ENODEV;
}

static int ufs_rpmb_runtime_resume(struct device *dev)
{
    struct ufs_hba *hba = wlun_dev_to_hba(dev);

    if (hba->sdev_rpmb)
        return ufshcd_clear_ua_wlun(hba, UFS_UPIU_RPMB_WLUN);
    return 0;
}

static int ufs_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_ua_wlun(hba, UFS_UPIU_RPMB_WLUN);
    return 0;
}

static const struct dev_pm_ops ufs_rpmb_pm_ops = {
    SET_RUNTIME_PM_OPS(NULL, ufs_rpmb_runtime_resume, NULL)
    SET_SYSTEM_SLEEP_PM_OPS(NULL, ufs_rpmb_resume)
};

static struct scsi_driver ufs_rpmb_template = {
    .gendrv = {
        .name = "ufs_rpmb_wlun",
        .owner = THIS_MODULE,
        .probe = ufs_rpmb_probe,
        .pm = &ufs_rpmb_pm_ops,
    },
};


> +
>  /**
>   * ufshcd_add_lus - probe and add UFS logical units
>   * @hba: per-adapter instance
> @@ -7755,6 +7853,12 @@ static int ufshcd_add_lus(struct ufs_hba *hba)
>  {
>  	int ret;
>  
> +	ret = scsi_register_driver(&ufs_wlun.gendrv);

That will attempt to register the driver every time a UFS host controller is
added (e.g. what happens if you unbind and re-bind the host controller).
Probably needs to be in ufshcd_core_init() with corresponding unregister in
ufshcd_core_exit()

> +	if (ret) {
> +		dev_err(hba->dev, "Register device wlun driver failed: %d\n",
> +			ret);
> +		return ret;
> +	}
>  	/* Add required well known logical units to scsi mid layer */
>  	ret = ufshcd_scsi_add_wlus(hba);
>  	if (ret)
> @@ -7964,6 +8068,7 @@ static void ufshcd_async_scan(void *data, async_cookie_t cookie)
>  		pm_runtime_put_sync(hba->dev);
>  		ufshcd_hba_exit(hba);
>  	}
> +	hba->init_done = true;
>  }
>  
>  static const struct attribute_group *ufshcd_driver_groups[] = {
> @@ -8475,7 +8580,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;
>  
> @@ -8650,31 +8756,19 @@ 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;
>  	enum ufs_pm_level pm_lvl;
>  	enum ufs_dev_pwr_mode req_dev_pwr_mode;
>  	enum uic_link_state req_link_state;
>  
> -	hba->pm_op_in_progress = 1;
> +	/*
> +	 * Is invoked when the device wlun is added to sysfs
> +	 * But by then hba->sdev_ufs_device may not be initialized.
> +	 */
> +	if (!hba->init_done)
> +		return 0;
>  	if (!ufshcd_is_shutdown_pm(pm_op)) {
>  		pm_lvl = ufshcd_is_runtime_pm(pm_op) ?
>  			 hba->rpm_lvl : hba->spm_lvl;
> @@ -8690,24 +8784,23 @@ static int ufshcd_suspend(struct ufs_hba *hba, enum ufs_pm_op pm_op)
>  	 * just gate the clocks.
>  	 */
>  	ufshcd_hold(hba, false);
> -	hba->clk_gating.is_suspended = true;
>  
>  	if (ufshcd_is_clkscaling_supported(hba))
>  		ufshcd_clk_scaling_suspend(hba, true);
>  
>  	if (req_dev_pwr_mode == UFS_ACTIVE_PWR_MODE &&
>  			req_link_state == UIC_LINK_ACTIVE_STATE) {
> -		goto disable_clks;
> +		goto enable_scaling;
>  	}
>  
>  	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)) {
> @@ -8719,7 +8812,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);
> @@ -8746,10 +8839,224 @@ 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;
> +			if (!ret)
> +				goto out;
>  		}
>  	}
> +enable_scaling:
> +	if (ufshcd_is_clkscaling_supported(hba))
> +		ufshcd_clk_scaling_suspend(hba, false);
> +
> +	hba->dev_info.b_rpm_dev_flush_capable = false;
> +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));
> +	}
> +	if (ret)
> +		ufshcd_update_evt_hist(hba, UFS_EVT_WL_SUSP_ERR, (u32)ret);
> +	ufshcd_release(hba);
> +	return ret;
> +}
> +
> +static int __ufshcd_wl_resume(struct ufs_hba *hba, enum ufs_pm_op pm_op)
> +{
> +	int ret = 0;
> +
> +	if (!hba->init_done)
> +		return 0;
> +
> +	ufshcd_hold(hba, false);
> +	if (ufshcd_is_ufs_dev_deepsleep(hba)) {
> +		/* The device is in DeepSleep but hba was not suspended */
> +		ret = ufshcd_reset_and_restore(hba);
> +		if (ret)
> +			goto out;
> +	}
> +
> +	if (!ufshcd_is_ufs_dev_active(hba)) {
> +		ret = ufshcd_set_dev_pwr_mode(hba, UFS_ACTIVE_PWR_MODE);
> +		if (ret)
> +			goto out;
> +	}
> +
> +	if (ufshcd_keep_autobkops_enabled_except_suspend(hba))
> +		ufshcd_enable_auto_bkops(hba);
> +	else
> +		/*
> +		 * If BKOPs operations are urgently needed at this moment then
> +		 * keep auto-bkops enabled or else disable it.
> +		 */
> +		ufshcd_urgent_bkops(hba);
> +
> +	if (hba->clk_scaling.is_allowed)
> +		ufshcd_resume_clkscaling(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);
> +	}
> +
> +out:
> +	if (ret)
> +		ufshcd_update_evt_hist(hba, UFS_EVT_WL_RES_ERR, (u32)ret);
> +	ufshcd_release(hba);
> +	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);
> +
> +	if (is_rpmb_wlun(sdev))
> +		return 0;
> +	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;
> +	ktime_t start = ktime_get();
> +
> +	hba = shost_priv(sdev->host);
> +
> +	if (is_rpmb_wlun(sdev)) {
> +		if (hba->sdev_rpmb)
> +			return ufshcd_clear_ua_wlun(hba, UFS_UPIU_RPMB_WLUN);
> +		return 0;
> +	}
> +
> +	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;
> +}
> +
> +static int ufshcd_wl_suspend(struct device *dev)
> +{
> +	struct scsi_device *sdev = to_scsi_device(dev);
> +	struct ufs_hba *hba;
> +	int ret;
> +	ktime_t start = ktime_get();
> +
> +	if (is_rpmb_wlun(sdev))
> +		return 0;
> +	hba = shost_priv(sdev->host);
> +	ret = __ufshcd_wl_suspend(hba, UFS_SYSTEM_PM);
> +	if (ret)
> +		dev_err(&sdev->sdev_gendev, "%s failed: %d\n", __func__,  ret);
> +
> +	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;
> +	ktime_t start = ktime_get();
> +
> +	if (pm_runtime_suspended(dev))
> +		return 0;
> +	hba = shost_priv(sdev->host);
> +
> +	if (is_rpmb_wlun(sdev)) {
> +		if (hba->sdev_rpmb)
> +			return ufshcd_clear_ua_wlun(hba, UFS_UPIU_RPMB_WLUN);
> +		return 0;
> +	}
> +
> +	ret = __ufshcd_wl_resume(hba, UFS_SYSTEM_PM);
> +	if (ret)
> +		dev_err(&sdev->sdev_gendev, "%s failed: %d\n", __func__, ret);
> +
> +	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);
> +
> +	return ret;
> +}
> +
> +static void ufshcd_wl_shutdown(struct device *dev)
> +{
> +	struct scsi_device *sdev = to_scsi_device(dev);
> +	struct ufs_hba *hba;
> +
> +	if (is_rpmb_wlun(sdev))
> +		return;
> +	hba = shost_priv(sdev->host);
> +	scsi_device_quiesce(sdev);
> +	shost_for_each_device(sdev, hba->host) {
> +		if (sdev == hba->sdev_ufs_device)
> +			continue;
> +		scsi_remove_device(sdev);

Shutdown needs to quiesce devices.  Removing devices is not advisable
because the upper layers cannot respond at this point, causing deadlocks.
Is scsi_remove_device() really needed here?

> +	}
> +	__ufshcd_wl_suspend(hba, UFS_SHUTDOWN_PM);
> +}
> +
> +/**
> + * 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 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
> + * link is powered off.
> + *
> + * Returns 0 for success and non-zero for failure
> + */
> +static int ufshcd_suspend(struct ufs_hba *hba, enum ufs_pm_op pm_op)
> +{
> +	int ret = 0;
> +	int check_for_bkops;
> +	enum ufs_pm_level pm_lvl;
> +	enum ufs_dev_pwr_mode req_dev_pwr_mode;
> +	enum uic_link_state req_link_state;
> +
> +	hba->pm_op_in_progress = 1;
> +	if (!ufshcd_is_shutdown_pm(pm_op)) {
> +		pm_lvl = ufshcd_is_runtime_pm(pm_op) ?
> +			 hba->rpm_lvl : hba->spm_lvl;
> +		req_dev_pwr_mode = ufs_get_pm_lvl_to_dev_pwr_mode(pm_lvl);
> +		req_link_state = ufs_get_pm_lvl_to_link_pwr_state(pm_lvl);
> +	} else {
> +		req_dev_pwr_mode = UFS_POWERDOWN_PWR_MODE;
> +		req_link_state = UIC_LINK_OFF_STATE;
> +	}
> +
> +	/*
> +	 * If we can't transition into any of the low power modes
> +	 * just gate the clocks.
> +	 */
> +	ufshcd_hold(hba, false);
> +	hba->clk_gating.is_suspended = true;
>  
>  	/*
>  	 * In the case of DeepSleep, the device is expected to remain powered
> @@ -8758,9 +9065,8 @@ static int ufshcd_suspend(struct ufs_hba *hba, enum ufs_pm_op pm_op)
>  	check_for_bkops = !ufshcd_is_ufs_dev_deepsleep(hba);
>  	ret = ufshcd_link_state_transition(hba, req_link_state, check_for_bkops);
>  	if (ret)
> -		goto set_dev_active;
> +		goto enable_gating;
>  
> -disable_clks:
>  	/*
>  	 * Call vendor specific suspend callback. As these callbacks may access
>  	 * vendor specific host controller register space call them before the
> @@ -8790,7 +9096,6 @@ static int ufshcd_suspend(struct ufs_hba *hba, enum ufs_pm_op pm_op)
>  	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
> @@ -8804,28 +9109,15 @@ static int ufshcd_suspend(struct ufs_hba *hba, enum ufs_pm_op pm_op)
>  		ufshcd_set_link_active(hba);
>  	else if (ufshcd_is_link_off(hba))
>  		ufshcd_host_reset_and_restore(hba);
> -set_dev_active:
>  	/* Can also get here needing to exit DeepSleep */
>  	if (ufshcd_is_ufs_dev_deepsleep(hba)) {
>  		ufshcd_device_reset(hba);
>  		ufshcd_host_reset_and_restore(hba);
>  	}
> -	if (!ufshcd_set_dev_pwr_mode(hba, UFS_ACTIVE_PWR_MODE))
> -		ufshcd_disable_auto_bkops(hba);
>  enable_gating:
> -	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)
> @@ -8846,10 +9138,8 @@ static int ufshcd_suspend(struct ufs_hba *hba, enum ufs_pm_op pm_op)
>  static int ufshcd_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);
> @@ -8901,43 +9191,14 @@ static int ufshcd_resume(struct ufs_hba *hba, enum ufs_pm_op pm_op)
>  			goto vendor_suspend;
>  	}
>  
> -	if (!ufshcd_is_ufs_dev_active(hba)) {
> -		ret = ufshcd_set_dev_pwr_mode(hba, UFS_ACTIVE_PWR_MODE);
> -		if (ret)
> -			goto set_old_link_state;
> -	}
> -
> -	if (ufshcd_keep_autobkops_enabled_except_suspend(hba))
> -		ufshcd_enable_auto_bkops(hba);
> -	else
> -		/*
> -		 * If BKOPs operations are urgently needed at this moment then
> -		 * keep auto-bkops enabled or else disable it.
> -		 */
> -		ufshcd_urgent_bkops(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);
>  
>  	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:
> diff --git a/drivers/scsi/ufs/ufshcd.h b/drivers/scsi/ufs/ufshcd.h
> index ee61f82..7662387 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,
> @@ -841,6 +843,8 @@ struct ufs_hba {
>  #ifdef CONFIG_DEBUG_FS
>  	struct dentry *debugfs_root;
>  #endif
> +	bool init_done;
> +	u32 luns_avail;
>  };
>  
>  /* Returns true if clocks can be gated. Otherwise false */
> diff --git a/include/trace/events/ufs.h b/include/trace/events/ufs.h
> index e151477..d9d233b 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,
> 

Re: [PATCH v4 1/2] scsi: ufs: Enable power management for wlun

[Asutosh Das]

On Tue, Feb 23 2021 at 12:23 -0800, Adrian Hunter wrote:
>On 23/02/21 1:04 am, 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 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.
>
>Hi
>
Hi Adrian
>I have a few more questions and comments.
>
Thanks for the review.
I'll go through your comments and fix it in the next version.

>At a glance it looked to me like the SCSI bus will leave a SCSI device
>runtime suspended at system suspend, but UFS device spm_lvl and rpm_lvl need
>not be the same.  Do you know how SCSI handles that case?
>

Good point.

Yes, it says that:
 /*
  * All the high-level SCSI drivers that implement runtime
  * PM treat runtime suspend, system suspend, and system
  * hibernate nearly identically. In all cases the requirements
  * for runtime suspension are stricter.
  */

It's not true in case of ufs.

I can think of a couple of approaches:

Adding a couple of more cb() to ufs device wlun:
1. .prepare() -> runtime resume the ufs device -> pm_runtime_get_noresume()
2. .resume_early() -> pm_runtime_put_noidle()

Or,

Add a check to SCSI pm such that, should a device expose a capability saying
that it's runtime-pm differs from its system-pm, scsi-pm invokes the said device's
system-pm cb(). I'm not sure if this approach would violate any underlying assumptions.

Also, I'm not sure which would be a better approach or if there's another approach entirely.
Appreciate any insights.

>>
>> 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/ufshcd.c  | 455 +++++++++++++++++++++++++++++++++++----------
>>  drivers/scsi/ufs/ufshcd.h  |   4 +
>>  include/trace/events/ufs.h |  20 ++
>>  3 files changed, 382 insertions(+), 97 deletions(-)
>>
>> diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
>> index 45624c7..1d8044a 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"
>> @@ -251,6 +252,11 @@ static int ufshcd_wb_toggle_flush_during_h8(struct ufs_hba *hba, bool set);
>>  static inline int ufshcd_wb_toggle_flush(struct ufs_hba *hba, bool enable);
>>  static void ufshcd_hba_vreg_set_lpm(struct ufs_hba *hba);
>>  static void ufshcd_hba_vreg_set_hpm(struct ufs_hba *hba);
>> +static int ufshcd_wl_runtime_suspend(struct device *dev);
>> +static int ufshcd_wl_runtime_resume(struct device *dev);
>> +static int ufshcd_wl_suspend(struct device *dev);
>> +static int ufshcd_wl_resume(struct device *dev);
>> +static void ufshcd_wl_shutdown(struct device *dev);
>>
>>  static inline bool ufshcd_valid_tag(struct ufs_hba *hba, int tag)
>>  {
>> @@ -1556,7 +1562,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);
>> +	scsi_autopm_get_device(hba->sdev_ufs_device);
>>  	ufshcd_hold(hba, false);
>>
>>  	hba->clk_scaling.is_enabled = value;
>> @@ -1572,7 +1578,7 @@ static ssize_t ufshcd_clkscale_enable_store(struct device *dev,
>>  	}
>>
>>  	ufshcd_release(hba);
>> -	pm_runtime_put_sync(hba->dev);
>> +	scsi_autopm_put_device(hba->sdev_ufs_device);
>>  out:
>>  	up(&hba->host_sem);
>>  	return err ? err : count;
>> @@ -2572,6 +2578,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;
>> @@ -4808,6 +4825,41 @@ 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);
>> +		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)
>> +			return;
>> +
>> +		pm_runtime_put_noidle(&hba->sdev_ufs_device->sdev_gendev);
>> +		pm_runtime_mark_last_busy(&hba->sdev_ufs_device->sdev_gendev);
>> +	} else {
>> +		/* device wlun is probed */
>> +		hba->luns_avail--;
>> +	}
>> +}
>> +
>> +/**
>>   * ufshcd_slave_alloc - handle initial SCSI device configurations
>>   * @sdev: pointer to SCSI device
>>   *
>> @@ -4838,6 +4890,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;
>>  }
>>
>> @@ -4984,16 +5038,9 @@ ufshcd_transfer_rsp_status(struct ufs_hba *hba, struct ufshcd_lrb *lrbp)
>>  			 * solution could be to abort the system suspend if
>>  			 * UFS device needs urgent BKOPs.
>>  			 */
>> -			if (!hba->pm_op_in_progress &&
>> -			    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);
>> -			}
>> +			if (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 */
>> @@ -5589,8 +5636,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);
>> +	scsi_autopm_get_device(hba->sdev_ufs_device);
>> +	scsi_autopm_put_device(hba->sdev_ufs_device);
>>  }
>>
>>  /**
>> @@ -5607,7 +5654,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) {
>> @@ -5623,14 +5669,6 @@ static void ufshcd_exception_event_handler(struct work_struct *work)
>>
>>  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;
>>  }
>>
>> @@ -7254,6 +7292,15 @@ static int ufshcd_scsi_add_wlus(struct ufs_hba *hba)
>>  		goto out;
>>  	}
>>  	ufshcd_blk_pm_runtime_init(hba->sdev_ufs_device);
>> +	/*
>> +	 * A pm_runtime_put_sync is invoked when this device enables blk_pm_runtime
>> +	 * & would suspend the device-wlun upon timer expiry.
>> +	 * But suspending device wlun _may_ put the ufs device in the pre-defined
>> +	 * low power mode (SSU <rpm_lvl>). Probing of other luns may fail then.
>> +	 * Don't allow this suspend until all the luns have been probed.
>> +	 * See pm_runtime_mark_last_busy in ufshcd_setup_links.
>> +	 */
>> +	pm_runtime_get_noresume(&hba->sdev_ufs_device->sdev_gendev);
>>  	scsi_device_put(hba->sdev_ufs_device);
>>
>>  	hba->sdev_rpmb = __scsi_add_device(hba->host, 0, 0,
>> @@ -7417,6 +7464,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);
>> @@ -7747,6 +7797,54 @@ static int ufshcd_device_params_init(struct ufs_hba *hba)
>>  	return ret;
>>  }
>>
>> +static int ufshcd_wl_probe(struct device *dev)
>> +{
>> +	struct scsi_device *sdev = to_scsi_device(dev);
>> +
>> +	/*
>> +	 * Use this driver for both device wlun and rpmb lun
>> +	 * It's used to send uac to rpmb wlun and ssu to device wlun
>> +	 */
>> +	if (sdev && (is_rpmb_wlun(sdev) || is_device_wlun(sdev)))
>> +		return 0;
>> +	return -ENODEV;
>> +}
>> +
>> +static int ufshcd_wl_remove(struct device *dev)
>> +{
>> +	struct scsi_device *sdev = to_scsi_device(dev);
>> +
>> +	scsi_remove_device(sdev);
>
>The 'probe' did not do the __scsi_add_device(), so does the 'remove' need to
>do scsi_remove_device()?
>
>> +	return 0;
>> +}
>> +
>> +static const struct dev_pm_ops ufshcd_wl_pm_ops = {
>> +	SET_RUNTIME_PM_OPS(ufshcd_wl_runtime_suspend, ufshcd_wl_runtime_resume, NULL)
>> +	SET_SYSTEM_SLEEP_PM_OPS(ufshcd_wl_suspend, ufshcd_wl_resume)
>
>.poweroff needs to set the power mode to PowerDown irrespective of spm_lvl,
>so not exactly the same as ufshcd_wl_suspend.
>
>Also, ufshcd_pci_poweroff() in ufshcd-pci.c should be removed since it won't
>work anymore.
>
>> +};
>> +
>> +/**
>> + * ufs_wlun - describes 2 wluns:
>> + * ufs-device wlun - used to send pm commands
>> + * and rpmb wlun - for rpmb commands
>> + * All luns including rpmb 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_wlun = {
>> +	.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,
>> +	},
>> +};
>
>WRT to RPMB, aren't all the callbacks optional, so that would leave only
>something like below.  Would that work?
>
>#define wlun_dev_to_hba(dv) shost_priv(to_scsi_device(dv)->host)
>
>static int ufs_rpmb_probe(struct device *dev)
>{
>    return is_rpmb_wlun(to_scsi_device(dev)) ? 0 : -ENODEV;
>}
>
>static int ufs_rpmb_runtime_resume(struct device *dev)
>{
>    struct ufs_hba *hba = wlun_dev_to_hba(dev);
>
>    if (hba->sdev_rpmb)
>        return ufshcd_clear_ua_wlun(hba, UFS_UPIU_RPMB_WLUN);
>    return 0;
>}
>
>static int ufs_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_ua_wlun(hba, UFS_UPIU_RPMB_WLUN);
>    return 0;
>}
>
>static const struct dev_pm_ops ufs_rpmb_pm_ops = {
>    SET_RUNTIME_PM_OPS(NULL, ufs_rpmb_runtime_resume, NULL)
>    SET_SYSTEM_SLEEP_PM_OPS(NULL, ufs_rpmb_resume)
>};
>
>static struct scsi_driver ufs_rpmb_template = {
>    .gendrv = {
>        .name = "ufs_rpmb_wlun",
>        .owner = THIS_MODULE,
>        .probe = ufs_rpmb_probe,
>        .pm = &ufs_rpmb_pm_ops,
>    },
>};
>
>
>> +
>>  /**
>>   * ufshcd_add_lus - probe and add UFS logical units
>>   * @hba: per-adapter instance
>> @@ -7755,6 +7853,12 @@ static int ufshcd_add_lus(struct ufs_hba *hba)
>>  {
>>  	int ret;
>>
>> +	ret = scsi_register_driver(&ufs_wlun.gendrv);
>
>That will attempt to register the driver every time a UFS host controller is
>added (e.g. what happens if you unbind and re-bind the host controller).
>Probably needs to be in ufshcd_core_init() with corresponding unregister in
>ufshcd_core_exit()
>
>> +	if (ret) {
>> +		dev_err(hba->dev, "Register device wlun driver failed: %d\n",
>> +			ret);
>> +		return ret;
>> +	}
>>  	/* Add required well known logical units to scsi mid layer */
>>  	ret = ufshcd_scsi_add_wlus(hba);
>>  	if (ret)
>> @@ -7964,6 +8068,7 @@ static void ufshcd_async_scan(void *data, async_cookie_t cookie)
>>  		pm_runtime_put_sync(hba->dev);
>>  		ufshcd_hba_exit(hba);
>>  	}
>> +	hba->init_done = true;
>>  }
>>
>>  static const struct attribute_group *ufshcd_driver_groups[] = {
>> @@ -8475,7 +8580,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;
>>
>> @@ -8650,31 +8756,19 @@ 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;
>>  	enum ufs_pm_level pm_lvl;
>>  	enum ufs_dev_pwr_mode req_dev_pwr_mode;
>>  	enum uic_link_state req_link_state;
>>
>> -	hba->pm_op_in_progress = 1;
>> +	/*
>> +	 * Is invoked when the device wlun is added to sysfs
>> +	 * But by then hba->sdev_ufs_device may not be initialized.
>> +	 */
>> +	if (!hba->init_done)
>> +		return 0;
>>  	if (!ufshcd_is_shutdown_pm(pm_op)) {
>>  		pm_lvl = ufshcd_is_runtime_pm(pm_op) ?
>>  			 hba->rpm_lvl : hba->spm_lvl;
>> @@ -8690,24 +8784,23 @@ static int ufshcd_suspend(struct ufs_hba *hba, enum ufs_pm_op pm_op)
>>  	 * just gate the clocks.
>>  	 */
>>  	ufshcd_hold(hba, false);
>> -	hba->clk_gating.is_suspended = true;
>>
>>  	if (ufshcd_is_clkscaling_supported(hba))
>>  		ufshcd_clk_scaling_suspend(hba, true);
>>
>>  	if (req_dev_pwr_mode == UFS_ACTIVE_PWR_MODE &&
>>  			req_link_state == UIC_LINK_ACTIVE_STATE) {
>> -		goto disable_clks;
>> +		goto enable_scaling;
>>  	}
>>
>>  	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)) {
>> @@ -8719,7 +8812,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);
>> @@ -8746,10 +8839,224 @@ 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;
>> +			if (!ret)
>> +				goto out;
>>  		}
>>  	}
>> +enable_scaling:
>> +	if (ufshcd_is_clkscaling_supported(hba))
>> +		ufshcd_clk_scaling_suspend(hba, false);
>> +
>> +	hba->dev_info.b_rpm_dev_flush_capable = false;
>> +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));
>> +	}
>> +	if (ret)
>> +		ufshcd_update_evt_hist(hba, UFS_EVT_WL_SUSP_ERR, (u32)ret);
>> +	ufshcd_release(hba);
>> +	return ret;
>> +}
>> +
>> +static int __ufshcd_wl_resume(struct ufs_hba *hba, enum ufs_pm_op pm_op)
>> +{
>> +	int ret = 0;
>> +
>> +	if (!hba->init_done)
>> +		return 0;
>> +
>> +	ufshcd_hold(hba, false);
>> +	if (ufshcd_is_ufs_dev_deepsleep(hba)) {
>> +		/* The device is in DeepSleep but hba was not suspended */
>> +		ret = ufshcd_reset_and_restore(hba);
>> +		if (ret)
>> +			goto out;
>> +	}
>> +
>> +	if (!ufshcd_is_ufs_dev_active(hba)) {
>> +		ret = ufshcd_set_dev_pwr_mode(hba, UFS_ACTIVE_PWR_MODE);
>> +		if (ret)
>> +			goto out;
>> +	}
>> +
>> +	if (ufshcd_keep_autobkops_enabled_except_suspend(hba))
>> +		ufshcd_enable_auto_bkops(hba);
>> +	else
>> +		/*
>> +		 * If BKOPs operations are urgently needed at this moment then
>> +		 * keep auto-bkops enabled or else disable it.
>> +		 */
>> +		ufshcd_urgent_bkops(hba);
>> +
>> +	if (hba->clk_scaling.is_allowed)
>> +		ufshcd_resume_clkscaling(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);
>> +	}
>> +
>> +out:
>> +	if (ret)
>> +		ufshcd_update_evt_hist(hba, UFS_EVT_WL_RES_ERR, (u32)ret);
>> +	ufshcd_release(hba);
>> +	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);
>> +
>> +	if (is_rpmb_wlun(sdev))
>> +		return 0;
>> +	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;
>> +	ktime_t start = ktime_get();
>> +
>> +	hba = shost_priv(sdev->host);
>> +
>> +	if (is_rpmb_wlun(sdev)) {
>> +		if (hba->sdev_rpmb)
>> +			return ufshcd_clear_ua_wlun(hba, UFS_UPIU_RPMB_WLUN);
>> +		return 0;
>> +	}
>> +
>> +	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;
>> +}
>> +
>> +static int ufshcd_wl_suspend(struct device *dev)
>> +{
>> +	struct scsi_device *sdev = to_scsi_device(dev);
>> +	struct ufs_hba *hba;
>> +	int ret;
>> +	ktime_t start = ktime_get();
>> +
>> +	if (is_rpmb_wlun(sdev))
>> +		return 0;
>> +	hba = shost_priv(sdev->host);
>> +	ret = __ufshcd_wl_suspend(hba, UFS_SYSTEM_PM);
>> +	if (ret)
>> +		dev_err(&sdev->sdev_gendev, "%s failed: %d\n", __func__,  ret);
>> +
>> +	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;
>> +	ktime_t start = ktime_get();
>> +
>> +	if (pm_runtime_suspended(dev))
>> +		return 0;
>> +	hba = shost_priv(sdev->host);
>> +
>> +	if (is_rpmb_wlun(sdev)) {
>> +		if (hba->sdev_rpmb)
>> +			return ufshcd_clear_ua_wlun(hba, UFS_UPIU_RPMB_WLUN);
>> +		return 0;
>> +	}
>> +
>> +	ret = __ufshcd_wl_resume(hba, UFS_SYSTEM_PM);
>> +	if (ret)
>> +		dev_err(&sdev->sdev_gendev, "%s failed: %d\n", __func__, ret);
>> +
>> +	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);
>> +
>> +	return ret;
>> +}
>> +
>> +static void ufshcd_wl_shutdown(struct device *dev)
>> +{
>> +	struct scsi_device *sdev = to_scsi_device(dev);
>> +	struct ufs_hba *hba;
>> +
>> +	if (is_rpmb_wlun(sdev))
>> +		return;
>> +	hba = shost_priv(sdev->host);
>> +	scsi_device_quiesce(sdev);
>> +	shost_for_each_device(sdev, hba->host) {
>> +		if (sdev == hba->sdev_ufs_device)
>> +			continue;
>> +		scsi_remove_device(sdev);
>
>Shutdown needs to quiesce devices.  Removing devices is not advisable
>because the upper layers cannot respond at this point, causing deadlocks.
>Is scsi_remove_device() really needed here?
>
>> +	}
>> +	__ufshcd_wl_suspend(hba, UFS_SHUTDOWN_PM);
>> +}
>> +
>> +/**
>> + * 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 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
>> + * link is powered off.
>> + *
>> + * Returns 0 for success and non-zero for failure
>> + */
>> +static int ufshcd_suspend(struct ufs_hba *hba, enum ufs_pm_op pm_op)
>> +{
>> +	int ret = 0;
>> +	int check_for_bkops;
>> +	enum ufs_pm_level pm_lvl;
>> +	enum ufs_dev_pwr_mode req_dev_pwr_mode;
>> +	enum uic_link_state req_link_state;
>> +
>> +	hba->pm_op_in_progress = 1;
>> +	if (!ufshcd_is_shutdown_pm(pm_op)) {
>> +		pm_lvl = ufshcd_is_runtime_pm(pm_op) ?
>> +			 hba->rpm_lvl : hba->spm_lvl;
>> +		req_dev_pwr_mode = ufs_get_pm_lvl_to_dev_pwr_mode(pm_lvl);
>> +		req_link_state = ufs_get_pm_lvl_to_link_pwr_state(pm_lvl);
>> +	} else {
>> +		req_dev_pwr_mode = UFS_POWERDOWN_PWR_MODE;
>> +		req_link_state = UIC_LINK_OFF_STATE;
>> +	}
>> +
>> +	/*
>> +	 * If we can't transition into any of the low power modes
>> +	 * just gate the clocks.
>> +	 */
>> +	ufshcd_hold(hba, false);
>> +	hba->clk_gating.is_suspended = true;
>>
>>  	/*
>>  	 * In the case of DeepSleep, the device is expected to remain powered
>> @@ -8758,9 +9065,8 @@ static int ufshcd_suspend(struct ufs_hba *hba, enum ufs_pm_op pm_op)
>>  	check_for_bkops = !ufshcd_is_ufs_dev_deepsleep(hba);
>>  	ret = ufshcd_link_state_transition(hba, req_link_state, check_for_bkops);
>>  	if (ret)
>> -		goto set_dev_active;
>> +		goto enable_gating;
>>
>> -disable_clks:
>>  	/*
>>  	 * Call vendor specific suspend callback. As these callbacks may access
>>  	 * vendor specific host controller register space call them before the
>> @@ -8790,7 +9096,6 @@ static int ufshcd_suspend(struct ufs_hba *hba, enum ufs_pm_op pm_op)
>>  	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
>> @@ -8804,28 +9109,15 @@ static int ufshcd_suspend(struct ufs_hba *hba, enum ufs_pm_op pm_op)
>>  		ufshcd_set_link_active(hba);
>>  	else if (ufshcd_is_link_off(hba))
>>  		ufshcd_host_reset_and_restore(hba);
>> -set_dev_active:
>>  	/* Can also get here needing to exit DeepSleep */
>>  	if (ufshcd_is_ufs_dev_deepsleep(hba)) {
>>  		ufshcd_device_reset(hba);
>>  		ufshcd_host_reset_and_restore(hba);
>>  	}
>> -	if (!ufshcd_set_dev_pwr_mode(hba, UFS_ACTIVE_PWR_MODE))
>> -		ufshcd_disable_auto_bkops(hba);
>>  enable_gating:
>> -	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)
>> @@ -8846,10 +9138,8 @@ static int ufshcd_suspend(struct ufs_hba *hba, enum ufs_pm_op pm_op)
>>  static int ufshcd_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);
>> @@ -8901,43 +9191,14 @@ static int ufshcd_resume(struct ufs_hba *hba, enum ufs_pm_op pm_op)
>>  			goto vendor_suspend;
>>  	}
>>
>> -	if (!ufshcd_is_ufs_dev_active(hba)) {
>> -		ret = ufshcd_set_dev_pwr_mode(hba, UFS_ACTIVE_PWR_MODE);
>> -		if (ret)
>> -			goto set_old_link_state;
>> -	}
>> -
>> -	if (ufshcd_keep_autobkops_enabled_except_suspend(hba))
>> -		ufshcd_enable_auto_bkops(hba);
>> -	else
>> -		/*
>> -		 * If BKOPs operations are urgently needed at this moment then
>> -		 * keep auto-bkops enabled or else disable it.
>> -		 */
>> -		ufshcd_urgent_bkops(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);
>>
>>  	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:
>> diff --git a/drivers/scsi/ufs/ufshcd.h b/drivers/scsi/ufs/ufshcd.h
>> index ee61f82..7662387 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,
>> @@ -841,6 +843,8 @@ struct ufs_hba {
>>  #ifdef CONFIG_DEBUG_FS
>>  	struct dentry *debugfs_root;
>>  #endif
>> +	bool init_done;
>> +	u32 luns_avail;
>>  };
>>
>>  /* Returns true if clocks can be gated. Otherwise false */
>> diff --git a/include/trace/events/ufs.h b/include/trace/events/ufs.h
>> index e151477..d9d233b 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,
>>
>

Re: [PATCH v4 1/2] scsi: ufs: Enable power management for wlun

[Asutosh Das]

On Tue, Feb 23 2021 at 12:23 -0800, Adrian Hunter wrote:
>On 23/02/21 1:04 am, 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 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.
>
>Hi
>
>I have a few more questions and comments.
>
>At a glance it looked to me like the SCSI bus will leave a SCSI device
>runtime suspended at system suspend, but UFS device spm_lvl and rpm_lvl need
>not be the same.  Do you know how SCSI handles that case?
>
>>
>> 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/ufshcd.c  | 455 +++++++++++++++++++++++++++++++++++----------
>>  drivers/scsi/ufs/ufshcd.h  |   4 +
>>  include/trace/events/ufs.h |  20 ++
>>  3 files changed, 382 insertions(+), 97 deletions(-)
>>
>> diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
>> index 45624c7..1d8044a 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"
>> @@ -251,6 +252,11 @@ static int ufshcd_wb_toggle_flush_during_h8(struct ufs_hba *hba, bool set);
>>  static inline int ufshcd_wb_toggle_flush(struct ufs_hba *hba, bool enable);
>>  static void ufshcd_hba_vreg_set_lpm(struct ufs_hba *hba);
>>  static void ufshcd_hba_vreg_set_hpm(struct ufs_hba *hba);
>> +static int ufshcd_wl_runtime_suspend(struct device *dev);
>> +static int ufshcd_wl_runtime_resume(struct device *dev);
>> +static int ufshcd_wl_suspend(struct device *dev);
>> +static int ufshcd_wl_resume(struct device *dev);
>> +static void ufshcd_wl_shutdown(struct device *dev);
>>
>>  static inline bool ufshcd_valid_tag(struct ufs_hba *hba, int tag)
>>  {
>> @@ -1556,7 +1562,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);
>> +	scsi_autopm_get_device(hba->sdev_ufs_device);
>>  	ufshcd_hold(hba, false);
>>
>>  	hba->clk_scaling.is_enabled = value;
>> @@ -1572,7 +1578,7 @@ static ssize_t ufshcd_clkscale_enable_store(struct device *dev,
>>  	}
>>
>>  	ufshcd_release(hba);
>> -	pm_runtime_put_sync(hba->dev);
>> +	scsi_autopm_put_device(hba->sdev_ufs_device);
>>  out:
>>  	up(&hba->host_sem);
>>  	return err ? err : count;
>> @@ -2572,6 +2578,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;
>> @@ -4808,6 +4825,41 @@ 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);
>> +		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)
>> +			return;
>> +
>> +		pm_runtime_put_noidle(&hba->sdev_ufs_device->sdev_gendev);
>> +		pm_runtime_mark_last_busy(&hba->sdev_ufs_device->sdev_gendev);
>> +	} else {
>> +		/* device wlun is probed */
>> +		hba->luns_avail--;
>> +	}
>> +}
>> +
>> +/**
>>   * ufshcd_slave_alloc - handle initial SCSI device configurations
>>   * @sdev: pointer to SCSI device
>>   *
>> @@ -4838,6 +4890,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;
>>  }
>>
>> @@ -4984,16 +5038,9 @@ ufshcd_transfer_rsp_status(struct ufs_hba *hba, struct ufshcd_lrb *lrbp)
>>  			 * solution could be to abort the system suspend if
>>  			 * UFS device needs urgent BKOPs.
>>  			 */
>> -			if (!hba->pm_op_in_progress &&
>> -			    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);
>> -			}
>> +			if (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 */
>> @@ -5589,8 +5636,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);
>> +	scsi_autopm_get_device(hba->sdev_ufs_device);
>> +	scsi_autopm_put_device(hba->sdev_ufs_device);
>>  }
>>
>>  /**
>> @@ -5607,7 +5654,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) {
>> @@ -5623,14 +5669,6 @@ static void ufshcd_exception_event_handler(struct work_struct *work)
>>
>>  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;
>>  }
>>
>> @@ -7254,6 +7292,15 @@ static int ufshcd_scsi_add_wlus(struct ufs_hba *hba)
>>  		goto out;
>>  	}
>>  	ufshcd_blk_pm_runtime_init(hba->sdev_ufs_device);
>> +	/*
>> +	 * A pm_runtime_put_sync is invoked when this device enables blk_pm_runtime
>> +	 * & would suspend the device-wlun upon timer expiry.
>> +	 * But suspending device wlun _may_ put the ufs device in the pre-defined
>> +	 * low power mode (SSU <rpm_lvl>). Probing of other luns may fail then.
>> +	 * Don't allow this suspend until all the luns have been probed.
>> +	 * See pm_runtime_mark_last_busy in ufshcd_setup_links.
>> +	 */
>> +	pm_runtime_get_noresume(&hba->sdev_ufs_device->sdev_gendev);
>>  	scsi_device_put(hba->sdev_ufs_device);
>>
>>  	hba->sdev_rpmb = __scsi_add_device(hba->host, 0, 0,
>> @@ -7417,6 +7464,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);
>> @@ -7747,6 +7797,54 @@ static int ufshcd_device_params_init(struct ufs_hba *hba)
>>  	return ret;
>>  }
>>
>> +static int ufshcd_wl_probe(struct device *dev)
>> +{
>> +	struct scsi_device *sdev = to_scsi_device(dev);
>> +
>> +	/*
>> +	 * Use this driver for both device wlun and rpmb lun
>> +	 * It's used to send uac to rpmb wlun and ssu to device wlun
>> +	 */
>> +	if (sdev && (is_rpmb_wlun(sdev) || is_device_wlun(sdev)))
>> +		return 0;
>> +	return -ENODEV;
>> +}
>> +
>> +static int ufshcd_wl_remove(struct device *dev)
>> +{
>> +	struct scsi_device *sdev = to_scsi_device(dev);
>> +
>> +	scsi_remove_device(sdev);
>
>The 'probe' did not do the __scsi_add_device(), so does the 'remove' need to
>do scsi_remove_device()?
>

No, I will remove 'scsi_remove_device()'.

>> +	return 0;
>> +}
>> +
>> +static const struct dev_pm_ops ufshcd_wl_pm_ops = {
>> +	SET_RUNTIME_PM_OPS(ufshcd_wl_runtime_suspend, ufshcd_wl_runtime_resume, NULL)
>> +	SET_SYSTEM_SLEEP_PM_OPS(ufshcd_wl_suspend, ufshcd_wl_resume)
>
>.poweroff needs to set the power mode to PowerDown irrespective of spm_lvl,
>so not exactly the same as ufshcd_wl_suspend.
>
Not exactly same - yes. But it passes UFS_SHUTDOWN_PM as argument which would
put the ufs device in power-off mode.

>Also, ufshcd_pci_poweroff() in ufshcd-pci.c should be removed since it won't
>work anymore.
>
I see that this function doesn't use UFS_SHUTDOWN_PM.
I'll remove this function.

>> +};
>> +
>> +/**
>> + * ufs_wlun - describes 2 wluns:
>> + * ufs-device wlun - used to send pm commands
>> + * and rpmb wlun - for rpmb commands
>> + * All luns including rpmb 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_wlun = {
>> +	.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,
>> +	},
>> +};
>
>WRT to RPMB, aren't all the callbacks optional, so that would leave only
>something like below.  Would that work?
>
>#define wlun_dev_to_hba(dv) shost_priv(to_scsi_device(dv)->host)
>
>static int ufs_rpmb_probe(struct device *dev)
>{
>    return is_rpmb_wlun(to_scsi_device(dev)) ? 0 : -ENODEV;
>}
>
>static int ufs_rpmb_runtime_resume(struct device *dev)
>{
>    struct ufs_hba *hba = wlun_dev_to_hba(dev);
>
>    if (hba->sdev_rpmb)
>        return ufshcd_clear_ua_wlun(hba, UFS_UPIU_RPMB_WLUN);
>    return 0;
>}
>
>static int ufs_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_ua_wlun(hba, UFS_UPIU_RPMB_WLUN);
>    return 0;
>}
>
>static const struct dev_pm_ops ufs_rpmb_pm_ops = {
>    SET_RUNTIME_PM_OPS(NULL, ufs_rpmb_runtime_resume, NULL)
>    SET_SYSTEM_SLEEP_PM_OPS(NULL, ufs_rpmb_resume)
>};
>
>static struct scsi_driver ufs_rpmb_template = {
>    .gendrv = {
>        .name = "ufs_rpmb_wlun",
>        .owner = THIS_MODULE,
>        .probe = ufs_rpmb_probe,
>        .pm = &ufs_rpmb_pm_ops,
>    },
>};
>
>
Thanks. I'll shamelessly copy this code :)

>> +
>>  /**
>>   * ufshcd_add_lus - probe and add UFS logical units
>>   * @hba: per-adapter instance
>> @@ -7755,6 +7853,12 @@ static int ufshcd_add_lus(struct ufs_hba *hba)
>>  {
>>  	int ret;
>>
>> +	ret = scsi_register_driver(&ufs_wlun.gendrv);
>
>That will attempt to register the driver every time a UFS host controller is
>added (e.g. what happens if you unbind and re-bind the host controller).
>Probably needs to be in ufshcd_core_init() with corresponding unregister in
>ufshcd_core_exit()

I see that when I bind/unbind the driver, it invokes probe and all of the init
again. I'll add scsi_unregister_driver() in ufshcd_remove(). 

>
>> +	if (ret) {
>> +		dev_err(hba->dev, "Register device wlun driver failed: %d\n",
>> +			ret);
>> +		return ret;
>> +	}
>>  	/* Add required well known logical units to scsi mid layer */
>>  	ret = ufshcd_scsi_add_wlus(hba);
>>  	if (ret)
>> @@ -7964,6 +8068,7 @@ static void ufshcd_async_scan(void *data, async_cookie_t cookie)
>>  		pm_runtime_put_sync(hba->dev);
>>  		ufshcd_hba_exit(hba);
>>  	}
>> +	hba->init_done = true;
>>  }
>>
>>  static const struct attribute_group *ufshcd_driver_groups[] = {
>> @@ -8475,7 +8580,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;
>>
>> @@ -8650,31 +8756,19 @@ 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;
>>  	enum ufs_pm_level pm_lvl;
>>  	enum ufs_dev_pwr_mode req_dev_pwr_mode;
>>  	enum uic_link_state req_link_state;
>>
>> -	hba->pm_op_in_progress = 1;
>> +	/*
>> +	 * Is invoked when the device wlun is added to sysfs
>> +	 * But by then hba->sdev_ufs_device may not be initialized.
>> +	 */
>> +	if (!hba->init_done)
>> +		return 0;
>>  	if (!ufshcd_is_shutdown_pm(pm_op)) {
>>  		pm_lvl = ufshcd_is_runtime_pm(pm_op) ?
>>  			 hba->rpm_lvl : hba->spm_lvl;
>> @@ -8690,24 +8784,23 @@ static int ufshcd_suspend(struct ufs_hba *hba, enum ufs_pm_op pm_op)
>>  	 * just gate the clocks.
>>  	 */
>>  	ufshcd_hold(hba, false);
>> -	hba->clk_gating.is_suspended = true;
>>
>>  	if (ufshcd_is_clkscaling_supported(hba))
>>  		ufshcd_clk_scaling_suspend(hba, true);
>>
>>  	if (req_dev_pwr_mode == UFS_ACTIVE_PWR_MODE &&
>>  			req_link_state == UIC_LINK_ACTIVE_STATE) {
>> -		goto disable_clks;
>> +		goto enable_scaling;
>>  	}
>>
>>  	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)) {
>> @@ -8719,7 +8812,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);
>> @@ -8746,10 +8839,224 @@ 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;
>> +			if (!ret)
>> +				goto out;
>>  		}
>>  	}
>> +enable_scaling:
>> +	if (ufshcd_is_clkscaling_supported(hba))
>> +		ufshcd_clk_scaling_suspend(hba, false);
>> +
>> +	hba->dev_info.b_rpm_dev_flush_capable = false;
>> +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));
>> +	}
>> +	if (ret)
>> +		ufshcd_update_evt_hist(hba, UFS_EVT_WL_SUSP_ERR, (u32)ret);
>> +	ufshcd_release(hba);
>> +	return ret;
>> +}
>> +
>> +static int __ufshcd_wl_resume(struct ufs_hba *hba, enum ufs_pm_op pm_op)
>> +{
>> +	int ret = 0;
>> +
>> +	if (!hba->init_done)
>> +		return 0;
>> +
>> +	ufshcd_hold(hba, false);
>> +	if (ufshcd_is_ufs_dev_deepsleep(hba)) {
>> +		/* The device is in DeepSleep but hba was not suspended */
>> +		ret = ufshcd_reset_and_restore(hba);
>> +		if (ret)
>> +			goto out;
>> +	}
>> +
>> +	if (!ufshcd_is_ufs_dev_active(hba)) {
>> +		ret = ufshcd_set_dev_pwr_mode(hba, UFS_ACTIVE_PWR_MODE);
>> +		if (ret)
>> +			goto out;
>> +	}
>> +
>> +	if (ufshcd_keep_autobkops_enabled_except_suspend(hba))
>> +		ufshcd_enable_auto_bkops(hba);
>> +	else
>> +		/*
>> +		 * If BKOPs operations are urgently needed at this moment then
>> +		 * keep auto-bkops enabled or else disable it.
>> +		 */
>> +		ufshcd_urgent_bkops(hba);
>> +
>> +	if (hba->clk_scaling.is_allowed)
>> +		ufshcd_resume_clkscaling(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);
>> +	}
>> +
>> +out:
>> +	if (ret)
>> +		ufshcd_update_evt_hist(hba, UFS_EVT_WL_RES_ERR, (u32)ret);
>> +	ufshcd_release(hba);
>> +	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);
>> +
>> +	if (is_rpmb_wlun(sdev))
>> +		return 0;
>> +	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;
>> +	ktime_t start = ktime_get();
>> +
>> +	hba = shost_priv(sdev->host);
>> +
>> +	if (is_rpmb_wlun(sdev)) {
>> +		if (hba->sdev_rpmb)
>> +			return ufshcd_clear_ua_wlun(hba, UFS_UPIU_RPMB_WLUN);
>> +		return 0;
>> +	}
>> +
>> +	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;
>> +}
>> +
>> +static int ufshcd_wl_suspend(struct device *dev)
>> +{
>> +	struct scsi_device *sdev = to_scsi_device(dev);
>> +	struct ufs_hba *hba;
>> +	int ret;
>> +	ktime_t start = ktime_get();
>> +
>> +	if (is_rpmb_wlun(sdev))
>> +		return 0;
>> +	hba = shost_priv(sdev->host);
>> +	ret = __ufshcd_wl_suspend(hba, UFS_SYSTEM_PM);
>> +	if (ret)
>> +		dev_err(&sdev->sdev_gendev, "%s failed: %d\n", __func__,  ret);
>> +
>> +	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;
>> +	ktime_t start = ktime_get();
>> +
>> +	if (pm_runtime_suspended(dev))
>> +		return 0;
>> +	hba = shost_priv(sdev->host);
>> +
>> +	if (is_rpmb_wlun(sdev)) {
>> +		if (hba->sdev_rpmb)
>> +			return ufshcd_clear_ua_wlun(hba, UFS_UPIU_RPMB_WLUN);
>> +		return 0;
>> +	}
>> +
>> +	ret = __ufshcd_wl_resume(hba, UFS_SYSTEM_PM);
>> +	if (ret)
>> +		dev_err(&sdev->sdev_gendev, "%s failed: %d\n", __func__, ret);
>> +
>> +	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);
>> +
>> +	return ret;
>> +}
>> +
>> +static void ufshcd_wl_shutdown(struct device *dev)
>> +{
>> +	struct scsi_device *sdev = to_scsi_device(dev);
>> +	struct ufs_hba *hba;
>> +
>> +	if (is_rpmb_wlun(sdev))
>> +		return;
>> +	hba = shost_priv(sdev->host);
>> +	scsi_device_quiesce(sdev);
>> +	shost_for_each_device(sdev, hba->host) {
>> +		if (sdev == hba->sdev_ufs_device)
>> +			continue;
>> +		scsi_remove_device(sdev);
>
>Shutdown needs to quiesce devices.  Removing devices is not advisable
>because the upper layers cannot respond at this point, causing deadlocks.
>Is scsi_remove_device() really needed here?
>
Thanks. I will change this.

>
>> +	}
>> +	__ufshcd_wl_suspend(hba, UFS_SHUTDOWN_PM);
>> +}
>> +
>> +/**
>> + * 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 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
>> + * link is powered off.
>> + *
>> + * Returns 0 for success and non-zero for failure
>> + */
>> +static int ufshcd_suspend(struct ufs_hba *hba, enum ufs_pm_op pm_op)
>> +{
>> +	int ret = 0;
>> +	int check_for_bkops;
>> +	enum ufs_pm_level pm_lvl;
>> +	enum ufs_dev_pwr_mode req_dev_pwr_mode;
>> +	enum uic_link_state req_link_state;
>> +
>> +	hba->pm_op_in_progress = 1;
>> +	if (!ufshcd_is_shutdown_pm(pm_op)) {
>> +		pm_lvl = ufshcd_is_runtime_pm(pm_op) ?
>> +			 hba->rpm_lvl : hba->spm_lvl;
>> +		req_dev_pwr_mode = ufs_get_pm_lvl_to_dev_pwr_mode(pm_lvl);
>> +		req_link_state = ufs_get_pm_lvl_to_link_pwr_state(pm_lvl);
>> +	} else {
>> +		req_dev_pwr_mode = UFS_POWERDOWN_PWR_MODE;
>> +		req_link_state = UIC_LINK_OFF_STATE;
>> +	}
>> +
>> +	/*
>> +	 * If we can't transition into any of the low power modes
>> +	 * just gate the clocks.
>> +	 */
>> +	ufshcd_hold(hba, false);
>> +	hba->clk_gating.is_suspended = true;
>>
>>  	/*
>>  	 * In the case of DeepSleep, the device is expected to remain powered
>> @@ -8758,9 +9065,8 @@ static int ufshcd_suspend(struct ufs_hba *hba, enum ufs_pm_op pm_op)
>>  	check_for_bkops = !ufshcd_is_ufs_dev_deepsleep(hba);
>>  	ret = ufshcd_link_state_transition(hba, req_link_state, check_for_bkops);
>>  	if (ret)
>> -		goto set_dev_active;
>> +		goto enable_gating;
>>
>> -disable_clks:
>>  	/*
>>  	 * Call vendor specific suspend callback. As these callbacks may access
>>  	 * vendor specific host controller register space call them before the
>> @@ -8790,7 +9096,6 @@ static int ufshcd_suspend(struct ufs_hba *hba, enum ufs_pm_op pm_op)
>>  	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
>> @@ -8804,28 +9109,15 @@ static int ufshcd_suspend(struct ufs_hba *hba, enum ufs_pm_op pm_op)
>>  		ufshcd_set_link_active(hba);
>>  	else if (ufshcd_is_link_off(hba))
>>  		ufshcd_host_reset_and_restore(hba);
>> -set_dev_active:
>>  	/* Can also get here needing to exit DeepSleep */
>>  	if (ufshcd_is_ufs_dev_deepsleep(hba)) {
>>  		ufshcd_device_reset(hba);
>>  		ufshcd_host_reset_and_restore(hba);
>>  	}
>> -	if (!ufshcd_set_dev_pwr_mode(hba, UFS_ACTIVE_PWR_MODE))
>> -		ufshcd_disable_auto_bkops(hba);
>>  enable_gating:
>> -	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)
>> @@ -8846,10 +9138,8 @@ static int ufshcd_suspend(struct ufs_hba *hba, enum ufs_pm_op pm_op)
>>  static int ufshcd_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);
>> @@ -8901,43 +9191,14 @@ static int ufshcd_resume(struct ufs_hba *hba, enum ufs_pm_op pm_op)
>>  			goto vendor_suspend;
>>  	}
>>
>> -	if (!ufshcd_is_ufs_dev_active(hba)) {
>> -		ret = ufshcd_set_dev_pwr_mode(hba, UFS_ACTIVE_PWR_MODE);
>> -		if (ret)
>> -			goto set_old_link_state;
>> -	}
>> -
>> -	if (ufshcd_keep_autobkops_enabled_except_suspend(hba))
>> -		ufshcd_enable_auto_bkops(hba);
>> -	else
>> -		/*
>> -		 * If BKOPs operations are urgently needed at this moment then
>> -		 * keep auto-bkops enabled or else disable it.
>> -		 */
>> -		ufshcd_urgent_bkops(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);
>>
>>  	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:
>> diff --git a/drivers/scsi/ufs/ufshcd.h b/drivers/scsi/ufs/ufshcd.h
>> index ee61f82..7662387 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,
>> @@ -841,6 +843,8 @@ struct ufs_hba {
>>  #ifdef CONFIG_DEBUG_FS
>>  	struct dentry *debugfs_root;
>>  #endif
>> +	bool init_done;
>> +	u32 luns_avail;
>>  };
>>
>>  /* Returns true if clocks can be gated. Otherwise false */
>> diff --git a/include/trace/events/ufs.h b/include/trace/events/ufs.h
>> index e151477..d9d233b 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,
>>
>

[PATCH v4 2/2] ufs: sysfs: Resume the proper scsi device

[Asutosh Das]

Resumes the actual scsi device the unit descriptor of which
is being accessed instead of the hba alone.

Signed-off-by: Asutosh Das <asutoshd@codeaurora.org>
---
 drivers/scsi/ufs/ufs-sysfs.c | 26 +++++++++++++++-----------
 1 file changed, 15 insertions(+), 11 deletions(-)

diff --git a/drivers/scsi/ufs/ufs-sysfs.c b/drivers/scsi/ufs/ufs-sysfs.c
index acc54f5..34481e3 100644
--- a/drivers/scsi/ufs/ufs-sysfs.c
+++ b/drivers/scsi/ufs/ufs-sysfs.c
@@ -297,10 +297,10 @@ static ssize_t ufs_sysfs_read_desc_param(struct ufs_hba *hba,
 		goto out;
 	}
 
-	pm_runtime_get_sync(hba->dev);
+	scsi_autopm_get_device(hba->sdev_ufs_device);
 	ret = ufshcd_read_desc_param(hba, desc_id, desc_index,
 				param_offset, desc_buf, param_size);
-	pm_runtime_put_sync(hba->dev);
+	scsi_autopm_put_device(hba->sdev_ufs_device);
 	if (ret) {
 		ret = -EINVAL;
 		goto out;
@@ -678,7 +678,7 @@ static ssize_t _name##_show(struct device *dev,				\
 		up(&hba->host_sem);					\
 		return -ENOMEM;						\
 	}								\
-	pm_runtime_get_sync(hba->dev);					\
+	scsi_autopm_get_device(hba->sdev_ufs_device);			\
 	ret = ufshcd_query_descriptor_retry(hba,			\
 		UPIU_QUERY_OPCODE_READ_DESC, QUERY_DESC_IDN_DEVICE,	\
 		0, 0, desc_buf, &desc_len);				\
@@ -695,7 +695,7 @@ static ssize_t _name##_show(struct device *dev,				\
 		goto out;						\
 	ret = sysfs_emit(buf, "%s\n", desc_buf);			\
 out:									\
-	pm_runtime_put_sync(hba->dev);					\
+	scsi_autopm_put_device(hba->sdev_ufs_device);			\
 	kfree(desc_buf);						\
 	up(&hba->host_sem);						\
 	return ret;							\
@@ -744,10 +744,10 @@ static ssize_t _name##_show(struct device *dev,				\
 	}								\
 	if (ufshcd_is_wb_flags(QUERY_FLAG_IDN##_uname))			\
 		index = ufshcd_wb_get_query_index(hba);			\
-	pm_runtime_get_sync(hba->dev);					\
+	scsi_autopm_get_device(hba->sdev_ufs_device);			\
 	ret = ufshcd_query_flag(hba, UPIU_QUERY_OPCODE_READ_FLAG,	\
 		QUERY_FLAG_IDN##_uname, index, &flag);			\
-	pm_runtime_put_sync(hba->dev);					\
+	scsi_autopm_put_device(hba->sdev_ufs_device);			\
 	if (ret) {							\
 		ret = -EINVAL;						\
 		goto out;						\
@@ -813,10 +813,10 @@ static ssize_t _name##_show(struct device *dev,				\
 	}								\
 	if (ufshcd_is_wb_attrs(QUERY_ATTR_IDN##_uname))			\
 		index = ufshcd_wb_get_query_index(hba);			\
-	pm_runtime_get_sync(hba->dev);					\
+	scsi_autopm_get_device(hba->sdev_ufs_device);			\
 	ret = ufshcd_query_attr(hba, UPIU_QUERY_OPCODE_READ_ATTR,	\
 		QUERY_ATTR_IDN##_uname, index, 0, &value);		\
-	pm_runtime_put_sync(hba->dev);					\
+	scsi_autopm_put_device(hba->sdev_ufs_device);			\
 	if (ret) {							\
 		ret = -EINVAL;						\
 		goto out;						\
@@ -899,11 +899,15 @@ static ssize_t _pname##_show(struct device *dev,			\
 	struct scsi_device *sdev = to_scsi_device(dev);			\
 	struct ufs_hba *hba = shost_priv(sdev->host);			\
 	u8 lun = ufshcd_scsi_to_upiu_lun(sdev->lun);			\
+	int ret;							\
 	if (!ufs_is_valid_unit_desc_lun(&hba->dev_info, lun,		\
 				_duname##_DESC_PARAM##_puname))		\
 		return -EINVAL;						\
-	return ufs_sysfs_read_desc_param(hba, QUERY_DESC_IDN_##_duname,	\
+	scsi_autopm_get_device(sdev);					\
+	ret = ufs_sysfs_read_desc_param(hba, QUERY_DESC_IDN_##_duname,	\
 		lun, _duname##_DESC_PARAM##_puname, buf, _size);	\
+	scsi_autopm_put_device(sdev);					\
+	return ret;							\
 }									\
 static DEVICE_ATTR_RO(_pname)
 
@@ -964,10 +968,10 @@ static ssize_t dyn_cap_needed_attribute_show(struct device *dev,
 		goto out;
 	}
 
-	pm_runtime_get_sync(hba->dev);
+	scsi_autopm_get_device(hba->sdev_ufs_device);
 	ret = ufshcd_query_attr(hba, UPIU_QUERY_OPCODE_READ_ATTR,
 		QUERY_ATTR_IDN_DYN_CAP_NEEDED, lun, 0, &value);
-	pm_runtime_put_sync(hba->dev);
+	scsi_autopm_put_device(hba->sdev_ufs_device);
 	if (ret) {
 		ret = -EINVAL;
 		goto out;
-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project.