Re: [PATCH] clk: qcom: ipq-cmn-pll: keep the CMN block bus clocks enabled
Jie Luo <[email protected]> Wed, 5 Aug 2026 12:52:26 +0800
| 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]> |
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