Re: [PATCH v4 02/10] iio: backend: add support for data alignment

[Nuno Sá]

On Fri, 2025-05-02 at 11:58 +0300, Antoniu Miclaus wrote:
> Add backend support for staring the capture synchronization.
> When activated, it initates a proccess that aligns the sample's most
> significant bit (MSB) based solely on the captured data, without
> considering any other external signals.
> 
> Signed-off-by: Antoniu Miclaus <antoniu.miclaus@xxxxxxxxxx>
> ---

Couple of notes from me...

> changes in v4:
>  - implement iio_backend_interface_data_align with timeout parameter.
>  drivers/iio/industrialio-backend.c | 14 ++++++++++++++
>  include/linux/iio/backend.h        |  3 +++
>  2 files changed, 17 insertions(+)
> 
> diff --git a/drivers/iio/industrialio-backend.c b/drivers/iio/industrialio-
> backend.c
> index 038c9e1e2857..b7cbbc7a8fcd 100644
> --- a/drivers/iio/industrialio-backend.c
> +++ b/drivers/iio/industrialio-backend.c
> @@ -796,6 +796,20 @@ int iio_backend_filter_type_set(struct iio_backend *back,
>  }
>  EXPORT_SYMBOL_NS_GPL(iio_backend_filter_type_set, "IIO_BACKEND");
>  
> +/**
> + * iio_backend_data_align - Perform the data alignment process.
> + * @back: Backend device
> + * @timeout: Timeout value.
> + *

dData align is not a straightforward thing so a description is definitely
helpful here. Please add one
> + * RETURNS:
> + * 0 on success, negative error number on failure.
> + */
> +int iio_backend_interface_data_align(struct iio_backend *back, u32 timeout)

It would be nice to have a suffix in the time parameter. I would do timeout_us.
We should also document/define what happens if 0 is passed. Should we accept it?
Should we block indefinitely? For starters, I would likely not accept 0.
 
> +{
> +	return iio_backend_op_call(back, interface_data_align, timeout);
> +}
> +EXPORT_SYMBOL_NS_GPL(iio_backend_interface_data_align, "IIO_BACKEND");
> +
>  /**
>   * iio_backend_ddr_enable - Enable interface DDR (Double Data Rate) mode
>   * @back: Backend device
> diff --git a/include/linux/iio/backend.h b/include/linux/iio/backend.h
> index 5526800f5d4a..452cb2838dad 100644
> --- a/include/linux/iio/backend.h
> +++ b/include/linux/iio/backend.h
> @@ -109,6 +109,7 @@ enum iio_backend_filter_type {
>   * @debugfs_print_chan_status: Print channel status into a buffer.
>   * @debugfs_reg_access: Read or write register value of backend.
>   * @filter_type_set: Set filter type.
> + * @interface_data_align: Perform the data alignment process.
>   * @ddr_enable: Enable interface DDR (Double Data Rate) mode.
>   * @ddr_disable: Disable interface DDR (Double Data Rate) mode.
>   * @data_stream_enable: Enable data stream.
> @@ -161,6 +162,7 @@ struct iio_backend_ops {
>  				  unsigned int writeval, unsigned int
> *readval);
>  	int (*filter_type_set)(struct iio_backend *back,
>  			       enum iio_backend_filter_type type);
> +	int (*interface_data_align)(struct iio_backend *back, u32 timeout);
>  	int (*ddr_enable)(struct iio_backend *back);
>  	int (*ddr_disable)(struct iio_backend *back);
>  	int (*data_stream_enable)(struct iio_backend *back);
> @@ -203,6 +205,7 @@ int devm_iio_backend_request_buffer(struct device *dev,
>  				    struct iio_dev *indio_dev);
>  int iio_backend_filter_type_set(struct iio_backend *back,
>  				enum iio_backend_filter_type type);
> +int iio_backend_interface_data_align(struct iio_backend *back, u32 timeout);
>  int iio_backend_ddr_enable(struct iio_backend *back);
>  int iio_backend_ddr_disable(struct iio_backend *back);
>  int iio_backend_data_stream_enable(struct iio_backend *back);