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

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]

Date:   Sat, 16 Jul 2022 17:03:44 +0100
From:   Jonathan Cameron <jic23@...nel.org>
To:     Angel Iglesias <ang.iglesiasg@...il.com>
Cc:     Lars-Peter Clausen <lars@...afoo.de>,
        "Rafael J. Wysocki" <rafael.j.wysocki@...el.com>,
        Ulf Hansson <ulf.hansson@...aro.org>,
        Paul Cercueil <paul@...pouillou.net>,
        linux-iio@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 2/3] iio: pressure: bmp280: Add support for BMP380
 sensor family

On Mon, 27 Jun 2022 17:42:49 +0200
Angel Iglesias <ang.iglesiasg@...il.com> 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@...il.com> 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@...il.com>  
> > 
> > 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