2018-07-06 12:41:29

by Asutosh Das (asd)

Subject: [PATCH v1 8/9] scsi: ufs: enable runtime pm only after ufshcd init

From: Gilad Broner <[email protected]>

Previous code enables runtime pm before ufshcd_init() is completed,
and before the hba struct is stored in the platform device private
data. This means that pm runtime calls will have null hba pointer
as well as partially initialized driver.
Instead, enable pm runtime only after ufshcd_init() is done and
after hba struct is stored in the platform device private data.

Signed-off-by: Gilad Broner <[email protected]>
Signed-off-by: Can Guo <[email protected]>
Signed-off-by: Asutosh Das <[email protected]>
---
drivers/scsi/ufs/ufshcd-pltfrm.c | 11 ++++-------
1 file changed, 4 insertions(+), 7 deletions(-)

diff --git a/drivers/scsi/ufs/ufshcd-pltfrm.c b/drivers/scsi/ufs/ufshcd-pltfrm.c
index 7dba799..8332d99 100644
--- a/drivers/scsi/ufs/ufshcd-pltfrm.c
+++ b/drivers/scsi/ufs/ufshcd-pltfrm.c
@@ -354,24 +354,21 @@ int ufshcd_pltfrm_init(struct platform_device *pdev,
}

ufshcd_parse_pm_levels(hba);
- pm_runtime_set_active(&pdev->dev);
- pm_runtime_enable(&pdev->dev);

ufshcd_init_lanes_per_dir(hba);

err = ufshcd_init(hba, mmio_base, irq);
if (err) {
dev_err(dev, "Initialization failed\n");
- goto out_disable_rpm;
+ goto dealloc_host;
}

platform_set_drvdata(pdev, hba);

- return 0;
+ pm_runtime_set_active(&pdev->dev);
+ pm_runtime_enable(&pdev->dev);

-out_disable_rpm:
- pm_runtime_disable(&pdev->dev);
- pm_runtime_set_suspended(&pdev->dev);
+ return 0;
dealloc_host:
ufshcd_dealloc_host(hba);
out:
--
Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project.

2018-07-21 00:07:09

by Subhash Jadavani

Subject: Re: [PATCH v1 8/9] scsi: ufs: enable runtime pm only after ufshcd init

On 2018-07-06 05:30, Asutosh Das wrote:
> From: Gilad Broner <[email protected]>
>
> Previous code enables runtime pm before ufshcd_init() is completed,
> and before the hba struct is stored in the platform device private
> data. This means that pm runtime calls will have null hba pointer
> as well as partially initialized driver.
> Instead, enable pm runtime only after ufshcd_init() is done and
> after hba struct is stored in the platform device private data.
>
> Signed-off-by: Gilad Broner <[email protected]>
> Signed-off-by: Can Guo <[email protected]>
> Signed-off-by: Asutosh Das <[email protected]>
> ---
> drivers/scsi/ufs/ufshcd-pltfrm.c | 11 ++++-------
> 1 file changed, 4 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/scsi/ufs/ufshcd-pltfrm.c
> b/drivers/scsi/ufs/ufshcd-pltfrm.c
> index 7dba799..8332d99 100644
> --- a/drivers/scsi/ufs/ufshcd-pltfrm.c
> +++ b/drivers/scsi/ufs/ufshcd-pltfrm.c
> @@ -354,24 +354,21 @@ int ufshcd_pltfrm_init(struct platform_device
> *pdev,
> }
>
> ufshcd_parse_pm_levels(hba);
> - pm_runtime_set_active(&pdev->dev);
> - pm_runtime_enable(&pdev->dev);
>
> ufshcd_init_lanes_per_dir(hba);
>
> err = ufshcd_init(hba, mmio_base, irq);
> if (err) {
> dev_err(dev, "Initialization failed\n");
> - goto out_disable_rpm;
> + goto dealloc_host;
> }
>
> platform_set_drvdata(pdev, hba);
>
> - return 0;
> + pm_runtime_set_active(&pdev->dev);
> + pm_runtime_enable(&pdev->dev);
>
> -out_disable_rpm:
> - pm_runtime_disable(&pdev->dev);
> - pm_runtime_set_suspended(&pdev->dev);
> + return 0;
> dealloc_host:
> ufshcd_dealloc_host(hba);
> out:

Looks good to me.
Reviewed-by: Subhash Jadavani <[email protected]>

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