The DPS310 has a 32-sample FIFO that buffers pressure and temperature
readings. The FIFO_EN bit was defined but never used.

This enables the FIFO in the probe after startup. Add the necessary
register defines and init/flush functions. If FIFO init fails, driver
falls back to single-shot mode.

Signed-off-by: Rupesh Majhi <zoone.rupert@gmail.com>
---
 drivers/iio/pressure/dps310.c | 73 +++++++++++++++++++++++++++++++++++
 1 file changed, 73 insertions(+)

diff --git a/drivers/iio/pressure/dps310.c b/drivers/iio/pressure/dps310.c
index f45af72a0554..83f1326f542a 100644
--- a/drivers/iio/pressure/dps310.c
+++ b/drivers/iio/pressure/dps310.c
@@ -53,6 +53,19 @@
 #define  DPS310_PRS_SHIFT_EN	BIT(4)
 #define  DPS310_FIFO_EN		BIT(5)
 #define  DPS310_SPI_EN		BIT(6)
+/* FIFO status register */
+#define DPS310_FIFO_STS		0x0B
+#define DPS310_FIFO_EMPTY	BIT(0)
+#define DPS310_FIFO_FULL	BIT(1)
+
+/* FIFO samples are read from DPS310_PRS_BASE (0x00). The lower 2 bits
+ * of the third byte encode the sample type; the remaining bits are
+ * the signed measurement value.
+ */
+#define DPS310_FIFO_TYPE_MASK	GENMASK(0, 0)
+#define DPS310_FIFO_TMP_SAMPLE	0x00
+#define DPS310_FIFO_PRS_SAMPLE	0x01
+
 #define DPS310_RESET		0x0c
 #define  DPS310_RESET_MAGIC	0x09
 #define DPS310_COEF_BASE	0x10
@@ -90,6 +103,7 @@ struct dps310_data {
 	s32 pressure_raw;
 	s32 temp_raw;
 	bool timeout_recovery_failed;
+	bool fifo_enabled;
 };
 
 static const struct iio_chan_spec dps310_channels[] = {
@@ -843,6 +857,61 @@ static const struct iio_info dps310_info = {
 	.write_raw = dps310_write_raw,
 };
 
+static int dps310_fifo_flush(struct dps310_data *data)
+{
+	int rc;
+
+	rc = regmap_write_bits(data->regmap, DPS310_CFG_REG,
+			DPS310_FIFO_EN, 0);
+	if (rc)
+		return rc;
+
+	data->fifo_enabled = false;
+	return 0;
+}
+
+static int __maybe_unused dps310_fifo_read_sample(struct dps310_data *data)
+{
+	int rc;
+	u8 val[3];
+	s32 raw;
+	u8 type;
+
+	rc = regmap_bulk_read(data->regmap, DPS310_PRS_BASE,
+			val, sizeof(val));
+	if (rc)
+		return rc;
+
+	type = val[2] & DPS310_FIFO_TYPE_MASK;
+	raw = (val[0] << 16) | (val[1] << 8) | (val[2] & ~DPS310_FIFO_TYPE_MASK);
+	raw = sign_extend32(raw, 23);
+
+	if (type == DPS310_FIFO_TMP_SAMPLE)
+		data->temp_raw = raw;
+	else
+		data->pressure_raw = raw;
+
+	return 0;
+}
+
+static int dps310_fifo_init(struct dps310_data *data)
+{
+	int rc;
+
+	rc = dps310_fifo_flush(data);
+	if (rc)
+		return rc;
+
+	rc = regmap_write_bits(data->regmap, DPS310_CFG_REG,
+			DPS310_FIFO_EN, DPS310_FIFO_EN);
+
+	if (rc)
+		return rc;
+
+	data->fifo_enabled = true;
+	return 0;
+}
+
 static int dps310_probe(struct i2c_client *client)
 {
 	const struct i2c_device_id *id = i2c_client_get_device_id(client);
@@ -877,6 +946,10 @@ static int dps310_probe(struct i2c_client *client)
 	if (rc)
 		return rc;
 
+	rc = dps310_fifo_init(data);
+	if (rc)
+		dev_warn(&client->dev,
+			"FIFO init failed (%d), continuing without FIFO\n", rc);
 	rc = devm_iio_device_register(&client->dev, iio);
 	if (rc)
 		return rc;
-- 
2.43.0

On Sun, 19 Jul 2026 00:17:26 +0300
Rupesh Majhi <zoone.rupert@gmail.com> wrote:

> The DPS310 has a 32-sample FIFO that buffers pressure and temperature
> readings. The FIFO_EN bit was defined but never used.
> 
> This enables the FIFO in the probe after startup. Add the necessary
> register defines and init/flush functions. If FIFO init fails, driver
> falls back to single-shot mode.
> 
> Signed-off-by: Rupesh Majhi <zoone.rupert@gmail.com>
Hi Rupesh,

This looks to be a v2. So check Documentation/process/submitting-patches.rst
for the format and use that if you need a v3.

> ---
Also change log needed here.

I thought you were going to submit v2 with the full buffered support?
Perhaps this was an accidental resend of the earlier code.

Jonathan

Add triggered buffer support so pressure and temperature can be captured
into a buffer instead of only through one-shot sysfs reads.

Pressure is a processed value in kPa computed from a non-linear
calibration polynomial. To keep full resolution in the buffer without
disagreeing with the sysfs unit, add raw and scale attributes for
pressure (raw in Pa, scale 1/1000 to kPa), following bme680; the existing
processed attribute is kept for ABI compatibility. Temperature is already
a full-resolution value in its base unit (millidegrees Celsius) and stays
a processed channel.

Pressure compensation depends on a temperature reading, so both channels
are always captured together. The device already runs in continuous
background mode, so no buffer setup ops are needed. Sysfs reads and
reconfiguration return -EBUSY while the buffer is enabled, as they share
the capture path's raw values and configuration.

Signed-off-by: Rupesh Majhi <zoone.rupert@gmail.com>
---
Changes in v3:
 - Rework from just enabling the FIFO into proper IIO triggered buffer
   support, as suggested by Jonathan.
 - Buffer the pressure and temperature values; add raw + scale for
   pressure (bme680-style) so buffered pressure keeps full resolution
   and stays consistent with the sysfs unit.
 - Drop the unused FIFO enable/flush/read helpers.

v2 was a resend that still contained the v1 FIFO-enable code.

Note: this touches dps310_probe() near the separately-sent fix "iio:
pressure: dps310: fix NULL pointer deref on ACPI probe". It was generated
on a plain base tree, so applying it after that fix needs a trivial
3-way merge/rebase - happy to resend in whatever order or branch you
prefer.

 drivers/iio/pressure/Kconfig  |   2 +
 drivers/iio/pressure/dps310.c | 141 ++++++++++++++++++++++++++++++++--
 2 files changed, 136 insertions(+), 7 deletions(-)

diff --git a/drivers/iio/pressure/Kconfig b/drivers/iio/pressure/Kconfig
index 838a8340c4c0..cef8b90b9ae7 100644
--- a/drivers/iio/pressure/Kconfig
+++ b/drivers/iio/pressure/Kconfig
@@ -112,6 +112,8 @@ config DPS310
 	tristate "Infineon DPS310 pressure and temperature sensor"
 	depends on I2C
 	select REGMAP_I2C
+	select IIO_BUFFER
+	select IIO_TRIGGERED_BUFFER
 	help
 	  Support for the Infineon DPS310 digital barometric pressure sensor.
 	  It can be accessed over I2C bus.
diff --git a/drivers/iio/pressure/dps310.c b/drivers/iio/pressure/dps310.c
index f45af72a0554..967a2043550b 100644
--- a/drivers/iio/pressure/dps310.c
+++ b/drivers/iio/pressure/dps310.c
@@ -20,8 +20,11 @@
 #include <linux/module.h>
 #include <linux/regmap.h>
 
+#include <linux/iio/buffer.h>
 #include <linux/iio/iio.h>
 #include <linux/iio/sysfs.h>
+#include <linux/iio/trigger_consumer.h>
+#include <linux/iio/triggered_buffer.h>
 
 #define DPS310_DEV_NAME		"dps310"
 
@@ -90,6 +93,12 @@ struct dps310_data {
 	s32 pressure_raw;
 	s32 temp_raw;
 	bool timeout_recovery_failed;
+
+	/* Buffer to hold a scan; timestamp is naturally aligned */
+	struct {
+		s32 chan[2];
+		aligned_s64 timestamp;
+	} scan __aligned(8);
 };
 
 static const struct iio_chan_spec dps310_channels[] = {
@@ -98,15 +107,38 @@ static const struct iio_chan_spec dps310_channels[] = {
 		.info_mask_separate = BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO) |
 			BIT(IIO_CHAN_INFO_SAMP_FREQ) |
 			BIT(IIO_CHAN_INFO_PROCESSED),
+		.scan_index = 0,
+		.scan_type = {
+			.sign = 's',
+			.realbits = 32,
+			.storagebits = 32,
+			.endianness = IIO_CPU,
+		},
 	},
 	{
 		.type = IIO_PRESSURE,
 		.info_mask_separate = BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO) |
 			BIT(IIO_CHAN_INFO_SAMP_FREQ) |
-			BIT(IIO_CHAN_INFO_PROCESSED),
+			BIT(IIO_CHAN_INFO_PROCESSED) |
+			BIT(IIO_CHAN_INFO_RAW) |
+			BIT(IIO_CHAN_INFO_SCALE),
+		.scan_index = 1,
+		.scan_type = {
+			.sign = 's',
+			.realbits = 32,
+			.storagebits = 32,
+			.endianness = IIO_CPU,
+		},
 	},
+	IIO_CHAN_SOFT_TIMESTAMP(2),
 };
 
+/*
+ * Pressure compensation needs a temperature reading, so the trigger
+ * handler always captures both channels; keep them enabled together.
+ */
+static const unsigned long dps310_scan_masks[] = { GENMASK(1, 0), 0 };
+
 /* To be called after checking the COEF_RDY bit in MEAS_CFG */
 static int dps310_get_coefs(struct dps310_data *data)
 {
@@ -583,8 +615,14 @@ static int dps310_write_raw(struct iio_dev *iio,
 	int rc;
 	struct dps310_data *data = iio_priv(iio);
 
-	if (mutex_lock_interruptible(&data->lock))
+	/* Don't reconfigure the sensor while a buffered capture is running */
+	if (!iio_device_claim_direct(iio))
+		return -EBUSY;
+
+	if (mutex_lock_interruptible(&data->lock)) {
+		iio_device_release_direct(iio);
 		return -EINTR;
+	}
 
 	switch (mask) {
 	case IIO_CHAN_INFO_SAMP_FREQ:
@@ -625,6 +663,7 @@ static int dps310_write_raw(struct iio_dev *iio,
 	}
 
 	mutex_unlock(&data->lock);
+	iio_device_release_direct(iio);
 	return rc;
 }
 
@@ -735,6 +774,23 @@ static int dps310_read_pressure(struct dps310_data *data, int *val, int *val2,
 		*val2 = 1000; /* Convert Pa to KPa per IIO ABI */
 		return IIO_VAL_FRACTIONAL;
 
+	case IIO_CHAN_INFO_RAW:
+		rc = dps310_read_pres_raw(data);
+		if (rc)
+			return rc;
+
+		rc = dps310_calculate_pressure(data, val);
+		if (rc)
+			return rc;
+
+		return IIO_VAL_INT;
+
+	case IIO_CHAN_INFO_SCALE:
+		/* Raw pressure is in Pa; scale to kPa per IIO ABI */
+		*val = 1;
+		*val2 = 1000;
+		return IIO_VAL_FRACTIONAL;
+
 	case IIO_CHAN_INFO_OVERSAMPLING_RATIO:
 		rc = dps310_get_pres_precision(data, val);
 		if (rc)
@@ -804,12 +860,10 @@ static int dps310_read_temp(struct dps310_data *data, int *val, int *val2,
 	}
 }
 
-static int dps310_read_raw(struct iio_dev *iio,
-			   struct iio_chan_spec const *chan,
-			   int *val, int *val2, long mask)
+static int dps310_read_channel(struct dps310_data *data,
+			       struct iio_chan_spec const *chan,
+			       int *val, int *val2, long mask)
 {
-	struct dps310_data *data = iio_priv(iio);
-
 	switch (chan->type) {
 	case IIO_PRESSURE:
 		return dps310_read_pressure(data, val, val2, mask);
@@ -822,6 +876,32 @@ static int dps310_read_raw(struct iio_dev *iio,
 	}
 }
 
+static int dps310_read_raw(struct iio_dev *iio,
+			   struct iio_chan_spec const *chan,
+			   int *val, int *val2, long mask)
+{
+	struct dps310_data *data = iio_priv(iio);
+	int rc;
+
+	switch (mask) {
+	case IIO_CHAN_INFO_RAW:
+	case IIO_CHAN_INFO_PROCESSED:
+		/*
+		 * Reading a sample uses the same raw values as the buffered
+		 * capture path, so only allow it outside of buffered mode.
+		 */
+		if (!iio_device_claim_direct(iio))
+			return -EBUSY;
+
+		rc = dps310_read_channel(data, chan, val, val2, mask);
+		iio_device_release_direct(iio);
+		return rc;
+
+	default:
+		return dps310_read_channel(data, chan, val, val2, mask);
+	}
+}
+
 static void dps310_reset(void *action_data)
 {
 	struct dps310_data *data = action_data;
@@ -843,6 +923,46 @@ static const struct iio_info dps310_info = {
 	.write_raw = dps310_write_raw,
 };
 
+static irqreturn_t dps310_trigger_handler(int irq, void *p)
+{
+	struct iio_poll_func *pf = p;
+	struct iio_dev *iio = pf->indio_dev;
+	struct dps310_data *data = iio_priv(iio);
+	int rc, pressure, temp;
+
+	/*
+	 * Don't hold data->lock across these calls: the read helpers take it
+	 * themselves and the mutex is not recursive (dps310_calculate_pressure
+	 * also grabs it with mutex_trylock to refresh the temperature).
+	 */
+	rc = dps310_read_pres_raw(data);
+	if (rc)
+		goto out;
+
+	rc = dps310_read_temp_raw(data);
+	if (rc)
+		goto out;
+
+	rc = dps310_calculate_pressure(data, &pressure);
+	if (rc)
+		goto out;
+
+	rc = dps310_calculate_temp(data, &temp);
+	if (rc)
+		goto out;
+
+	data->scan.chan[0] = temp;	/* millidegrees Celsius */
+	data->scan.chan[1] = pressure;	/* Pascals */
+
+	iio_push_to_buffers_with_ts(iio, &data->scan, sizeof(data->scan),
+				    pf->timestamp);
+
+out:
+	iio_trigger_notify_done(iio->trig);
+
+	return IRQ_HANDLED;
+}
+
 static int dps310_probe(struct i2c_client *client)
 {
 	const struct i2c_device_id *id = i2c_client_get_device_id(client);
@@ -863,6 +983,7 @@ static int dps310_probe(struct i2c_client *client)
 	iio->num_channels = ARRAY_SIZE(dps310_channels);
 	iio->info = &dps310_info;
 	iio->modes = INDIO_DIRECT_MODE;
+	iio->available_scan_masks = dps310_scan_masks;
 
 	data->regmap = devm_regmap_init_i2c(client, &dps310_regmap_config);
 	if (IS_ERR(data->regmap))
@@ -877,6 +998,12 @@ static int dps310_probe(struct i2c_client *client)
 	if (rc)
 		return rc;
 
+	rc = devm_iio_triggered_buffer_setup(&client->dev, iio,
+					     iio_pollfunc_store_time,
+					     dps310_trigger_handler, NULL);
+	if (rc)
+		return rc;
+
 	rc = devm_iio_device_register(&client->dev, iio);
 	if (rc)
 		return rc;
-- 
2.43.0

On Sun, 19 Jul 2026 02:51:58 +0300
Rupesh Majhi <zoone.rupert@gmail.com> wrote:

> Add triggered buffer support so pressure and temperature can be captured
> into a buffer instead of only through one-shot sysfs reads.
> 
> Pressure is a processed value in kPa computed from a non-linear
> calibration polynomial. To keep full resolution in the buffer without
> disagreeing with the sysfs unit, add raw and scale attributes for
> pressure (raw in Pa, scale 1/1000 to kPa), following bme680; the existing
> processed attribute is kept for ABI compatibility. 

Hmm. That is unfortunate.  Ideally in original driver we'd have only
output RAW (even though somewhat processed) then this would be the
same.   Agreed that given the constraints, this is the way we'll have to go.
Can you make sure to add some comments around the chan spec about this.
I don't want it copied into other drivers!

> Temperature is already
> a full-resolution value in its base unit (millidegrees Celsius) and stays
> a processed channel.
> 
> Pressure compensation depends on a temperature reading, so both channels
> are always captured together. The device already runs in continuous
> background mode, so no buffer setup ops are needed. Sysfs reads and
> reconfiguration return -EBUSY while the buffer is enabled, as they share
> the capture path's raw values and configuration.
> 
> Signed-off-by: Rupesh Majhi <zoone.rupert@gmail.com>
https://sashiko.dev/#/patchset/20260718211727.39752-1-zoone.rupert%40gmail.com

Some stuff in there to check. I haven't looked closely beyond checking
for usual false positives (none of those).

> ---
> Changes in v3:
>  - Rework from just enabling the FIFO into proper IIO triggered buffer
>    support, as suggested by Jonathan.
>  - Buffer the pressure and temperature values; add raw + scale for
>    pressure (bme680-style) so buffered pressure keeps full resolution
>    and stays consistent with the sysfs unit.
>  - Drop the unused FIFO enable/flush/read helpers.

Ah... This wasn't what I meant.  I was expecting to see the fifo
stuff but coupled with use of buffers.  It may not make sense to support
triggered buffer for that case.  Often doesn't for FIFO equiped parts where
the read out of a fifo is not reading just one 'scan' but many and where
timestamps are either not provided or are the subject of non trivial
interpolation of interrupt timings.

So does supporting an external trigger (what this does) actually make
sense for this device?  There are drivers where we do both but I would
expect to see that in one patch set as the mechanism for choosing fifo
(and watermarks interrupt) vs direct reads and a trigger is the complex
corner.   I think what would make sense for this device would be
that fifo mode is used when a trigger is not provided, but disabled when
one is.

Anyhow I've given this a quick review based on the fact we might
want to carry forward triggered buffer support as part of the solution.

> 
> v2 was a resend that still contained the v1 FIFO-enable code.
> 
> Note: this touches dps310_probe() near the separately-sent fix "iio:
> pressure: dps310: fix NULL pointer deref on ACPI probe". It was generated
> on a plain base tree, so applying it after that fix needs a trivial
> 3-way merge/rebase - happy to resend in whatever order or branch you
> prefer.
> 
>  drivers/iio/pressure/Kconfig  |   2 +
>  drivers/iio/pressure/dps310.c | 141 ++++++++++++++++++++++++++++++++--
>  2 files changed, 136 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/iio/pressure/Kconfig b/drivers/iio/pressure/Kconfig
> index 838a8340c4c0..cef8b90b9ae7 100644
> --- a/drivers/iio/pressure/Kconfig
> +++ b/drivers/iio/pressure/Kconfig
> @@ -112,6 +112,8 @@ config DPS310
>  	tristate "Infineon DPS310 pressure and temperature sensor"
>  	depends on I2C
>  	select REGMAP_I2C
> +	select IIO_BUFFER
> +	select IIO_TRIGGERED_BUFFER
>  	help
>  	  Support for the Infineon DPS310 digital barometric pressure sensor.
>  	  It can be accessed over I2C bus.
> diff --git a/drivers/iio/pressure/dps310.c b/drivers/iio/pressure/dps310.c
> index f45af72a0554..967a2043550b 100644
> --- a/drivers/iio/pressure/dps310.c
> +++ b/drivers/iio/pressure/dps310.c
> @@ -20,8 +20,11 @@
>  #include <linux/module.h>
>  #include <linux/regmap.h>
>  
> +#include <linux/iio/buffer.h>
>  #include <linux/iio/iio.h>
>  #include <linux/iio/sysfs.h>
> +#include <linux/iio/trigger_consumer.h>
> +#include <linux/iio/triggered_buffer.h>
>  
>  #define DPS310_DEV_NAME		"dps310"
>  
> @@ -90,6 +93,12 @@ struct dps310_data {
>  	s32 pressure_raw;
>  	s32 temp_raw;
>  	bool timeout_recovery_failed;
> +
> +	/* Buffer to hold a scan; timestamp is naturally aligned */
> +	struct {
> +		s32 chan[2];
> +		aligned_s64 timestamp;
> +	} scan __aligned(8);

See below.  No reason to have this in this structure. Just have local
instances on the stack.  + the discussion with Andy, that __aligned(8)
isn't doing anything.

>  };
>  
>  static const struct iio_chan_spec dps310_channels[] = {
> @@ -98,15 +107,38 @@ static const struct iio_chan_spec dps310_channels[] = {
>  		.info_mask_separate = BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO) |
>  			BIT(IIO_CHAN_INFO_SAMP_FREQ) |
>  			BIT(IIO_CHAN_INFO_PROCESSED),
> +		.scan_index = 0,

Andy mentioned this.  I'd generally prefer an enum used for this
as then anywhere else we can clearly see which scan_index we are working with.

> +		.scan_type = {
> +			.sign = 's',
> +			.realbits = 32,
> +			.storagebits = 32,
> +			.endianness = IIO_CPU,
> +		},
>  	},
>  	{
>  		.type = IIO_PRESSURE,
>  		.info_mask_separate = BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO) |
>  			BIT(IIO_CHAN_INFO_SAMP_FREQ) |
> -			BIT(IIO_CHAN_INFO_PROCESSED),
> +			BIT(IIO_CHAN_INFO_PROCESSED) |
> +			BIT(IIO_CHAN_INFO_RAW) |
> +			BIT(IIO_CHAN_INFO_SCALE),
> +		.scan_index = 1,
> +		.scan_type = {
> +			.sign = 's',
> +			.realbits = 32,
> +			.storagebits = 32,
> +			.endianness = IIO_CPU,
> +		},
>  	},
> +	IIO_CHAN_SOFT_TIMESTAMP(2),
>  };
>  
> +/*
> + * Pressure compensation needs a temperature reading, so the trigger
> + * handler always captures both channels; keep them enabled together.
> + */
> +static const unsigned long dps310_scan_masks[] = { GENMASK(1, 0), 0 };
> +
>  /* To be called after checking the COEF_RDY bit in MEAS_CFG */
>  static int dps310_get_coefs(struct dps310_data *data)
>  {
> @@ -583,8 +615,14 @@ static int dps310_write_raw(struct iio_dev *iio,
>  	int rc;
>  	struct dps310_data *data = iio_priv(iio);
>  
> -	if (mutex_lock_interruptible(&data->lock))
> +	/* Don't reconfigure the sensor while a buffered capture is running */
> +	if (!iio_device_claim_direct(iio))
> +		return -EBUSY;

Look at the ACQUIRE stuff in iio.h
> +
> +	if (mutex_lock_interruptible(&data->lock)) {

And similar ACQUIRE stuff for this one.  Given both are
release in all paths just before return it should simplify the flow a little.

> +		iio_device_release_direct(iio);
>  		return -EINTR;
> +	}
>  
>  	switch (mask) {
>  	case IIO_CHAN_INFO_SAMP_FREQ:
> @@ -625,6 +663,7 @@ static int dps310_write_raw(struct iio_dev *iio,
>  	}
>  
>  	mutex_unlock(&data->lock);
> +	iio_device_release_direct(iio);
>  	return rc;
>  }
>  
> @@ -735,6 +774,23 @@ static int dps310_read_pressure(struct dps310_data *data, int *val, int *val2,
>  		*val2 = 1000; /* Convert Pa to KPa per IIO ABI */
>  		return IIO_VAL_FRACTIONAL;
>  
> +	case IIO_CHAN_INFO_RAW:
> +		rc = dps310_read_pres_raw(data);
> +		if (rc)
> +			return rc;
> +
> +		rc = dps310_calculate_pressure(data, val);
> +		if (rc)
> +			return rc;

Maybe wrap this up in a little helper given sharing with PROCESSED.
Will make that a little more visually obvious.

> +
> +		return IIO_VAL_INT;
> +
> +	case IIO_CHAN_INFO_SCALE:
> +		/* Raw pressure is in Pa; scale to kPa per IIO ABI */
> +		*val = 1;
> +		*val2 = 1000;
> +		return IIO_VAL_FRACTIONAL;
> +
>  	case IIO_CHAN_INFO_OVERSAMPLING_RATIO:
>  		rc = dps310_get_pres_precision(data, val);
>  		if (rc)
> @@ -804,12 +860,10 @@ static int dps310_read_temp(struct dps310_data *data, int *val, int *val2,

...

> +static int dps310_read_raw(struct iio_dev *iio,
> +			   struct iio_chan_spec const *chan,
> +			   int *val, int *val2, long mask)
> +{
> +	struct dps310_data *data = iio_priv(iio);
> +	int rc;
> +
> +	switch (mask) {
> +	case IIO_CHAN_INFO_RAW:
> +	case IIO_CHAN_INFO_PROCESSED:
> +		/*
> +		 * Reading a sample uses the same raw values as the buffered
> +		 * capture path, so only allow it outside of buffered mode.
> +		 */
> +		if (!iio_device_claim_direct(iio))
> +			return -EBUSY;

Maybe add scope and use an ACQUIRE here similar to suggested above.
It is more marginal but might make sense to use it for consistency.

> +
> +		rc = dps310_read_channel(data, chan, val, val2, mask);
> +		iio_device_release_direct(iio);
> +		return rc;
> +
> +	default:
> +		return dps310_read_channel(data, chan, val, val2, mask);
> +	}
> +}
> +
>  static void dps310_reset(void *action_data)
>  {
>  	struct dps310_data *data = action_data;
> @@ -843,6 +923,46 @@ static const struct iio_info dps310_info = {
>  	.write_raw = dps310_write_raw,
>  };
>  
> +static irqreturn_t dps310_trigger_handler(int irq, void *p)
> +{
> +	struct iio_poll_func *pf = p;
> +	struct iio_dev *iio = pf->indio_dev;
> +	struct dps310_data *data = iio_priv(iio);
> +	int rc, pressure, temp;
> +
> +	/*
> +	 * Don't hold data->lock across these calls: the read helpers take it
> +	 * themselves and the mutex is not recursive (dps310_calculate_pressure
> +	 * also grabs it with mutex_trylock to refresh the temperature).

Would it make sense to provide a version of these calls that doesn't take the
lock then take it here to avoid bouncing it?

> +	 */
> +	rc = dps310_read_pres_raw(data);
> +	if (rc)
> +		goto out;
> +
> +	rc = dps310_read_temp_raw(data);

Given temperature and pressure are effectively independent sequences I think
you would be better off not providing available_scan_masks() but instead reading
only the ones that are enabled. Lots of examples of how to do that
in tree. 

> +	if (rc)
> +		goto out;
> +
> +	rc = dps310_calculate_pressure(data, &pressure);
> +	if (rc)
> +		goto out;
> +
> +	rc = dps310_calculate_temp(data, &temp);
> +	if (rc)
> +		goto out;
> +
> +	data->scan.chan[0] = temp;	/* millidegrees Celsius */
> +	data->scan.chan[1] = pressure;	/* Pascals */
> +

Drag the scan in here as a local variable. It's fairly small and there
is nothing stopping you having it on the stack given no DMA to it
or anything like that.

> +	iio_push_to_buffers_with_ts(iio, &data->scan, sizeof(data->scan),
> +				    pf->timestamp);
> +
> +out:
> +	iio_trigger_notify_done(iio->trig);
> +
> +	return IRQ_HANDLED;
> +}

On Mon, 20 Jul 2026, Jonathan Cameron wrote:
  > Ah... This wasn't what I meant.  I was expecting to see the fifo
  > stuff but coupled with use of buffers.
  > ...
  > I think what would make sense for this device would be
  > that fifo mode is used when a trigger is not provided, but disabled when
  > one is.

  Thanks for the detailed review, and sorry for taking the FIFO suggestion
  in the wrong direction. I'm happy to carry the triggered-buffer support
  forward and build toward the design you describe, FIFO used when no
  trigger is attached, disabled when one is.

  Before I rework, one scope question :) would you prefer the FIFO and
  external-trigger support in a single series (with the fifo-vs-trigger
  selection as one patch), or is it OK to land the triggered-buffer support
  first -> reworked per your comments -> and add the FIFO/watermark path as a
  follow-up? Either is fine by me, just wanted to structure easiest
for you to review.

  Either way I'll fold in the review comments:

  > Just have local instances on the stack.
  > + the discussion with Andy, that __aligned(8) isn't doing anything.
  Will move the scan buffer to a stack local and drop the __aligned(8).

  > Given temperature and pressure are effectively independent sequences I
  > think you would be better off not providing available_scan_masks()
  Makes sense - I'll drop available_scan_masks and push only the enabled
  channels (still reading temperature internally for pressure compensation).

  > Look at the ACQUIRE stuff in iio.h
  > And similar ACQUIRE stuff for this one.
  Will switch claim_direct and the mutex to the ACQUIRE()/guard() scoped
  helpers, so the release-in-every-path handling goes away.

  > Maybe wrap this up in a little helper given sharing with PROCESSED.
  Will factor the shared read-and-calculate into a helper for the RAW and
  PROCESSED cases.

  > Would it make sense to provide a version of these calls that doesn't take
  > the lock then take it here to avoid bouncing it?
  Sure will add lock-free inner helpers and take the lock once in the
  trigger handler rather than per call.

  > Can you make sure to add some comments around the chan spec about this.
  > I don't want it copied into other drivers!
  Will add a comment on the pressure channel explaining the raw+scale choice
  and it's relevancy to this driver.

  Thanks,
  Rupesh

On Mon, Jul 20, 2026 at 1:11 AM Jonathan Cameron
<jonathan.cameron@oss.qualcomm.com> wrote:
>
> On Sun, 19 Jul 2026 02:51:58 +0300
> Rupesh Majhi <zoone.rupert@gmail.com> wrote:
>
> > Add triggered buffer support so pressure and temperature can be captured
> > into a buffer instead of only through one-shot sysfs reads.
> >
> > Pressure is a processed value in kPa computed from a non-linear
> > calibration polynomial. To keep full resolution in the buffer without
> > disagreeing with the sysfs unit, add raw and scale attributes for
> > pressure (raw in Pa, scale 1/1000 to kPa), following bme680; the existing
> > processed attribute is kept for ABI compatibility.
>
> Hmm. That is unfortunate.  Ideally in original driver we'd have only
> output RAW (even though somewhat processed) then this would be the
> same.   Agreed that given the constraints, this is the way we'll have to go.
> Can you make sure to add some comments around the chan spec about this.
> I don't want it copied into other drivers!
>
> > Temperature is already
> > a full-resolution value in its base unit (millidegrees Celsius) and stays
> > a processed channel.
> >
> > Pressure compensation depends on a temperature reading, so both channels
> > are always captured together. The device already runs in continuous
> > background mode, so no buffer setup ops are needed. Sysfs reads and
> > reconfiguration return -EBUSY while the buffer is enabled, as they share
> > the capture path's raw values and configuration.
> >
> > Signed-off-by: Rupesh Majhi <zoone.rupert@gmail.com>
> https://sashiko.dev/#/patchset/20260718211727.39752-1-zoone.rupert%40gmail.com
>
> Some stuff in there to check. I haven't looked closely beyond checking
> for usual false positives (none of those).
>
> > ---
> > Changes in v3:
> >  - Rework from just enabling the FIFO into proper IIO triggered buffer
> >    support, as suggested by Jonathan.
> >  - Buffer the pressure and temperature values; add raw + scale for
> >    pressure (bme680-style) so buffered pressure keeps full resolution
> >    and stays consistent with the sysfs unit.
> >  - Drop the unused FIFO enable/flush/read helpers.
>
> Ah... This wasn't what I meant.  I was expecting to see the fifo
> stuff but coupled with use of buffers.  It may not make sense to support
> triggered buffer for that case.  Often doesn't for FIFO equiped parts where
> the read out of a fifo is not reading just one 'scan' but many and where
> timestamps are either not provided or are the subject of non trivial
> interpolation of interrupt timings.
>
> So does supporting an external trigger (what this does) actually make
> sense for this device?  There are drivers where we do both but I would
> expect to see that in one patch set as the mechanism for choosing fifo
> (and watermarks interrupt) vs direct reads and a trigger is the complex
> corner.   I think what would make sense for this device would be
> that fifo mode is used when a trigger is not provided, but disabled when
> one is.
>
> Anyhow I've given this a quick review based on the fact we might
> want to carry forward triggered buffer support as part of the solution.
>
> >
> > v2 was a resend that still contained the v1 FIFO-enable code.
> >
> > Note: this touches dps310_probe() near the separately-sent fix "iio:
> > pressure: dps310: fix NULL pointer deref on ACPI probe". It was generated
> > on a plain base tree, so applying it after that fix needs a trivial
> > 3-way merge/rebase - happy to resend in whatever order or branch you
> > prefer.
> >
> >  drivers/iio/pressure/Kconfig  |   2 +
> >  drivers/iio/pressure/dps310.c | 141 ++++++++++++++++++++++++++++++++--
> >  2 files changed, 136 insertions(+), 7 deletions(-)
> >
> > diff --git a/drivers/iio/pressure/Kconfig b/drivers/iio/pressure/Kconfig
> > index 838a8340c4c0..cef8b90b9ae7 100644
> > --- a/drivers/iio/pressure/Kconfig
> > +++ b/drivers/iio/pressure/Kconfig
> > @@ -112,6 +112,8 @@ config DPS310
> >       tristate "Infineon DPS310 pressure and temperature sensor"
> >       depends on I2C
> >       select REGMAP_I2C
> > +     select IIO_BUFFER
> > +     select IIO_TRIGGERED_BUFFER
> >       help
> >         Support for the Infineon DPS310 digital barometric pressure sensor.
> >         It can be accessed over I2C bus.
> > diff --git a/drivers/iio/pressure/dps310.c b/drivers/iio/pressure/dps310.c
> > index f45af72a0554..967a2043550b 100644
> > --- a/drivers/iio/pressure/dps310.c
> > +++ b/drivers/iio/pressure/dps310.c
> > @@ -20,8 +20,11 @@
> >  #include <linux/module.h>
> >  #include <linux/regmap.h>
> >
> > +#include <linux/iio/buffer.h>
> >  #include <linux/iio/iio.h>
> >  #include <linux/iio/sysfs.h>
> > +#include <linux/iio/trigger_consumer.h>
> > +#include <linux/iio/triggered_buffer.h>
> >
> >  #define DPS310_DEV_NAME              "dps310"
> >
> > @@ -90,6 +93,12 @@ struct dps310_data {
> >       s32 pressure_raw;
> >       s32 temp_raw;
> >       bool timeout_recovery_failed;
> > +
> > +     /* Buffer to hold a scan; timestamp is naturally aligned */
> > +     struct {
> > +             s32 chan[2];
> > +             aligned_s64 timestamp;
> > +     } scan __aligned(8);
>
> See below.  No reason to have this in this structure. Just have local
> instances on the stack.  + the discussion with Andy, that __aligned(8)
> isn't doing anything.
>
> >  };
> >
> >  static const struct iio_chan_spec dps310_channels[] = {
> > @@ -98,15 +107,38 @@ static const struct iio_chan_spec dps310_channels[] = {
> >               .info_mask_separate = BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO) |
> >                       BIT(IIO_CHAN_INFO_SAMP_FREQ) |
> >                       BIT(IIO_CHAN_INFO_PROCESSED),
> > +             .scan_index = 0,
>
> Andy mentioned this.  I'd generally prefer an enum used for this
> as then anywhere else we can clearly see which scan_index we are working with.
>
> > +             .scan_type = {
> > +                     .sign = 's',
> > +                     .realbits = 32,
> > +                     .storagebits = 32,
> > +                     .endianness = IIO_CPU,
> > +             },
> >       },
> >       {
> >               .type = IIO_PRESSURE,
> >               .info_mask_separate = BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO) |
> >                       BIT(IIO_CHAN_INFO_SAMP_FREQ) |
> > -                     BIT(IIO_CHAN_INFO_PROCESSED),
> > +                     BIT(IIO_CHAN_INFO_PROCESSED) |
> > +                     BIT(IIO_CHAN_INFO_RAW) |
> > +                     BIT(IIO_CHAN_INFO_SCALE),
> > +             .scan_index = 1,
> > +             .scan_type = {
> > +                     .sign = 's',
> > +                     .realbits = 32,
> > +                     .storagebits = 32,
> > +                     .endianness = IIO_CPU,
> > +             },
> >       },
> > +     IIO_CHAN_SOFT_TIMESTAMP(2),
> >  };
> >
> > +/*
> > + * Pressure compensation needs a temperature reading, so the trigger
> > + * handler always captures both channels; keep them enabled together.
> > + */
> > +static const unsigned long dps310_scan_masks[] = { GENMASK(1, 0), 0 };
> > +
> >  /* To be called after checking the COEF_RDY bit in MEAS_CFG */
> >  static int dps310_get_coefs(struct dps310_data *data)
> >  {
> > @@ -583,8 +615,14 @@ static int dps310_write_raw(struct iio_dev *iio,
> >       int rc;
> >       struct dps310_data *data = iio_priv(iio);
> >
> > -     if (mutex_lock_interruptible(&data->lock))
> > +     /* Don't reconfigure the sensor while a buffered capture is running */
> > +     if (!iio_device_claim_direct(iio))
> > +             return -EBUSY;
>
> Look at the ACQUIRE stuff in iio.h
> > +
> > +     if (mutex_lock_interruptible(&data->lock)) {
>
> And similar ACQUIRE stuff for this one.  Given both are
> release in all paths just before return it should simplify the flow a little.
>
> > +             iio_device_release_direct(iio);
> >               return -EINTR;
> > +     }
> >
> >       switch (mask) {
> >       case IIO_CHAN_INFO_SAMP_FREQ:
> > @@ -625,6 +663,7 @@ static int dps310_write_raw(struct iio_dev *iio,
> >       }
> >
> >       mutex_unlock(&data->lock);
> > +     iio_device_release_direct(iio);
> >       return rc;
> >  }
> >
> > @@ -735,6 +774,23 @@ static int dps310_read_pressure(struct dps310_data *data, int *val, int *val2,
> >               *val2 = 1000; /* Convert Pa to KPa per IIO ABI */
> >               return IIO_VAL_FRACTIONAL;
> >
> > +     case IIO_CHAN_INFO_RAW:
> > +             rc = dps310_read_pres_raw(data);
> > +             if (rc)
> > +                     return rc;
> > +
> > +             rc = dps310_calculate_pressure(data, val);
> > +             if (rc)
> > +                     return rc;
>
> Maybe wrap this up in a little helper given sharing with PROCESSED.
> Will make that a little more visually obvious.
>
> > +
> > +             return IIO_VAL_INT;
> > +
> > +     case IIO_CHAN_INFO_SCALE:
> > +             /* Raw pressure is in Pa; scale to kPa per IIO ABI */
> > +             *val = 1;
> > +             *val2 = 1000;
> > +             return IIO_VAL_FRACTIONAL;
> > +
> >       case IIO_CHAN_INFO_OVERSAMPLING_RATIO:
> >               rc = dps310_get_pres_precision(data, val);
> >               if (rc)
> > @@ -804,12 +860,10 @@ static int dps310_read_temp(struct dps310_data *data, int *val, int *val2,
>
> ...
>
> > +static int dps310_read_raw(struct iio_dev *iio,
> > +                        struct iio_chan_spec const *chan,
> > +                        int *val, int *val2, long mask)
> > +{
> > +     struct dps310_data *data = iio_priv(iio);
> > +     int rc;
> > +
> > +     switch (mask) {
> > +     case IIO_CHAN_INFO_RAW:
> > +     case IIO_CHAN_INFO_PROCESSED:
> > +             /*
> > +              * Reading a sample uses the same raw values as the buffered
> > +              * capture path, so only allow it outside of buffered mode.
> > +              */
> > +             if (!iio_device_claim_direct(iio))
> > +                     return -EBUSY;
>
> Maybe add scope and use an ACQUIRE here similar to suggested above.
> It is more marginal but might make sense to use it for consistency.
>
> > +
> > +             rc = dps310_read_channel(data, chan, val, val2, mask);
> > +             iio_device_release_direct(iio);
> > +             return rc;
> > +
> > +     default:
> > +             return dps310_read_channel(data, chan, val, val2, mask);
> > +     }
> > +}
> > +
> >  static void dps310_reset(void *action_data)
> >  {
> >       struct dps310_data *data = action_data;
> > @@ -843,6 +923,46 @@ static const struct iio_info dps310_info = {
> >       .write_raw = dps310_write_raw,
> >  };
> >
> > +static irqreturn_t dps310_trigger_handler(int irq, void *p)
> > +{
> > +     struct iio_poll_func *pf = p;
> > +     struct iio_dev *iio = pf->indio_dev;
> > +     struct dps310_data *data = iio_priv(iio);
> > +     int rc, pressure, temp;
> > +
> > +     /*
> > +      * Don't hold data->lock across these calls: the read helpers take it
> > +      * themselves and the mutex is not recursive (dps310_calculate_pressure
> > +      * also grabs it with mutex_trylock to refresh the temperature).
>
> Would it make sense to provide a version of these calls that doesn't take the
> lock then take it here to avoid bouncing it?
>
> > +      */
> > +     rc = dps310_read_pres_raw(data);
> > +     if (rc)
> > +             goto out;
> > +
> > +     rc = dps310_read_temp_raw(data);
>
> Given temperature and pressure are effectively independent sequences I think
> you would be better off not providing available_scan_masks() but instead reading
> only the ones that are enabled. Lots of examples of how to do that
> in tree.
>
> > +     if (rc)
> > +             goto out;
> > +
> > +     rc = dps310_calculate_pressure(data, &pressure);
> > +     if (rc)
> > +             goto out;
> > +
> > +     rc = dps310_calculate_temp(data, &temp);
> > +     if (rc)
> > +             goto out;
> > +
> > +     data->scan.chan[0] = temp;      /* millidegrees Celsius */
> > +     data->scan.chan[1] = pressure;  /* Pascals */
> > +
>
> Drag the scan in here as a local variable. It's fairly small and there
> is nothing stopping you having it on the stack given no DMA to it
> or anything like that.
>
> > +     iio_push_to_buffers_with_ts(iio, &data->scan, sizeof(data->scan),
> > +                                 pf->timestamp);
> > +
> > +out:
> > +     iio_trigger_notify_done(iio->trig);
> > +
> > +     return IRQ_HANDLED;
> > +}

On Mon, 20 Jul 2026 08:56:26 +0300
Rupert Zoone <zoone.rupert@gmail.com> wrote:

> On Mon, 20 Jul 2026, Jonathan Cameron wrote:
>   > Ah... This wasn't what I meant.  I was expecting to see the fifo
>   > stuff but coupled with use of buffers.
>   > ...
>   > I think what would make sense for this device would be
>   > that fifo mode is used when a trigger is not provided, but disabled when
>   > one is.  
> 
>   Thanks for the detailed review, and sorry for taking the FIFO suggestion
>   in the wrong direction. I'm happy to carry the triggered-buffer support
>   forward and build toward the design you describe, FIFO used when no
>   trigger is attached, disabled when one is.
> 
>   Before I rework, one scope question :) would you prefer the FIFO and
>   external-trigger support in a single series (with the fifo-vs-trigger
>   selection as one patch), or is it OK to land the triggered-buffer support
>   first -> reworked per your comments -> and add the FIFO/watermark path as a
>   follow-up? Either is fine by me, just wanted to structure easiest
> for you to review.

In this particular case I think a single series is more appropriate
as then we can clearly evaluate how the switch between the two modes
is handled.

> 
>   Either way I'll fold in the review comments:
> 
>   > Just have local instances on the stack.
>   > + the discussion with Andy, that __aligned(8) isn't doing anything.  
>   Will move the scan buffer to a stack local and drop the __aligned(8).

Please reply to these inline next time (so next to where they were made).
The context is useful to anyone trying to work out what these are about.
By all means crop out any code that has no more discussion in a given reply.

Where you are accepting a suggestion, don't bother mentioning it. Just crop
that bit or the thread out as we will be default assume you accept anything
you don't reply to :)


> 
>   > Given temperature and pressure are effectively independent sequences I
>   > think you would be better off not providing available_scan_masks()  
>   Makes sense - I'll drop available_scan_masks and push only the enabled
>   channels (still reading temperature internally for pressure compensation).
> 
>   > Look at the ACQUIRE stuff in iio.h
>   > And similar ACQUIRE stuff for this one.  
>   Will switch claim_direct and the mutex to the ACQUIRE()/guard() scoped
>   helpers, so the release-in-every-path handling goes away.
> 
>   > Maybe wrap this up in a little helper given sharing with PROCESSED.  
>   Will factor the shared read-and-calculate into a helper for the RAW and
>   PROCESSED cases.
> 
>   > Would it make sense to provide a version of these calls that doesn't take
>   > the lock then take it here to avoid bouncing it?  
>   Sure will add lock-free inner helpers and take the lock once in the
>   trigger handler rather than per call.
> 
>   > Can you make sure to add some comments around the chan spec about this.
>   > I don't want it copied into other drivers!  
>   Will add a comment on the pressure channel explaining the raw+scale choice
>   and it's relevancy to this driver.
> 
>   Thanks,
>   Rupesh
> 
> On Mon, Jul 20, 2026 at 1:11 AM Jonathan Cameron
> <jonathan.cameron@oss.qualcomm.com> wrote:
> >
> > On Sun, 19 Jul 2026 02:51:58 +0300
> > Rupesh Majhi <zoone.rupert@gmail.com> wrote:
> >  
> > > Add triggered buffer support so pressure and temperature can be captured
> > > into a buffer instead of only through one-shot sysfs reads.
> > >
> > > Pressure is a processed value in kPa computed from a non-linear
> > > calibration polynomial. To keep full resolution in the buffer without
> > > disagreeing with the sysfs unit, add raw and scale attributes for
> > > pressure (raw in Pa, scale 1/1000 to kPa), following bme680; the existing
> > > processed attribute is kept for ABI compatibility.  
> >
> > Hmm. That is unfortunate.  Ideally in original driver we'd have only
> > output RAW (even though somewhat processed) then this would be the
> > same.   Agreed that given the constraints, this is the way we'll have to go.
> > Can you make sure to add some comments around the chan spec about this.
> > I don't want it copied into other drivers!
> >  
> > > Temperature is already
> > > a full-resolution value in its base unit (millidegrees Celsius) and stays
> > > a processed channel.
> > >
> > > Pressure compensation depends on a temperature reading, so both channels
> > > are always captured together. The device already runs in continuous
> > > background mode, so no buffer setup ops are needed. Sysfs reads and
> > > reconfiguration return -EBUSY while the buffer is enabled, as they share
> > > the capture path's raw values and configuration.
> > >
> > > Signed-off-by: Rupesh Majhi <zoone.rupert@gmail.com>  
> > https://sashiko.dev/#/patchset/20260718211727.39752-1-zoone.rupert%40gmail.com
> >
> > Some stuff in there to check. I haven't looked closely beyond checking
> > for usual false positives (none of those).
> >  
> > > ---
> > > Changes in v3:
> > >  - Rework from just enabling the FIFO into proper IIO triggered buffer
> > >    support, as suggested by Jonathan.
> > >  - Buffer the pressure and temperature values; add raw + scale for
> > >    pressure (bme680-style) so buffered pressure keeps full resolution
> > >    and stays consistent with the sysfs unit.
> > >  - Drop the unused FIFO enable/flush/read helpers.  
> >
> > Ah... This wasn't what I meant.  I was expecting to see the fifo
> > stuff but coupled with use of buffers.  It may not make sense to support
> > triggered buffer for that case.  Often doesn't for FIFO equiped parts where
> > the read out of a fifo is not reading just one 'scan' but many and where
> > timestamps are either not provided or are the subject of non trivial
> > interpolation of interrupt timings.
> >
> > So does supporting an external trigger (what this does) actually make
> > sense for this device?  There are drivers where we do both but I would
> > expect to see that in one patch set as the mechanism for choosing fifo
> > (and watermarks interrupt) vs direct reads and a trigger is the complex
> > corner.   I think what would make sense for this device would be
> > that fifo mode is used when a trigger is not provided, but disabled when
> > one is.
> >
> > Anyhow I've given this a quick review based on the fact we might
> > want to carry forward triggered buffer support as part of the solution.
> >  
> > >
> > > v2 was a resend that still contained the v1 FIFO-enable code.
> > >
> > > Note: this touches dps310_probe() near the separately-sent fix "iio:
> > > pressure: dps310: fix NULL pointer deref on ACPI probe". It was generated
> > > on a plain base tree, so applying it after that fix needs a trivial
> > > 3-way merge/rebase - happy to resend in whatever order or branch you
> > > prefer.
> > >
> > >  drivers/iio/pressure/Kconfig  |   2 +
> > >  drivers/iio/pressure/dps310.c | 141 ++++++++++++++++++++++++++++++++--
> > >  2 files changed, 136 insertions(+), 7 deletions(-)
> > >
> > > diff --git a/drivers/iio/pressure/Kconfig b/drivers/iio/pressure/Kconfig
> > > index 838a8340c4c0..cef8b90b9ae7 100644
> > > --- a/drivers/iio/pressure/Kconfig
> > > +++ b/drivers/iio/pressure/Kconfig
> > > @@ -112,6 +112,8 @@ config DPS310
> > >       tristate "Infineon DPS310 pressure and temperature sensor"
> > >       depends on I2C
> > >       select REGMAP_I2C
> > > +     select IIO_BUFFER
> > > +     select IIO_TRIGGERED_BUFFER
> > >       help
> > >         Support for the Infineon DPS310 digital barometric pressure sensor.
> > >         It can be accessed over I2C bus.
> > > diff --git a/drivers/iio/pressure/dps310.c b/drivers/iio/pressure/dps310.c
> > > index f45af72a0554..967a2043550b 100644
> > > --- a/drivers/iio/pressure/dps310.c
> > > +++ b/drivers/iio/pressure/dps310.c
> > > @@ -20,8 +20,11 @@
> > >  #include <linux/module.h>
> > >  #include <linux/regmap.h>
> > >
> > > +#include <linux/iio/buffer.h>
> > >  #include <linux/iio/iio.h>
> > >  #include <linux/iio/sysfs.h>
> > > +#include <linux/iio/trigger_consumer.h>
> > > +#include <linux/iio/triggered_buffer.h>
> > >
> > >  #define DPS310_DEV_NAME              "dps310"
> > >
> > > @@ -90,6 +93,12 @@ struct dps310_data {
> > >       s32 pressure_raw;
> > >       s32 temp_raw;
> > >       bool timeout_recovery_failed;
> > > +
> > > +     /* Buffer to hold a scan; timestamp is naturally aligned */
> > > +     struct {
> > > +             s32 chan[2];
> > > +             aligned_s64 timestamp;
> > > +     } scan __aligned(8);  
> >
> > See below.  No reason to have this in this structure. Just have local
> > instances on the stack.  + the discussion with Andy, that __aligned(8)
> > isn't doing anything.
> >  
> > >  };
> > >
> > >  static const struct iio_chan_spec dps310_channels[] = {
> > > @@ -98,15 +107,38 @@ static const struct iio_chan_spec dps310_channels[] = {
> > >               .info_mask_separate = BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO) |
> > >                       BIT(IIO_CHAN_INFO_SAMP_FREQ) |
> > >                       BIT(IIO_CHAN_INFO_PROCESSED),
> > > +             .scan_index = 0,  
> >
> > Andy mentioned this.  I'd generally prefer an enum used for this
> > as then anywhere else we can clearly see which scan_index we are working with.
> >  
> > > +             .scan_type = {
> > > +                     .sign = 's',
> > > +                     .realbits = 32,
> > > +                     .storagebits = 32,
> > > +                     .endianness = IIO_CPU,
> > > +             },
> > >       },
> > >       {
> > >               .type = IIO_PRESSURE,
> > >               .info_mask_separate = BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO) |
> > >                       BIT(IIO_CHAN_INFO_SAMP_FREQ) |
> > > -                     BIT(IIO_CHAN_INFO_PROCESSED),
> > > +                     BIT(IIO_CHAN_INFO_PROCESSED) |
> > > +                     BIT(IIO_CHAN_INFO_RAW) |
> > > +                     BIT(IIO_CHAN_INFO_SCALE),
> > > +             .scan_index = 1,
> > > +             .scan_type = {
> > > +                     .sign = 's',
> > > +                     .realbits = 32,
> > > +                     .storagebits = 32,
> > > +                     .endianness = IIO_CPU,
> > > +             },
> > >       },
> > > +     IIO_CHAN_SOFT_TIMESTAMP(2),
> > >  };
> > >
> > > +/*
> > > + * Pressure compensation needs a temperature reading, so the trigger
> > > + * handler always captures both channels; keep them enabled together.
> > > + */
> > > +static const unsigned long dps310_scan_masks[] = { GENMASK(1, 0), 0 };
> > > +
> > >  /* To be called after checking the COEF_RDY bit in MEAS_CFG */
> > >  static int dps310_get_coefs(struct dps310_data *data)
> > >  {
> > > @@ -583,8 +615,14 @@ static int dps310_write_raw(struct iio_dev *iio,
> > >       int rc;
> > >       struct dps310_data *data = iio_priv(iio);
> > >
> > > -     if (mutex_lock_interruptible(&data->lock))
> > > +     /* Don't reconfigure the sensor while a buffered capture is running */
> > > +     if (!iio_device_claim_direct(iio))
> > > +             return -EBUSY;  
> >
> > Look at the ACQUIRE stuff in iio.h  
> > > +
> > > +     if (mutex_lock_interruptible(&data->lock)) {  
> >
> > And similar ACQUIRE stuff for this one.  Given both are
> > release in all paths just before return it should simplify the flow a little.
> >  
> > > +             iio_device_release_direct(iio);
> > >               return -EINTR;
> > > +     }
> > >
> > >       switch (mask) {
> > >       case IIO_CHAN_INFO_SAMP_FREQ:
> > > @@ -625,6 +663,7 @@ static int dps310_write_raw(struct iio_dev *iio,
> > >       }
> > >
> > >       mutex_unlock(&data->lock);
> > > +     iio_device_release_direct(iio);
> > >       return rc;
> > >  }
> > >
> > > @@ -735,6 +774,23 @@ static int dps310_read_pressure(struct dps310_data *data, int *val, int *val2,
> > >               *val2 = 1000; /* Convert Pa to KPa per IIO ABI */
> > >               return IIO_VAL_FRACTIONAL;
> > >
> > > +     case IIO_CHAN_INFO_RAW:
> > > +             rc = dps310_read_pres_raw(data);
> > > +             if (rc)
> > > +                     return rc;
> > > +
> > > +             rc = dps310_calculate_pressure(data, val);
> > > +             if (rc)
> > > +                     return rc;  
> >
> > Maybe wrap this up in a little helper given sharing with PROCESSED.
> > Will make that a little more visually obvious.
> >  
> > > +
> > > +             return IIO_VAL_INT;
> > > +
> > > +     case IIO_CHAN_INFO_SCALE:
> > > +             /* Raw pressure is in Pa; scale to kPa per IIO ABI */
> > > +             *val = 1;
> > > +             *val2 = 1000;
> > > +             return IIO_VAL_FRACTIONAL;
> > > +
> > >       case IIO_CHAN_INFO_OVERSAMPLING_RATIO:
> > >               rc = dps310_get_pres_precision(data, val);
> > >               if (rc)
> > > @@ -804,12 +860,10 @@ static int dps310_read_temp(struct dps310_data *data, int *val, int *val2,  
> >
> > ...
> >  
> > > +static int dps310_read_raw(struct iio_dev *iio,
> > > +                        struct iio_chan_spec const *chan,
> > > +                        int *val, int *val2, long mask)
> > > +{
> > > +     struct dps310_data *data = iio_priv(iio);
> > > +     int rc;
> > > +
> > > +     switch (mask) {
> > > +     case IIO_CHAN_INFO_RAW:
> > > +     case IIO_CHAN_INFO_PROCESSED:
> > > +             /*
> > > +              * Reading a sample uses the same raw values as the buffered
> > > +              * capture path, so only allow it outside of buffered mode.
> > > +              */
> > > +             if (!iio_device_claim_direct(iio))
> > > +                     return -EBUSY;  
> >
> > Maybe add scope and use an ACQUIRE here similar to suggested above.
> > It is more marginal but might make sense to use it for consistency.
> >  
> > > +
> > > +             rc = dps310_read_channel(data, chan, val, val2, mask);
> > > +             iio_device_release_direct(iio);
> > > +             return rc;
> > > +
> > > +     default:
> > > +             return dps310_read_channel(data, chan, val, val2, mask);
> > > +     }
> > > +}
> > > +
> > >  static void dps310_reset(void *action_data)
> > >  {
> > >       struct dps310_data *data = action_data;
> > > @@ -843,6 +923,46 @@ static const struct iio_info dps310_info = {
> > >       .write_raw = dps310_write_raw,
> > >  };
> > >
> > > +static irqreturn_t dps310_trigger_handler(int irq, void *p)
> > > +{
> > > +     struct iio_poll_func *pf = p;
> > > +     struct iio_dev *iio = pf->indio_dev;
> > > +     struct dps310_data *data = iio_priv(iio);
> > > +     int rc, pressure, temp;
> > > +
> > > +     /*
> > > +      * Don't hold data->lock across these calls: the read helpers take it
> > > +      * themselves and the mutex is not recursive (dps310_calculate_pressure
> > > +      * also grabs it with mutex_trylock to refresh the temperature).  
> >
> > Would it make sense to provide a version of these calls that doesn't take the
> > lock then take it here to avoid bouncing it?
> >  
> > > +      */
> > > +     rc = dps310_read_pres_raw(data);
> > > +     if (rc)
> > > +             goto out;
> > > +
> > > +     rc = dps310_read_temp_raw(data);  
> >
> > Given temperature and pressure are effectively independent sequences I think
> > you would be better off not providing available_scan_masks() but instead reading
> > only the ones that are enabled. Lots of examples of how to do that
> > in tree.
> >  
> > > +     if (rc)
> > > +             goto out;
> > > +
> > > +     rc = dps310_calculate_pressure(data, &pressure);
> > > +     if (rc)
> > > +             goto out;
> > > +
> > > +     rc = dps310_calculate_temp(data, &temp);
> > > +     if (rc)
> > > +             goto out;
> > > +
> > > +     data->scan.chan[0] = temp;      /* millidegrees Celsius */
> > > +     data->scan.chan[1] = pressure;  /* Pascals */
> > > +  
> >
> > Drag the scan in here as a local variable. It's fairly small and there
> > is nothing stopping you having it on the stack given no DMA to it
> > or anything like that.
> >  
> > > +     iio_push_to_buffers_with_ts(iio, &data->scan, sizeof(data->scan),
> > > +                                 pf->timestamp);
> > > +
> > > +out:
> > > +     iio_trigger_notify_done(iio->trig);
> > > +
> > > +     return IRQ_HANDLED;
> > > +}  
> 

On Sun, Jul 19, 2026 at 02:51:58AM +0300, Rupesh Majhi wrote:
> Add triggered buffer support so pressure and temperature can be captured
> into a buffer instead of only through one-shot sysfs reads.
> 
> Pressure is a processed value in kPa computed from a non-linear
> calibration polynomial. To keep full resolution in the buffer without
> disagreeing with the sysfs unit, add raw and scale attributes for
> pressure (raw in Pa, scale 1/1000 to kPa), following bme680; the existing
> processed attribute is kept for ABI compatibility. Temperature is already
> a full-resolution value in its base unit (millidegrees Celsius) and stays
> a processed channel.
> 
> Pressure compensation depends on a temperature reading, so both channels
> are always captured together. The device already runs in continuous
> background mode, so no buffer setup ops are needed. Sysfs reads and
> reconfiguration return -EBUSY while the buffer is enabled, as they share
> the capture path's raw values and configuration.

Do not reply to the same thread with a new version.
Do not reply within less than 24h.

The change needs more work, See my comments below.

...

> +	/* Buffer to hold a scan; timestamp is naturally aligned */
> +	struct {
> +		s32 chan[2];
> +		aligned_s64 timestamp;
> +	} scan __aligned(8);

We have a macro for this (alignment). Or even for the entire struct.

...

> +static const unsigned long dps310_scan_masks[] = { GENMASK(1, 0), 0 };

Jonathan usually asks to provide a bit-wise definitions and use them here
instead of GENMASK().

...

> +	case IIO_CHAN_INFO_SCALE:
> +		/* Raw pressure is in Pa; scale to kPa per IIO ABI */
> +		*val = 1;
> +		*val2 = 1000;

So, we have KILO in units.h.

> +		return IIO_VAL_FRACTIONAL;

-- 
With Best Regards,
Andy Shevchenko

On Sun, 19 Jul 2026 11:47:48 +0300
Andy Shevchenko <andriy.shevchenko@intel.com> wrote:

> On Sun, Jul 19, 2026 at 02:51:58AM +0300, Rupesh Majhi wrote:
> > Add triggered buffer support so pressure and temperature can be captured
> > into a buffer instead of only through one-shot sysfs reads.
> > 
> > Pressure is a processed value in kPa computed from a non-linear
> > calibration polynomial. To keep full resolution in the buffer without
> > disagreeing with the sysfs unit, add raw and scale attributes for
> > pressure (raw in Pa, scale 1/1000 to kPa), following bme680; the existing
> > processed attribute is kept for ABI compatibility. Temperature is already
> > a full-resolution value in its base unit (millidegrees Celsius) and stays
> > a processed channel.
> > 
> > Pressure compensation depends on a temperature reading, so both channels
> > are always captured together. The device already runs in continuous
> > background mode, so no buffer setup ops are needed. Sysfs reads and
> > reconfiguration return -EBUSY while the buffer is enabled, as they share
> > the capture path's raw values and configuration.  
> 
> Do not reply to the same thread with a new version.
> Do not reply within less than 24h.
> 
> The change needs more work, See my comments below.
> 
> ...
> 
> > +	/* Buffer to hold a scan; timestamp is naturally aligned */
> > +	struct {
> > +		s32 chan[2];
> > +		aligned_s64 timestamp;
> > +	} scan __aligned(8);  
> 
> We have a macro for this (alignment). Or even for the entire struct.

If (and I haven't checked) that is the required alignment it doesn't
need any marking at all.  The C spec requires a structure to be aligned
to the alignment of it's largest member.  Here alignment is forced to 8
by the aligned_s64.

> 
> ...
> 
> > +static const unsigned long dps310_scan_masks[] = { GENMASK(1, 0), 0 };  
> 
> Jonathan usually asks to provide a bit-wise definitions and use them here
> instead of GENMASK().
> 
> ...
> 
> > +	case IIO_CHAN_INFO_SCALE:
> > +		/* Raw pressure is in Pa; scale to kPa per IIO ABI */
> > +		*val = 1;
> > +		*val2 = 1000;  
> 
> So, we have KILO in units.h.
> 
> > +		return IIO_VAL_FRACTIONAL;  
>

On Sun, Jul 19, 2026 at 11:47:53AM +0300, Andy Shevchenko wrote:
> On Sun, Jul 19, 2026 at 02:51:58AM +0300, Rupesh Majhi wrote:

> Do not reply to the same thread with a new version.
> Do not reply within less than 24h.

To be clear, "do not reply with a new series within less than 24h".

-- 
With Best Regards,
Andy Shevchenko