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

Jie Luo <[email protected]>
Newsgroups org.kernel.vger.stable,org.kernel.vger.linux-arm-msm,org.kernel.vger.linux-clk,org.kernel.vger.linux-kernel
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
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.