Re: [PATCH v4 2/3] pmdomain: imx: scu-pd: allow building as a module

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Converting the IMX_SCU_PD driver to a loadable module introduces a critical probe ordering race condition that permanently breaks SCU clock initialization.
--

--- Patch [2]: [PATCH v4 2/3] pmdomain: imx: scu-pd: allow building as a module ---
commit 628d94a1fc8f8e8847b2257f097c88ea5e22ffe7
Author: Zhipeng Wang <[email protected]>

pmdomain: imx: scu-pd: allow building as a module

This commit converts the IMX SCU power domain driver to a tristate to
support Android GKI loadable modules. It relies on subsys_initcall() to
preserve probe ordering when built-in.

> diff --git a/drivers/pmdomain/imx/scu-pd.c b/drivers/pmdomain/imx/scu-pd.c
> index 3ec33667a308c..e3f9e741daf76 100644
> --- a/drivers/pmdomain/imx/scu-pd.c
> +++ b/drivers/pmdomain/imx/scu-pd.c
[ ... ]
> @@ -540,7 +541,12 @@ static struct platform_driver imx_sc_pd_driver = {
>  	},
>  	.probe = imx_sc_pd_probe,
>  };
> -builtin_platform_driver(imx_sc_pd_driver);
> +
> +static int __init imx_sc_pd_driver_init(void)
> +{
> +	return platform_driver_register(&imx_sc_pd_driver);
> +}
> +subsys_initcall(imx_sc_pd_driver_init);

[Severity: High]
Does converting this driver to a loadable module break the probe ordering
with the SCU clock driver?

Because subsys_initcall() behaves like module_init() when built as a module,
the power domain and clock modules will be loaded asynchronously by udev.

If the clock provider probes before the power domain module is loaded,
it fails to attach the power domain but proceeds to register the clocks
anyway:

drivers/clk/imx/clk-scu.c:imx_clk_scu_alloc_dev() {
        ...
        ret = imx_clk_scu_attach_pd(&pdev->dev, rsrc_id);
        if (ret)
                pr_warn("%s: failed to attached the power domain %d\n",
                        name, ret);

        ret = platform_device_add(pdev);
        ...
}

Since there is no fw_devlink dependency (the clock controller DT node
lacks a power-domains property) and no symbol dependency, the clock
driver will not defer its probe. This leaves the SCU clocks without
their required power domains, which causes hardware faults when the
clocks are later enabled.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=2