Re: [PATCH] clk: samsung: exynos850: Register clocks early

[Sam Protsenko]

On Sat, 23 Oct 2021 at 12:19, Krzysztof Kozlowski
<krzysztof.kozlowski@xxxxxxxxxxxxx> wrote:
>
> On 22/10/2021 23:56, Sam Protsenko wrote:
> > Some clocks must be registered before init calls. For example MCT clock
> > (from CMU_PERI) is needed for MCT timer driver, which is registered
> > with TIMER_OF_DECLARE(). By the time we get to core_initcall() used for
> > clk-exynos850 platform driver init, it's already too late. Inability to
> > get "mct" clock in MCT driver leads to kernel panic, as functions
> > registered with *_OF_DECLARE() can't do deferred calls. MCT timer driver
> > can't be fixed either, as it's acting as a clock source and it's
> > essential to register it in start_kernel() -> time_init().
> >
> > Let's register all Exynos850 clocks early, using
> > CLK_OF_DECLARE_DRIVER(), and do all stuff relying on "struct dev" object
> > (like runtime PM and enabling bus clock) later in platform driver probe.
> > Basically CLK_OF_DECLARE_DRIVER() matches CMU compatible, but clears
> > OF_POPULATED flag, which allows the same device to be matched again
> > later.
> >
> > Similar issue was discussed at [1] and addressed in commit 1f7db7bbf031
> > ("clk: renesas: cpg-mssr: Add early clock support"), as well as in
> > drivers/clk/mediatek/clk-mt2712.c.
> >
> > [1] https://patchwork.kernel.org/project/linux-renesas-soc/patch/20180829132954.64862-2-chris.brandt@xxxxxxxxxxx/
> >
> > Signed-off-by: Sam Protsenko <semen.protsenko@xxxxxxxxxx>
> > ---
> > Notes:
> >   - This patch should be applied on top of CMU_APM series
> >     (clk: samsung: exynos850: Implement CMU_APM domain)
> >   - I considered introducing some macro to reduce the code duplication
> >     (actually created one), but decided to go with plain code: this way
> >     it's easier to understand and navigate
> >   - Also considered registering only *some* clocks early: won't be
> >     possible, as leaf clocks (like MCT) depend on the whole clock tree.
> >     Registering early only *some* CMUs (like CMU_PERI) looks possible,
> >     but I'm not sure what is the best way to implement this, and that
> >     won't be consistent with other CMUs (too much code variation as for
> >     my taste)
> >
>
> In the long term it is better for entire kernel and SoC to support
> deferred probes and register clocks as devices, not as CLK_OF_DECLARE. I
> understand that it's not possible now to get rid of CLK_OF_DECLARE
> entirely, but we could meet half-way.
>

Agreed, the more general problem is probably insufficient dependency
resolving in kernel, so this is more like a workaround anyway. I'll
send v2 soon.

> The Exynos5433 has the same problem and it registers few core CMUs
> early: the TOP, CPUs, memory bus, peripheral buses including PERIS with
> MCT. I prefer this approach than yours. The only domains needing
> CLK_OF_DECLARE are TOP and PERIS. Maybe also CORE - one would have to
> check if GIC and CCI are needed early.
>

Let's register only CMU_PERI clocks early for now. If the need arises,
we can do the same for other domains later.

>
> Best regards,
> Krzysztof