From: Rosen Penev <rosenp@gmail.com>
To: linux-mips@vger.kernel.org,
Thomas Bogendoerfer <tsbogend@alpha.franken.de>,
Stephen Boyd <sboyd@kernel.org>
Cc: Brian Masney <bmasney+clk@redhat.com>,
Jerome Brunet <jbrunet+clk@baylibre.com>,
Alban Bedel <albeu@free.fr>,
Antony Pavlov <antonynpavlov@gmail.com>,
linux-kernel@vger.kernel.org (open list),
linux-clk@vger.kernel.org (open list:COMMON CLK FRAMEWORK)
Subject: [PATCH 6/8] clk: ath79: fix the error handling
Date: Tue, 3 Nov 2026 21:58:50 -0800 [thread overview]
Message-ID: <20261104055852.152340-7-rosenp@gmail.com> (raw)
In-Reply-To: <20261104055852.152340-1-rosenp@gmail.com>
Report errors instead of crashing, and clean up after them:
- Check the ioremap() of the AR934x SRIF block, which was used without
a check.
- Without a reference clock, ath79_clocks_init_dt() called clk_put() on
an error pointer, which warns, and left the stale pointer in the clock
table. Bail out early instead.
- Return registration errors from the SoC init functions instead of
panicking, log them, and free the clock data.
- Drop the BUG_ON() on the clock index, which is always a constant.
The PLL registers aren't needed once the rates are known, so unmap them
after the SoC init in all cases, and unmap the SRIF block as soon as the
DPLLs are read. Registering the CPU, DDR and AHB clocks together also
removes some repetition.
If the clocks can't be set up, plat_time_init() still reports that it
can't get the CPU clock.
Signed-off-by: Rosen Penev <rosenp@gmail.com>
---
drivers/clk/clk-ath79.c | 179 ++++++++++++++++++++++------------------
1 file changed, 100 insertions(+), 79 deletions(-)
diff --git a/drivers/clk/clk-ath79.c b/drivers/clk/clk-ath79.c
index 3afda29377b5..768c9e410b59 100644
--- a/drivers/clk/clk-ath79.c
+++ b/drivers/clk/clk-ath79.c
@@ -35,53 +35,71 @@ static const char * const clk_names[ATH79_CLK_END] = {
[ATH79_CLK_MDIO] = "mdio",
};
-static const char * __init ath79_clk_name(int type)
+static int __init ath79_add_clk(int type, struct clk_hw *hw)
{
- BUG_ON(type >= ARRAY_SIZE(clk_names) || !clk_names[type]);
- return clk_names[type];
+ if (IS_ERR(hw))
+ return PTR_ERR(hw);
+
+ clk_data->hws[type] = hw;
+ clk_hw_register_clkdev(hw, clk_names[type], NULL);
+ return 0;
}
-static void __init __ath79_set_clk(int type, const char *name,
- struct clk_hw *hw)
+static int __init ath79_set_clk(int type, unsigned long rate)
{
- if (IS_ERR(hw))
- panic("failed to allocate %s clock structure", clk_names[type]);
+ return ath79_add_clk(type, clk_hw_register_fixed_rate(NULL,
+ clk_names[type], NULL, 0, rate));
+}
- clk_data->hws[type] = hw;
- clk_hw_register_clkdev(hw, name, NULL);
+/* a clock derived from the reference clock by a fixed factor */
+static int __init ath79_set_ff_clk(int type, unsigned int mult,
+ unsigned int div)
+{
+ return ath79_add_clk(type, clk_hw_register_fixed_factor_parent_hw(NULL,
+ clk_names[type],
+ clk_data->hws[ATH79_CLK_REF],
+ 0, mult, div));
}
-static void __init ath79_set_clk(int type, unsigned long rate)
+static int __init ath79_set_clks(unsigned long cpu_rate,
+ unsigned long ddr_rate,
+ unsigned long ahb_rate)
{
- const char *name = ath79_clk_name(type);
- struct clk_hw *hw;
+ int ret;
+
+ ret = ath79_set_clk(ATH79_CLK_CPU, cpu_rate);
+ if (ret)
+ return ret;
- hw = clk_hw_register_fixed_rate(NULL, name, NULL, 0, rate);
- __ath79_set_clk(type, name, hw);
+ ret = ath79_set_clk(ATH79_CLK_DDR, ddr_rate);
+ if (ret)
+ return ret;
+
+ return ath79_set_clk(ATH79_CLK_AHB, ahb_rate);
}
-/* a clock derived from the reference clock by a fixed factor */
-static void __init ath79_set_ff_clk(int type, unsigned int mult,
- unsigned int div)
+static int __init ath79_set_ff_clks(unsigned int mult, unsigned int cpu_div,
+ unsigned int ddr_div, unsigned int ahb_div)
{
- const char *name = ath79_clk_name(type);
- struct clk_hw *hw;
+ int ret;
- hw = clk_hw_register_fixed_factor_parent_hw(NULL, name,
- clk_data->hws[ATH79_CLK_REF],
- 0, mult, div);
- __ath79_set_clk(type, name, hw);
+ ret = ath79_set_ff_clk(ATH79_CLK_CPU, mult, cpu_div);
+ if (ret)
+ return ret;
+
+ ret = ath79_set_ff_clk(ATH79_CLK_DDR, mult, ddr_div);
+ if (ret)
+ return ret;
+
+ return ath79_set_ff_clk(ATH79_CLK_AHB, mult, ahb_div);
}
static unsigned long __init ath79_get_ref_rate(void)
{
- if (!clk_data->hws[ATH79_CLK_REF])
- panic("ath79: no reference clock");
-
return clk_hw_get_rate(clk_data->hws[ATH79_CLK_REF]);
}
-static void __init ar71xx_clocks_init(void __iomem *pll_base)
+static int __init ar71xx_clocks_init(void __iomem *pll_base)
{
unsigned long ref_rate;
unsigned long cpu_rate;
@@ -107,18 +125,14 @@ static void __init ar71xx_clocks_init(void __iomem *pll_base)
div = (FIELD_GET(AR71XX_AHB_DIV, pll) + 1) * 2;
ahb_rate = cpu_rate / div;
- ath79_set_clk(ATH79_CLK_CPU, cpu_rate);
- ath79_set_clk(ATH79_CLK_DDR, ddr_rate);
- ath79_set_clk(ATH79_CLK_AHB, ahb_rate);
+ return ath79_set_clks(cpu_rate, ddr_rate, ahb_rate);
}
-static void __init ar724x_clocks_init(void __iomem *pll_base)
+static int __init ar724x_clocks_init(void __iomem *pll_base)
{
u32 mult, div, ddr_div, ahb_div;
u32 pll;
- ath79_get_ref_rate();
-
pll = __raw_readl(pll_base + AR724X_PLL_REG_CPU_CONFIG);
mult = FIELD_GET(AR724X_PLL_FB, pll);
@@ -127,12 +141,10 @@ static void __init ar724x_clocks_init(void __iomem *pll_base)
ddr_div = FIELD_GET(AR724X_DDR_DIV, pll) + 1;
ahb_div = (FIELD_GET(AR724X_AHB_DIV, pll) + 1) * 2;
- ath79_set_ff_clk(ATH79_CLK_CPU, mult, div);
- ath79_set_ff_clk(ATH79_CLK_DDR, mult, div * ddr_div);
- ath79_set_ff_clk(ATH79_CLK_AHB, mult, div * ahb_div);
+ return ath79_set_ff_clks(mult, div, div * ddr_div, div * ahb_div);
}
-static void __init ar933x_clocks_init(void __iomem *pll_base)
+static int __init ar933x_clocks_init(void __iomem *pll_base)
{
u32 clock_ctrl;
u32 ref_div;
@@ -143,8 +155,6 @@ static void __init ar933x_clocks_init(void __iomem *pll_base)
u32 ddr_div;
u32 ahb_div;
- ath79_get_ref_rate();
-
clock_ctrl = __raw_readl(pll_base + AR933X_PLL_CLOCK_CTRL_REG);
if (clock_ctrl & AR933X_PLL_CLOCK_CTRL_BYPASS) {
ref_div = 1;
@@ -174,12 +184,9 @@ static void __init ar933x_clocks_init(void __iomem *pll_base)
ahb_div = FIELD_GET(AR933X_PLL_CLOCK_CTRL_AHB_DIV, clock_ctrl) + 1;
}
- ath79_set_ff_clk(ATH79_CLK_CPU, ninit_mul,
- ref_div * out_div * cpu_div);
- ath79_set_ff_clk(ATH79_CLK_DDR, ninit_mul,
- ref_div * out_div * ddr_div);
- ath79_set_ff_clk(ATH79_CLK_AHB, ninit_mul,
- ref_div * out_div * ahb_div);
+ return ath79_set_ff_clks(ninit_mul, ref_div * out_div * cpu_div,
+ ref_div * out_div * ddr_div,
+ ref_div * out_div * ahb_div);
}
static u32 __init ar934x_get_pll_freq(u32 ref, u32 ref_div, u32 nint, u32 nfrac,
@@ -202,7 +209,7 @@ static u32 __init ar934x_get_pll_freq(u32 ref, u32 ref_div, u32 nint, u32 nfrac,
return ret;
}
-static void __init ar934x_clocks_init(void __iomem *pll_base)
+static int __init ar934x_clocks_init(void __iomem *pll_base)
{
unsigned long ref_rate;
unsigned long cpu_rate;
@@ -211,8 +218,11 @@ static void __init ar934x_clocks_init(void __iomem *pll_base)
u32 pll, out_div, ref_div, nint, nfrac, frac, clk_ctrl, postdiv;
u32 cpu_pll, ddr_pll;
void __iomem *dpll_base;
+ int ret;
dpll_base = ioremap(AR934X_SRIF_BASE, AR934X_SRIF_SIZE);
+ if (!dpll_base)
+ return -ENOMEM;
ref_rate = ath79_get_ref_rate();
@@ -256,6 +266,8 @@ static void __init ar934x_clocks_init(void __iomem *pll_base)
ddr_pll = ar934x_get_pll_freq(ref_rate, ref_div, nint,
nfrac, frac, out_div);
+ iounmap(dpll_base);
+
clk_ctrl = __raw_readl(pll_base + AR934X_PLL_CPU_DDR_CLK_CTRL_REG);
postdiv = FIELD_GET(AR934X_PLL_CPU_DDR_CLK_CTRL_CPU_POST_DIV, clk_ctrl);
@@ -285,18 +297,18 @@ static void __init ar934x_clocks_init(void __iomem *pll_base)
else
ahb_rate = cpu_pll / (postdiv + 1);
- ath79_set_clk(ATH79_CLK_CPU, cpu_rate);
- ath79_set_clk(ATH79_CLK_DDR, ddr_rate);
- ath79_set_clk(ATH79_CLK_AHB, ahb_rate);
+ ret = ath79_set_clks(cpu_rate, ddr_rate, ahb_rate);
+ if (ret)
+ return ret;
clk_ctrl = __raw_readl(pll_base + AR934X_PLL_SWITCH_CLOCK_CONTROL_REG);
if (clk_ctrl & AR934X_PLL_SWITCH_CLOCK_CONTROL_MDIO_CLK_SEL)
- ath79_set_clk(ATH79_CLK_MDIO, 100 * 1000 * 1000);
+ return ath79_set_clk(ATH79_CLK_MDIO, 100 * 1000 * 1000);
- iounmap(dpll_base);
+ return 0;
}
-static void __init qca953x_clocks_init(void __iomem *pll_base)
+static int __init qca953x_clocks_init(void __iomem *pll_base)
{
unsigned long ref_rate;
unsigned long cpu_rate;
@@ -356,12 +368,10 @@ static void __init qca953x_clocks_init(void __iomem *pll_base)
else
ahb_rate = cpu_pll / (postdiv + 1);
- ath79_set_clk(ATH79_CLK_CPU, cpu_rate);
- ath79_set_clk(ATH79_CLK_DDR, ddr_rate);
- ath79_set_clk(ATH79_CLK_AHB, ahb_rate);
+ return ath79_set_clks(cpu_rate, ddr_rate, ahb_rate);
}
-static void __init qca955x_clocks_init(void __iomem *pll_base)
+static int __init qca955x_clocks_init(void __iomem *pll_base)
{
unsigned long ref_rate;
unsigned long cpu_rate;
@@ -421,12 +431,10 @@ static void __init qca955x_clocks_init(void __iomem *pll_base)
else
ahb_rate = cpu_pll / (postdiv + 1);
- ath79_set_clk(ATH79_CLK_CPU, cpu_rate);
- ath79_set_clk(ATH79_CLK_DDR, ddr_rate);
- ath79_set_clk(ATH79_CLK_AHB, ahb_rate);
+ return ath79_set_clks(cpu_rate, ddr_rate, ahb_rate);
}
-static void __init qca956x_clocks_init(void __iomem *pll_base)
+static int __init qca956x_clocks_init(void __iomem *pll_base)
{
unsigned long ref_rate;
unsigned long cpu_rate;
@@ -493,19 +501,18 @@ static void __init qca956x_clocks_init(void __iomem *pll_base)
else
ahb_rate = cpu_pll / (postdiv + 1);
- ath79_set_clk(ATH79_CLK_CPU, cpu_rate);
- ath79_set_clk(ATH79_CLK_DDR, ddr_rate);
- ath79_set_clk(ATH79_CLK_AHB, ahb_rate);
+ return ath79_set_clks(cpu_rate, ddr_rate, ahb_rate);
}
static void __init ath79_clocks_init_dt(struct device_node *np)
{
struct clk *ref_clk;
void __iomem *pll_base;
+ int ret = -ENODEV;
clk_data = kzalloc_flex(*clk_data, hws, ATH79_CLK_END);
if (!clk_data)
- panic("ath79: can't allocate the clock data");
+ return;
clk_data->num = ATH79_CLK_END;
/*
@@ -515,8 +522,11 @@ static void __init ath79_clocks_init_dt(struct device_node *np)
ref_clk = of_clk_get(np, 0);
if (IS_ERR(ref_clk))
ref_clk = clk_get_sys(NULL, "ref");
- if (!IS_ERR(ref_clk))
- clk_data->hws[ATH79_CLK_REF] = __clk_get_hw(ref_clk);
+ if (IS_ERR(ref_clk)) {
+ pr_err("%pOF: no reference clock\n", np);
+ goto err_free;
+ }
+ clk_data->hws[ATH79_CLK_REF] = __clk_get_hw(ref_clk);
pll_base = of_iomap(np, 0);
if (!pll_base) {
@@ -525,36 +535,47 @@ static void __init ath79_clocks_init_dt(struct device_node *np)
}
if (of_device_is_compatible(np, "qca,ar7100-pll"))
- ar71xx_clocks_init(pll_base);
+ ret = ar71xx_clocks_init(pll_base);
else if (of_device_is_compatible(np, "qca,ar7240-pll") ||
of_device_is_compatible(np, "qca,ar9130-pll"))
- ar724x_clocks_init(pll_base);
+ ret = ar724x_clocks_init(pll_base);
else if (of_device_is_compatible(np, "qca,ar9330-pll"))
- ar933x_clocks_init(pll_base);
+ ret = ar933x_clocks_init(pll_base);
else if (of_device_is_compatible(np, "qca,ar9340-pll"))
- ar934x_clocks_init(pll_base);
+ ret = ar934x_clocks_init(pll_base);
else if (of_device_is_compatible(np, "qca,qca9530-pll"))
- qca953x_clocks_init(pll_base);
+ ret = qca953x_clocks_init(pll_base);
else if (of_device_is_compatible(np, "qca,qca9550-pll"))
- qca955x_clocks_init(pll_base);
+ ret = qca955x_clocks_init(pll_base);
else if (of_device_is_compatible(np, "qca,qca9560-pll"))
- qca956x_clocks_init(pll_base);
+ ret = qca956x_clocks_init(pll_base);
+
+ /* the rates are fixed, so the registers aren't needed any more */
+ iounmap(pll_base);
+
+ if (ret) {
+ pr_err("%pOF: failed to register the clocks: %pe\n", np,
+ ERR_PTR(ret));
+ goto err_clk;
+ }
if (!clk_data->hws[ATH79_CLK_MDIO])
clk_data->hws[ATH79_CLK_MDIO] = clk_data->hws[ATH79_CLK_REF];
- if (of_clk_add_hw_provider(np, of_clk_hw_onecell_get, clk_data)) {
- pr_err("%pOF: could not register clk provider\n", np);
- goto err_iounmap;
+ ret = of_clk_add_hw_provider(np, of_clk_hw_onecell_get, clk_data);
+ if (ret) {
+ pr_err("%pOF: could not register clk provider: %pe\n", np,
+ ERR_PTR(ret));
+ goto err_clk;
}
return;
-err_iounmap:
- iounmap(pll_base);
-
err_clk:
clk_put(ref_clk);
+err_free:
+ kfree(clk_data);
+ clk_data = NULL;
}
CLK_OF_DECLARE(ar7100_clk, "qca,ar7100-pll", ath79_clocks_init_dt);
--
2.56.0
next prev parent reply other threads:[~2026-11-04 5:59 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-11-04 5:58 [PATCH 0/8] clk: ath79: move the PLL clock driver to drivers/clk Rosen Penev
2026-11-04 5:58 ` [PATCH 1/8] MIPS: ath79: apply the QCA956x timer workaround in setup Rosen Penev
2026-11-04 5:58 ` [PATCH 2/8] MIPS: ath79: move the bootstrap reference clock to setup Rosen Penev
2026-11-04 5:58 ` [PATCH 3/8] clk: ath79: move the ath79 clock driver to drivers/clk Rosen Penev
2026-11-04 5:58 ` [PATCH 4/8] clk: ath79: use FIELD_GET() for the PLL register fields Rosen Penev
2026-11-04 5:58 ` [PATCH 5/8] clk: ath79: convert to clk_hw based registration Rosen Penev
2026-11-04 5:58 ` Rosen Penev [this message]
2026-11-04 5:58 ` [PATCH 7/8] clk: ath79: give each compatible its own init function Rosen Penev
2026-11-04 5:58 ` [PATCH 8/8] clk: ath79: share the fractional PLL calculation Rosen Penev
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20261104055852.152340-7-rosenp@gmail.com \
--to=rosenp@gmail.com \
--cc=albeu@free.fr \
--cc=antonynpavlov@gmail.com \
--cc=bmasney+clk@redhat.com \
--cc=jbrunet+clk@baylibre.com \
--cc=linux-clk@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mips@vger.kernel.org \
--cc=sboyd@kernel.org \
--cc=tsbogend@alpha.franken.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®