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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <ac67f76d4b4b5f4bf108c1457f1263c7.sboyd@kernel.org>
Date: Thu, 08 Aug 2024 15:48:03 -0700
From: Stephen Boyd <sboyd@...nel.org>
To: Naman Trivedi <naman.trivedimanojbhai@....com>, linux-arm-kernel@...ts.infradead.org, linux-clk@...r.kernel.org, michal.simek@....com, mturquette@...libre.com, senthilnathan.thangaraj@....com
Cc: linux-kernel@...r.kernel.org, Naman Trivedi Manojbhai <naman.trivedimanojbhai@....com>
Subject: Re: [PATCH V2] drivers: clk: zynqmp: remove clock name dependency

Quoting Naman Trivedi (2024-07-22 05:19:10)
> From: Naman Trivedi Manojbhai <naman.trivedimanojbhai@....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@....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'.