Re: [PATCH v2 2/2] pmdomain: imx: scu-pd: allow building as a module
| Newsgroups | dev.linux.lists.imx,org.kernel.vger.linux-devicetree |
|---|---|
| Message-ID | <[email protected]> |
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [High] Changing IMX_SCU_PD to a tristate exposes a critical probe order race condition with the SCU clock driver, leading to unpowered hardware and system crashes. -- --- Patch [2]: [PATCH v2 2/2] pmdomain: imx: scu-pd: allow building as a module --- commit d3bf08bb865becc5d6316a0d9e70a9dbcc5e5ef3 Author: Zhipeng Wang <[email protected]> pmdomain: imx: scu-pd: allow building as a module This commit converts CONFIG_IMX_SCU_PD to tristate to allow building as a loadable module, which is required for Android Generic Kernel Image (GKI) support. It adds MODULE_DEVICE_TABLE and a module_init function since the driver cannot be safely removed. > diff --git a/drivers/pmdomain/imx/Kconfig b/drivers/pmdomain/imx/Kconfig > index 00203615c65e9..472e9dec6bd4e 100644 > --- a/drivers/pmdomain/imx/Kconfig > +++ b/drivers/pmdomain/imx/Kconfig > @@ -21,7 +21,7 @@ config IMX9_BLK_CTRL > depends on PM_GENERIC_DOMAINS > > config IMX_SCU_PD > - bool "IMX SCU Power Domain driver" > + tristate "IMX SCU Power Domain driver" [Severity: High] Does allowing this driver to be built as a module expose a probe order race condition with the SCU clock driver? If the power domain driver is loaded asynchronously after the SCU clock driver (clk-imx8qxp), the clock-controller DT node lacks a power-domains property, so fw_devlink cannot enforce the correct probe order. In this scenario, the clock driver manually looks up the power domain provider using of_find_compatible_node() and calls of_genpd_add_device(). If the power domain driver has not probed yet, genpd_get_from_provider() will fail and return -ENOENT. The clock driver ignores this error in imx_clk_scu_alloc_dev() and registers the clock device without attaching it to the PM domain. When the clock is later requested, the Common Clock Framework's pm_runtime_resume_and_get() will succeed passively because there is no attached PM domain. Can this lead to system crashes or hardware access faults due to accessing unpowered hardware? -- Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=2