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

Paul Geurts posted 1 patch 2 months, 1 week ago
drivers/clk/imx/clk-imx8mm.c | 14 +++++++++++++-
1 file changed, 13 insertions(+), 1 deletion(-)
[PATCH] clk: clk-imx8mm: Initialize clocks in arch_initcall
Posted by Paul Geurts 2 months, 1 week ago
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")
Signed-off-by: Paul Geurts <paul.geurts@prodrive-technologies.com>
---
 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);
+
+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");
 
-- 
2.39.2
Re: [PATCH] clk: clk-imx8mm: Initialize clocks in arch_initcall
Posted by Abel Vesa 2 months ago
On 26-04-08 12:13:13, 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")
> Signed-off-by: Paul Geurts <paul.geurts@prodrive-technologies.com>

Nack.

Nothing wrong with probe deferring. It is there to ensure the order
the drivers probe in is correct.

Plus, moving it to arch_initcall won't solve anything.

fw_devlink will not stop the driver from probing if there is no provider
that this driver is waiting for. And if there is a provider that is
needed by this clock controller, moving it to arch_initcall won't
magically skip waiting for the provider anyway.
Re: [PATCH] clk: clk-imx8mm: Initialize clocks in arch_initcall
Posted by Peng Fan 2 months ago
On Wed, Apr 08, 2026 at 12:13:13PM +0200, 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.

Let's keep as it is, changing to arch_initcall() is not allowed.

Thanks,
Peng
Re: [PATCH] clk: clk-imx8mm: Initialize clocks in arch_initcall
Posted by Paul Geurts 2 months ago
> On Wed, Apr 08, 2026 at 12:13:13PM +0200, 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.
> 
> Let's keep as it is, changing to arch_initcall() is not allowed.

Why is it not allowed? This is an arch driver, so I think it should be
initialized in arch? I don't think the initcall system was intended to
initialize everything in late_initcall, which is effectively what this
problem is causing.

> 
> Thanks,
> Peng

Thanks!
Paul
Re: [PATCH] clk: clk-imx8mm: Initialize clocks in arch_initcall
Posted by Ahmad Fatoum 2 months, 1 week ago
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?

> Signed-off-by: Paul Geurts <paul.geurts@prodrive-technologies.com>
> ---
>  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?

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");
>  

-- 
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 |
Re: [PATCH] clk: clk-imx8mm: Initialize clocks in arch_initcall
Posted by Paul Geurts 2 months ago
> 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@prodrive-technologies.com>
> > ---
> >  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
Re: [PATCH] clk: clk-imx8mm: Initialize clocks in arch_initcall
Posted by Ahmad Fatoum 2 months ago
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 |