Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Sun, 14 Aug 2022 17:56:56 +0100
From:   Jonathan Cameron <jic23@kernel.org>
To:     Angel Iglesias <ang.iglesiasg@gmail.com>
Cc:     linux-iio <linux-iio@vger.kernel.org>,
        Lars-Peter Clausen <lars@metafoo.de>,
        Andy Shevchenko <andriy.shevchenko@linux.intel.com>,
        Nikita Yushchenko <nikita.yoush@cogentembedded.com>,
        Ulf Hansson <ulf.hansson@linaro.org>,
        "Rafael J. Wysocki" <rafael.j.wysocki@intel.com>,
        Paul Cercueil <paul@crapouillou.net>,
        linux-kernel@vger.kernel.org
Subject: Re: [PATCH v5 3/5] iio: pressure: bmp280: Add support for BMP380
 sensor family
Message-ID: <20220814175656.1d09f897@jic23-huawei>
In-Reply-To: <53b0132936a98943fb4284c4f72e3f4e6faa402d.camel@gmail.com>
References: <cover.1659872590.git.ang.iglesiasg@gmail.com>
        <462251c4bca30b53f06307043ad52d859eb8d5c1.1659872590.git.ang.iglesiasg@gmail.com>
        <20220814151512.097c7779@jic23-huawei>
        <53b0132936a98943fb4284c4f72e3f4e6faa402d.camel@gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

On Sun, 14 Aug 2022 16:37:58 +0200
Angel Iglesias <ang.iglesiasg@gmail.com> wrote:

> On Sun, 2022-08-14 at 15:15 +0100, Jonathan Cameron wrote:
> > On Sun,=C2=A0 7 Aug 2022 13:55:52 +0200
> > Angel Iglesias <ang.iglesiasg@gmail.com> wrote:
> >  =20
> > > Adds compatibility with the new generation of this sensor, the BMP380
> > >=20
> > > Includes basic sensor initialization to do pressure and temp
> > > measurements and allows tuning oversampling settings for each channel.
> > >=20
> > > The compensation algorithms are adapted from the device datasheet and
> > > the repository https://github.com/BoschSensortec/BMP3-Sensor-API
> > >=20
> > > Signed-off-by: Angel Iglesias <ang.iglesiasg@gmail.com> =20
> >=20
> > One additional comment from me inline.
> >=20
> > Thanks,
> >=20
> > Jonathan
> >=20
> >  =20
> > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0}
> > > @@ -1124,6 +1471,13 @@ int bmp280_common_probe(struct device *dev,
> > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=
=A0=C2=A0=C2=A0=C2=A0=C2=A0return -EINVAL;
> > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0}
> > > =C2=A0
> > > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0/* BMP3xx requires soft-re=
set as part of initialization */
> > > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0if (chip_id =3D=3D BMP380_=
CHIP_ID) { =20
> >=20
> > I'd prefer this to be based on a flag in chip_info so that we can
> > trivially add it to future devices by just setting that flag for the
> > chip_info added for the new device. =20
>=20
> How a new init or preinit callback? For now only BMP380 chip would use it=
, but I
> would like to get my hands on a BMP390 and the new BMP580 and extend the =
driver
> to support for them.

Hmm. Good point, I'd failed to noticed the register is bmp380 specific curr=
ently.
Maybe one to leave for a future refactor if you get those newer parts.

Jonathan

>=20
> > > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=
=C2=A0=C2=A0=C2=A0=C2=A0ret =3D bmp380_cmd(data, BMP380_CMD_SOFT_RESET);
> > > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=
=C2=A0=C2=A0=C2=A0=C2=A0if (ret < 0)
> > > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=
=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0ret=
urn ret;
> > > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0}
> > > +
> > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0ret =3D data->chip_in=
fo->chip_config(data);
> > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0if (ret < 0)
> > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=
=A0=C2=A0=C2=A0=C2=A0=C2=A0return ret; =20
> >  =20
>=20
> Kind regards,
> Angel