Re: [PATCH v3 1/4] iio: backend: update iio_backend_oversampling_ratio_set

[Jonathan Cameron <jic23@kernel.org>]

On Fri, 16 May 2025 10:06:18 -0500
David Lechner <dlechner@baylibre.com> wrote:

> On 5/16/25 5:58 AM, Pop Ioan Daniel wrote:
> > In the function iio_backend_oversampling_ratio_set the chan parameter
> > was added. The function can be used in contexts where the channel
is added

(tense is wrong given this patch is doing it).  However it should be
in imperative.

"Add chan parameter to iio_backed_oversampling_ration_set() to allow
for contexts where the channel must be specified. Modify all
existing users."


> > must be specified. All affected files have been modified.
> > 
> > Signed-off-by: Pop Ioan Daniel <pop.ioan-daniel@analog.com>
> > ---
> > changes in v3:
> >  - fix ad4851_set_oversampling_ratio function channel error
> >  drivers/iio/adc/ad4851.c           | 6 +++---
> >  drivers/iio/adc/adi-axi-adc.c      | 3 ++-
> >  drivers/iio/industrialio-backend.c | 3 ++-
> >  include/linux/iio/backend.h        | 3 ++-
> >  4 files changed, 9 insertions(+), 6 deletions(-)
> > 
> > diff --git a/drivers/iio/adc/ad4851.c b/drivers/iio/adc/ad4851.c
> > index 98ebc853db79..fccfca256670 100644
> > --- a/drivers/iio/adc/ad4851.c
> > +++ b/drivers/iio/adc/ad4851.c
> > @@ -294,7 +294,7 @@ static int ad4851_scale_fill(struct iio_dev *indio_dev)
> >  }
> >  
> >  static int ad4851_set_oversampling_ratio(struct iio_dev *indio_dev,
> > -					 const struct iio_chan_spec *chan,
> > +					 unsigned int chan,  
> 
> I think passing the channel here is misleading since this is setting the
> oversampling ratio for all channels, not just the one specified.
> 
> I would suggest to make a separate patch that removes the unused
> const struct iio_chan_spec *chan parameter first.
> 
> >  					 unsigned int osr)
> >  {
> >  	struct ad4851_state *st = iio_priv(indio_dev);
> > @@ -321,7 +321,7 @@ static int ad4851_set_oversampling_ratio(struct iio_dev *indio_dev,
> >  			return ret;
> >  	}
> >  
> > -	ret = iio_backend_oversampling_ratio_set(st->back, osr);
> > +	ret = iio_backend_oversampling_ratio_set(st->back, chan, osr);  
> 
> 
> Then in this patch, just pass 0 here instead of chan with a comment that
> the channel is ignored by the backend being used here.

Is it implausible that such a backend could be written for this device?  If
so then I agree.

J
> 
> >  	if (ret)
> >  		return ret;
> >    
>