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

[Jonathan Cameron <jic23@kernel.org>]

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