[PATCH] pmdomain: mediatek: Don't power cycle domains on probe deferral
Ryan Brue
ryanbrue.dev at gmail.com
Sun Nov 1 21:44:37 PST 2026
scpsys_add_one_domain() powers each domain on as soon as it is added,
before the probe has looked up the supplies and clocks of the domains
after it. When one of those is not available yet the probe defers, and
the cleanup powers off every domain added so far, whatever state it was
found in. The retry then powers them all on again.
A bootloader that leaves the display scanning out does not survive
that. On the Amazon Fire HD 10 (MT8173), mfg_async's domain-supply comes
from the MT6397 PMIC, which registers after the power controller first
probes, and power cycling the MM domain under the running display hangs
the SoC on almost every boot.
Power the domains on only once all of them have been added, so that a
deferral leaves the hardware as it was found, and on cleanup power off
only the domains this probe has turned on.
Fixes: 59b644b01cf4 ("soc: mediatek: Add MediaTek SCPSYS power domains")
Signed-off-by: Ryan Brue <ryanbrue.dev at gmail.com>
---
Found on an Amazon Fire HD 10 (2017), an MT8173 tablet whose board DTS
is not upstream yet. Its bootloader (LK) leaves DISP_OVL0 scanning out
when it starts the kernel, and the MT6397 regulators register about 50 ms
after the power controller first probes, so the probe often defers.
The deferral was forced for the A/B with a local test knob that returns
-EPROBE_DEFER at the first domain with a domain-supply. Each boot attempt
was counted from the host's USB log, since a hung board is reset by the
watchdog into the same kernel. The hang does not happen when the
display is stopped before the power controller probes, which is what
our downstream kernel does today.
elm, hana and the MT8173 EVB also give mfg_async a domain-supply, from a
DA9211 on I2C, so they probably take the same deferral path. Whether
their firmware leaves anything running across it, I don't know.
Only MT8173 (direct control) was tested. The HW-voter cleanup changes
the same way, powering off only what this probe powered on, but there is
no such hardware here.
This patch was developed with the help of an LLM.
---
With the deferral forced at mfg_async on every boot and the bootloader's
display left running, 61 of 63 boot attempts hung before userspace
without this patch, and none of 5 with it. With the patch, the cleanup
left MM on as the bootloader had left it on every boot; without it, it
powered off all seven domains the probe had turned on, MM included.
---
drivers/pmdomain/mediatek/mtk-pm-domains.c | 60 ++++++++++++++++++------------
1 file changed, 37 insertions(+), 23 deletions(-)
diff --git a/drivers/pmdomain/mediatek/mtk-pm-domains.c b/drivers/pmdomain/mediatek/mtk-pm-domains.c
index f0fa79e01f9c..6a81aadc18f3 100644
--- a/drivers/pmdomain/mediatek/mtk-pm-domains.c
+++ b/drivers/pmdomain/mediatek/mtk-pm-domains.c
@@ -982,12 +982,6 @@ generic_pm_domain *scpsys_add_one_domain(struct scpsys *scpsys, struct device_no
pd->genpd.flags |= GENPD_FLAG_IRQ_SAFE;
}
- /*
- * Initially turn on all domains to make the domains usable
- * with !CONFIG_PM and to get the hardware in sync with the
- * software. The unused domains will be switched off during
- * late_init time.
- */
if (MTK_SCPD_CAPS(pd, MTK_SCPD_KEEP_DEFAULT_OFF)) {
bool domain_is_on;
@@ -999,15 +993,8 @@ generic_pm_domain *scpsys_add_one_domain(struct scpsys *scpsys, struct device_no
if (domain_is_on)
dev_warn(scpsys->dev,
"%pOF: A default off power domain has been ON\n", node);
- } else {
- 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;
- }
-
- if (MTK_SCPD_CAPS(pd, MTK_SCPD_ALWAYS_ON))
- pd->genpd.flags |= GENPD_FLAG_ALWAYS_ON;
+ } else if (MTK_SCPD_CAPS(pd, MTK_SCPD_ALWAYS_ON)) {
+ pd->genpd.flags |= GENPD_FLAG_ALWAYS_ON;
}
if (MTK_SCPD_CAPS(pd, MTK_SCPD_ACTIVE_WAKEUP))
@@ -1085,7 +1072,7 @@ static int scpsys_add_subdomain(struct scpsys *scpsys, struct device_node *paren
return ret;
}
-static void scpsys_remove_one_domain(struct scpsys_domain *pd)
+static void scpsys_remove_one_domain(struct scpsys_domain *pd, bool powered_on)
{
struct scpsys *scpsys = pd->scpsys;
int ret;
@@ -1100,11 +1087,10 @@ static void scpsys_remove_one_domain(struct scpsys_domain *pd)
"failed to remove domain '%s' : %d - state may be inconsistent\n",
pd->genpd.name, ret);
- if (scpsys->soc_data->type == SCPSYS_MTCMOS_TYPE_HW_VOTER) {
- if (scpsys_hwv_domain_is_enable_done(pd))
+ if (powered_on) {
+ if (scpsys->soc_data->type == SCPSYS_MTCMOS_TYPE_HW_VOTER)
scpsys_hwv_power_off_internal(pd);
- } else {
- if (scpsys_domain_is_on(pd) || MTK_SCPD_CAPS(pd, MTK_SCPD_SIMPLE_PWRSEQ))
+ else
scpsys_power_off_internal(pd);
}
@@ -1112,7 +1098,8 @@ static void scpsys_remove_one_domain(struct scpsys_domain *pd)
clk_bulk_put(pd->num_subsys_clks, pd->subsys_clks);
}
-static void scpsys_domain_cleanup(struct scpsys *scpsys, u8 *domains_idx, u8 num_probed)
+static void scpsys_domain_cleanup(struct scpsys *scpsys, u8 *domains_idx,
+ u8 num_probed, u8 num_powered)
{
struct generic_pm_domain *genpd;
struct scpsys_domain *pd;
@@ -1124,7 +1111,8 @@ static void scpsys_domain_cleanup(struct scpsys *scpsys, u8 *domains_idx, u8 num
genpd = scpsys->pd_data.domains[pd_idx];
if (genpd) {
pd = to_scpsys_domain(genpd);
- scpsys_remove_one_domain(pd);
+ scpsys_remove_one_domain(pd, i < num_powered &&
+ !MTK_SCPD_CAPS(pd, MTK_SCPD_KEEP_DEFAULT_OFF));
}
}
}
@@ -1361,6 +1349,7 @@ static int scpsys_probe(struct platform_device *pdev)
struct scpsys *scpsys;
int num_domains, ret;
u8 num_added_pds = 0;
+ u8 num_powered_pds = 0;
u8 *added_pds_idx;
soc = of_device_get_match_data(&pdev->dev);
@@ -1436,6 +1425,31 @@ static int scpsys_probe(struct platform_device *pdev)
return ret;
}
+ /*
+ * Initially turn on all domains to make the domains usable
+ * with !CONFIG_PM and to get the hardware in sync with the
+ * software. The unused domains will be switched off during
+ * late_init time.
+ *
+ * Do this only once every domain has got its supply and clocks,
+ * so that a probe deferral leaves the hardware as it was found:
+ * the bootloader may still be using a domain that is already on.
+ */
+ for (; num_powered_pds < num_added_pds; num_powered_pds++) {
+ struct scpsys_domain *pd;
+
+ pd = to_scpsys_domain(scpsys->domains[added_pds_idx[num_powered_pds]]);
+ if (MTK_SCPD_CAPS(pd, MTK_SCPD_KEEP_DEFAULT_OFF))
+ continue;
+
+ ret = pd->genpd.power_on(&pd->genpd);
+ if (ret < 0) {
+ dev_err(dev, "%s: failed to power on domain: %d\n",
+ pd->genpd.name, ret);
+ goto err_cleanup_domains;
+ }
+ }
+
ret = of_genpd_add_provider_onecell(np, &scpsys->pd_data);
if (ret) {
dev_err(dev, "failed to add provider: %d\n", ret);
@@ -1446,7 +1460,7 @@ static int scpsys_probe(struct platform_device *pdev)
return 0;
err_cleanup_domains:
- scpsys_domain_cleanup(scpsys, added_pds_idx, num_added_pds);
+ scpsys_domain_cleanup(scpsys, added_pds_idx, num_added_pds, num_powered_pds);
return ret;
}
---
base-commit: 6628e7d4c6b25c8d18f1345e3a20d21829aae7e4
change-id: 20261009-rbrue-suez-upstreaming-mtk-pmdomain-defer-0cc87eb82400
Best regards,
--
Ryan Brue <ryanbrue.dev at gmail.com>
More information about the linux-arm-kernel
mailing list