Re: [RFC PATCH] clk: qcom: hfpll: return lock timeout from enable paths

[Konrad Dybcio]

On 6/29/26 5:14 PM, Antony Kurniawan Soemardi wrote:
> On 6/29/2026 4:15 PM, Konrad Dybcio wrote:
>> On 6/28/26 8:07 PM, Antony Kurniawan Soemardi wrote:
>>> On 6/24/2026 2:39 PM, Konrad Dybcio wrote:
>>>> On 6/24/26 3:57 AM, Antony Kurniawan Soemardi wrote:
>>>>> On 6/23/2026 4:43 PM, Konrad Dybcio wrote:
>>>>>> On 6/23/26 8:05 AM, Pengpeng Hou wrote:
>>>>>>> The HFPLL enable helper waits for the lock bit but ignores the
>>>>>>> regmap_read_poll_timeout() result. The polling condition is also
>>>>>>> inconsistent with clk_hfpll_init(), which treats the lock bit being set
>>>>>>> as the locked state.
>>>>>>>
>>>>>>> Wait for the lock bit to become set, return timeout errors from the
>>>>>>> helper, and propagate those errors through clk_hfpll_enable() and
>>>>>>> clk_hfpll_set_rate() instead of enabling the output unconditionally.
>>>>>>>
>>>>>>> Signed-off-by: Pengpeng Hou <pengpeng@xxxxxxxxxxx>
>>>>>>> ---
>>>>>>
>>>>>> This looks good on the surface..
>>>>>>
>>>>>> +Herman, Anthony, Dmitry could you please give this a spin on 8x60?
>>>>>>
>>>>>> Konrad
>>>>>
>>>>> Just to clarify, this patch impacts cpufreq and gpufreq for Qualcomm
>>>>> Krait era, is that correct?
>>>>
>>>> Seems that way - cpu, L2, and GPU, maybe others
>>>
>>> nope, tested on Sony Xperia SP (MSM8960T), the phone hangs
>>
>> [...]
>>
>>> [    2.679716] L2 @ Undefined rate. Forcing new rate
>>
>> This seems odd. What's the reported rate there?
> 
> if you're asking clk_get_rate(clks[l2_mux]), it's 0 Hz.

Hm, are the parents registered?

I see:

p_data[0].fw_name = hfpll_name; // "hfpll_l2" lookup via clock-names
p_data[0].name = hfpll_name; // legacy global clk lookup for
			     // clk.name == 'hfpll_l2'	
			     // (registered in gcc-msm8960.c)

p_data[1].hw = hfpll_div; // sourced from "hfpllN"
p_data[2].hw = sec_mux; // sourced from "acpuN_aux"

There's a fw_devlink between kraitcc and gcc already, since you
specify all the clocks (minus hfpll_l2):

https://lore.kernel.org/linux-arm-msm/20260514-expressatt_cpufreq-v1-3-487fd2d78859@xxxxxxxxx/

so it must have probed beforehand (unless you disable fw_devlink?)

Can you do `cat /sys/kernel/debug/clk/clk_summary` on a running system?

Konrad