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

[Stanislaw Pal <kuncy7@gmail.com>]

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;