On 5/10/20 6:46 pm, Bean Huo wrote: > Hi Adrian > > thanks for submitting your patch. this patch looks fine to me. > > > do you think the new deepsleep PM level aslo should be added in > "rpm_lvl" description in Documentation/ABI/testing/sysfs-driver-ufs? Thanks for looking at this. Yes, I missed that. Fixed in V3. > > Thanks, > Bean > > > On Mon, 2020-10-05 at 16:04 +0300, Adrian Hunter wrote: >> DeepSleep is a UFS v3.1 feature that achieves the lowest power >> consumption >> of the device, apart from power off. >> >> In DeepSleep mode, no commands are accepted, and the only way to exit >> is >> using a hardware reset or power cycle. >> >> This patch assumes that if a power cycle was an option, then power >> off >> would be preferable, so only exit via a hardware reset is supported. >> >> Drivers that wish to support DeepSleep need to set a new capability >> flag >> UFSHCD_CAP_DEEPSLEEP and provide a hardware reset via the existing >> ->device_reset() callback. >> >> It is assumed that UFS devices with wspecversion >= 0x310 support >> DeepSleep. >> >> Signed-off-by: Adrian Hunter <adrian.hunter@xxxxxxxxx> >> --- >> >> >> Changes in V2: >> >> >> Fix SSU command IMMED setting and consequently drop patch 2. >> >> >> drivers/scsi/ufs/ufs-sysfs.c | 7 +++++++ >> drivers/scsi/ufs/ufs.h | 1 + >> drivers/scsi/ufs/ufshcd.c | 39 >> ++++++++++++++++++++++++++++++++++-- >> drivers/scsi/ufs/ufshcd.h | 17 +++++++++++++++- >> include/trace/events/ufs.h | 3 ++- >> 5 files changed, 63 insertions(+), 4 deletions(-) >> >> diff --git a/drivers/scsi/ufs/ufs-sysfs.c b/drivers/scsi/ufs/ufs- >> sysfs.c >> index bdcd27faa054..08e72b7eef6a 100644 >> --- a/drivers/scsi/ufs/ufs-sysfs.c >> +++ b/drivers/scsi/ufs/ufs-sysfs.c >> @@ -28,6 +28,7 @@ static const char >> *ufschd_ufs_dev_pwr_mode_to_string( >> case UFS_ACTIVE_PWR_MODE: return "ACTIVE"; >> case UFS_SLEEP_PWR_MODE: return "SLEEP"; >> case UFS_POWERDOWN_PWR_MODE: return "POWERDOWN"; >> + case UFS_DEEPSLEEP_PWR_MODE: return "DEEPSLEEP"; >> default: return "UNKNOWN"; >> } >> } >> @@ -38,6 +39,7 @@ static inline ssize_t ufs_sysfs_pm_lvl_store(struct >> device *dev, >> bool rpm) >> { >> struct ufs_hba *hba = dev_get_drvdata(dev); >> + struct ufs_dev_info *dev_info = &hba->dev_info; >> unsigned long flags, value; >> >> if (kstrtoul(buf, 0, &value)) >> @@ -46,6 +48,11 @@ static inline ssize_t >> ufs_sysfs_pm_lvl_store(struct device *dev, >> if (value >= UFS_PM_LVL_MAX) >> return -EINVAL; >> >> + if (ufs_pm_lvl_states[value].dev_state == >> UFS_DEEPSLEEP_PWR_MODE && >> + (!(hba->caps & UFSHCD_CAP_DEEPSLEEP) || >> + !(dev_info->wspecversion >= 0x310))) >> + return -EINVAL; >> + >> spin_lock_irqsave(hba->host->host_lock, flags); >> if (rpm) >> hba->rpm_lvl = value; >> diff --git a/drivers/scsi/ufs/ufs.h b/drivers/scsi/ufs/ufs.h >> index f8ab16f30fdc..d593edb48767 100644 >> --- a/drivers/scsi/ufs/ufs.h >> +++ b/drivers/scsi/ufs/ufs.h >> @@ -442,6 +442,7 @@ enum ufs_dev_pwr_mode { >> UFS_ACTIVE_PWR_MODE = 1, >> UFS_SLEEP_PWR_MODE = 2, >> UFS_POWERDOWN_PWR_MODE = 3, >> + UFS_DEEPSLEEP_PWR_MODE = 4, >> }; >> >> #define UFS_WB_BUF_REMAIN_PERCENT(val) ((val) / 10) >> diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c >> index b8f573a02713..ccacf54ed7ef 100644 >> --- a/drivers/scsi/ufs/ufshcd.c >> +++ b/drivers/scsi/ufs/ufshcd.c >> @@ -163,6 +163,11 @@ struct ufs_pm_lvl_states ufs_pm_lvl_states[] = { >> {UFS_SLEEP_PWR_MODE, UIC_LINK_HIBERN8_STATE}, >> {UFS_POWERDOWN_PWR_MODE, UIC_LINK_HIBERN8_STATE}, >> {UFS_POWERDOWN_PWR_MODE, UIC_LINK_OFF_STATE}, >> + /* >> + * For DeepSleep, the link is first put in hibern8 and then >> off. >> + * Leaving the link in hibern8 is not supported. >> + */ >> + {UFS_DEEPSLEEP_PWR_MODE, UIC_LINK_OFF_STATE}, >> }; >> >> static inline enum ufs_dev_pwr_mode >> @@ -8292,7 +8297,8 @@ static int ufshcd_link_state_transition(struct >> ufs_hba *hba, >> } >> /* >> * If autobkops is enabled, link can't be turned off because >> - * turning off the link would also turn off the device. >> + * turning off the link would also turn off the device, except >> in the >> + * case of DeepSleep where the device is expected to remain >> powered. >> */ >> else if ((req_link_state == UIC_LINK_OFF_STATE) && >> (!check_for_bkops || !hba->auto_bkops_enabled)) { >> @@ -8302,6 +8308,9 @@ static int ufshcd_link_state_transition(struct >> ufs_hba *hba, >> * put the link in low power mode is to send the DME >> end point >> * to device and then send the DME reset command to >> local >> * unipro. But putting the link in hibern8 is much >> faster. >> + * >> + * Note also that putting the link in Hibern8 is a >> requirement >> + * for entering DeepSleep. >> */ >> ret = ufshcd_uic_hibern8_enter(hba); >> if (ret) { >> @@ -8434,6 +8443,7 @@ static void ufshcd_hba_vreg_set_hpm(struct >> ufs_hba *hba) >> 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; >> @@ -8519,7 +8529,13 @@ static int ufshcd_suspend(struct ufs_hba *hba, >> enum ufs_pm_op pm_op) >> } >> >> flush_work(&hba->eeh_work); >> - ret = ufshcd_link_state_transition(hba, req_link_state, 1); >> + >> + /* >> + * In the case of DeepSleep, the device is expected to remain >> powered >> + * with the link off, so do not check for bkops. >> + */ >> + 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; >> >> @@ -8560,11 +8576,25 @@ static int ufshcd_suspend(struct ufs_hba >> *hba, enum ufs_pm_op pm_op) >> if (hba->clk_scaling.is_allowed) >> ufshcd_resume_clkscaling(hba); >> 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 >> + * further below. >> + */ >> + if (ufshcd_is_ufs_dev_deepsleep(hba)) { >> + ufshcd_vops_device_reset(hba); >> + WARN_ON(!ufshcd_is_link_off(hba)); >> + } >> if (ufshcd_is_link_hibern8(hba) && >> !ufshcd_uic_hibern8_exit(hba)) >> 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_vops_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: >> @@ -8626,6 +8656,9 @@ static int ufshcd_resume(struct ufs_hba *hba, >> enum ufs_pm_op pm_op) >> if (ret) >> goto disable_vreg; >> >> + /* For DeepSleep, the only supported option is to have the link >> off */ >> + WARN_ON(ufshcd_is_ufs_dev_deepsleep(hba) && >> !ufshcd_is_link_off(hba)); >> + >> if (ufshcd_is_link_hibern8(hba)) { >> ret = ufshcd_uic_hibern8_exit(hba); >> if (!ret) { >> @@ -8639,6 +8672,8 @@ 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. >> + * Note, in the case of DeepSleep, the device will exit >> + * DeepSleep due to device reset. >> */ >> ret = ufshcd_reset_and_restore(hba); >> /* >> diff --git a/drivers/scsi/ufs/ufshcd.h b/drivers/scsi/ufs/ufshcd.h >> index 6663325ed8a0..8c6094fb35f4 100644 >> --- a/drivers/scsi/ufs/ufshcd.h >> +++ b/drivers/scsi/ufs/ufshcd.h >> @@ -114,16 +114,22 @@ enum uic_link_state { >> ((h)->curr_dev_pwr_mode = UFS_SLEEP_PWR_MODE) >> #define ufshcd_set_ufs_dev_poweroff(h) \ >> ((h)->curr_dev_pwr_mode = UFS_POWERDOWN_PWR_MODE) >> +#define ufshcd_set_ufs_dev_deepsleep(h) \ >> + ((h)->curr_dev_pwr_mode = UFS_DEEPSLEEP_PWR_MODE) >> #define ufshcd_is_ufs_dev_active(h) \ >> ((h)->curr_dev_pwr_mode == UFS_ACTIVE_PWR_MODE) >> #define ufshcd_is_ufs_dev_sleep(h) \ >> ((h)->curr_dev_pwr_mode == UFS_SLEEP_PWR_MODE) >> #define ufshcd_is_ufs_dev_poweroff(h) \ >> ((h)->curr_dev_pwr_mode == UFS_POWERDOWN_PWR_MODE) >> +#define ufshcd_is_ufs_dev_deepsleep(h) \ >> + ((h)->curr_dev_pwr_mode == UFS_DEEPSLEEP_PWR_MODE) >> >> /* >> * UFS Power management levels. >> - * Each level is in increasing order of power savings. >> + * Each level is in increasing order of power savings, except >> DeepSleep >> + * which is lower than PowerDown with power on but not PowerDown >> with >> + * power off. >> */ >> enum ufs_pm_level { >> UFS_PM_LVL_0, /* UFS_ACTIVE_PWR_MODE, UIC_LINK_ACTIVE_STATE */ >> @@ -132,6 +138,7 @@ enum ufs_pm_level { >> UFS_PM_LVL_3, /* UFS_SLEEP_PWR_MODE, UIC_LINK_HIBERN8_STATE */ >> UFS_PM_LVL_4, /* UFS_POWERDOWN_PWR_MODE, UIC_LINK_HIBERN8_STATE >> */ >> UFS_PM_LVL_5, /* UFS_POWERDOWN_PWR_MODE, UIC_LINK_OFF_STATE */ >> + UFS_PM_LVL_6, /* UFS_DEEPSLEEP_PWR_MODE, UIC_LINK_OFF_STATE */ >> UFS_PM_LVL_MAX >> }; >> >> @@ -591,6 +598,14 @@ enum ufshcd_caps { >> * inline crypto engine, if it is present >> */ >> UFSHCD_CAP_CRYPTO = 1 << 8, >> + >> + /* >> + * This capability allows the host controller driver to use >> DeepSleep, >> + * if it is supported by the UFS device. The host controller >> driver must >> + * support device hardware reset via the hba->device_reset() >> callback, >> + * in order to exit DeepSleep state. >> + */ >> + UFSHCD_CAP_DEEPSLEEP = 1 << 9, >> }; >> >> struct ufs_hba_variant_params { >> diff --git a/include/trace/events/ufs.h b/include/trace/events/ufs.h >> index 84841b3a7ffd..2362244c2a9e 100644 >> --- a/include/trace/events/ufs.h >> +++ b/include/trace/events/ufs.h >> @@ -19,7 +19,8 @@ >> #define UFS_PWR_MODES \ >> EM(UFS_ACTIVE_PWR_MODE) \ >> EM(UFS_SLEEP_PWR_MODE) \ >> - EMe(UFS_POWERDOWN_PWR_MODE) >> + EM(UFS_POWERDOWN_PWR_MODE) \ >> + EMe(UFS_DEEPSLEEP_PWR_MODE) >> >> #define UFSCHD_CLK_GATING_STATES \ >> EM(CLKS_OFF) \ >