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

[Peng Fan <peng.fan@oss.nxp.com>]

Sorry for late reply, just back from holiday.

On Fri, Feb 13, 2026 at 12:58:11AM +0100, Danilo Krummrich wrote:
>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].
>
>Besides that, 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.
>
>Also, 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.
>
>Hence, register the imx_clk_scu_driver from module_init() and unregister
>it in module_exit().
>
>Note that we first register the imx8qxp_clk_driver and then call
>imx_clk_scu_module_init() to avoid having to call
>imx_clk_scu_module_exit() in the unwind path of imx8qxp_clk_init().
>
>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/
>Tested-by: Alexander Stein <alexander.stein@ew.tq-group.com> # TQMa8x/MBa8x
>Link: https://lore.kernel.org/lkml/DFU7CEPUSG9A.1KKGVW4HIPMSH@kernel.org/ [1]
>Signed-off-by: Danilo Krummrich <dakr@kernel.org>
>---
>Changes in v2:
>  - Expand commit message, mentioning a third bug fixed by this patch and noting
>    the order of platform_driver_register() and imx_clk_scu_module_init() in
>    imx8qxp_clk_init().
>  - Rename imx8qxp_{init,exit} to imx8qxp_clk_{init,exit}.
>  - Link to v1: https://lore.kernel.org/driver-core/20260211142321.55404-1-dakr@kernel.org/
>---
> 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..c781425a005e 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_clk_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_clk_init);
>+
>+static void __exit imx8qxp_clk_exit(void)
>+{
>+	imx_clk_scu_module_exit();
>+	platform_driver_unregister(&imx8qxp_clk_driver);
>+}
>+module_exit(imx8qxp_clk_exit);

The clk driver is critical for the system to work, removing this driver will
make the system not work properly I think, so it may not make too much
value to add an exit function here.

Regards
Peng