Re: [PATCH v3 2/2] iio: frequency: adf4377: add clk provider support

[Andy Shevchenko]

On Tue, Nov 25, 2025 at 6:21 PM Andy Shevchenko
<andy.shevchenko@xxxxxxxxx> wrote:
> On Tue, Nov 25, 2025 at 5:48 PM Antoniu Miclaus
> <antoniu.miclaus@xxxxxxxxxx> wrote:

...

> > +static int adf4377_clk_register(struct adf4377_state *st)
> > +{
> > +       struct spi_device *spi = st->spi;

Also consider to have

  struct device *dev = &spi->dev;

here, and use it below to make less LoCs and better to read code.

> > +       struct clk_init_data init;
> > +       struct clk_parent_data parent_data;
> > +       int ret;
> > +
> > +       if (!device_property_present(&spi->dev, "#clock-cells"))
> > +               return 0;
> > +
> > +       if (device_property_read_string(&spi->dev, "clock-output-names", &init.name)) {
>
> I would rather split this to two
>
>   ret = device_property_read_string(...);
>   if (ret) {
>     ...
>   }
>
> which makes the intention more explicit, otherwise it's not clear what
> the return values are and what you do with them (since I know it's
> int, the rule of thumb to assign to int variable and check it, for
> boolean it's fine to do if (boolean_fn(...)) in the code).
>
> > +               init.name = devm_kasprintf(&spi->dev, GFP_KERNEL, "%s-clk",
> > +                                          fwnode_get_name(dev_fwnode(&spi->dev)));
>
> %pfw / %pfwP
>
> > +               if (!init.name)
> > +                       return -ENOMEM;
> > +       }
> > +
> > +       parent_data.fw_name = "ref_in";
> > +
> > +       init.ops = &adf4377_clk_ops;
> > +       init.parent_data = &parent_data;
> > +       init.num_parents = 1;
> > +       init.flags = CLK_SET_RATE_PARENT;
> > +
> > +       st->hw.init = &init;
> > +       ret = devm_clk_hw_register(&spi->dev, &st->hw);
> > +       if (ret)
> > +               return ret;
> > +
> > +       ret = devm_of_clk_add_hw_provider(&spi->dev, of_clk_hw_simple_get, &st->hw);
> > +       if (ret)
> > +               return ret;
> > +
> > +       st->clkout = st->hw.clk;
> > +
> > +       return 0;
> > +}
>
> Otherwise LGTM.
>
> --
> With Best Regards,
> Andy Shevchenko



-- 
With Best Regards,
Andy Shevchenko