[PATCH] clk: scu/imx8qxp: do not register driver in probe()

[PATCH] clk: scu/imx8qxp: do not register driver in probe()

[Danilo Krummrich]

imx_clk_scu_init() registers the imx_clk_scu_driver while commonly being
called from IMX driver's probe() callbacks.

However, it neither makes sense to register drivers from probe()
callbacks of other drivers, nor does the driver core allow registering
drivers with a device lock already being held.

The latter was revealed by commit dc23806a7c47 ("driver core: enforce
device_lock for driver_match_device()") leading to a deadlock condition
described in [1].

Additionally, nothing seems to unregister the imx_clk_scu_driver once
the corresponding driver module is unloaded, which leaves the
driver-core with a dangling pointer.

Hence, register the imx_clk_scu_driver from module_init() and unregister
it in module_exit().

Fixes: dc23806a7c47 ("driver core: enforce device_lock for driver_match_device()")
Fixes: 220175cd3979 ("clk: imx: scu: fix build break when compiled as modules")
Reported-by: Alexander Stein <alexander.stein@ew.tq-group.com>
Closes: https://lore.kernel.org/lkml/13955113.uLZWGnKmhe@steina-w/
Link: https://lore.kernel.org/lkml/DFU7CEPUSG9A.1KKGVW4HIPMSH@kernel.org/ [1]
Signed-off-by: Danilo Krummrich <dakr@kernel.org>
---
I plan to send the driver-core PR for 7.0-rc1 soon, which will also contain
commit dc23806a7c47 ("driver core: enforce device_lock for
driver_match_device()").

It also contains an IOMMU commit similar to this one:
https://patch.msgid.link/20260121141215.29658-1-dakr@kernel.org

The commit was originally scheduled for 6.19-rc7, but was deferred to 7.0-rc1 in
case more of those cases pop up (which now happened).
---
 drivers/clk/imx/clk-imx8qxp.c | 24 +++++++++++++++++++++++-
 drivers/clk/imx/clk-scu.c     | 12 +++++++++++-
 drivers/clk/imx/clk-scu.h     |  2 ++
 3 files changed, 36 insertions(+), 2 deletions(-)

diff --git a/drivers/clk/imx/clk-imx8qxp.c b/drivers/clk/imx/clk-imx8qxp.c
index 3ae162625bb1..d89a2f40771e 100644
--- a/drivers/clk/imx/clk-imx8qxp.c
+++ b/drivers/clk/imx/clk-imx8qxp.c
@@ -346,7 +346,29 @@ static struct platform_driver imx8qxp_clk_driver = {
 	},
 	.probe = imx8qxp_clk_probe,
 };
-module_platform_driver(imx8qxp_clk_driver);
+
+static int __init imx8qxp_init(void)
+{
+	int ret;
+
+	ret = platform_driver_register(&imx8qxp_clk_driver);
+	if (ret)
+		return ret;
+
+	ret = imx_clk_scu_module_init();
+	if (ret)
+		platform_driver_unregister(&imx8qxp_clk_driver);
+
+	return ret;
+}
+module_init(imx8qxp_init);
+
+static void __exit imx8qxp_exit(void)
+{
+	imx_clk_scu_module_exit();
+	platform_driver_unregister(&imx8qxp_clk_driver);
+}
+module_exit(imx8qxp_exit);
 
 MODULE_AUTHOR("Aisheng Dong <aisheng.dong@nxp.com>");
 MODULE_DESCRIPTION("NXP i.MX8QXP clock driver");
diff --git a/drivers/clk/imx/clk-scu.c b/drivers/clk/imx/clk-scu.c
index 34c9dc1fb20e..c90d21e05f91 100644
--- a/drivers/clk/imx/clk-scu.c
+++ b/drivers/clk/imx/clk-scu.c
@@ -191,6 +191,16 @@ static bool imx_scu_clk_is_valid(u32 rsrc_id)
 	return p != NULL;
 }
 
+int __init imx_clk_scu_module_init(void)
+{
+	return platform_driver_register(&imx_clk_scu_driver);
+}
+
+void __exit imx_clk_scu_module_exit(void)
+{
+	return platform_driver_unregister(&imx_clk_scu_driver);
+}
+
 int imx_clk_scu_init(struct device_node *np,
 		     const struct imx_clk_scu_rsrc_table *data)
 {
@@ -215,7 +225,7 @@ int imx_clk_scu_init(struct device_node *np,
 		rsrc_table = data;
 	}
 
-	return platform_driver_register(&imx_clk_scu_driver);
+	return 0;
 }
 
 /*
diff --git a/drivers/clk/imx/clk-scu.h b/drivers/clk/imx/clk-scu.h
index af7b697f51ca..ca82f2cce897 100644
--- a/drivers/clk/imx/clk-scu.h
+++ b/drivers/clk/imx/clk-scu.h
@@ -25,6 +25,8 @@ extern const struct imx_clk_scu_rsrc_table imx_clk_scu_rsrc_imx8dxl;
 extern const struct imx_clk_scu_rsrc_table imx_clk_scu_rsrc_imx8qxp;
 extern const struct imx_clk_scu_rsrc_table imx_clk_scu_rsrc_imx8qm;
 
+int __init imx_clk_scu_module_init(void);
+void __exit imx_clk_scu_module_exit(void);
 int imx_clk_scu_init(struct device_node *np,
 		     const struct imx_clk_scu_rsrc_table *data);
 struct clk_hw *imx_scu_of_clk_src_get(struct of_phandle_args *clkspec,

base-commit: 192c0159402e6bfbe13de6f8379546943297783d
-- 
2.52.0

Re: [PATCH] clk: scu/imx8qxp: do not register driver in probe()

[Alexander Stein]

Hi Danilo,

Am Mittwoch, 11. Februar 2026, 15:23:16 CET schrieb Danilo Krummrich:
> imx_clk_scu_init() registers the imx_clk_scu_driver while commonly being
> called from IMX driver's probe() callbacks.
> 
> However, it neither makes sense to register drivers from probe()
> callbacks of other drivers, nor does the driver core allow registering
> drivers with a device lock already being held.
> 
> The latter was revealed by commit dc23806a7c47 ("driver core: enforce
> device_lock for driver_match_device()") leading to a deadlock condition
> described in [1].
> 
> Additionally, nothing seems to unregister the imx_clk_scu_driver once
> the corresponding driver module is unloaded, which leaves the
> driver-core with a dangling pointer.
> 
> Hence, register the imx_clk_scu_driver from module_init() and unregister
> it in module_exit().
> 
> Fixes: dc23806a7c47 ("driver core: enforce device_lock for driver_match_device()")
> Fixes: 220175cd3979 ("clk: imx: scu: fix build break when compiled as modules")
> Reported-by: Alexander Stein <alexander.stein@ew.tq-group.com>
> Closes: https://lore.kernel.org/lkml/13955113.uLZWGnKmhe@steina-w/
> Link: https://lore.kernel.org/lkml/DFU7CEPUSG9A.1KKGVW4HIPMSH@kernel.org/ [1]
> Signed-off-by: Danilo Krummrich <dakr@kernel.org>

Thanks for the patch.
Tested-by: Alexander Stein <alexander.stein@ew.tq-group.com> # TQMa8x/MBa8x

> ---
> I plan to send the driver-core PR for 7.0-rc1 soon, which will also contain
> commit dc23806a7c47 ("driver core: enforce device_lock for
> driver_match_device()").
> 
> It also contains an IOMMU commit similar to this one:
> https://patch.msgid.link/20260121141215.29658-1-dakr@kernel.org
> 
> The commit was originally scheduled for 6.19-rc7, but was deferred to 7.0-rc1 in
> case more of those cases pop up (which now happened).
> ---
>  drivers/clk/imx/clk-imx8qxp.c | 24 +++++++++++++++++++++++-
>  drivers/clk/imx/clk-scu.c     | 12 +++++++++++-
>  drivers/clk/imx/clk-scu.h     |  2 ++
>  3 files changed, 36 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/clk/imx/clk-imx8qxp.c b/drivers/clk/imx/clk-imx8qxp.c
> index 3ae162625bb1..d89a2f40771e 100644
> --- a/drivers/clk/imx/clk-imx8qxp.c
> +++ b/drivers/clk/imx/clk-imx8qxp.c
> @@ -346,7 +346,29 @@ static struct platform_driver imx8qxp_clk_driver = {
>  	},
>  	.probe = imx8qxp_clk_probe,
>  };
> -module_platform_driver(imx8qxp_clk_driver);
> +
> +static int __init imx8qxp_init(void)
> +{
> +	int ret;
> +
> +	ret = platform_driver_register(&imx8qxp_clk_driver);
> +	if (ret)
> +		return ret;
> +
> +	ret = imx_clk_scu_module_init();
> +	if (ret)
> +		platform_driver_unregister(&imx8qxp_clk_driver);
> +
> +	return ret;
> +}
> +module_init(imx8qxp_init);
> +
> +static void __exit imx8qxp_exit(void)
> +{
> +	imx_clk_scu_module_exit();
> +	platform_driver_unregister(&imx8qxp_clk_driver);
> +}
> +module_exit(imx8qxp_exit);
>  
>  MODULE_AUTHOR("Aisheng Dong <aisheng.dong@nxp.com>");
>  MODULE_DESCRIPTION("NXP i.MX8QXP clock driver");
> diff --git a/drivers/clk/imx/clk-scu.c b/drivers/clk/imx/clk-scu.c
> index 34c9dc1fb20e..c90d21e05f91 100644
> --- a/drivers/clk/imx/clk-scu.c
> +++ b/drivers/clk/imx/clk-scu.c
> @@ -191,6 +191,16 @@ static bool imx_scu_clk_is_valid(u32 rsrc_id)
>  	return p != NULL;
>  }
>  
> +int __init imx_clk_scu_module_init(void)
> +{
> +	return platform_driver_register(&imx_clk_scu_driver);
> +}
> +
> +void __exit imx_clk_scu_module_exit(void)
> +{
> +	return platform_driver_unregister(&imx_clk_scu_driver);
> +}
> +
>  int imx_clk_scu_init(struct device_node *np,
>  		     const struct imx_clk_scu_rsrc_table *data)
>  {
> @@ -215,7 +225,7 @@ int imx_clk_scu_init(struct device_node *np,
>  		rsrc_table = data;
>  	}
>  
> -	return platform_driver_register(&imx_clk_scu_driver);
> +	return 0;
>  }
>  
>  /*
> diff --git a/drivers/clk/imx/clk-scu.h b/drivers/clk/imx/clk-scu.h
> index af7b697f51ca..ca82f2cce897 100644
> --- a/drivers/clk/imx/clk-scu.h
> +++ b/drivers/clk/imx/clk-scu.h
> @@ -25,6 +25,8 @@ extern const struct imx_clk_scu_rsrc_table imx_clk_scu_rsrc_imx8dxl;
>  extern const struct imx_clk_scu_rsrc_table imx_clk_scu_rsrc_imx8qxp;
>  extern const struct imx_clk_scu_rsrc_table imx_clk_scu_rsrc_imx8qm;
>  
> +int __init imx_clk_scu_module_init(void);
> +void __exit imx_clk_scu_module_exit(void);
>  int imx_clk_scu_init(struct device_node *np,
>  		     const struct imx_clk_scu_rsrc_table *data);
>  struct clk_hw *imx_scu_of_clk_src_get(struct of_phandle_args *clkspec,
> 
> base-commit: 192c0159402e6bfbe13de6f8379546943297783d
> 


-- 
TQ-Systems GmbH | Mühlstraße 2, Gut Delling | 82229 Seefeld, Germany
Amtsgericht München, HRB 105018
Geschäftsführer: Detlef Schneider, Rüdiger Stahl, Stefan Schneider
http://www.tq-group.com/

Re: [PATCH] clk: scu/imx8qxp: do not register driver in probe()

[Daniel Baluta]

On 2/11/26 16:43, Alexander Stein wrote:
> Hi Danilo,
>
> Am Mittwoch, 11. Februar 2026, 15:23:16 CET schrieb Danilo Krummrich:
>> imx_clk_scu_init() registers the imx_clk_scu_driver while commonly being
>> called from IMX driver's probe() callbacks.
>>
>> However, it neither makes sense to register drivers from probe()
>> callbacks of other drivers, nor does the driver core allow registering
>> drivers with a device lock already being held.
>>
>> The latter was revealed by commit dc23806a7c47 ("driver core: enforce
>> device_lock for driver_match_device()") leading to a deadlock condition
>> described in [1].
>>
>> Additionally, nothing seems to unregister the imx_clk_scu_driver once
>> the corresponding driver module is unloaded, which leaves the
>> driver-core with a dangling pointer.
>>
>> Hence, register the imx_clk_scu_driver from module_init() and unregister
>> it in module_exit().
>>
>> Fixes: dc23806a7c47 ("driver core: enforce device_lock for driver_match_device()")
>> Fixes: 220175cd3979 ("clk: imx: scu: fix build break when compiled as modules")
>> Reported-by: Alexander Stein <alexander.stein@ew.tq-group.com>
>> Closes: https://lore.kernel.org/lkml/13955113.uLZWGnKmhe@steina-w/
>> Link: https://lore.kernel.org/lkml/DFU7CEPUSG9A.1KKGVW4HIPMSH@kernel.org/ [1]
>> Signed-off-by: Danilo Krummrich <dakr@kernel.org>
> Thanks for the patch.
> Tested-by: Alexander Stein <alexander.stein@ew.tq-group.com> # TQMa8x/MBa8x
>
>> ---
>> I plan to send the driver-core PR for 7.0-rc1 soon, which will also contain
>> commit dc23806a7c47 ("driver core: enforce device_lock for
>> driver_match_device()").
>>
>> It also contains an IOMMU commit similar to this one:
>> https://patch.msgid.link/20260121141215.29658-1-dakr@kernel.org
>>
>> The commit was originally scheduled for 6.19-rc7, but was deferred to 7.0-rc1 in
>> case more of those cases pop up (which now happened).
>> ---
>>  drivers/clk/imx/clk-imx8qxp.c | 24 +++++++++++++++++++++++-
>>  drivers/clk/imx/clk-scu.c     | 12 +++++++++++-
>>  drivers/clk/imx/clk-scu.h     |  2 ++
>>  3 files changed, 36 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/clk/imx/clk-imx8qxp.c b/drivers/clk/imx/clk-imx8qxp.c
>> index 3ae162625bb1..d89a2f40771e 100644
>> --- a/drivers/clk/imx/clk-imx8qxp.c
>> +++ b/drivers/clk/imx/clk-imx8qxp.c
>> @@ -346,7 +346,29 @@ static struct platform_driver imx8qxp_clk_driver = {
>>  	},
>>  	.probe = imx8qxp_clk_probe,
>>  };
>> -module_platform_driver(imx8qxp_clk_driver);
>> +
>> +static int __init imx8qxp_init(void)
>>
>>
I would call this imx8qxp_clk_init. Same for imx8qxp_exit.


>> +{
>> +	int ret;
>> +
>> +	ret = platform_driver_register(&imx8qxp_clk_driver);
>> +	if (ret)
>> +		return ret;
>> +
>> +	ret = imx_clk_scu_module_init();
>> +	if (ret)
>> +		platform_driver_unregister(&imx8qxp_clk_driver);
>> +
>> +	return ret;
>>
Also, because the logical flow is that CLK driver is uing SCU for calls I would first call
imx_clk_scu_module_init and then register the imx8qxp_clk driver. 

But there is no functionality issues your your approach too, just a better logical flow.

Thanks,
Daniel.

Re: [PATCH] clk: scu/imx8qxp: do not register driver in probe()

[Danilo Krummrich]

On Wed Feb 11, 2026 at 3:59 PM CET, Daniel Baluta wrote:
> On 2/11/26 16:43, Alexander Stein wrote:
>> Am Mittwoch, 11. Februar 2026, 15:23:16 CET schrieb Danilo Krummrich:
>>> imx_clk_scu_init() registers the imx_clk_scu_driver while commonly being
>>> called from IMX driver's probe() callbacks.
>>>
>>> However, it neither makes sense to register drivers from probe()
>>> callbacks of other drivers, nor does the driver core allow registering
>>> drivers with a device lock already being held.
>>>
>>> The latter was revealed by commit dc23806a7c47 ("driver core: enforce
>>> device_lock for driver_match_device()") leading to a deadlock condition
>>> described in [1].
>>>
>>> Additionally, nothing seems to unregister the imx_clk_scu_driver once
>>> the corresponding driver module is unloaded, which leaves the
>>> driver-core with a dangling pointer.

Actually, this fixes a third bug: If there are multiple matching devices for the
imx8qxp_clk_driver, imx8qxp_clk_probe() calls imx_clk_scu_init() multiple times.
However, any subsequent call after the first one will fail, since the driver
core does not allow to register the same struct platform_driver multiple times.

>>> diff --git a/drivers/clk/imx/clk-imx8qxp.c b/drivers/clk/imx/clk-imx8qxp.c
>>> index 3ae162625bb1..d89a2f40771e 100644
>>> --- a/drivers/clk/imx/clk-imx8qxp.c
>>> +++ b/drivers/clk/imx/clk-imx8qxp.c
>>> @@ -346,7 +346,29 @@ static struct platform_driver imx8qxp_clk_driver = {
>>>  	},
>>>  	.probe = imx8qxp_clk_probe,
>>>  };
>>> -module_platform_driver(imx8qxp_clk_driver);
>>> +
>>> +static int __init imx8qxp_init(void)
>>>
>>>
> I would call this imx8qxp_clk_init. Same for imx8qxp_exit.

Sure.

>>> +{
>>> +	int ret;
>>> +
>>> +	ret = platform_driver_register(&imx8qxp_clk_driver);
>>> +	if (ret)
>>> +		return ret;
>>> +
>>> +	ret = imx_clk_scu_module_init();
>>> +	if (ret)
>>> +		platform_driver_unregister(&imx8qxp_clk_driver);
>>> +
>>> +	return ret;
>>>
> Also, because the logical flow is that CLK driver is uing SCU for calls I would first call
> imx_clk_scu_module_init and then register the imx8qxp_clk driver. 
>
> But there is no functionality issues your your approach too, just a better logical flow.

Sure, I will send a v2 shortly, as it would be good to get this into an early
-fixes PR.

Thanks,
Danilo

Re: [PATCH] clk: scu/imx8qxp: do not register driver in probe()

[Danilo Krummrich]

On Wed Feb 11, 2026 at 3:59 PM CET, Daniel Baluta wrote:
> On 2/11/26 16:43, Alexander Stein wrote:
>> Am Mittwoch, 11. Februar 2026, 15:23:16 CET schrieb Danilo Krummrich:
>>> +{
>>> +	int ret;
>>> +
>>> +	ret = platform_driver_register(&imx8qxp_clk_driver);
>>> +	if (ret)
>>> +		return ret;
>>> +
>>> +	ret = imx_clk_scu_module_init();
>>> +	if (ret)
>>> +		platform_driver_unregister(&imx8qxp_clk_driver);
>>> +
>>> +	return ret;
>>>
> Also, because the logical flow is that CLK driver is uing SCU for calls I would first call
> imx_clk_scu_module_init and then register the imx8qxp_clk driver. 
>
> But there is no functionality issues your your approach too, just a better logical flow.

I now remember why I kept it this way around. If we swap it we have to call
imx_clk_scu_module_exit() in the unwind path of imx8qxp_clk_init().
Consequently, we'd have to drop __exit from imx_clk_scu_module_exit().

Please let me know what you prefer.

Thanks,
Danilo

Re: [PATCH] clk: scu/imx8qxp: do not register driver in probe()

[Daniel Baluta]

On Thu, Feb 12, 2026 at 7:22 PM Danilo Krummrich <dakr@kernel.org> wrote:
>
> On Wed Feb 11, 2026 at 3:59 PM CET, Daniel Baluta wrote:
> > On 2/11/26 16:43, Alexander Stein wrote:
> >> Am Mittwoch, 11. Februar 2026, 15:23:16 CET schrieb Danilo Krummrich:
> >>> +{
> >>> +   int ret;
> >>> +
> >>> +   ret = platform_driver_register(&imx8qxp_clk_driver);
> >>> +   if (ret)
> >>> +           return ret;
> >>> +
> >>> +   ret = imx_clk_scu_module_init();
> >>> +   if (ret)
> >>> +           platform_driver_unregister(&imx8qxp_clk_driver);
> >>> +
> >>> +   return ret;
> >>>
> > Also, because the logical flow is that CLK driver is uing SCU for calls I would first call
> > imx_clk_scu_module_init and then register the imx8qxp_clk driver.
> >
> > But there is no functionality issues your your approach too, just a better logical flow.
>
> I now remember why I kept it this way around. If we swap it we have to call
> imx_clk_scu_module_exit() in the unwind path of imx8qxp_clk_init().
> Consequently, we'd have to drop __exit from imx_clk_scu_module_exit().
>
> Please let me know what you prefer.

I see. I prefer your initial implementation then maybe with an
explanation in the commit message.