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

[Ahmad Fatoum]

Hello Paul,

Cc += Saravana

On 4/9/26 11:16 AM, Paul Geurts wrote:
>> 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.

There's no one initcall order that will make drivers across all systems
equally happy. That's why there are probe deferrals in the first place.

> 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.

Ok, so the problem is that the probes happen too late. Does the total
boot time take considerably longer or are you just unhappy with the
ordering?

> 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.

I suspect we could improve the situation with "post-init-providers"
hints, but I haven't used it myself so far.
Maybe Saravana could give some advice once the problem is better understood?

>> 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.

Ok, then this is patch is not acceptable. What's buildable as module
should work as module. I don't personally build it as module either, but
removing the possibility will break users relying on it for Android GKI,
I presume.

We thus need to find a different, better, way.

Cheers,
Ahmad

> 
>>
>> 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
> 
> 

-- 
Pengutronix e.K.                  |                             |
Steuerwalder Str. 21              | http://www.pengutronix.de/  |
31137 Hildesheim, Germany         | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686  | Fax:   +49-5121-206917-5555 |