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

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

[Pengpeng Hou]

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@iscas.ac.cn>
---
 drivers/clk/qcom/clk-hfpll.c | 27 +++++++++++++++++----------
 1 file changed, 17 insertions(+), 10 deletions(-)

diff --git a/drivers/clk/qcom/clk-hfpll.c b/drivers/clk/qcom/clk-hfpll.c
index 705352aff067..e2059682e440 100644
--- a/drivers/clk/qcom/clk-hfpll.c
+++ b/drivers/clk/qcom/clk-hfpll.c
@@ -54,12 +54,13 @@ static void __clk_hfpll_init_once(struct clk_hw *hw)
 	h->init_done = true;
 }
 
-static void __clk_hfpll_enable(struct clk_hw *hw)
+static int __clk_hfpll_enable(struct clk_hw *hw)
 {
 	struct clk_hfpll *h = to_clk_hfpll(hw);
 	struct hfpll_data const *hd = h->d;
 	struct regmap *regmap = h->clkr.regmap;
 	u32 val;
+	int ret;
 
 	__clk_hfpll_init_once(hw);
 
@@ -76,19 +77,23 @@ static void __clk_hfpll_enable(struct clk_hw *hw)
 	regmap_update_bits(regmap, hd->mode_reg, PLL_RESET_N, PLL_RESET_N);
 
 	/* Wait for PLL to lock. */
-	if (hd->status_reg)
+	if (hd->status_reg) {
 		/*
 		 * Busy wait. Should never timeout, we add a timeout to
 		 * prevent any sort of stall.
 		 */
-		regmap_read_poll_timeout(regmap, hd->status_reg, val,
-					 !(val & BIT(hd->lock_bit)), 0,
-					 100 * USEC_PER_MSEC);
-	else
+		ret = regmap_read_poll_timeout(regmap, hd->status_reg, val,
+					       val & BIT(hd->lock_bit), 0,
+					       100 * USEC_PER_MSEC);
+		if (ret)
+			return ret;
+	} else {
 		udelay(60);
+	}
 
 	/* Enable PLL output. */
 	regmap_update_bits(regmap, hd->mode_reg, PLL_OUTCTRL, PLL_OUTCTRL);
+	return 0;
 }
 
 /* Enable an already-configured HFPLL. */
@@ -99,14 +104,15 @@ static int clk_hfpll_enable(struct clk_hw *hw)
 	struct hfpll_data const *hd = h->d;
 	struct regmap *regmap = h->clkr.regmap;
 	u32 mode;
+	int ret = 0;
 
 	spin_lock_irqsave(&h->lock, flags);
 	regmap_read(regmap, hd->mode_reg, &mode);
 	if (!(mode & (PLL_BYPASSNL | PLL_RESET_N | PLL_OUTCTRL)))
-		__clk_hfpll_enable(hw);
+		ret = __clk_hfpll_enable(hw);
 	spin_unlock_irqrestore(&h->lock, flags);
 
-	return 0;
+	return ret;
 }
 
 static void __clk_hfpll_disable(struct clk_hfpll *h)
@@ -161,6 +167,7 @@ static int clk_hfpll_set_rate(struct clk_hw *hw, unsigned long rate,
 	unsigned long flags;
 	u32 l_val, val;
 	bool enabled;
+	int ret = 0;
 
 	l_val = rate / parent_rate;
 
@@ -183,11 +190,11 @@ static int clk_hfpll_set_rate(struct clk_hw *hw, unsigned long rate,
 	regmap_write(regmap, hd->l_reg, l_val);
 
 	if (enabled)
-		__clk_hfpll_enable(hw);
+		ret = __clk_hfpll_enable(hw);
 
 	spin_unlock_irqrestore(&h->lock, flags);
 
-	return 0;
+	return ret;
 }
 
 static unsigned long clk_hfpll_recalc_rate(struct clk_hw *hw,
-- 
2.50.1 (Apple Git-155)

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

[Konrad Dybcio]

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@iscas.ac.cn>
> ---

This looks good on the surface.. 

+Herman, Anthony, Dmitry could you please give this a spin on 8x60?

Konrad

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

[Antony Kurniawan Soemardi]

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@iscas.ac.cn>
>> ---
> 
> 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?

I could try this over the weekend on top of Rudraksha Gupta's cpufreq
patch for msm8960 [1]. We don't have a gpu patch for msm8960 on LKML
yet...

(also my name doesn't have an "h" :D )

[1] 
https://lore.kernel.org/all/20260527-expressatt_cpufreq-v2-0-b9b7726ccb6d@gmail.com/

-- 
Thanks,
Antony K. S.

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

[Konrad Dybcio]

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@iscas.ac.cn>
>>> ---
>>
>> 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

> I could try this over the weekend on top of Rudraksha Gupta's cpufreq
> patch for msm8960 [1]. We don't have a gpu patch for msm8960 on LKML
> yet...

That's fine

> (also my name doesn't have an "h" :D )

My mistake!

Konrad

> 
> [1] https://lore.kernel.org/all/20260527-expressatt_cpufreq-v2-0-b9b7726ccb6d@gmail.com/
> 

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

[Antony Kurniawan Soemardi]

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@iscas.ac.cn>
>>>> ---
>>>
>>> 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

[    0.000000] Booting Linux on physical CPU 0x0
[    0.000000] Linux version 7.1.0 (pmos@03766073382c) (Alpine clang 
version 22.1.8, LLD 22.1.8) #10 SMP Sun Jun 28 17:50:44 UTC 2026
[    0.000000] CPU: ARMv7 Processor [512f04d0] revision 0 (ARMv7), 
cr=10c5787d
[    0.000000] CPU: div instructions available: patching division code
[    0.000000] CPU: PIPT / VIPT nonaliasing data cache, PIPT instruction 
cache
[    0.000000] OF: fdt: Machine model: Sony Xperia SP
[    0.000000] random: crng init done
[    0.000000] earlycon: msm_serial_dm0 at MMIO 0x1a040000 (options 
'115200n8')
[    0.000000] printk: legacy bootconsole [msm_serial_dm0] enabled
[    0.000000] Memory policy: Data cache writealloc
[    0.000000] efi: UEFI not found.
[    0.000000] cma: Reserved 64 MiB at 0xbc000000
[    0.000000] OF: reserved mem: Reserved memory: No reserved-memory 
node in the DT
[    0.000000] Zone ranges:
[    0.000000]   DMA      [mem 0x0000000080200000-0x00000000b01fffff]
[    0.000000]   Normal   empty
[    0.000000]   HighMem  [mem 0x00000000b0200000-0x00000000bfffffff]
[    0.000000] Movable zone start for each node
[    0.000000] Early memory node ranges
[    0.000000]   node   0: [mem 0x0000000080200000-0x00000000bfffffff]
[    0.000000] Initmem setup node 0 [mem 
0x0000000080200000-0x00000000bfffffff]
[    0.000000] On node 0, zone DMA: 512 pages in unavailable ranges
[    0.000000] percpu: Embedded 20 pages/cpu s49932 r8192 d23796 u81920
[    0.000000] Kernel command line: console=ttyMSM0 earlycon earlyprintk 
fw_devlink=permissive 
pmos_boot_uuid=af65c02d-2fac-4fb9-ac5c-342f6ee9acdf pmos_root_uuid=02s
[    0.000000] Unknown kernel command line parameters "earlyprintk 
pmos_boot_uuid=af65c02d-2fac-4fb9-ac5c-342f6ee9acdf 
pmos_root_uuid=02b642f1-ec7b-4d1c-8212-ebdce18.
[    0.000000] printk: log buffer data + meta data: 131072 + 409600 = 
540672 bytes
[    0.000000] Dentry cache hash table entries: 131072 (order: 7, 524288 
bytes, linear)
[    0.000000] Inode-cache hash table entries: 65536 (order: 6, 262144 
bytes, linear)
[    0.000000] Built 1 zonelists, mobility grouping on.  Total pages: 261632
[    0.000000] mem auto-init: stack:all(zero), heap alloc:off, heap free:off
[    0.000000] SLUB: HWalign=64, Order=0-3, MinObjects=0, CPUs=2, Nodes=1
[    0.000000] ftrace: allocating 71217 entries in 209 pages
[    0.000000] ftrace: allocated 209 pages with 4 groups
[    0.000000] rcu: Hierarchical RCU implementation.
[    0.000000] rcu:     RCU event tracing is enabled.
[    0.000000] rcu:     RCU restricting CPUs from NR_CPUS=16 to 
nr_cpu_ids=2.
[    0.000000]  Rude variant of Tasks RCU enabled.
[    0.000000]  Tracing variant of Tasks RCU enabled.
[    0.000000] rcu: RCU calculated value of scheduler-enlistment delay 
is 10 jiffies.
[    0.000000] rcu: Adjusting geometry for rcu_fanout_leaf=16, nr_cpu_ids=2
[    0.000000] RCU Tasks Rude: Setting shift to 1 and lim to 1 
rcu_task_cb_adjust=1 rcu_task_cpu_ids=2.
[    0.000000] NR_IRQS: 16, nr_irqs: 16, preallocated irqs: 16
[    0.000000] rcu: srcu_init: Setting srcu_struct sizes based on 
contention.
[    0.000000] clocksource: jiffies: mask: 0xffffffff max_cycles: 
0xffffffff, max_idle_ns: 19112604462750000 ns
[    0.000000] clocksource: dg_timer: mask: 0xffffffff max_cycles: 
0xffffffff, max_idle_ns: 283149695806 ns
[    0.000003] sched_clock: 32 bits at 7MHz, resolution 148ns, wraps 
every 318145725365ns
[    0.008509] Switching to timer-based delay loop, resolution 148ns
[    0.021062] Console: colour dummy device 80x30
[    0.022498] Calibrating delay loop (skipped), value calculated using 
timer frequency.. 13.50 BogoMIPS (lpj=67500)
[    0.026739] CPU: Testing write buffer coherency: ok
[    0.037149] pid_max: default: 32768 minimum: 301
[    0.043671] LSM support for eBPF active
[    0.046794] Mount-cache hash table entries: 2048 (order: 1, 8192 
bytes, linear)
[    0.050243] Mountpoint-cache hash table entries: 2048 (order: 1, 8192 
bytes, linear)
[    0.058084] VFS: Finished mounting rootfs on nullfs
[    0.067344] CPU0: thread -1, cpu 0, socket 0, mpidr 80000000
[    0.070197] qcom_scm: convention: smc legacy
[    0.079085] Setting up static identity map for 0x80500000 - 0x805000b0
[    0.083075] rcu: Hierarchical SRCU implementation.
[    0.086652] rcu:     Max phase no-delay instances is 1000.
[    0.091980] Timer migration: 1 hierarchy levels; 8 children per 
group; 1 crossnode level
[    0.107264] EFI services will not be available.
[    0.107803] smp: Bringing up secondary CPUs ...
[    0.113189] CPU1: thread -1, cpu 1, socket 0, mpidr 80000001
[    0.114181] smp: Brought up 1 node, 2 CPUs
[    0.121166] SMP: Total of 2 processors activated (27.00 BogoMIPS).
[    0.124946] CPU: All CPU(s) started in SVC mode.
[    0.131645] Memory: 914204K/1046528K available (24576K kernel code, 
3059K rwdata, 8308K rodata, 3072K init, 461K bss, 62808K reserved, 
65536K cma-reserved, 194560)
[    0.137540] devtmpfs: initialized
[    0.176746] VFP support v0.3: implementor 51 architecture 64 part 4d 
variant 2 rev 0
[    0.177699] posixtimers hash table entries: 1024 (order: 1, 8192 
bytes, linear)
[    0.183800] futex hash table entries: 512 (32768 bytes on 1 NUMA 
nodes, total 32 KiB, linear).
[    0.210792] DMI: not present or invalid.
[    0.212311] NET: Registered PF_NETLINK/PF_ROUTE protocol family
[    0.216112] DMA: preallocated 256 KiB pool for atomic coherent 
allocations
[    0.234714] thermal_sys: Registered thermal governor 'step_wise'
[    0.234948] cpuidle: using governor menu
[    0.241913] No ATAGs?
[    0.244049] hw-breakpoint: Failed to enable monitor mode on CPU 0.
[    0.262963] Serial: AMBA PL011 UART driver
[    0.285450] /soc/clock-controller@900000: Fixed dependency cycle(s) 
with /soc/clock-controller@28000000
[    0.286260] /soc/usb@12500000: Fixed dependency cycle(s) with 
/soc/usb@12500000/ulpi/phy
[    0.294023] /soc/usb@12500000/ulpi/phy: Fixed dependency cycle(s) 
with /soc/usb@12500000
[    0.302327] /soc/clock-controller@28000000: Fixed dependency cycle(s) 
with /soc/clock-controller@900000
[    0.329630] /soc/clock-controller@900000: Fixed dependency cycle(s) 
with /soc/clock-controller@28000000
[    0.351469] /soc/usb@12500000: Fixed dependency cycle(s) with 
/soc/usb@12500000/ulpi/phy
[    0.351769] /soc/usb@12500000/ulpi/phy: Fixed dependency cycle(s) 
with /soc/usb@12500000
[    0.369289] kprobes: kprobe jump-optimization is enabled. All kprobes 
are optimized if possible.
[    0.394747] raid6: using neon recovery algorithm
[    0.561191] raid6: int32x1  gen()    92 MB/s
[    0.733047] raid6: int32x2  gen()    99 MB/s
[    0.904827] raid6: int32x4  gen()   162 MB/s
[    1.076823] raid6: int32x8  gen()   205 MB/s
[    1.248860] raid6: neonx1   gen()   785 MB/s
[    1.420782] raid6: neonx2   gen()  1047 MB/s
[    1.592689] raid6: neonx4   gen()   907 MB/s
[    1.764529] raid6: neonx8   gen()   934 MB/s
[    1.764570] raid6: using algorithm neonx2 gen() 1047 MB/s
[    1.936416] raid6: .... xor() 630 MB/s, rmw enabled
[    1.954284] iommu: Default domain type: Translated
[    1.954336] iommu: DMA domain TLB invalidation policy: strict mode
[    1.960850] SCSI subsystem initialized
[    1.969167] usbcore: registered new interface driver usbfs
[    1.969334] usbcore: registered new interface driver hub
[    1.973637] usbcore: registered new device driver usb
[    1.983264] pps_core: LinuxPPS API ver. 1 registered
[    1.983977] pps_core: Software ver. 5.3.6 - Copyright 2005-2007 
Rodolfo Giometti <giometti@linux.it>
[    1.989049] PTP clock support registered
[    1.998685] EDAC MC: Ver: 3.0.0
[    2.002983] scmi_core: SCMI protocol bus registered
[    2.010594] vgaarb: loaded
[    2.017546] clocksource: Switched to clocksource dg_timer
[    2.048670] NET: Registered PF_INET protocol family
[    2.048993] IP idents hash table entries: 16384 (order: 5, 131072 
bytes, linear)
[    2.054734] tcp_listen_portaddr_hash hash table entries: 512 (order: 
0, 4096 bytes, linear)
[    2.060149] Table-perturb hash table entries: 65536 (order: 6, 262144 
bytes, linear)
[    2.068274] TCP established hash table entries: 8192 (order: 3, 32768 
bytes, linear)
[    2.076241] TCP bind hash table entries: 8192 (order: 5, 131072 
bytes, linear)
[    2.084002] TCP: Hash tables configured (established 8192 bind 8192)
[    2.091130] UDP hash table entries: 512 (order: 3, 28672 bytes, linear)
[    2.097811] NET: Registered PF_UNIX/PF_LOCAL protocol family
[    2.105019] RPC: Registered named UNIX socket transport module.
[    2.109698] RPC: Registered udp transport module.
[    2.115225] RPC: Registered tcp transport module.
[    2.120187] RPC: Registered tcp-with-tls transport module.
[    2.124774] RPC: Registered tcp NFSv4.1 backchannel transport module.
[    2.130267] PCI: CLS 0 bytes, default 64
[    2.140192] Initialise system trusted keyrings
[    2.147886] Trying to unpack rootfs image as initramfs...
[    2.171410] workingset: timestamp_bits=14 (anon: 9) max_order=18 
bucket_order=4 (anon: 9)
[    2.195378] NFS: Registering the id_resolver key type
[    2.195508] Key type id_resolver registered
[    2.214561] Key type id_legacy registered
[    2.214759] nfs4filelayout_init: NFSv4 File Layout Driver Registering...
[    2.217858] nfs4flexfilelayout_init: NFSv4 Flexfile Layout Driver 
Registering...
[    2.227868] Key type asymmetric registered
[    2.231752] Asymmetric key parser 'x509' registered
[    2.235860] Block layer SCSI generic (bsg) driver version 0.4 loaded 
(major 245)
[    2.268473] io scheduler mq-deadline registered
[    2.268570] io scheduler kyber registered
[    2.272081] io scheduler bfq registered
[    2.276199] xor: measuring software checksum speed
[    2.282001]    neon            :  1506 MB/sec
[    2.290889]    32regs          :   517 MB/sec
[    2.297519]    8regs           :   499 MB/sec
[    2.301990]    arm4regs        :   744 MB/sec
[    2.302054] xor: using function: neon (1506 MB/sec)
[    2.543901] ledtrig-cpu: registered to indicate activity on CPUs
[    2.679716] L2 @ Undefined rate. Forcing new rate

-- 
Thanks,
Antony K. S.

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

[Konrad Dybcio]

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@iscas.ac.cn>
>>>>> ---
>>>>
>>>> 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?

Konrad

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

[Antony Kurniawan Soemardi]

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@iscas.ac.cn>
>>>>>> ---
>>>>>
>>>>> 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.

without Pengpeng Hou's patch, the kernel continues with the
following logs:

[    2.442850] ledtrig-cpu: registered to indicate activity on CPUs
[    2.516664] L2 @ Undefined rate. Forcing new rate.
[    2.516870] L2 @ 391500 KHz
[    2.527751] CPU0 @ 918000 KHz
[    2.527813] CPU1 @ Undefined rate. Forcing new rate.
[    2.529921] CPU1 @ 391500 KHz
[    2.613351] gsbi 1a000000.gsbi: GSBI port protocol: 6 crci: 0

-- 
Thanks,
Antony K. S.

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@iscas.ac.cn>
>>>>>>> ---
>>>>>>
>>>>>> 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@gmail.com/

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

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

[Antony Kurniawan Soemardi]

On 7/1/2026 7:11 PM, Konrad Dybcio wrote:
> 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@iscas.ac.cn>
>>>>>>>> ---
>>>>>>>
>>>>>>> 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?

pardon me for stupid question, but how do I verify the parent
registration?

> 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@gmail.com/
> 
> so it must have probed beforehand (unless you disable fw_devlink?)

I removed the fw_devlink kernel arg, the kraitcc is probed much later,
but still stuck with the same "L2 @ undefined rate"
> Can you do `cat /sys/kernel/debug/clk/clk_summary` on a running system?

below is the mainline kernel, with cpufreq patch

sony-huashan:/home/user# cat /sys/kernel/debug/clk/clk_summary
                                  enable  prepare  protect 
                  duty  hardware                            connection
    clock                          count    count    count        rate 
accuracy phase  cycle    enable   consumer                         id
---------------------------------------------------------------------------------------------------------------------------------------------
  qsb                                 0       0        0        1 
    0          0     50000      Y   deviceless 
no_connection_id
[...]
  pxo_board                           4       4        1        27000000 
    0          0     50000      Y   deviceless 
no_connection_id
[...]
     hfpll_l2                         1       1        0 
783000000   0          0     50000      Y      deviceless 
       no_connection_id
        hfpll_l2_div                  3       3        0 
391500000   0          0     50000      Y         deviceless 
          no_connection_id
           krait_l2_pri_mux           2       2        0 
391500000   0          0     50000      Y            deviceless 
             no_connection_id
     hfpll1                           1       1        0 
783000000   0          0     50000      Y      deviceless 
       no_connection_id
        hfpll1_div                    2       2        0 
391500000   0          0     50000      Y         deviceless 
          no_connection_id
           krait1_pri_mux             1       1        0 
391500000   0          0     50000      Y            cpu1 
             no_connection_id
  
                                             cpu1 
     no_connection_id
  
                                             deviceless 
     no_connection_id
     hfpll0                           2       2        0 
1512000000  0          0     50000      Y      deviceless 
       no_connection_id
        krait0_pri_mux                1       1        0 
1512000000  0          0     50000      Y         cpu0 
          no_connection_id
  
                                          cpu0 
  no_connection_id
  
                                          deviceless 
  no_connection_id
        hfpll0_div                    1       1        0 
756000000   0          0     50000      Y         deviceless 
          no_connection_id
[...]
  krait_l2_sec_mux                    2       2        0        0 
    0          0     50000      Y   deviceless 
no_connection_id
  krait1_sec_mux                      1       1        0        0 
    0          0     50000      Y   deviceless 
no_connection_id
  krait0_sec_mux                      1       1        0        0 
    0          0     50000      Y   deviceless 
no_connection_id
  pll4_vote                           0       0        0        0 
    0          0     50000      ?   deviceless 
no_connection_id

below is the mainline kernel, without cpufreq patch

sony-huashan:/home/user# cat /sys/kernel/debug/clk/clk_summary
                                  enable  prepare  protect 
                  duty  hardware                            connection
    clock                          count    count    count        rate 
accuracy phase  cycle    enable   consumer                         id
---------------------------------------------------------------------------------------------------------------------------------------------
[...]
  pxo_board                           1       1        1        27000000 
    0          0     50000      Y   deviceless 
no_connection_id
[...]
     hfpll_l2                         0       0        0        0 
    0          0     50000      N      deviceless 
no_connection_id
     hfpll1                           0       0        0 
918000000   0          0     50000      N      deviceless 
       no_connection_id
     hfpll0                           0       0        0 
918000000   0          0     50000      Y      deviceless 
       no_connection_id
[...]
  pll4_vote                           0       0        0        0 
    0          0     50000      ?   deviceless 
no_connection_id


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
[   20.759339] L2 @ 0 KHz. Forcing new rate.
[   20.764520] L2 @ Undefined rate. Forcing new rate.

-- 
Thanks,
Antony K. S.

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

[Konrad Dybcio]

[-- Attachment #1: Type: text/plain, Size: 2187 bytes --]

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?

Konrad

[-- Attachment #2: 0001-clk-qcom-clk-hfpll-Fix-inverted-break-condition-in-..patch --]
[-- Type: text/x-patch, Size: 1299 bytes --]

From 5c65b279afcc53a7afbd63d743e056ca00b299a0 Mon Sep 17 00:00:00 2001
From: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
Date: Mon, 6 Jul 2026 12:46:24 +0200
Subject: [PATCH] clk: qcom: clk-hfpll: Fix inverted break condition in .enable

The commit referenced in the Fixes tag did away with an open-coded
while-loop, replacing it with a nicer wrapper.

The syntax of that wrapper however, includes a 'break condition',
contrary to the while-loop which expects a 'keep going condition'.

As a result, regmap_read_poll_timeout() is looking for the opposite of
the desired condition. Fix that.

Fixes: fcfbfe373d41 ("clk: qcom: clk-hfpll: use poll_timeout macro")
Signed-off-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
---
 drivers/clk/qcom/clk-hfpll.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/clk/qcom/clk-hfpll.c b/drivers/clk/qcom/clk-hfpll.c
index 705352aff067..62fbf511003e 100644
--- a/drivers/clk/qcom/clk-hfpll.c
+++ b/drivers/clk/qcom/clk-hfpll.c
@@ -82,7 +82,7 @@ static void __clk_hfpll_enable(struct clk_hw *hw)
 		 * prevent any sort of stall.
 		 */
 		regmap_read_poll_timeout(regmap, hd->status_reg, val,
-					 !(val & BIT(hd->lock_bit)), 0,
+					 val & BIT(hd->lock_bit), 0,
 					 100 * USEC_PER_MSEC);
 	else
 		udelay(60);
-- 
2.54.0

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.

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

[Konrad Dybcio]

On 7/8/26 6:04 PM, Antony Kurniawan Soemardi wrote:
> On 7/6/2026 5:58 PM, Konrad Dybcio wrote:
>> On 7/4/26 11:02 AM, Antony Kurniawan Soemardi wrote:

[...]

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

Okay, can you try dumping the same register from a running device on
downstream? The only change in values that we're seeing is BIT(2)
being set in 0x3400 (which means "set the RESET_N bit", a.k.a.
"request un-stopping the PLL")

Konrad