Re: [PATCH] clk: scu/imx8qxp: do not register driver in probe()
From: Daniel Baluta <hidden>
Date: 2026-02-11 14:56:31
Also in:
driver-core, imx, linux-clk, lkml
On 2/11/26 16:43, Alexander Stein wrote:
Hi Danilo, Am Mittwoch, 11. Februar 2026, 15:23:16 CET schrieb Danilo Krummrich:quoted
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 <redacted> Closes: https://lore.kernel.org/lkml/13955113.uLZWGnKmhe@steina-w/ (local) Link: https://lore.kernel.org/lkml/DFU7CEPUSG9A.1KKGVW4HIPMSH@kernel.org/ (local) [1] Signed-off-by: Danilo Krummrich <dakr@kernel.org>Thanks for the patch. Tested-by: Alexander Stein <redacted> # TQMa8x/MBa8xquoted
--- 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.
quoted
+{ + 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.