[PATCH] clk: qcom: ipq-cmn-pll: keep the CMN block bus clocks enabled

[PATCH] clk: qcom: ipq-cmn-pll: keep the CMN block bus clocks enabled

[Stanislaw Pal]

The probe function takes a runtime PM reference to enable the GCC AHB &
SYS clocks of the CMN PLL block, registers the clocks, and then drops
the reference. Once the autosuspend kicks in, pm_clk gates both clocks
a few milliseconds after probe has returned.

That is wrong on two counts:

  - The clock ops access the CMN PLL registers without taking a runtime
    PM reference of their own, so any later clk_set_rate() or rate
    recalculation touches the register file with its bus clock gated.

  - On IPQ5018 the consequences are not contained to this device:
    gating the CMN block bus clocks makes the SoC hang on a subsequent
    bus access. Measured on a TP-Link Archer AX55 v1 (IPQ5018), the
    machine dies silently within a few milliseconds of the CMN PLL
    probe returning - mid-character on the UART - and the victim is
    whichever device happens to probe next. Whether a given boot
    survives the window is a micro-timing lottery: different binary
    layouts of the same kernel ranged from occasional failures to a
    100% reproducible boot loop.

Keep the runtime PM usage count elevated on the probe success path, so
the bus clocks stay enabled. With this one change the same board went
from boot-looping to surviving every boot attempt (verified across
three previously-failing kernel binaries, plus repeated soft and hard
resets on the final one).

Fixes: f81715a4c87c ("clk: qcom: Add CMN PLL clock controller driver for IPQ SoC")
Cc: stable@vger.kernel.org
Signed-off-by: Stanislaw Pal <kuncy7@gmail.com>
---
One more observation from the same debugging session, for the people
who know this silicon: on IPQ5018 the CMN_PLL_LOCKED bit (offset 0x64,
bit 8) never asserts. Every clk_cmn_pll_set_rate() call runs its
regmap_read_poll_timeout() to the full 100 ms timeout - including under
the bootloader-programmed configuration the hardware demonstrably runs
fine with - and nobody notices, because clk_change_rate() ignores the
.set_rate return value, so the -ETIMEDOUT is swallowed silently. Is the
lock status readable at a different offset on this SoC, or is the bit
simply not functional there? Happy to send a follow-up (surfacing the
error, or skipping the poll where it cannot work) once someone can say
what the intended behavior is.

 drivers/clk/qcom/ipq-cmn-pll.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

--- a/drivers/clk/qcom/ipq-cmn-pll.c
+++ b/drivers/clk/qcom/ipq-cmn-pll.c
@@ -454,11 +454,19 @@
 
 	/* Register CMN PLL clock and fixed rate output clocks. */
 	ret = ipq_cmn_pll_register_clks(pdev);
-	pm_runtime_put(dev);
-	if (ret)
+	if (ret) {
+		pm_runtime_put(dev);
 		return dev_err_probe(dev, ret,
 				     "Failed to register CMN PLL clocks\n");
+	}
 
+	/*
+	 * Keep the runtime PM usage count elevated on the success path, so
+	 * the CMN block AHB & SYS clocks stay enabled: the clock ops access
+	 * the CMN PLL registers without taking a runtime PM reference, and
+	 * on IPQ5018 gating these clocks after probe hangs the SoC on a
+	 * subsequent bus access.
+	 */
 	return 0;
 }
 

Re: [PATCH] clk: qcom: ipq-cmn-pll: keep the CMN block bus clocks enabled

[Mieczyslaw Nalewaj]

On 7/30/2026 9:13 PM, Stanislaw Pal wrote:
> The probe function takes a runtime PM reference to enable the GCC AHB &
> SYS clocks of the CMN PLL block, registers the clocks, and then drops
> the reference. Once the autosuspend kicks in, pm_clk gates both clocks
> a few milliseconds after probe has returned.
> 
> That is wrong on two counts:
> 
>   - The clock ops access the CMN PLL registers without taking a runtime
>     PM reference of their own, so any later clk_set_rate() or rate
>     recalculation touches the register file with its bus clock gated.
> 
>   - On IPQ5018 the consequences are not contained to this device:
>     gating the CMN block bus clocks makes the SoC hang on a subsequent
>     bus access. Measured on a TP-Link Archer AX55 v1 (IPQ5018), the
>     machine dies silently within a few milliseconds of the CMN PLL
>     probe returning - mid-character on the UART - and the victim is
>     whichever device happens to probe next. Whether a given boot
>     survives the window is a micro-timing lottery: different binary
>     layouts of the same kernel ranged from occasional failures to a
>     100% reproducible boot loop.
> 
> Keep the runtime PM usage count elevated on the probe success path, so
> the bus clocks stay enabled. With this one change the same board went
> from boot-looping to surviving every boot attempt (verified across
> three previously-failing kernel binaries, plus repeated soft and hard
> resets on the final one).
> 
> Fixes: f81715a4c87c ("clk: qcom: Add CMN PLL clock controller driver for IPQ SoC")
> Cc: stable@vger.kernel.org
> Signed-off-by: Stanislaw Pal <kuncy7@gmail.com>
> ---
Reviewed-by: Mieczyslaw Nalewaj <namiltd@yahoo.com>

Re: [PATCH] clk: qcom: ipq-cmn-pll: keep the CMN block bus clocks enabled

[Jie Luo]

On 7/31/2026 3:13 AM, Stanislaw Pal wrote:
> The probe function takes a runtime PM reference to enable the GCC AHB &
> SYS clocks of the CMN PLL block, registers the clocks, and then drops
> the reference. Once the autosuspend kicks in, pm_clk gates both clocks
> a few milliseconds after probe has returned.
> 
> That is wrong on two counts:
> 
>   - The clock ops access the CMN PLL registers without taking a runtime
>     PM reference of their own, so any later clk_set_rate() or rate
>     recalculation touches the register file with its bus clock gated.

clk_cmn_pll_set_rate() is invoked as part of the AHB and SYS clock
enable sequence. The CMN PLL clock rate is configured according to the
DTS assigned-clock-rates-u64 property, which is applied when
devm_of_clk_add_hw_provider() (called inside
ipq_cmn_pll_register_clks()) triggers the call chain:

of_clk_add_hw_provider() → of_clk_set_defaults()

This corresponds to the following DTS entries:
assigned-clocks = <&cmn_pll IPQ5018_CMN_PLL_CLK>;
assigned-clock-rates-u64 = /bits/ 64 <4800000000>;

> 
>   - On IPQ5018 the consequences are not contained to this device:
>     gating the CMN block bus clocks makes the SoC hang on a subsequent
>     bus access. Measured on a TP-Link Archer AX55 v1 (IPQ5018), the
>     machine dies silently within a few milliseconds of the CMN PLL
>     probe returning - mid-character on the UART - and the victim is
>     whichever device happens to probe next. Whether a given boot
>     survives the window is a micro-timing lottery: different binary
>     layouts of the same kernel ranged from occasional failures to a
>     100% reproducible boot loop.
> 
> Keep the runtime PM usage count elevated on the probe success path, so
> the bus clocks stay enabled. With this one change the same board went
> from boot-looping to surviving every boot attempt (verified across
> three previously-failing kernel binaries, plus repeated soft and hard
> resets on the final one).

The CMN PLL AHB/SYS clocks will remain enabled as long as a downstream
consumer of the CMN PLL clocks is active — this behaviour was confirmed
experimentally on the IPQ9574 platform as example.

# insmod ipq-cmn-pll.ko
#
# devmem 0x9b780
0x00000000
#
# insmod nsscc-ipq9574.ko
# devmem 0x9b780
0x000000FF
#

> 
> Fixes: f81715a4c87c ("clk: qcom: Add CMN PLL clock controller driver for IPQ SoC")
> Cc: stable@vger.kernel.org
> Signed-off-by: Stanislaw Pal <kuncy7@gmail.com>
> ---
> One more observation from the same debugging session, for the people
> who know this silicon: on IPQ5018 the CMN_PLL_LOCKED bit (offset 0x64,
> bit 8) never asserts. Every clk_cmn_pll_set_rate() call runs its
> regmap_read_poll_timeout() to the full 100 ms timeout - including under
> the bootloader-programmed configuration the hardware demonstrably runs
> fine with - and nobody notices, because clk_change_rate() ignores the
> .set_rate return value, so the -ETIMEDOUT is swallowed silently. Is the
> lock status readable at a different offset on this SoC, or is the bit
> simply not functional there? Happy to send a follow-up (surfacing the
> error, or skipping the poll where it cannot work) once someone can say
> what the intended behavior is.

The CMN_PLL_LOCKED bit offset and behavior are consistent across all IPQ
platforms — the same register layout applies to IPQ5018.

> 
>  drivers/clk/qcom/ipq-cmn-pll.c | 12 ++++++++++--
>  1 file changed, 10 insertions(+), 2 deletions(-)
> 
> --- a/drivers/clk/qcom/ipq-cmn-pll.c
> +++ b/drivers/clk/qcom/ipq-cmn-pll.c
> @@ -454,11 +454,19 @@
>  
>  	/* Register CMN PLL clock and fixed rate output clocks. */
>  	ret = ipq_cmn_pll_register_clks(pdev);
> -	pm_runtime_put(dev);
> -	if (ret)
> +	if (ret) {
> +		pm_runtime_put(dev);
>  		return dev_err_probe(dev, ret,
>  				     "Failed to register CMN PLL clocks\n");
> +	}
>  
> +	/*
> +	 * Keep the runtime PM usage count elevated on the success path, so
> +	 * the CMN block AHB & SYS clocks stay enabled: the clock ops access
> +	 * the CMN PLL registers without taking a runtime PM reference, and
> +	 * on IPQ5018 gating these clocks after probe hangs the SoC on a
> +	 * subsequent bus access.
> +	 */
>  	return 0;
>  }
>  
> 

Re: [PATCH] clk: qcom: ipq-cmn-pll: keep the CMN block bus clocks enabled

[Stanislaw Pal]

On 8/3/2026 Jie Luo wrote:
> The CMN PLL AHB/SYS clocks will remain enabled as long as a downstream
> consumer of the CMN PLL clocks is active — this behaviour was confirmed
> experimentally on the IPQ9574 platform as example.

That is exactly the mechanism - and it is exactly what IPQ5018 does not
have. On IPQ9574 the nsscc node consumes the CMN PLL outputs in DT:

  ipq9574.dtsi:
    nsscc: clock-controller@39b00000 {
        clocks = <&xo_board_clk>,
                 <&cmn_pll NSS_1200MHZ_CLK>,
                 <&cmn_pll PPE_353MHZ_CLK>, ...

so the device link holds the supplier active, which is what your
insmod/devmem experiment shows.

On IPQ5018 there is no such consumer: nothing in ipq5018.dtsi references
any cmn_pll output clock - the only occurrences of the phandle are the
provider node itself and its own assigned-clocks. The actual users of
the CMN outputs on this SoC (the internal GE PHY and the uniphy blocks)
take them directly in hardware, with no DT linkage, so no device link
ever holds the provider active. A few ms after probe the autosuspend
gates the AHB/SYS clocks, and the box dies on the next bus access -
which is the measured behaviour the patch description quotes.

So I would frame the patch as: keep the usage count elevated on SoCs
where the hardware consumes the CMN outputs behind Linux's back. If the
preferred long-term shape is instead to describe those consumers in DT
(or to make the clock ops take runtime PM references of their own), I am
happy to help test either on IPQ5018 hardware - but until one of those
exists, this one-liner is what makes the SoC boot reliably, which is why
I kept it minimal and Cc'd stable.

> The CMN_PLL_LOCKED bit offset and behavior are consistent across all
> IPQ platforms — the same register layout applies to IPQ5018.

Understood - but then there may be something else wrong on this SoC,
because the measurement is unambiguous: on IPQ5018 the bit at offset
0x64 bit 8 never asserts. Every clk_cmn_pll_set_rate() call runs
regmap_read_poll_timeout() to the full 100 ms timeout, including for the
very configuration the bootloader programmed and the board demonstrably
runs on (ethernet and wifi clocks all functional). The -ETIMEDOUT is
then swallowed because clk_change_rate() ignores the .set_rate return
value, so nothing is ever logged. If the layout is the same, is there
anything that gates lock detection on this SoC (analog block state,
reference selection, a status-enable bit) that the bootloader may leave
in a different state than the driver expects? I can run any register
dump or experiment on the board that would help pin it down.

Thanks for looking at this, and thanks Mieczyslaw for the review.

Stanislaw

Re: [PATCH] clk: qcom: ipq-cmn-pll: keep the CMN block bus clocks enabled

[Jie Luo]

On 8/3/2026 5:10 PM, Stanislaw Pal wrote:
>> The CMN PLL AHB/SYS clocks will remain enabled as long as a downstream
>> consumer of the CMN PLL clocks is active — this behaviour was confirmed
>> experimentally on the IPQ9574 platform as example.
> That is exactly the mechanism - and it is exactly what IPQ5018 does not
> have. On IPQ9574 the nsscc node consumes the CMN PLL outputs in DT:
> 
>   ipq9574.dtsi:
>     nsscc: clock-controller@39b00000 {
>         clocks = <&xo_board_clk>,
>                  <&cmn_pll NSS_1200MHZ_CLK>,
>                  <&cmn_pll PPE_353MHZ_CLK>, ...
> 
> so the device link holds the supplier active, which is what your
> insmod/devmem experiment shows.
> 
> On IPQ5018 there is no such consumer: nothing in ipq5018.dtsi references
> any cmn_pll output clock - the only occurrences of the phandle are the
> provider node itself and its own assigned-clocks. The actual users of
> the CMN outputs on this SoC (the internal GE PHY and the uniphy blocks)
> take them directly in hardware, with no DT linkage, so no device link
> ever holds the provider active. A few ms after probe the autosuspend
> gates the AHB/SYS clocks, and the box dies on the next bus access -
> which is the measured behaviour the patch description quotes.

The CMN PLL output clocks do not depend on the AHB or SYS clocks. They
continue to operate correctly at the fixed rates even when the AHB
and SYS clocks are disabled. Therefore, once the CMN PLL module is
loaded, its output clocks are expected to operate at the correct
frequencies.

The downstream consumer is used to keep the AHB and SYS clocks enabled,
allowing the CMN PLL registers to be accessed.

Re: [PATCH] clk: qcom: ipq-cmn-pll: keep the CMN block bus clocks enabled

[Stanislaw Pal]

On 8/4/2026 Jie Luo wrote:
> The CMN PLL output clocks do not depend on the AHB or SYS clocks. They
> continue to operate correctly at the fixed rates even when the AHB
> and SYS clocks are disabled. Therefore, once the CMN PLL module is
> loaded, its output clocks are expected to operate at the correct
> frequencies.
>
> The downstream consumer is used to keep the AHB and SYS clocks enabled,
> allowing the CMN PLL registers to be accessed.

Agreed on both points, and they match what I measured: with the bus
clocks gated the PLL outputs keep running (ethernet and wifi stay
clocked), only register access dies.

But I think these two points together are exactly the argument for the
patch. The register accesses do not stop when there is no consumer: the
CCF invokes the driver's ops regardless. clk_cmn_pll_recalc_rate() does
two regmap_read()s and runs on any clk_get_rate() of the PLL and on
every debugfs clk_summary read - the latter user-triggerable at an
arbitrary time. clk_cmn_pll_set_rate() likewise accesses registers
whenever a rate is set. On IPQ5018, where no DT consumer exists at all,
every one of those calls after probe touches the block with AHB/SYS
gated, and that is the measured hang - the boards died during boot with
no userspace involved, so an in-kernel path hits it too.

And note the consumer mechanism only guarantees access "while the
consumer is active": on the SoCs that do have a DT consumer, a
runtime-suspended consumer plus a clk_summary read is the same
access-with-gated-clocks situation, just harder to hit. So having the
provider hold the reference for as long as it can be asked to service
clk ops - i.e. while bound - seems like the robust shape regardless of
platform.

I have just posted v2 which does exactly that, in a cleaner form:
devm_pm_runtime_get_noresume() in probe, so the reference is dropped
automatically on unbind and the existing put in the error path stays
untouched.

Thanks,
Stanislaw

Re: [PATCH] clk: qcom: ipq-cmn-pll: keep the CMN block bus clocks enabled

[Jie Luo]

On 8/4/2026 7:58 PM, Stanislaw Pal wrote:
> On 8/4/2026 Jie Luo wrote:
>> The CMN PLL output clocks do not depend on the AHB or SYS clocks. They
>> continue to operate correctly at the fixed rates even when the AHB
>> and SYS clocks are disabled. Therefore, once the CMN PLL module is
>> loaded, its output clocks are expected to operate at the correct
>> frequencies.
>>
>> The downstream consumer is used to keep the AHB and SYS clocks enabled,
>> allowing the CMN PLL registers to be accessed.
> 
> Agreed on both points, and they match what I measured: with the bus
> clocks gated the PLL outputs keep running (ethernet and wifi stay
> clocked), only register access dies.
> 
> But I think these two points together are exactly the argument for the
> patch. The register accesses do not stop when there is no consumer: the
> CCF invokes the driver's ops regardless. clk_cmn_pll_recalc_rate() does
> two regmap_read()s and runs on any clk_get_rate() of the PLL and on
> every debugfs clk_summary read - the latter user-triggerable at an
> arbitrary time. clk_cmn_pll_set_rate() likewise accesses registers
> whenever a rate is set. On IPQ5018, where no DT consumer exists at all,
> every one of those calls after probe touches the block with AHB/SYS
> gated, and that is the measured hang - the boards died during boot with
> no userspace involved, so an in-kernel path hits it too.
> 
> And note the consumer mechanism only guarantees access "while the
> consumer is active": on the SoCs that do have a DT consumer, a
> runtime-suspended consumer plus a clk_summary read is the same
> access-with-gated-clocks situation, just harder to hit. So having the
> provider hold the reference for as long as it can be asked to service
> clk ops - i.e. while bound - seems like the robust shape regardless of
> platform.
> 
> I have just posted v2 which does exactly that, in a cleaner form:
> devm_pm_runtime_get_noresume() in probe, so the reference is dropped
> automatically on unbind and the existing put in the error path stays
> untouched.

Is there any use case that requires accessing the CMN PLL registers when
no downstream consumer is active? If not, I don’t think a fix is needed
here. As you may have observed, debugfs clk_summary can still display
the clock rate correctly even when there is no downstream consumer and
the AHB and SYS clocks are disabled.

> 
> Thanks,
> Stanislaw

Re: [PATCH] clk: qcom: ipq-cmn-pll: keep the CMN block bus clocks enabled

[Stanislaw Pal]

On 8/5/2026 Jie Luo wrote:
> Is there any use case that requires accessing the CMN PLL registers when
> no downstream consumer is active? If not, I don't think a fix is needed
> here. As you may have observed, debugfs clk_summary can still display
> the clock rate correctly even when there is no downstream consumer and
> the AHB and SYS clocks are disabled.

The clk_summary observation does not show what it seems to show, and I
have to correct my own previous mail on the same point: this driver does
not set CLK_GET_RATE_NOCACHE, and clk_core_get_rate_recalc() only calls
.recalc_rate for clocks that have that flag. So clk_summary (and
clk_get_rate()) return the rate cached at registration time, when probe
still held the bus clocks enabled - no register access happens at all.
It displaying correct rates with the clocks gated is exactly the cached
value; it says nothing about whether an actual access would survive.

As for the use case: on IPQ5018 it is booting the SoC. With the clocks
gated after probe, boards hang within milliseconds - 100% reproducible
on some builds, before userspace exists, with no consumer anywhere - and
the only variable that changes the outcome is holding this reference.
That is also why I do not think moving runtime PM references into the
clk ops would help this platform: by your own argument nothing calls the
ops at that point, yet the SoC still dies. Whatever the fatal access is
- a CCF path we have not pinned down, or something else in the same
clock domain - the platform demonstrably does not survive the gate
itself.

One more general point: the CCF gives no guarantee that clk API calls
only happen while some consumer device is runtime-active. The DT device
link keeps the supplier active while the consumer *device* is active,
but a consumer is free to call clk_set_rate()/clk_round_rate() at any
time, including while itself runtime-suspended - and on IPQ5018 there is
no link at all. So "registers are only accessed while a consumer is
active" is not an invariant anything enforces; it just happens to hold
on the platforms where the DT wiring exists.

If there is an alternative fix you would prefer - a different clock
handoff from the bootloader state, describing the in-silicon consumers
in DT, anything else - I am happy to test it on this hardware. But as it
stands, a mainline kernel cannot boot reliably on IPQ5018 without this
change, which is why I believe a fix is needed and Cc'd stable.

Thanks,
Stanislaw

Re: [PATCH] clk: qcom: ipq-cmn-pll: keep the CMN block bus clocks enabled

[Jie Luo]

On 8/5/2026 4:12 PM, Stanislaw Pal wrote:
> On 8/5/2026 Jie Luo wrote:
>> Is there any use case that requires accessing the CMN PLL registers when
>> no downstream consumer is active? If not, I don't think a fix is needed
>> here. As you may have observed, debugfs clk_summary can still display
>> the clock rate correctly even when there is no downstream consumer and
>> the AHB and SYS clocks are disabled.
> 
> The clk_summary observation does not show what it seems to show, and I
> have to correct my own previous mail on the same point: this driver does
> not set CLK_GET_RATE_NOCACHE, and clk_core_get_rate_recalc() only calls
> .recalc_rate for clocks that have that flag. So clk_summary (and
> clk_get_rate()) return the rate cached at registration time, when probe
> still held the bus clocks enabled - no register access happens at all.
> It displaying correct rates with the clocks gated is exactly the cached
> value; it says nothing about whether an actual access would survive.
> 
> As for the use case: on IPQ5018 it is booting the SoC. With the clocks
> gated after probe, boards hang within milliseconds - 100% reproducible
> on some builds, before userspace exists, with no consumer anywhere - and
> the only variable that changes the outcome is holding this reference.
> That is also why I do not think moving runtime PM references into the
> clk ops would help this platform: by your own argument nothing calls the
> ops at that point, yet the SoC still dies. Whatever the fatal access is
> - a CCF path we have not pinned down, or something else in the same
> clock domain - the platform demonstrably does not survive the gate
> itself.

Once the CMN PLL module has been loaded, there should be no further need
to access its registers during normal operation. The CMN PLL provides
fixed-rate clocks, and its output clocks should be initialized and
operating at the correct fixed rates after the module is loaded for the
IPQ5018 platform.

Regarding the hang issue you mentioned, it should not be related to the
inability to access the CMN PLL registers. The actual root cause of the
hang should be investigated separately.

> 
> One more general point: the CCF gives no guarantee that clk API calls
> only happen while some consumer device is runtime-active. The DT device
> link keeps the supplier active while the consumer *device* is active,
> but a consumer is free to call clk_set_rate()/clk_round_rate() at any
> time, including while itself runtime-suspended - and on IPQ5018 there is
> no link at all. So "registers are only accessed while a consumer is
> active" is not an invariant anything enforces; it just happens to hold
> on the platforms where the DT wiring exists.
> 
> If there is an alternative fix you would prefer - a different clock
> handoff from the bootloader state, describing the in-silicon consumers
> in DT, anything else - I am happy to test it on this hardware. But as it
> stands, a mainline kernel cannot boot reliably on IPQ5018 without this
> change, which is why I believe a fix is needed and Cc'd stable.
> 
> Thanks,
> Stanislaw

Re: [PATCH] clk: qcom: ipq-cmn-pll: keep the CMN block bus clocks enabled

[Mieczyslaw Nalewaj]

On 8/6/2026 4:33 AM, Jie Luo wrote:
> 
> 
> On 8/5/2026 4:12 PM, Stanislaw Pal wrote:
>> On 8/5/2026 Jie Luo wrote:
>>> Is there any use case that requires accessing the CMN PLL registers when
>>> no downstream consumer is active? If not, I don't think a fix is needed
>>> here. As you may have observed, debugfs clk_summary can still display
>>> the clock rate correctly even when there is no downstream consumer and
>>> the AHB and SYS clocks are disabled.
>>
>> The clk_summary observation does not show what it seems to show, and I
>> have to correct my own previous mail on the same point: this driver does
>> not set CLK_GET_RATE_NOCACHE, and clk_core_get_rate_recalc() only calls
>> .recalc_rate for clocks that have that flag. So clk_summary (and
>> clk_get_rate()) return the rate cached at registration time, when probe
>> still held the bus clocks enabled - no register access happens at all.
>> It displaying correct rates with the clocks gated is exactly the cached
>> value; it says nothing about whether an actual access would survive.
>>
>> As for the use case: on IPQ5018 it is booting the SoC. With the clocks
>> gated after probe, boards hang within milliseconds - 100% reproducible
>> on some builds, before userspace exists, with no consumer anywhere - and
>> the only variable that changes the outcome is holding this reference.
>> That is also why I do not think moving runtime PM references into the
>> clk ops would help this platform: by your own argument nothing calls the
>> ops at that point, yet the SoC still dies. Whatever the fatal access is
>> - a CCF path we have not pinned down, or something else in the same
>> clock domain - the platform demonstrably does not survive the gate
>> itself.
> 
> Once the CMN PLL module has been loaded, there should be no further need
> to access its registers during normal operation. The CMN PLL provides
> fixed-rate clocks, and its output clocks should be initialized and
> operating at the correct fixed rates after the module is loaded for the
> IPQ5018 platform.
> 
> Regarding the hang issue you mentioned, it should not be related to the
> inability to access the CMN PLL registers. The actual root cause of the
> hang should be investigated separately.
> 
Following along here - just want to make sure this doesn't get lost:
Stanislaw pointed out earlier that clk_summary can't actually be used
as evidence here, since this driver doesn't set CLK_GET_RATE_NOCACHE
and so never reads the registers live; it just returns the cached
registration-time rate. That seems like a fairly load-bearing point
for the discussion and I haven't seen it addressed yet.

Given the hang is reportedly 100% reproducible pre-userspace, is there
a concrete next step to root-cause it, or is v3 the right call for now
with a follow-up tracked separately?

Re: [PATCH] clk: qcom: ipq-cmn-pll: keep the CMN block bus clocks enabled

[Stanislaw Pal]

On 8/8/2026 Mieczyslaw Nalewaj wrote:
> Given the hang is reportedly 100% reproducible pre-userspace, is there
> a concrete next step to root-cause it, or is v3 the right call for now
> with a follow-up tracked separately?

There is a concrete step, and I ran it today on a third board - this
time with no code of mine involved at all.

Hardware: GL.iNet GL-B3000 (IPQ5018), a supported in-tree OpenWrt
board, running the current bone-stock OpenWrt snapshot (kernel
6.18.41, which does not carry this patch). Out of the box it boot
loops: the last line on earlycon is at ~0.39s (the final initcall
before the driver probes start), then silence and a watchdog reset,
100% reproducible.

Adding exactly one thing to the kernel command line - no rebuild, no
patch:

    initcall_blacklist=ipq_cmn_pll_clk_driver_init

makes the same image boot: serial, SPI-NAND, SMEM partitions,
remoteproc all come up.

Combined with what we already know from the other two boards, this
isolates the trigger fairly tightly:

  - cmn-pll probe runs, last PM reference dropped (vanilla): dies
  - cmn-pll probe never runs (blacklist, clocks stay as the
    bootloader left them, i.e. enabled): boots
  - cmn-pll probe runs, reference held: boots - verified on this same
    board today with an image carrying v3 of this patch and nothing
    else changed; it comes up fully (shell over SSH, NAND, remoteproc),
    and clk_summary shows the CMN block bus clocks held enabled by the
    provider device.

The only variable separating the dying case from both surviving ones
is the AHB/SYS gate after probe. What exactly performs the fatal
access afterwards is the remaining open question - and this board is
well suited to answer it, since it has full U-Boot control and can run
experiment kernels from RAM. I intend to bisect that next (my current
suspicion is that the CMN block AHB clock also feeds the register path
of neighbouring blocks in the same region - MDIO at 0x88000/0x90000,
uniphy at 0x98000, CMN at 0x9b000 - which would explain why the CCF's
own runtime PM handling around the clk ops cannot help here). I am
happy to run any experiment Jie would like to see on this hardware.

So to answer the question directly: I believe v3 is the right call now
- three IPQ5018 boards, including a stock-image one, cannot boot
without it and the cost of keeping two bus clocks of a small block
enabled is negligible - with the exact-access root-causing tracked as
a follow-up. If the follow-up ends up pointing at a cleaner fix (e.g.
describing the real consumers of these bus clocks in DT), I will
gladly send it as a successor.

Thanks,
Stanislaw

Re: [PATCH] clk: qcom: ipq-cmn-pll: keep the CMN block bus clocks enabled

[Jie Luo]

On 8/10/2026 12:10 AM, Stanislaw Pal wrote:
> On 8/8/2026 Mieczyslaw Nalewaj wrote:
>> Given the hang is reportedly 100% reproducible pre-userspace, is there
>> a concrete next step to root-cause it, or is v3 the right call for now
>> with a follow-up tracked separately?
> 
> There is a concrete step, and I ran it today on a third board - this
> time with no code of mine involved at all.
> 
> Hardware: GL.iNet GL-B3000 (IPQ5018), a supported in-tree OpenWrt
> board, running the current bone-stock OpenWrt snapshot (kernel
> 6.18.41, which does not carry this patch). Out of the box it boot
> loops: the last line on earlycon is at ~0.39s (the final initcall
> before the driver probes start), then silence and a watchdog reset,
> 100% reproducible.
> 
> Adding exactly one thing to the kernel command line - no rebuild, no
> patch:
> 
>     initcall_blacklist=ipq_cmn_pll_clk_driver_init
> 
> makes the same image boot: serial, SPI-NAND, SMEM partitions,
> remoteproc all come up.
> 
> Combined with what we already know from the other two boards, this
> isolates the trigger fairly tightly:
> 
>   - cmn-pll probe runs, last PM reference dropped (vanilla): dies
>   - cmn-pll probe never runs (blacklist, clocks stay as the
>     bootloader left them, i.e. enabled): boots
>   - cmn-pll probe runs, reference held: boots - verified on this same
>     board today with an image carrying v3 of this patch and nothing
>     else changed; it comes up fully (shell over SSH, NAND, remoteproc),
>     and clk_summary shows the CMN block bus clocks held enabled by the
>     provider device.
> 
> The only variable separating the dying case from both surviving ones
> is the AHB/SYS gate after probe. What exactly performs the fatal
> access afterwards is the remaining open question - and this board is
> well suited to answer it, since it has full U-Boot control and can run
> experiment kernels from RAM. I intend to bisect that next (my current
> suspicion is that the CMN block AHB clock also feeds the register path
> of neighbouring blocks in the same region - MDIO at 0x88000/0x90000,
> uniphy at 0x98000, CMN at 0x9b000 - which would explain why the CCF's
> own runtime PM handling around the clk ops cannot help here). I am
> happy to run any experiment Jie would like to see on this hardware.
> 
> So to answer the question directly: I believe v3 is the right call now
> - three IPQ5018 boards, including a stock-image one, cannot boot
> without it and the cost of keeping two bus clocks of a small block
> enabled is negligible - with the exact-access root-causing tracked as
> a follow-up. If the follow-up ends up pointing at a cleaner fix (e.g.
> describing the real consumers of these bus clocks in DT), I will
> gladly send it as a successor.
> 
> Thanks,
> Stanislaw

I tested the latest linux-next code with the following changes:

Enabled the MDIO interface and added the Ethernet PHY as DTS below.
Enabled CONFIG_IPQ_CMN_PLL and CONFIG_MDIO_IPQ4019 as kernel modules.

--- a/arch/arm64/boot/dts/qcom/ipq5018.dtsi



+++ b/arch/arm64/boot/dts/qcom/ipq5018.dtsi



@@ -219,12 +219,11 @@ mdio0: mdio@88000 {



                              <0x019475c4 0x4>;



                        #address-cells = <1>;



                        #size-cells = <0>;



+                       clock-frequency = <6250000>;







                        clocks = <&gcc GCC_MDIO0_AHB_CLK>;



                        clock-names = "gcc_mdio_ahb_clk";







-                       status = "disabled";



-



                        ge_phy: ethernet-phy@7 {



                                compatible = "ethernet-phy-id004d.d0c0";



                                reg = <7>;



@@ -241,11 +240,16 @@ mdio1: mdio@90000 {



                        reg = <0x00090000 0x64>;



                        #address-cells = <1>;



                        #size-cells = <0>;



+                       pinctrl-0 = <&mdio_pins>;



+                       pinctrl-names = "default";



+                       clock-frequency = <6250000>;







                        clocks = <&gcc GCC_MDIO1_AHB_CLK>;



                        clock-names = "gcc_mdio_ahb_clk";







-                       status = "disabled";



+                       ethernet-phy@1c {



+                               reg = <0x1c>;



+                       };



                };







                cmn_pll: clock-controller@9b000 {



@@ -427,6 +431,23 @@ uart1_pins: uart1-state {



                                drive-strength = <8>;



                                bias-pull-down;



                        };



+



+                       mdio_pins: mdio-state {



+                               mdc-pins {



+                                       pins = "gpio36";



+                                       function = "mdc";



+                                       drive-strength = <8>;



+                                       bias-pull-up;



+                               };



+



+                               mdio-pins {



+                                       pins = "gpio37";



+                                       function = "mdio";



+                                       drive-strength = <8>;



+                                       bias-pull-up;



+                               };



+                       };



+



                };

The board booted successfully on the IPQ5018 RDP platform.

# insmod ipq-cmn-pll.ko
#
# devmem 0x1856308
0x80000000
# devmem 0x185630c
0x80000000
#
# insmod mdio-ipq4019.ko
#
# ls -l /sys/bus/mdio_bus/devices/
88000.mdio-1:07/  90000.mdio-1:1c/
# ls -l /sys/bus/mdio_bus/devices/
total 0
lrwxrwxrwx    1 root     root             0 Jan  1 00:01 88000.mdio-1:07
->
../../../devices/platform/soc@0/88000.mdio/mdio_bus/88000.mdio-1/88000.mdio-1:07
lrwxrwxrwx    1 root     root             0 Jan  1 00:01 90000.mdio-1:1c
->
../../../devices/platform/soc@0/90000.mdio/mdio_bus/90000.mdio-1/90000.mdio-1:1c

Re: [PATCH] clk: qcom: ipq-cmn-pll: keep the CMN block bus clocks enabled

[Stanislaw Pal]

On 8/11/2026 Jie Luo wrote:
> The board booted successfully on the IPQ5018 RDP platform.
>
> # insmod ipq-cmn-pll.ko
> # devmem 0x1856308
> 0x80000000
> # insmod mdio-ipq4019.ko
> # ls -l /sys/bus/mdio_bus/devices/
> 88000.mdio-1:07/  90000.mdio-1:1c/

Thank you for running this. Your result is correct, and I can now
reproduce the equivalent on the failing board - together the experiments
finally bound the problem tightly. I spent the evening on a GL-B3000
running three variants of the same tree, all with the fix reverted
(i.e. vanilla put in probe). Full data below.

Variant 1: gate delayed to idle. Vanilla behaviour, but the last
reference is dropped via pm_runtime_put_autosuspend() with a 60 s
autosuspend delay, and uniphy (the only in-tree consumer of the PLL
outputs in the OpenWrt tree; mainline has none) disabled in DT so the
gate actually lands. Result: the gate lands at ~75 s on an idle system
and the board does not care. runtime_status reads "suspended", both
WiFi radios keep serving clients, the console works, nothing in dmesg.
This is your RDP result reproduced on the board that dies: with the
system quiet, gating these clocks is harmless, and nothing in steady
state needs them - your efficiency argument is confirmed.

Variant 2: same DT (uniphy disabled), unmodified vanilla driver, so the
same gate lands right after probe, during early boot. Result over seven
boots of the identical image: one survived, six died with the familiar
signature - silence before the serial console comes up, then a watchdog
reset. So the crash does not need the ethernet path at all: it happens
with every PLL consumer disabled in DT, and it is probabilistic.

Variant 3 is the stock configuration (uniphy enabled): 100% boot loop
on this board, and the same on three boards from three vendors.

One methodological note for fairness: variants 1 and 2 are initramfs
images booted from RAM over tftp, so their early-boot activity profile
differs from a normal flash boot (no UBI attach in the fatal window,
for one). The 6-of-7 ratio is specific to that path and I would not
generalize the number. The stock 100% failure, however, *is* the
normal NAND boot path, so both paths are represented in the data and
both die - only the probability differs with the timing profile, which
is itself consistent with the collision picture.

Putting it together:

  gate at idle          -> safe, deterministically (your RDP, my V1)
  gate during boot      -> dies, probabilistically (V2: 6 of 7)
  never gate (this fix) -> boots, deterministically (3 boards)

The only variable separating V1 from V2 is *when* the gate lands. That
also retires my earlier "shared CSR bridge" theory - with the whole
ethernet path disabled the board still dies - and explains every odd
observation from the last month: the victim being whichever device
probes next, and the failure probability swinging wildly with binary
layout (both are just micro-timing of an asynchronous pm_clk_suspend
landing somewhere in early-boot bus activity; which transaction it
collides with is still not pinned down).

It also shows why a consumer-based model cannot close this hole: the
fatal window lies *between* the cmn-pll probe and the moment any
consumer could possibly take its first reference. On boards where
Linux is entered with these clocks running (every bootloader does), the
put in probe is what creates that window.

I am open on the shape of the fix - if you would rather see the
reference dropped once boot has settled, or a synchronous gate, I will
gladly test that on this board. But the boards are unbootable today,
and holding the reference is the smallest change that deterministically
removes the window, so I would still ask for v3 (or an equivalent) now,
with refinements as follow-ups.

Thanks,
Stanislaw

[PATCH v2] clk: qcom: ipq-cmn-pll: keep the CMN block bus clocks enabled

[Stanislaw Pal]

The probe function takes a runtime PM reference to enable the GCC AHB &
SYS clocks of the CMN PLL block, registers the clocks, and then drops
the reference, letting pm_clk gate both clocks a few milliseconds after
probe has returned. The clock ops access the CMN PLL registers without
a runtime PM reference of their own, and on IPQ5018 gating the CMN
block bus clocks makes the SoC hang on a subsequent bus access: boards
died silently within milliseconds of the CMN PLL probe, up to a 100%
reproducible boot loop, depending on binary layout (micro-timing).

Take a devres-managed runtime PM reference in probe, so the bus clocks
stay enabled for as long as the driver is bound and the reference is
released again on unbind.

Fixes: f81715a4c87c ("clk: qcom: Add CMN PLL clock controller driver for IPQ SoC")
Cc: stable@vger.kernel.org
Signed-off-by: Stanislaw Pal <kuncy7@gmail.com>
---
Changes in v2:
- Use devm_pm_runtime_get_noresume() instead of simply skipping the
  pm_runtime_put() on the probe success path. The v1 arrangement left
  the usage count elevated with nothing to balance it on unbind; the
  devres action releases it. Spotted by the Sashiko automated review
  (thanks to Mieczyslaw Nalewaj for pointing it out). Note that
  pm_runtime_reinit() on unbind does set the status back to suspended,
  so the leak did not have the re-bind consequences the report
  suggested - but it was a leak nonetheless, and the devres form is
  the idiomatic way to express "keep this device resumed while bound".
- The diff is now purely additive; the existing error handling in
  probe is left untouched.

Note for stable: devm_pm_runtime_get_noresume() was added in v6.16 by
commit 73db799bf5ef ("PM: runtime: Add new devm functions"), while this
driver dates back to v6.14. On 6.14.y/6.15.y (both EOL) the equivalent
is to move the pm_runtime_put() out of probe and add one to
ipq_cmn_pll_clk_remove() instead.

v1: https://lore.kernel.org/linux-clk/20260730191353.557494-1-kuncy7@gmail.com/

 drivers/clk/qcom/ipq-cmn-pll.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

--- a/drivers/clk/qcom/ipq-cmn-pll.c
+++ b/drivers/clk/qcom/ipq-cmn-pll.c
@@ -437,6 +437,17 @@ static int ipq_cmn_pll_clk_probe(struct
 	if (ret)
 		return ret;

+	/*
+	 * The clock ops access the CMN PLL registers without taking a
+	 * runtime PM reference of their own, and on IPQ5018 gating the CMN
+	 * block AHB & SYS clocks after probe hangs the SoC on a subsequent
+	 * bus access. Hold a reference for as long as the driver is bound
+	 * so that the bus clocks stay enabled.
+	 */
+	ret = devm_pm_runtime_get_noresume(dev);
+	if (ret)
+		return ret;
+
 	/* Register CMN PLL clock and fixed rate output clocks. */
 	ret = ipq_cmn_pll_register_clks(pdev);
 	pm_runtime_put(dev);

Re: [PATCH v2] clk: qcom: ipq-cmn-pll: keep the CMN block bus clocks enabled

[Mieczyslaw Nalewaj]

On 8/4/2026 1:53 PM, Stanislaw Pal wrote:
> The probe function takes a runtime PM reference to enable the GCC AHB &
> SYS clocks of the CMN PLL block, registers the clocks, and then drops
> the reference, letting pm_clk gate both clocks a few milliseconds after
> probe has returned. The clock ops access the CMN PLL registers without
> a runtime PM reference of their own, and on IPQ5018 gating the CMN
> block bus clocks makes the SoC hang on a subsequent bus access: boards
> died silently within milliseconds of the CMN PLL probe, up to a 100%
> reproducible boot loop, depending on binary layout (micro-timing).
> 
> Take a devres-managed runtime PM reference in probe, so the bus clocks
> stay enabled for as long as the driver is bound and the reference is
> released again on unbind.
> 
> Fixes: f81715a4c87c ("clk: qcom: Add CMN PLL clock controller driver for IPQ 

[...]

> +	/*
> +	 * The clock ops access the CMN PLL registers without taking a
> +	 * runtime PM reference of their own, and on IPQ5018 gating the CMN
> +	 * block AHB & SYS clocks after probe hangs the SoC on a subsequent
> +	 * bus access. Hold a reference for as long as the driver is bound
> +	 * so that the bus clocks stay enabled.
> +	 */
> +	ret = devm_pm_runtime_get_noresume(dev);
> +	if (ret)
> +		return ret;
> +
>  	/* Register CMN PLL clock and fixed rate output clocks. */
>  	ret = ipq_cmn_pll_register_clks(pdev);
>  	pm_runtime_put(dev);

Does this error path leak a runtime PM reference?

devm_pm_runtime_get_noresume() returns before reaching the unconditional pm_runtime_put(dev) further down. If it fails, the earlier pm_runtime_resume_and_get(dev) reference is never released, leaving the usage count elevated permanently — probe returning an error means there's no matching remove() to clean it up.

Suggested fix:

ret = devm_pm_runtime_get_noresume(dev);
if (ret) {
	pm_runtime_put(dev);
	return ret;
}


This failure mode is rare (devm_pm_runtime_get_noresume() only fails on devres allocation failure, and undoes its own get internally in that case), but the code as written still leaves the earlier reference unbalanced on this path.

Mieczyslaw Nalewaj

[PATCH v3] clk: qcom: ipq-cmn-pll: keep the CMN block bus clocks enabled

[Stanislaw Pal]

The probe function takes a runtime PM reference to enable the GCC AHB &
SYS clocks of the CMN PLL block, registers the clocks, and then drops
the reference, letting pm_clk gate both clocks a few milliseconds after
probe has returned. The clock ops access the CMN PLL registers without
a runtime PM reference of their own, and on IPQ5018 gating the CMN
block bus clocks makes the SoC hang on a subsequent bus access: boards
died silently within milliseconds of the CMN PLL probe, up to a 100%
reproducible boot loop, depending on binary layout (micro-timing).

Take a devres-managed runtime PM reference in probe, so the bus clocks
stay enabled for as long as the driver is bound and the reference is
released again on unbind.

Fixes: f81715a4c87c ("clk: qcom: Add CMN PLL clock controller driver for IPQ SoC")
Cc: stable@vger.kernel.org
Signed-off-by: Stanislaw Pal <kuncy7@gmail.com>
---
Changes in v3:
- Fix a reference leak on the devm_pm_runtime_get_noresume() failure
  path: v2 placed the call after pm_runtime_resume_and_get(), so an
  error return skipped the pm_runtime_put() further down and left that
  reference unbalanced. Spotted by Mieczyslaw Nalewaj.
  Rather than unwinding explicitly, the devres get is now taken before
  pm_runtime_resume_and_get(). Both helpers undo their own get on
  failure (devm_add_action_or_reset() runs the action,
  pm_runtime_get_active() calls pm_runtime_put_noidle()), so no error
  path needs cleanup at all. Happy to switch to the explicit
  pm_runtime_put() form if that reads better.

Changes in v2:
- Use devm_pm_runtime_get_noresume() instead of simply skipping the
  pm_runtime_put() on the probe success path. The v1 arrangement left
  the usage count elevated with nothing to balance it on unbind; the
  devres action releases it.
- The diff is purely additive; the existing error handling in probe is
  left untouched.

Note for stable: devm_pm_runtime_get_noresume() was added in v6.16 by
commit 73db799bf5ef ("PM: runtime: Add new devm functions"), while this
driver dates back to v6.14. On 6.14.y/6.15.y (both EOL) the equivalent
is to move the pm_runtime_put() out of probe and add one to
ipq_cmn_pll_clk_remove() instead.

v1: https://lore.kernel.org/linux-clk/20260730191353.557494-1-kuncy7@gmail.com/
v2: https://lore.kernel.org/linux-clk/20260804115359.16633-1-kuncy7@gmail.com/

 drivers/clk/qcom/ipq-cmn-pll.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

--- a/drivers/clk/qcom/ipq-cmn-pll.c
+++ b/drivers/clk/qcom/ipq-cmn-pll.c
@@ -433,6 +433,17 @@ static int ipq_cmn_pll_clk_probe(struct
 	if (ret)
 		return dev_err_probe(dev, ret, "Failed to add SYS clock\n");
 
+	/*
+	 * The clock ops access the CMN PLL registers without taking a
+	 * runtime PM reference of their own, and on IPQ5018 gating the CMN
+	 * block AHB & SYS clocks after probe hangs the SoC on a subsequent
+	 * bus access. Hold a reference for as long as the driver is bound
+	 * so that the bus clocks stay enabled.
+	 */
+	ret = devm_pm_runtime_get_noresume(dev);
+	if (ret)
+		return ret;
+
 	ret = pm_runtime_resume_and_get(dev);
 	if (ret)
 		return ret;

Re: [PATCH v3] clk: qcom: ipq-cmn-pll: keep the CMN block bus clocks enabled

[Stanislaw Pal]

Recording a tag that came in off-list, so it does not get lost: a second
user hit the same hang independently, on a board neither I nor anyone in
this thread has touched.

Georg Seema is bringing up a Cudy P5 (also IPQ5018). Without this patch
the board hangs during boot; with it applied it boots reliably. He found
the patch on his own while debugging that hang, and gave the tag on the
OpenWrt pull request that carries it:

  https://github.com/openwrt/openwrt/pull/24653

Tested-by: Georg Seema <georgseema@gmail.com>

That makes three IPQ5018 boards from three vendors - TP-Link Archer AX55
v1, GL.iNet GL-B3000 and Cudy P5 - where gating the CMN block bus clocks
after probe kills the boot, and holding the reference fixes it.

I am not repeating the earlier arguments here. The bisection of the
fatal access is still on my list and I will come back with v4 and a
corrected explanation once I have named the register and the code path,
rather than before.

Thanks,
Stanislaw