Re: [PATCH v2 13/16] iio: core: introduce iio_device_{claim|release}_buffer_mode() APIs

From: Jonathan Cameron <jic23@kernel.org>
To: "Nuno Sá" <noname.nuno@gmail.com>
Cc: "Andy Shevchenko" <andy.shevchenko@gmail.com>,
	"Nuno Sá" <nuno.sa@analog.com>,
	linux-amlogic@lists.infradead.org, linux-imx@nxp.com,
	linux-iio@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
	linux-rockchip@lists.infradead.org,
	"Heiko Stuebner" <heiko@sntech.de>,
	"Martin Blumenstingl" <martin.blumenstingl@googlemail.com>,
	"Neil Armstrong" <narmstrong@baylibre.com>,
	"Shawn Guo" <shawnguo@kernel.org>,
	"Lars-Peter Clausen" <lars@metafoo.de>,
	"Jyoti Bhayana" <jbhayana@google.com>,
	"Hans de Goede" <hdegoede@redhat.com>,
	"Andriy Tryshnivskyy" <andriy.tryshnivskyy@opensynergy.com>,
	"Pengutronix Kernel Team" <kernel@pengutronix.de>,
	"Miquel Raynal" <miquel.raynal@bootlin.com>,
	"Cixi Geng" <cixi.geng1@unisoc.com>,
	"Baolin Wang" <baolin.wang@linux.alibaba.com>,
	"Ciprian Regus" <ciprian.regus@analog.com>,
	"Fabio Estevam" <festevam@gmail.com>,
	"Sascha Hauer" <s.hauer@pengutronix.de>,
	"Alexandru Ardelean" <aardelean@deviqon.com>,
	"Florian Boor" <florian.boor@kernelconcepts.de>,
	"Michael Hennerich" <Michael.Hennerich@analog.com>,
	"Orson Zhai" <orsonzhai@gmail.com>,
	"Chen-Yu Tsai" <wens@csie.org>,
	"Chunyan Zhang" <zhang.lyra@gmail.com>,
	"Vladimir Zapolskiy" <vz@mleia.com>,
	"Jerome Brunet" <jbrunet@baylibre.com>,
	"Haibo Chen" <haibo.chen@nxp.com>,
	"Kevin Hilman" <khilman@baylibre.com>
Subject: Re: [PATCH v2 13/16] iio: core: introduce iio_device_{claim|release}_buffer_mode() APIs
Date: Sun, 9 Oct 2022 12:41:20 +0100	[thread overview]
Message-ID: <20221009124120.39a43103@jic23-huawei> (raw)
In-Reply-To: <609d7e50d76a2a2aa2bbaddfed8a846c91854065.camel@gmail.com>

On Wed, 05 Oct 2022 10:37:39 +0200
Nuno Sá <noname.nuno@gmail.com> wrote:

> On Tue, 2022-10-04 at 17:08 +0300, Andy Shevchenko wrote:
> > On Tue, Oct 4, 2022 at 4:49 PM Nuno Sá <nuno.sa@analog.com> wrote:  
> > > 
> > > These APIs are analogous to iio_device_claim_direct_mode() and
> > > iio_device_release_direct_mode() but, as the name suggests, with
> > > the
> > > logic flipped. While this looks odd enough, it will have at least
> > > two
> > > users (in following changes) and it will be important to move the
> > > iio
> > > mlock to the private struct.  
> > 
> > ...
> >   
> > > +int iio_device_claim_buffer_mode(struct iio_dev *indio_dev)
> > > +{
> > > +       mutex_lock(&indio_dev->mlock);
> > > +
> > > +       if (iio_buffer_enabled(indio_dev))  
> > 
> > Do you need to annotate these two APIs to make sparse happy about
> > locking balance?
> > 
> > (Try to run `make W=1 C=1 ...` with your patches and look if any new
> > warnings appear.)  
> 
> make W=1 C=1 drivers/iio/industrialio-core.o
> #  UPD     include/config/kernel.release

...

> drivers/iio/industrialio-core.c:2100: warning: expecting prototype for
> iio_device_claim_buffered_mode(). Prototype was for
> iio_device_claim_buffer_mode() instead

That one wants fixing as this patch introduces it.

> 
> Don't really see anything odd in here... Am I missing something? 
> 
> Anyways, I guess you mean annotations as __acquires() and
> __releases()... Well, this API is pretty much analogous to
> iio_device_claim_direct_mode() which also don't have such annotations.
> Thus, I would say to add them (if we are going too) in a future patch
> to both APIs...
> 
> Also fine with adding them now if Jonathan feels it's necessary.

I've wondered for a while why we don't get reports as
a result of those not being annotated.  However, follow up patch
probably makes sense rather than rolling it into this series.

Jonathan
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel