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

From: Jonathan Cameron
Date: Sat Jul 16 2022 - 11:55:52 EST


On Mon, 27 Jun 2022 18:10:48 +0200
Angel Iglesias <ang.iglesiasg@xxxxxxxxx> 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@xxxxxxxxx> 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@xxxxxxxxx>
> > 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.