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

[Stephen Boyd]

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.