The scan mask for the BME280 supports humidity measurement and
needs to be distinguished from the rest in order for the timestamp
to be able to work.
Signed-off-by: Vasileios Amoiridis <vassilisamir@gmail.com>
---
drivers/iio/pressure/bmp280-core.c | 111 +++++++++++++++++++++++++----
drivers/iio/pressure/bmp280.h | 1 +
2 files changed, 98 insertions(+), 14 deletions(-)
diff --git a/drivers/iio/pressure/bmp280-core.c b/drivers/iio/pressure/bmp280-core.c
index 312bc2617583..ddfcd23f29a0 100644
--- a/drivers/iio/pressure/bmp280-core.c
+++ b/drivers/iio/pressure/bmp280-core.c
@@ -134,7 +134,28 @@ enum {
BMP380_P11 = 20,
};
+enum bmp280_scan {
+ BMP280_TEMP,
+ BMP280_PRESS,
+ BME280_HUMID
+};
+
static const struct iio_chan_spec bmp280_channels[] = {
+ {
+ .type = IIO_TEMP,
+ /* PROCESSED maintained for ABI backwards compatibility */
+ .info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED) |
+ BIT(IIO_CHAN_INFO_RAW) |
+ BIT(IIO_CHAN_INFO_SCALE) |
+ BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO),
+ .scan_index = 0,
+ .scan_type = {
+ .sign = 's',
+ .realbits = 32,
+ .storagebits = 32,
+ .endianness = IIO_CPU,
+ },
+ },
{
.type = IIO_PRESSURE,
/* PROCESSED maintained for ABI backwards compatibility */
@@ -142,7 +163,18 @@ static const struct iio_chan_spec bmp280_channels[] = {
BIT(IIO_CHAN_INFO_RAW) |
BIT(IIO_CHAN_INFO_SCALE) |
BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO),
+ .scan_index = 1,
+ .scan_type = {
+ .sign = 'u',
+ .realbits = 32,
+ .storagebits = 32,
+ .endianness = IIO_CPU,
+ },
},
+ IIO_CHAN_SOFT_TIMESTAMP(2),
+};
+
+static const struct iio_chan_spec bme280_channels[] = {
{
.type = IIO_TEMP,
/* PROCESSED maintained for ABI backwards compatibility */
@@ -150,28 +182,48 @@ static const struct iio_chan_spec bmp280_channels[] = {
BIT(IIO_CHAN_INFO_RAW) |
BIT(IIO_CHAN_INFO_SCALE) |
BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO),
+ .scan_index = 0,
+ .scan_type = {
+ .sign = 's',
+ .realbits = 32,
+ .storagebits = 32,
+ .endianness = IIO_CPU,
+ },
},
{
- .type = IIO_HUMIDITYRELATIVE,
+ .type = IIO_PRESSURE,
/* PROCESSED maintained for ABI backwards compatibility */
.info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED) |
BIT(IIO_CHAN_INFO_RAW) |
BIT(IIO_CHAN_INFO_SCALE) |
BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO),
+ .scan_index = 1,
+ .scan_type = {
+ .sign = 'u',
+ .realbits = 32,
+ .storagebits = 32,
+ .endianness = IIO_CPU,
+ },
},
-};
-
-static const struct iio_chan_spec bmp380_channels[] = {
{
- .type = IIO_PRESSURE,
+ .type = IIO_HUMIDITYRELATIVE,
/* PROCESSED maintained for ABI backwards compatibility */
.info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED) |
BIT(IIO_CHAN_INFO_RAW) |
BIT(IIO_CHAN_INFO_SCALE) |
BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO),
- .info_mask_shared_by_all = BIT(IIO_CHAN_INFO_SAMP_FREQ) |
- BIT(IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY),
+ .scan_index = 2,
+ .scan_type = {
+ .sign = 'u',
+ .realbits = 32,
+ .storagebits = 32,
+ .endianness = IIO_CPU,
+ },
},
+ IIO_CHAN_SOFT_TIMESTAMP(3),
+};
+
+static const struct iio_chan_spec bmp380_channels[] = {
{
.type = IIO_TEMP,
/* PROCESSED maintained for ABI backwards compatibility */
@@ -181,9 +233,16 @@ static const struct iio_chan_spec bmp380_channels[] = {
BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO),
.info_mask_shared_by_all = BIT(IIO_CHAN_INFO_SAMP_FREQ) |
BIT(IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY),
+ .scan_index = 0,
+ .scan_type = {
+ .sign = 's',
+ .realbits = 32,
+ .storagebits = 32,
+ .endianness = IIO_CPU,
+ },
},
{
- .type = IIO_HUMIDITYRELATIVE,
+ .type = IIO_PRESSURE,
/* PROCESSED maintained for ABI backwards compatibility */
.info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED) |
BIT(IIO_CHAN_INFO_RAW) |
@@ -191,7 +250,15 @@ static const struct iio_chan_spec bmp380_channels[] = {
BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO),
.info_mask_shared_by_all = BIT(IIO_CHAN_INFO_SAMP_FREQ) |
BIT(IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY),
+ .scan_index = 1,
+ .scan_type = {
+ .sign = 'u',
+ .realbits = 32,
+ .storagebits = 32,
+ .endianness = IIO_CPU,
+ },
},
+ IIO_CHAN_SOFT_TIMESTAMP(2),
};
static int bmp280_read_calib(struct bmp280_data *data)
@@ -825,6 +892,16 @@ static const struct iio_info bmp280_info = {
.write_raw = &bmp280_write_raw,
};
+static const unsigned long bmp280_avail_scan_masks[] = {
+ BIT(BMP280_PRESS) | BIT(BMP280_TEMP),
+ 0
+};
+
+static const unsigned long bme280_avail_scan_masks[] = {
+ BIT(BME280_HUMID) | BIT(BMP280_PRESS) | BIT(BMP280_TEMP),
+ 0
+};
+
static int bmp280_chip_config(struct bmp280_data *data)
{
u8 osrs = FIELD_PREP(BMP280_OSRS_TEMP_MASK, data->oversampling_temp + 1) |
@@ -866,7 +943,8 @@ const struct bmp280_chip_info bmp280_chip_info = {
.regmap_config = &bmp280_regmap_config,
.start_up_time = 2000,
.channels = bmp280_channels,
- .num_channels = 2,
+ .num_channels = 3,
+ .avail_scan_masks = bmp280_avail_scan_masks,
.oversampling_temp_avail = bmp280_oversampling_avail,
.num_oversampling_temp_avail = ARRAY_SIZE(bmp280_oversampling_avail),
@@ -925,8 +1003,9 @@ const struct bmp280_chip_info bme280_chip_info = {
.num_chip_id = ARRAY_SIZE(bme280_chip_ids),
.regmap_config = &bmp280_regmap_config,
.start_up_time = 2000,
- .channels = bmp280_channels,
- .num_channels = 3,
+ .channels = bme280_channels,
+ .num_channels = 4,
+ .avail_scan_masks = bme280_avail_scan_masks,
.oversampling_temp_avail = bmp280_oversampling_avail,
.num_oversampling_temp_avail = ARRAY_SIZE(bmp280_oversampling_avail),
@@ -1300,7 +1379,8 @@ const struct bmp280_chip_info bmp380_chip_info = {
.regmap_config = &bmp380_regmap_config,
.start_up_time = 2000,
.channels = bmp380_channels,
- .num_channels = 2,
+ .num_channels = 3,
+ .avail_scan_masks = bmp280_avail_scan_masks,
.oversampling_temp_avail = bmp380_oversampling_avail,
.num_oversampling_temp_avail = ARRAY_SIZE(bmp380_oversampling_avail),
@@ -1795,7 +1875,8 @@ const struct bmp280_chip_info bmp580_chip_info = {
.regmap_config = &bmp580_regmap_config,
.start_up_time = 2000,
.channels = bmp380_channels,
- .num_channels = 2,
+ .num_channels = 3,
+ .avail_scan_masks = bmp280_avail_scan_masks,
.oversampling_temp_avail = bmp580_oversampling_avail,
.num_oversampling_temp_avail = ARRAY_SIZE(bmp580_oversampling_avail),
@@ -2054,7 +2135,8 @@ const struct bmp280_chip_info bmp180_chip_info = {
.regmap_config = &bmp180_regmap_config,
.start_up_time = 2000,
.channels = bmp280_channels,
- .num_channels = 2,
+ .num_channels = 3,
+ .avail_scan_masks = bmp280_avail_scan_masks,
.oversampling_temp_avail = bmp180_oversampling_temp_avail,
.num_oversampling_temp_avail =
@@ -2166,6 +2248,7 @@ int bmp280_common_probe(struct device *dev,
/* Apply initial values from chip info structure */
indio_dev->channels = chip_info->channels;
indio_dev->num_channels = chip_info->num_channels;
+ indio_dev->available_scan_masks = chip_info->avail_scan_masks;
data->oversampling_press = chip_info->oversampling_press_default;
data->oversampling_humid = chip_info->oversampling_humid_default;
data->oversampling_temp = chip_info->oversampling_temp_default;
diff --git a/drivers/iio/pressure/bmp280.h b/drivers/iio/pressure/bmp280.h
index 6d1dca31dd52..8cc3eed70c18 100644
--- a/drivers/iio/pressure/bmp280.h
+++ b/drivers/iio/pressure/bmp280.h
@@ -427,6 +427,7 @@ struct bmp280_chip_info {
const struct iio_chan_spec *channels;
int num_channels;
unsigned int start_up_time;
+ const unsigned long *avail_scan_masks;
const int *oversampling_temp_avail;
int num_oversampling_temp_avail;
--
2.25.1On Tue, 19 Mar 2024 01:29:24 +0100 Vasileios Amoiridis <vassilisamir@gmail.com> wrote: > The scan mask for the BME280 supports humidity measurement and > needs to be distinguished from the rest in order for the timestamp > to be able to work. This needs a rewrite. I read that as the measurement needed to be distinguished from other measurements, not the device needs to be distinguished from other devices. It's also unusual to update the scan masks to add timestamps or scan_index / scan_type before they are actually used. Hence this first patch should just split the definitions but not add the additional elements until the next patch that adds buffered support (where these get used). So resulting code is fine, but the patch breakup can be improved so all the bits we need to look at for a given feature are in a single patch (but not the refactoring). Jonathan
On Tue, Mar 19, 2024 at 01:29:24AM +0100, Vasileios Amoiridis wrote:
> The scan mask for the BME280 supports humidity measurement and
> needs to be distinguished from the rest in order for the timestamp
> to be able to work.
...
> +enum bmp280_scan {
> + BMP280_TEMP,
> + BMP280_PRESS,
> + BME280_HUMID
The last is not a terminator, please leave trailing comma.
> +};
--
With Best Regards,
Andy Shevchenko
On Wed, Mar 20, 2024 at 01:07:07PM +0200, Andy Shevchenko wrote:
> On Tue, Mar 19, 2024 at 01:29:24AM +0100, Vasileios Amoiridis wrote:
> > The scan mask for the BME280 supports humidity measurement and
> > needs to be distinguished from the rest in order for the timestamp
> > to be able to work.
>
> ...
>
> > +enum bmp280_scan {
> > + BMP280_TEMP,
> > + BMP280_PRESS,
> > + BME280_HUMID
>
> The last is not a terminator, please leave trailing comma.
>
> > +};
>
What do you mean it is not a terminator? In general with the enum
variables I would write:
enum var { a, b, c };
Why in this case there is a comma needed after the BME280_HUMID element?
Cheers,
Vasilis
> --
> With Best Regards,
> Andy Shevchenko
>
>
On Wed, Mar 20, 2024 at 07:45:16PM +0100, Vasileios Amoiridis wrote:
> On Wed, Mar 20, 2024 at 01:07:07PM +0200, Andy Shevchenko wrote:
> > On Tue, Mar 19, 2024 at 01:29:24AM +0100, Vasileios Amoiridis wrote:
...
> > > +enum bmp280_scan {
> > > + BMP280_TEMP,
> > > + BMP280_PRESS,
> > > + BME280_HUMID
> >
> > The last is not a terminator, please leave trailing comma.
> >
> > > +};
>
> What do you mean it is not a terminator? In general with the enum
> variables I would write:
>
> enum var { a, b, c };
This example is different to what you used. I.o.w. _this_ example is okay.
> Why in this case there is a comma needed after the BME280_HUMID element?
It's pure style issue that helps to avoid the unneeded churn in the future in
case the list is getting expanded. You can easily imagine what I mean.
--
With Best Regards,
Andy Shevchenko
On Wed, Mar 20, 2024 at 10:38:03PM +0200, Andy Shevchenko wrote:
> On Wed, Mar 20, 2024 at 07:45:16PM +0100, Vasileios Amoiridis wrote:
> > On Wed, Mar 20, 2024 at 01:07:07PM +0200, Andy Shevchenko wrote:
> > > On Tue, Mar 19, 2024 at 01:29:24AM +0100, Vasileios Amoiridis wrote:
>
> ...
>
> > > > +enum bmp280_scan {
> > > > + BMP280_TEMP,
> > > > + BMP280_PRESS,
> > > > + BME280_HUMID
> > >
> > > The last is not a terminator, please leave trailing comma.
> > >
> > > > +};
> >
> > What do you mean it is not a terminator? In general with the enum
> > variables I would write:
> >
> > enum var { a, b, c };
>
> This example is different to what you used. I.o.w. _this_ example is okay.
>
> > Why in this case there is a comma needed after the BME280_HUMID element?
>
> It's pure style issue that helps to avoid the unneeded churn in the future in
> case the list is getting expanded. You can easily imagine what I mean.
>
Ok, that definitely makes sense, thank you! In general, should this be applied
to structs as well?
> --
> With Best Regards,
> Andy Shevchenko
>
>
On Wed, Mar 20, 2024 at 10:31:39PM +0100, Vasileios Amoiridis wrote:
> On Wed, Mar 20, 2024 at 10:38:03PM +0200, Andy Shevchenko wrote:
> > On Wed, Mar 20, 2024 at 07:45:16PM +0100, Vasileios Amoiridis wrote:
> > > On Wed, Mar 20, 2024 at 01:07:07PM +0200, Andy Shevchenko wrote:
> > > > On Tue, Mar 19, 2024 at 01:29:24AM +0100, Vasileios Amoiridis wrote:
...
> > > > > +enum bmp280_scan {
> > > > > + BMP280_TEMP,
> > > > > + BMP280_PRESS,
> > > > > + BME280_HUMID
> > > >
> > > > The last is not a terminator, please leave trailing comma.
> > > >
> > > > > +};
> > >
> > > What do you mean it is not a terminator? In general with the enum
> > > variables I would write:
> > >
> > > enum var { a, b, c };
> >
> > This example is different to what you used. I.o.w. _this_ example is okay.
> >
> > > Why in this case there is a comma needed after the BME280_HUMID element?
> >
> > It's pure style issue that helps to avoid the unneeded churn in the future in
> > case the list is getting expanded. You can easily imagine what I mean.
> >
>
> Ok, that definitely makes sense, thank you! In general, should this be applied
> to structs as well?
Yes, to structs and/or arrays initializers when the list has a potential
expanding. We don't have trailing comma when:
1) it's a terminator entry (nothing must be after);
2) it's on the one line (as in your above example).
--
With Best Regards,
Andy Shevchenko
© 2016 - 2026 Red Hat, Inc.