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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20241123160559.56c57fc7@jic23-huawei>
Date: Sat, 23 Nov 2024 16:05:59 +0000
From: Jonathan Cameron <jic23@...nel.org>
To: Nuno Sá <noname.nuno@...il.com>
Cc: Antoniu Miclaus <antoniu.miclaus@...log.com>, robh@...nel.org,
 conor+dt@...nel.org, dlechner@...libre.com, linux-iio@...r.kernel.org,
 devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
 linux-pwm@...r.kernel.org
Subject: Re: [PATCH v6 3/8] iio: backend: add API for oversampling

On Sat, 23 Nov 2024 16:02:49 +0000
Jonathan Cameron <jic23@...nel.org> wrote:

> On Mon, 11 Nov 2024 16:41:02 +0100
> Nuno Sá <noname.nuno@...il.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@...log.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á
> > 
> >   
> 
>