Re: [PATCH v3 1/3] clk: socfpga: agilex: register the SP timer clocks early

Re: [PATCH v3 1/3] clk: socfpga: agilex: register the SP timer clocks early

[kernel test robot]

Unfiltered warning: this smatch report is sent as-is and has not been
reviewed for false positives, so please treat it with due skepticism.

BCC: lkp@intel.com
CC: oe-kbuild-all@lists.linux.dev
In-Reply-To: <1fc28c08edede0f96b5eca860d5c344e185f18da.1791871090.git.adrian.ho.yin.ng@altera.com>
References: <1fc28c08edede0f96b5eca860d5c344e185f18da.1791871090.git.adrian.ho.yin.ng@altera.com>
TO: Adrian Ng Ho Yin <adrian.ho.yin.ng@altera.com>
TO: Dinh Nguyen <dinguyen@kernel.org>
TO: Michael Turquette <mturquette@baylibre.com>
TO: Stephen Boyd <sboyd@kernel.org>
TO: linux-clk@vger.kernel.org
TO: linux-kernel@vger.kernel.org
CC: Adrian Ng Ho Yin <adrian.ho.yin.ng@altera.com>

Hi Adrian,

kernel test robot noticed the following build warnings:

[auto build test WARNING on 1e7f56205813a2c48cdb3e9a4b0a24f49fd9a548]

url:    https://github.com/intel-lab-lkp/linux/commits/Adrian-Ng-Ho-Yin/clk-socfpga-agilex-register-the-SP-timer-clocks-early/20261013-150752
base:   1e7f56205813a2c48cdb3e9a4b0a24f49fd9a548
patch link:    https://lore.kernel.org/r/1fc28c08edede0f96b5eca860d5c344e185f18da.1791871090.git.adrian.ho.yin.ng%40altera.com
patch subject: [PATCH v3 1/3] clk: socfpga: agilex: register the SP timer clocks early
:::::: branch date: 7 days ago
:::::: commit date: 7 days ago
config: arm64-randconfig-r2202-20261020 (https://download.01.org/0day-ci/archive/20261020/202610201741.iarcON9M-lkp@intel.com/config)
compiler: aarch64-linux-gcc (GCC) 12.5.0
smatch: v0.5.0-9187-g5189e3fb

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 <lkp@intel.com>
| Reported-by: Dan Carpenter <error27@gmail.com>
| Closes: https://lore.kernel.org/r/202610201741.iarcON9M-lkp@intel.com/

smatch warnings:
drivers/clk/socfpga/clk-agilex.c:635 agilex_clkmgr_init() warn: 'clk_data' can also be NULL
drivers/clk/socfpga/clk-agilex.c:656 n5x_clkmgr_init() warn: 'clk_data' can also be NULL

vim +/clk_data +635 drivers/clk/socfpga/clk-agilex.c

59988b8a07910f Adrian Ng Ho Yin 2026-10-13  626  
59988b8a07910f Adrian Ng Ho Yin 2026-10-13  627  static int agilex_clkmgr_init(struct platform_device *pdev)
59988b8a07910f Adrian Ng Ho Yin 2026-10-13  628  {
59988b8a07910f Adrian Ng Ho Yin 2026-10-13  629  	struct stratix10_clock_data *clk_data;
59988b8a07910f Adrian Ng Ho Yin 2026-10-13  630  
59988b8a07910f Adrian Ng Ho Yin 2026-10-13  631  	clk_data = agilex_clkmgr_get_clk_data(pdev, agilex_clk_register_early);
59988b8a07910f Adrian Ng Ho Yin 2026-10-13  632  	if (IS_ERR(clk_data))
59988b8a07910f Adrian Ng Ho Yin 2026-10-13  633  		return PTR_ERR(clk_data);
59988b8a07910f Adrian Ng Ho Yin 2026-10-13  634  
59988b8a07910f Adrian Ng Ho Yin 2026-10-13 @635  	agilex_clk_register_c_perip(agilex_main_perip_c_clks,
59988b8a07910f Adrian Ng Ho Yin 2026-10-13  636  				 ARRAY_SIZE(agilex_main_perip_c_clks), clk_data);
59988b8a07910f Adrian Ng Ho Yin 2026-10-13  637  
59988b8a07910f Adrian Ng Ho Yin 2026-10-13  638  	agilex_clk_register_cnt_perip(agilex_main_perip_cnt_clks,
59988b8a07910f Adrian Ng Ho Yin 2026-10-13  639  				   ARRAY_SIZE(agilex_main_perip_cnt_clks),
59988b8a07910f Adrian Ng Ho Yin 2026-10-13  640  				   clk_data);
59988b8a07910f Adrian Ng Ho Yin 2026-10-13  641  
59988b8a07910f Adrian Ng Ho Yin 2026-10-13  642  	agilex_clk_register_gate(agilex_gate_clks, ARRAY_SIZE(agilex_gate_clks),
59988b8a07910f Adrian Ng Ho Yin 2026-10-13  643  			      clk_data);
59988b8a07910f Adrian Ng Ho Yin 2026-10-13  644  
59988b8a07910f Adrian Ng Ho Yin 2026-10-13  645  	return agilex_clkmgr_finish(pdev, clk_data);
59988b8a07910f Adrian Ng Ho Yin 2026-10-13  646  }
59988b8a07910f Adrian Ng Ho Yin 2026-10-13  647  
59988b8a07910f Adrian Ng Ho Yin 2026-10-13  648  static int n5x_clkmgr_init(struct platform_device *pdev)
59988b8a07910f Adrian Ng Ho Yin 2026-10-13  649  {
59988b8a07910f Adrian Ng Ho Yin 2026-10-13  650  	struct stratix10_clock_data *clk_data;
59988b8a07910f Adrian Ng Ho Yin 2026-10-13  651  
59988b8a07910f Adrian Ng Ho Yin 2026-10-13  652  	clk_data = agilex_clkmgr_get_clk_data(pdev, n5x_clk_register_early);
59988b8a07910f Adrian Ng Ho Yin 2026-10-13  653  	if (IS_ERR(clk_data))
59988b8a07910f Adrian Ng Ho Yin 2026-10-13  654  		return PTR_ERR(clk_data);
a0f9819cbe9952 Dinh Nguyen      2021-02-12  655  
a0f9819cbe9952 Dinh Nguyen      2021-02-12 @656  	n5x_clk_register_c_perip(n5x_main_perip_c_clks,
a0f9819cbe9952 Dinh Nguyen      2021-02-12  657  				 ARRAY_SIZE(n5x_main_perip_c_clks), clk_data);
a0f9819cbe9952 Dinh Nguyen      2021-02-12  658  
a0f9819cbe9952 Dinh Nguyen      2021-02-12  659  	agilex_clk_register_cnt_perip(agilex_main_perip_cnt_clks,
a0f9819cbe9952 Dinh Nguyen      2021-02-12  660  				   ARRAY_SIZE(agilex_main_perip_cnt_clks),
a0f9819cbe9952 Dinh Nguyen      2021-02-12  661  				   clk_data);
a0f9819cbe9952 Dinh Nguyen      2021-02-12  662  
a0f9819cbe9952 Dinh Nguyen      2021-02-12  663  	agilex_clk_register_gate(agilex_gate_clks, ARRAY_SIZE(agilex_gate_clks),
a0f9819cbe9952 Dinh Nguyen      2021-02-12  664  			      clk_data);
59988b8a07910f Adrian Ng Ho Yin 2026-10-13  665  
59988b8a07910f Adrian Ng Ho Yin 2026-10-13  666  	return agilex_clkmgr_finish(pdev, clk_data);
a0f9819cbe9952 Dinh Nguyen      2021-02-12  667  }
a0f9819cbe9952 Dinh Nguyen      2021-02-12  668  

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

[PATCH v3 0/3] clk: socfpga: register the SP timer clocks early

[Adrian Ng Ho Yin]

The DW APB timers on Stratix10, Agilex (including eASIC N5X) and Agilex5
are probed through TIMER_OF_DECLARE() from time_init(), but their clock,
l4_sp_clk, comes from a clock manager that is a platform driver
registered at core_initcall(). The timer's clk_get() returns
-EPROBE_DEFER, which timer_probe() does not report, and the timer is
never registered because TIMER_OF_DECLARE() callbacks are not retried.

Rather than moving the whole clock manager to CLK_OF_DECLARE() as v1 did,
this version keeps the platform driver and, following
clk-mt8173-infracfg.c, uses CLK_OF_DECLARE_DRIVER() to register only
l4_sp_clk and the clocks its rate depends on from of_clk_init():

  Agilex/N5X: boot_clk, main_pll, periph_pll, main_pll_c1, peri_pll_c1,
              noc_free_clk, l4_sp_clk
  Agilex5:    boot_clk, main_pll, periph_pll, main_pll_c3, peri_pll_c1,
              noc_free_clk, l4_sp_clk
  Stratix10:  boot_clk, main_pll, periph_pll, main_noc_base_clk,
              peri_noc_base_clk, noc_free_clk, l4_sp_clk

All other clock IDs return -EPROBE_DEFER until the platform driver probes,
registers the remaining clocks into the same provider and turns any
unused IDs into -ENOENT. If the early registration fails, probe falls
back to registering everything as before.

Boot-tested on Agilex7 (FM61), Stratix10 L-tile and Agilex5 (065B Premium
devkit), comparing each board with and without this series:

  - with the series, timer1 is registered as a clocksource within 0.5 ms
    of boot and timer0 as the broadcast clockevent device; without it,
    neither is registered and bc_hrtimer is used for broadcast instead
  - the clock manager still binds as a platform device, no devices are
    left deferred, and clocks registered at probe (EMAC, SD/MMC, USB,
    DMA, watchdog) have their expected rates and consumers

eASIC N5X is compile-tested only.

Changes in v3:
- Drop redundant #include <linux/io.h>; of_address.h already provides
  iounmap() (Dinh)
- Always publish clk_data before adding the provider; use of_iomap() for
  both early and probe-fallback setup so a failed provider add cannot
  double-register early clocks or leave clocks pointing at a
  devm-unmapped base

Changes in v2:
- Keep the clock managers as platform drivers instead of converting
  them to CLK_OF_DECLARE() (Brian)
- Register only l4_sp_clk and its parents early, using
  CLK_OF_DECLARE_DRIVER() as in clk-mt8173-infracfg.c, instead of the
  whole controller (Jerome)
- Return the result of of_clk_add_hw_provider() from probe
- Update subjects and commit messages to match the new approach

v2: https://lore.kernel.org/all/cover.1790935401.git.adrian.ho.yin.ng@altera.com/
v1: https://lore.kernel.org/all/cover.1789096458.git.adrian.ho.yin.ng@altera.com/

Adrian Ng Ho Yin (3):
  clk: socfpga: agilex: register the SP timer clocks early
  clk: socfpga: agilex5: register the SP timer clocks early
  clk: socfpga: stratix10: register the SP timer clocks early

 drivers/clk/socfpga/clk-agilex.c  | 242 +++++++++++++++++++++++-------
 drivers/clk/socfpga/clk-agilex5.c | 155 +++++++++++++++----
 drivers/clk/socfpga/clk-s10.c     | 146 +++++++++++++++---
 3 files changed, 434 insertions(+), 109 deletions(-)


base-commit: 1e7f56205813a2c48cdb3e9a4b0a24f49fd9a548
-- 
2.49.GIT

[PATCH v3 1/3] clk: socfpga: agilex: register the SP timer clocks early

[Adrian Ng Ho Yin]

The DW APB timers are probed through TIMER_OF_DECLARE() from time_init()
and cannot defer, but the clock manager is a platform driver registered
at core_initcall(). The timer's clk_get() therefore fails and the timer
is never brought up.

Register only l4_sp_clk and its parents (boot_clk, main_pll, periph_pll,
main_pll_c1, peri_pll_c1 and noc_free_clk) from of_clk_init() using
CLK_OF_DECLARE_DRIVER(), with every other clock left as -EPROBE_DEFER.
The remaining clocks are still registered when the platform driver
probes, which reuses the early provider.

Signed-off-by: Adrian Ng Ho Yin <adrian.ho.yin.ng@altera.com>
---
 drivers/clk/socfpga/clk-agilex.c | 242 ++++++++++++++++++++++++-------
 1 file changed, 187 insertions(+), 55 deletions(-)

diff --git a/drivers/clk/socfpga/clk-agilex.c b/drivers/clk/socfpga/clk-agilex.c
index 2bdea1997b5e..445d814e2f3a 100644
--- a/drivers/clk/socfpga/clk-agilex.c
+++ b/drivers/clk/socfpga/clk-agilex.c
@@ -5,6 +5,7 @@
 #include <linux/slab.h>
 #include <linux/clk-provider.h>
 #include <linux/of.h>
+#include <linux/of_address.h>
 #include <linux/platform_device.h>
 
 #include <dt-bindings/clock/agilex-clock.h>
@@ -232,24 +233,49 @@ static const struct stratix10_pll_clock agilex_pll_clks[] = {
 	  0, 0x9c},
 };
 
+/*
+ * The SP timers are probed from time_init() and cannot defer, so their
+ * l4_sp_clk and its parents are registered from of_clk_init(). Everything
+ * else is registered when the platform driver probes.
+ */
+static const struct n5x_perip_c_clock n5x_early_perip_c_clks[] = {
+	{ AGILEX_MAIN_PLL_C1_CLK, "main_pll_c1", "main_pll", NULL, 1, 0, 0x54, 8},
+	{ AGILEX_PERIPH_PLL_C1_CLK, "peri_pll_c1", "periph_pll", NULL, 1, 0, 0xA8, 8},
+};
+
+static const struct stratix10_perip_c_clock agilex_early_perip_c_clks[] = {
+	{ AGILEX_MAIN_PLL_C1_CLK, "main_pll_c1", "main_pll", NULL, 1, 0, 0x5C},
+	{ AGILEX_PERIPH_PLL_C1_CLK, "peri_pll_c1", "periph_pll", NULL, 1, 0, 0xB0},
+};
+
+static const struct stratix10_perip_cnt_clock agilex_early_perip_cnt_clks[] = {
+	{ AGILEX_NOC_FREE_CLK, "noc_free_clk", NULL, noc_free_mux, ARRAY_SIZE(noc_free_mux),
+	  0, 0x40, 0, 0, 0},
+};
+
+static const struct stratix10_gate_clock agilex_early_gate_clks[] = {
+	/*
+	 * The l4_sp_clk feeds a 100 MHz clock to various peripherals, one of them
+	 * being the SP timers, thus cannot get gated.
+	 */
+	{ AGILEX_L4_SP_CLK, "l4_sp_clk", NULL, noc_mux, ARRAY_SIZE(noc_mux), CLK_IS_CRITICAL, 0x24,
+	  3, 0x44, 16, 2, 0x30, 1, 0},
+};
+
 static const struct n5x_perip_c_clock n5x_main_perip_c_clks[] = {
 	{ AGILEX_MAIN_PLL_C0_CLK, "main_pll_c0", "main_pll", NULL, 1, 0, 0x54, 0},
-	{ AGILEX_MAIN_PLL_C1_CLK, "main_pll_c1", "main_pll", NULL, 1, 0, 0x54, 8},
 	{ AGILEX_MAIN_PLL_C2_CLK, "main_pll_c2", "main_pll", NULL, 1, 0, 0x54, 16},
 	{ AGILEX_MAIN_PLL_C3_CLK, "main_pll_c3", "main_pll", NULL, 1, 0, 0x54, 24},
 	{ AGILEX_PERIPH_PLL_C0_CLK, "peri_pll_c0", "periph_pll", NULL, 1, 0, 0xA8, 0},
-	{ AGILEX_PERIPH_PLL_C1_CLK, "peri_pll_c1", "periph_pll", NULL, 1, 0, 0xA8, 8},
 	{ AGILEX_PERIPH_PLL_C2_CLK, "peri_pll_c2", "periph_pll", NULL, 1, 0, 0xA8, 16},
 	{ AGILEX_PERIPH_PLL_C3_CLK, "peri_pll_c3", "periph_pll", NULL, 1, 0, 0xA8, 24},
 };
 
 static const struct stratix10_perip_c_clock agilex_main_perip_c_clks[] = {
 	{ AGILEX_MAIN_PLL_C0_CLK, "main_pll_c0", "main_pll", NULL, 1, 0, 0x58},
-	{ AGILEX_MAIN_PLL_C1_CLK, "main_pll_c1", "main_pll", NULL, 1, 0, 0x5C},
 	{ AGILEX_MAIN_PLL_C2_CLK, "main_pll_c2", "main_pll", NULL, 1, 0, 0x64},
 	{ AGILEX_MAIN_PLL_C3_CLK, "main_pll_c3", "main_pll", NULL, 1, 0, 0x68},
 	{ AGILEX_PERIPH_PLL_C0_CLK, "peri_pll_c0", "periph_pll", NULL, 1, 0, 0xAC},
-	{ AGILEX_PERIPH_PLL_C1_CLK, "peri_pll_c1", "periph_pll", NULL, 1, 0, 0xB0},
 	{ AGILEX_PERIPH_PLL_C2_CLK, "peri_pll_c2", "periph_pll", NULL, 1, 0, 0xB8},
 	{ AGILEX_PERIPH_PLL_C3_CLK, "peri_pll_c3", "periph_pll", NULL, 1, 0, 0xBC},
 };
@@ -257,8 +283,6 @@ static const struct stratix10_perip_c_clock agilex_main_perip_c_clks[] = {
 static const struct stratix10_perip_cnt_clock agilex_main_perip_cnt_clks[] = {
 	{ AGILEX_MPU_FREE_CLK, "mpu_free_clk", NULL, mpu_free_mux, ARRAY_SIZE(mpu_free_mux),
 	   0, 0x3C, 0, 0, 0},
-	{ AGILEX_NOC_FREE_CLK, "noc_free_clk", NULL, noc_free_mux, ARRAY_SIZE(noc_free_mux),
-	  0, 0x40, 0, 0, 0},
 	{ AGILEX_L3_MAIN_FREE_CLK, "l3_main_free_clk", "noc_free_clk", NULL,
 	  1, 0, 0, 1, 0, 0},
 	{ AGILEX_L4_SYS_FREE_CLK, "l4_sys_free_clk", NULL, noc_mux, ARRAY_SIZE(noc_mux), 0,
@@ -292,12 +316,6 @@ static const struct stratix10_gate_clock agilex_gate_clks[] = {
 	  1, 0x44, 0, 2, 0x30, 1, 0},
 	{ AGILEX_L4_MP_CLK, "l4_mp_clk", NULL, noc_mux, ARRAY_SIZE(noc_mux), 0, 0x24,
 	  2, 0x44, 8, 2, 0x30, 1, 0},
-	/*
-	 * The l4_sp_clk feeds a 100 MHz clock to various peripherals, one of them
-	 * being the SP timers, thus cannot get gated.
-	 */
-	{ AGILEX_L4_SP_CLK, "l4_sp_clk", NULL, noc_mux, ARRAY_SIZE(noc_mux), CLK_IS_CRITICAL, 0x24,
-	  3, 0x44, 16, 2, 0x30, 1, 0},
 	{ AGILEX_CS_AT_CLK, "cs_at_clk", NULL, noc_mux, ARRAY_SIZE(noc_mux), 0, 0x24,
 	  4, 0x44, 24, 2, 0x30, 1, 0},
 	{ AGILEX_CS_TRACE_CLK, "cs_trace_clk", NULL, noc_mux, ARRAY_SIZE(noc_mux), 0, 0x24,
@@ -454,33 +472,166 @@ static int n5x_clk_register_pll(const struct stratix10_pll_clock *clks,
 	return 0;
 }
 
-static int agilex_clkmgr_init(struct platform_device *pdev)
+static struct stratix10_clock_data *agilex_clk_data;
+
+static struct stratix10_clock_data *agilex_clk_data_alloc(void __iomem *base)
 {
-	struct device_node *np = pdev->dev.of_node;
-	struct device *dev = &pdev->dev;
 	struct stratix10_clock_data *clk_data;
-	void __iomem *base;
-	int i, num_clks;
-
-	base = devm_platform_ioremap_resource(pdev, 0);
-	if (IS_ERR(base))
-		return PTR_ERR(base);
-
-	num_clks = AGILEX_NUM_CLKS;
+	int i;
 
-	clk_data = devm_kzalloc(dev, struct_size(clk_data, clk_data.hws,
-				num_clks), GFP_KERNEL);
+	clk_data = kzalloc(struct_size(clk_data, clk_data.hws, AGILEX_NUM_CLKS),
+			   GFP_KERNEL);
 	if (!clk_data)
-		return -ENOMEM;
+		return NULL;
 
-	clk_data->clk_data.num = num_clks;
 	clk_data->base = base;
+	clk_data->clk_data.num = AGILEX_NUM_CLKS;
 
-	for (i = 0; i < num_clks; i++)
-		clk_data->clk_data.hws[i] = ERR_PTR(-ENOENT);
+	for (i = 0; i < AGILEX_NUM_CLKS; i++)
+		clk_data->clk_data.hws[i] = ERR_PTR(-EPROBE_DEFER);
+
+	return clk_data;
+}
 
+static void agilex_clk_register_early(struct stratix10_clock_data *clk_data)
+{
 	agilex_clk_register_pll(agilex_pll_clks, ARRAY_SIZE(agilex_pll_clks), clk_data);
 
+	agilex_clk_register_c_perip(agilex_early_perip_c_clks,
+				    ARRAY_SIZE(agilex_early_perip_c_clks), clk_data);
+
+	agilex_clk_register_cnt_perip(agilex_early_perip_cnt_clks,
+				      ARRAY_SIZE(agilex_early_perip_cnt_clks),
+				      clk_data);
+
+	agilex_clk_register_gate(agilex_early_gate_clks,
+				 ARRAY_SIZE(agilex_early_gate_clks), clk_data);
+}
+
+static void n5x_clk_register_early(struct stratix10_clock_data *clk_data)
+{
+	n5x_clk_register_pll(agilex_pll_clks, ARRAY_SIZE(agilex_pll_clks), clk_data);
+
+	n5x_clk_register_c_perip(n5x_early_perip_c_clks,
+				 ARRAY_SIZE(n5x_early_perip_c_clks), clk_data);
+
+	agilex_clk_register_cnt_perip(agilex_early_perip_cnt_clks,
+				      ARRAY_SIZE(agilex_early_perip_cnt_clks),
+				      clk_data);
+
+	agilex_clk_register_gate(agilex_early_gate_clks,
+				 ARRAY_SIZE(agilex_early_gate_clks), clk_data);
+}
+
+/*
+ * The registered clocks keep using the mapping and clk_data, so neither is
+ * released once the early clocks have been registered.
+ */
+static int
+agilex_clkmgr_setup(struct device_node *np,
+		    void (*register_early)(struct stratix10_clock_data *))
+{
+	struct stratix10_clock_data *clk_data;
+	void __iomem *base;
+
+	base = of_iomap(np, 0);
+	if (!base) {
+		pr_err("%s: failed to map clock registers\n", __func__);
+		return -ENOMEM;
+	}
+
+	clk_data = agilex_clk_data_alloc(base);
+	if (!clk_data) {
+		iounmap(base);
+		return -ENOMEM;
+	}
+
+	register_early(clk_data);
+	agilex_clk_data = clk_data;
+
+	return 0;
+}
+
+static int agilex_clkmgr_add_provider(struct device_node *np)
+{
+	static bool added;
+	int ret;
+
+	if (added)
+		return 0;
+
+	ret = of_clk_add_hw_provider(np, of_clk_hw_onecell_get,
+				     &agilex_clk_data->clk_data);
+	if (ret)
+		return ret;
+
+	added = true;
+
+	return 0;
+}
+
+static void __init
+agilex_clkmgr_of_init(struct device_node *np,
+		      void (*register_early)(struct stratix10_clock_data *))
+{
+	if (agilex_clkmgr_setup(np, register_early))
+		return;
+
+	if (agilex_clkmgr_add_provider(np))
+		pr_err("%s: failed to add clock provider\n", __func__);
+}
+
+static void __init agilex_clkmgr_early_init(struct device_node *np)
+{
+	agilex_clkmgr_of_init(np, agilex_clk_register_early);
+}
+
+CLK_OF_DECLARE_DRIVER(agilex_clkmgr, "intel,agilex-clkmgr",
+		      agilex_clkmgr_early_init);
+
+static void __init n5x_clkmgr_early_init(struct device_node *np)
+{
+	agilex_clkmgr_of_init(np, n5x_clk_register_early);
+}
+
+CLK_OF_DECLARE_DRIVER(n5x_clkmgr, "intel,easic-n5x-clkmgr",
+		      n5x_clkmgr_early_init);
+
+static struct stratix10_clock_data *
+agilex_clkmgr_get_clk_data(struct platform_device *pdev,
+			   void (*register_early)(struct stratix10_clock_data *))
+{
+	int ret;
+
+	if (!agilex_clk_data) {
+		ret = agilex_clkmgr_setup(pdev->dev.of_node, register_early);
+		if (ret)
+			return ERR_PTR(ret);
+	}
+
+	return agilex_clk_data;
+}
+
+static int agilex_clkmgr_finish(struct platform_device *pdev,
+				struct stratix10_clock_data *clk_data)
+{
+	int i;
+
+	for (i = 0; i < AGILEX_NUM_CLKS; i++)
+		if (clk_data->clk_data.hws[i] == ERR_PTR(-EPROBE_DEFER))
+			clk_data->clk_data.hws[i] = ERR_PTR(-ENOENT);
+
+	return agilex_clkmgr_add_provider(pdev->dev.of_node);
+}
+
+static int agilex_clkmgr_init(struct platform_device *pdev)
+{
+	struct stratix10_clock_data *clk_data;
+
+	clk_data = agilex_clkmgr_get_clk_data(pdev, agilex_clk_register_early);
+	if (IS_ERR(clk_data))
+		return PTR_ERR(clk_data);
+
 	agilex_clk_register_c_perip(agilex_main_perip_c_clks,
 				 ARRAY_SIZE(agilex_main_perip_c_clks), clk_data);
 
@@ -490,36 +641,17 @@ static int agilex_clkmgr_init(struct platform_device *pdev)
 
 	agilex_clk_register_gate(agilex_gate_clks, ARRAY_SIZE(agilex_gate_clks),
 			      clk_data);
-	of_clk_add_hw_provider(np, of_clk_hw_onecell_get, &clk_data->clk_data);
-	return 0;
+
+	return agilex_clkmgr_finish(pdev, clk_data);
 }
 
 static int n5x_clkmgr_init(struct platform_device *pdev)
 {
-	struct device_node *np = pdev->dev.of_node;
-	struct device *dev = &pdev->dev;
 	struct stratix10_clock_data *clk_data;
-	void __iomem *base;
-	int i, num_clks;
 
-	base = devm_platform_ioremap_resource(pdev, 0);
-	if (IS_ERR(base))
-		return PTR_ERR(base);
-
-	num_clks = AGILEX_NUM_CLKS;
-
-	clk_data = devm_kzalloc(dev, struct_size(clk_data, clk_data.hws,
-				num_clks), GFP_KERNEL);
-	if (!clk_data)
-		return -ENOMEM;
-
-	clk_data->base = base;
-	clk_data->clk_data.num = num_clks;
-
-	for (i = 0; i < num_clks; i++)
-		clk_data->clk_data.hws[i] = ERR_PTR(-ENOENT);
-
-	n5x_clk_register_pll(agilex_pll_clks, ARRAY_SIZE(agilex_pll_clks), clk_data);
+	clk_data = agilex_clkmgr_get_clk_data(pdev, n5x_clk_register_early);
+	if (IS_ERR(clk_data))
+		return PTR_ERR(clk_data);
 
 	n5x_clk_register_c_perip(n5x_main_perip_c_clks,
 				 ARRAY_SIZE(n5x_main_perip_c_clks), clk_data);
@@ -530,8 +662,8 @@ static int n5x_clkmgr_init(struct platform_device *pdev)
 
 	agilex_clk_register_gate(agilex_gate_clks, ARRAY_SIZE(agilex_gate_clks),
 			      clk_data);
-	of_clk_add_hw_provider(np, of_clk_hw_onecell_get, &clk_data->clk_data);
-	return 0;
+
+	return agilex_clkmgr_finish(pdev, clk_data);
 }
 
 static int agilex_clkmgr_probe(struct platform_device *pdev)
-- 
2.49.GIT

Re: [PATCH v3 1/3] clk: socfpga: agilex: register the SP timer clocks early

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] clk: socfpga: lockless publication of clk_hw pointers without memory barriers
- [Medium] clk: socfpga: silent failure on clock registration OOM due to missing NULL check
--

commit fc4b5a56305f3513e2f688af55d3564b1822bc8c
Author: Adrian Ng Ho Yin <adrian.ho.yin.ng@altera.com>

clk: socfpga: agilex: register the SP timer clocks early

The DW APB timers are probed from time_init() and cannot defer. This commit
registers l4_sp_clk and its parents early using CLK_OF_DECLARE_DRIVER(),
leaving the rest as -EPROBE_DEFER, which are subsequently registered when
the platform driver probes.

> diff --git a/drivers/clk/socfpga/clk-agilex.c b/drivers/clk/socfpga/clk-agilex.c
> index 2bdea1997b5ef..445d814e2f3ac 100644
> --- a/drivers/clk/socfpga/clk-agilex.c
> +++ b/drivers/clk/socfpga/clk-agilex.c
[ ... ]
> @@ -454,33 +472,166 @@ static int n5x_clk_register_pll(const struct stratix10_pll_clock *clks,
[ ... ]
> +static int agilex_clkmgr_add_provider(struct device_node *np)
> +{
> +	static bool added;
> +	int ret;
> +
> +	if (added)
> +		return 0;
> +
> +	ret = of_clk_add_hw_provider(np, of_clk_hw_onecell_get,
> +				     &agilex_clk_data->clk_data);

[Severity: High]
Does exposing the clock data array here during early boot allow
concurrent readers to see uninitialized memory?

The global array is exposed via of_clk_add_hw_provider() before all
clocks are populated. Later, during the platform driver probe in
agilex_clkmgr_init(), the remaining clocks are initialized and their
pointers are published locklessly:

drivers/clk/socfpga/clk-agilex.c:agilex_clk_register_c_perip() {
	...
	hw_clk = s10_register_periph(&clks[i], base);
	...
	data->clk_data.hws[clks[i].id] = hw_clk;
}

Since the array is mutated without memory barriers like smp_store_release()
or shared locks, could weakly ordered architectures reorder the publication
of the pointer before the initialization of the memory?

If a concurrent consumer requests a clock during this window, they might read
the new hw_clk pointer but observe uninitialized memory inside the clk_hw
structure, potentially leading to data corruption or a kernel panic.

[ ... ]
> +static int agilex_clkmgr_finish(struct platform_device *pdev,
> +				struct stratix10_clock_data *clk_data)
> +{
> +	int i;
> +
> +	for (i = 0; i < AGILEX_NUM_CLKS; i++)
> +		if (clk_data->clk_data.hws[i] == ERR_PTR(-EPROBE_DEFER))
> +			clk_data->clk_data.hws[i] = ERR_PTR(-ENOENT);

[Severity: High]
Is it safe to mutate the globally exposed hws array locklessly here?

This loop is running during the delayed platform probe, modifying the array
while consumers might be concurrently accessing it.

> +
> +	return agilex_clkmgr_add_provider(pdev->dev.of_node);
> +}
> +
> +static int agilex_clkmgr_init(struct platform_device *pdev)
> +{
> +	struct stratix10_clock_data *clk_data;
> +
> +	clk_data = agilex_clkmgr_get_clk_data(pdev, agilex_clk_register_early);
> +	if (IS_ERR(clk_data))
> +		return PTR_ERR(clk_data);
> +
>  	agilex_clk_register_c_perip(agilex_main_perip_c_clks,
>  				 ARRAY_SIZE(agilex_main_perip_c_clks), clk_data);

[Severity: Medium]
Does the missing NULL check in agilex_clk_register_c_perip() cause a silent
failure on memory allocation errors?

In agilex_clk_register_c_perip(), the return value of s10_register_periph()
is checked with IS_ERR():

drivers/clk/socfpga/clk-agilex.c:agilex_clk_register_c_perip() {
	...
	hw_clk = s10_register_periph(&clks[i], base);
	if (IS_ERR(hw_clk)) {
		pr_err("%s: failed to register clock %s\n",
		       __func__, clks[i].name);
		continue;
	}
	data->clk_data.hws[clks[i].id] = hw_clk;
}

However, s10_register_periph() returns NULL on memory allocation failure.
This means the IS_ERR() check evaluates to false, failing to catch the error.

Could this cause NULL to silently overwrite the original -EPROBE_DEFER in the
hws array? If consumers request this clock, they would receive a NULL clock
pointer, which the common clock framework treats as a valid optional dummy
clock. This masks the initialization failure and might allow dependent hardware
to proceed without being clocked.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1791871090.git.adrian.ho.yin.ng@altera.com?part=1

Re: [PATCH v3 1/3] clk: socfpga: agilex: register the SP timer clocks early

[Dinh Nguyen]

On 10/13/26 02:07, Adrian Ng Ho Yin wrote:
> The DW APB timers are probed through TIMER_OF_DECLARE() from time_init()
> and cannot defer, but the clock manager is a platform driver registered
> at core_initcall(). The timer's clk_get() therefore fails and the timer
> is never brought up.
> 
> Register only l4_sp_clk and its parents (boot_clk, main_pll, periph_pll,
> main_pll_c1, peri_pll_c1 and noc_free_clk) from of_clk_init() using
> CLK_OF_DECLARE_DRIVER(), with every other clock left as -EPROBE_DEFER.
> The remaining clocks are still registered when the platform driver
> probes, which reuses the early provider.
> 
> Signed-off-by: Adrian Ng Ho Yin <adrian.ho.yin.ng@altera.com>
> ---
>   drivers/clk/socfpga/clk-agilex.c | 242 ++++++++++++++++++++++++-------
>   1 file changed, 187 insertions(+), 55 deletions(-)
> 
I think this needs a Fixes tag:

Fixes: 80c6b7a0894f ("clk: socfpga: agilex: add clock driver for the 
Agilex platform")

and the Cc: stable

Other than that, it looks good to me.

Dinh