Re: [PATCH v4 3/3] clk: imx: imx8qxp: add soft dependency on SCU power domain driver

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

New issues:
- [High] The `MODULE_SOFTDEP` declaration specifies an incorrect module name, rendering the fix completely ineffective.

Pre-existing issues:
- [High] Ignoring -EPROBE_DEFER causes silent device registration without power domains, leading to hardware exceptions.
--

--- Patch [3]: [PATCH v4 3/3] clk: imx: imx8qxp: add soft dependency on SCU power domain driver ---
commit f47fec3b0de49112a338b6cde236c5713303a35f
Author: Zhipeng Wang <[email protected]>

clk: imx: imx8qxp: add soft dependency on SCU power domain driver

This commit adds a soft dependency on the SCU power domain module to ensure
it is loaded before the i.MX8QXP clock driver. This attempts to work around
silent failures when attaching power domains during device initialization.

> diff --git a/drivers/clk/imx/clk-imx8qxp.c b/drivers/clk/imx/clk-imx8qxp.c
> index c781425a005ef..23ef6b959dee8 100644
> --- a/drivers/clk/imx/clk-imx8qxp.c
> +++ b/drivers/clk/imx/clk-imx8qxp.c
> @@ -373,3 +373,4 @@ module_exit(imx8qxp_clk_exit);
>  MODULE_AUTHOR("Aisheng Dong <[email protected]>");
>  MODULE_DESCRIPTION("NXP i.MX8QXP clock driver");
>  MODULE_LICENSE("GPL v2");
> +MODULE_SOFTDEP("pre: imx_scu_pd");

[Severity: High]
Does this soft dependency specify the correct module name?

The Makefile entry obj-$(CONFIG_IMX_SCU_PD) += scu-pd.o typically results
in a module named scu-pd (or scu_pd). When modprobe parses imx_scu_pd,
it won't find the module, ignoring the dependency and rendering the
workaround ineffective.

[Severity: High]
This isn't a bug introduced by this patch, but the patch relies on a module
load workaround rather than fixing the root cause of the probe order issue.

In drivers/clk/imx/clk-scu.c, imx_clk_scu_alloc_dev() silently ignores
errors from imx_clk_scu_attach_pd():

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

If the power domain provider probes after the clock provider or defers its
probe, of_genpd_add_device() returns -EPROBE_DEFER. Because this error
is ignored, clocks are registered without power domains attached, leading to
hardware exceptions or power leaks when those clocks are accessed.

Should the -EPROBE_DEFER error be propagated up the call stack instead?

This would allow the standard kernel deferral mechanism to reliably enforce
the initialization ordering, handling manual module loads and provider probe
deferrals gracefully without relying on MODULE_SOFTDEP.

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