Re: [PATCH 3/3] iio: pressure: bmp280: Adds more tunable config parameters for BMP380

[Jonathan Cameron <jic23@kernel.org>]

On Mon, 27 Jun 2022 18:10:48 +0200
Angel Iglesias <ang.iglesiasg@gmail.com> wrote:

> On dom, 2022-06-26 at 15:39 +0100, Jonathan Cameron wrote:
> > On Sat, 25 Jun 2022 17:10:36 +0200
> > Angel Iglesias <ang.iglesiasg@gmail.com> wrote:
> >   
> > > Allows to configure the IIR filter coefficient and the sampling
> > > frequency
> > > The IIR filter coefficient is exposed using the sysfs attribute
> > > "filter_low_pass_3db_frequency"
> > > 
> > > Signed-off-by: Angel Iglesias <ang.iglesiasg@gmail.com>  
> > Hi Angel
> > 
> > A few comments inline,  
> > > ---

> 
> > > +               *val2 = data->chip_info->sampling_freq_avail[data-  
> > > >sampling_freq][1];  
> > > +               ret = IIO_VAL_INT_PLUS_MICRO;
> > > +               break;
> > > +       case IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY:
> > > +               *val = data->chip_info-  
> > > >iir_filter_coeffs_avail[data->iir_filter_coeff];  
> > > +               ret = IIO_VAL_INT;
> > > +               break;
> > >         default:
> > >                 ret = -EINVAL;
> > >                 break;
> > > @@ -577,6 +647,39 @@ static int
> > > bmp280_write_oversampling_ratio_press(struct bmp280_data *data,
> > >         return -EINVAL;
> > >  }
> > >  
> > > +static int bmp280_write_sampling_frequency(struct bmp280_data
> > > *data,
> > > +                                          int val, int val2)
> > > +{
> > > +       int i;
> > > +       const int (*avail)[2] = data->chip_info-  
> > > >sampling_freq_avail;  
> > > +       const int n = data->chip_info->num_sampling_freq_avail;
> > > +
> > > +       for (i = 0; i < n; i++) {
> > > +               if (avail[i][0] == val && avail[i][1] == val2) {
> > > +                       data->sampling_freq = i;  
> > better to only set the cached value if the write succeeds.
> > 
> > e.g.
> >                         ret = data->chip_info->chip_config(data);
> >                         if (ret)
> >                                 return ret;
> > 
> >                         data->sampling_freq = i;
> > 
> >                         return 0;  
> 
> Ok! Should I restore the previous value in case of failure?

The condition is pretty much unknowable, so normally we just assume
failure means it didn't write.