Re: [PATCH V2] drivers: clk: zynqmp: remove clock name dependency

[Stephen Boyd <sboyd@kernel.org>]

Quoting Naman Trivedi (2024-07-22 05:19:10)
> From: Naman Trivedi Manojbhai <naman.trivedimanojbhai@amd.com>
> 
> Use struct clk_parent_data to register the clock parents with the clock
> framework instead of parent name.
> 
> Signed-off-by: Naman Trivedi Manojbhai <naman.trivedimanojbhai@amd.com>

This is great! Thanks for doing this.

> diff --git a/drivers/clk/zynqmp/clk-gate-zynqmp.c b/drivers/clk/zynqmp/clk-gate-zynqmp.c
> index b89e55737198..6bb9704ee1d3 100644
> --- a/drivers/clk/zynqmp/clk-gate-zynqmp.c
> +++ b/drivers/clk/zynqmp/clk-gate-zynqmp.c
> @@ -104,8 +104,8 @@ static const struct clk_ops zynqmp_clk_gate_ops = {
>   *
>   * Return: clock hardware of the registered clock gate
>   */
> -struct clk_hw *zynqmp_clk_register_gate(const char *name, u32 clk_id,
> -                                       const char * const *parents,
> +struct clk_hw *zynqmp_clk_register_gate(struct device_node *np, const char *name, u32 clk_id,

General comment: Please use 'struct device' instead so that this driver
isn't DT specific. When you do that you can similarly use
devm_clk_hw_register() instead and introduce a lot of automatic cleanup.
If you want to do that in two steps that's fine. One patch that uses
parent_data/parent_hws and one that uses devm_ APIs and struct device to
register.

> diff --git a/drivers/clk/zynqmp/clkc.c b/drivers/clk/zynqmp/clkc.c
> index a91d98e238c2..b791a459280e 100644
> --- a/drivers/clk/zynqmp/clkc.c
> +++ b/drivers/clk/zynqmp/clkc.c
> @@ -543,7 +554,7 @@ static int zynqmp_clock_get_parents(u32 clk_id, struct clock_parent *parents,
>   * Return: 0 on success else error+reason
>   */
>  static int zynqmp_get_parent_list(struct device_node *np, u32 clk_id,
> -                                 const char **parent_list, u32 *num_parents)
> +                                 struct clk_parent_data *parent_list, u32 *num_parents)
>  {
>         int i = 0, ret;
>         u32 total_parents = clock[clk_id].num_parents;
> @@ -555,18 +566,30 @@ static int zynqmp_get_parent_list(struct device_node *np, u32 clk_id,
>  
>         for (i = 0; i < total_parents; i++) {
>                 if (!parents[i].flag) {
> -                       parent_list[i] = parents[i].name;
> +                       ret = of_property_match_string(np, "clock-names",
> +                                                      parents[i].name);

You shouldn't need to match 'clock-names'. The order of that property is
fixed in the binding, which means you can simply use the index that the
name is at in the binding in 'struct parent_data'.