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

[Palash Kambar <quic_pkambar@quicinc.com>]

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
>>
>