[PATCH v2 0/7] pressure: bmp280: Minor cleanup and interrupt support

[PATCH v2 0/7] pressure: bmp280: Minor cleanup and interrupt support

[Vasileios Amoiridis]

Based on iio testing branch.

Changes in v2:

[PATCH v2 1/7] <=> [PATCH v1 4/10]
	- Added comment to enum indexes for humidity parameters
	- Made more clear the bit handling in case of calib->H4

[PATCH v2 4/7] <=> [PATCH v1 7/10]
	- Used const arrays for local BMP280_MODE_* variables
	- Added comment for why we check humidity oversampling
	- Added comment on stubs

[PATCH v2 5/7] <=> [PATCH v1 8/10]
	- Used only INT as interrupt since the device has only one irq line
	- Used drive-open-drain

[PATCH v2 6/7] <=> [PATCH v1 9/10]
	- Generalized IIO trigger code to be able to adopt more easily FIFO
	  irqs by using a bmpxxx_irq_thread_handler() function which handles the
          irq handling.

---
v1: https://lore.kernel.org/linux-iio/20240711211558.106327-1-vassilisamir@gmail.com/

Depends on this series [1].

This series aims to add hardware trigger support and extend the functionality
of the driver. Sensors BMP3xx and BMP5xx have an interrupt pin which can be
used in order to inform about a specific event in the sensor. For now, the
data ready event is used, and is added as a DRDY interrupt in the driver.

The interrupt is supported only in rising modes for now, and it doesn't support
latched mode.

Other interrupts such as, FIFO-FULL, FIFO-WATERMARK, Out of range values etc.
are not supported for the moment, and only the DRDY interrupt is supported.

While working on the trigger, FORCED MODE instead of NORMAL MODE was added to
the driver for use in the oneshot capture reads. There is no need for the
driver to continuously produce data, without using them and without properly
notifying the user when those data became available. This can produce high
incosistencies between the acquisition time and the readout of the sensor.
The data now, in the case of the .read_raw() function is using the FORCED MODE,
which samples and calculates the values at that moment.

Last commit, is just moving the interrupt interface of a very old sensor to be
consistent with the new ones, and no functional changes are intended.
ubject: [PATCH v2 0/7] *** SUBJECT HERE ***

Vasileios Amoiridis (7):
  iio: pressure: bmp280: Use bulk read for humidity calibration data
  iio: pressure: bmp280: Add support for bmp280 soft reset
  iio: pressure: bmp280: Remove config error check for IIR filter
    updates
  iio: pressure: bmp280: Use sleep and forced mode for oneshot captures
  dt-bindings: iio: pressure: bmp085: Add interrupts for BMP3xx and
    BMP5xx devices
  iio: pressure: bmp280: Add data ready trigger support
  iio: pressure bmp280: Move bmp085 interrupt to new configuration

 .../bindings/iio/pressure/bmp085.yaml         |   7 +-
 drivers/iio/pressure/bmp280-core.c            | 739 ++++++++++++++++--
 drivers/iio/pressure/bmp280-i2c.c             |   4 +-
 drivers/iio/pressure/bmp280-regmap.c          |   2 +-
 drivers/iio/pressure/bmp280-spi.c             |   4 +-
 drivers/iio/pressure/bmp280.h                 |  51 +-
 6 files changed, 715 insertions(+), 92 deletions(-)


base-commit: 47ee461357f9da5a35d5f43527b7804a6a5744cb
-- 
2.25.1

[PATCH v2 1/7] iio: pressure: bmp280: Use bulk read for humidity calibration data

[Vasileios Amoiridis]

Convert individual reads to a bulk read for the humidity calibration data.

Signed-off-by: Vasileios Amoiridis <vassilisamir@gmail.com>
---
 drivers/iio/pressure/bmp280-core.c | 62 ++++++++++--------------------
 drivers/iio/pressure/bmp280.h      |  6 +++
 2 files changed, 27 insertions(+), 41 deletions(-)

diff --git a/drivers/iio/pressure/bmp280-core.c b/drivers/iio/pressure/bmp280-core.c
index 3deaa57bb3f5..d5e5eb22667a 100644
--- a/drivers/iio/pressure/bmp280-core.c
+++ b/drivers/iio/pressure/bmp280-core.c
@@ -118,6 +118,12 @@ enum bmp580_odr {
  */
 enum { T1, T2, T3, P1, P2, P3, P4, P5, P6, P7, P8, P9 };
 
+/*
+ * These enums are used for indexing into the array of humidity parameters
+ * for BME280.
+ */
+enum { H2 = 0, H3 = 2, H4 = 3, H5 = 4, H6 = 6 };
+
 enum {
 	/* Temperature calib indexes */
 	BMP380_T1 = 0,
@@ -344,6 +350,7 @@ static int bme280_read_calib(struct bmp280_data *data)
 {
 	struct bmp280_calib *calib = &data->calib.bmp280;
 	struct device *dev = data->dev;
+	s16 h4_upper, h4_lower;
 	unsigned int tmp;
 	int ret;
 
@@ -352,14 +359,6 @@ static int bme280_read_calib(struct bmp280_data *data)
 	if (ret)
 		return ret;
 
-	/*
-	 * Read humidity calibration values.
-	 * Due to some odd register addressing we cannot just
-	 * do a big bulk read. Instead, we have to read each Hx
-	 * value separately and sometimes do some bit shifting...
-	 * Humidity data is only available on BME280.
-	 */
-
 	ret = regmap_read(data->regmap, BME280_REG_COMP_H1, &tmp);
 	if (ret) {
 		dev_err(dev, "failed to read H1 comp value\n");
@@ -368,43 +367,24 @@ static int bme280_read_calib(struct bmp280_data *data)
 	calib->H1 = tmp;
 
 	ret = regmap_bulk_read(data->regmap, BME280_REG_COMP_H2,
-			       &data->le16, sizeof(data->le16));
-	if (ret) {
-		dev_err(dev, "failed to read H2 comp value\n");
-		return ret;
-	}
-	calib->H2 = sign_extend32(le16_to_cpu(data->le16), 15);
-
-	ret = regmap_read(data->regmap, BME280_REG_COMP_H3, &tmp);
-	if (ret) {
-		dev_err(dev, "failed to read H3 comp value\n");
-		return ret;
-	}
-	calib->H3 = tmp;
-
-	ret = regmap_bulk_read(data->regmap, BME280_REG_COMP_H4,
-			       &data->be16, sizeof(data->be16));
+			       data->bme280_humid_cal_buf,
+			       sizeof(data->bme280_humid_cal_buf));
 	if (ret) {
-		dev_err(dev, "failed to read H4 comp value\n");
+		dev_err(dev, "failed to read humidity calibration values\n");
 		return ret;
 	}
-	calib->H4 = sign_extend32(((be16_to_cpu(data->be16) >> 4) & 0xff0) |
-				  (be16_to_cpu(data->be16) & 0xf), 11);
 
-	ret = regmap_bulk_read(data->regmap, BME280_REG_COMP_H5,
-			       &data->le16, sizeof(data->le16));
-	if (ret) {
-		dev_err(dev, "failed to read H5 comp value\n");
-		return ret;
-	}
-	calib->H5 = sign_extend32(FIELD_GET(BME280_COMP_H5_MASK, le16_to_cpu(data->le16)), 11);
-
-	ret = regmap_read(data->regmap, BME280_REG_COMP_H6, &tmp);
-	if (ret) {
-		dev_err(dev, "failed to read H6 comp value\n");
-		return ret;
-	}
-	calib->H6 = sign_extend32(tmp, 7);
+	calib->H2 = get_unaligned_le16(&data->bme280_humid_cal_buf[H2]);
+	calib->H3 = data->bme280_humid_cal_buf[H3];
+	h4_upper = FIELD_GET(BME280_COMP_H4_GET_MASK_UP,
+			get_unaligned_be16(&data->bme280_humid_cal_buf[H4]));
+	h4_upper = FIELD_PREP(BME280_COMP_H4_PREP_MASK_UP, h4_upper);
+	h4_lower = FIELD_GET(BME280_COMP_H4_MASK_LOW,
+			get_unaligned_be16(&data->bme280_humid_cal_buf[H4]));
+	calib->H4 = sign_extend32(h4_upper | h4_lower, 11);
+	calib->H5 = sign_extend32(FIELD_GET(BME280_COMP_H5_MASK,
+			get_unaligned_le16(&data->bme280_humid_cal_buf[H5])), 11);
+	calib->H6 = data->bme280_humid_cal_buf[H6];
 
 	return 0;
 }
diff --git a/drivers/iio/pressure/bmp280.h b/drivers/iio/pressure/bmp280.h
index ccacc67c1473..9bea0b84d2f4 100644
--- a/drivers/iio/pressure/bmp280.h
+++ b/drivers/iio/pressure/bmp280.h
@@ -257,8 +257,13 @@
 #define BME280_REG_COMP_H5		0xE5
 #define BME280_REG_COMP_H6		0xE7
 
+#define BME280_COMP_H4_GET_MASK_UP	GENMASK(15, 8)
+#define BME280_COMP_H4_PREP_MASK_UP	GENMASK(11, 4)
+#define BME280_COMP_H4_MASK_LOW		GENMASK(3, 0)
 #define BME280_COMP_H5_MASK		GENMASK(15, 4)
 
+#define BME280_CONTIGUOUS_CALIB_REGS	7
+
 #define BME280_OSRS_HUMIDITY_MASK	GENMASK(2, 0)
 #define BME280_OSRS_HUMIDITY_SKIP	0
 #define BME280_OSRS_HUMIDITY_1X		1
@@ -423,6 +428,7 @@ struct bmp280_data {
 		/* Calibration data buffers */
 		__le16 bmp280_cal_buf[BMP280_CONTIGUOUS_CALIB_REGS / 2];
 		__be16 bmp180_cal_buf[BMP180_REG_CALIB_COUNT / 2];
+		u8 bme280_humid_cal_buf[BME280_CONTIGUOUS_CALIB_REGS];
 		u8 bmp380_cal_buf[BMP380_CALIB_REG_COUNT];
 		/* Miscellaneous, endianness-aware data buffers */
 		__le16 le16;
-- 
2.25.1

Re: [PATCH v2 1/7] iio: pressure: bmp280: Use bulk read for humidity calibration data

[Jonathan Cameron]

On Fri, 26 Jul 2024 01:10:33 +0200
Vasileios Amoiridis <vassilisamir@gmail.com> wrote:

> Convert individual reads to a bulk read for the humidity calibration data.
> 
> Signed-off-by: Vasileios Amoiridis <vassilisamir@gmail.com>

One comment inline.  Short version is move that complicated field start enum
next to the code so we don't need to say so much for it to make sense.

> ---
>  drivers/iio/pressure/bmp280-core.c | 62 ++++++++++--------------------
>  drivers/iio/pressure/bmp280.h      |  6 +++
>  2 files changed, 27 insertions(+), 41 deletions(-)
> 
> diff --git a/drivers/iio/pressure/bmp280-core.c b/drivers/iio/pressure/bmp280-core.c
> index 3deaa57bb3f5..d5e5eb22667a 100644
> --- a/drivers/iio/pressure/bmp280-core.c
> +++ b/drivers/iio/pressure/bmp280-core.c
> @@ -118,6 +118,12 @@ enum bmp580_odr {
>   */
>  enum { T1, T2, T3, P1, P2, P3, P4, P5, P6, P7, P8, P9 };
>  
> +/*
> + * These enums are used for indexing into the array of humidity parameters
> + * for BME280.

I was thinking of a comment that also mentioned the overlap. Perhaps something like
...


Index of the byte containing the start of each humidity parameter. Some
parameters stretch across multiple bytes including into the start of the byte
where another humidity parameter begins. Unaligned be/le accesses are used
to allow fields to be extracted with FIELD_GET(). 

Or, just refer to the field layout being complex and to see
bme280_read_calib function.

Actually come to think of it, just move this enum down there so it
is local to the code and the usage is more obvious / comment less important.

> + */
> +enum { H2 = 0, H3 = 2, H4 = 3, H5 = 4, H6 = 6 };
> +
>  enum {
>  	/* Temperature calib indexes */
>  	BMP380_T1 = 0,
> @@ -344,6 +350,7 @@ static int bme280_read_calib(struct bmp280_data *data)
>  {
>  	struct bmp280_calib *calib = &data->calib.bmp280;
>  	struct device *dev = data->dev;
> +	s16 h4_upper, h4_lower;
>  	unsigned int tmp;
>  	int ret;
>  
> @@ -352,14 +359,6 @@ static int bme280_read_calib(struct bmp280_data *data)
>  	if (ret)
>  		return ret;
>  
> -	/*
> -	 * Read humidity calibration values.
> -	 * Due to some odd register addressing we cannot just
> -	 * do a big bulk read. Instead, we have to read each Hx
> -	 * value separately and sometimes do some bit shifting...
> -	 * Humidity data is only available on BME280.
> -	 */
> -
>  	ret = regmap_read(data->regmap, BME280_REG_COMP_H1, &tmp);
>  	if (ret) {
>  		dev_err(dev, "failed to read H1 comp value\n");
> @@ -368,43 +367,24 @@ static int bme280_read_calib(struct bmp280_data *data)
>  	calib->H1 = tmp;
>  
>  	ret = regmap_bulk_read(data->regmap, BME280_REG_COMP_H2,
> -			       &data->le16, sizeof(data->le16));
> -	if (ret) {
> -		dev_err(dev, "failed to read H2 comp value\n");
> -		return ret;
> -	}
> -	calib->H2 = sign_extend32(le16_to_cpu(data->le16), 15);
> -
> -	ret = regmap_read(data->regmap, BME280_REG_COMP_H3, &tmp);
> -	if (ret) {
> -		dev_err(dev, "failed to read H3 comp value\n");
> -		return ret;
> -	}
> -	calib->H3 = tmp;
> -
> -	ret = regmap_bulk_read(data->regmap, BME280_REG_COMP_H4,
> -			       &data->be16, sizeof(data->be16));
> +			       data->bme280_humid_cal_buf,
> +			       sizeof(data->bme280_humid_cal_buf));
>  	if (ret) {
> -		dev_err(dev, "failed to read H4 comp value\n");
> +		dev_err(dev, "failed to read humidity calibration values\n");
>  		return ret;
>  	}
> -	calib->H4 = sign_extend32(((be16_to_cpu(data->be16) >> 4) & 0xff0) |
> -				  (be16_to_cpu(data->be16) & 0xf), 11);
>  
> -	ret = regmap_bulk_read(data->regmap, BME280_REG_COMP_H5,
> -			       &data->le16, sizeof(data->le16));
> -	if (ret) {
> -		dev_err(dev, "failed to read H5 comp value\n");
> -		return ret;
> -	}
> -	calib->H5 = sign_extend32(FIELD_GET(BME280_COMP_H5_MASK, le16_to_cpu(data->le16)), 11);
> -
> -	ret = regmap_read(data->regmap, BME280_REG_COMP_H6, &tmp);
> -	if (ret) {
> -		dev_err(dev, "failed to read H6 comp value\n");
> -		return ret;
> -	}
> -	calib->H6 = sign_extend32(tmp, 7);
> +	calib->H2 = get_unaligned_le16(&data->bme280_humid_cal_buf[H2]);
> +	calib->H3 = data->bme280_humid_cal_buf[H3];
> +	h4_upper = FIELD_GET(BME280_COMP_H4_GET_MASK_UP,
> +			get_unaligned_be16(&data->bme280_humid_cal_buf[H4]));
> +	h4_upper = FIELD_PREP(BME280_COMP_H4_PREP_MASK_UP, h4_upper);
> +	h4_lower = FIELD_GET(BME280_COMP_H4_MASK_LOW,
> +			get_unaligned_be16(&data->bme280_humid_cal_buf[H4]));
> +	calib->H4 = sign_extend32(h4_upper | h4_lower, 11);
> +	calib->H5 = sign_extend32(FIELD_GET(BME280_COMP_H5_MASK,
> +			get_unaligned_le16(&data->bme280_humid_cal_buf[H5])), 11);
> +	calib->H6 = data->bme280_humid_cal_buf[H6];
>  
>  	return 0;
>  }
> diff --git a/drivers/iio/pressure/bmp280.h b/drivers/iio/pressure/bmp280.h
> index ccacc67c1473..9bea0b84d2f4 100644
> --- a/drivers/iio/pressure/bmp280.h
> +++ b/drivers/iio/pressure/bmp280.h
> @@ -257,8 +257,13 @@
>  #define BME280_REG_COMP_H5		0xE5
>  #define BME280_REG_COMP_H6		0xE7
>  
> +#define BME280_COMP_H4_GET_MASK_UP	GENMASK(15, 8)
> +#define BME280_COMP_H4_PREP_MASK_UP	GENMASK(11, 4)
> +#define BME280_COMP_H4_MASK_LOW		GENMASK(3, 0)
>  #define BME280_COMP_H5_MASK		GENMASK(15, 4)
>  
> +#define BME280_CONTIGUOUS_CALIB_REGS	7
> +
>  #define BME280_OSRS_HUMIDITY_MASK	GENMASK(2, 0)
>  #define BME280_OSRS_HUMIDITY_SKIP	0
>  #define BME280_OSRS_HUMIDITY_1X		1
> @@ -423,6 +428,7 @@ struct bmp280_data {
>  		/* Calibration data buffers */
>  		__le16 bmp280_cal_buf[BMP280_CONTIGUOUS_CALIB_REGS / 2];
>  		__be16 bmp180_cal_buf[BMP180_REG_CALIB_COUNT / 2];
> +		u8 bme280_humid_cal_buf[BME280_CONTIGUOUS_CALIB_REGS];
>  		u8 bmp380_cal_buf[BMP380_CALIB_REG_COUNT];
>  		/* Miscellaneous, endianness-aware data buffers */
>  		__le16 le16;

Re: [PATCH v2 1/7] iio: pressure: bmp280: Use bulk read for humidity calibration data

[Vasileios Amoiridis]

On Sun, Jul 28, 2024 at 04:46:21PM +0100, Jonathan Cameron wrote:
> On Fri, 26 Jul 2024 01:10:33 +0200
> Vasileios Amoiridis <vassilisamir@gmail.com> wrote:
> 
> > Convert individual reads to a bulk read for the humidity calibration data.
> > 
> > Signed-off-by: Vasileios Amoiridis <vassilisamir@gmail.com>
> 
> One comment inline.  Short version is move that complicated field start enum
> next to the code so we don't need to say so much for it to make sense.
> 
> > ---
> >  drivers/iio/pressure/bmp280-core.c | 62 ++++++++++--------------------
> >  drivers/iio/pressure/bmp280.h      |  6 +++
> >  2 files changed, 27 insertions(+), 41 deletions(-)
> > 
> > diff --git a/drivers/iio/pressure/bmp280-core.c b/drivers/iio/pressure/bmp280-core.c
> > index 3deaa57bb3f5..d5e5eb22667a 100644
> > --- a/drivers/iio/pressure/bmp280-core.c
> > +++ b/drivers/iio/pressure/bmp280-core.c
> > @@ -118,6 +118,12 @@ enum bmp580_odr {
> >   */
> >  enum { T1, T2, T3, P1, P2, P3, P4, P5, P6, P7, P8, P9 };
> >  
> > +/*
> > + * These enums are used for indexing into the array of humidity parameters
> > + * for BME280.
> 
> I was thinking of a comment that also mentioned the overlap. Perhaps something like
> ...
> 
> 
> Index of the byte containing the start of each humidity parameter. Some
> parameters stretch across multiple bytes including into the start of the byte
> where another humidity parameter begins. Unaligned be/le accesses are used
> to allow fields to be extracted with FIELD_GET(). 
> 
> Or, just refer to the field layout being complex and to see
> bme280_read_calib function.
> 
> Actually come to think of it, just move this enum down there so it
> is local to the code and the usage is more obvious / comment less important.
> 

Hi Jonathan,

Thanks for the feedback once again. Sorry for the late reply, I was taking
some time off for vacation :)

Indeed your comment makes sense, I can do that.

Cheers,
Vasilis
> > + */
> > +enum { H2 = 0, H3 = 2, H4 = 3, H5 = 4, H6 = 6 };
> > +
> >  enum {
> >  	/* Temperature calib indexes */
> >  	BMP380_T1 = 0,
> > @@ -344,6 +350,7 @@ static int bme280_read_calib(struct bmp280_data *data)
> >  {
> >  	struct bmp280_calib *calib = &data->calib.bmp280;
> >  	struct device *dev = data->dev;
> > +	s16 h4_upper, h4_lower;
> >  	unsigned int tmp;
> >  	int ret;
> >  
> > @@ -352,14 +359,6 @@ static int bme280_read_calib(struct bmp280_data *data)
> >  	if (ret)
> >  		return ret;
> >  
> > -	/*
> > -	 * Read humidity calibration values.
> > -	 * Due to some odd register addressing we cannot just
> > -	 * do a big bulk read. Instead, we have to read each Hx
> > -	 * value separately and sometimes do some bit shifting...
> > -	 * Humidity data is only available on BME280.
> > -	 */
> > -
> >  	ret = regmap_read(data->regmap, BME280_REG_COMP_H1, &tmp);
> >  	if (ret) {
> >  		dev_err(dev, "failed to read H1 comp value\n");
> > @@ -368,43 +367,24 @@ static int bme280_read_calib(struct bmp280_data *data)
> >  	calib->H1 = tmp;
> >  
> >  	ret = regmap_bulk_read(data->regmap, BME280_REG_COMP_H2,
> > -			       &data->le16, sizeof(data->le16));
> > -	if (ret) {
> > -		dev_err(dev, "failed to read H2 comp value\n");
> > -		return ret;
> > -	}
> > -	calib->H2 = sign_extend32(le16_to_cpu(data->le16), 15);
> > -
> > -	ret = regmap_read(data->regmap, BME280_REG_COMP_H3, &tmp);
> > -	if (ret) {
> > -		dev_err(dev, "failed to read H3 comp value\n");
> > -		return ret;
> > -	}
> > -	calib->H3 = tmp;
> > -
> > -	ret = regmap_bulk_read(data->regmap, BME280_REG_COMP_H4,
> > -			       &data->be16, sizeof(data->be16));
> > +			       data->bme280_humid_cal_buf,
> > +			       sizeof(data->bme280_humid_cal_buf));
> >  	if (ret) {
> > -		dev_err(dev, "failed to read H4 comp value\n");
> > +		dev_err(dev, "failed to read humidity calibration values\n");
> >  		return ret;
> >  	}
> > -	calib->H4 = sign_extend32(((be16_to_cpu(data->be16) >> 4) & 0xff0) |
> > -				  (be16_to_cpu(data->be16) & 0xf), 11);
> >  
> > -	ret = regmap_bulk_read(data->regmap, BME280_REG_COMP_H5,
> > -			       &data->le16, sizeof(data->le16));
> > -	if (ret) {
> > -		dev_err(dev, "failed to read H5 comp value\n");
> > -		return ret;
> > -	}
> > -	calib->H5 = sign_extend32(FIELD_GET(BME280_COMP_H5_MASK, le16_to_cpu(data->le16)), 11);
> > -
> > -	ret = regmap_read(data->regmap, BME280_REG_COMP_H6, &tmp);
> > -	if (ret) {
> > -		dev_err(dev, "failed to read H6 comp value\n");
> > -		return ret;
> > -	}
> > -	calib->H6 = sign_extend32(tmp, 7);
> > +	calib->H2 = get_unaligned_le16(&data->bme280_humid_cal_buf[H2]);
> > +	calib->H3 = data->bme280_humid_cal_buf[H3];
> > +	h4_upper = FIELD_GET(BME280_COMP_H4_GET_MASK_UP,
> > +			get_unaligned_be16(&data->bme280_humid_cal_buf[H4]));
> > +	h4_upper = FIELD_PREP(BME280_COMP_H4_PREP_MASK_UP, h4_upper);
> > +	h4_lower = FIELD_GET(BME280_COMP_H4_MASK_LOW,
> > +			get_unaligned_be16(&data->bme280_humid_cal_buf[H4]));
> > +	calib->H4 = sign_extend32(h4_upper | h4_lower, 11);
> > +	calib->H5 = sign_extend32(FIELD_GET(BME280_COMP_H5_MASK,
> > +			get_unaligned_le16(&data->bme280_humid_cal_buf[H5])), 11);
> > +	calib->H6 = data->bme280_humid_cal_buf[H6];
> >  
> >  	return 0;
> >  }
> > diff --git a/drivers/iio/pressure/bmp280.h b/drivers/iio/pressure/bmp280.h
> > index ccacc67c1473..9bea0b84d2f4 100644
> > --- a/drivers/iio/pressure/bmp280.h
> > +++ b/drivers/iio/pressure/bmp280.h
> > @@ -257,8 +257,13 @@
> >  #define BME280_REG_COMP_H5		0xE5
> >  #define BME280_REG_COMP_H6		0xE7
> >  
> > +#define BME280_COMP_H4_GET_MASK_UP	GENMASK(15, 8)
> > +#define BME280_COMP_H4_PREP_MASK_UP	GENMASK(11, 4)
> > +#define BME280_COMP_H4_MASK_LOW		GENMASK(3, 0)
> >  #define BME280_COMP_H5_MASK		GENMASK(15, 4)
> >  
> > +#define BME280_CONTIGUOUS_CALIB_REGS	7
> > +
> >  #define BME280_OSRS_HUMIDITY_MASK	GENMASK(2, 0)
> >  #define BME280_OSRS_HUMIDITY_SKIP	0
> >  #define BME280_OSRS_HUMIDITY_1X		1
> > @@ -423,6 +428,7 @@ struct bmp280_data {
> >  		/* Calibration data buffers */
> >  		__le16 bmp280_cal_buf[BMP280_CONTIGUOUS_CALIB_REGS / 2];
> >  		__be16 bmp180_cal_buf[BMP180_REG_CALIB_COUNT / 2];
> > +		u8 bme280_humid_cal_buf[BME280_CONTIGUOUS_CALIB_REGS];
> >  		u8 bmp380_cal_buf[BMP380_CALIB_REG_COUNT];
> >  		/* Miscellaneous, endianness-aware data buffers */
> >  		__le16 le16;
> 

[PATCH v2 2/7] iio: pressure: bmp280: Add support for bmp280 soft reset

[Vasileios Amoiridis]

The BM(P/E)28x devices have an option for soft reset which is also
recommended by the Bosch Sensortech BME2 Sensor API to be used before the
initial configuration of the device.

Link: https://github.com/boschsensortec/BME280_SensorAPI/blob/bme280_v3.5.1/bme280.c#L429
Signed-off-by: Vasileios Amoiridis <vassilisamir@gmail.com>
---
 drivers/iio/pressure/bmp280-core.c | 28 ++++++++++++++++++++++++++++
 drivers/iio/pressure/bmp280.h      |  3 +++
 2 files changed, 31 insertions(+)

diff --git a/drivers/iio/pressure/bmp280-core.c b/drivers/iio/pressure/bmp280-core.c
index d5e5eb22667a..acbc33aacc09 100644
--- a/drivers/iio/pressure/bmp280-core.c
+++ b/drivers/iio/pressure/bmp280-core.c
@@ -963,6 +963,32 @@ static const unsigned long bme280_avail_scan_masks[] = {
 	0
 };
 
+static int bmp280_preinit(struct bmp280_data *data)
+{
+	unsigned int reg;
+	int ret;
+
+	ret = regmap_write(data->regmap, BMP280_REG_RESET, BMP280_RST_SOFT_CMD);
+	if (ret) {
+		dev_err(data->dev, "Failed to reset device.\n");
+		return ret;
+	}
+
+	usleep_range(data->start_up_time, data->start_up_time + 500);
+
+	ret = regmap_read(data->regmap, BMP280_REG_STATUS, &reg);
+	if (ret) {
+		dev_err(data->dev, "Failed to read status register.\n");
+		return ret;
+	}
+	if (reg & BMP280_REG_STATUS_IM_UPDATE) {
+		dev_err(data->dev, "Failed to copy NVM contents.\n");
+		return ret;
+	}
+
+	return 0;
+}
+
 static int bmp280_chip_config(struct bmp280_data *data)
 {
 	u8 osrs = FIELD_PREP(BMP280_OSRS_TEMP_MASK, data->oversampling_temp + 1) |
@@ -1079,6 +1105,7 @@ const struct bmp280_chip_info bmp280_chip_info = {
 	.read_temp = bmp280_read_temp,
 	.read_press = bmp280_read_press,
 	.read_calib = bmp280_read_calib,
+	.preinit = bmp280_preinit,
 
 	.trigger_handler = bmp280_trigger_handler,
 };
@@ -1196,6 +1223,7 @@ const struct bmp280_chip_info bme280_chip_info = {
 	.read_press = bmp280_read_press,
 	.read_humid = bme280_read_humid,
 	.read_calib = bme280_read_calib,
+	.preinit = bmp280_preinit,
 
 	.trigger_handler = bme280_trigger_handler,
 };
diff --git a/drivers/iio/pressure/bmp280.h b/drivers/iio/pressure/bmp280.h
index 9bea0b84d2f4..a9f220c1f77a 100644
--- a/drivers/iio/pressure/bmp280.h
+++ b/drivers/iio/pressure/bmp280.h
@@ -205,6 +205,9 @@
 #define BMP280_REG_CONFIG		0xF5
 #define BMP280_REG_CTRL_MEAS		0xF4
 #define BMP280_REG_STATUS		0xF3
+#define BMP280_REG_STATUS_IM_UPDATE	BIT(0)
+#define BMP280_REG_RESET		0xE0
+#define BMP280_RST_SOFT_CMD		0xB6
 
 #define BMP280_REG_COMP_TEMP_START	0x88
 #define BMP280_COMP_TEMP_REG_COUNT	6
-- 
2.25.1

Re: [PATCH v2 2/7] iio: pressure: bmp280: Add support for bmp280 soft reset

[Jonathan Cameron]

On Fri, 26 Jul 2024 01:10:34 +0200
Vasileios Amoiridis <vassilisamir@gmail.com> wrote:

> The BM(P/E)28x devices have an option for soft reset which is also
> recommended by the Bosch Sensortech BME2 Sensor API to be used before the
> initial configuration of the device.
> 
> Link: https://github.com/boschsensortec/BME280_SensorAPI/blob/bme280_v3.5.1/bme280.c#L429
> Signed-off-by: Vasileios Amoiridis <vassilisamir@gmail.com>
Trivial passing comment seeing as you are going do be doing a v3 anyway.

Jonathan

> ---
>  drivers/iio/pressure/bmp280-core.c | 28 ++++++++++++++++++++++++++++
>  drivers/iio/pressure/bmp280.h      |  3 +++
>  2 files changed, 31 insertions(+)
> 
> diff --git a/drivers/iio/pressure/bmp280-core.c b/drivers/iio/pressure/bmp280-core.c
> index d5e5eb22667a..acbc33aacc09 100644
> --- a/drivers/iio/pressure/bmp280-core.c
> +++ b/drivers/iio/pressure/bmp280-core.c
> @@ -963,6 +963,32 @@ static const unsigned long bme280_avail_scan_masks[] = {
>  	0
>  };
>  
> +static int bmp280_preinit(struct bmp280_data *data)
> +{
> +	unsigned int reg;
> +	int ret;
> +
> +	ret = regmap_write(data->regmap, BMP280_REG_RESET, BMP280_RST_SOFT_CMD);
> +	if (ret) {
> +		dev_err(data->dev, "Failed to reset device.\n");
> +		return ret;
Is this only ever called from probe?

If so, return dev_err_probe() which will save a few lines of code.

> +	}
> +
> +	usleep_range(data->start_up_time, data->start_up_time + 500);
> +
> +	ret = regmap_read(data->regmap, BMP280_REG_STATUS, &reg);
> +	if (ret) {
> +		dev_err(data->dev, "Failed to read status register.\n");
> +		return ret;
> +	}
> +	if (reg & BMP280_REG_STATUS_IM_UPDATE) {
> +		dev_err(data->dev, "Failed to copy NVM contents.\n");
> +		return ret;
> +	}
> +
> +	return 0;
> +}
> +
>  static int bmp280_chip_config(struct bmp280_data *data)
>  {
>  	u8 osrs = FIELD_PREP(BMP280_OSRS_TEMP_MASK, data->oversampling_temp + 1) |
> @@ -1079,6 +1105,7 @@ const struct bmp280_chip_info bmp280_chip_info = {
>  	.read_temp = bmp280_read_temp,
>  	.read_press = bmp280_read_press,
>  	.read_calib = bmp280_read_calib,
> +	.preinit = bmp280_preinit,
>  
>  	.trigger_handler = bmp280_trigger_handler,
>  };
> @@ -1196,6 +1223,7 @@ const struct bmp280_chip_info bme280_chip_info = {
>  	.read_press = bmp280_read_press,
>  	.read_humid = bme280_read_humid,
>  	.read_calib = bme280_read_calib,
> +	.preinit = bmp280_preinit,
>  
>  	.trigger_handler = bme280_trigger_handler,
>  };
> diff --git a/drivers/iio/pressure/bmp280.h b/drivers/iio/pressure/bmp280.h
> index 9bea0b84d2f4..a9f220c1f77a 100644
> --- a/drivers/iio/pressure/bmp280.h
> +++ b/drivers/iio/pressure/bmp280.h
> @@ -205,6 +205,9 @@
>  #define BMP280_REG_CONFIG		0xF5
>  #define BMP280_REG_CTRL_MEAS		0xF4
>  #define BMP280_REG_STATUS		0xF3
> +#define BMP280_REG_STATUS_IM_UPDATE	BIT(0)
> +#define BMP280_REG_RESET		0xE0
> +#define BMP280_RST_SOFT_CMD		0xB6
>  
>  #define BMP280_REG_COMP_TEMP_START	0x88
>  #define BMP280_COMP_TEMP_REG_COUNT	6

Re: [PATCH v2 2/7] iio: pressure: bmp280: Add support for bmp280 soft reset

[Vasileios Amoiridis]

On Sun, Jul 28, 2024 at 04:49:01PM +0100, Jonathan Cameron wrote:
> On Fri, 26 Jul 2024 01:10:34 +0200
> Vasileios Amoiridis <vassilisamir@gmail.com> wrote:
> 
> > The BM(P/E)28x devices have an option for soft reset which is also
> > recommended by the Bosch Sensortech BME2 Sensor API to be used before the
> > initial configuration of the device.
> > 
> > Link: https://github.com/boschsensortec/BME280_SensorAPI/blob/bme280_v3.5.1/bme280.c#L429
> > Signed-off-by: Vasileios Amoiridis <vassilisamir@gmail.com>
> Trivial passing comment seeing as you are going do be doing a v3 anyway.
> 
> Jonathan
> 
> > ---
> >  drivers/iio/pressure/bmp280-core.c | 28 ++++++++++++++++++++++++++++
> >  drivers/iio/pressure/bmp280.h      |  3 +++
> >  2 files changed, 31 insertions(+)
> > 
> > diff --git a/drivers/iio/pressure/bmp280-core.c b/drivers/iio/pressure/bmp280-core.c
> > index d5e5eb22667a..acbc33aacc09 100644
> > --- a/drivers/iio/pressure/bmp280-core.c
> > +++ b/drivers/iio/pressure/bmp280-core.c
> > @@ -963,6 +963,32 @@ static const unsigned long bme280_avail_scan_masks[] = {
> >  	0
> >  };
> >  
> > +static int bmp280_preinit(struct bmp280_data *data)
> > +{
> > +	unsigned int reg;
> > +	int ret;
> > +
> > +	ret = regmap_write(data->regmap, BMP280_REG_RESET, BMP280_RST_SOFT_CMD);
> > +	if (ret) {
> > +		dev_err(data->dev, "Failed to reset device.\n");
> > +		return ret;
> Is this only ever called from probe?
> 
> If so, return dev_err_probe() which will save a few lines of code.
> 

Hi Jonathan,

Indeed, this is being called only from probe so I could change that as you
proposed!

Cheers,
Vasilis

> > +	}
> > +
> > +	usleep_range(data->start_up_time, data->start_up_time + 500);
> > +
> > +	ret = regmap_read(data->regmap, BMP280_REG_STATUS, &reg);
> > +	if (ret) {
> > +		dev_err(data->dev, "Failed to read status register.\n");
> > +		return ret;
> > +	}
> > +	if (reg & BMP280_REG_STATUS_IM_UPDATE) {
> > +		dev_err(data->dev, "Failed to copy NVM contents.\n");
> > +		return ret;
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> >  static int bmp280_chip_config(struct bmp280_data *data)
> >  {
> >  	u8 osrs = FIELD_PREP(BMP280_OSRS_TEMP_MASK, data->oversampling_temp + 1) |
> > @@ -1079,6 +1105,7 @@ const struct bmp280_chip_info bmp280_chip_info = {
> >  	.read_temp = bmp280_read_temp,
> >  	.read_press = bmp280_read_press,
> >  	.read_calib = bmp280_read_calib,
> > +	.preinit = bmp280_preinit,
> >  
> >  	.trigger_handler = bmp280_trigger_handler,
> >  };
> > @@ -1196,6 +1223,7 @@ const struct bmp280_chip_info bme280_chip_info = {
> >  	.read_press = bmp280_read_press,
> >  	.read_humid = bme280_read_humid,
> >  	.read_calib = bme280_read_calib,
> > +	.preinit = bmp280_preinit,
> >  
> >  	.trigger_handler = bme280_trigger_handler,
> >  };
> > diff --git a/drivers/iio/pressure/bmp280.h b/drivers/iio/pressure/bmp280.h
> > index 9bea0b84d2f4..a9f220c1f77a 100644
> > --- a/drivers/iio/pressure/bmp280.h
> > +++ b/drivers/iio/pressure/bmp280.h
> > @@ -205,6 +205,9 @@
> >  #define BMP280_REG_CONFIG		0xF5
> >  #define BMP280_REG_CTRL_MEAS		0xF4
> >  #define BMP280_REG_STATUS		0xF3
> > +#define BMP280_REG_STATUS_IM_UPDATE	BIT(0)
> > +#define BMP280_REG_RESET		0xE0
> > +#define BMP280_RST_SOFT_CMD		0xB6
> >  
> >  #define BMP280_REG_COMP_TEMP_START	0x88
> >  #define BMP280_COMP_TEMP_REG_COUNT	6
> 

[PATCH v2 3/7] iio: pressure: bmp280: Remove config error check for IIR filter updates

[Vasileios Amoiridis]

When there is a change in the configuration of the BMP3xx device, several
steps take place. These steps include:

1) Update the OSR settings and check if there was an update
2) Update the ODR settings and check if there was an update
3) Update the IIR settings and check if there was an update
4) Check if there was an update with the following procedure:
	a) Set sensor to SLEEP mode and after to NORMAL mode to trigger
	   a new measurement.
	b) Wait the maximum amount possible depending on the OSR settings
	c) Check the configuration error register if there was an error
	   during the configuration of the sensor.

This check is necessary, because there could be a case where the OSR is
too high for the requested ODR so either the ODR needs to be slower or the
OSR needs to be less. This is something that is checked internally by the
sensor when it runs in NORMAL mode.

In the BMP58x devices the previous steps are done internally by the sensor.

The IIR filter settings do not depend on the OSR or ODR settings, and there
is no need to run a check in case they change.

Signed-off-by: Vasileios Amoiridis <vassilisamir@gmail.com>
---
 drivers/iio/pressure/bmp280-core.c | 14 +++++---------
 1 file changed, 5 insertions(+), 9 deletions(-)

diff --git a/drivers/iio/pressure/bmp280-core.c b/drivers/iio/pressure/bmp280-core.c
index acbc33aacc09..431fbe9f3ee9 100644
--- a/drivers/iio/pressure/bmp280-core.c
+++ b/drivers/iio/pressure/bmp280-core.c
@@ -1553,14 +1553,12 @@ static int bmp380_chip_config(struct bmp280_data *data)
 	change = change || aux;
 
 	/* Set filter data */
-	ret = regmap_update_bits_check(data->regmap, BMP380_REG_CONFIG, BMP380_FILTER_MASK,
-				       FIELD_PREP(BMP380_FILTER_MASK, data->iir_filter_coeff),
-				       &aux);
+	ret = regmap_update_bits(data->regmap, BMP380_REG_CONFIG, BMP380_FILTER_MASK,
+				 FIELD_PREP(BMP380_FILTER_MASK, data->iir_filter_coeff));
 	if (ret) {
 		dev_err(data->dev, "failed to write config register\n");
 		return ret;
 	}
-	change = change || aux;
 
 	if (change) {
 		/*
@@ -2149,15 +2147,13 @@ static int bmp580_chip_config(struct bmp280_data *data)
 	reg_val = FIELD_PREP(BMP580_DSP_IIR_PRESS_MASK, data->iir_filter_coeff) |
 		  FIELD_PREP(BMP580_DSP_IIR_TEMP_MASK, data->iir_filter_coeff);
 
-	ret = regmap_update_bits_check(data->regmap, BMP580_REG_DSP_IIR,
-				       BMP580_DSP_IIR_PRESS_MASK |
-				       BMP580_DSP_IIR_TEMP_MASK,
-				       reg_val, &aux);
+	ret = regmap_update_bits(data->regmap, BMP580_REG_DSP_IIR,
+				 BMP580_DSP_IIR_PRESS_MASK |
+				 BMP580_DSP_IIR_TEMP_MASK, reg_val);
 	if (ret) {
 		dev_err(data->dev, "failed to write config register\n");
 		return ret;
 	}
-	change = change || aux;
 
 	/* Restore sensor to normal operation mode */
 	ret = regmap_write_bits(data->regmap, BMP580_REG_ODR_CONFIG,
-- 
2.25.1

[PATCH v2 4/7] iio: pressure: bmp280: Use sleep and forced mode for oneshot captures

[Vasileios Amoiridis]

This commit adds forced mode support in sensors BMP28x, BME28x, BMP3xx
and BMP58x. Sensors BMP18x and BMP085 are old and do not support this
feature so their operation is not affected at all.

Essentially, up to now, the rest of the sensors were used in normal mode
all the time. This means that they are continuously doing measurements
even though these measurements are not used. Even though the sensor does
provide PM support, to cover all the possible use cases, the sensor needs
to go into sleep mode and wake up whenever necessary.

This commit, adds sleep and forced mode support. Essentially, the sensor
sleeps all the time except for when a measurement is requested. When there
is a request for a measurement, the sensor is put into forced mode, starts
the measurement and after it is done we read the output and we put it again
in sleep mode.

For really fast and more deterministic measurements, the triggered buffer
interface can be used, since the sensor is still used in normal mode for
that use case.

This commit does not add though support for DEEP STANDBY, Low Power NORMAL
and CONTINUOUS modes, supported only by the BMP58x version.

Signed-off-by: Vasileios Amoiridis <vassilisamir@gmail.com>
---
 drivers/iio/pressure/bmp280-core.c | 264 ++++++++++++++++++++++++++---
 drivers/iio/pressure/bmp280.h      |  18 ++
 2 files changed, 263 insertions(+), 19 deletions(-)

diff --git a/drivers/iio/pressure/bmp280-core.c b/drivers/iio/pressure/bmp280-core.c
index 431fbe9f3ee9..4a8d2ed4a9c4 100644
--- a/drivers/iio/pressure/bmp280-core.c
+++ b/drivers/iio/pressure/bmp280-core.c
@@ -615,6 +615,14 @@ static int bmp280_read_raw_impl(struct iio_dev *indio_dev,
 
 	switch (mask) {
 	case IIO_CHAN_INFO_PROCESSED:
+		ret = data->chip_info->set_mode(data, BMP280_FORCED);
+		if (ret)
+			return ret;
+
+		ret = data->chip_info->wait_conv(data);
+		if (ret)
+			return ret;
+
 		switch (chan->type) {
 		case IIO_HUMIDITYRELATIVE:
 			ret = data->chip_info->read_humid(data, &chan_value);
@@ -644,6 +652,14 @@ static int bmp280_read_raw_impl(struct iio_dev *indio_dev,
 			return -EINVAL;
 		}
 	case IIO_CHAN_INFO_RAW:
+		ret = data->chip_info->set_mode(data, BMP280_FORCED);
+		if (ret)
+			return ret;
+
+		ret = data->chip_info->wait_conv(data);
+		if (ret)
+			return ret;
+
 		switch (chan->type) {
 		case IIO_HUMIDITYRELATIVE:
 			ret = data->chip_info->read_humid(data, &chan_value);
@@ -989,6 +1005,67 @@ static int bmp280_preinit(struct bmp280_data *data)
 	return 0;
 }
 
+static const u8 bmp280_operation_mode[] = { BMP280_MODE_SLEEP,
+					    BMP280_MODE_FORCED,
+					    BMP280_MODE_NORMAL };
+
+static int bmp280_set_mode(struct bmp280_data *data, enum bmp280_op_mode mode)
+{
+	int ret;
+
+	switch (mode) {
+	case BMP280_SLEEP:
+	case BMP280_FORCED:
+	case BMP280_NORMAL:
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	ret = regmap_write_bits(data->regmap, BMP280_REG_CTRL_MEAS,
+				BMP280_MODE_MASK, bmp280_operation_mode[mode]);
+	if (ret) {
+		dev_err(data->dev, "failed to  write ctrl_meas register\n");
+		return ret;
+	}
+
+	return 0;
+}
+
+static int bmp280_wait_conv(struct bmp280_data *data)
+{
+	unsigned int reg;
+	int ret, meas_time;
+
+	meas_time = BMP280_MEAS_OFFSET;
+
+	/* Check if we are using a BME280 device */
+	if (data->oversampling_humid)
+		meas_time += (1 << data->oversampling_humid) * BMP280_MEAS_DUR +
+			       BMP280_PRESS_HUMID_MEAS_OFFSET;
+
+	/* Pressure measurement time */
+	meas_time += (1 << data->oversampling_press) * BMP280_MEAS_DUR +
+		      BMP280_PRESS_HUMID_MEAS_OFFSET;
+
+	/* Temperature measurement time */
+	meas_time += (1 << data->oversampling_temp) * BMP280_MEAS_DUR;
+
+	usleep_range(meas_time, meas_time * 12 / 10);
+
+	ret = regmap_read(data->regmap, BMP280_REG_STATUS, &reg);
+	if (ret) {
+		dev_err(data->dev, "failed to read status register\n");
+		return ret;
+	}
+	if (reg & BMP280_REG_STATUS_MEAS_BIT) {
+		dev_err(data->dev, "Measurement cycle didn't complete\n");
+		return -EBUSY;
+	}
+
+	return 0;
+}
+
 static int bmp280_chip_config(struct bmp280_data *data)
 {
 	u8 osrs = FIELD_PREP(BMP280_OSRS_TEMP_MASK, data->oversampling_temp + 1) |
@@ -999,7 +1076,7 @@ static int bmp280_chip_config(struct bmp280_data *data)
 				BMP280_OSRS_TEMP_MASK |
 				BMP280_OSRS_PRESS_MASK |
 				BMP280_MODE_MASK,
-				osrs | BMP280_MODE_NORMAL);
+				osrs | BMP280_MODE_SLEEP);
 	if (ret) {
 		dev_err(data->dev, "failed to write ctrl_meas register\n");
 		return ret;
@@ -1105,6 +1182,8 @@ const struct bmp280_chip_info bmp280_chip_info = {
 	.read_temp = bmp280_read_temp,
 	.read_press = bmp280_read_press,
 	.read_calib = bmp280_read_calib,
+	.set_mode = bmp280_set_mode,
+	.wait_conv = bmp280_wait_conv,
 	.preinit = bmp280_preinit,
 
 	.trigger_handler = bmp280_trigger_handler,
@@ -1223,6 +1302,8 @@ const struct bmp280_chip_info bme280_chip_info = {
 	.read_press = bmp280_read_press,
 	.read_humid = bme280_read_humid,
 	.read_calib = bme280_read_calib,
+	.set_mode = bmp280_set_mode,
+	.wait_conv = bmp280_wait_conv,
 	.preinit = bmp280_preinit,
 
 	.trigger_handler = bme280_trigger_handler,
@@ -1510,6 +1591,68 @@ static int bmp380_preinit(struct bmp280_data *data)
 	return bmp380_cmd(data, BMP380_CMD_SOFT_RESET);
 }
 
+static const u8 bmp380_operation_mode[] = { BMP380_MODE_SLEEP,
+					    BMP380_MODE_FORCED,
+					    BMP380_MODE_NORMAL };
+
+static int bmp380_set_mode(struct bmp280_data *data, enum bmp280_op_mode mode)
+{
+	int ret;
+
+	switch (mode) {
+	case BMP280_SLEEP:
+	case BMP280_FORCED:
+	case BMP280_NORMAL:
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	ret = regmap_write_bits(data->regmap, BMP380_REG_POWER_CONTROL,
+				BMP380_MODE_MASK,
+				FIELD_PREP(BMP380_MODE_MASK,
+					   bmp380_operation_mode[mode]));
+	if (ret) {
+		dev_err(data->dev, "failed to  write power control register\n");
+		return ret;
+	}
+
+	return 0;
+}
+
+static int bmp380_wait_conv(struct bmp280_data *data)
+{
+	unsigned int reg;
+	int ret, meas_time;
+
+	/* Offset measurement time */
+	meas_time = BMP380_MEAS_OFFSET;
+
+	/* Pressure measurement time */
+	meas_time += (1 << data->oversampling_press) * BMP380_MEAS_DUR +
+		      BMP380_PRESS_MEAS_OFFSET;
+
+	/* Temperature measurement time */
+	meas_time += (1 << data->oversampling_temp) * BMP380_MEAS_DUR +
+		      BMP380_TEMP_MEAS_OFFSET;
+
+	usleep_range(meas_time, meas_time * 12 / 10);
+
+	ret = regmap_read(data->regmap, BMP380_REG_STATUS, &reg);
+	if (ret) {
+		dev_err(data->dev, "failed to read status register\n");
+		return ret;
+	}
+
+	if (!(reg & BMP380_STATUS_DRDY_PRESS_MASK) ||
+	    !(reg & BMP380_STATUS_DRDY_TEMP_MASK)) {
+		dev_err(data->dev, "Measurement cycle didn't complete.\n");
+		return -EBUSY;
+	}
+
+	return 0;
+}
+
 static int bmp380_chip_config(struct bmp280_data *data)
 {
 	bool change = false, aux;
@@ -1570,17 +1713,15 @@ static int bmp380_chip_config(struct bmp280_data *data)
 		 * Resets sensor measurement loop toggling between sleep and
 		 * normal operating modes.
 		 */
-		ret = regmap_write_bits(data->regmap, BMP380_REG_POWER_CONTROL,
-					BMP380_MODE_MASK,
-					FIELD_PREP(BMP380_MODE_MASK, BMP380_MODE_SLEEP));
+		ret = bmp380_set_mode(data, BMP280_SLEEP);
 		if (ret) {
 			dev_err(data->dev, "failed to set sleep mode\n");
 			return ret;
 		}
-		usleep_range(2000, 2500);
-		ret = regmap_write_bits(data->regmap, BMP380_REG_POWER_CONTROL,
-					BMP380_MODE_MASK,
-					FIELD_PREP(BMP380_MODE_MASK, BMP380_MODE_NORMAL));
+
+		usleep_range(data->start_up_time, data->start_up_time + 500);
+
+		ret = bmp380_set_mode(data, BMP280_NORMAL);
 		if (ret) {
 			dev_err(data->dev, "failed to set normal mode\n");
 			return ret;
@@ -1606,6 +1747,17 @@ static int bmp380_chip_config(struct bmp280_data *data)
 		}
 	}
 
+	/* Dummy read to empty data registers. */
+	ret = bmp380_read_press(data, &tmp);
+	if (ret)
+		return ret;
+
+	ret = bmp380_set_mode(data, BMP280_SLEEP);
+	if (ret) {
+		dev_err(data->dev, "failed to set sleep mode\n");
+		return ret;
+	}
+
 	return 0;
 }
 
@@ -1698,6 +1850,8 @@ const struct bmp280_chip_info bmp380_chip_info = {
 	.read_temp = bmp380_read_temp,
 	.read_press = bmp380_read_press,
 	.read_calib = bmp380_read_calib,
+	.set_mode = bmp380_set_mode,
+	.wait_conv = bmp380_wait_conv,
 	.preinit = bmp380_preinit,
 
 	.trigger_handler = bmp380_trigger_handler,
@@ -2085,6 +2239,64 @@ static int bmp580_preinit(struct bmp280_data *data)
 	return PTR_ERR_OR_ZERO(devm_nvmem_register(config.dev, &config));
 }
 
+static const u8 bmp580_operation_mode[] = { BMP580_MODE_SLEEP,
+					    BMP580_MODE_FORCED,
+					    BMP580_MODE_NORMAL };
+
+static int bmp580_set_mode(struct bmp280_data *data, enum bmp280_op_mode mode)
+{
+	int ret;
+
+	switch (mode) {
+	case BMP280_SLEEP:
+		break;
+	case BMP280_FORCED:
+		ret = regmap_set_bits(data->regmap, BMP580_REG_DSP_CONFIG,
+				      BMP580_DSP_IIR_FORCED_FLUSH);
+		if (ret) {
+			dev_err(data->dev,
+				"Could not flush IIR filter constants.\n");
+			return ret;
+		}
+		break;
+	case BMP280_NORMAL:
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	ret = regmap_write_bits(data->regmap, BMP580_REG_ODR_CONFIG,
+				BMP580_MODE_MASK,
+				FIELD_PREP(BMP580_MODE_MASK,
+					   bmp580_operation_mode[mode]));
+	if (ret) {
+		dev_err(data->dev, "failed to  write power control register\n");
+		return ret;
+	}
+
+	return 0;
+}
+
+static int bmp580_wait_conv(struct bmp280_data *data)
+{
+	/*
+	 * Taken from datasheet, Section 2 "Specification, Table 3 "Electrical
+	 * characteristics
+	 */
+	const int time_conv_press[] = { 0, 1050, 1785, 3045, 5670, 10920, 21420,
+					42420, 84420};
+	const int time_conv_temp[] = { 0, 1050, 1105, 1575, 2205, 3465, 6090,
+				       11340, 21840};
+	int meas_time;
+
+	meas_time = 4000 + time_conv_temp[data->oversampling_temp] +
+			   time_conv_press[data->oversampling_press];
+
+	usleep_range(meas_time, meas_time * 12 / 10);
+
+	return 0;
+}
+
 static int bmp580_chip_config(struct bmp280_data *data)
 {
 	bool change = false, aux;
@@ -2155,17 +2367,6 @@ static int bmp580_chip_config(struct bmp280_data *data)
 		return ret;
 	}
 
-	/* Restore sensor to normal operation mode */
-	ret = regmap_write_bits(data->regmap, BMP580_REG_ODR_CONFIG,
-				BMP580_MODE_MASK,
-				FIELD_PREP(BMP580_MODE_MASK, BMP580_MODE_NORMAL));
-	if (ret) {
-		dev_err(data->dev, "failed to set normal mode\n");
-		return ret;
-	}
-	/* From datasheet's table 4: electrical characteristics */
-	usleep_range(3000, 3500);
-
 	if (change) {
 		/*
 		 * Check if ODR and OSR settings are valid or we are
@@ -2261,6 +2462,8 @@ const struct bmp280_chip_info bmp580_chip_info = {
 	.chip_config = bmp580_chip_config,
 	.read_temp = bmp580_read_temp,
 	.read_press = bmp580_read_press,
+	.set_mode = bmp580_set_mode,
+	.wait_conv = bmp580_wait_conv,
 	.preinit = bmp580_preinit,
 
 	.trigger_handler = bmp580_trigger_handler,
@@ -2508,6 +2711,19 @@ static int bmp180_read_press(struct bmp280_data *data, u32 *comp_press)
 	return 0;
 }
 
+/* Keep compatibility with future generations of the sensor */
+static int bmp180_set_mode(struct bmp280_data *data, enum bmp280_op_mode mode)
+{
+	return 0;
+}
+
+/* Keep compatibility with future generations of the sensor */
+static int bmp180_wait_conv(struct bmp280_data *data)
+{
+	return 0;
+}
+
+/* Keep compatibility with future generations of the sensor */
 static int bmp180_chip_config(struct bmp280_data *data)
 {
 	return 0;
@@ -2578,6 +2794,8 @@ const struct bmp280_chip_info bmp180_chip_info = {
 	.read_temp = bmp180_read_temp,
 	.read_press = bmp180_read_press,
 	.read_calib = bmp180_read_calib,
+	.set_mode = bmp180_set_mode,
+	.wait_conv = bmp180_wait_conv,
 
 	.trigger_handler = bmp180_trigger_handler,
 };
@@ -2630,6 +2848,7 @@ static int bmp280_buffer_preenable(struct iio_dev *indio_dev)
 	struct bmp280_data *data = iio_priv(indio_dev);
 
 	pm_runtime_get_sync(data->dev);
+	data->chip_info->set_mode(data, BMP280_NORMAL);
 
 	return 0;
 }
@@ -2800,6 +3019,10 @@ int bmp280_common_probe(struct device *dev,
 			return ret;
 	}
 
+	ret = data->chip_info->set_mode(data, BMP280_SLEEP);
+	if (ret)
+		return dev_err_probe(dev, ret, "Failed to set sleep mode\n");
+
 	/* Enable runtime PM */
 	pm_runtime_get_noresume(dev);
 	pm_runtime_set_active(dev);
@@ -2825,6 +3048,9 @@ static int bmp280_runtime_suspend(struct device *dev)
 	struct iio_dev *indio_dev = dev_get_drvdata(dev);
 	struct bmp280_data *data = iio_priv(indio_dev);
 
+	data->chip_info->set_mode(data, BMP280_SLEEP);
+
+	usleep_range(2500, 3000);
 	return regulator_bulk_disable(BMP280_NUM_SUPPLIES, data->supplies);
 }
 
diff --git a/drivers/iio/pressure/bmp280.h b/drivers/iio/pressure/bmp280.h
index a9f220c1f77a..f5d192509d61 100644
--- a/drivers/iio/pressure/bmp280.h
+++ b/drivers/iio/pressure/bmp280.h
@@ -170,6 +170,11 @@
 #define BMP380_MODE_FORCED		1
 #define BMP380_MODE_NORMAL		3
 
+#define BMP380_MEAS_OFFSET		234
+#define BMP380_MEAS_DUR			2020
+#define BMP380_TEMP_MEAS_OFFSET		163
+#define BMP380_PRESS_MEAS_OFFSET	392
+
 #define BMP380_MIN_TEMP			-4000
 #define BMP380_MAX_TEMP			8500
 #define BMP380_MIN_PRES			3000000
@@ -206,6 +211,7 @@
 #define BMP280_REG_CTRL_MEAS		0xF4
 #define BMP280_REG_STATUS		0xF3
 #define BMP280_REG_STATUS_IM_UPDATE	BIT(0)
+#define BMP280_REG_STATUS_MEAS_BIT	BIT(3)
 #define BMP280_REG_RESET		0xE0
 #define BMP280_RST_SOFT_CMD		0xB6
 
@@ -246,6 +252,10 @@
 #define BMP280_MODE_FORCED		1
 #define BMP280_MODE_NORMAL		3
 
+#define BMP280_MEAS_OFFSET		1250
+#define BMP280_MEAS_DUR			2300
+#define BMP280_PRESS_HUMID_MEAS_OFFSET	575
+
 /* BME280 specific registers */
 #define BME280_REG_HUMIDITY_LSB		0xFE
 #define BME280_REG_HUMIDITY_MSB		0xFD
@@ -384,6 +394,12 @@ struct bmp380_calib {
 	s8  P11;
 };
 
+enum bmp280_op_mode {
+	BMP280_SLEEP,
+	BMP280_FORCED,
+	BMP280_NORMAL,
+};
+
 struct bmp280_data {
 	struct device *dev;
 	struct mutex lock;
@@ -485,6 +501,8 @@ struct bmp280_chip_info {
 	int (*read_humid)(struct bmp280_data *data, u32 *adc_humidity);
 	int (*read_calib)(struct bmp280_data *data);
 	int (*preinit)(struct bmp280_data *data);
+	int (*set_mode)(struct bmp280_data *data, enum bmp280_op_mode mode);
+	int (*wait_conv)(struct bmp280_data *data);
 
 	irqreturn_t (*trigger_handler)(int irq, void *p);
 };
-- 
2.25.1

Re: [PATCH v2 4/7] iio: pressure: bmp280: Use sleep and forced mode for oneshot captures

[Jonathan Cameron]

On Fri, 26 Jul 2024 01:10:36 +0200
Vasileios Amoiridis <vassilisamir@gmail.com> wrote:

> This commit adds forced mode support in sensors BMP28x, BME28x, BMP3xx
> and BMP58x. Sensors BMP18x and BMP085 are old and do not support this
> feature so their operation is not affected at all.
> 
> Essentially, up to now, the rest of the sensors were used in normal mode
> all the time. This means that they are continuously doing measurements
> even though these measurements are not used. Even though the sensor does
> provide PM support, to cover all the possible use cases, the sensor needs
> to go into sleep mode and wake up whenever necessary.
> 
> This commit, adds sleep and forced mode support. Essentially, the sensor
> sleeps all the time except for when a measurement is requested. When there
> is a request for a measurement, the sensor is put into forced mode, starts
> the measurement and after it is done we read the output and we put it again
> in sleep mode.
> 
> For really fast and more deterministic measurements, the triggered buffer
> interface can be used, since the sensor is still used in normal mode for
> that use case.
> 
> This commit does not add though support for DEEP STANDBY, Low Power NORMAL
> and CONTINUOUS modes, supported only by the BMP58x version.
> 
> Signed-off-by: Vasileios Amoiridis <vassilisamir@gmail.com>
One question inline about the corner case of buffered capture in progress
when the machine is suspended.  We'd like the device to carry on feeding
us data on resume. Does that happen?

Jonathan


>  	.trigger_handler = bmp380_trigger_handler,
> @@ -2085,6 +2239,64 @@ static int bmp580_preinit(struct bmp280_data *data)
>  	return PTR_ERR_OR_ZERO(devm_nvmem_register(config.dev, &config));
>  }
>  
> +static const u8 bmp580_operation_mode[] = { BMP580_MODE_SLEEP,
> +					    BMP580_MODE_FORCED,
> +					    BMP580_MODE_NORMAL };
> +


> +
> +static int bmp580_wait_conv(struct bmp280_data *data)
> +{
> +	/*
> +	 * Taken from datasheet, Section 2 "Specification, Table 3 "Electrical
> +	 * characteristics
> +	 */
> +	const int time_conv_press[] = { 0, 1050, 1785, 3045, 5670, 10920, 21420,
> +					42420, 84420};
> +	const int time_conv_temp[] = { 0, 1050, 1105, 1575, 2205, 3465, 6090,
> +				       11340, 21840};
space before }

Also stick a static in front of them or Colin will ;)
Aim being to makes sure they aren't pointlessly allocated on the stack
if the compiler doesn't do something clever with them.

> +	int meas_time;
> +
> +	meas_time = 4000 + time_conv_temp[data->oversampling_temp] +
> +			   time_conv_press[data->oversampling_press];
> +
> +	usleep_range(meas_time, meas_time * 12 / 10);
> +
> +	return 0;
> +}
>
>  
> +/* Keep compatibility with future generations of the sensor */
> +static int bmp180_set_mode(struct bmp280_data *data, enum bmp280_op_mode mode)
> +{
> +	return 0;
> +}
> +
> +/* Keep compatibility with future generations of the sensor */
> +static int bmp180_wait_conv(struct bmp280_data *data)
> +{
> +	return 0;
> +}
> +
> +/* Keep compatibility with future generations of the sensor */

What does this comment mean?  I'm in favour of course, but don't understand
why it is here and above the stub calls.


> @@ -2825,6 +3048,9 @@ static int bmp280_runtime_suspend(struct device *dev)
>  	struct iio_dev *indio_dev = dev_get_drvdata(dev);
>  	struct bmp280_data *data = iio_priv(indio_dev);
>  
> +	data->chip_info->set_mode(data, BMP280_SLEEP);

What happens if the device is in buffered mode and you suspend?
I'd expect to see the power mode stashed somewhere and restored in resume.

> +
> +	usleep_range(2500, 3000);
>  	return regulator_bulk_disable(BMP280_NUM_SUPPLIES, data->supplies);
>  }
>  

Re: [PATCH v2 4/7] iio: pressure: bmp280: Use sleep and forced mode for oneshot captures

[Vasileios Amoiridis]

On Sun, Jul 28, 2024 at 04:57:24PM +0100, Jonathan Cameron wrote:
> On Fri, 26 Jul 2024 01:10:36 +0200
> Vasileios Amoiridis <vassilisamir@gmail.com> wrote:
> 
> > This commit adds forced mode support in sensors BMP28x, BME28x, BMP3xx
> > and BMP58x. Sensors BMP18x and BMP085 are old and do not support this
> > feature so their operation is not affected at all.
> > 
> > Essentially, up to now, the rest of the sensors were used in normal mode
> > all the time. This means that they are continuously doing measurements
> > even though these measurements are not used. Even though the sensor does
> > provide PM support, to cover all the possible use cases, the sensor needs
> > to go into sleep mode and wake up whenever necessary.
> > 
> > This commit, adds sleep and forced mode support. Essentially, the sensor
> > sleeps all the time except for when a measurement is requested. When there
> > is a request for a measurement, the sensor is put into forced mode, starts
> > the measurement and after it is done we read the output and we put it again
> > in sleep mode.
> > 
> > For really fast and more deterministic measurements, the triggered buffer
> > interface can be used, since the sensor is still used in normal mode for
> > that use case.
> > 
> > This commit does not add though support for DEEP STANDBY, Low Power NORMAL
> > and CONTINUOUS modes, supported only by the BMP58x version.
> > 
> > Signed-off-by: Vasileios Amoiridis <vassilisamir@gmail.com>
> One question inline about the corner case of buffered capture in progress
> when the machine is suspended.  We'd like the device to carry on feeding
> us data on resume. Does that happen?
> 
> Jonathan
> 

Hi Jonathan,

This is actually a corner case that I couldn't think of. I will have to think
it a bit more and come back on that.

> 
> >  	.trigger_handler = bmp380_trigger_handler,
> > @@ -2085,6 +2239,64 @@ static int bmp580_preinit(struct bmp280_data *data)
> >  	return PTR_ERR_OR_ZERO(devm_nvmem_register(config.dev, &config));
> >  }
> >  
> > +static const u8 bmp580_operation_mode[] = { BMP580_MODE_SLEEP,
> > +					    BMP580_MODE_FORCED,
> > +					    BMP580_MODE_NORMAL };
> > +
> 
> 
> > +
> > +static int bmp580_wait_conv(struct bmp280_data *data)
> > +{
> > +	/*
> > +	 * Taken from datasheet, Section 2 "Specification, Table 3 "Electrical
> > +	 * characteristics
> > +	 */
> > +	const int time_conv_press[] = { 0, 1050, 1785, 3045, 5670, 10920, 21420,
> > +					42420, 84420};
> > +	const int time_conv_temp[] = { 0, 1050, 1105, 1575, 2205, 3465, 6090,
> > +				       11340, 21840};
> space before }
> 
> Also stick a static in front of them or Colin will ;)
> Aim being to makes sure they aren't pointlessly allocated on the stack
> if the compiler doesn't do something clever with them.
> 

Ack.

> > +	int meas_time;
> > +
> > +	meas_time = 4000 + time_conv_temp[data->oversampling_temp] +
> > +			   time_conv_press[data->oversampling_press];
> > +
> > +	usleep_range(meas_time, meas_time * 12 / 10);
> > +
> > +	return 0;
> > +}
> >
> >  
> > +/* Keep compatibility with future generations of the sensor */
> > +static int bmp180_set_mode(struct bmp280_data *data, enum bmp280_op_mode mode)
> > +{
> > +	return 0;
> > +}
> > +
> > +/* Keep compatibility with future generations of the sensor */
> > +static int bmp180_wait_conv(struct bmp280_data *data)
> > +{
> > +	return 0;
> > +}
> > +
> > +/* Keep compatibility with future generations of the sensor */
> 
> What does this comment mean?  I'm in favour of course, but don't understand
> why it is here and above the stub calls.
> 
> 

This is for the bm(p/e)(2/3/5)80 devices which actually use those functions.
Maybe instead of "future" I should have put "newer".

> > @@ -2825,6 +3048,9 @@ static int bmp280_runtime_suspend(struct device *dev)
> >  	struct iio_dev *indio_dev = dev_get_drvdata(dev);
> >  	struct bmp280_data *data = iio_priv(indio_dev);
> >  
> > +	data->chip_info->set_mode(data, BMP280_SLEEP);
> 
> What happens if the device is in buffered mode and you suspend?
> I'd expect to see the power mode stashed somewhere and restored in resume.
> 

As said before, I will investigate it and come back with more info.

Cheers,
Vasilis
> > +
> > +	usleep_range(2500, 3000);
> >  	return regulator_bulk_disable(BMP280_NUM_SUPPLIES, data->supplies);
> >  }
> >  

[PATCH v2 5/7] dt-bindings: iio: pressure: bmp085: Add interrupts for BMP3xx and BMP5xx devices

[Vasileios Amoiridis]

Add interrupt options for BMP3xx and BMP5xx devices as well.

Signed-off-by: Vasileios Amoiridis <vassilisamir@gmail.com>
---
 Documentation/devicetree/bindings/iio/pressure/bmp085.yaml | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/iio/pressure/bmp085.yaml b/Documentation/devicetree/bindings/iio/pressure/bmp085.yaml
index 6fda887ee9d4..072313201edb 100644
--- a/Documentation/devicetree/bindings/iio/pressure/bmp085.yaml
+++ b/Documentation/devicetree/bindings/iio/pressure/bmp085.yaml
@@ -48,9 +48,14 @@ properties:
 
   interrupts:
     description:
-      interrupt mapping for IRQ (BMP085 only)
+      interrupt mapping for IRQ. Supported in BMP085, BMP3xx, BMP5xx
     maxItems: 1
 
+  drive-open-drain:
+    desription:
+      set if the interrupt pin should be configured as open drain.
+      If not set, defaults to push-pull configuration.
+
 required:
   - compatible
   - vddd-supply
-- 
2.25.1

Re: [PATCH v2 5/7] dt-bindings: iio: pressure: bmp085: Add interrupts for BMP3xx and BMP5xx devices

[Rob Herring (Arm)]

On Fri, 26 Jul 2024 01:10:37 +0200, Vasileios Amoiridis wrote:
> Add interrupt options for BMP3xx and BMP5xx devices as well.
> 
> Signed-off-by: Vasileios Amoiridis <vassilisamir@gmail.com>
> ---
>  Documentation/devicetree/bindings/iio/pressure/bmp085.yaml | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 

My bot found errors running 'make dt_binding_check' on your patch:

yamllint warnings/errors:

dtschema/dtc warnings/errors:
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/iio/pressure/bmp085.yaml: properties:drive-open-drain: 'anyOf' conditional failed, one must be fixed:
	'desription' is not one of ['$ref', 'additionalItems', 'additionalProperties', 'allOf', 'anyOf', 'const', 'contains', 'default', 'dependencies', 'dependentRequired', 'dependentSchemas', 'deprecated', 'description', 'else', 'enum', 'exclusiveMaximum', 'exclusiveMinimum', 'items', 'if', 'minItems', 'minimum', 'maxItems', 'maximum', 'multipleOf', 'not', 'oneOf', 'pattern', 'patternProperties', 'properties', 'required', 'then', 'typeSize', 'unevaluatedProperties', 'uniqueItems']
	'type' was expected
	from schema $id: http://devicetree.org/meta-schemas/keywords.yaml#

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20240725231039.614536-6-vassilisamir@gmail.com

The base for the series is generally the latest rc1. A different dependency
should be noted in *this* patch.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit after running the above command yourself. Note
that DT_SCHEMA_FILES can be set to your schema file to speed up checking
your schema. However, it must be unset to test all examples with your schema.

[PATCH v2 6/7] iio: pressure: bmp280: Add data ready trigger support

[Vasileios Amoiridis]

The BMP3xx and BMP5xx sensors have an interrupt pin which can be used as
a trigger for when there are data ready in the sensor for pick up.

This use case is used along with NORMAL_MODE in the sensor, which allows
the sensor to do consecutive measurements depending on the ODR rate value.

The trigger pin can be configured to be open-drain or push-pull and either
rising or falling edge.

No support is added yet for interrupts for FIFO, WATERMARK and out of range
values.

Signed-off-by: Vasileios Amoiridis <vassilisamir@gmail.com>
---
 drivers/iio/pressure/bmp280-core.c   | 309 ++++++++++++++++++++++++++-
 drivers/iio/pressure/bmp280-regmap.c |   2 +-
 drivers/iio/pressure/bmp280.h        |  23 +-
 3 files changed, 328 insertions(+), 6 deletions(-)

diff --git a/drivers/iio/pressure/bmp280-core.c b/drivers/iio/pressure/bmp280-core.c
index 4a8d2ed4a9c4..4238f37b7805 100644
--- a/drivers/iio/pressure/bmp280-core.c
+++ b/drivers/iio/pressure/bmp280-core.c
@@ -37,12 +37,14 @@
 #include <linux/module.h>
 #include <linux/nvmem-provider.h>
 #include <linux/pm_runtime.h>
+#include <linux/property.h>
 #include <linux/random.h>
 #include <linux/regmap.h>
 #include <linux/regulator/consumer.h>
 
 #include <linux/iio/buffer.h>
 #include <linux/iio/iio.h>
+#include <linux/iio/trigger.h>
 #include <linux/iio/trigger_consumer.h>
 #include <linux/iio/triggered_buffer.h>
 
@@ -1761,6 +1763,148 @@ static int bmp380_chip_config(struct bmp280_data *data)
 	return 0;
 }
 
+static void bmp380_trigger_reenable(struct iio_trigger *trig)
+{
+	struct bmp280_data *data = iio_trigger_get_drvdata(trig);
+	unsigned int tmp;
+	int ret;
+
+	ret = regmap_read(data->regmap, BMP380_REG_INT_STATUS, &tmp);
+	if (ret)
+		dev_err(data->dev, "Failed to reset interrupt\n");
+}
+
+static int bmp380_data_rdy_trigger_set_state(struct iio_trigger *trig,
+					     bool state)
+{
+	struct bmp280_data *data = iio_trigger_get_drvdata(trig);
+	int ret;
+
+	guard(mutex)(&data->lock);
+
+	ret = regmap_update_bits(data->regmap, BMP380_REG_INT_CTRL,
+				 BMP380_INT_CTRL_DRDY_EN,
+				 FIELD_PREP(BMP380_INT_CTRL_DRDY_EN,
+					    state ? 1 : 0));
+	if (ret) {
+		dev_err(data->dev, "Could not enable/disable interrupt\n");
+		return ret;
+	}
+
+	return 0;
+}
+
+static const struct iio_trigger_ops bmp380_trigger_ops = {
+	.set_trigger_state = &bmp380_data_rdy_trigger_set_state,
+	.reenable = &bmp380_trigger_reenable,
+};
+
+static int bmp380_int_config(struct bmp280_data *data)
+{
+	int ret, int_cfg = FIELD_PREP(BMP380_INT_CTRL_OPEN_DRAIN,
+				      data->trig_open_drain) |
+			   FIELD_PREP(BMP380_INT_CTRL_LEVEL,
+				      data->trig_active_high);
+
+	ret = regmap_update_bits(data->regmap, BMP380_REG_INT_CTRL,
+				 BMP380_INT_CTRL_SETTINGS_MASK, int_cfg);
+	if (ret) {
+		dev_err(data->dev, "Could not set interrupt settings\n");
+		return ret;
+	}
+
+	return 0;
+}
+
+static irqreturn_t bmp380_irq_thread_handler(int irq, void *p)
+{
+	struct iio_dev *indio_dev = p;
+	struct bmp280_data *data = iio_priv(indio_dev);
+	unsigned int int_ctrl;
+	int ret;
+
+	scoped_guard(mutex, &data->lock) {
+		ret = regmap_read(data->regmap, BMP380_REG_INT_STATUS, &int_ctrl);
+		if (ret)
+			return IRQ_NONE;
+	}
+
+	if (FIELD_GET(BMP380_INT_STATUS_DRDY, int_ctrl))
+		iio_trigger_poll_nested(data->trig);
+
+	return IRQ_HANDLED;
+}
+
+static int bmp380_trigger_probe(struct iio_dev *indio_dev)
+{
+	struct bmp280_data *data = iio_priv(indio_dev);
+	struct fwnode_handle *fwnode;
+	int ret, irq, irq_type;
+	struct irq_data *desc;
+
+	fwnode = dev_fwnode(data->dev);
+	if (!fwnode)
+		return -ENODEV;
+
+	irq = fwnode_irq_get(fwnode, 0);
+	if (!irq) {
+		dev_err(data->dev, "No interrupt found\n");
+		return -ENODEV;
+	}
+
+	desc = irq_get_irq_data(irq);
+	if (!desc)
+		return -EINVAL;
+
+	irq_type = irqd_get_trigger_type(desc);
+	switch (irq_type) {
+	case IRQF_TRIGGER_RISING:
+		data->trig_active_high = true;
+		break;
+	case IRQF_TRIGGER_FALLING:
+		data->trig_active_high = false;
+		break;
+	default:
+		dev_err(data->dev, "Invalid interrupt type specified\n");
+		return -EINVAL;
+	}
+
+	data->trig_open_drain = fwnode_property_read_bool(fwnode,
+							  "int-open-drain");
+
+	ret = bmp380_int_config(data);
+	if (ret)
+		return ret;
+
+	data->trig = devm_iio_trigger_alloc(data->dev, "%s-dev%d",
+					    indio_dev->name,
+					    iio_device_id(indio_dev));
+	if (!data->trig)
+		return -ENOMEM;
+
+	data->trig->ops = &bmp380_trigger_ops;
+	iio_trigger_set_drvdata(data->trig, data);
+
+	ret = devm_request_threaded_irq(data->dev, irq, NULL,
+					bmp380_irq_thread_handler, IRQF_ONESHOT,
+					indio_dev->name, indio_dev);
+	if (ret) {
+		dev_err(data->dev, "request irq failed\n");
+		return ret;
+	}
+
+	ret = devm_iio_trigger_register(data->dev, data->trig);
+	if (ret) {
+		dev_err(data->dev, "iio trigger register failed\n");
+		return ret;
+	}
+
+	indio_dev->trig = iio_trigger_get(data->trig);
+
+	return 0;
+}
+
+
 static irqreturn_t bmp380_trigger_handler(int irq, void *p)
 {
 	struct iio_poll_func *pf = p;
@@ -1854,6 +1998,7 @@ const struct bmp280_chip_info bmp380_chip_info = {
 	.wait_conv = bmp380_wait_conv,
 	.preinit = bmp380_preinit,
 
+	.trigger_probe = bmp380_trigger_probe,
 	.trigger_handler = bmp380_trigger_handler,
 };
 EXPORT_SYMBOL_NS(bmp380_chip_info, IIO_BMP280);
@@ -2390,6 +2535,154 @@ static int bmp580_chip_config(struct bmp280_data *data)
 	return 0;
 }
 
+static void bmp580_trigger_reenable(struct iio_trigger *trig)
+{
+	struct bmp280_data *data = iio_trigger_get_drvdata(trig);
+	unsigned int tmp;
+	int ret;
+
+	ret = regmap_read(data->regmap, BMP580_REG_INT_STATUS, &tmp);
+	if (ret)
+		dev_err(data->dev, "Failed to reset interrupt\n");
+}
+
+static int bmp580_data_rdy_trigger_set_state(struct iio_trigger *trig,
+					     bool state)
+{
+	struct bmp280_data *data = iio_trigger_get_drvdata(trig);
+	int ret;
+
+	guard(mutex)(&data->lock);
+
+	ret = regmap_update_bits(data->regmap, BMP580_REG_INT_CONFIG,
+				 BMP580_INT_CONFIG_INT_EN,
+				 FIELD_PREP(BMP580_INT_CONFIG_INT_EN,
+					    state ? 1 : 0));
+	if (ret) {
+		dev_err(data->dev, "Could not enable/disable interrupt\n");
+		return ret;
+	}
+
+	return 0;
+}
+
+static const struct iio_trigger_ops bmp580_trigger_ops = {
+	.set_trigger_state = &bmp580_data_rdy_trigger_set_state,
+	.reenable = &bmp580_trigger_reenable,
+};
+
+static int bmp580_int_config(struct bmp280_data *data)
+{
+	int ret, int_cfg = FIELD_PREP(BMP580_INT_CONFIG_OPEN_DRAIN,
+				      data->trig_open_drain) |
+			   FIELD_PREP(BMP580_INT_CONFIG_LEVEL,
+				      data->trig_active_high);
+
+	ret = regmap_update_bits(data->regmap, BMP580_REG_INT_CONFIG,
+				 BMP580_INT_CONFIG_MASK, int_cfg);
+	if (ret) {
+		dev_err(data->dev, "Could not set interrupt settings\n");
+		return ret;
+	}
+
+	ret = regmap_set_bits(data->regmap, BMP580_REG_INT_SOURCE,
+			      BMP580_INT_SOURCE_DRDY);
+	if (ret) {
+		dev_err(data->dev, "Could not set interrupt source\n");
+		return ret;
+	}
+
+	return 0;
+}
+
+static irqreturn_t bmp580_irq_thread_handler(int irq, void *p)
+{
+	struct iio_dev *indio_dev = p;
+	struct bmp280_data *data = iio_priv(indio_dev);
+	unsigned int int_ctrl;
+	int ret;
+
+	scoped_guard(mutex, &data->lock) {
+	ret = regmap_read(data->regmap, BMP580_REG_INT_STATUS, &int_ctrl);
+	if (ret)
+		return IRQ_NONE;
+	}
+
+	if (FIELD_GET(BMP580_INT_STATUS_DRDY_MASK, int_ctrl))
+		iio_trigger_poll_nested(data->trig);
+
+	return IRQ_HANDLED;
+}
+
+static int bmp580_trigger_probe(struct iio_dev *indio_dev)
+{
+	struct bmp280_data *data = iio_priv(indio_dev);
+	struct fwnode_handle *fwnode;
+	int ret, irq, irq_type;
+	struct irq_data *desc;
+
+	fwnode = dev_fwnode(data->dev);
+	if (!fwnode)
+		return -ENODEV;
+
+	irq = fwnode_irq_get(fwnode, 0);
+	if (!irq) {
+		dev_err(data->dev, "No interrupt found\n");
+		return -ENODEV;
+	}
+
+	desc = irq_get_irq_data(irq);
+	if (!desc)
+		return -EINVAL;
+
+	irq_type = irqd_get_trigger_type(desc);
+	switch (irq_type) {
+	case IRQF_TRIGGER_RISING:
+		data->trig_active_high = true;
+		break;
+	case IRQF_TRIGGER_FALLING:
+		data->trig_active_high = false;
+		break;
+	default:
+		dev_err(data->dev, "Invalid interrupt type specified\n");
+		return -EINVAL;
+	}
+
+	data->trig_open_drain = fwnode_property_read_bool(fwnode,
+							  "int-open-drain");
+
+	ret = bmp580_int_config(data);
+	if (ret)
+		return ret;
+
+	data->trig = devm_iio_trigger_alloc(data->dev, "%s-dev%d",
+					    indio_dev->name,
+					    iio_device_id(indio_dev));
+	if (!data->trig)
+		return -ENOMEM;
+
+	data->trig->ops = &bmp580_trigger_ops;
+	iio_trigger_set_drvdata(data->trig, data);
+
+	ret = devm_request_threaded_irq(data->dev, irq, NULL,
+					bmp580_irq_thread_handler, IRQF_ONESHOT,
+					indio_dev->name, indio_dev);
+	if (ret) {
+		dev_err(data->dev, "request irq failed\n");
+		return ret;
+	}
+
+	ret = devm_iio_trigger_register(data->dev, data->trig);
+	if (ret) {
+		dev_err(data->dev, "iio trigger register failed\n");
+		return ret;
+	}
+
+	indio_dev->trig = iio_trigger_get(data->trig);
+
+	return 0;
+}
+
 static irqreturn_t bmp580_trigger_handler(int irq, void *p)
 {
 	struct iio_poll_func *pf = p;
@@ -2466,6 +2759,7 @@ const struct bmp280_chip_info bmp580_chip_info = {
 	.wait_conv = bmp580_wait_conv,
 	.preinit = bmp580_preinit,
 
+	.trigger_probe = bmp580_trigger_probe,
 	.trigger_handler = bmp580_trigger_handler,
 };
 EXPORT_SYMBOL_NS(bmp580_chip_info, IIO_BMP280);
@@ -3013,10 +3307,17 @@ int bmp280_common_probe(struct device *dev,
 	 * however as it happens, the BMP085 shares the chip ID of BMP180
 	 * so we look for an IRQ if we have that.
 	 */
-	if (irq > 0 && (chip_id  == BMP180_CHIP_ID)) {
-		ret = bmp085_fetch_eoc_irq(dev, name, irq, data);
-		if (ret)
-			return ret;
+	if (irq > 0) {
+		if (chip_id == BMP180_CHIP_ID) {
+			ret = bmp085_fetch_eoc_irq(dev, name, irq, data);
+			if (ret)
+				return ret;
+		}
+		if (data->chip_info->trigger_probe) {
+			ret = data->chip_info->trigger_probe(indio_dev);
+			if (ret)
+				return ret;
+		}
 	}
 
 	ret = data->chip_info->set_mode(data, BMP280_SLEEP);
diff --git a/drivers/iio/pressure/bmp280-regmap.c b/drivers/iio/pressure/bmp280-regmap.c
index d27d68edd906..cccdf8fc6c09 100644
--- a/drivers/iio/pressure/bmp280-regmap.c
+++ b/drivers/iio/pressure/bmp280-regmap.c
@@ -109,7 +109,7 @@ static bool bmp380_is_writeable_reg(struct device *dev, unsigned int reg)
 	case BMP380_REG_FIFO_WATERMARK_LSB:
 	case BMP380_REG_FIFO_WATERMARK_MSB:
 	case BMP380_REG_POWER_CONTROL:
-	case BMP380_REG_INT_CONTROL:
+	case BMP380_REG_INT_CTRL:
 	case BMP380_REG_IF_CONFIG:
 	case BMP380_REG_ODR:
 	case BMP380_REG_OSR:
diff --git a/drivers/iio/pressure/bmp280.h b/drivers/iio/pressure/bmp280.h
index f5d192509d61..754eda367941 100644
--- a/drivers/iio/pressure/bmp280.h
+++ b/drivers/iio/pressure/bmp280.h
@@ -55,8 +55,17 @@
 #define BMP580_CMD_NVM_WRITE_SEQ_1	0xA0
 #define BMP580_CMD_SOFT_RESET		0xB6
 
+#define BMP580_INT_STATUS_DRDY_MASK	BIT(0)
 #define BMP580_INT_STATUS_POR_MASK	BIT(4)
 
+#define BMP580_INT_SOURCE_DRDY		BIT(0)
+
+#define BMP580_INT_CONFIG_MASK		GENMASK(3, 0)
+#define BMP580_INT_CONFIG_LATCH		BIT(0)
+#define BMP580_INT_CONFIG_LEVEL		BIT(1)
+#define BMP580_INT_CONFIG_OPEN_DRAIN	BIT(2)
+#define BMP580_INT_CONFIG_INT_EN	BIT(3)
+
 #define BMP580_STATUS_CORE_RDY_MASK	BIT(0)
 #define BMP580_STATUS_NVM_RDY_MASK	BIT(1)
 #define BMP580_STATUS_NVM_ERR_MASK	BIT(2)
@@ -117,7 +126,7 @@
 #define BMP380_REG_OSR			0x1C
 #define BMP380_REG_POWER_CONTROL	0x1B
 #define BMP380_REG_IF_CONFIG		0x1A
-#define BMP380_REG_INT_CONTROL		0x19
+#define BMP380_REG_INT_CTRL		0x19
 #define BMP380_REG_INT_STATUS		0x11
 #define BMP380_REG_EVENT		0x10
 #define BMP380_REG_STATUS		0x03
@@ -175,6 +184,14 @@
 #define BMP380_TEMP_MEAS_OFFSET		163
 #define BMP380_PRESS_MEAS_OFFSET	392
 
+#define BMP380_INT_STATUS_DRDY		BIT(3)
+
+#define BMP380_INT_CTRL_SETTINGS_MASK	GENMASK(2, 0)
+#define BMP380_INT_CTRL_OPEN_DRAIN	BIT(0)
+#define BMP380_INT_CTRL_LEVEL		BIT(1)
+#define BMP380_INT_CTRL_LATCH		BIT(2)
+#define BMP380_INT_CTRL_DRDY_EN		BIT(6)
+
 #define BMP380_MIN_TEMP			-4000
 #define BMP380_MAX_TEMP			8500
 #define BMP380_MIN_PRES			3000000
@@ -406,6 +423,9 @@ struct bmp280_data {
 	struct regmap *regmap;
 	struct completion done;
 	bool use_eoc;
+	bool trig_open_drain;
+	bool trig_active_high;
+	struct iio_trigger *trig;
 	const struct bmp280_chip_info *chip_info;
 	union {
 		struct bmp180_calib bmp180;
@@ -504,6 +524,7 @@ struct bmp280_chip_info {
 	int (*set_mode)(struct bmp280_data *data, enum bmp280_op_mode mode);
 	int (*wait_conv)(struct bmp280_data *data);
 
+	int (*trigger_probe)(struct iio_dev *indio_dev);
 	irqreturn_t (*trigger_handler)(int irq, void *p);
 };
 
-- 
2.25.1

Re: [PATCH v2 6/7] iio: pressure: bmp280: Add data ready trigger support

[Jonathan Cameron]

On Fri, 26 Jul 2024 01:10:38 +0200
Vasileios Amoiridis <vassilisamir@gmail.com> wrote:

> The BMP3xx and BMP5xx sensors have an interrupt pin which can be used as
> a trigger for when there are data ready in the sensor for pick up.
> 
> This use case is used along with NORMAL_MODE in the sensor, which allows
> the sensor to do consecutive measurements depending on the ODR rate value.
> 
> The trigger pin can be configured to be open-drain or push-pull and either
> rising or falling edge.
> 
> No support is added yet for interrupts for FIFO, WATERMARK and out of range
> values.
> 
> Signed-off-by: Vasileios Amoiridis <vassilisamir@gmail.com>
Hi Vasileios,

A few minor things inline, including a suggestion that perhaps the trigger_probe()
functions can be combined to reduce duplication. That would use a
__bmp280_trigger_probe(struct iio_dev *, struct iio_trigger_ops *,
                       + some function pointers).

Perhaps it's not worth it - I didn't try writing the actual code!

Jonathan

> ---
>  drivers/iio/pressure/bmp280-core.c   | 309 ++++++++++++++++++++++++++-
>  drivers/iio/pressure/bmp280-regmap.c |   2 +-
>  drivers/iio/pressure/bmp280.h        |  23 +-
>  3 files changed, 328 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/iio/pressure/bmp280-core.c b/drivers/iio/pressure/bmp280-core.c
> index 4a8d2ed4a9c4..4238f37b7805 100644
> --- a/drivers/iio/pressure/bmp280-core.c
> +++ b/drivers/iio/pressure/bmp280-core.c
> @@ -37,12 +37,14 @@



> +static irqreturn_t bmp380_irq_thread_handler(int irq, void *p)
> +{
> +	struct iio_dev *indio_dev = p;
> +	struct bmp280_data *data = iio_priv(indio_dev);
> +	unsigned int int_ctrl;
> +	int ret;
> +
> +	scoped_guard(mutex, &data->lock) {
> +		ret = regmap_read(data->regmap, BMP380_REG_INT_STATUS, &int_ctrl);
> +		if (ret)
> +			return IRQ_NONE;
> +	}
> +
> +	if (FIELD_GET(BMP380_INT_STATUS_DRDY, int_ctrl))
> +		iio_trigger_poll_nested(data->trig);
> +
> +	return IRQ_HANDLED;
> +}
> +
> +static int bmp380_trigger_probe(struct iio_dev *indio_dev)

Two of these functions are very similar.  Perhaps define a common
function that takes a function call for int config, the ops, and
interrupt handler as arguments then add device specific
calls that use that.



> +{
> +	struct bmp280_data *data = iio_priv(indio_dev);
> +	struct fwnode_handle *fwnode;
> +	int ret, irq, irq_type;
> +	struct irq_data *desc;
> +
> +	fwnode = dev_fwnode(data->dev);
> +	if (!fwnode)
> +		return -ENODEV;
> +
> +	irq = fwnode_irq_get(fwnode, 0);
> +	if (!irq) {
> +		dev_err(data->dev, "No interrupt found\n");
> +		return -ENODEV;
> +	}
> +
> +	desc = irq_get_irq_data(irq);
> +	if (!desc)
> +		return -EINVAL;
> +
> +	irq_type = irqd_get_trigger_type(desc);
> +	switch (irq_type) {
> +	case IRQF_TRIGGER_RISING:
> +		data->trig_active_high = true;
> +		break;
> +	case IRQF_TRIGGER_FALLING:
> +		data->trig_active_high = false;
> +		break;
> +	default:
> +		dev_err(data->dev, "Invalid interrupt type specified\n");
> +		return -EINVAL;
> +	}
> +
> +	data->trig_open_drain = fwnode_property_read_bool(fwnode,
> +							  "int-open-drain");
> +
> +	ret = bmp380_int_config(data);
> +	if (ret)
> +		return ret;
> +
> +	data->trig = devm_iio_trigger_alloc(data->dev, "%s-dev%d",
> +					    indio_dev->name,
> +					    iio_device_id(indio_dev));
> +	if (!data->trig)
> +		return -ENOMEM;
> +
> +	data->trig->ops = &bmp380_trigger_ops;
> +	iio_trigger_set_drvdata(data->trig, data);
> +
> +	ret = devm_request_threaded_irq(data->dev, irq, NULL,
> +					bmp380_irq_thread_handler, IRQF_ONESHOT,
> +					indio_dev->name, indio_dev);
> +	if (ret) {
> +		dev_err(data->dev, "request irq failed\n");
> +		return ret;
> +	}
> +
> +	ret = devm_iio_trigger_register(data->dev, data->trig);
> +	if (ret) {
> +		dev_err(data->dev, "iio trigger register failed\n");
> +		return ret;
> +	}
> +
> +	indio_dev->trig = iio_trigger_get(data->trig);
> +
> +	return 0;
> +}
> +
> +

one blank line only.

>  static irqreturn_t bmp380_trigger_handler(int irq, void *p)
>  {
>  	struct iio_poll_func *pf = p;
> @@ -1854,6 +1998,7 @@ const struct bmp280_chip_info bmp380_chip_info = {
>  	.wait_conv = bmp380_wait_conv,
>  	.preinit = bmp380_preinit,
>  
> +	.trigger_probe = bmp380_trigger_probe,
>  	.trigger_handler = bmp380_trigger_handler,
>  };
>  EXPORT_SYMBOL_NS(bmp380_chip_info, IIO_BMP280);
> @@ -2390,6 +2535,154 @@ static int bmp580_chip_config(struct bmp280_data *data)
>  	return 0;
>  }
>

...

> +static irqreturn_t bmp580_irq_thread_handler(int irq, void *p)
> +{
> +	struct iio_dev *indio_dev = p;
> +	struct bmp280_data *data = iio_priv(indio_dev);
> +	unsigned int int_ctrl;
> +	int ret;
> +
> +	scoped_guard(mutex, &data->lock) {

Indent wrong.

> +	ret = regmap_read(data->regmap, BMP580_REG_INT_STATUS, &int_ctrl);
> +	if (ret)
> +		return IRQ_NONE;
> +	}
> +
> +	if (FIELD_GET(BMP580_INT_STATUS_DRDY_MASK, int_ctrl))
> +		iio_trigger_poll_nested(data->trig);
> +
> +	return IRQ_HANDLED;
> +}
> +
> +static int bmp580_trigger_probe(struct iio_dev *indio_dev)
> +{
...

> +
> +	data->trig = devm_iio_trigger_alloc(data->dev, "%s-dev%d",
> +					    indio_dev->name,
> +					    iio_device_id(indio_dev));
> +	if (!data->trig)
> +		return -ENOMEM;
> +
> +	data->trig->ops = &bmp580_trigger_ops;
> +	iio_trigger_set_drvdata(data->trig, data);
> +
> +	ret = devm_request_threaded_irq(data->dev, irq, NULL,
> +					bmp580_irq_thread_handler, IRQF_ONESHOT,
> +					indio_dev->name, indio_dev);
> +	if (ret) {
> +		dev_err(data->dev, "request irq failed\n");

Only in probe paths I think, so return dev_err_probe() thoughout these
trigger setup callbacks.



> +}

> diff --git a/drivers/iio/pressure/bmp280-regmap.c b/drivers/iio/pressure/bmp280-regmap.c
> index d27d68edd906..cccdf8fc6c09 100644
> --- a/drivers/iio/pressure/bmp280-regmap.c
> +++ b/drivers/iio/pressure/bmp280-regmap.c
> @@ -109,7 +109,7 @@ static bool bmp380_is_writeable_reg(struct device *dev, unsigned int reg)
>  	case BMP380_REG_FIFO_WATERMARK_LSB:
>  	case BMP380_REG_FIFO_WATERMARK_MSB:
>  	case BMP380_REG_POWER_CONTROL:
> -	case BMP380_REG_INT_CONTROL:
> +	case BMP380_REG_INT_CTRL:

Unrelated change.  I'm also not sure it's worth making.

>  	case BMP380_REG_IF_CONFIG:
>  	case BMP380_REG_ODR:
>  	case BMP380_REG_OSR:
> diff --git a/drivers/iio/pressure/bmp280.h b/drivers/iio/pressure/bmp280.h
> index f5d192509d61..754eda367941 100644
> --- a/drivers/iio/pressure/bmp280.h
> +++ b/drivers/iio/pressure/bmp280.h
> @@ -55,8 +55,17 @@
>  #define BMP580_CMD_NVM_WRITE_SEQ_1	0xA0
>  #define BMP580_CMD_SOFT_RESET		0xB6
>  
> +#define BMP580_INT_STATUS_DRDY_MASK	BIT(0)
>  #define BMP580_INT_STATUS_POR_MASK	BIT(4)
>  
> +#define BMP580_INT_SOURCE_DRDY		BIT(0)
> +
> +#define BMP580_INT_CONFIG_MASK		GENMASK(3, 0)
> +#define BMP580_INT_CONFIG_LATCH		BIT(0)
> +#define BMP580_INT_CONFIG_LEVEL		BIT(1)
> +#define BMP580_INT_CONFIG_OPEN_DRAIN	BIT(2)
> +#define BMP580_INT_CONFIG_INT_EN	BIT(3)
> +
>  #define BMP580_STATUS_CORE_RDY_MASK	BIT(0)
>  #define BMP580_STATUS_NVM_RDY_MASK	BIT(1)
>  #define BMP580_STATUS_NVM_ERR_MASK	BIT(2)
> @@ -117,7 +126,7 @@
>  #define BMP380_REG_OSR			0x1C
>  #define BMP380_REG_POWER_CONTROL	0x1B
>  #define BMP380_REG_IF_CONFIG		0x1A
> -#define BMP380_REG_INT_CONTROL		0x19
> +#define BMP380_REG_INT_CTRL		0x19
As above.

Jonathan

Re: [PATCH v2 6/7] iio: pressure: bmp280: Add data ready trigger support

[Vasileios Amoiridis]

On Sun, Jul 28, 2024 at 05:06:50PM +0100, Jonathan Cameron wrote:
> On Fri, 26 Jul 2024 01:10:38 +0200
> Vasileios Amoiridis <vassilisamir@gmail.com> wrote:
> 
> > The BMP3xx and BMP5xx sensors have an interrupt pin which can be used as
> > a trigger for when there are data ready in the sensor for pick up.
> > 
> > This use case is used along with NORMAL_MODE in the sensor, which allows
> > the sensor to do consecutive measurements depending on the ODR rate value.
> > 
> > The trigger pin can be configured to be open-drain or push-pull and either
> > rising or falling edge.
> > 
> > No support is added yet for interrupts for FIFO, WATERMARK and out of range
> > values.
> > 
> > Signed-off-by: Vasileios Amoiridis <vassilisamir@gmail.com>
> Hi Vasileios,
> 
> A few minor things inline, including a suggestion that perhaps the trigger_probe()
> functions can be combined to reduce duplication. That would use a
> __bmp280_trigger_probe(struct iio_dev *, struct iio_trigger_ops *,
>                        + some function pointers).
> 
> Perhaps it's not worth it - I didn't try writing the actual code!
> 
> Jonathan
> 

Hi Jonathan,

Initially, in the v1 of the series, I had designed differently the handling of the
interrupts, and having split functions was making more sense in case you had
sensors with extra interrupts.

Since you explained to me how to do it properly, I think that now, what you are
proposing makes sense.

> > ---
> >  drivers/iio/pressure/bmp280-core.c   | 309 ++++++++++++++++++++++++++-
> >  drivers/iio/pressure/bmp280-regmap.c |   2 +-
> >  drivers/iio/pressure/bmp280.h        |  23 +-
> >  3 files changed, 328 insertions(+), 6 deletions(-)
> > 
> > diff --git a/drivers/iio/pressure/bmp280-core.c b/drivers/iio/pressure/bmp280-core.c
> > index 4a8d2ed4a9c4..4238f37b7805 100644
> > --- a/drivers/iio/pressure/bmp280-core.c
> > +++ b/drivers/iio/pressure/bmp280-core.c
> > @@ -37,12 +37,14 @@
> 
> 
> 
> > +static irqreturn_t bmp380_irq_thread_handler(int irq, void *p)
> > +{
> > +	struct iio_dev *indio_dev = p;
> > +	struct bmp280_data *data = iio_priv(indio_dev);
> > +	unsigned int int_ctrl;
> > +	int ret;
> > +
> > +	scoped_guard(mutex, &data->lock) {
> > +		ret = regmap_read(data->regmap, BMP380_REG_INT_STATUS, &int_ctrl);
> > +		if (ret)
> > +			return IRQ_NONE;
> > +	}
> > +
> > +	if (FIELD_GET(BMP380_INT_STATUS_DRDY, int_ctrl))
> > +		iio_trigger_poll_nested(data->trig);
> > +
> > +	return IRQ_HANDLED;
> > +}
> > +
> > +static int bmp380_trigger_probe(struct iio_dev *indio_dev)
> 
> Two of these functions are very similar.  Perhaps define a common
> function that takes a function call for int config, the ops, and
> interrupt handler as arguments then add device specific
> calls that use that.
> 

True, I will think if it could generate any issues in the future with newer
sensors adding more interrupts or with the current ones (bmp580 supports much
more interrupts than the bmp380) and I will act accordingly.

> 
> 
> > +{
> > +	struct bmp280_data *data = iio_priv(indio_dev);
> > +	struct fwnode_handle *fwnode;
> > +	int ret, irq, irq_type;
> > +	struct irq_data *desc;
> > +
> > +	fwnode = dev_fwnode(data->dev);
> > +	if (!fwnode)
> > +		return -ENODEV;
> > +
> > +	irq = fwnode_irq_get(fwnode, 0);
> > +	if (!irq) {
> > +		dev_err(data->dev, "No interrupt found\n");
> > +		return -ENODEV;
> > +	}
> > +
> > +	desc = irq_get_irq_data(irq);
> > +	if (!desc)
> > +		return -EINVAL;
> > +
> > +	irq_type = irqd_get_trigger_type(desc);
> > +	switch (irq_type) {
> > +	case IRQF_TRIGGER_RISING:
> > +		data->trig_active_high = true;
> > +		break;
> > +	case IRQF_TRIGGER_FALLING:
> > +		data->trig_active_high = false;
> > +		break;
> > +	default:
> > +		dev_err(data->dev, "Invalid interrupt type specified\n");
> > +		return -EINVAL;
> > +	}
> > +
> > +	data->trig_open_drain = fwnode_property_read_bool(fwnode,
> > +							  "int-open-drain");
> > +
> > +	ret = bmp380_int_config(data);
> > +	if (ret)
> > +		return ret;
> > +
> > +	data->trig = devm_iio_trigger_alloc(data->dev, "%s-dev%d",
> > +					    indio_dev->name,
> > +					    iio_device_id(indio_dev));
> > +	if (!data->trig)
> > +		return -ENOMEM;
> > +
> > +	data->trig->ops = &bmp380_trigger_ops;
> > +	iio_trigger_set_drvdata(data->trig, data);
> > +
> > +	ret = devm_request_threaded_irq(data->dev, irq, NULL,
> > +					bmp380_irq_thread_handler, IRQF_ONESHOT,
> > +					indio_dev->name, indio_dev);
> > +	if (ret) {
> > +		dev_err(data->dev, "request irq failed\n");
> > +		return ret;
> > +	}
> > +
> > +	ret = devm_iio_trigger_register(data->dev, data->trig);
> > +	if (ret) {
> > +		dev_err(data->dev, "iio trigger register failed\n");
> > +		return ret;
> > +	}
> > +
> > +	indio_dev->trig = iio_trigger_get(data->trig);
> > +
> > +	return 0;
> > +}
> > +
> > +
> 
> one blank line only.
> 

Ack.

> >  static irqreturn_t bmp380_trigger_handler(int irq, void *p)
> >  {
> >  	struct iio_poll_func *pf = p;
> > @@ -1854,6 +1998,7 @@ const struct bmp280_chip_info bmp380_chip_info = {
> >  	.wait_conv = bmp380_wait_conv,
> >  	.preinit = bmp380_preinit,
> >  
> > +	.trigger_probe = bmp380_trigger_probe,
> >  	.trigger_handler = bmp380_trigger_handler,
> >  };
> >  EXPORT_SYMBOL_NS(bmp380_chip_info, IIO_BMP280);
> > @@ -2390,6 +2535,154 @@ static int bmp580_chip_config(struct bmp280_data *data)
> >  	return 0;
> >  }
> >
> 
> ...
> 
> > +static irqreturn_t bmp580_irq_thread_handler(int irq, void *p)
> > +{
> > +	struct iio_dev *indio_dev = p;
> > +	struct bmp280_data *data = iio_priv(indio_dev);
> > +	unsigned int int_ctrl;
> > +	int ret;
> > +
> > +	scoped_guard(mutex, &data->lock) {
> 
> Indent wrong.
> 

Ack.

> > +	ret = regmap_read(data->regmap, BMP580_REG_INT_STATUS, &int_ctrl);
> > +	if (ret)
> > +		return IRQ_NONE;
> > +	}
> > +
> > +	if (FIELD_GET(BMP580_INT_STATUS_DRDY_MASK, int_ctrl))
> > +		iio_trigger_poll_nested(data->trig);
> > +
> > +	return IRQ_HANDLED;
> > +}
> > +
> > +static int bmp580_trigger_probe(struct iio_dev *indio_dev)
> > +{
> ...
> 
> > +
> > +	data->trig = devm_iio_trigger_alloc(data->dev, "%s-dev%d",
> > +					    indio_dev->name,
> > +					    iio_device_id(indio_dev));
> > +	if (!data->trig)
> > +		return -ENOMEM;
> > +
> > +	data->trig->ops = &bmp580_trigger_ops;
> > +	iio_trigger_set_drvdata(data->trig, data);
> > +
> > +	ret = devm_request_threaded_irq(data->dev, irq, NULL,
> > +					bmp580_irq_thread_handler, IRQF_ONESHOT,
> > +					indio_dev->name, indio_dev);
> > +	if (ret) {
> > +		dev_err(data->dev, "request irq failed\n");
> 
> Only in probe paths I think, so return dev_err_probe() thoughout these
> trigger setup callbacks.
> 

Ack.

> 
> 
> > +}
> 
> > diff --git a/drivers/iio/pressure/bmp280-regmap.c b/drivers/iio/pressure/bmp280-regmap.c
> > index d27d68edd906..cccdf8fc6c09 100644
> > --- a/drivers/iio/pressure/bmp280-regmap.c
> > +++ b/drivers/iio/pressure/bmp280-regmap.c
> > @@ -109,7 +109,7 @@ static bool bmp380_is_writeable_reg(struct device *dev, unsigned int reg)
> >  	case BMP380_REG_FIFO_WATERMARK_LSB:
> >  	case BMP380_REG_FIFO_WATERMARK_MSB:
> >  	case BMP380_REG_POWER_CONTROL:
> > -	case BMP380_REG_INT_CONTROL:
> > +	case BMP380_REG_INT_CTRL:
> 
> Unrelated change.  I'm also not sure it's worth making.
> 

I did it because this is tha name in the datasheet and it was also helping
with the 80 char limit. But I can leave it as it is, there is no problem.

> >  	case BMP380_REG_IF_CONFIG:
> >  	case BMP380_REG_ODR:
> >  	case BMP380_REG_OSR:
> > diff --git a/drivers/iio/pressure/bmp280.h b/drivers/iio/pressure/bmp280.h
> > index f5d192509d61..754eda367941 100644
> > --- a/drivers/iio/pressure/bmp280.h
> > +++ b/drivers/iio/pressure/bmp280.h
> > @@ -55,8 +55,17 @@
> >  #define BMP580_CMD_NVM_WRITE_SEQ_1	0xA0
> >  #define BMP580_CMD_SOFT_RESET		0xB6
> >  
> > +#define BMP580_INT_STATUS_DRDY_MASK	BIT(0)
> >  #define BMP580_INT_STATUS_POR_MASK	BIT(4)
> >  
> > +#define BMP580_INT_SOURCE_DRDY		BIT(0)
> > +
> > +#define BMP580_INT_CONFIG_MASK		GENMASK(3, 0)
> > +#define BMP580_INT_CONFIG_LATCH		BIT(0)
> > +#define BMP580_INT_CONFIG_LEVEL		BIT(1)
> > +#define BMP580_INT_CONFIG_OPEN_DRAIN	BIT(2)
> > +#define BMP580_INT_CONFIG_INT_EN	BIT(3)
> > +
> >  #define BMP580_STATUS_CORE_RDY_MASK	BIT(0)
> >  #define BMP580_STATUS_NVM_RDY_MASK	BIT(1)
> >  #define BMP580_STATUS_NVM_ERR_MASK	BIT(2)
> > @@ -117,7 +126,7 @@
> >  #define BMP380_REG_OSR			0x1C
> >  #define BMP380_REG_POWER_CONTROL	0x1B
> >  #define BMP380_REG_IF_CONFIG		0x1A
> > -#define BMP380_REG_INT_CONTROL		0x19
> > +#define BMP380_REG_INT_CTRL		0x19
> As above.
> 
> Jonathan
> 

Cheers,
Vasilis

[PATCH v2 7/7] iio: pressure bmp280: Move bmp085 interrupt to new configuration

[Vasileios Amoiridis]

This commit intends to add the old BMP085 sensor to the new IRQ interface
of the sensor consistence. No functional changes intended.

The BMP085 sensor is equivalent with the BMP180 with the only difference of
BMP085 having an extra interrupt pin to inform about an End of Conversion.

Signed-off-by: Vasileios Amoiridis <vassilisamir@gmail.com>
---
 drivers/iio/pressure/bmp280-core.c | 72 +++++++++++++++++++++++-------
 drivers/iio/pressure/bmp280-i2c.c  |  4 +-
 drivers/iio/pressure/bmp280-spi.c  |  4 +-
 drivers/iio/pressure/bmp280.h      |  1 +
 4 files changed, 60 insertions(+), 21 deletions(-)

diff --git a/drivers/iio/pressure/bmp280-core.c b/drivers/iio/pressure/bmp280-core.c
index 4238f37b7805..e4d017358b68 100644
--- a/drivers/iio/pressure/bmp280-core.c
+++ b/drivers/iio/pressure/bmp280-core.c
@@ -3104,13 +3104,19 @@ static irqreturn_t bmp085_eoc_irq(int irq, void *d)
 	return IRQ_HANDLED;
 }
 
-static int bmp085_fetch_eoc_irq(struct device *dev,
-				const char *name,
-				int irq,
-				struct bmp280_data *data)
+static int bmp085_trigger_probe(struct iio_dev *indio_dev)
 {
+	struct bmp280_data *data = iio_priv(indio_dev);
+	struct device *dev = data->dev;
+	struct fwnode_handle *fwnode;
 	unsigned long irq_trig;
-	int ret;
+	int ret, irq;
+
+	fwnode = dev_fwnode(data->dev);
+	if (!fwnode)
+		return -ENODEV;
+
+	irq = fwnode_irq_get(fwnode, 0);
 
 	irq_trig = irqd_get_trigger_type(irq_get_irq_data(irq));
 	if (irq_trig != IRQF_TRIGGER_RISING) {
@@ -3120,13 +3126,12 @@ static int bmp085_fetch_eoc_irq(struct device *dev,
 
 	init_completion(&data->done);
 
-	ret = devm_request_threaded_irq(dev,
-			irq,
-			bmp085_eoc_irq,
-			NULL,
-			irq_trig,
-			name,
-			data);
+	ret = devm_request_irq(dev,
+			       irq,
+			       bmp085_eoc_irq,
+			       irq_trig,
+			       indio_dev->name,
+			       data);
 	if (ret) {
 		/* Bail out without IRQ but keep the driver in place */
 		dev_err(dev, "unable to request DRDY IRQ\n");
@@ -3137,6 +3142,44 @@ static int bmp085_fetch_eoc_irq(struct device *dev,
 	return 0;
 }
 
+const struct bmp280_chip_info bmp085_chip_info = {
+	.id_reg = bmp180_chip_info.id_reg,
+	.chip_id = bmp180_chip_info.chip_id,
+	.num_chip_id = bmp180_chip_info.num_chip_id,
+	.regmap_config = bmp180_chip_info.regmap_config,
+	.start_up_time = bmp180_chip_info.start_up_time,
+	.channels = bmp180_chip_info.channels,
+	.num_channels = bmp180_chip_info.num_channels,
+	.avail_scan_masks = bmp180_chip_info.avail_scan_masks,
+
+	.oversampling_temp_avail = bmp180_chip_info.oversampling_temp_avail,
+	.num_oversampling_temp_avail =
+		bmp180_chip_info.num_oversampling_temp_avail,
+	.oversampling_temp_default = bmp180_chip_info.oversampling_temp_default,
+
+	.oversampling_press_avail = bmp180_chip_info.oversampling_press_avail,
+	.num_oversampling_press_avail =
+		bmp180_chip_info.num_oversampling_press_avail,
+	.oversampling_press_default =
+		bmp180_chip_info.oversampling_press_default,
+
+	.temp_coeffs = bmp180_chip_info.temp_coeffs,
+	.temp_coeffs_type = bmp180_chip_info.temp_coeffs_type,
+	.press_coeffs = bmp180_chip_info.press_coeffs,
+	.press_coeffs_type = bmp180_chip_info.press_coeffs_type,
+
+	.chip_config = bmp180_chip_info.chip_config,
+	.read_temp = bmp180_chip_info.read_temp,
+	.read_press = bmp180_chip_info.read_press,
+	.read_calib = bmp180_chip_info.read_calib,
+	.set_mode = bmp180_chip_info.set_mode,
+	.wait_conv = bmp180_chip_info.wait_conv,
+
+	.trigger_probe = bmp085_trigger_probe,
+	.trigger_handler = bmp180_trigger_handler,
+};
+EXPORT_SYMBOL_NS(bmp085_chip_info, IIO_BMP280);
+
 static int bmp280_buffer_preenable(struct iio_dev *indio_dev)
 {
 	struct bmp280_data *data = iio_priv(indio_dev);
@@ -3308,11 +3351,6 @@ int bmp280_common_probe(struct device *dev,
 	 * so we look for an IRQ if we have that.
 	 */
 	if (irq > 0) {
-		if (chip_id == BMP180_CHIP_ID) {
-			ret = bmp085_fetch_eoc_irq(dev, name, irq, data);
-			if (ret)
-				return ret;
-		}
 		if (data->chip_info->trigger_probe) {
 			ret = data->chip_info->trigger_probe(indio_dev);
 			if (ret)
diff --git a/drivers/iio/pressure/bmp280-i2c.c b/drivers/iio/pressure/bmp280-i2c.c
index 5c3a63b4327c..2f7b25984c7b 100644
--- a/drivers/iio/pressure/bmp280-i2c.c
+++ b/drivers/iio/pressure/bmp280-i2c.c
@@ -27,7 +27,7 @@ static int bmp280_i2c_probe(struct i2c_client *client)
 }
 
 static const struct of_device_id bmp280_of_i2c_match[] = {
-	{ .compatible = "bosch,bmp085", .data = &bmp180_chip_info },
+	{ .compatible = "bosch,bmp085", .data = &bmp085_chip_info },
 	{ .compatible = "bosch,bmp180", .data = &bmp180_chip_info },
 	{ .compatible = "bosch,bmp280", .data = &bmp280_chip_info },
 	{ .compatible = "bosch,bme280", .data = &bme280_chip_info },
@@ -38,7 +38,7 @@ static const struct of_device_id bmp280_of_i2c_match[] = {
 MODULE_DEVICE_TABLE(of, bmp280_of_i2c_match);
 
 static const struct i2c_device_id bmp280_i2c_id[] = {
-	{"bmp085", (kernel_ulong_t)&bmp180_chip_info },
+	{"bmp085", (kernel_ulong_t)&bmp085_chip_info },
 	{"bmp180", (kernel_ulong_t)&bmp180_chip_info },
 	{"bmp280", (kernel_ulong_t)&bmp280_chip_info },
 	{"bme280", (kernel_ulong_t)&bme280_chip_info },
diff --git a/drivers/iio/pressure/bmp280-spi.c b/drivers/iio/pressure/bmp280-spi.c
index d18549d9bb64..49aa8c2cd85b 100644
--- a/drivers/iio/pressure/bmp280-spi.c
+++ b/drivers/iio/pressure/bmp280-spi.c
@@ -114,7 +114,7 @@ static int bmp280_spi_probe(struct spi_device *spi)
 }
 
 static const struct of_device_id bmp280_of_spi_match[] = {
-	{ .compatible = "bosch,bmp085", .data = &bmp180_chip_info },
+	{ .compatible = "bosch,bmp085", .data = &bmp085_chip_info },
 	{ .compatible = "bosch,bmp180", .data = &bmp180_chip_info },
 	{ .compatible = "bosch,bmp181", .data = &bmp180_chip_info },
 	{ .compatible = "bosch,bmp280", .data = &bmp280_chip_info },
@@ -126,7 +126,7 @@ static const struct of_device_id bmp280_of_spi_match[] = {
 MODULE_DEVICE_TABLE(of, bmp280_of_spi_match);
 
 static const struct spi_device_id bmp280_spi_id[] = {
-	{ "bmp085", (kernel_ulong_t)&bmp180_chip_info },
+	{ "bmp085", (kernel_ulong_t)&bmp085_chip_info },
 	{ "bmp180", (kernel_ulong_t)&bmp180_chip_info },
 	{ "bmp181", (kernel_ulong_t)&bmp180_chip_info },
 	{ "bmp280", (kernel_ulong_t)&bmp280_chip_info },
diff --git a/drivers/iio/pressure/bmp280.h b/drivers/iio/pressure/bmp280.h
index 754eda367941..1307eda6f283 100644
--- a/drivers/iio/pressure/bmp280.h
+++ b/drivers/iio/pressure/bmp280.h
@@ -529,6 +529,7 @@ struct bmp280_chip_info {
 };
 
 /* Chip infos for each variant */
+extern const struct bmp280_chip_info bmp085_chip_info;
 extern const struct bmp280_chip_info bmp180_chip_info;
 extern const struct bmp280_chip_info bmp280_chip_info;
 extern const struct bmp280_chip_info bme280_chip_info;
-- 
2.25.1

Re: [PATCH v2 7/7] iio: pressure bmp280: Move bmp085 interrupt to new configuration

[kernel test robot]

Hi Vasileios,

kernel test robot noticed the following build errors:

[auto build test ERROR on 47ee461357f9da5a35d5f43527b7804a6a5744cb]

url:    https://github.com/intel-lab-lkp/linux/commits/Vasileios-Amoiridis/iio-pressure-bmp280-Use-bulk-read-for-humidity-calibration-data/20240726-071712
base:   47ee461357f9da5a35d5f43527b7804a6a5744cb
patch link:    https://lore.kernel.org/r/20240725231039.614536-8-vassilisamir%40gmail.com
patch subject: [PATCH v2 7/7] iio: pressure bmp280: Move bmp085 interrupt to new configuration
config: i386-buildonly-randconfig-005-20240726 (https://download.01.org/0day-ci/archive/20240726/202407261832.zSKjAoTj-lkp@intel.com/config)
compiler: gcc-7 (Ubuntu 7.5.0-6ubuntu2) 7.5.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240726/202407261832.zSKjAoTj-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202407261832.zSKjAoTj-lkp@intel.com/

All errors (new ones prefixed by >>):

>> drivers/iio/pressure/bmp280-core.c:3146:12: error: initializer element is not constant
     .id_reg = bmp180_chip_info.id_reg,
               ^~~~~~~~~~~~~~~~
   drivers/iio/pressure/bmp280-core.c:3146:12: note: (near initialization for 'bmp085_chip_info.id_reg')
   drivers/iio/pressure/bmp280-core.c:3147:13: error: initializer element is not constant
     .chip_id = bmp180_chip_info.chip_id,
                ^~~~~~~~~~~~~~~~
   drivers/iio/pressure/bmp280-core.c:3147:13: note: (near initialization for 'bmp085_chip_info.chip_id')
   drivers/iio/pressure/bmp280-core.c:3148:17: error: initializer element is not constant
     .num_chip_id = bmp180_chip_info.num_chip_id,
                    ^~~~~~~~~~~~~~~~
   drivers/iio/pressure/bmp280-core.c:3148:17: note: (near initialization for 'bmp085_chip_info.num_chip_id')
   drivers/iio/pressure/bmp280-core.c:3149:19: error: initializer element is not constant
     .regmap_config = bmp180_chip_info.regmap_config,
                      ^~~~~~~~~~~~~~~~
   drivers/iio/pressure/bmp280-core.c:3149:19: note: (near initialization for 'bmp085_chip_info.regmap_config')
   drivers/iio/pressure/bmp280-core.c:3150:19: error: initializer element is not constant
     .start_up_time = bmp180_chip_info.start_up_time,
                      ^~~~~~~~~~~~~~~~
   drivers/iio/pressure/bmp280-core.c:3150:19: note: (near initialization for 'bmp085_chip_info.start_up_time')
   drivers/iio/pressure/bmp280-core.c:3151:14: error: initializer element is not constant
     .channels = bmp180_chip_info.channels,
                 ^~~~~~~~~~~~~~~~
   drivers/iio/pressure/bmp280-core.c:3151:14: note: (near initialization for 'bmp085_chip_info.channels')
   drivers/iio/pressure/bmp280-core.c:3152:18: error: initializer element is not constant
     .num_channels = bmp180_chip_info.num_channels,
                     ^~~~~~~~~~~~~~~~
   drivers/iio/pressure/bmp280-core.c:3152:18: note: (near initialization for 'bmp085_chip_info.num_channels')
   drivers/iio/pressure/bmp280-core.c:3153:22: error: initializer element is not constant
     .avail_scan_masks = bmp180_chip_info.avail_scan_masks,
                         ^~~~~~~~~~~~~~~~
   drivers/iio/pressure/bmp280-core.c:3153:22: note: (near initialization for 'bmp085_chip_info.avail_scan_masks')
   drivers/iio/pressure/bmp280-core.c:3155:29: error: initializer element is not constant
     .oversampling_temp_avail = bmp180_chip_info.oversampling_temp_avail,
                                ^~~~~~~~~~~~~~~~
   drivers/iio/pressure/bmp280-core.c:3155:29: note: (near initialization for 'bmp085_chip_info.oversampling_temp_avail')
   drivers/iio/pressure/bmp280-core.c:3157:3: error: initializer element is not constant
      bmp180_chip_info.num_oversampling_temp_avail,
      ^~~~~~~~~~~~~~~~
   drivers/iio/pressure/bmp280-core.c:3157:3: note: (near initialization for 'bmp085_chip_info.num_oversampling_temp_avail')
   drivers/iio/pressure/bmp280-core.c:3158:31: error: initializer element is not constant
     .oversampling_temp_default = bmp180_chip_info.oversampling_temp_default,
                                  ^~~~~~~~~~~~~~~~
   drivers/iio/pressure/bmp280-core.c:3158:31: note: (near initialization for 'bmp085_chip_info.oversampling_temp_default')
   drivers/iio/pressure/bmp280-core.c:3160:30: error: initializer element is not constant
     .oversampling_press_avail = bmp180_chip_info.oversampling_press_avail,
                                 ^~~~~~~~~~~~~~~~
   drivers/iio/pressure/bmp280-core.c:3160:30: note: (near initialization for 'bmp085_chip_info.oversampling_press_avail')
   drivers/iio/pressure/bmp280-core.c:3162:3: error: initializer element is not constant
      bmp180_chip_info.num_oversampling_press_avail,
      ^~~~~~~~~~~~~~~~
   drivers/iio/pressure/bmp280-core.c:3162:3: note: (near initialization for 'bmp085_chip_info.num_oversampling_press_avail')
   drivers/iio/pressure/bmp280-core.c:3164:3: error: initializer element is not constant
      bmp180_chip_info.oversampling_press_default,
      ^~~~~~~~~~~~~~~~
   drivers/iio/pressure/bmp280-core.c:3164:3: note: (near initialization for 'bmp085_chip_info.oversampling_press_default')
   drivers/iio/pressure/bmp280-core.c:3166:17: error: initializer element is not constant
     .temp_coeffs = bmp180_chip_info.temp_coeffs,
                    ^~~~~~~~~~~~~~~~
   drivers/iio/pressure/bmp280-core.c:3166:17: note: (near initialization for 'bmp085_chip_info.temp_coeffs')
   drivers/iio/pressure/bmp280-core.c:3167:22: error: initializer element is not constant
     .temp_coeffs_type = bmp180_chip_info.temp_coeffs_type,
                         ^~~~~~~~~~~~~~~~
   drivers/iio/pressure/bmp280-core.c:3167:22: note: (near initialization for 'bmp085_chip_info.temp_coeffs_type')
   drivers/iio/pressure/bmp280-core.c:3168:18: error: initializer element is not constant
     .press_coeffs = bmp180_chip_info.press_coeffs,
                     ^~~~~~~~~~~~~~~~
   drivers/iio/pressure/bmp280-core.c:3168:18: note: (near initialization for 'bmp085_chip_info.press_coeffs')
   drivers/iio/pressure/bmp280-core.c:3169:23: error: initializer element is not constant
     .press_coeffs_type = bmp180_chip_info.press_coeffs_type,
                          ^~~~~~~~~~~~~~~~
   drivers/iio/pressure/bmp280-core.c:3169:23: note: (near initialization for 'bmp085_chip_info.press_coeffs_type')
   drivers/iio/pressure/bmp280-core.c:3171:17: error: initializer element is not constant
     .chip_config = bmp180_chip_info.chip_config,
                    ^~~~~~~~~~~~~~~~
   drivers/iio/pressure/bmp280-core.c:3171:17: note: (near initialization for 'bmp085_chip_info.chip_config')
   drivers/iio/pressure/bmp280-core.c:3172:15: error: initializer element is not constant
     .read_temp = bmp180_chip_info.read_temp,
                  ^~~~~~~~~~~~~~~~
   drivers/iio/pressure/bmp280-core.c:3172:15: note: (near initialization for 'bmp085_chip_info.read_temp')
   drivers/iio/pressure/bmp280-core.c:3173:16: error: initializer element is not constant
     .read_press = bmp180_chip_info.read_press,
                   ^~~~~~~~~~~~~~~~
   drivers/iio/pressure/bmp280-core.c:3173:16: note: (near initialization for 'bmp085_chip_info.read_press')
   drivers/iio/pressure/bmp280-core.c:3174:16: error: initializer element is not constant
     .read_calib = bmp180_chip_info.read_calib,
                   ^~~~~~~~~~~~~~~~
   drivers/iio/pressure/bmp280-core.c:3174:16: note: (near initialization for 'bmp085_chip_info.read_calib')
   drivers/iio/pressure/bmp280-core.c:3175:14: error: initializer element is not constant
     .set_mode = bmp180_chip_info.set_mode,
                 ^~~~~~~~~~~~~~~~
   drivers/iio/pressure/bmp280-core.c:3175:14: note: (near initialization for 'bmp085_chip_info.set_mode')
   drivers/iio/pressure/bmp280-core.c:3176:15: error: initializer element is not constant
     .wait_conv = bmp180_chip_info.wait_conv,
                  ^~~~~~~~~~~~~~~~
   drivers/iio/pressure/bmp280-core.c:3176:15: note: (near initialization for 'bmp085_chip_info.wait_conv')


vim +3146 drivers/iio/pressure/bmp280-core.c

  3144	
  3145	const struct bmp280_chip_info bmp085_chip_info = {
> 3146		.id_reg = bmp180_chip_info.id_reg,
  3147		.chip_id = bmp180_chip_info.chip_id,
  3148		.num_chip_id = bmp180_chip_info.num_chip_id,
  3149		.regmap_config = bmp180_chip_info.regmap_config,
  3150		.start_up_time = bmp180_chip_info.start_up_time,
  3151		.channels = bmp180_chip_info.channels,
  3152		.num_channels = bmp180_chip_info.num_channels,
  3153		.avail_scan_masks = bmp180_chip_info.avail_scan_masks,
  3154	
  3155		.oversampling_temp_avail = bmp180_chip_info.oversampling_temp_avail,
  3156		.num_oversampling_temp_avail =
  3157			bmp180_chip_info.num_oversampling_temp_avail,
  3158		.oversampling_temp_default = bmp180_chip_info.oversampling_temp_default,
  3159	
  3160		.oversampling_press_avail = bmp180_chip_info.oversampling_press_avail,
  3161		.num_oversampling_press_avail =
  3162			bmp180_chip_info.num_oversampling_press_avail,
  3163		.oversampling_press_default =
  3164			bmp180_chip_info.oversampling_press_default,
  3165	
  3166		.temp_coeffs = bmp180_chip_info.temp_coeffs,
  3167		.temp_coeffs_type = bmp180_chip_info.temp_coeffs_type,
  3168		.press_coeffs = bmp180_chip_info.press_coeffs,
  3169		.press_coeffs_type = bmp180_chip_info.press_coeffs_type,
  3170	
  3171		.chip_config = bmp180_chip_info.chip_config,
  3172		.read_temp = bmp180_chip_info.read_temp,
  3173		.read_press = bmp180_chip_info.read_press,
  3174		.read_calib = bmp180_chip_info.read_calib,
  3175		.set_mode = bmp180_chip_info.set_mode,
  3176		.wait_conv = bmp180_chip_info.wait_conv,
  3177	
  3178		.trigger_probe = bmp085_trigger_probe,
  3179		.trigger_handler = bmp180_trigger_handler,
  3180	};
  3181	EXPORT_SYMBOL_NS(bmp085_chip_info, IIO_BMP280);
  3182	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH v2 7/7] iio: pressure bmp280: Move bmp085 interrupt to new configuration

[kernel test robot]

Hi Vasileios,

kernel test robot noticed the following build errors:

[auto build test ERROR on 47ee461357f9da5a35d5f43527b7804a6a5744cb]

url:    https://github.com/intel-lab-lkp/linux/commits/Vasileios-Amoiridis/iio-pressure-bmp280-Use-bulk-read-for-humidity-calibration-data/20240726-071712
base:   47ee461357f9da5a35d5f43527b7804a6a5744cb
patch link:    https://lore.kernel.org/r/20240725231039.614536-8-vassilisamir%40gmail.com
patch subject: [PATCH v2 7/7] iio: pressure bmp280: Move bmp085 interrupt to new configuration
config: s390-randconfig-002-20240726 (https://download.01.org/0day-ci/archive/20240727/202407270859.izJnB7MZ-lkp@intel.com/config)
compiler: clang version 16.0.6 (https://github.com/llvm/llvm-project 7cbf1a2591520c2491aa35339f227775f4d3adf6)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240727/202407270859.izJnB7MZ-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202407270859.izJnB7MZ-lkp@intel.com/

All errors (new ones prefixed by >>):

   In file included from drivers/iio/pressure/bmp280-core.c:36:
   In file included from include/linux/irq.h:20:
   In file included from include/linux/io.h:14:
   In file included from arch/s390/include/asm/io.h:93:
   include/asm-generic/io.h:548:31: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           val = __raw_readb(PCI_IOBASE + addr);
                             ~~~~~~~~~~ ^
   include/asm-generic/io.h:561:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           val = __le16_to_cpu((__le16 __force)__raw_readw(PCI_IOBASE + addr));
                                                           ~~~~~~~~~~ ^
   include/uapi/linux/byteorder/big_endian.h:37:59: note: expanded from macro '__le16_to_cpu'
   #define __le16_to_cpu(x) __swab16((__force __u16)(__le16)(x))
                                                             ^
   include/uapi/linux/swab.h:102:54: note: expanded from macro '__swab16'
   #define __swab16(x) (__u16)__builtin_bswap16((__u16)(x))
                                                        ^
   In file included from drivers/iio/pressure/bmp280-core.c:36:
   In file included from include/linux/irq.h:20:
   In file included from include/linux/io.h:14:
   In file included from arch/s390/include/asm/io.h:93:
   include/asm-generic/io.h:574:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           val = __le32_to_cpu((__le32 __force)__raw_readl(PCI_IOBASE + addr));
                                                           ~~~~~~~~~~ ^
   include/uapi/linux/byteorder/big_endian.h:35:59: note: expanded from macro '__le32_to_cpu'
   #define __le32_to_cpu(x) __swab32((__force __u32)(__le32)(x))
                                                             ^
   include/uapi/linux/swab.h:115:54: note: expanded from macro '__swab32'
   #define __swab32(x) (__u32)__builtin_bswap32((__u32)(x))
                                                        ^
   In file included from drivers/iio/pressure/bmp280-core.c:36:
   In file included from include/linux/irq.h:20:
   In file included from include/linux/io.h:14:
   In file included from arch/s390/include/asm/io.h:93:
   include/asm-generic/io.h:585:33: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           __raw_writeb(value, PCI_IOBASE + addr);
                               ~~~~~~~~~~ ^
   include/asm-generic/io.h:595:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           __raw_writew((u16 __force)cpu_to_le16(value), PCI_IOBASE + addr);
                                                         ~~~~~~~~~~ ^
   include/asm-generic/io.h:605:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           __raw_writel((u32 __force)cpu_to_le32(value), PCI_IOBASE + addr);
                                                         ~~~~~~~~~~ ^
   include/asm-generic/io.h:693:20: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           readsb(PCI_IOBASE + addr, buffer, count);
                  ~~~~~~~~~~ ^
   include/asm-generic/io.h:701:20: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           readsw(PCI_IOBASE + addr, buffer, count);
                  ~~~~~~~~~~ ^
   include/asm-generic/io.h:709:20: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           readsl(PCI_IOBASE + addr, buffer, count);
                  ~~~~~~~~~~ ^
   include/asm-generic/io.h:718:21: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           writesb(PCI_IOBASE + addr, buffer, count);
                   ~~~~~~~~~~ ^
   include/asm-generic/io.h:727:21: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           writesw(PCI_IOBASE + addr, buffer, count);
                   ~~~~~~~~~~ ^
   include/asm-generic/io.h:736:21: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           writesl(PCI_IOBASE + addr, buffer, count);
                   ~~~~~~~~~~ ^
>> drivers/iio/pressure/bmp280-core.c:3146:29: error: initializer element is not a compile-time constant
           .id_reg = bmp180_chip_info.id_reg,
                     ~~~~~~~~~~~~~~~~~^~~~~~
   12 warnings and 1 error generated.


vim +3146 drivers/iio/pressure/bmp280-core.c

  3144	
  3145	const struct bmp280_chip_info bmp085_chip_info = {
> 3146		.id_reg = bmp180_chip_info.id_reg,
  3147		.chip_id = bmp180_chip_info.chip_id,
  3148		.num_chip_id = bmp180_chip_info.num_chip_id,
  3149		.regmap_config = bmp180_chip_info.regmap_config,
  3150		.start_up_time = bmp180_chip_info.start_up_time,
  3151		.channels = bmp180_chip_info.channels,
  3152		.num_channels = bmp180_chip_info.num_channels,
  3153		.avail_scan_masks = bmp180_chip_info.avail_scan_masks,
  3154	
  3155		.oversampling_temp_avail = bmp180_chip_info.oversampling_temp_avail,
  3156		.num_oversampling_temp_avail =
  3157			bmp180_chip_info.num_oversampling_temp_avail,
  3158		.oversampling_temp_default = bmp180_chip_info.oversampling_temp_default,
  3159	
  3160		.oversampling_press_avail = bmp180_chip_info.oversampling_press_avail,
  3161		.num_oversampling_press_avail =
  3162			bmp180_chip_info.num_oversampling_press_avail,
  3163		.oversampling_press_default =
  3164			bmp180_chip_info.oversampling_press_default,
  3165	
  3166		.temp_coeffs = bmp180_chip_info.temp_coeffs,
  3167		.temp_coeffs_type = bmp180_chip_info.temp_coeffs_type,
  3168		.press_coeffs = bmp180_chip_info.press_coeffs,
  3169		.press_coeffs_type = bmp180_chip_info.press_coeffs_type,
  3170	
  3171		.chip_config = bmp180_chip_info.chip_config,
  3172		.read_temp = bmp180_chip_info.read_temp,
  3173		.read_press = bmp180_chip_info.read_press,
  3174		.read_calib = bmp180_chip_info.read_calib,
  3175		.set_mode = bmp180_chip_info.set_mode,
  3176		.wait_conv = bmp180_chip_info.wait_conv,
  3177	
  3178		.trigger_probe = bmp085_trigger_probe,
  3179		.trigger_handler = bmp180_trigger_handler,
  3180	};
  3181	EXPORT_SYMBOL_NS(bmp085_chip_info, IIO_BMP280);
  3182	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH v2 7/7] iio: pressure bmp280: Move bmp085 interrupt to new configuration

[Jonathan Cameron]

On Fri, 26 Jul 2024 01:10:39 +0200
Vasileios Amoiridis <vassilisamir@gmail.com> wrote:

> This commit intends to add the old BMP085 sensor to the new IRQ interface
> of the sensor consistence. No functional changes intended.
> 
> The BMP085 sensor is equivalent with the BMP180 with the only difference of
> BMP085 having an extra interrupt pin to inform about an End of Conversion.
> 
> Signed-off-by: Vasileios Amoiridis <vassilisamir@gmail.com>
Trivial comments inline + as the build bot pointed out you can't use data from
one array to fill the other.

Jonathan

> ---
>  drivers/iio/pressure/bmp280-core.c | 72 +++++++++++++++++++++++-------
>  drivers/iio/pressure/bmp280-i2c.c  |  4 +-
>  drivers/iio/pressure/bmp280-spi.c  |  4 +-
>  drivers/iio/pressure/bmp280.h      |  1 +
>  4 files changed, 60 insertions(+), 21 deletions(-)
> 
> diff --git a/drivers/iio/pressure/bmp280-core.c b/drivers/iio/pressure/bmp280-core.c
> index 4238f37b7805..e4d017358b68 100644
> --- a/drivers/iio/pressure/bmp280-core.c
> +++ b/drivers/iio/pressure/bmp280-core.c
> @@ -3104,13 +3104,19 @@ static irqreturn_t bmp085_eoc_irq(int irq, void *d)
>  	return IRQ_HANDLED;
>  }
>  
> -static int bmp085_fetch_eoc_irq(struct device *dev,
> -				const char *name,
> -				int irq,
> -				struct bmp280_data *data)
> +static int bmp085_trigger_probe(struct iio_dev *indio_dev)
>  {
> +	struct bmp280_data *data = iio_priv(indio_dev);
> +	struct device *dev = data->dev;
> +	struct fwnode_handle *fwnode;
>  	unsigned long irq_trig;
> -	int ret;
> +	int ret, irq;
> +
> +	fwnode = dev_fwnode(data->dev);
> +	if (!fwnode)
> +		return -ENODEV;
> +
> +	irq = fwnode_irq_get(fwnode, 0);
>  
>  	irq_trig = irqd_get_trigger_type(irq_get_irq_data(irq));
>  	if (irq_trig != IRQF_TRIGGER_RISING) {
> @@ -3120,13 +3126,12 @@ static int bmp085_fetch_eoc_irq(struct device *dev,
>  
>  	init_completion(&data->done);
>  
> -	ret = devm_request_threaded_irq(dev,
> -			irq,
> -			bmp085_eoc_irq,
> -			NULL,
> -			irq_trig,
> -			name,
> -			data);
> +	ret = devm_request_irq(dev,
> +			       irq,
> +			       bmp085_eoc_irq,
> +			       irq_trig,
> +			       indio_dev->name,
> +			       data);
Whilst here, put some of those parameters on the same line (staying below
80 chars).

>  	if (ret) {
>  		/* Bail out without IRQ but keep the driver in place */
>  		dev_err(dev, "unable to request DRDY IRQ\n");
> @@ -3137,6 +3142,44 @@ static int bmp085_fetch_eoc_irq(struct device *dev,
>  	return 0;
>  }
>  
> +const struct bmp280_chip_info bmp085_chip_info = {
> +	.id_reg = bmp180_chip_info.id_reg,
As the build bot has pointed out you can't do this.
Annoying but just duplicate the original structure with whatever
changes you need.

Re: [PATCH v2 7/7] iio: pressure bmp280: Move bmp085 interrupt to new configuration

[Vasileios Amoiridis]

On Sun, Jul 28, 2024 at 05:09:15PM +0100, Jonathan Cameron wrote:
> On Fri, 26 Jul 2024 01:10:39 +0200
> Vasileios Amoiridis <vassilisamir@gmail.com> wrote:
> 
> > This commit intends to add the old BMP085 sensor to the new IRQ interface
> > of the sensor consistence. No functional changes intended.
> > 
> > The BMP085 sensor is equivalent with the BMP180 with the only difference of
> > BMP085 having an extra interrupt pin to inform about an End of Conversion.
> > 
> > Signed-off-by: Vasileios Amoiridis <vassilisamir@gmail.com>
> Trivial comments inline + as the build bot pointed out you can't use data from
> one array to fill the other.
> 
> Jonathan
> 
> > ---
> >  drivers/iio/pressure/bmp280-core.c | 72 +++++++++++++++++++++++-------
> >  drivers/iio/pressure/bmp280-i2c.c  |  4 +-
> >  drivers/iio/pressure/bmp280-spi.c  |  4 +-
> >  drivers/iio/pressure/bmp280.h      |  1 +
> >  4 files changed, 60 insertions(+), 21 deletions(-)
> > 
> > diff --git a/drivers/iio/pressure/bmp280-core.c b/drivers/iio/pressure/bmp280-core.c
> > index 4238f37b7805..e4d017358b68 100644
> > --- a/drivers/iio/pressure/bmp280-core.c
> > +++ b/drivers/iio/pressure/bmp280-core.c
> > @@ -3104,13 +3104,19 @@ static irqreturn_t bmp085_eoc_irq(int irq, void *d)
> >  	return IRQ_HANDLED;
> >  }
> >  
> > -static int bmp085_fetch_eoc_irq(struct device *dev,
> > -				const char *name,
> > -				int irq,
> > -				struct bmp280_data *data)
> > +static int bmp085_trigger_probe(struct iio_dev *indio_dev)
> >  {
> > +	struct bmp280_data *data = iio_priv(indio_dev);
> > +	struct device *dev = data->dev;
> > +	struct fwnode_handle *fwnode;
> >  	unsigned long irq_trig;
> > -	int ret;
> > +	int ret, irq;
> > +
> > +	fwnode = dev_fwnode(data->dev);
> > +	if (!fwnode)
> > +		return -ENODEV;
> > +
> > +	irq = fwnode_irq_get(fwnode, 0);
> >  
> >  	irq_trig = irqd_get_trigger_type(irq_get_irq_data(irq));
> >  	if (irq_trig != IRQF_TRIGGER_RISING) {
> > @@ -3120,13 +3126,12 @@ static int bmp085_fetch_eoc_irq(struct device *dev,
> >  
> >  	init_completion(&data->done);
> >  
> > -	ret = devm_request_threaded_irq(dev,
> > -			irq,
> > -			bmp085_eoc_irq,
> > -			NULL,
> > -			irq_trig,
> > -			name,
> > -			data);
> > +	ret = devm_request_irq(dev,
> > +			       irq,
> > +			       bmp085_eoc_irq,
> > +			       irq_trig,
> > +			       indio_dev->name,
> > +			       data);
> Whilst here, put some of those parameters on the same line (staying below
> 80 chars).
> 

Ack. I was aiming for as less intrusive change as possible.

> >  	if (ret) {
> >  		/* Bail out without IRQ but keep the driver in place */
> >  		dev_err(dev, "unable to request DRDY IRQ\n");
> > @@ -3137,6 +3142,44 @@ static int bmp085_fetch_eoc_irq(struct device *dev,
> >  	return 0;
> >  }
> >  
> > +const struct bmp280_chip_info bmp085_chip_info = {
> > +	.id_reg = bmp180_chip_info.id_reg,
> As the build bot has pointed out you can't do this.
> Annoying but just duplicate the original structure with whatever
> changes you need.
> 

Extremely annoying because it is litteraly just one addition in the
new array and everything else stays the same...

Cheers,
Vasilis