Re: [PATCH v2] of: remove internal arguments from of_property_for_each_u32()

[Stephen Boyd]

Quoting Luca Ceresoli (2024-07-17 09:16:32)
> diff --git a/drivers/clk/clk-si5351.c b/drivers/clk/clk-si5351.c
> index 4ce83c5265b8..d4904f59f83f 100644
> --- a/drivers/clk/clk-si5351.c
> +++ b/drivers/clk/clk-si5351.c
> @@ -1175,8 +1175,8 @@ static int si5351_dt_parse(struct i2c_client *client,
>  {
>         struct device_node *child, *np = client->dev.of_node;
>         struct si5351_platform_data *pdata;
> -       struct property *prop;
> -       const __be32 *p;
> +       u32 array[4];
> +       int sz, i;
>         int num = 0;
>         u32 val;
>  
> @@ -1191,20 +1191,24 @@ static int si5351_dt_parse(struct i2c_client *client,
>          * property silabs,pll-source : <num src>, [<..>]
>          * allow to selectively set pll source
>          */
> -       of_property_for_each_u32(np, "silabs,pll-source", prop, p, num) {
> +       sz = of_property_read_variable_u32_array(np, "silabs,pll-source", array, 2, 4);
> +       sz = (sz == -EINVAL) ? 0 : sz; /* Missing property is OK */
> +       if (sz < 0)
> +               return dev_err_probe(&client->dev, sz, "invalid pll-source");

Needs a newline on the printk message.

> +       if (sz % 2)
> +               return dev_err_probe(&client->dev, -EINVAL,
> +                                    "missing pll-source for pll %d\n", array[sz - 1]);
> +
> +       for (i = 0; i < sz; i += 2) {
> +               num = array[i];
> +               val = array[i + 1];
> +
>                 if (num >= 2) {
>                         dev_err(&client->dev,
>                                 "invalid pll %d on pll-source prop\n", num);
>                         return -EINVAL;
>                 }
>  
> -               p = of_prop_next_u32(prop, p, &val);
> -               if (!p) {
> -                       dev_err(&client->dev,
> -                               "missing pll-source for pll %d\n", num);
> -                       return -EINVAL;
> -               }
> -
>                 switch (val) {
>                 case 0:
>                         pdata->pll_src[num] = SI5351_PLL_SRC_XTAL;
> @@ -1232,19 +1236,24 @@ static int si5351_dt_parse(struct i2c_client *client,
>         pdata->pll_reset[0] = true;
>         pdata->pll_reset[1] = true;
>  
> -       of_property_for_each_u32(np, "silabs,pll-reset-mode", prop, p, num) {
> +       sz = of_property_read_variable_u32_array(np, "silabs,pll-reset-mode", array, 2, 4);
> +       sz = (sz == -EINVAL) ? 0 : sz; /* Missing property is OK */
> +       if (sz < 0)
> +               return dev_err_probe(&client->dev, sz, "invalid pll-reset-mode");

Needs a newline on the printk message.

> +       if (sz % 2)
> +               return dev_err_probe(&client->dev, -EINVAL,
> +                                    "missing pll-reset-mode for pll %d\n", array[sz - 1]);
> +

With those fixed

Acked-by: Stephen Boyd <sboyd@xxxxxxxxxx> # clk