Re: [PATCH 03/15] iio: adc: axp288_adc: do not use internal iio_dev lock

["Nuno Sá" <noname.nuno@gmail.com>]

On Tue, 2022-09-20 at 18:12 +0300, Andy Shevchenko wrote:
> On Tue, Sep 20, 2022 at 4:46 PM Sa, Nuno <Nuno.Sa@analog.com> wrote:
> > > -----Original Message-----
> > > From: Andy Shevchenko <andy.shevchenko@gmail.com>
> > > Sent: Tuesday, September 20, 2022 3:39 PM
> > > On Tue, Sep 20, 2022 at 4:37 PM Andy Shevchenko
> > > <andy.shevchenko@gmail.com> wrote:
> > > > On Tue, Sep 20, 2022 at 4:18 PM Sa, Nuno <Nuno.Sa@analog.com>
> > > > wrote:
> > > > > > On Tue, Sep 20, 2022 at 2:28 PM Nuno Sá
> > > > > > <nuno.sa@analog.com>
> > > wrote:
> > > 
> > > ...
> > > 
> > > > > > >         info = iio_priv(indio_dev);
> > > > > > > +       mutex_init(&info->lock);
> > > > > > >         info->irq = platform_get_irq(pdev, 0);
> > > > > > >         if (info->irq < 0)
> > > > > > >                 return info->irq;
> > > > > > 
> > > > > > Consider initializing it as late as possible, like after
> > > > > > IRQ retrieval
> > > > > > in this context (maybe even deeper, but no context
> > > > > > available). Ditto
> > > > > > for the rest of the series.
> > > > > 
> > > > > Any special reason for it (maybe related to lockdep
> > > > > :wondering: ) ? Just
> > > > > curious as I never noticed such a pattern when initializing
> > > > > mutexes.
> > > > 
> > > > Yes. Micro-optimization based on the rule "don't create a
> > > > resource in
> > > > case of known error".
> > > > 
> > > > OTOH, you have to be sure that the mutex (and generally
> > > > speaking a
> > > > locking) should be initialized early enough.
> > 
> > Typically not really needed during probe...
> 
> Actually as long as you expose the ABI to the user anything can
> happen, means that your code should be ready to receive the requests
> in any possible callbacks / file ops. Same applies to the IRQ
> handler.
> So it's very important to initialize locking just in time. Hence I
> can
> say that "typically it needs to be carefully placed".
> 

Yes, I'm aware of that... For some reason I just thought about code
paths directly on probe. Anyways, hopefully these drivers mostly do the
right thing and register the IIO device as late as possible (ideally
the last thing to be done). The same goes for IRQs and for IIO, when
used as part of triggered buffering, the lock is often only used in the
trigger handler which means it's only reachable after the ABI is
exposed... 

- Nuno Sá
> >