public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH] clk: clk-imx8mm: Initialize clocks in arch_initcall
@ 2026-04-08 10:13 Paul Geurts
  2026-04-08 10:21 ` Ahmad Fatoum
                   ` (2 more replies)
  0 siblings, 3 replies; 7+ messages in thread
From: Paul Geurts @ 2026-04-08 10:13 UTC (permalink / raw)
  To: abelvesa, peng.fan, mturquette, sboyd, Frank.Li, s.hauer, kernel,
	festevam, shawnguo, linux-clk, imx, linux-arm-kernel,
	linux-kernel
  Cc: martijn.de.gouw, Paul Geurts

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


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH] clk: clk-imx8mm: Initialize clocks in arch_initcall
  2026-04-08 10:13 [PATCH] clk: clk-imx8mm: Initialize clocks in arch_initcall Paul Geurts
@ 2026-04-08 10:21 ` Ahmad Fatoum
  2026-04-09  9:16   ` Paul Geurts
  2026-04-09  6:51 ` Peng Fan
  2026-04-09 13:54 ` Abel Vesa
  2 siblings, 1 reply; 7+ messages in thread
From: Ahmad Fatoum @ 2026-04-08 10:21 UTC (permalink / raw)
  To: Paul Geurts, abelvesa, peng.fan, mturquette, sboyd, Frank.Li,
	s.hauer, kernel, festevam, shawnguo, linux-clk, imx,
	linux-arm-kernel, linux-kernel
  Cc: martijn.de.gouw

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 |


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] clk: clk-imx8mm: Initialize clocks in arch_initcall
  2026-04-08 10:13 [PATCH] clk: clk-imx8mm: Initialize clocks in arch_initcall Paul Geurts
  2026-04-08 10:21 ` Ahmad Fatoum
@ 2026-04-09  6:51 ` Peng Fan
  2026-04-09  9:29   ` Paul Geurts
  2026-04-09 13:54 ` Abel Vesa
  2 siblings, 1 reply; 7+ messages in thread
From: Peng Fan @ 2026-04-09  6:51 UTC (permalink / raw)
  To: Paul Geurts
  Cc: abelvesa, peng.fan, mturquette, sboyd, Frank.Li, s.hauer, kernel,
	festevam, shawnguo, linux-clk, imx, linux-arm-kernel,
	linux-kernel, martijn.de.gouw

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

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] clk: clk-imx8mm: Initialize clocks in arch_initcall
  2026-04-08 10:21 ` Ahmad Fatoum
@ 2026-04-09  9:16   ` Paul Geurts
  2026-04-09 11:34     ` Ahmad Fatoum
  0 siblings, 1 reply; 7+ messages in thread
From: Paul Geurts @ 2026-04-09  9:16 UTC (permalink / raw)
  To: abelvesa, peng.fan, mturquette, sboyd, Frank.Li, s.hauer, kernel,
	festevam, shawnguo, linux-clk, imx, linux-arm-kernel,
	linux-kernel
  Cc: martijn.de.gouw

> 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

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] clk: clk-imx8mm: Initialize clocks in arch_initcall
  2026-04-09  6:51 ` Peng Fan
@ 2026-04-09  9:29   ` Paul Geurts
  0 siblings, 0 replies; 7+ messages in thread
From: Paul Geurts @ 2026-04-09  9:29 UTC (permalink / raw)
  To: peng.fan
  Cc: abelvesa, mturquette, sboyd, Frank.Li, s.hauer, kernel, festevam,
	shawnguo, linux-clk, imx, linux-arm-kernel, linux-kernel,
	martijn.de.gouw, paul.geurts

> 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

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] clk: clk-imx8mm: Initialize clocks in arch_initcall
  2026-04-09  9:16   ` Paul Geurts
@ 2026-04-09 11:34     ` Ahmad Fatoum
  0 siblings, 0 replies; 7+ messages in thread
From: Ahmad Fatoum @ 2026-04-09 11:34 UTC (permalink / raw)
  To: Paul Geurts
  Cc: martijn.de.gouw, Saravana Kannan, abelvesa, peng.fan, mturquette,
	sboyd, Frank.Li, s.hauer, kernel, festevam, shawnguo, linux-clk,
	imx, linux-arm-kernel, linux-kernel

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 |


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] clk: clk-imx8mm: Initialize clocks in arch_initcall
  2026-04-08 10:13 [PATCH] clk: clk-imx8mm: Initialize clocks in arch_initcall Paul Geurts
  2026-04-08 10:21 ` Ahmad Fatoum
  2026-04-09  6:51 ` Peng Fan
@ 2026-04-09 13:54 ` Abel Vesa
  2 siblings, 0 replies; 7+ messages in thread
From: Abel Vesa @ 2026-04-09 13:54 UTC (permalink / raw)
  To: Paul Geurts
  Cc: abelvesa, peng.fan, mturquette, sboyd, Frank.Li, s.hauer, kernel,
	festevam, shawnguo, linux-clk, imx, linux-arm-kernel,
	linux-kernel, martijn.de.gouw

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.

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2026-04-09 13:54 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-04-08 10:13 [PATCH] clk: clk-imx8mm: Initialize clocks in arch_initcall Paul Geurts
2026-04-08 10:21 ` Ahmad Fatoum
2026-04-09  9:16   ` Paul Geurts
2026-04-09 11:34     ` Ahmad Fatoum
2026-04-09  6:51 ` Peng Fan
2026-04-09  9:29   ` Paul Geurts
2026-04-09 13:54 ` Abel Vesa

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox