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

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