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

[Antony Kurniawan Soemardi]

On 7/6/2026 5:58 PM, Konrad Dybcio wrote:
> On 7/4/26 11:02 AM, Antony Kurniawan Soemardi wrote:
>
> [...]
>
> > > > > > > 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?
> >
> > pardon me for stupid question, but how do I verify the parent
> > registration?
>
> Parents are just other locks that happen to be specified in the
> parent_data/parent_names field of this L2 clock.
>
> For checking if they're registered at all, you can check if they're
> present in the debugfs summary I mentioned (they would also have their
> own directory in /sys/kernel/debug/clk). For checking if they're
> registered at some specific point in time, you could hack in a
> clk_get() and null-check it
>
> [...]
>
> > Do you have other idea how to solve this? I added some pr_err if the
> > read poll is timeout:
> >
> > [   11.164436] [pmOS-rd]:   ❬❬ PMOS STAGE 2 ❭❭
> > [   20.400232] krait_add_pri_mux: krait0_pri_mux, hfpll_name: hfpll0
> > [   20.400793] Enabling HFPLL hfpll1
> > [   20.455491] krait_add_pri_mux: krait1_pri_mux, hfpll_name: hfpll1
> > [   20.456081] Enabling HFPLL hfpll_l2
> > [   20.560623] HFPLL hfpll_l2 failed to lock (val=0x6, ret=-110, hd->lock_bit=0)
> > [   20.758961] krait_add_pri_mux: krait_l2_pri_mux, hfpll_name: hfpll_l2
>
> I noticed a bug (patch attached) in the enable function. I then realized
> the patch from the OP also fixes it.
>
> I then tried to validate that the code we have in Linux even checks the
> right register.. Unfortunately because this SoC is so old, it's difficult
> for me to find docs for it (and the register layout I have at hand isn't
> itself enough to deduce some things), so that's inconclusive.
>
> I then took to msm-3.x and lk sources, but they simply never check the
> value of the status register, they just call udelay(60) with presumably
> very high hopes and a lot of prayers..
>
> Can you check the state of the gcc_base+0x3420 register before and after
> the /* De-assert active-low PLL reset. */ line?

I assume gcc_base is regmap on clk-hfpll.c, I dumped 0x3400 to 0x3420:

[   21.089748] HFPLL hfpll_l2 mode_reg=0x3400 (before reset) regs:
[   21.089775]  3400:00000002
[   21.090300]  3404:7845c665
[   21.095944]  3408:00000000
[   21.098714]  340c:00000000
[   21.101404]  3410:00000001
[   21.104098]  3414:0108c000
[   21.106787]  3418:00000000
[   21.109478]  341c:00000000
[   21.112169]  3420:00000703
[   21.114858]
[   21.120243] HFPLL hfpll_l2 mode_reg=0x3400 (after reset) regs:
[   21.120262]  3400:00000006
[   21.121903]  3404:7845c665
[   21.127540]  3408:00000000
[   21.130228]  340c:00000000
[   21.132916]  3410:00000001
[   21.135605]  3414:0108c000
[   21.138299]  3418:00000000
[   21.140993]  341c:00000006
[   21.143681]  3420:00000703
[   21.146367]
[   21.251760] HFPLL hfpll_l2 failed to lock (val=0x6, ret=-110, 
hd->lock_bit=0)
[   21.271774] L2 @ Undefined rate. Forcing new rate.

--
Thanks,
Antony K. S.