Re: [PATCH 5/8] iio: accel: kx022a: Switch to sparse friendly iio_device_claim/release_direct()

[David Lechner <dlechner@baylibre.com>]

On 2/18/25 1:39 AM, Matti Vaittinen wrote:
> On 17/02/2025 16:01, Jonathan Cameron wrote:
>> From: Jonathan Cameron <Jonathan.Cameron@huawei.com>
>>
>> These new functions allow sparse to find failures to release
>> direct mode reducing chances of bugs over the claim_direct_mode()
>> functions that are deprecated.
>>
>> Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
>> Cc: Matti Vaittinen <mazziesaccount@gmail.com>
>> ---
>>   drivers/iio/accel/kionix-kx022a.c | 14 ++++++--------
>>   1 file changed, 6 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/iio/accel/kionix-kx022a.c b/drivers/iio/accel/kionix-kx022a.c
>> index 727e007c5fc1..07dcf5f0599f 100644
>> --- a/drivers/iio/accel/kionix-kx022a.c
>> +++ b/drivers/iio/accel/kionix-kx022a.c
>> @@ -577,13 +577,12 @@ static int kx022a_write_raw(struct iio_dev *idev,
>>        * issues if users trust the watermark to be reached within known
>>        * time-limit).
>>        */
>> -    ret = iio_device_claim_direct_mode(idev);
>> -    if (ret)
>> -        return ret;
>> +    if (!iio_device_claim_direct(idev))
>> +        return -EBUSY;
> 
> Not really in the scope of this review - but in my opinion the logic of this check is terribly counter intuitive. I mean,
> 
>> +    if (iio_device_claim_direct(idev))
>> +        return -EBUSY;

I'm curious how you read this then. I read this as:

"If claiming direct mode succeeded, then return an error!"

Returning an error on success seem very counterintuitive to me.
And the way Jonathan implemented it seems the logical way to do it.

"If claiming direct mode did not succeed, then return an error."

> 
> would feel much more familiar. I actually had to look up the implementation of the iio_device_claim_direct() to see this was not a bug.
> 
> Other than that this looks very good to me.
> 
> Yours,
>     -- Matti