[v2] iio: adc: dfsdm: add scaling support

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

Posted by Olivier Moysan 1 year, 5 months ago

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);
+
 static struct iio_backend *iio_backend_from_indio_dev_parent(const struct device *dev)
 {
 	struct iio_backend *back = ERR_PTR(-ENODEV), *iter;
diff --git a/include/linux/iio/backend.h b/include/linux/iio/backend.h
index 8099759d7242..24185718b20d 100644
--- a/include/linux/iio/backend.h
+++ b/include/linux/iio/backend.h
@@ -81,6 +81,7 @@ enum iio_backend_sample_trigger {
  * @extend_chan_spec: Extend an IIO channel.
  * @ext_info_set: Extended info setter.
  * @ext_info_get: Extended info getter.
+ * @read_raw: Read value from a backend device
  **/
 struct iio_backend_ops {
 	int (*enable)(struct iio_backend *back);
@@ -113,6 +114,8 @@ struct iio_backend_ops {
 			    const char *buf, size_t len);
 	int (*ext_info_get)(struct iio_backend *back, uintptr_t private,
 			    const struct iio_chan_spec *chan, char *buf);
+	int (*read_raw)(struct iio_backend *back, struct iio_chan_spec const *chan, int *val,
+			int *val2, long mask);
 };
 
 int iio_backend_chan_enable(struct iio_backend *back, unsigned int chan);
@@ -141,7 +144,8 @@ ssize_t iio_backend_ext_info_set(struct iio_dev *indio_dev, uintptr_t private,
 				 const char *buf, size_t len);
 ssize_t iio_backend_ext_info_get(struct iio_dev *indio_dev, uintptr_t private,
 				 const struct iio_chan_spec *chan, char *buf);
-
+int iio_backend_read_raw(struct iio_backend *back, struct iio_chan_spec const *chan, int *val,
+			 int *val2, long mask);
 int iio_backend_extend_chan_spec(struct iio_dev *indio_dev,
 				 struct iio_backend *back,
 				 struct iio_chan_spec *chan);
-- 
2.25.1

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

Posted by Jonathan Cameron 1 year, 5 months ago

On Tue, 25 Jun 2024 17:07:09 +0200
Olivier Moysan <olivier.moysan@foss.st.com> 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>
Other than line wrapping moans this looks good to me.

J
> ---
>  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.

For IIO code, please still wrap at 80 chars unless there is a good reason to
got longer.

> + *
> + * 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,
Likewise. Wrap this shorter.

> +			 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);
> +
>  static struct iio_backend *iio_backend_from_indio_dev_parent(const struct device *dev)
>  {
>  	struct iio_backend *back = ERR_PTR(-ENODEV), *iter;
> diff --git a/include/linux/iio/backend.h b/include/linux/iio/backend.h
> index 8099759d7242..24185718b20d 100644
> --- a/include/linux/iio/backend.h
> +++ b/include/linux/iio/backend.h
> @@ -81,6 +81,7 @@ enum iio_backend_sample_trigger {
>   * @extend_chan_spec: Extend an IIO channel.
>   * @ext_info_set: Extended info setter.
>   * @ext_info_get: Extended info getter.
> + * @read_raw: Read value from a backend device
>   **/
>  struct iio_backend_ops {
>  	int (*enable)(struct iio_backend *back);
> @@ -113,6 +114,8 @@ struct iio_backend_ops {
>  			    const char *buf, size_t len);
>  	int (*ext_info_get)(struct iio_backend *back, uintptr_t private,
>  			    const struct iio_chan_spec *chan, char *buf);
> +	int (*read_raw)(struct iio_backend *back, struct iio_chan_spec const *chan, int *val,
And here.
> +			int *val2, long mask);
>  };
>  
>  int iio_backend_chan_enable(struct iio_backend *back, unsigned int chan);
> @@ -141,7 +144,8 @@ ssize_t iio_backend_ext_info_set(struct iio_dev *indio_dev, uintptr_t private,
>  				 const char *buf, size_t len);
>  ssize_t iio_backend_ext_info_get(struct iio_dev *indio_dev, uintptr_t private,
>  				 const struct iio_chan_spec *chan, char *buf);
> -
> +int iio_backend_read_raw(struct iio_backend *back, struct iio_chan_spec const *chan, int *val,
and here.

> +			 int *val2, long mask);
>  int iio_backend_extend_chan_spec(struct iio_dev *indio_dev,
>  				 struct iio_backend *back,
>  				 struct iio_chan_spec *chan);

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

Posted by Nuno Sá 1 year, 5 months ago

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 :)

- Nuno Sá

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

Posted by Olivier MOYSAN 1 year, 5 months ago

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á
> 
>