[v1] ufs: ufs-qcom: disable lane clocks during phy hibern8

[PATCH] ufs: ufs-qcom: disable lane clocks during phy hibern8

Posted by Palash Kambar 2 months, 3 weeks ago

The UFS lane clocks ensure that the PHY is adequately powered and
synchronized before initiating the link. Currently, these clocks
remain enabled even after the link enters the Hibern8 state and
are only turned off during runtime or system suspend.

Modify the behavior to disable the lane clocks immediately after
the link transitions to Hibern8, thereby reducing the power
consumption.

Signed-off-by: Palash Kambar <quic_pkambar@quicinc.com>
---
 drivers/ufs/host/ufs-qcom.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/drivers/ufs/host/ufs-qcom.c b/drivers/ufs/host/ufs-qcom.c
index 318dca7fe3d7..50e174d9b406 100644
--- a/drivers/ufs/host/ufs-qcom.c
+++ b/drivers/ufs/host/ufs-qcom.c
@@ -1141,6 +1141,13 @@ static int ufs_qcom_setup_clocks(struct ufs_hba *hba, bool on,
 	case PRE_CHANGE:
 		if (on) {
 			ufs_qcom_icc_update_bw(host);
+			if (ufs_qcom_is_link_hibern8(hba)) {
+				err = ufs_qcom_enable_lane_clks(host);
+				if (err) {
+					dev_err(hba->dev, "enable lane clks failed, ret=%d\n", err);
+					return err;
+				}
+			}
 		} else {
 			if (!ufs_qcom_is_link_active(hba)) {
 				/* disable device ref_clk */
@@ -1166,6 +1173,9 @@ static int ufs_qcom_setup_clocks(struct ufs_hba *hba, bool on,
 			if (ufshcd_is_hs_mode(&hba->pwr_info))
 				ufs_qcom_dev_ref_clk_ctrl(host, true);
 		} else {
+			if (ufs_qcom_is_link_hibern8(hba))
+				ufs_qcom_disable_lane_clks(host);
+
 			ufs_qcom_icc_set_bw(host, ufs_qcom_bw_table[MODE_MIN][0][0].mem_bw,
 					    ufs_qcom_bw_table[MODE_MIN][0][0].cfg_bw);
 		}
-- 
2.34.1

Re: [PATCH] ufs: ufs-qcom: disable lane clocks during phy hibern8

Posted by Manivannan Sadhasivam 1 month, 3 weeks ago

On Tue, Jul 15, 2025 at 09:35:24PM GMT, Palash Kambar wrote:
> The UFS lane clocks ensure that the PHY is adequately powered and
> synchronized before initiating the link. Currently, these clocks
> remain enabled even after the link enters the Hibern8 state and
> are only turned off during runtime or system suspend.
> 
> Modify the behavior to disable the lane clocks immediately after
> the link transitions to Hibern8, thereby reducing the power
> consumption.
> 

This statement is technically misleading. You are disabling lane clocks in
ufs_qcom_setup_clocks(), which gets called during suspend/resume/clk_gate phase.

But if you want to disable the clocks immediately after Hibern8, you may want to
add the disable/enable logic in hibern8_notify() callback.

If that is not a strict requirement and you want to do it in
ufs_qcom_setup_clocks(), you have to reword the description.

- Mani

> Signed-off-by: Palash Kambar <quic_pkambar@quicinc.com>
> ---
>  drivers/ufs/host/ufs-qcom.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/drivers/ufs/host/ufs-qcom.c b/drivers/ufs/host/ufs-qcom.c
> index 318dca7fe3d7..50e174d9b406 100644
> --- a/drivers/ufs/host/ufs-qcom.c
> +++ b/drivers/ufs/host/ufs-qcom.c
> @@ -1141,6 +1141,13 @@ static int ufs_qcom_setup_clocks(struct ufs_hba *hba, bool on,
>  	case PRE_CHANGE:
>  		if (on) {
>  			ufs_qcom_icc_update_bw(host);
> +			if (ufs_qcom_is_link_hibern8(hba)) {
> +				err = ufs_qcom_enable_lane_clks(host);
> +				if (err) {
> +					dev_err(hba->dev, "enable lane clks failed, ret=%d\n", err);
> +					return err;
> +				}
> +			}
>  		} else {
>  			if (!ufs_qcom_is_link_active(hba)) {
>  				/* disable device ref_clk */
> @@ -1166,6 +1173,9 @@ static int ufs_qcom_setup_clocks(struct ufs_hba *hba, bool on,
>  			if (ufshcd_is_hs_mode(&hba->pwr_info))
>  				ufs_qcom_dev_ref_clk_ctrl(host, true);
>  		} else {
> +			if (ufs_qcom_is_link_hibern8(hba))
> +				ufs_qcom_disable_lane_clks(host);
> +
>  			ufs_qcom_icc_set_bw(host, ufs_qcom_bw_table[MODE_MIN][0][0].mem_bw,
>  					    ufs_qcom_bw_table[MODE_MIN][0][0].cfg_bw);
>  		}
> -- 
> 2.34.1
> 

-- 
மணிவண்ணன் சதாசிவம்

Re: [PATCH] ufs: ufs-qcom: disable lane clocks during phy hibern8

Posted by Palash Kambar 1 month, 3 weeks ago


On 8/13/2025 3:49 PM, Manivannan Sadhasivam wrote:
> On Tue, Jul 15, 2025 at 09:35:24PM GMT, Palash Kambar wrote:
>> The UFS lane clocks ensure that the PHY is adequately powered and
>> synchronized before initiating the link. Currently, these clocks
>> remain enabled even after the link enters the Hibern8 state and
>> are only turned off during runtime or system suspend.
>>
>> Modify the behavior to disable the lane clocks immediately after
>> the link transitions to Hibern8, thereby reducing the power
>> consumption.
>>
> 
> This statement is technically misleading. You are disabling lane clocks in
> ufs_qcom_setup_clocks(), which gets called during suspend/resume/clk_gate phase.
> 
> But if you want to disable the clocks immediately after Hibern8, you may want to
> add the disable/enable logic in hibern8_notify() callback.
> 
> If that is not a strict requirement and you want to do it in
> ufs_qcom_setup_clocks(), you have to reword the description.
> 

Hi Mani,
Hibern8 entry and exit can be initiated from various contexts, including
clock scaling. Given this, it may not be ideal to toggle the lane clock on
every Hibern8 transition. Moreover, since all resources related to the PHY
and controller are managed within ufs_qcom_setup_clocks(), it seems more
appropriate to handle this logic within that function. Additionally, since
ufs_qcom_setup_clock() is invoked immediately after the link enters
Hibern8 via gate work, any delay appears to be minimal.

I’ll ensure these points are clearly reflected in the commit message.

-Palash K

> 
>> Signed-off-by: Palash Kambar <quic_pkambar@quicinc.com>
>> ---
>>  drivers/ufs/host/ufs-qcom.c | 10 ++++++++++
>>  1 file changed, 10 insertions(+)
>>
>> diff --git a/drivers/ufs/host/ufs-qcom.c b/drivers/ufs/host/ufs-qcom.c
>> index 318dca7fe3d7..50e174d9b406 100644
>> --- a/drivers/ufs/host/ufs-qcom.c
>> +++ b/drivers/ufs/host/ufs-qcom.c
>> @@ -1141,6 +1141,13 @@ static int ufs_qcom_setup_clocks(struct ufs_hba *hba, bool on,
>>  	case PRE_CHANGE:
>>  		if (on) {
>>  			ufs_qcom_icc_update_bw(host);
>> +			if (ufs_qcom_is_link_hibern8(hba)) {
>> +				err = ufs_qcom_enable_lane_clks(host);
>> +				if (err) {
>> +					dev_err(hba->dev, "enable lane clks failed, ret=%d\n", err);
>> +					return err;
>> +				}
>> +			}
>>  		} else {
>>  			if (!ufs_qcom_is_link_active(hba)) {
>>  				/* disable device ref_clk */
>> @@ -1166,6 +1173,9 @@ static int ufs_qcom_setup_clocks(struct ufs_hba *hba, bool on,
>>  			if (ufshcd_is_hs_mode(&hba->pwr_info))
>>  				ufs_qcom_dev_ref_clk_ctrl(host, true);
>>  		} else {
>> +			if (ufs_qcom_is_link_hibern8(hba))
>> +				ufs_qcom_disable_lane_clks(host);
>> +
>>  			ufs_qcom_icc_set_bw(host, ufs_qcom_bw_table[MODE_MIN][0][0].mem_bw,
>>  					    ufs_qcom_bw_table[MODE_MIN][0][0].cfg_bw);
>>  		}
>> -- 
>> 2.34.1
>>
>