public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2] clk: scu/imx8qxp: do not register driver in probe()
@ 2026-02-12 23:58 Danilo Krummrich
  2026-02-13  8:58 ` Daniel Baluta
                   ` (3 more replies)
  0 siblings, 4 replies; 11+ messages in thread
From: Danilo Krummrich @ 2026-02-12 23:58 UTC (permalink / raw)
  To: abelvesa, peng.fan, daniel.baluta, mturquette, sboyd, Frank.Li,
	s.hauer, kernel, festevam, gregkh, rafael, hanguidong02
  Cc: driver-core, linux-kernel, linux-clk, imx, linux-arm-kernel,
	Danilo Krummrich, Alexander Stein

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);
 
 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.53.0


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

* Re: [PATCH v2] clk: scu/imx8qxp: do not register driver in probe()
  2026-02-12 23:58 [PATCH v2] clk: scu/imx8qxp: do not register driver in probe() Danilo Krummrich
@ 2026-02-13  8:58 ` Daniel Baluta
  2026-02-23 20:21 ` Danilo Krummrich
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 11+ messages in thread
From: Daniel Baluta @ 2026-02-13  8:58 UTC (permalink / raw)
  To: Danilo Krummrich, abelvesa, peng.fan, mturquette, sboyd, Frank.Li,
	s.hauer, kernel, festevam, gregkh, rafael, hanguidong02
  Cc: driver-core, linux-kernel, linux-clk, imx, linux-arm-kernel,
	Alexander Stein

On 2/13/26 01:58, 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>


Reviewed-by: Daniel Baluta <daniel.baluta@nxp.com>



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

* Re: [PATCH v2] clk: scu/imx8qxp: do not register driver in probe()
  2026-02-12 23:58 [PATCH v2] clk: scu/imx8qxp: do not register driver in probe() Danilo Krummrich
  2026-02-13  8:58 ` Daniel Baluta
@ 2026-02-23 20:21 ` Danilo Krummrich
  2026-02-24  9:57   ` Abel Vesa
  2026-02-25  8:16 ` Peng Fan
  2026-03-18 15:39 ` Abel Vesa
  3 siblings, 1 reply; 11+ messages in thread
From: Danilo Krummrich @ 2026-02-23 20:21 UTC (permalink / raw)
  To: abelvesa, peng.fan, daniel.baluta, mturquette, sboyd, Frank.Li,
	s.hauer, kernel, festevam, gregkh, rafael, hanguidong02
  Cc: driver-core, linux-kernel, linux-clk, imx, linux-arm-kernel,
	Alexander Stein

On Fri Feb 13, 2026 at 12:58 AM CET, 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>

Just a quick reminder now that -rc1 is out; I think it would be good if this
could go to Linus for -rc2.

(Just in case it helps, please let me know if you want me to take it through the
driver-core tree for this purpose.)

Thanks,
Danilo

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

* Re: [PATCH v2] clk: scu/imx8qxp: do not register driver in probe()
  2026-02-23 20:21 ` Danilo Krummrich
@ 2026-02-24  9:57   ` Abel Vesa
  2026-02-24 12:04     ` Danilo Krummrich
  0 siblings, 1 reply; 11+ messages in thread
From: Abel Vesa @ 2026-02-24  9:57 UTC (permalink / raw)
  To: Danilo Krummrich
  Cc: abelvesa, peng.fan, daniel.baluta, mturquette, sboyd, Frank.Li,
	s.hauer, kernel, festevam, gregkh, rafael, hanguidong02,
	driver-core, linux-kernel, linux-clk, imx, linux-arm-kernel,
	Alexander Stein

On 26-02-23 21:21:29, Danilo Krummrich wrote:
> On Fri Feb 13, 2026 at 12:58 AM CET, 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>

Acked-by: Abel Vesa <abelvesa@kernel.org>

> 
> Just a quick reminder now that -rc1 is out; I think it would be good if this
> could go to Linus for -rc2.
> 
> (Just in case it helps, please let me know if you want me to take it through the
> driver-core tree for this purpose.)

Sure. Go ahead.

Thanks!

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

* Re: [PATCH v2] clk: scu/imx8qxp: do not register driver in probe()
  2026-02-24  9:57   ` Abel Vesa
@ 2026-02-24 12:04     ` Danilo Krummrich
  0 siblings, 0 replies; 11+ messages in thread
From: Danilo Krummrich @ 2026-02-24 12:04 UTC (permalink / raw)
  To: Abel Vesa
  Cc: abelvesa, peng.fan, daniel.baluta, mturquette, sboyd, Frank.Li,
	s.hauer, kernel, festevam, gregkh, rafael, hanguidong02,
	driver-core, linux-kernel, linux-clk, imx, linux-arm-kernel,
	Alexander Stein

On Tue Feb 24, 2026 at 10:57 AM CET, Abel Vesa wrote:
> On 26-02-23 21:21:29, Danilo Krummrich wrote:
>> On Fri Feb 13, 2026 at 12:58 AM CET, 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>
>
> Acked-by: Abel Vesa <abelvesa@kernel.org>

Applied to driver-core-linus, thanks!

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

* Re: [PATCH v2] clk: scu/imx8qxp: do not register driver in probe()
  2026-02-12 23:58 [PATCH v2] clk: scu/imx8qxp: do not register driver in probe() Danilo Krummrich
  2026-02-13  8:58 ` Daniel Baluta
  2026-02-23 20:21 ` Danilo Krummrich
@ 2026-02-25  8:16 ` Peng Fan
  2026-02-25 11:31   ` Danilo Krummrich
  2026-03-18 15:39 ` Abel Vesa
  3 siblings, 1 reply; 11+ messages in thread
From: Peng Fan @ 2026-02-25  8:16 UTC (permalink / raw)
  To: Danilo Krummrich
  Cc: abelvesa, peng.fan, daniel.baluta, mturquette, sboyd, Frank.Li,
	s.hauer, kernel, festevam, gregkh, rafael, hanguidong02,
	driver-core, linux-kernel, linux-clk, imx, linux-arm-kernel,
	Alexander Stein

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

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

* Re: [PATCH v2] clk: scu/imx8qxp: do not register driver in probe()
  2026-02-25  8:16 ` Peng Fan
@ 2026-02-25 11:31   ` Danilo Krummrich
  2026-02-27  0:55     ` Peng Fan
  0 siblings, 1 reply; 11+ messages in thread
From: Danilo Krummrich @ 2026-02-25 11:31 UTC (permalink / raw)
  To: Peng Fan
  Cc: abelvesa, peng.fan, daniel.baluta, mturquette, sboyd, Frank.Li,
	s.hauer, kernel, festevam, gregkh, rafael, hanguidong02,
	driver-core, linux-kernel, linux-clk, imx, linux-arm-kernel,
	Alexander Stein

On Wed Feb 25, 2026 at 9:16 AM CET, Peng Fan wrote:
>>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.

The exit function was there before, just hidden by the module_platform_driver()
macro.

If a driver can be built as module, then we have to unregister the driver, when
the module is unloaded.

If this driver should always be builtin, that is a separate change, independent
from this fix.

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

* RE: [PATCH v2] clk: scu/imx8qxp: do not register driver in probe()
  2026-02-25 11:31   ` Danilo Krummrich
@ 2026-02-27  0:55     ` Peng Fan
  0 siblings, 0 replies; 11+ messages in thread
From: Peng Fan @ 2026-02-27  0:55 UTC (permalink / raw)
  To: Danilo Krummrich, Peng Fan (OSS)
  Cc: abelvesa@kernel.org, Daniel Baluta (OSS), mturquette@baylibre.com,
	sboyd@kernel.org, Frank Li, s.hauer@pengutronix.de,
	kernel@pengutronix.de, festevam@gmail.com,
	gregkh@linuxfoundation.org, rafael@kernel.org,
	hanguidong02@gmail.com, driver-core@lists.linux.dev,
	linux-kernel@vger.kernel.org, linux-clk@vger.kernel.org,
	imx@lists.linux.dev, linux-arm-kernel@lists.infradead.org,
	Alexander Stein

> Subject: Re: [PATCH v2] clk: scu/imx8qxp: do not register driver in
> probe()
> 
> On Wed Feb 25, 2026 at 9:16 AM CET, Peng Fan wrote:
> >>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.
> 
> The exit function was there before, just hidden by the
> module_platform_driver() macro.
> 
> If a driver can be built as module, then we have to unregister the driver,
> when the module is unloaded.

I see. thanks!

Regards
Peng

> 
> If this driver should always be builtin, that is a separate change,
> independent from this fix.

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

* Re: [PATCH v2] clk: scu/imx8qxp: do not register driver in probe()
  2026-02-12 23:58 [PATCH v2] clk: scu/imx8qxp: do not register driver in probe() Danilo Krummrich
                   ` (2 preceding siblings ...)
  2026-02-25  8:16 ` Peng Fan
@ 2026-03-18 15:39 ` Abel Vesa
  2026-03-18 15:42   ` Danilo Krummrich
  3 siblings, 1 reply; 11+ messages in thread
From: Abel Vesa @ 2026-03-18 15:39 UTC (permalink / raw)
  To: abelvesa, peng.fan, daniel.baluta, mturquette, sboyd, Frank.Li,
	s.hauer, kernel, festevam, gregkh, rafael, hanguidong02,
	Danilo Krummrich
  Cc: driver-core, linux-kernel, linux-clk, imx, linux-arm-kernel,
	Alexander Stein


On Fri, 13 Feb 2026 00:58:11 +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.
> 
> [...]

Applied, thanks!

[1/1] clk: scu/imx8qxp: do not register driver in probe()
      commit: 93a6d39e78bfa78b2160621aafbb490e063f12d9

Best regards,
-- 
Abel Vesa <abel.vesa@oss.qualcomm.com>

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

* Re: [PATCH v2] clk: scu/imx8qxp: do not register driver in probe()
  2026-03-18 15:39 ` Abel Vesa
@ 2026-03-18 15:42   ` Danilo Krummrich
  2026-03-18 16:37     ` Abel Vesa
  0 siblings, 1 reply; 11+ messages in thread
From: Danilo Krummrich @ 2026-03-18 15:42 UTC (permalink / raw)
  To: Abel Vesa
  Cc: abelvesa, peng.fan, daniel.baluta, mturquette, sboyd, Frank.Li,
	s.hauer, kernel, festevam, gregkh, rafael, hanguidong02,
	driver-core, linux-kernel, linux-clk, imx, linux-arm-kernel,
	Alexander Stein

On Wed Mar 18, 2026 at 4:39 PM CET, Abel Vesa wrote:
>
> On Fri, 13 Feb 2026 00:58:11 +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.
>> 
>> [...]
>
> Applied, thanks!
>
> [1/1] clk: scu/imx8qxp: do not register driver in probe()
>       commit: 93a6d39e78bfa78b2160621aafbb490e063f12d9

This is already in Linus' tree, you gave your ACK in [1] for me to take it
through the driver-core tree.

[1] https://lore.kernel.org/all/66i6cgh5zmz32ikwjvrz3gtezjotltqmfzdai7ybwjhanx3bif@i5g62ziautwq/

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

* Re: [PATCH v2] clk: scu/imx8qxp: do not register driver in probe()
  2026-03-18 15:42   ` Danilo Krummrich
@ 2026-03-18 16:37     ` Abel Vesa
  0 siblings, 0 replies; 11+ messages in thread
From: Abel Vesa @ 2026-03-18 16:37 UTC (permalink / raw)
  To: Danilo Krummrich
  Cc: abelvesa, peng.fan, daniel.baluta, mturquette, sboyd, Frank.Li,
	s.hauer, kernel, festevam, gregkh, rafael, hanguidong02,
	driver-core, linux-kernel, linux-clk, imx, linux-arm-kernel,
	Alexander Stein

On 26-03-18 16:42:49, Danilo Krummrich wrote:
> On Wed Mar 18, 2026 at 4:39 PM CET, Abel Vesa wrote:
> >
> > On Fri, 13 Feb 2026 00:58:11 +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.
> >> 
> >> [...]
> >
> > Applied, thanks!
> >
> > [1/1] clk: scu/imx8qxp: do not register driver in probe()
> >       commit: 93a6d39e78bfa78b2160621aafbb490e063f12d9
> 
> This is already in Linus' tree, you gave your ACK in [1] for me to take it
> through the driver-core tree.

Will drop from my tree then. Sorry about that.

Thanks for pointing this out.

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

end of thread, other threads:[~2026-03-18 16:37 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-02-12 23:58 [PATCH v2] clk: scu/imx8qxp: do not register driver in probe() Danilo Krummrich
2026-02-13  8:58 ` Daniel Baluta
2026-02-23 20:21 ` Danilo Krummrich
2026-02-24  9:57   ` Abel Vesa
2026-02-24 12:04     ` Danilo Krummrich
2026-02-25  8:16 ` Peng Fan
2026-02-25 11:31   ` Danilo Krummrich
2026-02-27  0:55     ` Peng Fan
2026-03-18 15:39 ` Abel Vesa
2026-03-18 15:42   ` Danilo Krummrich
2026-03-18 16:37     ` Abel Vesa

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