Re: [PATCH 08/10] ufs-qcom: phy/hcd: Refactoring phy clock handling

[Vivek Gautam]

On Thu, Oct 20, 2016 at 12:48 AM, Subhash Jadavani
 wrote:
> On 2016-10-19 10:45, Vivek Gautam wrote:
>>
>> Hi,
>>
>>
>> On Wed, Oct 19, 2016 at 1:43 AM, Subhash Jadavani
>>  wrote:
>>>
>>> On 2016-10-18 07:28, Vivek Gautam wrote:


 Add phy clock enable code to phy_power_on/off callbacks, and
 remove explicit calls to enable these phy clocks from the
 ufs-qcom hcd driver.

 Signed-off-by: Vivek Gautam 
 ---

 Changes since v1:
  - staticized ufs_qcom_phy_enable(/disable)_ref_clk(),
  - staticized ufs_qcom_phy_enable(/disable)_iface_clk()
  - removed function declaration and export symbol for these APIs.
>>
>>
>> [snip]
>>
 --- a/drivers/scsi/ufs/ufs-qcom.c
 +++ b/drivers/scsi/ufs/ufs-qcom.c
 @@ -1112,17 +1112,6 @@ static int ufs_qcom_setup_clocks(struct ufs_hba
 *hba, bool on)
 return 0;

 if (on) {
 -   err = ufs_qcom_phy_enable_iface_clk(host->generic_phy);
 -   if (err)
 -   goto out;
 -
 -   err = ufs_qcom_phy_enable_ref_clk(host->generic_phy);
 -   if (err) {
 -   dev_err(hba->dev, "%s enable phy ref clock
 failed,
 err=%d\n",
 -   __func__, err);
 -
 ufs_qcom_phy_disable_iface_clk(host->generic_phy);
 -   goto out;
 -   }
>>>
>>>
>>>
>>> Now that you are moving these ref clk enable/disable to phy_power_on/off
>>> and
>>> these phy_power_on/off are called only in runtime suspend/resume (3
>>> seconds
>>> after last UFS access).
>>> Goal is to disable the phy reference clock during aggressive gating (10ms
>>> from last UFS access) so shouldn't we call the phy_power_on/off from
>>> these
>>> setup_clocks() function as well?
>>>
>>
>> So setup_clocks() is called for aggressive clock gating as well ?
>> If that's the case then yes, we may need to call. But we should try to
>> understand here. The phy_power_off turns off all the clocks - reflclk,
>> and other interface clocks. Do we want all of them to be turned off ?
>
>
> Yes, we want to turn off the ref clock (& other clocks) during aggressive
> gating.

Ok.

>
>>
>> phy_power_off will also turn off the PHY. Do we want all this for
>> aggressive
>> clock gating ?
>
>
> Yes, PHY rails can be powered off both during the aggressive clk gating and
> runtime suspend. But as the regulator on/off latencies could be higher
> (especially for the shared rail), we were turning them off doing it in
> runtime suspend only (via phy_power_off()).
> Now that phy_power_on/off is managing both clocks and regulators, and we
> will really want to turn off the ref clocks during clock gating, there is no
> option but to call phy_power_on/off during aggressive gating.

Alright then, i will update the change to add phy_power_off/on()
during aggressive clock gating.


Thanks
Vivek

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

Re: [PATCH 08/10] ufs-qcom: phy/hcd: Refactoring phy clock handling

[Vivek Gautam]

On Thu, Oct 20, 2016 at 12:48 AM, Subhash Jadavani
 wrote:
> On 2016-10-19 10:45, Vivek Gautam wrote:
>>
>> Hi,
>>
>>
>> On Wed, Oct 19, 2016 at 1:43 AM, Subhash Jadavani
>>  wrote:
>>>
>>> On 2016-10-18 07:28, Vivek Gautam wrote:


 Add phy clock enable code to phy_power_on/off callbacks, and
 remove explicit calls to enable these phy clocks from the
 ufs-qcom hcd driver.

 Signed-off-by: Vivek Gautam 
 ---

 Changes since v1:
  - staticized ufs_qcom_phy_enable(/disable)_ref_clk(),
  - staticized ufs_qcom_phy_enable(/disable)_iface_clk()
  - removed function declaration and export symbol for these APIs.
>>
>>
>> [snip]
>>
 --- a/drivers/scsi/ufs/ufs-qcom.c
 +++ b/drivers/scsi/ufs/ufs-qcom.c
 @@ -1112,17 +1112,6 @@ static int ufs_qcom_setup_clocks(struct ufs_hba
 *hba, bool on)
 return 0;

 if (on) {
 -   err = ufs_qcom_phy_enable_iface_clk(host->generic_phy);
 -   if (err)
 -   goto out;
 -
 -   err = ufs_qcom_phy_enable_ref_clk(host->generic_phy);
 -   if (err) {
 -   dev_err(hba->dev, "%s enable phy ref clock
 failed,
 err=%d\n",
 -   __func__, err);
 -
 ufs_qcom_phy_disable_iface_clk(host->generic_phy);
 -   goto out;
 -   }
>>>
>>>
>>>
>>> Now that you are moving these ref clk enable/disable to phy_power_on/off
>>> and
>>> these phy_power_on/off are called only in runtime suspend/resume (3
>>> seconds
>>> after last UFS access).
>>> Goal is to disable the phy reference clock during aggressive gating (10ms
>>> from last UFS access) so shouldn't we call the phy_power_on/off from
>>> these
>>> setup_clocks() function as well?
>>>
>>
>> So setup_clocks() is called for aggressive clock gating as well ?
>> If that's the case then yes, we may need to call. But we should try to
>> understand here. The phy_power_off turns off all the clocks - reflclk,
>> and other interface clocks. Do we want all of them to be turned off ?
>
>
> Yes, we want to turn off the ref clock (& other clocks) during aggressive
> gating.

Ok.

>
>>
>> phy_power_off will also turn off the PHY. Do we want all this for
>> aggressive
>> clock gating ?
>
>
> Yes, PHY rails can be powered off both during the aggressive clk gating and
> runtime suspend. But as the regulator on/off latencies could be higher
> (especially for the shared rail), we were turning them off doing it in
> runtime suspend only (via phy_power_off()).
> Now that phy_power_on/off is managing both clocks and regulators, and we
> will really want to turn off the ref clocks during clock gating, there is no
> option but to call phy_power_on/off during aggressive gating.

Alright then, i will update the change to add phy_power_off/on()
during aggressive clock gating.


Thanks
Vivek

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

Re: [PATCH 08/10] ufs-qcom: phy/hcd: Refactoring phy clock handling

[Subhash Jadavani]

On 2016-10-19 10:45, Vivek Gautam wrote:

Hi,


On Wed, Oct 19, 2016 at 1:43 AM, Subhash Jadavani
 wrote:

On 2016-10-18 07:28, Vivek Gautam wrote:


Add phy clock enable code to phy_power_on/off callbacks, and
remove explicit calls to enable these phy clocks from the
ufs-qcom hcd driver.

Signed-off-by: Vivek Gautam 
---

Changes since v1:
 - staticized ufs_qcom_phy_enable(/disable)_ref_clk(),
 - staticized ufs_qcom_phy_enable(/disable)_iface_clk()
 - removed function declaration and export symbol for these APIs.


[snip]


--- a/drivers/scsi/ufs/ufs-qcom.c
+++ b/drivers/scsi/ufs/ufs-qcom.c
@@ -1112,17 +1112,6 @@ static int ufs_qcom_setup_clocks(struct 
ufs_hba

*hba, bool on)
return 0;

if (on) {
-   err = 
ufs_qcom_phy_enable_iface_clk(host->generic_phy);

-   if (err)
-   goto out;
-
-   err = ufs_qcom_phy_enable_ref_clk(host->generic_phy);
-   if (err) {
-   dev_err(hba->dev, "%s enable phy ref clock 
failed,

err=%d\n",
-   __func__, err);
-   
ufs_qcom_phy_disable_iface_clk(host->generic_phy);

-   goto out;
-   }



Now that you are moving these ref clk enable/disable to 
phy_power_on/off and
these phy_power_on/off are called only in runtime suspend/resume (3 
seconds

after last UFS access).
Goal is to disable the phy reference clock during aggressive gating 
(10ms
from last UFS access) so shouldn't we call the phy_power_on/off from 
these

setup_clocks() function as well?



So setup_clocks() is called for aggressive clock gating as well ?
If that's the case then yes, we may need to call. But we should try to
understand here. The phy_power_off turns off all the clocks - reflclk,
and other interface clocks. Do we want all of them to be turned off ?


Yes, we want to turn off the ref clock (& other clocks) during 
aggressive gating.




phy_power_off will also turn off the PHY. Do we want all this for 
aggressive

clock gating ?


Yes, PHY rails can be powered off both during the aggressive clk gating 
and runtime suspend. But as the regulator on/off latencies could be 
higher (especially for the shared rail), we were turning them off doing 
it in runtime suspend only (via phy_power_off()).
Now that phy_power_on/off is managing both clocks and regulators, and we 
will really want to turn off the ref clocks during clock gating, there 
is no option but to call phy_power_on/off during aggressive gating.





[snip]


Regards
Vivek


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

Re: [PATCH 08/10] ufs-qcom: phy/hcd: Refactoring phy clock handling

[Subhash Jadavani]

On 2016-10-19 10:45, Vivek Gautam wrote:

Hi,


On Wed, Oct 19, 2016 at 1:43 AM, Subhash Jadavani
 wrote:

On 2016-10-18 07:28, Vivek Gautam wrote:


Add phy clock enable code to phy_power_on/off callbacks, and
remove explicit calls to enable these phy clocks from the
ufs-qcom hcd driver.

Signed-off-by: Vivek Gautam 
---

Changes since v1:
 - staticized ufs_qcom_phy_enable(/disable)_ref_clk(),
 - staticized ufs_qcom_phy_enable(/disable)_iface_clk()
 - removed function declaration and export symbol for these APIs.


[snip]


--- a/drivers/scsi/ufs/ufs-qcom.c
+++ b/drivers/scsi/ufs/ufs-qcom.c
@@ -1112,17 +1112,6 @@ static int ufs_qcom_setup_clocks(struct 
ufs_hba

*hba, bool on)
return 0;

if (on) {
-   err = 
ufs_qcom_phy_enable_iface_clk(host->generic_phy);

-   if (err)
-   goto out;
-
-   err = ufs_qcom_phy_enable_ref_clk(host->generic_phy);
-   if (err) {
-   dev_err(hba->dev, "%s enable phy ref clock 
failed,

err=%d\n",
-   __func__, err);
-   
ufs_qcom_phy_disable_iface_clk(host->generic_phy);

-   goto out;
-   }



Now that you are moving these ref clk enable/disable to 
phy_power_on/off and
these phy_power_on/off are called only in runtime suspend/resume (3 
seconds

after last UFS access).
Goal is to disable the phy reference clock during aggressive gating 
(10ms
from last UFS access) so shouldn't we call the phy_power_on/off from 
these

setup_clocks() function as well?



So setup_clocks() is called for aggressive clock gating as well ?
If that's the case then yes, we may need to call. But we should try to
understand here. The phy_power_off turns off all the clocks - reflclk,
and other interface clocks. Do we want all of them to be turned off ?


Yes, we want to turn off the ref clock (& other clocks) during 
aggressive gating.




phy_power_off will also turn off the PHY. Do we want all this for 
aggressive

clock gating ?


Yes, PHY rails can be powered off both during the aggressive clk gating 
and runtime suspend. But as the regulator on/off latencies could be 
higher (especially for the shared rail), we were turning them off doing 
it in runtime suspend only (via phy_power_off()).
Now that phy_power_on/off is managing both clocks and regulators, and we 
will really want to turn off the ref clocks during clock gating, there 
is no option but to call phy_power_on/off during aggressive gating.





[snip]


Regards
Vivek


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

Re: [PATCH 08/10] ufs-qcom: phy/hcd: Refactoring phy clock handling

[Vivek Gautam]

Hi,


On Wed, Oct 19, 2016 at 1:43 AM, Subhash Jadavani
 wrote:
> On 2016-10-18 07:28, Vivek Gautam wrote:
>>
>> Add phy clock enable code to phy_power_on/off callbacks, and
>> remove explicit calls to enable these phy clocks from the
>> ufs-qcom hcd driver.
>>
>> Signed-off-by: Vivek Gautam 
>> ---
>>
>> Changes since v1:
>>  - staticized ufs_qcom_phy_enable(/disable)_ref_clk(),
>>  - staticized ufs_qcom_phy_enable(/disable)_iface_clk()
>>  - removed function declaration and export symbol for these APIs.

[snip]

>> --- a/drivers/scsi/ufs/ufs-qcom.c
>> +++ b/drivers/scsi/ufs/ufs-qcom.c
>> @@ -1112,17 +1112,6 @@ static int ufs_qcom_setup_clocks(struct ufs_hba
>> *hba, bool on)
>> return 0;
>>
>> if (on) {
>> -   err = ufs_qcom_phy_enable_iface_clk(host->generic_phy);
>> -   if (err)
>> -   goto out;
>> -
>> -   err = ufs_qcom_phy_enable_ref_clk(host->generic_phy);
>> -   if (err) {
>> -   dev_err(hba->dev, "%s enable phy ref clock failed,
>> err=%d\n",
>> -   __func__, err);
>> -   ufs_qcom_phy_disable_iface_clk(host->generic_phy);
>> -   goto out;
>> -   }
>
>
> Now that you are moving these ref clk enable/disable to phy_power_on/off and
> these phy_power_on/off are called only in runtime suspend/resume (3 seconds
> after last UFS access).
> Goal is to disable the phy reference clock during aggressive gating (10ms
> from last UFS access) so shouldn't we call the phy_power_on/off from these
> setup_clocks() function as well?
>

So setup_clocks() is called for aggressive clock gating as well ?
If that's the case then yes, we may need to call. But we should try to
understand here. The phy_power_off turns off all the clocks - reflclk,
and other interface clocks. Do we want all of them to be turned off ?

phy_power_off will also turn off the PHY. Do we want all this for aggressive
clock gating ?


[snip]


Regards
Vivek

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

Re: [PATCH 08/10] ufs-qcom: phy/hcd: Refactoring phy clock handling

[Vivek Gautam]

Hi,


On Wed, Oct 19, 2016 at 1:43 AM, Subhash Jadavani
 wrote:
> On 2016-10-18 07:28, Vivek Gautam wrote:
>>
>> Add phy clock enable code to phy_power_on/off callbacks, and
>> remove explicit calls to enable these phy clocks from the
>> ufs-qcom hcd driver.
>>
>> Signed-off-by: Vivek Gautam 
>> ---
>>
>> Changes since v1:
>>  - staticized ufs_qcom_phy_enable(/disable)_ref_clk(),
>>  - staticized ufs_qcom_phy_enable(/disable)_iface_clk()
>>  - removed function declaration and export symbol for these APIs.

[snip]

>> --- a/drivers/scsi/ufs/ufs-qcom.c
>> +++ b/drivers/scsi/ufs/ufs-qcom.c
>> @@ -1112,17 +1112,6 @@ static int ufs_qcom_setup_clocks(struct ufs_hba
>> *hba, bool on)
>> return 0;
>>
>> if (on) {
>> -   err = ufs_qcom_phy_enable_iface_clk(host->generic_phy);
>> -   if (err)
>> -   goto out;
>> -
>> -   err = ufs_qcom_phy_enable_ref_clk(host->generic_phy);
>> -   if (err) {
>> -   dev_err(hba->dev, "%s enable phy ref clock failed,
>> err=%d\n",
>> -   __func__, err);
>> -   ufs_qcom_phy_disable_iface_clk(host->generic_phy);
>> -   goto out;
>> -   }
>
>
> Now that you are moving these ref clk enable/disable to phy_power_on/off and
> these phy_power_on/off are called only in runtime suspend/resume (3 seconds
> after last UFS access).
> Goal is to disable the phy reference clock during aggressive gating (10ms
> from last UFS access) so shouldn't we call the phy_power_on/off from these
> setup_clocks() function as well?
>

So setup_clocks() is called for aggressive clock gating as well ?
If that's the case then yes, we may need to call. But we should try to
understand here. The phy_power_off turns off all the clocks - reflclk,
and other interface clocks. Do we want all of them to be turned off ?

phy_power_off will also turn off the PHY. Do we want all this for aggressive
clock gating ?


[snip]


Regards
Vivek

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

Re: [PATCH 08/10] ufs-qcom: phy/hcd: Refactoring phy clock handling

[Subhash Jadavani]

On 2016-10-18 07:28, Vivek Gautam wrote:

Add phy clock enable code to phy_power_on/off callbacks, and
remove explicit calls to enable these phy clocks from the
ufs-qcom hcd driver.

Signed-off-by: Vivek Gautam 
---

Changes since v1:
 - staticized ufs_qcom_phy_enable(/disable)_ref_clk(),
 - staticized ufs_qcom_phy_enable(/disable)_iface_clk()
 - removed function declaration and export symbol for these APIs.

 drivers/phy/phy-qcom-ufs.c   | 36 
++--

 drivers/scsi/ufs/ufs-qcom.c  | 15 ---
 include/linux/phy/phy-qcom-ufs.h | 18 --
 3 files changed, 18 insertions(+), 51 deletions(-)

diff --git a/drivers/phy/phy-qcom-ufs.c b/drivers/phy/phy-qcom-ufs.c
index 144b11f..a425cc2 100644
--- a/drivers/phy/phy-qcom-ufs.c
+++ b/drivers/phy/phy-qcom-ufs.c
@@ -373,10 +373,9 @@ out:
return ret;
 }

-int ufs_qcom_phy_enable_ref_clk(struct phy *generic_phy)
+static int ufs_qcom_phy_enable_ref_clk(struct ufs_qcom_phy *phy)
 {
int ret = 0;
-   struct ufs_qcom_phy *phy = get_ufs_qcom_phy(generic_phy);

if (phy->is_ref_clk_enabled)
goto out;
@@ -423,7 +422,6 @@ out_disable_src:
 out:
return ret;
 }
-EXPORT_SYMBOL_GPL(ufs_qcom_phy_enable_ref_clk);

 static
 int ufs_qcom_phy_disable_vreg(struct device *dev,
@@ -448,10 +446,8 @@ out:
return ret;
 }

-void ufs_qcom_phy_disable_ref_clk(struct phy *generic_phy)
+static void ufs_qcom_phy_disable_ref_clk(struct ufs_qcom_phy *phy)
 {
-   struct ufs_qcom_phy *phy = get_ufs_qcom_phy(generic_phy);
-
if (phy->is_ref_clk_enabled) {
clk_disable_unprepare(phy->ref_clk);
/*
@@ -464,7 +460,6 @@ void ufs_qcom_phy_disable_ref_clk(struct phy 
*generic_phy)

phy->is_ref_clk_enabled = false;
}
 }
-EXPORT_SYMBOL_GPL(ufs_qcom_phy_disable_ref_clk);

 #define UFS_REF_CLK_EN (1 << 5)

@@ -517,9 +512,8 @@ void ufs_qcom_phy_disable_dev_ref_clk(struct phy
*generic_phy)
 EXPORT_SYMBOL_GPL(ufs_qcom_phy_disable_dev_ref_clk);

 /* Turn ON M-PHY RMMI interface clocks */
-int ufs_qcom_phy_enable_iface_clk(struct phy *generic_phy)
+static int ufs_qcom_phy_enable_iface_clk(struct ufs_qcom_phy *phy)
 {
-   struct ufs_qcom_phy *phy = get_ufs_qcom_phy(generic_phy);
int ret = 0;

if (phy->is_iface_clk_enabled)
@@ -543,20 +537,16 @@ int ufs_qcom_phy_enable_iface_clk(struct phy 
*generic_phy)

 out:
return ret;
 }
-EXPORT_SYMBOL_GPL(ufs_qcom_phy_enable_iface_clk);

 /* Turn OFF M-PHY RMMI interface clocks */
-void ufs_qcom_phy_disable_iface_clk(struct phy *generic_phy)
+void ufs_qcom_phy_disable_iface_clk(struct ufs_qcom_phy *phy)
 {
-   struct ufs_qcom_phy *phy = get_ufs_qcom_phy(generic_phy);
-
if (phy->is_iface_clk_enabled) {
clk_disable_unprepare(phy->tx_iface_clk);
clk_disable_unprepare(phy->rx_iface_clk);
phy->is_iface_clk_enabled = false;
}
 }
-EXPORT_SYMBOL_GPL(ufs_qcom_phy_disable_iface_clk);

 int ufs_qcom_phy_start_serdes(struct phy *generic_phy)
 {
@@ -674,13 +664,20 @@ int ufs_qcom_phy_power_on(struct phy 
*generic_phy)

goto out_disable_phy;
}

-   err = ufs_qcom_phy_enable_ref_clk(generic_phy);
+   err = ufs_qcom_phy_enable_iface_clk(phy_common);
if (err) {
-   dev_err(dev, "%s enable phy ref clock failed, err=%d\n",
+   dev_err(dev, "%s enable phy iface clock failed, err=%d\n",
__func__, err);
goto out_disable_pll;
}

+   err = ufs_qcom_phy_enable_ref_clk(phy_common);
+   if (err) {
+   dev_err(dev, "%s enable phy ref clock failed, err=%d\n",
+   __func__, err);
+   goto out_disable_iface_clk;
+   }
+
/* enable device PHY ref_clk pad rail */
if (phy_common->vddp_ref_clk.reg) {
err = ufs_qcom_phy_enable_vreg(dev,
@@ -696,7 +693,9 @@ int ufs_qcom_phy_power_on(struct phy *generic_phy)
goto out;

 out_disable_ref_clk:
-   ufs_qcom_phy_disable_ref_clk(generic_phy);
+   ufs_qcom_phy_disable_ref_clk(phy_common);
+out_disable_iface_clk:
+   ufs_qcom_phy_disable_iface_clk(phy_common);
 out_disable_pll:
ufs_qcom_phy_disable_vreg(dev, _common->vdda_pll);
 out_disable_phy:
@@ -715,7 +714,8 @@ int ufs_qcom_phy_power_off(struct phy *generic_phy)
if (phy_common->vddp_ref_clk.reg)
ufs_qcom_phy_disable_vreg(phy_common->dev,
  _common->vddp_ref_clk);
-   ufs_qcom_phy_disable_ref_clk(generic_phy);
+   ufs_qcom_phy_disable_ref_clk(phy_common);
+   ufs_qcom_phy_disable_iface_clk(phy_common);

ufs_qcom_phy_disable_vreg(phy_common->dev, _common->vdda_pll);
ufs_qcom_phy_disable_vreg(phy_common->dev, _common->vdda_phy);
diff --git a/drivers/scsi/ufs/ufs-qcom.c b/drivers/scsi/ufs/ufs-qcom.c
index 

Re: [PATCH 08/10] ufs-qcom: phy/hcd: Refactoring phy clock handling

[Subhash Jadavani]

On 2016-10-18 07:28, Vivek Gautam wrote:

Add phy clock enable code to phy_power_on/off callbacks, and
remove explicit calls to enable these phy clocks from the
ufs-qcom hcd driver.

Signed-off-by: Vivek Gautam 
---

Changes since v1:
 - staticized ufs_qcom_phy_enable(/disable)_ref_clk(),
 - staticized ufs_qcom_phy_enable(/disable)_iface_clk()
 - removed function declaration and export symbol for these APIs.

 drivers/phy/phy-qcom-ufs.c   | 36 
++--

 drivers/scsi/ufs/ufs-qcom.c  | 15 ---
 include/linux/phy/phy-qcom-ufs.h | 18 --
 3 files changed, 18 insertions(+), 51 deletions(-)

diff --git a/drivers/phy/phy-qcom-ufs.c b/drivers/phy/phy-qcom-ufs.c
index 144b11f..a425cc2 100644
--- a/drivers/phy/phy-qcom-ufs.c
+++ b/drivers/phy/phy-qcom-ufs.c
@@ -373,10 +373,9 @@ out:
return ret;
 }

-int ufs_qcom_phy_enable_ref_clk(struct phy *generic_phy)
+static int ufs_qcom_phy_enable_ref_clk(struct ufs_qcom_phy *phy)
 {
int ret = 0;
-   struct ufs_qcom_phy *phy = get_ufs_qcom_phy(generic_phy);

if (phy->is_ref_clk_enabled)
goto out;
@@ -423,7 +422,6 @@ out_disable_src:
 out:
return ret;
 }
-EXPORT_SYMBOL_GPL(ufs_qcom_phy_enable_ref_clk);

 static
 int ufs_qcom_phy_disable_vreg(struct device *dev,
@@ -448,10 +446,8 @@ out:
return ret;
 }

-void ufs_qcom_phy_disable_ref_clk(struct phy *generic_phy)
+static void ufs_qcom_phy_disable_ref_clk(struct ufs_qcom_phy *phy)
 {
-   struct ufs_qcom_phy *phy = get_ufs_qcom_phy(generic_phy);
-
if (phy->is_ref_clk_enabled) {
clk_disable_unprepare(phy->ref_clk);
/*
@@ -464,7 +460,6 @@ void ufs_qcom_phy_disable_ref_clk(struct phy 
*generic_phy)

phy->is_ref_clk_enabled = false;
}
 }
-EXPORT_SYMBOL_GPL(ufs_qcom_phy_disable_ref_clk);

 #define UFS_REF_CLK_EN (1 << 5)

@@ -517,9 +512,8 @@ void ufs_qcom_phy_disable_dev_ref_clk(struct phy
*generic_phy)
 EXPORT_SYMBOL_GPL(ufs_qcom_phy_disable_dev_ref_clk);

 /* Turn ON M-PHY RMMI interface clocks */
-int ufs_qcom_phy_enable_iface_clk(struct phy *generic_phy)
+static int ufs_qcom_phy_enable_iface_clk(struct ufs_qcom_phy *phy)
 {
-   struct ufs_qcom_phy *phy = get_ufs_qcom_phy(generic_phy);
int ret = 0;

if (phy->is_iface_clk_enabled)
@@ -543,20 +537,16 @@ int ufs_qcom_phy_enable_iface_clk(struct phy 
*generic_phy)

 out:
return ret;
 }
-EXPORT_SYMBOL_GPL(ufs_qcom_phy_enable_iface_clk);

 /* Turn OFF M-PHY RMMI interface clocks */
-void ufs_qcom_phy_disable_iface_clk(struct phy *generic_phy)
+void ufs_qcom_phy_disable_iface_clk(struct ufs_qcom_phy *phy)
 {
-   struct ufs_qcom_phy *phy = get_ufs_qcom_phy(generic_phy);
-
if (phy->is_iface_clk_enabled) {
clk_disable_unprepare(phy->tx_iface_clk);
clk_disable_unprepare(phy->rx_iface_clk);
phy->is_iface_clk_enabled = false;
}
 }
-EXPORT_SYMBOL_GPL(ufs_qcom_phy_disable_iface_clk);

 int ufs_qcom_phy_start_serdes(struct phy *generic_phy)
 {
@@ -674,13 +664,20 @@ int ufs_qcom_phy_power_on(struct phy 
*generic_phy)

goto out_disable_phy;
}

-   err = ufs_qcom_phy_enable_ref_clk(generic_phy);
+   err = ufs_qcom_phy_enable_iface_clk(phy_common);
if (err) {
-   dev_err(dev, "%s enable phy ref clock failed, err=%d\n",
+   dev_err(dev, "%s enable phy iface clock failed, err=%d\n",
__func__, err);
goto out_disable_pll;
}

+   err = ufs_qcom_phy_enable_ref_clk(phy_common);
+   if (err) {
+   dev_err(dev, "%s enable phy ref clock failed, err=%d\n",
+   __func__, err);
+   goto out_disable_iface_clk;
+   }
+
/* enable device PHY ref_clk pad rail */
if (phy_common->vddp_ref_clk.reg) {
err = ufs_qcom_phy_enable_vreg(dev,
@@ -696,7 +693,9 @@ int ufs_qcom_phy_power_on(struct phy *generic_phy)
goto out;

 out_disable_ref_clk:
-   ufs_qcom_phy_disable_ref_clk(generic_phy);
+   ufs_qcom_phy_disable_ref_clk(phy_common);
+out_disable_iface_clk:
+   ufs_qcom_phy_disable_iface_clk(phy_common);
 out_disable_pll:
ufs_qcom_phy_disable_vreg(dev, _common->vdda_pll);
 out_disable_phy:
@@ -715,7 +714,8 @@ int ufs_qcom_phy_power_off(struct phy *generic_phy)
if (phy_common->vddp_ref_clk.reg)
ufs_qcom_phy_disable_vreg(phy_common->dev,
  _common->vddp_ref_clk);
-   ufs_qcom_phy_disable_ref_clk(generic_phy);
+   ufs_qcom_phy_disable_ref_clk(phy_common);
+   ufs_qcom_phy_disable_iface_clk(phy_common);

ufs_qcom_phy_disable_vreg(phy_common->dev, _common->vdda_pll);
ufs_qcom_phy_disable_vreg(phy_common->dev, _common->vdda_phy);
diff --git a/drivers/scsi/ufs/ufs-qcom.c b/drivers/scsi/ufs/ufs-qcom.c
index 3aedf73..6e4ce5f 100644
---