diff mbox series

[v6,3/8] iio: backend: add API for oversampling

Message ID 20241111121203.3699-4-antoniu.miclaus@analog.com
State Handled Elsewhere
Headers show
Series *** Add support for AD485x DAS Family *** | expand

Commit Message

Antoniu Miclaus Nov. 11, 2024, 12:11 p.m. UTC
Add backend support for enabling/disabling oversampling.

Signed-off-by: Antoniu Miclaus <antoniu.miclaus@analog.com>
---
changes in v6:
 - add iio backend commit for oversampling enable/disable
 drivers/iio/industrialio-backend.c | 14 ++++++++++++++
 include/linux/iio/backend.h        |  3 +++
 2 files changed, 17 insertions(+)

Comments

Nuno Sá Nov. 11, 2024, 3:41 p.m. UTC | #1
On Mon, 2024-11-11 at 14:11 +0200, Antoniu Miclaus wrote:
> Add backend support for enabling/disabling oversampling.
> 
> Signed-off-by: Antoniu Miclaus <antoniu.miclaus@analog.com>
> ---
> changes in v6:
>  - add iio backend commit for oversampling enable/disable
>  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 ea184fc2c838..6ba445ba3dd0 100644
> --- a/drivers/iio/industrialio-backend.c
> +++ b/drivers/iio/industrialio-backend.c
> @@ -681,6 +681,20 @@ int iio_backend_data_size_set(struct iio_backend *back,
> unsigned int size)
>  }
>  EXPORT_SYMBOL_NS_GPL(iio_backend_data_size_set, IIO_BACKEND);
>  
> +/**
> + * iio_backend_oversampling_en - set the data width/size in the data bus.

Seems unrelated?

> + * @back: Backend device
> + * @en: oversampling enabled/disabled.
> + *
> + * Return:
> + * 0 on success, negative error number on failure.
> + */
> +int iio_backend_oversampling_en(struct iio_backend *back, bool en)
> +{
> +	return iio_backend_op_call(back, oversampling_en, en);
> +}
> +EXPORT_SYMBOL_NS_GPL(iio_backend_oversampling_en, IIO_BACKEND);
> +

There was some discussion around having APIs with a boolean parameter (actually
even improving - in terms of callbacks - further with some generic
getter/setter's) or having two callbacks:

iio_backend_oversampling_enable()
iio_backend_oversampling_disable()

I'm guessing you don't really want to do any major conversion/refactoring at
this point in your series so I have a slight preference for just keeping the
current style of dedicated enable and disable APIs (irrespective of being the
better approach or not). Please consider it, if you have to re-spin the series.

- Nuno Sá
Jonathan Cameron Nov. 23, 2024, 4:02 p.m. UTC | #2
On Mon, 11 Nov 2024 16:41:02 +0100
Nuno Sá <noname.nuno@gmail.com> wrote:

> On Mon, 2024-11-11 at 14:11 +0200, Antoniu Miclaus wrote:
> > Add backend support for enabling/disabling oversampling.
> > 
> > Signed-off-by: Antoniu Miclaus <antoniu.miclaus@analog.com>
> > ---
> > changes in v6:
> >  - add iio backend commit for oversampling enable/disable
> >  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 ea184fc2c838..6ba445ba3dd0 100644
> > --- a/drivers/iio/industrialio-backend.c
> > +++ b/drivers/iio/industrialio-backend.c
> > @@ -681,6 +681,20 @@ int iio_backend_data_size_set(struct iio_backend *back,
> > unsigned int size)
> >  }
> >  EXPORT_SYMBOL_NS_GPL(iio_backend_data_size_set, IIO_BACKEND);
> >  
> > +/**
> > + * iio_backend_oversampling_en - set the data width/size in the data bus.  
> 
> Seems unrelated?
> 
> > + * @back: Backend device
> > + * @en: oversampling enabled/disabled.
> > + *
> > + * Return:
> > + * 0 on success, negative error number on failure.
> > + */
> > +int iio_backend_oversampling_en(struct iio_backend *back, bool en)
Odd to just be on or off vs a count of how much to oversample by
with 1 meaning don't oversample, 2,4,8 etc saying how much to oversample by.

> > +{
> > +	return iio_backend_op_call(back, oversampling_en, en);
> > +}
> > +EXPORT_SYMBOL_NS_GPL(iio_backend_oversampling_en, IIO_BACKEND);
> > +  
> 
> There was some discussion around having APIs with a boolean parameter (actually
> even improving - in terms of callbacks - further with some generic
> getter/setter's) or having two callbacks:
> 
> iio_backend_oversampling_enable()
> iio_backend_oversampling_disable()
> 
> I'm guessing you don't really want to do any major conversion/refactoring at
> this point in your series so I have a slight preference for just keeping the
> current style of dedicated enable and disable APIs (irrespective of being the
> better approach or not). Please consider it, if you have to re-spin the series.
Agreed. Keep it consistent for now.  I don't mind a proposal to refactor
the lot (though not yet convinced either way on it being a good idea)
but I don't want to see it inconsistent.

> 
> - Nuno Sá
> 
>
Jonathan Cameron Nov. 23, 2024, 4:05 p.m. UTC | #3
On Sat, 23 Nov 2024 16:02:49 +0000
Jonathan Cameron <jic23@kernel.org> wrote:

> On Mon, 11 Nov 2024 16:41:02 +0100
> Nuno Sá <noname.nuno@gmail.com> wrote:
> 
> > On Mon, 2024-11-11 at 14:11 +0200, Antoniu Miclaus wrote:  
> > > Add backend support for enabling/disabling oversampling.
> > > 
> > > Signed-off-by: Antoniu Miclaus <antoniu.miclaus@analog.com>
> > > ---
> > > changes in v6:
> > >  - add iio backend commit for oversampling enable/disable
> > >  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 ea184fc2c838..6ba445ba3dd0 100644
> > > --- a/drivers/iio/industrialio-backend.c
> > > +++ b/drivers/iio/industrialio-backend.c
> > > @@ -681,6 +681,20 @@ int iio_backend_data_size_set(struct iio_backend *back,
> > > unsigned int size)
> > >  }
> > >  EXPORT_SYMBOL_NS_GPL(iio_backend_data_size_set, IIO_BACKEND);
> > >  
> > > +/**
> > > + * iio_backend_oversampling_en - set the data width/size in the data bus.    
> > 
> > Seems unrelated?
> >   
> > > + * @back: Backend device
> > > + * @en: oversampling enabled/disabled.
> > > + *
> > > + * Return:
> > > + * 0 on success, negative error number on failure.
> > > + */
> > > +int iio_backend_oversampling_en(struct iio_backend *back, bool en)  
> Odd to just be on or off vs a count of how much to oversample by
> with 1 meaning don't oversample, 2,4,8 etc saying how much to oversample by.

Looking at the code, why does the backend care?  Seems you only set the ratio
on the ADC, not the in the FPGA IP. So I don't follow how that is useful

Jonathan


> 
> > > +{
> > > +	return iio_backend_op_call(back, oversampling_en, en);
> > > +}
> > > +EXPORT_SYMBOL_NS_GPL(iio_backend_oversampling_en, IIO_BACKEND);
> > > +    
> > 
> > There was some discussion around having APIs with a boolean parameter (actually
> > even improving - in terms of callbacks - further with some generic
> > getter/setter's) or having two callbacks:
> > 
> > iio_backend_oversampling_enable()
> > iio_backend_oversampling_disable()
> > 
> > I'm guessing you don't really want to do any major conversion/refactoring at
> > this point in your series so I have a slight preference for just keeping the
> > current style of dedicated enable and disable APIs (irrespective of being the
> > better approach or not). Please consider it, if you have to re-spin the series.  
> Agreed. Keep it consistent for now.  I don't mind a proposal to refactor
> the lot (though not yet convinced either way on it being a good idea)
> but I don't want to see it inconsistent.
> 
> > 
> > - Nuno Sá
> > 
> >   
> 
>
diff mbox series

Patch

diff --git a/drivers/iio/industrialio-backend.c b/drivers/iio/industrialio-backend.c
index ea184fc2c838..6ba445ba3dd0 100644
--- a/drivers/iio/industrialio-backend.c
+++ b/drivers/iio/industrialio-backend.c
@@ -681,6 +681,20 @@  int iio_backend_data_size_set(struct iio_backend *back, unsigned int size)
 }
 EXPORT_SYMBOL_NS_GPL(iio_backend_data_size_set, IIO_BACKEND);
 
+/**
+ * iio_backend_oversampling_en - set the data width/size in the data bus.
+ * @back: Backend device
+ * @en: oversampling enabled/disabled.
+ *
+ * Return:
+ * 0 on success, negative error number on failure.
+ */
+int iio_backend_oversampling_en(struct iio_backend *back, bool en)
+{
+	return iio_backend_op_call(back, oversampling_en, en);
+}
+EXPORT_SYMBOL_NS_GPL(iio_backend_oversampling_en, IIO_BACKEND);
+
 /**
  * iio_backend_extend_chan_spec - Extend an IIO channel
  * @back: Backend device
diff --git a/include/linux/iio/backend.h b/include/linux/iio/backend.h
index 59b6651b7eaf..b8f63823b3db 100644
--- a/include/linux/iio/backend.h
+++ b/include/linux/iio/backend.h
@@ -94,6 +94,7 @@  enum iio_backend_interface_type {
  * @ext_info_get: Extended info getter.
  * @interface_type_get: Interface type.
  * @data_size_set: Data size.
+ * @oversampling_en: Oversampling.
  * @read_raw: Read a channel attribute from a backend device
  * @debugfs_print_chan_status: Print channel status into a buffer.
  * @debugfs_reg_access: Read or write register value of backend.
@@ -132,6 +133,7 @@  struct iio_backend_ops {
 	int (*interface_type_get)(struct iio_backend *back,
 				  enum iio_backend_interface_type *type);
 	int (*data_size_set)(struct iio_backend *back, unsigned int size);
+	int (*oversampling_en)(struct iio_backend *back, bool en);
 	int (*read_raw)(struct iio_backend *back,
 			struct iio_chan_spec const *chan, int *val, int *val2,
 			long mask);
@@ -183,6 +185,7 @@  ssize_t iio_backend_ext_info_get(struct iio_dev *indio_dev, uintptr_t private,
 int iio_backend_interface_type_get(struct iio_backend *back,
 				   enum iio_backend_interface_type *type);
 int iio_backend_data_size_set(struct iio_backend *back, unsigned int size);
+int iio_backend_oversampling_en(struct iio_backend *back, bool en);
 int iio_backend_read_raw(struct iio_backend *back,
 			 struct iio_chan_spec const *chan, int *val, int *val2,
 			 long mask);