[PATCH] clk: mediatek: pllfh: Fix IO remapping leak in register_pllfhs error path
From: Louis-Alexis Eyraud <hidden>
Date: 2026-07-08 09:19:26
Also in:
linux-clk, linux-mediatek, lkml
Subsystem:
common clk framework, the rest · Maintainers:
Michael Turquette, Stephen Boyd, Linus Torvalds
When mtk_clk_register_pllfhs function fails to register a PLL, it
unregisters all PLLs and cleans up itself in its error path before
returning, so the function callers don't need to do it.
But contrary to mtk_clk_unregister_pllfhs function, that does almost
the same sequence, it does not free the IO memory mapped on fhctl node,
leading to a leak.
Fix this leak by factorizing the cleanup sequence in a new private
function and use it both mtk_clk_register_pllfhs and
mtk_clk_unregister_pllfhs functions.
Also, change the loop index start value to avoid the -1 operation on
index at each loop.
Fixes: d7964de8a8ea ("clk: mediatek: Add new clock driver to handle FHCTL hardware")
Signed-off-by: Louis-Alexis Eyraud <redacted>
---
This patch aims to fix a issue, detected by Sashiko
during another series review ([1]), regarding the use of
mtk_clk_register_pllfhs function.
The patch is based on linux-next tree (tag: next-20260707).
[1]: https://sashiko.dev/#/patchset/20260701-mt8189-clocks-system-base-v1-0-2b048feea50a%40collabora.com?part=6
---
drivers/clk/mediatek/clk-pllfh.c | 87 ++++++++++++++++++----------------------
1 file changed, 39 insertions(+), 48 deletions(-)
diff --git a/drivers/clk/mediatek/clk-pllfh.c b/drivers/clk/mediatek/clk-pllfh.c
index aa95cd9197b3..7463aa35d7e3 100644
--- a/drivers/clk/mediatek/clk-pllfh.c
+++ b/drivers/clk/mediatek/clk-pllfh.c@@ -197,6 +197,43 @@ static void mtk_clk_unregister_pllfh(struct clk_hw *hw) kfree(fh); } +static void mtk_clk_cleanup_pllfhs(const struct mtk_pll_data *plls, int num_plls, + struct mtk_pllfh_data *pllfhs, int num_fhs, + struct clk_hw_onecell_data *clk_data) +{ + void __iomem *base = NULL, *fhctl_base = NULL; + int i; + + for (i = num_plls - 1; i > 0; i--) { + const struct mtk_pll_data *pll = &plls[i]; + struct mtk_pllfh_data *pllfh; + bool use_fhctl; + + if (IS_ERR_OR_NULL(clk_data->hws[pll->id])) + continue; + + pllfh = get_pllfh_by_id(pllfhs, num_fhs, pll->id); + use_fhctl = fhctl_is_supported_and_enabled(pllfh); + + if (use_fhctl) { + fhctl_base = pllfh->state.base; + mtk_clk_unregister_pllfh(clk_data->hws[pll->id]); + } else { + base = mtk_clk_pll_get_base(clk_data->hws[pll->id], + pll); + mtk_clk_unregister_pll(clk_data->hws[pll->id]); + } + + clk_data->hws[pll->id] = ERR_PTR(-ENOENT); + } + + if (fhctl_base) + iounmap(fhctl_base); + + iounmap(base); +} + + int mtk_clk_register_pllfhs(struct device *dev, const struct mtk_pll_data *plls, int num_plls, struct mtk_pllfh_data *pllfhs, int num_fhs,
@@ -238,24 +275,7 @@ int mtk_clk_register_pllfhs(struct device *dev, return 0; err: - while (--i >= 0) { - const struct mtk_pll_data *pll = &plls[i]; - struct mtk_pllfh_data *pllfh; - bool use_fhctl; - - pllfh = get_pllfh_by_id(pllfhs, num_fhs, pll->id); - use_fhctl = fhctl_is_supported_and_enabled(pllfh); - - if (use_fhctl) - mtk_clk_unregister_pllfh(clk_data->hws[pll->id]); - else - mtk_clk_unregister_pll(clk_data->hws[pll->id]); - - clk_data->hws[pll->id] = ERR_PTR(-ENOENT); - } - - iounmap(base); - + mtk_clk_cleanup_pllfhs(plls, i, pllfhs, num_fhs, clk_data); return PTR_ERR(hw); } EXPORT_SYMBOL_GPL(mtk_clk_register_pllfhs);
@@ -264,38 +284,9 @@ void mtk_clk_unregister_pllfhs(const struct mtk_pll_data *plls, int num_plls, struct mtk_pllfh_data *pllfhs, int num_fhs, struct clk_hw_onecell_data *clk_data) { - void __iomem *base = NULL, *fhctl_base = NULL; - int i; - if (!clk_data) return; - for (i = num_plls; i > 0; i--) { - const struct mtk_pll_data *pll = &plls[i - 1]; - struct mtk_pllfh_data *pllfh; - bool use_fhctl; - - if (IS_ERR_OR_NULL(clk_data->hws[pll->id])) - continue; - - pllfh = get_pllfh_by_id(pllfhs, num_fhs, pll->id); - use_fhctl = fhctl_is_supported_and_enabled(pllfh); - - if (use_fhctl) { - fhctl_base = pllfh->state.base; - mtk_clk_unregister_pllfh(clk_data->hws[pll->id]); - } else { - base = mtk_clk_pll_get_base(clk_data->hws[pll->id], - pll); - mtk_clk_unregister_pll(clk_data->hws[pll->id]); - } - - clk_data->hws[pll->id] = ERR_PTR(-ENOENT); - } - - if (fhctl_base) - iounmap(fhctl_base); - - iounmap(base); + mtk_clk_cleanup_pllfhs(plls, num_plls, pllfhs, num_fhs, clk_data); } EXPORT_SYMBOL_GPL(mtk_clk_unregister_pllfhs);
--- base-commit: b3a87fbe738f19cbe19411cf749f3d8c09c04de4 change-id: 20260708-clk-mediatek-pllfh-fix-7e9ca35989e2 Best regards, -- Louis-Alexis Eyraud [off-list ref]