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

From: "Sa, Nuno" <Nuno.Sa@analog.com>
To: Andy Shevchenko <andy.shevchenko@gmail.com>
Cc: "linux-arm-kernel@lists.infradead.org"
	<linux-arm-kernel@lists.infradead.org>,
	"linux-rockchip@lists.infradead.org"
	<linux-rockchip@lists.infradead.org>,
	"linux-amlogic@lists.infradead.org"
	<linux-amlogic@lists.infradead.org>,
	"linux-imx@nxp.com" <linux-imx@nxp.com>,
	"linux-iio@vger.kernel.org" <linux-iio@vger.kernel.org>,
	Chunyan Zhang <zhang.lyra@gmail.com>,
	"Hennerich, Michael" <Michael.Hennerich@analog.com>,
	Martin Blumenstingl <martin.blumenstingl@googlemail.com>,
	Sascha Hauer <s.hauer@pengutronix.de>,
	Cixi Geng <cixi.geng1@unisoc.com>,
	Kevin Hilman <khilman@baylibre.com>,
	Vladimir Zapolskiy <vz@mleia.com>,
	Pengutronix Kernel Team <kernel@pengutronix.de>,
	Alexandru Ardelean <aardelean@deviqon.com>,
	Fabio Estevam <festevam@gmail.com>,
	Andriy Tryshnivskyy <andriy.tryshnivskyy@opensynergy.com>,
	Haibo Chen <haibo.chen@nxp.com>, Shawn Guo <shawnguo@kernel.org>,
	Hans de Goede <hdegoede@redhat.com>,
	Miquel Raynal <miquel.raynal@bootlin.com>,
	Jerome Brunet <jbrunet@baylibre.com>,
	Heiko Stuebner <heiko@sntech.de>,
	Florian Boor <florian.boor@kernelconcepts.de>,
	"Regus, Ciprian" <Ciprian.Regus@analog.com>,
	Lars-Peter Clausen <lars@metafoo.de>,
	Jonathan Cameron <jic23@kernel.org>,
	Neil Armstrong <narmstrong@baylibre.com>,
	Baolin Wang <baolin.wang@linux.alibaba.com>,
	Jyoti Bhayana <jbhayana@google.com>, Chen-Yu Tsai <wens@csie.org>,
	Orson Zhai <orsonzhai@gmail.com>
Subject: RE: [PATCH 03/15] iio: adc: axp288_adc: do not use internal iio_dev lock
Date: Tue, 20 Sep 2022 13:46:31 +0000	[thread overview]
Message-ID: <SJ0PR03MB6778EEF81693C026CE131B49994C9@SJ0PR03MB6778.namprd03.prod.outlook.com> (raw)
In-Reply-To: <CAHp75VfdUOTd9tRXdTVq_prNf_T2=iQ4tEjusHC=gXDZvb3Qeg@mail.gmail.com>

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

> 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á
_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic