[PATCH v3] clk: qcom: ipq-cmn-pll: keep the CMN block bus clocks enabled
Stanislaw Pal <[email protected]> Wed, 5 Aug 2026 21:36:38 +0200
| Newsgroups | org.kernel.vger.linux-clk,org.kernel.vger.linux-arm-msm,org.kernel.vger.linux-kernel,org.kernel.vger.stable |
|---|---|
| Message-ID | <[email protected]> |
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: [email protected]
Signed-off-by: Stanislaw Pal <[email protected]>
---
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/[email protected]/
v2: https://lore.kernel.org/linux-clk/[email protected]/
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;