Re: [PATCH 1/7] iio: frequency: adrf6780: use dev_err_probe in probe path

From: Andy Shevchenko

Date: Fri Feb 20 2026 - 09:52:13 EST


On Fri, Feb 20, 2026 at 02:41:41PM +0000, Miclaus, Antoniu wrote:
> > From: Andy Shevchenko <andriy.shevchenko@xxxxxxxxx>
> > Sent: Friday, February 20, 2026 3:59 PM
> > On Fri, Feb 20, 2026 at 03:56:58PM +0200, Andy Shevchenko wrote:
> > > On Fri, Feb 20, 2026 at 03:11:39PM +0200, Antoniu Miclaus wrote:

...

> > > > ret = __adrf6780_spi_update_bits(st, ADRF6780_REG_CONTROL,
> > > > ADRF6780_SOFT_RESET_MSK,
> > > >
> > FIELD_PREP(ADRF6780_SOFT_RESET_MSK, 1));
> > > > - if (ret) {
> > > > - dev_err(&spi->dev, "ADRF6780 SPI software reset failed.\n");
> > > > - return ret;
> > > > - }
> > > > + if (ret)
> > > > + return dev_err_probe(&spi->dev, ret,
> > > > + "ADRF6780 SPI software reset failed.\n");
> > >
> > > Consider adding
> > >
> > > struct device *dev = &spi->dev;
>
> Definitely makes sense, but for me feels a bit out of the scope of this series.

I would use it as ad-hoc material and wouldn't bother of a separate series.
Otherwise it will be a churn of modifying the same line twice.

That's why I consider this as a good compromise (we modify 3 cases here
if I'm not mistaken).

> Want me to add the *dev = &spi->dev; here or in a separate patchseries?
>
> > > at the top, so this line becomes
> > >
> > > return dev_err_probe(dev, ret, "ADRF6780 SPI software reset
> > failed.\n");
> > >
> > > (while at it, I would dare to drop 'SPI' from the message as is easy to derive
> > that
> > > the device in question is connected to SPI bus.)
> > >
> > > return dev_err_probe(dev, ret, "ADRF6780 software reset
> > failed.\n");

...

> > > Ditto for the rest.
> >
> > And also for the entire series (except patch 5 where it seems already
> > the case).

--
With Best Regards,
Andy Shevchenko