[PATCH 0/3] scsi: ufs: runtime pm fixes

[PATCH 0/3] scsi: ufs: runtime pm fixes

[Rajendra Nayak]

These are some runtime pm fixes I stumbled upon while
working on adding support for controlling multiple
power domains (needed on qualcomm sdm845 platforms)
in the ufshcd-pltfrm driver.

Rajendra Nayak (3):
  scsi: ufs: Fix runtime pm handling in ufshcd-pltfrm
  scsi: ufs: Add error checks for pm_runtime_get_sync()
  scsi: ufs: qcom: Remove unnecessary runtime pm calls

 drivers/scsi/ufs/ufs-qcom.c      | 2 --
 drivers/scsi/ufs/ufshcd-pltfrm.c | 3 +++
 drivers/scsi/ufs/ufshcd.c        | 5 ++++-
 3 files changed, 7 insertions(+), 3 deletions(-)

-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation

[PATCH 1/3] scsi: ufs: Fix runtime pm handling in ufshcd-pltfrm

[Rajendra Nayak]

runtime pm calls as part of ufshcd_init() can trigger a
ufshcd_runtime_suspend() which always fails until the point
we do a platform_set_drvdata(), setting the devices 'power.runtime_error'

Use pm_runtime_get_noresume()/pm_runtime_put_noidle() to prevent
this from happening.

Signed-off-by: Rajendra Nayak <rnayak@codeaurora.org>
---
 drivers/scsi/ufs/ufshcd-pltfrm.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/scsi/ufs/ufshcd-pltfrm.c b/drivers/scsi/ufs/ufshcd-pltfrm.c
index 895a9b5ac989..974441fa4e75 100644
--- a/drivers/scsi/ufs/ufshcd-pltfrm.c
+++ b/drivers/scsi/ufs/ufshcd-pltfrm.c
@@ -340,6 +340,7 @@ int ufshcd_pltfrm_init(struct platform_device *pdev,
 		goto dealloc_host;
 	}
 
+	pm_runtime_get_noresume(&pdev->dev);
 	pm_runtime_set_active(&pdev->dev);
 	pm_runtime_enable(&pdev->dev);
 
@@ -352,12 +353,14 @@ int ufshcd_pltfrm_init(struct platform_device *pdev,
 	}
 
 	platform_set_drvdata(pdev, hba);
+	pm_runtime_put_noidle(&pdev->dev);
 
 	return 0;
 
 out_disable_rpm:
 	pm_runtime_disable(&pdev->dev);
 	pm_runtime_set_suspended(&pdev->dev);
+	pm_runtime_put_noidle(&pdev->dev);
 dealloc_host:
 	ufshcd_dealloc_host(hba);
 out:
-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation

[PATCH 2/3] scsi: ufs: Add error checks for pm_runtime_get_sync()

[Rajendra Nayak]

Add an error check for pm_runtime_get_sync(), ignoring this can
hide issues with the runtime pm handling in the driver.

Signed-off-by: Rajendra Nayak <rnayak@codeaurora.org>
---
 drivers/scsi/ufs/ufshcd.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index 2ddf24466a62..060dc38cc582 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -8357,7 +8357,10 @@ int ufshcd_init(struct ufs_hba *hba, void __iomem *mmio_base, unsigned int irq)
 	}
 
 	/* Hold auto suspend until async scan completes */
-	pm_runtime_get_sync(dev);
+	err = pm_runtime_get_sync(dev);
+	if (err < 0)
+		goto out_remove_scsi_host;
+
 	atomic_set(&hba->scsi_block_reqs_cnt, 0);
 	/*
 	 * We are assuming that device wasn't put in sleep/power-down
-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation

[PATCH 3/3] scsi: ufs: qcom: Remove unnecessary runtime pm calls

[Rajendra Nayak]

Doing a runtime pm get/put as part of ufs_qcom_testbus_config()
seems completely unnecessary, since this is called only early
during ufs_qcom_init() when the runtime operations are not
completely setup for ufs runtime suspend/resume to even work.

Signed-off-by: Rajendra Nayak <rnayak@codeaurora.org>
---
 drivers/scsi/ufs/ufs-qcom.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/scsi/ufs/ufs-qcom.c b/drivers/scsi/ufs/ufs-qcom.c
index 3aeadb14aae1..3590741eea2f 100644
--- a/drivers/scsi/ufs/ufs-qcom.c
+++ b/drivers/scsi/ufs/ufs-qcom.c
@@ -1558,7 +1558,6 @@ int ufs_qcom_testbus_config(struct ufs_qcom_host *host)
 	}
 	mask <<= offset;
 
-	pm_runtime_get_sync(host->hba->dev);
 	ufshcd_hold(host->hba, false);
 	ufshcd_rmwl(host->hba, TEST_BUS_SEL,
 		    (u32)host->testbus.select_major << 19,
@@ -1573,7 +1572,6 @@ int ufs_qcom_testbus_config(struct ufs_qcom_host *host)
 	 */
 	mb();
 	ufshcd_release(host->hba);
-	pm_runtime_put_sync(host->hba->dev);
 
 	return 0;
 }
-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation

RE: [PATCH 2/3] scsi: ufs: Add error checks for pm_runtime_get_sync()

[Avri Altman]

Hi,
> 
> Add an error check for pm_runtime_get_sync(), ignoring this can
> hide issues with the runtime pm handling in the driver.
Can you elaborate on those issues?
I guess you've encountered some during your bring-up.

> 
> Signed-off-by: Rajendra Nayak <rnayak@codeaurora.org>
> ---
>  drivers/scsi/ufs/ufshcd.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
> index 2ddf24466a62..060dc38cc582 100644
> --- a/drivers/scsi/ufs/ufshcd.c
> +++ b/drivers/scsi/ufs/ufshcd.c
> @@ -8357,7 +8357,10 @@ int ufshcd_init(struct ufs_hba *hba, void __iomem
> *mmio_base, unsigned int irq)
>  	}
> 
>  	/* Hold auto suspend until async scan completes */
> -	pm_runtime_get_sync(dev);
> +	err = pm_runtime_get_sync(dev);
> +	if (err < 0)
> +		goto out_remove_scsi_host;
> +
>  	atomic_set(&hba->scsi_block_reqs_cnt, 0);
>  	/*
>  	 * We are assuming that device wasn't put in sleep/power-down
On your first patch you are incrementing the device's usage counter,
To avoid any suspend during probe, but this comment above state
 some assumption of this very same issue.
Is it still valid?

Thanks,
Avri