On 12/11/2021 09:09, Adrian Hunter wrote:
> On 27/10/2021 16:06, Adrian Hunter wrote:
>> If the UFS Device WLUN is runtime suspended and is in the same power
>> mode, link state and b_rpm_dev_flush_capable (BKOP or WB buffer flush etc)
>> state, then it can remain runtime suspended instead of being runtime
>> resumed and then system suspended.
>>
>> The following patch has cleared the way for that to happen:
>> scsi: core: pm: Only runtime resume if necessary
>>
>> So amend the logic accordingly.
>>
>> Note, the ufs-hisi driver uses different RPM and SPM, but it is made
>> explicit by a new parameter to suspend prepare.
>>
>> Signed-off-by: Adrian Hunter <adrian.hunter@xxxxxxxxx>
>> Reviewed-by: Asutosh Das <asutoshd@xxxxxxxxxxxxxx>
>
> Any more comments on this?
Seems like there are no objections, so can this be applied for 5.17 ?
>
>> ---
>> drivers/scsi/ufs/ufs-hisi.c | 8 ++++++-
>> drivers/scsi/ufs/ufshcd.c | 45 ++++++++++++++++++++++++++++++++-----
>> drivers/scsi/ufs/ufshcd.h | 11 +++++++++
>> 3 files changed, 58 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/scsi/ufs/ufs-hisi.c b/drivers/scsi/ufs/ufs-hisi.c
>> index 8c7e8d321746..ab1a7ebd89b1 100644
>> --- a/drivers/scsi/ufs/ufs-hisi.c
>> +++ b/drivers/scsi/ufs/ufs-hisi.c
>> @@ -396,6 +396,12 @@ static int ufs_hisi_pwr_change_notify(struct ufs_hba *hba,
>> return ret;
>> }
>>
>> +static int ufs_hisi_suspend_prepare(struct device *dev)
>> +{
>> + /* RPM and SPM are different. Refer ufs_hisi_suspend() */
>> + return __ufshcd_suspend_prepare(dev, false);
>> +}
>> +
>> static int ufs_hisi_suspend(struct ufs_hba *hba, enum ufs_pm_op pm_op,
>> enum ufs_notify_change_status status)
>> {
>> @@ -578,7 +584,7 @@ static int ufs_hisi_remove(struct platform_device *pdev)
>> static const struct dev_pm_ops ufs_hisi_pm_ops = {
>> SET_SYSTEM_SLEEP_PM_OPS(ufshcd_system_suspend, ufshcd_system_resume)
>> SET_RUNTIME_PM_OPS(ufshcd_runtime_suspend, ufshcd_runtime_resume, NULL)
>> - .prepare = ufshcd_suspend_prepare,
>> + .prepare = ufs_hisi_suspend_prepare,
>> .complete = ufshcd_resume_complete,
>> };
>>
>> diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
>> index 04cb67995750..115ea16f5a22 100644
>> --- a/drivers/scsi/ufs/ufshcd.c
>> +++ b/drivers/scsi/ufs/ufshcd.c
>> @@ -9707,7 +9707,27 @@ void ufshcd_resume_complete(struct device *dev)
>> }
>> EXPORT_SYMBOL_GPL(ufshcd_resume_complete);
>>
>> -int ufshcd_suspend_prepare(struct device *dev)
>> +static bool ufshcd_rpm_ok_for_spm(struct ufs_hba *hba)
>> +{
>> + struct device *dev = &hba->sdev_ufs_device->sdev_gendev;
>> + enum ufs_dev_pwr_mode dev_pwr_mode;
>> + enum uic_link_state link_state;
>> + unsigned long flags;
>> + bool res;
>> +
>> + spin_lock_irqsave(&dev->power.lock, flags);
>> + dev_pwr_mode = ufs_get_pm_lvl_to_dev_pwr_mode(hba->spm_lvl);
>> + link_state = ufs_get_pm_lvl_to_link_pwr_state(hba->spm_lvl);
>> + res = pm_runtime_suspended(dev) &&
>> + hba->curr_dev_pwr_mode == dev_pwr_mode &&
>> + hba->uic_link_state == link_state &&
>> + !hba->dev_info.b_rpm_dev_flush_capable;
>> + spin_unlock_irqrestore(&dev->power.lock, flags);
>> +
>> + return res;
>> +}
>> +
>> +int __ufshcd_suspend_prepare(struct device *dev, bool rpm_ok_for_spm)
>> {
>> struct ufs_hba *hba = dev_get_drvdata(dev);
>> int ret;
>> @@ -9719,15 +9739,30 @@ int ufshcd_suspend_prepare(struct device *dev)
>> * Refer ufshcd_resume_complete()
>> */
>> if (hba->sdev_ufs_device) {
>> - ret = ufshcd_rpm_get_sync(hba);
>> - if (ret < 0 && ret != -EACCES) {
>> - ufshcd_rpm_put(hba);
>> - return ret;
>> + /* Prevent runtime suspend */
>> + ufshcd_rpm_get_noresume(hba);
>> + /*
>> + * Check if already runtime suspended in same state as system
>> + * suspend would be.
>> + */
>> + if (!rpm_ok_for_spm || !ufshcd_rpm_ok_for_spm(hba)) {
>> + /* RPM state is not ok for SPM, so runtime resume */
>> + ret = ufshcd_rpm_resume(hba);
>> + if (ret < 0 && ret != -EACCES) {
>> + ufshcd_rpm_put(hba);
>> + return ret;
>> + }
>> }
>> hba->complete_put = true;
>> }
>> return 0;
>> }
>> +EXPORT_SYMBOL_GPL(__ufshcd_suspend_prepare);
>> +
>> +int ufshcd_suspend_prepare(struct device *dev)
>> +{
>> + return __ufshcd_suspend_prepare(dev, true);
>> +}
>> EXPORT_SYMBOL_GPL(ufshcd_suspend_prepare);
>>
>> #ifdef CONFIG_PM_SLEEP
>> diff --git a/drivers/scsi/ufs/ufshcd.h b/drivers/scsi/ufs/ufshcd.h
>> index 3f5dc6732fe1..b9492f300bd1 100644
>> --- a/drivers/scsi/ufs/ufshcd.h
>> +++ b/drivers/scsi/ufs/ufshcd.h
>> @@ -1199,6 +1199,7 @@ int ufshcd_exec_raw_upiu_cmd(struct ufs_hba *hba,
>>
>> int ufshcd_wb_toggle(struct ufs_hba *hba, bool enable);
>> int ufshcd_suspend_prepare(struct device *dev);
>> +int __ufshcd_suspend_prepare(struct device *dev, bool rpm_ok_for_spm);
>> void ufshcd_resume_complete(struct device *dev);
>>
>> /* Wrapper functions for safely calling variant operations */
>> @@ -1408,6 +1409,16 @@ static inline int ufshcd_rpm_put_sync(struct ufs_hba *hba)
>> return pm_runtime_put_sync(&hba->sdev_ufs_device->sdev_gendev);
>> }
>>
>> +static inline void ufshcd_rpm_get_noresume(struct ufs_hba *hba)
>> +{
>> + pm_runtime_get_noresume(&hba->sdev_ufs_device->sdev_gendev);
>> +}
>> +
>> +static inline int ufshcd_rpm_resume(struct ufs_hba *hba)
>> +{
>> + return pm_runtime_resume(&hba->sdev_ufs_device->sdev_gendev);
>> +}
>> +
>> static inline int ufshcd_rpm_put(struct ufs_hba *hba)
>> {
>> return pm_runtime_put(&hba->sdev_ufs_device->sdev_gendev);
>>
>
