Re: [PATCH v2 1/8] iio: add read raw service to iio backend framework

[Olivier MOYSAN <olivier.moysan@foss.st.com>]

Hi Nuno,

On 6/26/24 10:50, Nuno Sá wrote:
> On Tue, 2024-06-25 at 17:07 +0200, Olivier Moysan wrote:
>> Add iio_backend_read_raw() service to support attributes read
>> from an IIO backend.
>>
>> Signed-off-by: Olivier Moysan <olivier.moysan@foss.st.com>
>> Reviewed-by: Nuno Sa <nuno.sa@analog.com>
>> ---
>>   drivers/iio/industrialio-backend.c | 21 +++++++++++++++++++++
>>   include/linux/iio/backend.h        |  6 +++++-
>>   2 files changed, 26 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/iio/industrialio-backend.c b/drivers/iio/industrialio-
>> backend.c
>> index 929aff4040ed..0e2653de1956 100644
>> --- a/drivers/iio/industrialio-backend.c
>> +++ b/drivers/iio/industrialio-backend.c
>> @@ -357,6 +357,27 @@ int devm_iio_backend_request_buffer(struct device *dev,
>>   }
>>   EXPORT_SYMBOL_NS_GPL(devm_iio_backend_request_buffer, IIO_BACKEND);
>>   
>> +/**
>> + * iio_backend_read_raw - Request a channel attribute from the IIO backend.
>> + * @back:	Backend device
>> + * @chan:	IIO channel reference
>> + * @val:	First element of the returned value
>> + * @val2:	Second element of the returned value
>> + * @mask:	Specify value to retrieve
>> + *
>> + * This callback replicates the read_raw callback of the IIO framework, and
>> is intended to
>> + * request miscellaneous channel attributes from the backend device.
>> + *
>> + * RETURNS:
>> + * 0 on success, negative error number on failure.
>> + */
>> +int iio_backend_read_raw(struct iio_backend *back, struct iio_chan_spec const
>> *chan, int *val,
>> +			 int *val2, long mask)
>> +{
>> +	return iio_backend_op_call(back, read_raw, chan, val, val2, mask);
>> +}
>> +EXPORT_SYMBOL_NS_GPL(iio_backend_read_raw, IIO_BACKEND);
> 
> I actually got an idea when looking at this for my existential crisis between
> dedicated APIs and a catch all .read_raw() :). What we can do is just provide
> the .read_raw() or write_raw() ops to backends (so we minimize the number of
> ops) and then we build on top of them for providing more readable (depending on
> the case; some cases it does make sense to just call iio_backend_read_raw())
> APIs to frontends.
> 
> So in your case you could have in backend.h
> 
> static inline int iio_backend_read_scale(...)
> {
> 	return iio_backend_read_raw(..., IIO_CHAN_INFO_SCALE);
> }
> 
> Naturally no need for you to do this right now in your series. Just wanted to
> write it down before I go into other stuff and forget about this :)
> 

Yes, this is a good compromise. Such helpers are more user-friendly for 
the consumer. As long as I have to push a v3, I might take the 
opportunity to add this.

BRs
Olivier

> - Nuno Sá
> 
>