drivers/clk/qcom/ipq-cmn-pll.c | 11 +++++++++++ 1 file changed, 11 insertions(+)
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);
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
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;
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
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 asynchronously a few
milliseconds after probe has returned.
On IPQ5018 that gate races with early-boot activity on the bus and can
hang the SoC: boards die silently right after the CMN PLL probe, before
the next initcall gets to run, and the watchdog resets them. Whether a
given kernel binary survives depends on micro-timing, ranging from an
occasional hang to a 100% reproducible boot loop. The failure has been
reported independently on three boards from three vendors (TP-Link
Archer AX55 v1, GL.iNet GL-B3000, Cudy P5).
Isolation on the Cudy P5 (by Georg Seema) shows the failure is a
matter of timing against boot activity, not a steady-state clock
dependency:
- the probe completes in ~628 us, pm_runtime_put() returns, and the
board dies before the next initcall starts;
- stretching the end of probe by ~15 ms (first unintentionally with
debug prints, then with usleep_range()) makes the same kernel boot
reliably;
- on that board an enabled UNIPHY0 node is what arms the failure;
MDIO0/1, GMAC0/1 and the attached QCA8337 switch do not trigger it.
The armed configuration differs per board: on the GL-B3000 the failure
persists with UNIPHY0 disabled (6 of 7 boots die), so the gate collides
with whatever bus activity is in flight at that moment rather than with
one specific peripheral.
Nor is a delay a workaround: replayed on the GL-B3000, stretching the
end of probe by the same 15 ms - or by a full 2 s - still ends in a
watchdog reset (8 of 8 boots each). A delay only helps where the
sensitive activity happens to be finished before the gate lands, and
how far into boot that extends is board-specific.
Consistently with the race picture, gating the very same clocks on an
idle, fully booted system is harmless: delaying the gate via runtime PM
autosuspend to ~75 s after boot on the GL-B3000 leaves a fully working
system (runtime_status "suspended", WiFi serving clients), matching the
module-insertion test on the IPQ5018 RDP posted by Jie Luo in the
review thread. The clocks are not needed in steady state; it is the
gate landing amid boot activity that kills the SoC. The window between
the CMN PLL probe and the first reference taken by any consumer is
exactly where the gate lands, so no consumer-side scheme can cover it.
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
Tested-by: Georg Seema <georgseema@gmail.com>
Signed-off-by: Stanislaw Pal <kuncy7@gmail.com>
---
Changes in v4:
- No functional change; the code differs from v3 only in the comment.
Commit message and comment rewritten now that the failure mode has
been isolated: v3 claimed the clock ops access the registers without
a runtime PM reference of their own, which is not accurate (the
common clock framework wraps provider ops in clk_pm_runtime_get() /
put()); the actual failure is the race on the gate transition
described above.
- Added Georg Seema's Tested-by from the OpenWrt pull request carrying
this patch (https://github.com/openwrt/openwrt/pull/24653); the Cudy
P5 isolation above is his work, quoted with his permission.
- The idle-gate / boot-gate measurements on the GL-B3000 referenced
above were posted earlier in this thread:
https://lore.kernel.org/linux-clk/20260811195317.128954-1-kuncy7@gmail.com/
- New measurement for this revision: the probe-stretch experiment
replayed on the GL-B3000 (vanilla put plus usleep_range(15000, 16000),
then plus msleep(2000), everything else stock) dies 8 of 8 boots with
either delay, while the same 15 ms rescues the Cudy P5 - the basis
for the "delay is not a workaround" paragraph above.
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, so no error path needs cleanup at all.
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.
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/
v3: https://lore.kernel.org/linux-clk/20260805193638.15327-1-kuncy7@gmail.com/
drivers/clk/qcom/ipq-cmn-pll.c | 12 ++++++++++++
1 file changed, 12 insertions(+)
--- a/drivers/clk/qcom/ipq-cmn-pll.c
+++ b/drivers/clk/qcom/ipq-cmn-pll.c
@@ -448,6 +448,18 @@ static int ipq_cmn_pll_clk_probe(struct
if (ret)
return dev_err_probe(dev, ret, "Failed to add SYS clock\n");
+ /*
+ * Gating the CMN block AHB & SYS clocks is only safe on an idle
+ * system: without this reference the gate lands asynchronously a
+ * few milliseconds after probe, in the middle of the early-boot
+ * probe activity, and on IPQ5018 that races with other bus
+ * traffic and hangs the SoC. Hold the 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;
Please drop this patch.
Gabor's fix for the same hang has been applied:
33f9cb56cc28 ("clk: qcom: gcc-ipq5018: mark 'gpll0_main' clock as critical")
It addresses the platform-level cause: 'gpll0_main' feeds the CPUs
through GPLL0 during early boot, before the APCS clock is registered
and CCF can see that consumer, so the first driver to drop a runtime PM
reference on a GCC clock under it takes the CPUs down with it. The
ipq-cmn-pll probe was only the first driver to do so on the AX55 v1; I
had verified on that board that Gabor's change alone gives a reliable
boot with this patch absent, and OpenWrt has since replaced this patch
with his in its tree.
That also answers the two open questions here. Jie's suggestion of
making the GCC a consumer of the CMN PLL outputs is no longer needed
for this hang, and Konrad's question about crashdump mode I cannot
answer: I never captured that state, and with the fix in place I have
nothing left that reproduces it.
Thank you all for the review, and Gabor for finding the actual cause.
Best regards,
Stanislaw
Hi Stephen, Bjorn, Just a quick ping regarding this series. The discussion between Stanislaw and Qualcomm (Jie Luo) has been fully settled in late August. It was proven that handling this on the UNIPHY side is not possible due to the early stage of the watchdog reset. Are there any remaining concerns, or can this v4 be picked up for the next cycle? Mieczyslaw Nalewaj<namiltd@yahoo.com>
On 9/8/26 12:57 PM, Mieczyslaw Nalewaj wrote: > Hi Stephen, Bjorn, > > Just a quick ping regarding this series. The discussion between Stanislaw and Qualcomm (Jie Luo) has been fully settled in late August. It was proven that handling this on the UNIPHY side is not possible due to the early stage of the watchdog reset. > > Are there any remaining concerns, or can this v4 be picked up for the next cycle? Since it doesn't seem like a proper fix is coming soon, this is much better than keeping the platform broken in the meantime Acked-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com> Konrad
© 2016 - 2026 Red Hat, Inc.