Re: [PATCH v4 RESEND] clk: socfpga: agilex: add support for the Intel Agilex5

[Stephen Boyd <sboyd@kernel.org>]

Quoting Matthew Gerlach (2025-04-17 07:52:38)
> From: Niravkumar L Rabara <niravkumar.l.rabara@intel.com>
> 
> Add support for Intel's SoCFPGA Agilex5 platform. The clock manager
> driver for the Agilex5 is very similar to the Agilex platform, so
> it is reusing most of the Agilex clock driver code.
> 
> Signed-off-by: Teh Wen Ping <wen.ping.teh@intel.com>
> Reviewed-by: Dinh Nguyen <dinguyen@kernel.org>
> Signed-off-by: Niravkumar L Rabara <niravkumar.l.rabara@intel.com>
> Signed-off-by: Matthew Gerlach <matthew.gerlach@altera.com>
> ---
> Changes in v4:
> - Add .index to clk_parent_data.

It's useful to link to the previous round with lore links. Please do it
next time.

> 
> Changes in v3:
> - Used different name for stratix10_clock_data pointer.
> - Used a single function call, devm_platform_ioremap_resource().
> - Used only .name in clk_parent_data.
> 
> Stephen suggested to use .fw_name or .index, But since the changes are on top
> of existing driver and current driver code is not using clk_hw and removing
> .name and using .fw_name and/or .index resulting in parent clock_rate &
> recalc_rate to 0.
> 
> diff --git a/drivers/clk/socfpga/clk-agilex.c b/drivers/clk/socfpga/clk-agilex.c
> index 8dd94f64756b..a5ed2a22426e 100644
> --- a/drivers/clk/socfpga/clk-agilex.c
> +++ b/drivers/clk/socfpga/clk-agilex.c
> @@ -334,6 +336,375 @@ static const struct stratix10_gate_clock agilex_gate_clks[] = {
>           10, 0, 0, 0, 0, 0, 4},
>  };
>  
> +static const struct clk_parent_data agilex5_pll_mux[] = {
> +       { .name = "osc1", .index = AGILEX5_OSC1, },
> +       { .name = "cb-intosc-hs-div2-clk", .index = AGILEX5_CB_INTOSC_HS_DIV2_CLK, },
> +       { .name = "f2s-free-clk", .index = AGILEX5_F2S_FREE_CLK, },
> +};
> +
> +static const struct clk_parent_data agilex5_boot_mux[] = {
> +       { .name = "osc1", .index = AGILEX5_OSC1, },
> +       { .name = "cb-intosc-hs-div2-clk", .index = AGILEX5_CB_INTOSC_HS_DIV2_CLK, },
> +};
> +
> +static const struct clk_parent_data agilex5_core0_free_mux[] = {
> +       { .name = "main_pll_c1", .index = AGILEX5_MAIN_PLL_C1_CLK, },
> +       { .name = "peri_pll_c0", .index = AGILEX5_MAIN_PLL_C0_CLK, },

The index doesn't work this way. The number indicates which index in the
DT node's 'clocks' property to use as the parent. It doesn't indicate
which index in this clk provider to use. I don't see any 'clocks'
property in the binding for this compatible "intel,agilex5-clkmgr", so
this doesn't make any sense either.

If you can't use clk_hw pointers then just stick to the old way of doing
it with string names and no struct clk_parent_data usage.

> +       { .name = "osc1", .index = AGILEX5_OSC1, },
> +       { .name = "cb-intosc-hs-div2-clk", .index = AGILEX5_CB_INTOSC_HS_DIV2_CLK, },
> +       { .name = "f2s-free-clk", .index = AGILEX5_F2S_FREE_CLK, },
> +};
> +