Re: [PATCH 4/4] clk: implement sync_state support

[Brian Masney]

On Mon, Jun 29, 2026 at 03:44:26PM +0200, Konrad Dybcio wrote:
> On 6/26/26 6:32 PM, Brian Masney wrote:
> > The existing support for disabling unused clks runs in the late initcall
> > stage, and it has been known for a long time that this is broken since
> > it runs too early in the boot up process. It doesn't work for kernel
> > modules, and it also doesn't work if all of the consumers haven't fully
> > probed yet. Folks have long recommended to boot certain platforms with
> > clk_ignore_unused to work around issues with disabling unused clks.
> 
> [...]
> 
> >     [    0.366737] clk: Disabling unused clocks not associated with a device
> >     [    0.367232] PM: genpd: Disabling unused power domains
> >     [    7.791413] qcom-qmp-pcie-phy 1c24000.phy: clk: Disabling unused clocks
> >     [    7.799702] qcom_aoss_qmp c300000.power-management: clk: Disabling unused clocks
> >     [    8.548820] qcom-qmp-pcie-phy 1c14000.phy: clk: Disabling unused clocks
> >     [    9.121849] qcom-qmp-usb-phy 88f1000.phy: clk: Disabling unused clocks
> >     [    9.121985] qcom-qmp-usb-phy 88ef000.phy: clk: Disabling unused clocks
> >     [    9.122691] qcom-edp-phy aec5a00.phy: clk: Disabling unused clocks
> 
> Many of these drivers only register fixed "virtual" clocks that
> don't lead to any registers being altered and only exist to convey
> what the clocks look like on the hw level.. but I don't think we
> have great infra to skip that..
> 
> >     [    9.122760] disp_cc-sc8280xp af00000.clock-controller: clk: Disabling unused clocks
> >     [    9.142121] qcom-qmp-combo-phy 88eb000.phy: clk: Disabling unused clocks
> >     [    9.169149] qcom-qmp-combo-phy 8903000.phy: clk: Disabling unused clocks
> >     [   16.057997] qcom-cpufreq-hw 18591000.cpufreq: clk: Disabling unused clocks
> >     [   16.058149] clk-rpmh 18200000.rsc:clock-controller: clk: Disabling unused clocks
> >     [   16.334879] qcom-qmp-pcie-phy 1c06000.phy: clk: Disabling unused clocks
> >     [   16.706113] camcc-sc8280xp ad00000.clock-controller: clk: Disabling unused clocks
> >     [   21.565731] q6prm-lpass-clock 3000000.remoteproc:glink-edge:gpr:service@2:clock-controller: clk: Disabling unused clocks
> >     [   21.597069] va_macro 3370000.codec: clk: Disabling unused clocks
> >     [   21.605039] rx_macro 3200000.rxmacro: clk: Disabling unused clocks
> >     [   21.630313] wsa_macro 3240000.codec: clk: Disabling unused clocks
> >     [   21.635069] tx_macro 3220000.txmacro: clk: Disabling unused clocks
> 
> This is a bit noisy, but then it's necessary for debugging the
> related hangs. Maybe if it turns out to be a huge issue, we can
> hide it behind a _dbg() in the future.

Agreed. Personally I think we have this go in with dev_info() initially,
then after it's been there for one release, move it to dev_dbg().

> I was hoping/expecting that sync_state would completely replace the
> late initcall (which would also simplify this diff), but I'm surprised
> to learn that there's a whole bunch of clk_register(dev=NULL)
> calls in the kernel (are most of them doing it for no good reason
> by chance?)

Yes, correct there are a ton of clk drivers that use
clk_register(dev=NULL) to register clks from __init early in the boot.
My understanding of one use case is that this is needed to setup arch
timers that have a dependency on a clock.

My understanding is that lots of drivers that register clks in __init
don't need to do it from there.

Brian