[PATCH v2] clk: mediatek: mt8173-mfgtop: do not use the clk API from power_on
From: Ryan Brue
Date: Sun Sep 13 2026 - 17:48:12 EST
clk_mt8173_mfgtop_power_on() calls clk_prepare_enable() on the mfg_26m gate
that this same driver provides. That is a layering inversion which cannot
work: clk_core_prepare() calls clk_pm_runtime_get() on the clock provider
-- this driver's own device -- and genpd power transitions run at _noirq
time, where runtime PM is disabled and pm_runtime_resume_and_get() returns
-EACCES.
Measured on mt8173 (amazon-suez) during resume from suspend-to-RAM,
immediately after the secondary CPUs come back up:
clk-mt8173-mfgtop 13fff000.clock-controller: 26 MHz clock enable failed: -13
The failure is not survivable, because genpd_sync_power_on() ignores what
power_on() returns and marks the domain on regardless. The clock is then
left ungated-but-unprepared, and the next power_off() underflows its
refcount:
mfg_26m already disabled
mfg_26m already unprepared
WARNING: drivers/clk/clk.c:1048 at clk_core_unprepare
Workqueue: pm genpd_power_off_work_fn
clk_unprepare / clk_mt8173_mfgtop_power_off / _genpd_power_off
Gate the bit with a direct regmap write instead. The vendor driver
(mtk_mfgsys.c) also drives the four MFG CG bits with raw register writes
from its power sequencing. The runtime-PM path was never affected, only the
_noirq transitions of system suspend and resume. mfg_26m stays registered
as a clock for any consumer that wants it; nothing currently does.
Because the gate is now driven from the domain callbacks, its enable count
in the clock framework stays 0 while the domain is on. Mark it
CLK_IGNORE_UNUSED so that clk_disable_unused() does not gate the 26 MHz
clock under an already-powered domain when a consumer probes before late
init; the domain callbacks are the only writers.
Fixes: ebd0b73d2137 ("clk: mediatek: Add mt8173-mfgtop driver")
Assisted-by: LLM
Signed-off-by: Ryan Brue <ryanbrue.dev@xxxxxxxxx>
---
Changes in v2:
- Mark mfg_26m CLK_IGNORE_UNUSED: with the gate driven from the domain
callbacks its CCF enable count stays 0, so clk_disable_unused() would gate
the 26 MHz clock under an already-powered domain if a consumer probed
before late init (reported by the Sashiko review of v1). Verified on the
Fire HD 10: the gate follows the domain (hardware Y with the GPU active, N
after unbind) while the enable count stays 0, and the GPU keeps working.
- Link to v1: https://patch.msgid.link/20260912-mfgtop-no-clk-api-power-on-v1-1-def157ad1277@xxxxxxxxx
---
drivers/clk/mediatek/clk-mt8173-mfgtop.c | 29 +++++++++++------------------
1 file changed, 11 insertions(+), 18 deletions(-)
diff --git a/drivers/clk/mediatek/clk-mt8173-mfgtop.c b/drivers/clk/mediatek/clk-mt8173-mfgtop.c
index 5669ca9954dc..53918ad7f402 100644
--- a/drivers/clk/mediatek/clk-mt8173-mfgtop.c
+++ b/drivers/clk/mediatek/clk-mt8173-mfgtop.c
@@ -30,6 +30,8 @@ static const struct mtk_gate_regs mfg_cg_regs = {
.set_ofs = 0x0004,
};
+#define MFG_CG_26M BIT(3)
+
#define GATE_MFG(_id, _name, _parent, _shift, _flags) \
GATE_MTK_FLAGS(_id, _name, _parent, &mfg_cg_regs, _shift, &mtk_clk_gate_ops_setclr, _flags)
@@ -38,7 +40,8 @@ static const struct mtk_gate mfg_clks[] = {
GATE_MFG(CLK_MFG_AXI, "mfg_axi", "axi_mfg_in_sel", 0, CLK_SET_RATE_PARENT),
GATE_MFG(CLK_MFG_MEM, "mfg_mem", "mem_mfg_in_sel", 1, CLK_SET_RATE_PARENT),
GATE_MFG(CLK_MFG_G3D, "mfg_g3d", "mfg_sel", 2, CLK_SET_RATE_PARENT),
- GATE_MFG(CLK_MFG_26M, "mfg_26m", "clk26m", 3, 0),
+ /* Driven by the power domain callbacks, not by a clk consumer */
+ GATE_MFG(CLK_MFG_26M, "mfg_26m", "clk26m", 3, CLK_IGNORE_UNUSED),
};
struct mt8173_mfgtop_data {
@@ -46,7 +49,6 @@ struct mt8173_mfgtop_data {
struct regmap *regmap;
struct generic_pm_domain genpd;
struct of_phandle_args parent_pd, child_pd;
- struct clk *clk_26m;
};
/* Delay count in clock cycles */
@@ -66,12 +68,12 @@ struct mt8173_mfgtop_data {
static int clk_mt8173_mfgtop_power_on(struct generic_pm_domain *domain)
{
struct mt8173_mfgtop_data *data = container_of(domain, struct mt8173_mfgtop_data, genpd);
- int ret;
- /* drives internal power management */
- ret = clk_prepare_enable(data->clk_26m);
- if (ret)
- return ret;
+ /*
+ * Drives internal power management. Written directly: the clk API
+ * needs runtime PM, which is disabled during _noirq transitions.
+ */
+ regmap_write(data->regmap, mfg_cg_regs.clr_ofs, MFG_CG_26M);
/* Power on/off delays for various signals */
regmap_write(data->regmap, MFG_ACTIVE_POWER_CON0,
@@ -103,7 +105,7 @@ static int clk_mt8173_mfgtop_power_off(struct generic_pm_domain *domain)
regmap_write(data->regmap, 0xec, 0);
/* drives internal power management */
- clk_disable_unprepare(data->clk_26m);
+ regmap_write(data->regmap, mfg_cg_regs.set_ofs, MFG_CG_26M);
return 0;
}
@@ -155,16 +157,10 @@ static int clk_mt8173_mfgtop_probe(struct platform_device *pdev)
goto put_pm_runtime;
}
- data->clk_26m = clk_hw_get_clk(data->clk_data->hws[CLK_MFG_26M], "26m");
- if (IS_ERR(data->clk_26m)) {
- ret = dev_err_probe(dev, PTR_ERR(data->clk_26m), "Failed to get 26 MHz clock\n");
- goto unregister_clks;
- }
-
ret = of_clk_add_hw_provider(node, of_clk_hw_onecell_get, data->clk_data);
if (ret) {
dev_err_probe(dev, ret, "Failed to add clk OF provider\n");
- goto put_26m_clk;
+ goto unregister_clks;
}
data->genpd.name = "mfg-top";
@@ -197,8 +193,6 @@ static int clk_mt8173_mfgtop_probe(struct platform_device *pdev)
pm_genpd_remove(&data->genpd);
del_clk_provider:
of_clk_del_provider(node);
-put_26m_clk:
- clk_put(data->clk_26m);
unregister_clks:
mtk_clk_unregister_gates(mfg_clks, ARRAY_SIZE(mfg_clks), data->clk_data);
put_pm_runtime:
@@ -217,7 +211,6 @@ static void clk_mt8173_mfgtop_remove(struct platform_device *pdev)
of_genpd_del_provider(node);
pm_genpd_remove(&data->genpd);
of_clk_del_provider(node);
- clk_put(data->clk_26m);
mtk_clk_unregister_gates(mfg_clks, ARRAY_SIZE(mfg_clks), data->clk_data);
of_node_put(data->parent_pd.np);
}
---
base-commit: df2908090cda368b01ff43709f51890076c56157
change-id: 20260912-mfgtop-no-clk-api-power-on-a5e0a3a43b33
Best regards,
--
Ryan Brue <ryanbrue.dev@xxxxxxxxx>