On Wed, 21 Sep 2022 11:07:50 +0200 Nuno Sá <noname.nuno@xxxxxxxxx> wrote: > 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@xxxxxxxxxx> wrote: > > > > -----Original Message----- > > > > From: Andy Shevchenko <andy.shevchenko@xxxxxxxxx> > > > > Sent: Tuesday, September 20, 2022 3:39 PM > > > > On Tue, Sep 20, 2022 at 4:37 PM Andy Shevchenko > > > > <andy.shevchenko@xxxxxxxxx> wrote: > > > > > On Tue, Sep 20, 2022 at 4:18 PM Sa, Nuno <Nuno.Sa@xxxxxxxxxx> > > > > > wrote: > > > > > > > On Tue, Sep 20, 2022 at 2:28 PM Nuno Sá > > > > > > > <nuno.sa@xxxxxxxxxx> > > > > 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... Can't say I feel that strongly about a mutex_init() placement, but no harm in moving them later - indeed before the iio_device_register() should be correct - though care needed as might be some unnecessary locks taken in probe because of code sharing (and them previously being harmless) Jonathan > > - Nuno Sá > > >
