Skip to contentSkip to conversation

pmdomain: mediatek: Break lock dependency to `prepare_lock`

PATCH Superseded Tzung-Bi Shih 2 participants 2 messages Dec 29, 2025 – Dec 30, 2025

Subsystems (2)
Arm MediaTek Generic PM domains

v1 of 2 Superseded Latest is v2 →

lore
Share & tools
lore

Tzung-Bi Shih

author
#0 lore raw

PATCH 1/1 pmdomain: mediatek: Break lock dependency to `prepare_lock`

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.

Quoted 9 lines

-> #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.

Quoted 4 lines

-> #1 (&genpd->mlock){+.+.}-{3:3}:
__mutex_lock_common
mutex_lock_nested
genpd_lock_mtx

Quoted 8 lines

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.

Quoted 8 lines
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 <[email protected]>
---
 drivers/pmdomain/mediatek/mtk-pm-domains.c | 99 +++++++++++++++++-----
 1 file changed, 79 insertions(+), 20 deletions(-)
drivers/pmdomain/mediatek/mtk-pm-domains.c +79 -20
index 80561d27f2b2..2da0f62988c0 100644
@@ -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,59 @@ static int scpsys_probe(struct platform_device *pdev)
 	return ret;
 }
 
+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;
+}
+
+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);
Signature · 1 line
2.52.0.351.gbe84eed79e-goog
build bot
#1 lore raw
kernel test robot noticed the following build warnings:

Hi Tzung-Bi,

kernel test robot noticed the following build warnings:

[auto build test WARNING on linus/master]
[also build test WARNING on v6.19-rc3 next-20251219]
[cannot apply to amd-pstate/linux-next amd-pstate/bleeding-edge] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Tzung-Bi-Shih/pmdomain-mediatek-Break-lock-dependency-to-prepare_lock/20251229-123525
base:   linus/master
patch link:    https://lore.kernel.org/r/20251229043244.4103262-1-tzungbi%40kernel.org
patch subject: [PATCH] pmdomain: mediatek: Break lock dependency to `prepare_lock`
config: arm-randconfig-003-20251230 (https://download.01.org/0day-ci/archive/20251230/[email protected]/config)
compiler: arm-linux-gnueabi-gcc (GCC) 10.5.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20251230/[email protected]/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <[email protected]>
| Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/

All warnings (new ones prefixed by >>):

>> drivers/pmdomain/mediatek/mtk-pm-domains.c:1304:12: warning: 'scpsys_resume' defined but not used [-Wunused-function]
    1304 | static int scpsys_resume(struct device *dev)
         |            ^~~~~~~~~~~~~
>> drivers/pmdomain/mediatek/mtk-pm-domains.c:1285:12: warning: 'scpsys_suspend' defined but not used [-Wunused-function]

1285 | static int scpsys_suspend(struct device *dev)
| ^~~~~~~~~~~~~~

vim +/scpsys_resume +1304 drivers/pmdomain/mediatek/mtk-pm-domains.c

1284

> 1285	static int scpsys_suspend(struct device *dev)
  1286	{
  1287		struct scpsys *scpsys = dev_get_drvdata(dev);
  1288		struct generic_pm_domain *genpd;
  1289		struct scpsys_domain *pd;
  1290		int i;
  1291	
  1292		for (i = 0; i < scpsys->pd_data.num_domains; i++) {
  1293			genpd = scpsys->pd_data.domains[i];
  1294			if (!genpd)
  1295				continue;
  1296	
  1297			pd = to_scpsys_domain(genpd);
  1298			clk_bulk_unprepare(pd->num_clks, pd->clks);
  1299			clk_bulk_unprepare(pd->num_subsys_clks, pd->subsys_clks);
  1300		}
  1301		return 0;
  1302	}
  1303	
> 1304	static int scpsys_resume(struct device *dev)

1305 {
1306 struct scpsys *scpsys = dev_get_drvdata(dev);
1307 struct generic_pm_domain *genpd;
1308 struct scpsys_domain *pd;
1309 int i, ret;
1310
1311 for (i = 0; i < scpsys->pd_data.num_domains; i++) {
1312 genpd = scpsys->pd_data.domains[i];
1313 if (!genpd)
1314 continue;
1315
1316 pd = to_scpsys_domain(genpd);
1317 ret = clk_bulk_prepare(pd->num_clks, pd->clks);
1318 if (ret)
1319 return ret;
1320 ret = clk_bulk_prepare(pd->num_subsys_clks, pd->subsys_clks);
1321 if (ret)
1322 return ret;
1323 }
1324 return 0;
1325 }
1326

Signature · 2 lines
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki