On Sat, Aug 13, 2022 at 7:26 PM Jonathan Cameron <jic23@xxxxxxxxxx> wrote: > > On Mon, 8 Aug 2022 22:47:38 +0200 > Uwe Kleine-König <u.kleine-koenig@xxxxxxxxxxxxxx> wrote: > > > Make use of devm_clk_get_enabled() to replace some code that effectively > > open codes this new function. > > > > Reviewed-by: Andy Shevchenko <andy.shevchenko@xxxxxxxxx> > > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@xxxxxxxxxxxxxx> > Looks fine to me, but there is a subtle reordering + it does even more > non parsing stuff in a function called _parse. > > Anyhow, would like Antoniu or someone else from ADI to take a quick look if > possible before I pick this one up. ... > > - st->clkin = devm_clk_get(&spi->dev, "lo_in"); > > + st->clkin = devm_clk_get_enabled(&spi->dev, "lo_in"); > > if (IS_ERR(st->clkin)) > > return dev_err_probe(&spi->dev, PTR_ERR(st->clkin), > > "failed to get the LO input clock\n"); So it seems better to drop above and... > > - ret = clk_prepare_enable(st->clkin); > > - if (ret) > > - return ret; > > - > > - ret = devm_add_action_or_reset(&spi->dev, admv1013_clk_disable, st->clkin); > > - if (ret) > > - return ret; ...put a call here. This will make parse() look more parse. -- With Best Regards, Andy Shevchenko
