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

[Sa, Nuno]

> -----Original Message-----
> From: Andy Shevchenko <andy.shevchenko at gmail.com>
> Sent: Tuesday, September 20, 2022 3:39 PM
> To: Sa, Nuno <Nuno.Sa at analog.com>
> Cc: linux-arm-kernel at lists.infradead.org; linux-rockchip at lists.infradead.org;
> linux-amlogic at lists.infradead.org; linux-imx at nxp.com; linux-
> iio at vger.kernel.org; Chunyan Zhang <zhang.lyra at gmail.com>; Hennerich,
> Michael <Michael.Hennerich at analog.com>; Martin Blumenstingl
> <martin.blumenstingl at googlemail.com>; Sascha Hauer
> <s.hauer at pengutronix.de>; Cixi Geng <cixi.geng1 at unisoc.com>; Kevin
> Hilman <khilman at baylibre.com>; Vladimir Zapolskiy <vz at mleia.com>;
> Pengutronix Kernel Team <kernel at pengutronix.de>; Alexandru Ardelean
> <aardelean at deviqon.com>; Fabio Estevam <festevam at gmail.com>; Andriy
> Tryshnivskyy <andriy.tryshnivskyy at opensynergy.com>; Haibo Chen
> <haibo.chen at nxp.com>; Shawn Guo <shawnguo at kernel.org>; Hans de
> Goede <hdegoede at redhat.com>; Miquel Raynal
> <miquel.raynal at bootlin.com>; Jerome Brunet <jbrunet at baylibre.com>;
> Heiko Stuebner <heiko at sntech.de>; Florian Boor
> <florian.boor at kernelconcepts.de>; Regus, Ciprian
> <Ciprian.Regus at analog.com>; Lars-Peter Clausen <lars at metafoo.de>;
> Jonathan Cameron <jic23 at kernel.org>; Neil Armstrong
> <narmstrong at baylibre.com>; Baolin Wang
> <baolin.wang at linux.alibaba.com>; Jyoti Bhayana <jbhayana at google.com>;
> Chen-Yu Tsai <wens at csie.org>; Orson Zhai <orsonzhai at gmail.com>
> Subject: Re: [PATCH 03/15] iio: adc: axp288_adc: do not use internal iio_dev
> lock
> 
> [External]
> 
> On Tue, Sep 20, 2022 at 4:37 PM Andy Shevchenko
> <andy.shevchenko at gmail.com> wrote:
> > On Tue, Sep 20, 2022 at 4:18 PM Sa, Nuno <Nuno.Sa at analog.com> wrote:
> > > > On Tue, Sep 20, 2022 at 2:28 PM Nuno Sá <nuno.sa at 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...

> Note that "micro" in the above can be multiplied by 'k', where 'k' is
> the amount of deferred probes (probably not the case here, but again,
> "generally speaking").
> 

Well, I don't think 'mutex_init()' does that much that really matters in
these patches but ok, that rule is indeed a good practice that sometimes
makes a difference. And since I will definitely need a v2, I can make that
change. Where applicable, the best place is probably before registering the
IIO device...

- Nuno Sá