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

["Danilo Krummrich" <dakr@kernel.org>]

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