Re: [PATCH v2 2/7] iio: core: Refactor iio_device_claim_direct() implementation

["Kurt Borja" <kuurtb@gmail.com>]

On Tue Dec 23, 2025 at 12:20 PM -05, David Lechner wrote:
> On 12/11/25 8:45 PM, Kurt Borja wrote:
>> In order to eventually unify the locking API, implement
>> iio_device_claim_direct() fully inline, with the use of
>> __iio_dev_mode_lock(), which takes care of sparse annotations.
>> 
>> Signed-off-by: Kurt Borja <kuurtb@gmail.com>
>> ---

...

>> diff --git a/include/linux/iio/iio.h b/include/linux/iio/iio.h
>> index aecda887d833..76398dbfa5ca 100644
>> --- a/include/linux/iio/iio.h
>> +++ b/include/linux/iio/iio.h
>> @@ -664,30 +664,42 @@ int iio_push_event(struct iio_dev *indio_dev, u64 ev_code, s64 timestamp);
>>  
>>  void __iio_dev_mode_lock(struct iio_dev *indio_dev) __acquires(indio_dev);
>>  void __iio_dev_mode_unlock(struct iio_dev *indio_dev) __releases(indio_dev);
>> -bool __iio_device_claim_direct(struct iio_dev *indio_dev);
>> -void __iio_device_release_direct(struct iio_dev *indio_dev);
>>  
>> -/*
>> - * Helper functions that allow claim and release of direct mode
>> - * in a fashion that doesn't generate many false positives from sparse.
>> - * Note this must remain static inline in the header so that sparse
>> - * can see the __acquire() marking. Revisit when sparse supports
>> - * __cond_acquires()
>
> I think we should leave a comment that says these functions need to stay
> as static inline so that sparse works. Although I'm wondering how sparse

I'll keep that part!

> could still work since __acquire() and __release() are removed here. IIRC
> it was a bit finicky about that.

Now __iio_dev_mode_{lock,unlock}() are declared with __acquires() and
__releases() tags. These are then used conditionally in
iio_device_claim_direct().

I did test every driver with `make C=2` and it seems to work in the same
way.

...

>> +/**
>> + * iio_device_release_direct - Releases claim on direct mode
>> + * @indio_dev:	the iio_dev associated with the device
>> + *
>> + * Release the claim. Device is no longer guaranteed to stay
>> + * in direct mode.
>> + *
>> + * Use with iio_device_claim_direct()
>> + */
>>  static inline void iio_device_release_direct(struct iio_dev *indio_dev)
>
> It could make more sense to make this:
>
> #define iio_device_release_direct __iio_dev_mode_unlock
>
> To make it clear that the auto cleanup functions that come later
> don't call iio_device_release_direct() but rather call
> __iio_dev_mode_unlock() directly.
>
> Otherwise, someone could be tempted to modify the iio_device_release_direct()
> function and the changes would not have an effect when auto cleanup is used.
>
> (Same applies to the release buffer mode function.)
>

Agree. I'll implement it like this.

-- 
 ~ Kurt