Re: [PATCH] clk: clk-imx8mm: Initialize clocks in arch_initcall

[Paul Geurts]

> Hello Paul,
> 
> On 4/8/26 12:13 PM, Paul Geurts wrote:
> > The i.MX8MM clock driver is implemented as module_platform_driver();,
> > which makes it initialize in device_initcall(). This means that all
> > drivers referencing the clock driver nodes in the device tree are
> > deferred by fw_devlink, which are most of the i.MX8M platform drivers.
> >
> > Explicitly initialize the clock driver in arch_initcall(), to make sure
> > the clock driver is ready when the rest of the drivers are probed.
> >
> > Fixes: af7e7ee0e428 ("clk: imx8mm: Switch to platform driver")
> 
> Your commit message doesn't explain why this was a problem for you.
> Does it delay your boot? What makes this patch a fix?

Yes I could update that in the commit description. The problem is that because
of this, _all_ hardware is initialized in late_initcall, as that is where
deferred probes are handled. For embedded devices, some sign of life is
expected by most people during boot. Especially when an initrd needs to be
unpacked, this sign of life is going to take a very long time. Some display
controllers don't even get enough time to show the boot logo because of this.
I don't think the idea behind the initcall levels is that _everything_ is
initialized in late.

> 
> > Signed-off-by: Paul Geurts <paul.geurts@xxxxxxxxxxxxxxxxxxxxxxxxx>
> > ---
> >  drivers/clk/imx/clk-imx8mm.c | 14 +++++++++++++-
> >  1 file changed, 13 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/clk/imx/clk-imx8mm.c b/drivers/clk/imx/clk-imx8mm.c
> > index 319af4deec01..7b2cf867b920 100644
> > --- a/drivers/clk/imx/clk-imx8mm.c
> > +++ b/drivers/clk/imx/clk-imx8mm.c
> > @@ -636,7 +636,19 @@ static struct platform_driver imx8mm_clk_driver = {
> >                .of_match_table = imx8mm_clk_of_match,
> >        },
> >  };
> > -module_platform_driver(imx8mm_clk_driver);
> > +
> > +static int __init imx8mm_clk_init(void)
> > +{
> > +     return platform_driver_register(&imx8mm_clk_driver);
> > +}
> > +arch_initcall(imx8mm_clk_init);
> 
> What happens if you build the driver as module with your changes applied?

On module insertion, there is no initcall level, and initialization is
performed on insertion (AFAIK). Fact is that the system would probably
not boot when this is built as a module, as there are no peripheral clocks
without it.

> 
> Cheers,
> Ahmad
> 
> > +
> > +static void __exit imx8mm_clk_exit(void)
> > +{
> > +     platform_driver_unregister(&imx8mm_clk_driver);
> > +}
> > +module_exit(imx8mm_clk_exit);
> > +
> >  module_param(mcore_booted, bool, S_IRUGO);
> >  MODULE_PARM_DESC(mcore_booted, "See Cortex-M core is booted or not");
> > 

Thanks!
Paul