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

[Jonathan Cameron]

On Wed, 05 Oct 2022 10:37:39 +0200
Nuno Sá <noname.nuno at 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 at 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
>