linux-kernel - Re: [PATCH v2 2/2] clk: clk-conf: support assigned-clock-rates-u64

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] [day] [month] [year] [list]

Message-ID: <4bab63206ed25311b70bf5a0c98bc54e.sboyd@kernel.org>
Date: Mon, 29 Jul 2024 17:22:59 -0700
From: Stephen Boyd <sboyd@...nel.org>
To: Luca Ceresoli <luca.ceresoli@...tlin.com>, Michael Turquette <mturquette@...libre.com>, Peng Fan (OSS) <peng.fan@....nxp.com>, Rob Herring <robh@...nel.org>, Saravana Kannan <saravanak@...gle.com>
Cc: devicetree@...r.kernel.org, linux-kernel@...r.kernel.org, linux-clk@...r.kernel.org, Peng Fan <peng.fan@....com>
Subject: Re: [PATCH v2 2/2] clk: clk-conf: support assigned-clock-rates-u64

Quoting Peng Fan (OSS) (2024-07-28 19:30:53)
> diff --git a/drivers/clk/clk-conf.c b/drivers/clk/clk-conf.c
> index 058420562020..37b72600b296 100644
> --- a/drivers/clk/clk-conf.c
> +++ b/drivers/clk/clk-conf.c
> @@ -78,47 +78,89 @@ static int __set_clk_parents(struct device_node *node, bool clk_supplier)
[...]
> +static int __set_clk_rates(struct device_node *node, bool clk_supplier)
> +{
> +       int rc, index = 0;
> +       u64 rate;
> +       u32 rate_32;
> +       bool is_rate_32 = false;
> +
> +       if (!of_find_property(node, "assigned-clock-rates-u64", NULL))
> +               is_rate_32 = true;
> +
> +       if (is_rate_32) {
> +               of_property_for_each_u32(node, "assigned-clock-rates", rate_32) {
> +                       if (rate_32) {
> +                               rc = __set_clk_rate(node, clk_supplier, index, rate_32);
> +
> +                               if (rc == 1 && !clk_supplier)
> +                                       return 0;
> +
> +                               if (rc < 0) {
> +                                       /* skip empty (null) phandles */
> +                                       if (rc == -ENOENT)
> +                                               continue;
> +                                       else
> +                                               return rc;
> +                               }
>                         }
> +                       index++;
> +               }
> +       } else {
> +               of_property_for_each_u64(node, "assigned-clock-rates-u64", rate) {
> +                       if (rate) {
> +                               rc = __set_clk_rate(node, clk_supplier, index, rate);
>  
> -                       rc = clk_set_rate(clk, rate);
> -                       if (rc < 0)
> -                               pr_err("clk: couldn't set %s clk rate to %u (%d), current rate: %lu\n",
> -                                      __clk_get_name(clk), rate, rc,
> -                                      clk_get_rate(clk));
> -                       clk_put(clk);
> +                               if (rc == 1 && !clk_supplier)
> +                                       return 0;
> +
> +                               if (rc < 0) {
> +                                       /* skip empty (null) phandles */
> +                                       if (rc == -ENOENT)
> +                                               continue;
> +                                       else
> +                                               return rc;
> +                               }
> +                       }
> +                       index++;

This patch is hard to read. I suspect that's because this code to skip
empty phandles is repeated. Can you expand
of_property_for_each_u{32,64}() and combine the loops? Doing that should
allow us to avoid repeating the same logic twice.