Re: [PATCH 2/3] iio: pressure: bmp280: Add support for BMP380 sensor family

[Jonathan Cameron]

On Mon, 27 Jun 2022 17:42:49 +0200
Angel Iglesias <ang.iglesiasg@xxxxxxxxx> wrote:

> On sáb, 2022-06-25 at 16:46 +0100, Jonathan Cameron wrote:
> > On Sat, 25 Jun 2022 17:09:12 +0200
> > Angel Iglesias <ang.iglesiasg@xxxxxxxxx> wrote:
> >   
> > > Adds compatibility with the new generation of this sensor, the
> > > BMP380
> > > 
> > > Included basic sensor initialization to do pressure and temp
> > > measurements and allows tuning oversampling settings for each
> > > channel
> > > The compensation algorithms are adapted from the device datasheet
> > > and
> > > the repository https://github.com/BoschSensortec/BMP3-Sensor-API
> > > 
> > > Signed-off-by: Angel Iglesias <ang.iglesiasg@xxxxxxxxx>  
> > 
> > Hi Angel,
> > 
> > A few comments inline, but mostly looks good to me.

First a process comment.  Cut out anything you agree with.
Too many emails to read so focus on the bits where there are questions
or they will get missed.  Reviewers are happy to assume you agree
with them if you don't say otherwise :)


> > 
> > Jonathan
...

> 
> > > +
> > > +#define BMP380_FILTER_MASK             GENMASK(3, 1)
> > > +#define BMP380_FILTER_OFF              0
> > > +#define BMP380_FILTER_1X               BIT(1)
> > > +#define BMP380_FILTER_3X               BIT(2)
> > > +#define BMP380_FILTER_7X               (BIT(2) | BIT(1))
> > > +#define BMP380_FILTER_15X              BIT(3)
> > > +#define BMP380_FILTER_31X              (BIT(3) | BIT(1))
> > > +#define BMP380_FILTER_63X              (BIT(3) | BIT(2))
> > > +#define BMP380_FILTER_127X             (BIT(3) | BIT(2) | BIT(1))  
> > 
> > these are values, 0,1,2,3,4,5,6,7 not a bunch of bits.
> > So use with FIELD_PREP()  
> 
> Should I convert the values to a enumeration or simply declare the
> macros with the values? Thanks in advance

Slight preference for defines with the values.

J