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

PATCH Superseded Danilo Krummrich 4 participants 6 messages Feb 11 – Feb 12

Subsystems (4)

Arm i.MX and Layerscape Common clk Driver core NXP i.MX clock drivers

lore

Share & tools

lore

Danilo Krummrich

author maintainer

2026-02-11 14:23 UTC #0 lore raw

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

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 <[email protected]>

Closes https://lore.kernel.org/lkml/13955113.uLZWGnKmhe@steina-w/ open here

Link https://lore.kernel.org/lkml/[email protected]/ [1] open here

Signed-off-by Danilo Krummrich <[email protected]>

---
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/[email protected]

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

drivers/clk/imx/clk-imx8qxp.c +23 -1

index 3ae162625bb1..d89a2f40771e 100644
@@ -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 <[email protected]>");
 MODULE_DESCRIPTION("NXP i.MX8QXP clock driver");

drivers/clk/imx/clk-scu.c +11 -1

index 34c9dc1fb20e..c90d21e05f91 100644
@@ -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;
 }
 
 /*

drivers/clk/imx/clk-scu.h +2 -0

index af7b697f51ca..ca82f2cce897 100644
@@ -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

Signature · 1 line

2.52.0

Alexander Stein

Tested-by on v1 1/1

2026-02-11 14:44 UTC #1 lore raw

Hi Danilo,

Am Mittwoch, 11. Februar 2026, 15:23:16 CET schrieb Danilo Krummrich:

Quoted 18 lines

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

Quoted 6 lines

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 <[email protected]>
Closes https://lore.kernel.org/lkml/13955113.uLZWGnKmhe@steina-w/ open here
Link https://lore.kernel.org/lkml/[email protected]/ [1] open here
Signed-off-by Danilo Krummrich <[email protected]>

Thanks for the patch.

Tested-by Alexander Stein <[email protected]> # TQMa8x/MBa8x

Quoted 97 lines

---
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/[email protected]

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

drivers/clk/imx/clk-imx8qxp.c +23 -1

index 3ae162625bb1..d89a2f40771e 100644
@@ -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 <[email protected]>");
 MODULE_DESCRIPTION("NXP i.MX8QXP clock driver");

drivers/clk/imx/clk-scu.c +11 -1

index 34c9dc1fb20e..c90d21e05f91 100644
@@ -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;
 }
 
 /*

drivers/clk/imx/clk-scu.h +2 -0

index af7b697f51ca..ca82f2cce897 100644
@@ -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

Signature · 4 lines

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/

Daniel Baluta

2026-02-11 14:56 UTC #2 lore raw

Quoted 46 lines · Alexander Stein

Hi Danilo,
Am Mittwoch, 11. Februar 2026, 15:23:16 CET schrieb Danilo Krummrich:
Quoted 18 lines
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().
Quoted 6 lines
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 <[email protected]>
Closes https://lore.kernel.org/lkml/13955113.uLZWGnKmhe@steina-w/ open here
Link https://lore.kernel.org/lkml/[email protected]/ [1] open here
Signed-off-by Danilo Krummrich <[email protected]>
Thanks for the patch.
Tested-by Alexander Stein <[email protected]> # TQMa8x/MBa8x
Quoted 16 lines
---
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/[email protected]
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(-)

Quoted 13 lines

drivers/clk/imx/clk-imx8qxp.c static struct platform_driver imx8qxp_clk_driver = { · line 352
Quoted 13 lines
drivers/clk/imx/clk-imx8qxp.c +2 -1
index 3ae162625bb1..d89a2f40771e 100644
@@ -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.

Quoted 5 lines

Quoted 5 lines
drivers/clk/imx/clk-imx8qxp.c +5 -0
+{
+	int ret;
+
+	ret = platform_driver_register(&imx8qxp_clk_driver);
+	if (ret)

Quoted 8 lines

drivers/clk/imx/clk-imx8qxp.c static struct platform_driver imx8qxp_clk_driver = { · line 372
Quoted 8 lines
drivers/clk/imx/clk-imx8qxp.c +7 -0
+		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.

Danilo Krummrich

author maintainer

2026-02-12 10:58 UTC #3 lore raw

Quoted 9 lines · Daniel Baluta

Quoted 8 lines · Alexander Stein
Am Mittwoch, 11. Februar 2026, 15:23:16 CET schrieb Danilo Krummrich:
Quoted 7 lines
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.

Quoted 7 lines

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

Quoted 6 lines

Quoted 6 lines
Quoted 6 lines
drivers/clk/imx/clk-imx8qxp.c +0 -0
index 3ae162625bb1..d89a2f40771e 100644
@@ -346,7 +346,29 @@ static struct platform_driver imx8qxp_clk_driver = {
 	},

Quoted 8 lines

Quoted 7 lines
drivers/clk/imx/clk-imx8qxp.c static struct platform_driver imx8qxp_clk_driver = {
Quoted 7 lines
drivers/clk/imx/clk-imx8qxp.c +2 -1
 	.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.

Quoted 16 lines

Quoted 5 lines
Quoted 5 lines
drivers/clk/imx/clk-imx8qxp.c +5 -0
+{
+	int ret;
+
+	ret = platform_driver_register(&imx8qxp_clk_driver);
+	if (ret)
Quoted 8 lines
drivers/clk/imx/clk-imx8qxp.c static struct platform_driver imx8qxp_clk_driver = { · line 372
Quoted 8 lines
drivers/clk/imx/clk-imx8qxp.c +7 -0
+		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.

Quoted 1 line

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

Danilo Krummrich

author maintainer

2026-02-12 17:22 UTC #4 lore raw

In reply to Daniel Baluta

Quoted 18 lines · Daniel Baluta

Quoted 14 lines · Alexander Stein
Am Mittwoch, 11. Februar 2026, 15:23:16 CET schrieb Danilo Krummrich:
drivers/clk/imx/clk-imx8qxp.c static struct platform_driver imx8qxp_clk_driver = { · line 362
Quoted 13 lines
drivers/clk/imx/clk-imx8qxp.c +12 -0
+{
+	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.

Quoted 1 line

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

Daniel Baluta

2026-02-12 19:43 UTC #5 lore raw

Quoted 20 lines · Danilo Krummrich

Quoted 18 lines · Daniel Baluta
Quoted 14 lines · Alexander Stein
Am Mittwoch, 11. Februar 2026, 15:23:16 CET schrieb Danilo Krummrich:
drivers/clk/imx/clk-imx8qxp.c static struct platform_driver imx8qxp_clk_driver = { · line 362
Quoted 13 lines
drivers/clk/imx/clk-imx8qxp.c +12 -0
+{
+   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.

Quoted 7 lines

Quoted 1 line
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.