Hi Kiwoong,
On 2020-07-02 16:20, Kiwoong Kim wrote:
When you set uic_link_state during sleep statae to
UIC_LINK_OFF_STATE, UFS driver does interface initialization
that is a series of some steps including fDeviceInit and thus,
You might feel that its latency is a little bit longer.
This patch is run it asynchronously to reduce system wake-up time.
Have you considered the existing async suspend/resume support from
kernel power management framework? Can device_enable_async_suspend()
serve your purpose?
I don't see how this change works in two ways
#1 During system resume, how do you make sure resume of children
devices come after the resume of their parent? To be more specific,
in UFS's case, how do you make sure scsi devices (sda, sdb...) start
to resume only AFTER hba resume is finished (I mean all steps in hba
probe finished, not just schedule the async resume work). Your test
passed only because that in current scsi device's resume ops, no actual
commands need to be sent during scsi devices resume. If some commands
need to be sent during scsi device resume (say SSU commands), you would
run into error since scsi commands are sent before host is fully
resumed.
If you use kernel power management framework, during resume, dpm_wait()
is called for each device to make its parent has finished resuming.
#2 ufshcd_resume() is called in both hba system and runtime resume path.
Your change even makes hba runtime resume ops "async". How can you make
sure that hba is resumed after pm_runtime_get_sync() returns? Besides,
I don't think it can work well along with block layer PM.
Thanks,
Can Guo
Signed-off-by: Kiwoong Kim <kwmad.kim@xxxxxxxxxxx>
---
drivers/scsi/ufs/Kconfig | 10 ++++
drivers/scsi/ufs/ufshcd.c | 120
++++++++++++++++++++++++++++++++++------------
2 files changed, 100 insertions(+), 30 deletions(-)
diff --git a/drivers/scsi/ufs/Kconfig b/drivers/scsi/ufs/Kconfig
index 8cd9026..723e7cb 100644
--- a/drivers/scsi/ufs/Kconfig
+++ b/drivers/scsi/ufs/Kconfig
@@ -172,3 +172,13 @@ config SCSI_UFS_EXYNOS
Select this if you have UFS host controller on EXYNOS chipset.
If unsure, say N.
+
+config SCSI_UFSHCD_ASYNC_INIT
+ bool "Asynchronous UFS interface initialization support"
+ depends on SCSI_UFSHCD
+ default n
+ ---help---
+ This selects the support of doing UFS interface initialization
+ asynchronously when you set link state to link off,
+ i.e. UIC_LINK_OFF_STATE, to reduce system wake-up time.
+ Select this if you have UFS Host Controller.
diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index 52abe82..b65d38c 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -8319,6 +8319,80 @@ static int ufshcd_suspend(struct ufs_hba *hba,
enum ufs_pm_op pm_op)
return ret;
}
+static int ufshcd_post_resume(struct ufs_hba *hba)
+{
+ int ret;
+
+ 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);
+
+ hba->clk_gating.is_suspended = false;
+
+ if (hba->clk_scaling.is_allowed)
+ ufshcd_resume_clkscaling(hba);
+
+ /* 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);
+ }
+
+ /* Schedule clock gating in case of no access to UFS device yet */
+ ufshcd_release(hba);
+out:
+ return ret;
+}
+
+#if defined(SCSI_UFSHCD_ASYNC_INIT)
+static void ufshcd_async_resume(void *data, async_cookie_t cookie)
+{
+ struct ufs_hba *hba = (struct ufs_hba *)data;
+ unsigned long flags;
+ int ret = 0;
+ int retries = 2;
+
+ /* transition to block requests */
+ spin_lock_irqsave(hba->host->host_lock, flags);
+ hba->ufshcd_state = UFSHCD_STATE_RESET;
+ spin_unlock_irqrestore(hba->host->host_lock, flags);
+
+ /* initialize, instead of set_old_link_state ?? */
+ do {
+ ret = ufshcd_reset_and_restore(hba);
+ if (ret) {
+ dev_err(hba->dev, "%s: reset and restore failed\n",
+ __func__);
+ spin_lock_irqsave(hba->host->host_lock, flags);
+ hba->ufshcd_state = UFSHCD_STATE_ERROR;
+ hba->pm_op_in_progress = 0;
+ spin_unlock_irqrestore(hba->host->host_lock, flags);
+ return;
+ }
+ ret = ufshcd_post_resume(hba);
+ } while (ret && --retries);
+ if (ret)
+ goto reset;
+
+ hba->pm_op_in_progress = 0;
+ if (ret)
+ ufshcd_update_reg_hist(&hba->ufs_stats.resume_err, (u32)ret);
+}
+#endif
+
/**
* ufshcd_resume - helper function for resume operations
* @hba: per adapter instance
@@ -8370,6 +8444,14 @@ static int ufshcd_resume(struct ufs_hba *hba,
enum ufs_pm_op pm_op)
* A full initialization of the host and the device is
* required since the link was put to off during suspend.
*/
+#if defined(SCSI_UFSHCD_ASYNC_INIT)
+ /*
+ * Assuems error free since ufshcd_probe_hba failure is
+ * uncorrectable.
+ */
+ ufshcd_async_schedule(ufshcd_async_resume, hba);
+ goto out_new;
+#else
ret = ufshcd_reset_and_restore(hba);
/*
* ufshcd_reset_and_restore() should have already
@@ -8377,38 +8459,12 @@ static int ufshcd_resume(struct ufs_hba *hba,
enum ufs_pm_op pm_op)
*/
if (ret || !ufshcd_is_link_active(hba))
goto vendor_suspend;
+#endif
}
- 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 (hba->clk_scaling.is_allowed)
- ufshcd_resume_clkscaling(hba);
-
- /* 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);
- }
-
- /* Schedule clock gating in case of no access to UFS device yet */
- ufshcd_release(hba);
+ ret = ufshcd_post_resume(hba);
+ if (ret)
+ goto set_old_link_state;
goto out;
@@ -8427,6 +8483,10 @@ static int ufshcd_resume(struct ufs_hba *hba,
enum ufs_pm_op pm_op)
hba->pm_op_in_progress = 0;
if (ret)
ufshcd_update_reg_hist(&hba->ufs_stats.resume_err, (u32)ret);
+ /* For async init, pm_op_in_progress still needs to be one */
+#if defined(SCSI_UFSHCD_ASYNC_INIT)
+out_new:
+#endif
return ret;
}
