Re: [PATCH] clk: qcom: ipq-cmn-pll: keep the CMN block bus clocks enabled
Stanislaw Pal <[email protected]> Tue, 4 Aug 2026 13:58:17 +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]> |
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