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