Re: [PATCH v9 6/7] iio: adc: ad9467: convert to backend framework

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, 07 Feb 2024 10:23:36 +0100
Nuno Sá <noname.nuno@xxxxxxxxx> wrote:

> On Tue, 2024-02-06 at 19:51 +0200, Andy Shevchenko wrote:
> > On Tue, Feb 6, 2024 at 6:51 PM Nuno Sá <noname.nuno@xxxxxxxxx> wrote:  
> > > On Tue, 2024-02-06 at 16:20 +0200, Andy Shevchenko wrote:  
> > > > On Tue, Feb 6, 2024 at 12:08 PM Nuno Sa via B4 Relay
> > > > <devnull+nuno.sa.analog.com@xxxxxxxxxx> wrote:  
> > 
> > ...
> >   
> > > > > +       st->back = devm_iio_backend_get(&st->spi->dev, NULL);
> > > > > +       /* If not found, don't error out as we might have legacy DT
> > > > > property */
> > > > > +       if (!IS_ERR(st->back))
> > > > > +               return 0;
> > > > > +       if (PTR_ERR(st->back) != -ENOENT)
> > > > > +               return PTR_ERR(st->back);  
> > > > 
> > > > This looks hackish...  
> > > 
> > > This was suggested by Jonathan so I'm keen in leaving it as-is  
> > 
> > I think I proposed how to improve it. Jonathan?
> >   
> 
> Well, you did change it around so that we have the typical 'if (ret)' pattern.
> But, honestly, that is also bringing an arguably useless helper. So, yes,
> currently we are not doing the typical "check for errors first" pattern but I do
> think that the code is perfectly fine and readable as it is.
> 
> It's also already pretty late in the review process of the series and there was
> already some back and forth so to me, the above is really a minor detail and I'm
> not keen on changing it at this stage (unless Jonathan really feels strong about
> doing it).
It's odd code I don't expect to see anywhere else, so I'm not that fussed if
it takes a little more thinking than normal (and you have a comment there to help
us!)

So I'm fine with leaving this as it stands

Jonathan

> 
> I'll spin a v10 with your dev_err_probe() suggestion later today but at this
> point I would prefer to not have to spin another one (afterwards) unless really
> needed (meaning, not because of minor details).
> 
> - Nuno Sá






[Index of Archives]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Input]     [Linux Kernel]     [Linux SCSI]     [X.org]

  Powered by Linux