From: Stanislaw Pal <kuncy7@gmail.com>
To: Jie Luo <jie.luo@oss.qualcomm.com>
Cc: Bjorn Andersson <andersson@kernel.org>,
	Stephen Boyd <sboyd@kernel.org>,
	Michael Turquette <mturquette@baylibre.com>,
	Mieczyslaw Nalewaj <namiltd@yahoo.com>,
	Brian Masney <bmasney@redhat.com>,
	linux-arm-msm@vger.kernel.org,
	linux-clk@vger.kernel.org,
	linux-kernel@vger.kernel.org,
	stable@vger.kernel.org
Subject: Re: [PATCH] clk: qcom: ipq-cmn-pll: keep the CMN block bus clocks enabled
Date: Tue,  4 Aug 2026 13:58:17 +0200
Message-ID: <20260804115817.16886-1-kuncy7@gmail.com>
In-Reply-To: <8af46926-fecc-4396-973e-2290ae285998@oss.qualcomm.com>
References: <8af46926-fecc-4396-973e-2290ae285998@oss.qualcomm.com>
Precedence: bulk
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit

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