[PATCH v2] pmdomain: mediatek: Break lock dependency to `prepare_lock`
Chen-Yu Tsai
wenst at chromium.org
Thu Jan 22 00:37:55 PST 2026
On Wed, Dec 31, 2025 at 11:54 AM Tzung-Bi Shih <tzungbi at kernel.org> wrote:
>
> Break a circular locking dependency between the Generic Power Domain
> lock (`genpd->mlock`) and the clock framework's `prepare_lock`. Move
> the prepare() to the domain initialization phase and the unprepare()
> to the cleanup phase.
>
> The possible deadlock occurs in the following scenario:
>
> 1. `genpd_power_on` acquires `genpd->mlock` and then calls the driver's
> `scpsys_power_on`. The driver calls `clk_bulk_prepare_enable`,
> which attempts to acquire `prepare_lock`.
>
> > -> #0 (prepare_lock){+.+.}-{3:3}:
> > __lock_acquire
> > lock_acquire
> > __mutex_lock_common
> > mutex_lock_nested
> > clk_prepare
> > clk_bulk_prepare
> > scpsys_power_on
> > genpd_power_on
>
> 2. A clock provider (managed by a power domain) is resumed.
> `clk_prepare` acquires `prepare_lock` and triggers a runtime resume of
> its power domain, which attempts to acquire `genpd->mlock`.
>
> > -> #1 (&genpd->mlock){+.+.}-{3:3}:
> > __mutex_lock_common
> > mutex_lock_nested
> > genpd_lock_mtx
> > genpd_runtime_resume
> > __rpm_callback
> > rpm_callback
> > rpm_resume
> > __pm_runtime_resume
> > clk_core_prepare
> > clk_prepare
> > clk_bulk_prepare
>
> This creates a cycle: `mlock` -> `prepare_lock` -> `mlock`.
>
> > Possible unsafe locking scenario:
> >
> > CPU0 CPU1
> > ---- ----
> > lock(&genpd->mlock);
> > lock(prepare_lock);
> > lock(&genpd->mlock);
> > lock(prepare_lock);
>
> This breaks the dependency chain in #0.
>
> This is a revert of f0fce06e345d ("soc: mtk-pm-domains: Fix the clock
> prepared issue"). However, addressing the issue by moving the
> unprepare()/prepare() to PM suspend()/resume() callbacks.
>
> Signed-off-by: Tzung-Bi Shih <tzungbi at kernel.org>
Reviewed-by: Chen-Yu Tsai <wenst at chromium.org>
Tested-by: Chen-Yu Tsai <wenst at chromium.org> # MT8183 & MT8188 no regressions
This is unfortunately a known problem of the interaction between pmdomains
and the clk subsystem. This *breaks* the circular lock dependency. AFAIU
this works because the clk prepare op is a no-op, because all the clocks
are MMIO based.
There is another question of whether this actually deadlocks or not, i.e.
if the involved &genpd->mlock is the same object or not. Perhaps the
accuracy of lockdep could be improved by setting a different lock class
key and name? See what regmap does.
> ---
> v2:
> - Fix build error reported by "kernel test robot <lkp at intel.com>".
>
> v1: https://lore.kernel.org/all/20251229043244.4103262-1-tzungbi@kernel.org/
>
> drivers/pmdomain/mediatek/mtk-pm-domains.c | 101 +++++++++++++++++----
> 1 file changed, 81 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/pmdomain/mediatek/mtk-pm-domains.c b/drivers/pmdomain/mediatek/mtk-pm-domains.c
> index 80561d27f2b2..c371b08c9170 100644
> --- a/drivers/pmdomain/mediatek/mtk-pm-domains.c
> +++ b/drivers/pmdomain/mediatek/mtk-pm-domains.c
> @@ -318,12 +318,12 @@ static int scpsys_hwv_power_on(struct generic_pm_domain *genpd)
> if (ret)
> goto err_infra;
>
> - ret = clk_bulk_prepare_enable(pd->num_clks, pd->clks);
> + ret = clk_bulk_enable(pd->num_clks, pd->clks);
> if (ret)
> goto err_reg;
>
> /* For HWV the subsys clocks refer to the HWV low power subsystem */
> - ret = clk_bulk_prepare_enable(pd->num_subsys_clks, pd->subsys_clks);
> + ret = clk_bulk_enable(pd->num_subsys_clks, pd->subsys_clks);
> if (ret)
> goto err_disable_clks;
>
> @@ -365,7 +365,7 @@ static int scpsys_hwv_power_on(struct generic_pm_domain *genpd)
> }
>
> /* It's done! Disable the HWV low power subsystem clocks */
> - clk_bulk_disable_unprepare(pd->num_subsys_clks, pd->subsys_clks);
> + clk_bulk_disable(pd->num_subsys_clks, pd->subsys_clks);
>
> if (MTK_SCPD_CAPS(pd, MTK_SCPD_INFRA_PWR_CTL))
> scpsys_sec_infra_power_on(false);
> @@ -373,9 +373,9 @@ static int scpsys_hwv_power_on(struct generic_pm_domain *genpd)
> return 0;
>
> err_disable_subsys_clks:
> - clk_bulk_disable_unprepare(pd->num_subsys_clks, pd->subsys_clks);
> + clk_bulk_disable(pd->num_subsys_clks, pd->subsys_clks);
> err_disable_clks:
> - clk_bulk_disable_unprepare(pd->num_clks, pd->clks);
> + clk_bulk_disable(pd->num_clks, pd->clks);
> err_reg:
> scpsys_regulator_disable(pd->supply);
> err_infra:
> @@ -398,7 +398,7 @@ static int scpsys_hwv_power_off(struct generic_pm_domain *genpd)
> return ret;
> }
>
> - ret = clk_bulk_prepare_enable(pd->num_subsys_clks, pd->subsys_clks);
> + ret = clk_bulk_enable(pd->num_subsys_clks, pd->subsys_clks);
> if (ret)
> goto err_infra;
>
> @@ -437,8 +437,8 @@ static int scpsys_hwv_power_off(struct generic_pm_domain *genpd)
> if (ret)
> goto err_disable_subsys_clks;
>
> - clk_bulk_disable_unprepare(pd->num_subsys_clks, pd->subsys_clks);
> - clk_bulk_disable_unprepare(pd->num_clks, pd->clks);
> + clk_bulk_disable(pd->num_subsys_clks, pd->subsys_clks);
> + clk_bulk_disable(pd->num_clks, pd->clks);
>
> scpsys_regulator_disable(pd->supply);
>
> @@ -448,7 +448,7 @@ static int scpsys_hwv_power_off(struct generic_pm_domain *genpd)
> return 0;
>
> err_disable_subsys_clks:
> - clk_bulk_disable_unprepare(pd->num_subsys_clks, pd->subsys_clks);
> + clk_bulk_disable(pd->num_subsys_clks, pd->subsys_clks);
> err_infra:
> if (MTK_SCPD_CAPS(pd, MTK_SCPD_INFRA_PWR_CTL))
> scpsys_sec_infra_power_on(false);
> @@ -616,7 +616,7 @@ static int scpsys_power_on(struct generic_pm_domain *genpd)
> if (ret)
> return ret;
>
> - ret = clk_bulk_prepare_enable(pd->num_clks, pd->clks);
> + ret = clk_bulk_enable(pd->num_clks, pd->clks);
> if (ret)
> goto err_reg;
>
> @@ -638,8 +638,7 @@ static int scpsys_power_on(struct generic_pm_domain *genpd)
> * access.
> */
> if (!MTK_SCPD_CAPS(pd, MTK_SCPD_STRICT_BUS_PROTECTION)) {
> - ret = clk_bulk_prepare_enable(pd->num_subsys_clks,
> - pd->subsys_clks);
> + ret = clk_bulk_enable(pd->num_subsys_clks, pd->subsys_clks);
> if (ret)
> goto err_pwr_ack;
> }
> @@ -653,8 +652,7 @@ static int scpsys_power_on(struct generic_pm_domain *genpd)
> goto err_disable_sram;
>
> if (MTK_SCPD_CAPS(pd, MTK_SCPD_STRICT_BUS_PROTECTION)) {
> - ret = clk_bulk_prepare_enable(pd->num_subsys_clks,
> - pd->subsys_clks);
> + ret = clk_bulk_enable(pd->num_subsys_clks, pd->subsys_clks);
> if (ret)
> goto err_enable_bus_protect;
> }
> @@ -667,10 +665,9 @@ static int scpsys_power_on(struct generic_pm_domain *genpd)
> scpsys_sram_disable(pd);
> err_disable_subsys_clks:
> if (!MTK_SCPD_CAPS(pd, MTK_SCPD_STRICT_BUS_PROTECTION))
> - clk_bulk_disable_unprepare(pd->num_subsys_clks,
> - pd->subsys_clks);
> + clk_bulk_disable(pd->num_subsys_clks, pd->subsys_clks);
> err_pwr_ack:
> - clk_bulk_disable_unprepare(pd->num_clks, pd->clks);
> + clk_bulk_disable(pd->num_clks, pd->clks);
> err_reg:
> scpsys_regulator_disable(pd->supply);
> return ret;
> @@ -695,7 +692,7 @@ static int scpsys_power_off(struct generic_pm_domain *genpd)
> regmap_set_bits(scpsys->base, pd->data->ext_buck_iso_offs,
> pd->data->ext_buck_iso_mask);
>
> - clk_bulk_disable_unprepare(pd->num_subsys_clks, pd->subsys_clks);
> + clk_bulk_disable(pd->num_subsys_clks, pd->subsys_clks);
>
> if (MTK_SCPD_CAPS(pd, MTK_SCPD_MODEM_PWRSEQ))
> scpsys_modem_pwrseq_off(pd);
> @@ -708,7 +705,7 @@ static int scpsys_power_off(struct generic_pm_domain *genpd)
> if (ret < 0)
> return ret;
>
> - clk_bulk_disable_unprepare(pd->num_clks, pd->clks);
> + clk_bulk_disable(pd->num_clks, pd->clks);
>
> scpsys_regulator_disable(pd->supply);
>
> @@ -855,6 +852,14 @@ generic_pm_domain *scpsys_add_one_domain(struct scpsys *scpsys, struct device_no
> pd->genpd.flags |= GENPD_FLAG_IRQ_SAFE;
> }
>
> + ret = clk_bulk_prepare(pd->num_clks, pd->clks);
> + if (ret)
> + goto err_put_subsys_clocks;
> +
> + ret = clk_bulk_prepare(pd->num_subsys_clks, pd->subsys_clks);
> + if (ret)
> + goto err_unprepare_clocks;
> +
> /*
> * Initially turn on all domains to make the domains usable
> * with !CONFIG_PM and to get the hardware in sync with the
> @@ -869,7 +874,7 @@ generic_pm_domain *scpsys_add_one_domain(struct scpsys *scpsys, struct device_no
> ret = pd->genpd.power_on(&pd->genpd);
> if (ret < 0) {
> dev_err(scpsys->dev, "%pOF: failed to power on domain: %d\n", node, ret);
> - goto err_put_subsys_clocks;
> + goto err_unprepare_subsys_clocks;
> }
>
> if (MTK_SCPD_CAPS(pd, MTK_SCPD_ALWAYS_ON))
> @@ -888,6 +893,10 @@ generic_pm_domain *scpsys_add_one_domain(struct scpsys *scpsys, struct device_no
>
> return scpsys->pd_data.domains[id];
>
> +err_unprepare_subsys_clocks:
> + clk_bulk_unprepare(pd->num_subsys_clks, pd->subsys_clks);
> +err_unprepare_clocks:
> + clk_bulk_unprepare(pd->num_clks, pd->clks);
> err_put_subsys_clocks:
> clk_bulk_put(pd->num_subsys_clks, pd->subsys_clks);
> err_put_clocks:
> @@ -965,6 +974,8 @@ static void scpsys_remove_one_domain(struct scpsys_domain *pd)
> if (scpsys_domain_is_on(pd))
> scpsys_power_off(&pd->genpd);
>
> + clk_bulk_unprepare(pd->num_clks, pd->clks);
> + clk_bulk_unprepare(pd->num_subsys_clks, pd->subsys_clks);
> clk_bulk_put(pd->num_clks, pd->clks);
> clk_bulk_put(pd->num_subsys_clks, pd->subsys_clks);
> }
> @@ -1208,6 +1219,7 @@ static int scpsys_probe(struct platform_device *pdev)
> if (!scpsys)
> return -ENOMEM;
>
> + platform_set_drvdata(pdev, scpsys);
> scpsys->dev = dev;
> scpsys->soc_data = soc;
>
> @@ -1270,12 +1282,61 @@ static int scpsys_probe(struct platform_device *pdev)
> return ret;
> }
>
> +#ifdef CONFIG_PM_SLEEP
> +static int scpsys_suspend(struct device *dev)
> +{
> + struct scpsys *scpsys = dev_get_drvdata(dev);
> + struct generic_pm_domain *genpd;
> + struct scpsys_domain *pd;
> + int i;
> +
> + for (i = 0; i < scpsys->pd_data.num_domains; i++) {
> + genpd = scpsys->pd_data.domains[i];
> + if (!genpd)
> + continue;
> +
> + pd = to_scpsys_domain(genpd);
> + clk_bulk_unprepare(pd->num_clks, pd->clks);
> + clk_bulk_unprepare(pd->num_subsys_clks, pd->subsys_clks);
> + }
> + return 0;
> +}
> +
> +static int scpsys_resume(struct device *dev)
> +{
> + struct scpsys *scpsys = dev_get_drvdata(dev);
> + struct generic_pm_domain *genpd;
> + struct scpsys_domain *pd;
> + int i, ret;
> +
> + for (i = 0; i < scpsys->pd_data.num_domains; i++) {
> + genpd = scpsys->pd_data.domains[i];
> + if (!genpd)
> + continue;
> +
> + pd = to_scpsys_domain(genpd);
> + ret = clk_bulk_prepare(pd->num_clks, pd->clks);
> + if (ret)
> + return ret;
> + ret = clk_bulk_prepare(pd->num_subsys_clks, pd->subsys_clks);
> + if (ret)
> + return ret;
> + }
> + return 0;
> +}
> +#endif
> +
> +static const struct dev_pm_ops scpsys_pm_ops = {
> + SET_SYSTEM_SLEEP_PM_OPS(scpsys_suspend, scpsys_resume)
> +};
> +
> static struct platform_driver scpsys_pm_domain_driver = {
> .probe = scpsys_probe,
> .driver = {
> .name = "mtk-power-controller",
> .suppress_bind_attrs = true,
> .of_match_table = scpsys_of_match,
> + .pm = &scpsys_pm_ops,
> },
> };
> builtin_platform_driver(scpsys_pm_domain_driver);
> --
> 2.52.0.351.gbe84eed79e-goog
>
>
More information about the Linux-mediatek
mailing list