[PATCH v5 0/3] iio: pressure: dps310: FIFO and triggered buffer support

[PATCH v5 0/3] iio: pressure: dps310: FIFO and triggered buffer support

[Rupesh Majhi]

The dps310 has no buffer support today. This series adds it, with the
hardware FIFO used when no external trigger is attached and the FIFO
left disabled in favor of the trigger when one is, so the switch between
the two modes can be reviewed together rather than in two submissions.

Patch 1 fixes the CFG_REG bit definitions and replaces the standalone
fix I sent on 27 July, which Jonathan asked me to fold in here instead:

  Link: https://lore.kernel.org/linux-iio/20260728223009.0cb86996@jic23-huawei/

All three of those defines have been wrong since the driver was added,
but only P_SHIFT has a user and only that one misbehaves, so the patch
carries a Fixes tag for the original driver and one for the commit that
added the first user of P_SHIFT, along with Cc: stable. The FIFO enable
is needed by patch 3.

The three INT_SEL interrupt enables at bits 6 to 4 are still not
defined. Nothing uses them, the driver has no interrupt path, and the
binding has no interrupts property, so adding unused defines to a fix
did not seem worth it. David also asked for the register defines to be
sorted low to high. That is a cleanup series of its own once this lands.

Patch 2 adds the triggered buffer path.

Patch 3 adds the hardware FIFO and the selection between it and an
attached trigger. Those started out as separate patches, but the branch
on iio_device_get_current_mode() is four lines and the FIFO patch is
wrong without it, since postenable would otherwise start the FIFO while
a trigger was driving the buffer. Splitting them would only have left a
broken commit in between, so they are one patch.

Verified on an Infineon DPS310 breakout wired to a BeagleBone Black,
running this series on 7.2.0-rc2. Two modules built from the same tree,
differing only in the three CFG_REG defines corrected here, loaded
seconds apart. Three reads of in_pressure_input per oversampling ratio,
ambient 98.4 kPa and 27.2 degC:

  OSR    before              after
    1    98.433  98.428      98.445  98.446
    8    98.460  98.460      98.477  98.479
   16    -ERANGE             98.566  98.564
   32    -ERANGE             98.428  98.427
   64    -ERANGE             98.464  98.463
  128    98.439  98.440      98.434  98.434

Pressure oversampling 16, 32 and 64 return -ERANGE before the fix.
P_SHIFT is never enabled, so the result register no longer matches the
scale factor the compensation divides by, and
dps310_calculate_pressure() ends up negative. 128 is not affected in
practice. Temperature is unaffected throughout, since TMP_SHIFT_EN was
already defined correctly.

Everything else was checked with checkpatch --strict and a W=1 build,
plus an arm build for aspeed_g5 and a boot under qemu-system-arm -M
rainier-bmc, which covers probe, the sysfs values, raw times scale
matching processed, EBUSY on sysfs reads while the buffer is enabled,
and all three scan mask combinations. QEMU's dps310 model implements
neither the FIFO nor the interrupt, so patch 3 was tested on the
BeagleBone Black above only.

On hardware, patch 3 was checked with both channels enabled, temperature
only and pressure only, at 8 Hz and at 128 Hz. A blocking read returns
in every case, which is the part that needs the timer: with no
interrupt, hwfifo_flush_to_buffer alone would leave a reader asleep on
rb->pollq. At 128 Hz, 100 scans arrive in 0.81 s, so the batching is
real. Timestamps are monotonic throughout and land on the configured
period, 125.0000 ms at 8 Hz, except where a drain collected more than
the rate accounts for and the batch is compressed to stay ordered. With
a sysfs trigger attached the FIFO stays disabled and the trigger drives
the buffer, at the rate trigger_now is written.

Changes since v3 (no v4 was posted):
- the series now includes the FIFO, not just a triggered buffer
- CFG_REG bit fix folded in as patch 1
- scan buffer is a stack local instead of living in struct dps310_data
- available_scan_masks dropped, only enabled channels are pushed
- claim_direct and the mutex use ACQUIRE() scoping
- shared helper for the raw and processed reads
- comment on the pressure channel spec warning against copying the
  raw plus scale approach
- the stale file header claiming only a single temperature read is
  supported goes with patch 3, which is what finally disproves it

Rupesh Majhi (3):
  iio: pressure: dps310: fix CFG_REG bit definitions
  iio: pressure: dps310: add triggered buffer support
  iio: pressure: dps310: add hardware FIFO support

 drivers/iio/pressure/Kconfig  |   2 +
 drivers/iio/pressure/dps310.c | 718 ++++++++++++++++++++++++++++++----
 2 files changed, 646 insertions(+), 74 deletions(-)

-- 
2.43.0

[PATCH v5 1/3] iio: pressure: dps310: fix CFG_REG bit definitions

[Rupesh Majhi]

Three of the CFG_REG bit defines do not match the datasheet. P_SHIFT is
bit 2, FIFO_EN is bit 1 and SPI_MODE is bit 0, but the driver defines them
as BIT(4), BIT(5) and BIT(6). Those three positions are INT_PRS, INT_TMP
and INT_FIFO, the measurement ready and FIFO full interrupt enables for the
SDO pin. All three have had the wrong value since the driver was added,
when only the temperature shift bit had a user.

DPS310_PRS_SHIFT_EN got its first user when pressure support was added, and
there it is a real bug. The datasheet requires the pressure result
bit-shift to be enabled when the oversampling rate is higher than 8 times,
so dps310_set_pres_precision() sets it for oversampling ratios of 16 and
above. With the wrong definition it leaves P_SHIFT clear and toggles the
pressure ready interrupt instead. The result register is then never
shifted, so it no longer matches the scale factor the compensation divides
by. On a DPS310 breakout, reading in_pressure_input at oversampling 16, 32
and 64 returns -ERANGE, because dps310_calculate_pressure() ends up
negative. Oversampling 128 was not observed to be affected.

DPS310_FIFO_EN and DPS310_SPI_EN still have no users, so correcting them
changes nothing on its own, but the FIFO enable is needed by the hardware
FIFO support later in this series.

Temperature is not affected, T_SHIFT is bit 3 and DPS310_TMP_SHIFT_EN
already matches it.

Fixes: ba6ec48e76bc ("iio: Add driver for Infineon DPS310")
Fixes: d711a3c7dc82 ("iio: dps310: Add pressure sensing capability")
Cc: stable@vger.kernel.org
Signed-off-by: Rupesh Majhi <zoone.rupert@gmail.com>
---
 drivers/iio/pressure/dps310.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/iio/pressure/dps310.c b/drivers/iio/pressure/dps310.c
index f45af72a0554..68382960382f 100644
--- a/drivers/iio/pressure/dps310.c
+++ b/drivers/iio/pressure/dps310.c
@@ -50,9 +50,9 @@
 #define DPS310_CFG_REG		0x09
 #define  DPS310_INT_HL		BIT(7)
 #define  DPS310_TMP_SHIFT_EN	BIT(3)
-#define  DPS310_PRS_SHIFT_EN	BIT(4)
-#define  DPS310_FIFO_EN		BIT(5)
-#define  DPS310_SPI_EN		BIT(6)
+#define  DPS310_PRS_SHIFT_EN	BIT(2)
+#define  DPS310_FIFO_EN		BIT(1)
+#define  DPS310_SPI_EN		BIT(0)
 #define DPS310_RESET		0x0c
 #define  DPS310_RESET_MAGIC	0x09
 #define DPS310_COEF_BASE	0x10
-- 
2.43.0

[PATCH v5 2/3] iio: pressure: dps310: add triggered buffer support

[Rupesh Majhi]

Add triggered buffer support so pressure and temperature can be captured
into a buffer instead of only through one-shot sysfs reads. The device
measures continuously in background mode, so a capture is just a read of
the latest results and no buffer setup ops are needed.

Pressure has to be computed from the raw register value with the
compensation polynomial in section 4.9.1 of the datasheet, which also
needs a temperature reading. To keep full resolution in the buffer
without disagreeing with the unit the existing processed attribute
reports, add raw and scale attributes for pressure, raw in Pa and scale
1/1000 to convert to kPa. The channel definition carries a comment
explaining why it is done this way here and why it should not be copied
into other drivers.

Temperature is already a full resolution value in its base unit of
millidegrees Celsius, so it stays a processed channel.

Either channel can be enabled on its own. Temperature is always sampled
because the pressure compensation needs it, but only the enabled channels
are pushed to the buffer.

The raw read helpers are split into variants that expect the lock to be
held, so the trigger handler takes the lock once per scan instead of once
per value. That also lets dps310_calculate_pressure() drop its
mutex_trylock() dance, as the temperature refresh now always happens
under the caller's lock instead of only when the lock happened to be
free.

Sysfs sample reads and reconfiguration return -EBUSY while the buffer is
enabled, since they share the capture path's values and configuration.

Signed-off-by: Rupesh Majhi <zoone.rupert@gmail.com>
---
 drivers/iio/pressure/Kconfig  |   2 +
 drivers/iio/pressure/dps310.c | 305 ++++++++++++++++++++++++++--------
 2 files changed, 242 insertions(+), 65 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 68382960382f..0d6e65766469 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"
 
@@ -92,19 +95,56 @@ struct dps310_data {
 	bool timeout_recovery_failed;
 };
 
+enum dps310_scan_index {
+	DPS310_SCAN_TEMP,
+	DPS310_SCAN_PRESSURE,
+};
+
 static const struct iio_chan_spec dps310_channels[] = {
 	{
 		.type = IIO_TEMP,
 		.info_mask_separate = BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO) |
 			BIT(IIO_CHAN_INFO_SAMP_FREQ) |
 			BIT(IIO_CHAN_INFO_PROCESSED),
+		.scan_index = DPS310_SCAN_TEMP,
+		.scan_type = {
+			.sign = 's',
+			.realbits = 32,
+			.storagebits = 32,
+			.endianness = IIO_CPU,
+		},
 	},
 	{
 		.type = IIO_PRESSURE,
+		/*
+		 * Pressure is only meaningful once the raw register value has
+		 * been run through the compensation polynomial in section 4.9.1
+		 * of the datasheet, which needs a temperature reading as well.
+		 * So what is reported as _raw here is already compensated, in
+		 * Pa, and _scale converts it to the kPa the ABI asks for. The
+		 * _processed attribute reports the same value and predates
+		 * buffer support, so it has to stay.
+		 *
+		 * Please do not copy this pattern into other drivers. A raw
+		 * attribute that is not the raw register value is only
+		 * tolerable here because the alternative is either losing
+		 * resolution in the buffer or breaking existing users of
+		 * _processed.
+		 */
 		.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 = DPS310_SCAN_PRESSURE,
+		.scan_type = {
+			.sign = 's',
+			.realbits = 32,
+			.storagebits = 32,
+			.endianness = IIO_CPU,
+		},
 	},
+	IIO_CHAN_SOFT_TIMESTAMP(2),
 };
 
 /* To be called after checking the COEF_RDY bit in MEAS_CFG */
@@ -463,7 +503,8 @@ static int dps310_ready(struct dps310_data *data, int ready_bit, int timeout)
 	return 0;
 }
 
-static int dps310_read_pres_raw(struct dps310_data *data)
+/* Called with lock held */
+static int dps310_read_pres_raw_locked(struct dps310_data *data)
 {
 	int rc;
 	int rate;
@@ -471,30 +512,25 @@ static int dps310_read_pres_raw(struct dps310_data *data)
 	s32 raw;
 	u8 val[3];
 
-	if (mutex_lock_interruptible(&data->lock))
-		return -EINTR;
-
 	rc = dps310_get_pres_samp_freq(data, &rate);
 	if (rc)
-		goto done;
+		return rc;
 
 	timeout = DPS310_POLL_TIMEOUT_US(rate);
 
 	/* Poll for sensor readiness; base the timeout upon the sample rate. */
 	rc = dps310_ready(data, DPS310_PRS_RDY, timeout);
 	if (rc)
-		goto done;
+		return rc;
 
 	rc = regmap_bulk_read(data->regmap, DPS310_PRS_BASE, val, sizeof(val));
 	if (rc < 0)
-		goto done;
+		return rc;
 
 	raw = (val[0] << 16) | (val[1] << 8) | val[2];
 	data->pressure_raw = sign_extend32(raw, 23);
 
-done:
-	mutex_unlock(&data->lock);
-	return rc;
+	return 0;
 }
 
 /* Called with lock held */
@@ -514,31 +550,45 @@ static int dps310_read_temp_ready(struct dps310_data *data)
 	return 0;
 }
 
-static int dps310_read_temp_raw(struct dps310_data *data)
+/* Called with lock held */
+static int dps310_read_temp_raw_locked(struct dps310_data *data)
 {
 	int rc;
 	int rate;
 	int timeout;
 
-	if (mutex_lock_interruptible(&data->lock))
-		return -EINTR;
-
 	rc = dps310_get_temp_samp_freq(data, &rate);
 	if (rc)
-		goto done;
+		return rc;
 
 	timeout = DPS310_POLL_TIMEOUT_US(rate);
 
 	/* Poll for sensor readiness; base the timeout upon the sample rate. */
 	rc = dps310_ready(data, DPS310_TMP_RDY, timeout);
 	if (rc)
-		goto done;
+		return rc;
 
-	rc = dps310_read_temp_ready(data);
+	return dps310_read_temp_ready(data);
+}
 
-done:
-	mutex_unlock(&data->lock);
-	return rc;
+/*
+ * Refresh the cached temperature if a new measurement is ready, so that the
+ * pressure compensation below uses a recent value. Errors are not fatal here,
+ * the previous temperature is used instead.
+ *
+ * Called with lock held.
+ */
+static void dps310_refresh_temp_locked(struct dps310_data *data)
+{
+	int rc;
+	int t_ready;
+
+	rc = regmap_read(data->regmap, DPS310_MEAS_CFG, &t_ready);
+	if (rc)
+		return;
+
+	if (t_ready & DPS310_TMP_RDY)
+		dps310_read_temp_ready(data);
 }
 
 static bool dps310_is_writeable_reg(struct device *dev, unsigned int reg)
@@ -580,59 +630,52 @@ static int dps310_write_raw(struct iio_dev *iio,
 			    struct iio_chan_spec const *chan, int val,
 			    int val2, long mask)
 {
-	int rc;
 	struct dps310_data *data = iio_priv(iio);
 
-	if (mutex_lock_interruptible(&data->lock))
+	/* Reconfiguring mid-capture would change the values being captured */
+	IIO_DEV_ACQUIRE_DIRECT_MODE(iio, claim);
+	if (IIO_DEV_ACQUIRE_FAILED(claim))
+		return -EBUSY;
+
+	ACQUIRE(mutex_intr, lock)(&data->lock);
+	if (ACQUIRE_ERR(mutex_intr, &lock))
 		return -EINTR;
 
 	switch (mask) {
 	case IIO_CHAN_INFO_SAMP_FREQ:
 		switch (chan->type) {
 		case IIO_PRESSURE:
-			rc = dps310_set_pres_samp_freq(data, val);
-			break;
+			return dps310_set_pres_samp_freq(data, val);
 
 		case IIO_TEMP:
-			rc = dps310_set_temp_samp_freq(data, val);
-			break;
+			return dps310_set_temp_samp_freq(data, val);
 
 		default:
-			rc = -EINVAL;
-			break;
+			return -EINVAL;
 		}
-		break;
 
 	case IIO_CHAN_INFO_OVERSAMPLING_RATIO:
 		switch (chan->type) {
 		case IIO_PRESSURE:
-			rc = dps310_set_pres_precision(data, val);
-			break;
+			return dps310_set_pres_precision(data, val);
 
 		case IIO_TEMP:
-			rc = dps310_set_temp_precision(data, val);
-			break;
+			return dps310_set_temp_precision(data, val);
 
 		default:
-			rc = -EINVAL;
-			break;
+			return -EINVAL;
 		}
-		break;
 
 	default:
-		rc = -EINVAL;
-		break;
+		return -EINVAL;
 	}
-
-	mutex_unlock(&data->lock);
-	return rc;
 }
 
+/* Called with lock held */
 static int dps310_calculate_pressure(struct dps310_data *data, int *val)
 {
 	int i;
 	int rc;
-	int t_ready;
 	int kpi;
 	int kti;
 	s64 rem = 0ULL;
@@ -656,15 +699,6 @@ static int dps310_calculate_pressure(struct dps310_data *data, int *val)
 	kp = (s64)kpi;
 	kt = (s64)kti;
 
-	/* Refresh temp if it's ready, otherwise just use the latest value */
-	if (mutex_trylock(&data->lock)) {
-		rc = regmap_read(data->regmap, DPS310_MEAS_CFG, &t_ready);
-		if (rc >= 0 && t_ready & DPS310_TMP_RDY)
-			dps310_read_temp_ready(data);
-
-		mutex_unlock(&data->lock);
-	}
-
 	p = (s64)data->pressure_raw;
 	t = (s64)data->temp_raw;
 
@@ -710,6 +744,28 @@ static int dps310_calculate_pressure(struct dps310_data *data, int *val)
 	return 0;
 }
 
+/*
+ * Sample the pressure and compensate it. Shared by the raw and processed
+ * attributes, which report the same value in different units, and takes the
+ * lock once for the whole sequence.
+ */
+static int dps310_read_pressure_value(struct dps310_data *data, int *val)
+{
+	int rc;
+
+	ACQUIRE(mutex_intr, lock)(&data->lock);
+	if (ACQUIRE_ERR(mutex_intr, &lock))
+		return -EINTR;
+
+	rc = dps310_read_pres_raw_locked(data);
+	if (rc)
+		return rc;
+
+	dps310_refresh_temp_locked(data);
+
+	return dps310_calculate_pressure(data, val);
+}
+
 static int dps310_read_pressure(struct dps310_data *data, int *val, int *val2,
 				long mask)
 {
@@ -723,18 +779,27 @@ static int dps310_read_pressure(struct dps310_data *data, int *val, int *val2,
 
 		return IIO_VAL_INT;
 
-	case IIO_CHAN_INFO_PROCESSED:
-		rc = dps310_read_pres_raw(data);
+	case IIO_CHAN_INFO_RAW:
+		rc = dps310_read_pressure_value(data, val);
 		if (rc)
 			return rc;
 
-		rc = dps310_calculate_pressure(data, val);
+		return IIO_VAL_INT;
+
+	case IIO_CHAN_INFO_PROCESSED:
+		rc = dps310_read_pressure_value(data, val);
 		if (rc)
 			return rc;
 
 		*val2 = 1000; /* Convert Pa to KPa per IIO ABI */
 		return IIO_VAL_FRACTIONAL;
 
+	case IIO_CHAN_INFO_SCALE:
+		/* The raw value is in Pa, the ABI wants kPa */
+		*val = 1;
+		*val2 = 1000;
+		return IIO_VAL_FRACTIONAL;
+
 	case IIO_CHAN_INFO_OVERSAMPLING_RATIO:
 		rc = dps310_get_pres_precision(data, val);
 		if (rc)
@@ -768,6 +833,21 @@ static int dps310_calculate_temp(struct dps310_data *data, int *val)
 	return 0;
 }
 
+static int dps310_read_temp_value(struct dps310_data *data, int *val)
+{
+	int rc;
+
+	ACQUIRE(mutex_intr, lock)(&data->lock);
+	if (ACQUIRE_ERR(mutex_intr, &lock))
+		return -EINTR;
+
+	rc = dps310_read_temp_raw_locked(data);
+	if (rc)
+		return rc;
+
+	return dps310_calculate_temp(data, val);
+}
+
 static int dps310_read_temp(struct dps310_data *data, int *val, int *val2,
 			    long mask)
 {
@@ -782,11 +862,7 @@ static int dps310_read_temp(struct dps310_data *data, int *val, int *val2,
 		return IIO_VAL_INT;
 
 	case IIO_CHAN_INFO_PROCESSED:
-		rc = dps310_read_temp_raw(data);
-		if (rc)
-			return rc;
-
-		rc = dps310_calculate_temp(data, val);
+		rc = dps310_read_temp_value(data, val);
 		if (rc)
 			return rc;
 
@@ -804,12 +880,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 +896,97 @@ 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);
+
+	switch (mask) {
+	case IIO_CHAN_INFO_RAW:
+	case IIO_CHAN_INFO_PROCESSED: {
+		/*
+		 * Sampling here consumes the same measurement the capture path
+		 * reads, so refuse while the buffer is enabled.
+		 */
+		IIO_DEV_ACQUIRE_DIRECT_MODE(iio, claim);
+		if (IIO_DEV_ACQUIRE_FAILED(claim))
+			return -EBUSY;
+
+		return dps310_read_channel(data, chan, val, val2, mask);
+	}
+	default:
+		return dps310_read_channel(data, chan, val, val2, mask);
+	}
+}
+
+/* Called with lock held */
+static int dps310_fill_scan(struct iio_dev *iio, u8 *buffer)
+{
+	struct dps310_data *data = iio_priv(iio);
+	int rc;
+	int pos = 0;
+	s32 value;
+
+	/*
+	 * The pressure compensation needs a temperature reading, so temperature
+	 * is sampled even when only the pressure channel is enabled.
+	 */
+	rc = dps310_read_temp_raw_locked(data);
+	if (rc)
+		return rc;
+
+	if (test_bit(DPS310_SCAN_TEMP, iio->active_scan_mask)) {
+		rc = dps310_calculate_temp(data, &value);
+		if (rc)
+			return rc;
+
+		/* Millidegrees Celsius */
+		memcpy(&buffer[pos], &value, sizeof(value));
+		pos += sizeof(value);
+	}
+
+	if (test_bit(DPS310_SCAN_PRESSURE, iio->active_scan_mask)) {
+		rc = dps310_read_pres_raw_locked(data);
+		if (rc)
+			return rc;
+
+		rc = dps310_calculate_pressure(data, &value);
+		if (rc)
+			return rc;
+
+		/* Pascals, see the comment on the channel definition */
+		memcpy(&buffer[pos], &value, sizeof(value));
+	}
+
+	return 0;
+}
+
+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);
+	/*
+	 * Either channel can be enabled on its own, so the offset of the second
+	 * value depends on the scan mask and the layout cannot be described
+	 * with a structure. Sized for both 32-bit channels plus the timestamp.
+	 */
+	u8 buffer[16] __aligned(8) = { };
+	int rc = 0;
+
+	scoped_guard(mutex, &data->lock)
+		rc = dps310_fill_scan(iio, buffer);
+
+	if (!rc)
+		iio_push_to_buffers_with_ts(iio, buffer, sizeof(buffer),
+					    pf->timestamp);
+
+	iio_trigger_notify_done(iio->trig);
+
+	return IRQ_HANDLED;
+}
+
 static void dps310_reset(void *action_data)
 {
 	struct dps310_data *data = action_data;
@@ -877,6 +1042,16 @@ static int dps310_probe(struct i2c_client *client)
 	if (rc)
 		return rc;
 
+	/*
+	 * The device measures continuously in background mode, so a capture is
+	 * just a read of the latest results and no buffer setup ops are needed.
+	 */
+	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

[PATCH v5 3/3] iio: pressure: dps310: add hardware FIFO support

[Rupesh Majhi]

The DPS310 has a 32 entry FIFO, shared between the pressure and
temperature streams, which the driver has never used. Enable it for
buffered capture so a reader is woken once per batch of samples rather
than once per sample.

The FIFO is used when no external trigger is attached, and left disabled
in favor of the trigger when one is. That selection needs no policy of
its own: iio_verify_update() already picks INDIO_BUFFER_TRIGGERED when a
trigger is present and INDIO_BUFFER_SOFTWARE when it is not, so both
modes are advertised and the buffer setup ops branch on
iio_device_get_current_mode(). This follows
drivers/iio/pressure/rohm-bm1390.c.

The drain is timer driven rather than interrupt driven. The binding has
no interrupts property and no in-tree device tree wires the INT pin, so
there is no interrupt to use. hwfifo_flush_to_buffer alone is not enough
either: iio_buffer_read() sleeps on rb->pollq with no timeout and only a
push wakes it, so a blocking reader would hang with samples sitting in
the hardware. The flush hook is still provided so poll() and
non-blocking readers can pull early. It stops at the read rather than
after it when the caller limits the count, because entries leave the
hardware as they are read and any collected past the limit would have to
be discarded.

Because the hardware stops recording when the FIFO is full instead of
overwriting, a late drain loses the newest samples rather than the
oldest, so the interval is kept below half the time the FIFO takes to
fill. The DPS310 has no configurable hardware watermark, only a
FIFO-full condition, so the value passed to hwfifo_set_watermark() is
taken as the number of scans the user is prepared to wait for and bounds
the interval from the other side.

Entries carry no timestamps. They are synthesised by working back from
the drain at the configured sample period, anchored so that a batch
never starts before the previous one ended. Where a drain collected more
than the configured rate accounts for, the batch is spread across the
window instead so the timestamps stay monotonic. These are estimates,
not hardware timestamps.

Every entry is read through the pressure registers whichever measurement
produced it, with the type tagged in the LSB and 0x800000 returned once
the FIFO is empty. Pressure entries drive the scans and reuse the most
recent temperature entry for compensation, which keeps the two
configured rates independent; pressure entries arriving before any
temperature cannot be compensated and are dropped. With only the
temperature channel enabled there is nothing to pair with, so
temperature drives the scans itself, and both the drain and the
watermark-to-interval conversion follow whichever rate is driving.

The file header still claimed only a single temperature read was
supported, which this patch is the last word against, so it goes too.

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

diff --git a/drivers/iio/pressure/dps310.c b/drivers/iio/pressure/dps310.c
index 0d6e65766469..0a93c64d6db5 100644
--- a/drivers/iio/pressure/dps310.c
+++ b/drivers/iio/pressure/dps310.c
@@ -2,16 +2,11 @@
 // Copyright IBM Corp 2019
 /*
  * The DPS310 is a barometric pressure and temperature sensor.
- * Currently only reading a single temperature is supported by
- * this driver.
  *
  * https://www.infineon.com/dgdl/?fileId=5546d462576f34750157750826c42242
  *
  * Temperature calculation:
  *   c0 * 0.5 + c1 * T_raw / kT °C
- *
- * TODO:
- *  - Optionally support the FIFO
  */
 
 #include <linux/i2c.h>
@@ -19,6 +14,8 @@
 #include <linux/math64.h>
 #include <linux/module.h>
 #include <linux/regmap.h>
+#include <linux/units.h>
+#include <linux/workqueue.h>
 
 #include <linux/iio/buffer.h>
 #include <linux/iio/iio.h>
@@ -57,9 +54,31 @@
 #define  DPS310_FIFO_EN		BIT(1)
 #define  DPS310_SPI_EN		BIT(0)
 #define DPS310_RESET		0x0c
+#define  DPS310_FIFO_FLUSH	BIT(7)
 #define  DPS310_RESET_MAGIC	0x09
 #define DPS310_COEF_BASE	0x10
 
+/*
+ * Section 4.8: the FIFO holds 32 entries shared between the pressure and
+ * temperature streams, and stops recording once full rather than overwriting.
+ * A late drain therefore loses the newest samples, not the oldest.
+ */
+#define DPS310_FIFO_DEPTH	32
+
+/* Read back in place of a sample once the FIFO has been drained */
+#define DPS310_FIFO_EMPTY_VAL	0x800000
+
+/* The LSB of a FIFO entry tags which measurement produced it */
+#define DPS310_FIFO_TAG_PRS	BIT(0)
+
+/*
+ * Bounds on the drain interval. The lower bound keeps a fast rate from
+ * flooding the workqueue; the upper bound keeps the FIFO from filling while
+ * nothing is looking at it.
+ */
+#define DPS310_DRAIN_MIN_MS	20
+#define DPS310_DRAIN_MAX_MS	2000
+
 /* Make sure sleep time is <= 30ms for usleep_range */
 #define DPS310_POLL_SLEEP_US(t)		min(30000, (t) / 8)
 /* Silently handle error in rate value here */
@@ -93,6 +112,15 @@ struct dps310_data {
 	s32 pressure_raw;
 	s32 temp_raw;
 	bool timeout_recovery_failed;
+
+	/* FIFO capture state, used only while the hardware FIFO is enabled */
+	struct iio_dev *iio;
+	struct delayed_work fifo_work;
+	unsigned int watermark;
+	unsigned int drain_interval_ms;
+	s64 fifo_timestamp;
+	s32 fifo_temp_raw;
+	bool fifo_temp_valid;
 };
 
 enum dps310_scan_index {
@@ -962,6 +990,344 @@ static int dps310_fill_scan(struct iio_dev *iio, u8 *buffer)
 	return 0;
 }
 
+/* Called with lock held */
+static int dps310_fifo_hw_flush(struct dps310_data *data)
+{
+	return regmap_write(data->regmap, DPS310_RESET, DPS310_FIFO_FLUSH);
+}
+
+/* Called with lock held */
+static int dps310_fifo_set_enable(struct dps310_data *data, bool enable)
+{
+	return regmap_write_bits(data->regmap, DPS310_CFG_REG, DPS310_FIFO_EN,
+				 enable ? DPS310_FIFO_EN : 0);
+}
+
+/*
+ * There is no interrupt wired on any in-tree platform and the binding has no
+ * interrupts property, so the FIFO is drained on a timer. The interval has to
+ * stay below the time the FIFO takes to fill, because the hardware stops
+ * recording when full instead of overwriting: draining late loses the newest
+ * samples rather than the oldest.
+ *
+ * Called with lock held.
+ */
+static int dps310_fifo_interval(struct dps310_data *data, unsigned int *ms)
+{
+	bool pressure_enabled = test_bit(DPS310_SCAN_PRESSURE,
+					 data->iio->active_scan_mask);
+	unsigned int fill_ms, want_ms;
+	int rc, prs_rate, tmp_rate;
+
+	rc = dps310_get_pres_samp_freq(data, &prs_rate);
+	if (rc)
+		return rc;
+
+	rc = dps310_get_temp_samp_freq(data, &tmp_rate);
+	if (rc)
+		return rc;
+
+	/* Both streams share the same entries, so they fill it together. */
+	fill_ms = MSEC_PER_SEC * DPS310_FIFO_DEPTH / (prs_rate + tmp_rate);
+
+	/*
+	 * The DPS310 has no configurable hardware watermark, only a FIFO-full
+	 * condition, so the watermark is taken as the number of scans the user
+	 * is prepared to wait for and drives the drain interval instead. Scans
+	 * come at the rate of whichever measurement drives them, which is not
+	 * the pressure rate when only the temperature channel is enabled.
+	 */
+	want_ms = data->watermark * MSEC_PER_SEC /
+		  (pressure_enabled ? prs_rate : tmp_rate);
+
+	*ms = clamp_t(unsigned int, min(want_ms, fill_ms / 2),
+		      DPS310_DRAIN_MIN_MS, DPS310_DRAIN_MAX_MS);
+
+	return 0;
+}
+
+/*
+ * Read a single FIFO entry. Returns 1 if a sample was read, 0 once the FIFO is
+ * empty, or a negative error. Called with lock held.
+ */
+static int dps310_fifo_read_entry(struct dps310_data *data, s32 *value,
+				  bool *is_pressure)
+{
+	u8 val[3];
+	s32 raw;
+	int rc;
+
+	/*
+	 * Every entry is read through the pressure registers regardless of
+	 * which measurement produced it, with the type tagged in the LSB.
+	 */
+	rc = regmap_bulk_read(data->regmap, DPS310_PRS_BASE, val, sizeof(val));
+	if (rc < 0)
+		return rc;
+
+	raw = (val[0] << 16) | (val[1] << 8) | val[2];
+	if (raw == DPS310_FIFO_EMPTY_VAL)
+		return 0;
+
+	*is_pressure = raw & DPS310_FIFO_TAG_PRS;
+	*value = sign_extend32(raw, 23);
+
+	return 1;
+}
+
+/* Called with lock held */
+static int dps310_fifo_push_scan(struct dps310_data *data, s32 temp_raw,
+				 s32 pressure_raw, s64 timestamp)
+{
+	struct iio_dev *iio = data->iio;
+	u8 buffer[16] __aligned(8) = { };
+	int pos = 0, rc;
+	s32 value;
+
+	/*
+	 * The compensation helpers read the cached raw values. Sysfs reads take
+	 * the direct-mode claim, so they cannot be looking at these while a
+	 * buffered capture is running.
+	 */
+	data->temp_raw = temp_raw;
+	data->pressure_raw = pressure_raw;
+
+	if (test_bit(DPS310_SCAN_TEMP, iio->active_scan_mask)) {
+		rc = dps310_calculate_temp(data, &value);
+		if (rc)
+			return rc;
+
+		memcpy(&buffer[pos], &value, sizeof(value));
+		pos += sizeof(value);
+	}
+
+	if (test_bit(DPS310_SCAN_PRESSURE, iio->active_scan_mask)) {
+		rc = dps310_calculate_pressure(data, &value);
+		if (rc)
+			return rc;
+
+		memcpy(&buffer[pos], &value, sizeof(value));
+	}
+
+	iio_push_to_buffers_with_ts(iio, buffer, sizeof(buffer), timestamp);
+
+	return 0;
+}
+
+/*
+ * Drain the FIFO and push the samples it held, stopping once max_scans scans
+ * are in hand or draining everything when max_scans is zero. Stopping at the
+ * read rather than after it matters: entries leave the hardware as they are
+ * read, so any collected beyond the caller's limit would have to be discarded.
+ *
+ * Returns the number of scans pushed. Called with lock held.
+ */
+static int dps310_fifo_drain(struct dps310_data *data, s64 now,
+			     unsigned int max_scans)
+{
+	bool pressure_enabled = test_bit(DPS310_SCAN_PRESSURE,
+					 data->iio->active_scan_mask);
+	bool temp_valid = data->fifo_temp_valid;
+	bool is_pressure[DPS310_FIFO_DEPTH];
+	s32 raw[DPS310_FIFO_DEPTH];
+	unsigned int i, n = 0, scans = 0, pushed = 0;
+	s64 interval, first;
+	int rc, rate;
+
+	/*
+	 * Empty the hardware first and compensate afterwards, so the time spent
+	 * in the polynomial is not time the FIFO spends filling.
+	 *
+	 * Pressure entries drive the scans and reuse the most recent
+	 * temperature, so the two rates stay independent; entries arriving
+	 * before any temperature cannot be compensated and are dropped. With
+	 * only the temperature channel enabled there is nothing to pair with
+	 * and temperature drives the scans itself.
+	 */
+	for (i = 0; i < DPS310_FIFO_DEPTH; i++) {
+		rc = dps310_fifo_read_entry(data, &raw[n], &is_pressure[n]);
+		if (rc < 0)
+			return rc;
+		if (!rc)
+			break;
+
+		if (!is_pressure[n]) {
+			temp_valid = true;
+			if (!pressure_enabled)
+				scans++;
+		} else if (pressure_enabled && temp_valid) {
+			scans++;
+		}
+		n++;
+
+		if (max_scans && scans >= max_scans)
+			break;
+	}
+
+	if (!scans)
+		return 0;
+
+	/*
+	 * FIFO entries carry no timestamps. They are synthesised by working
+	 * back from the drain with the configured period of whichever
+	 * measurement drives the scans, so the spacing matches the sampling
+	 * frequency the user asked for instead of varying with how much each
+	 * drain happened to collect. These are estimates, not hardware
+	 * timestamps.
+	 */
+	rc = pressure_enabled ? dps310_get_pres_samp_freq(data, &rate) :
+				dps310_get_temp_samp_freq(data, &rate);
+	if (rc)
+		return rc;
+
+	interval = div_s64(NSEC_PER_SEC, rate);
+	first = now - (s64)(scans - 1) * interval;
+
+	/*
+	 * A batch must not start before the previous one ended, or the buffer
+	 * would carry timestamps that go backwards. If this drain collected
+	 * more than the configured rate accounts for, spread it across the
+	 * window since the last sample instead.
+	 */
+	if (data->fifo_timestamp && first <= data->fifo_timestamp) {
+		interval = max_t(s64, div_s64(now - data->fifo_timestamp, scans), 1);
+		first = data->fifo_timestamp + interval;
+	}
+
+	for (i = 0; i < n; i++) {
+		s64 timestamp;
+
+		if (!is_pressure[i]) {
+			data->fifo_temp_raw = raw[i];
+			data->fifo_temp_valid = true;
+
+			if (pressure_enabled)
+				continue;
+		} else if (!pressure_enabled || !data->fifo_temp_valid) {
+			continue;
+		}
+
+		timestamp = first + (s64)pushed * interval;
+
+		rc = dps310_fifo_push_scan(data,
+					   is_pressure[i] ? data->fifo_temp_raw
+							  : raw[i],
+					   is_pressure[i] ? raw[i] : 0,
+					   timestamp);
+		if (rc)
+			return rc;
+
+		data->fifo_timestamp = timestamp;
+		pushed++;
+	}
+
+	return pushed;
+}
+
+static void dps310_fifo_work(struct work_struct *work)
+{
+	struct dps310_data *data = container_of(to_delayed_work(work),
+						struct dps310_data, fifo_work);
+	int rc;
+
+	scoped_guard(mutex, &data->lock)
+		rc = dps310_fifo_drain(data, iio_get_time_ns(data->iio), 0);
+
+	if (rc < 0)
+		dev_dbg(&data->client->dev, "FIFO drain failed: %d\n", rc);
+
+	schedule_delayed_work(&data->fifo_work,
+			      msecs_to_jiffies(data->drain_interval_ms));
+}
+
+static int dps310_hwfifo_set_watermark(struct iio_dev *iio, unsigned int val)
+{
+	struct dps310_data *data = iio_priv(iio);
+
+	data->watermark = clamp_t(unsigned int, val, 1, DPS310_FIFO_DEPTH);
+
+	return 0;
+}
+
+static int dps310_hwfifo_flush(struct iio_dev *iio, unsigned int count)
+{
+	struct dps310_data *data = iio_priv(iio);
+	int rc;
+
+	/*
+	 * With a trigger attached the FIFO is left disabled, and the pressure
+	 * registers then hold the latest measurement rather than queued entries
+	 * with an empty marker to stop on. There is nothing to drain.
+	 */
+	if (iio_device_get_current_mode(iio) != INDIO_BUFFER_SOFTWARE)
+		return 0;
+
+	scoped_guard(mutex, &data->lock)
+		rc = dps310_fifo_drain(data, iio_get_time_ns(iio), count);
+
+	return rc;
+}
+
+static int dps310_buffer_postenable(struct iio_dev *iio)
+{
+	struct dps310_data *data = iio_priv(iio);
+	int rc;
+
+	/*
+	 * An attached trigger drives the capture instead, so the FIFO stays
+	 * disabled and the two never both feed the buffer.
+	 */
+	if (iio_device_get_current_mode(iio) == INDIO_BUFFER_TRIGGERED)
+		return 0;
+
+	guard(mutex)(&data->lock);
+
+	data->fifo_temp_valid = false;
+	data->fifo_timestamp = 0;
+
+	rc = dps310_fifo_interval(data, &data->drain_interval_ms);
+	if (rc)
+		return rc;
+
+	/* Drop whatever accumulated before the buffer was enabled */
+	rc = dps310_fifo_hw_flush(data);
+	if (rc)
+		return rc;
+
+	rc = dps310_fifo_set_enable(data, true);
+	if (rc)
+		return rc;
+
+	schedule_delayed_work(&data->fifo_work,
+			      msecs_to_jiffies(data->drain_interval_ms));
+
+	return 0;
+}
+
+static int dps310_buffer_predisable(struct iio_dev *iio)
+{
+	struct dps310_data *data = iio_priv(iio);
+	int rc;
+
+	if (iio_device_get_current_mode(iio) == INDIO_BUFFER_TRIGGERED)
+		return 0;
+
+	cancel_delayed_work_sync(&data->fifo_work);
+
+	guard(mutex)(&data->lock);
+
+	rc = dps310_fifo_set_enable(data, false);
+	if (rc)
+		return rc;
+
+	return dps310_fifo_hw_flush(data);
+}
+
+static const struct iio_buffer_setup_ops dps310_buffer_setup_ops = {
+	.postenable = dps310_buffer_postenable,
+	.predisable = dps310_buffer_predisable,
+};
+
 static irqreturn_t dps310_trigger_handler(int irq, void *p)
 {
 	struct iio_poll_func *pf = p;
@@ -994,6 +1360,17 @@ static void dps310_reset(void *action_data)
 	dps310_reset_wait(data);
 }
 
+/*
+ * The drain reschedules itself, so make sure it is stopped before the device
+ * goes away even if the buffer was never disabled cleanly.
+ */
+static void dps310_cancel_fifo_work(void *action_data)
+{
+	struct dps310_data *data = action_data;
+
+	cancel_delayed_work_sync(&data->fifo_work);
+}
+
 static const struct regmap_config dps310_regmap_config = {
 	.reg_bits = 8,
 	.val_bits = 8,
@@ -1006,6 +1383,8 @@ static const struct regmap_config dps310_regmap_config = {
 static const struct iio_info dps310_info = {
 	.read_raw = dps310_read_raw,
 	.write_raw = dps310_write_raw,
+	.hwfifo_set_watermark = dps310_hwfifo_set_watermark,
+	.hwfifo_flush_to_buffer = dps310_hwfifo_flush,
 };
 
 static int dps310_probe(struct i2c_client *client)
@@ -1021,13 +1400,22 @@ static int dps310_probe(struct i2c_client *client)
 
 	data = iio_priv(iio);
 	data->client = client;
+	data->iio = iio;
+	data->watermark = 1;
 	mutex_init(&data->lock);
+	INIT_DELAYED_WORK(&data->fifo_work, dps310_fifo_work);
 
 	iio->name = id->name;
 	iio->channels = dps310_channels;
 	iio->num_channels = ARRAY_SIZE(dps310_channels);
 	iio->info = &dps310_info;
-	iio->modes = INDIO_DIRECT_MODE;
+	/*
+	 * Both buffer modes are advertised so that iio_verify_update() picks
+	 * INDIO_BUFFER_TRIGGERED when a trigger is attached and falls back to
+	 * INDIO_BUFFER_SOFTWARE, which the FIFO path uses, when one is not.
+	 */
+	iio->modes = INDIO_DIRECT_MODE | INDIO_BUFFER_TRIGGERED |
+		     INDIO_BUFFER_SOFTWARE;
 
 	data->regmap = devm_regmap_init_i2c(client, &dps310_regmap_config);
 	if (IS_ERR(data->regmap))
@@ -1043,12 +1431,19 @@ static int dps310_probe(struct i2c_client *client)
 		return rc;
 
 	/*
-	 * The device measures continuously in background mode, so a capture is
-	 * just a read of the latest results and no buffer setup ops are needed.
+	 * The device measures continuously in background mode, so a triggered
+	 * capture is just a read of the latest results. The setup ops start and
+	 * stop the FIFO drain when no trigger is attached.
 	 */
 	rc = devm_iio_triggered_buffer_setup(&client->dev, iio,
 					     iio_pollfunc_store_time,
-					     dps310_trigger_handler, NULL);
+					     dps310_trigger_handler,
+					     &dps310_buffer_setup_ops);
+	if (rc)
+		return rc;
+
+	rc = devm_add_action_or_reset(&client->dev, dps310_cancel_fifo_work,
+				      data);
 	if (rc)
 		return rc;
 
-- 
2.43.0

Re: [PATCH v5 0/3] iio: pressure: dps310: FIFO and triggered buffer support

[Andy Shevchenko]

On Mon, Aug 17, 2026 at 08:07:22PM +0300, Rupesh Majhi wrote:
> The dps310 has no buffer support today. This series adds it, with the
> hardware FIFO used when no external trigger is attached and the FIFO
> left disabled in favor of the trigger when one is, so the switch between
> the two modes can be reviewed together rather than in two submissions.
> 
> Patch 1 fixes the CFG_REG bit definitions and replaces the standalone
> fix I sent on 27 July, which Jonathan asked me to fold in here instead:
> 
>   Link: https://lore.kernel.org/linux-iio/20260728223009.0cb86996@jic23-huawei/
> 
> All three of those defines have been wrong since the driver was added,
> but only P_SHIFT has a user and only that one misbehaves, so the patch
> carries a Fixes tag for the original driver and one for the commit that
> added the first user of P_SHIFT, along with Cc: stable. The FIFO enable
> is needed by patch 3.
> 
> The three INT_SEL interrupt enables at bits 6 to 4 are still not
> defined. Nothing uses them, the driver has no interrupt path, and the
> binding has no interrupts property, so adding unused defines to a fix
> did not seem worth it. David also asked for the register defines to be
> sorted low to high. That is a cleanup series of its own once this lands.
> 
> Patch 2 adds the triggered buffer path.
> 
> Patch 3 adds the hardware FIFO and the selection between it and an
> attached trigger. Those started out as separate patches, but the branch
> on iio_device_get_current_mode() is four lines and the FIFO patch is
> wrong without it, since postenable would otherwise start the FIFO while
> a trigger was driving the buffer. Splitting them would only have left a
> broken commit in between, so they are one patch.
> 
> Verified on an Infineon DPS310 breakout wired to a BeagleBone Black,
> running this series on 7.2.0-rc2. Two modules built from the same tree,
> differing only in the three CFG_REG defines corrected here, loaded
> seconds apart. Three reads of in_pressure_input per oversampling ratio,
> ambient 98.4 kPa and 27.2 degC:
> 
>   OSR    before              after
>     1    98.433  98.428      98.445  98.446
>     8    98.460  98.460      98.477  98.479
>    16    -ERANGE             98.566  98.564
>    32    -ERANGE             98.428  98.427
>    64    -ERANGE             98.464  98.463
>   128    98.439  98.440      98.434  98.434
> 
> Pressure oversampling 16, 32 and 64 return -ERANGE before the fix.
> P_SHIFT is never enabled, so the result register no longer matches the
> scale factor the compensation divides by, and
> dps310_calculate_pressure() ends up negative. 128 is not affected in
> practice. Temperature is unaffected throughout, since TMP_SHIFT_EN was
> already defined correctly.
> 
> Everything else was checked with checkpatch --strict and a W=1 build,
> plus an arm build for aspeed_g5 and a boot under qemu-system-arm -M
> rainier-bmc, which covers probe, the sysfs values, raw times scale
> matching processed, EBUSY on sysfs reads while the buffer is enabled,
> and all three scan mask combinations. QEMU's dps310 model implements
> neither the FIFO nor the interrupt, so patch 3 was tested on the
> BeagleBone Black above only.
> 
> On hardware, patch 3 was checked with both channels enabled, temperature
> only and pressure only, at 8 Hz and at 128 Hz. A blocking read returns
> in every case, which is the part that needs the timer: with no
> interrupt, hwfifo_flush_to_buffer alone would leave a reader asleep on
> rb->pollq. At 128 Hz, 100 scans arrive in 0.81 s, so the batching is
> real. Timestamps are monotonic throughout and land on the configured
> period, 125.0000 ms at 8 Hz, except where a drain collected more than
> the rate accounts for and the batch is compressed to stay ordered. With
> a sysfs trigger attached the FIFO stays disabled and the trigger drives
> the buffer, at the rate trigger_now is written.

Are the commit messages are written with AI? Please, do it yourself.
They are way too overloaded with unneeded noise and details. Make them
to be straight to the point.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v5 0/3] iio: pressure: dps310: FIFO and triggered buffer support

[Rupert Zoone]

On Mon, Aug 17, 2026 at 09:47 PM +0300, Andy Shevchenko wrote:
> Are the commit messages are written with AI? Please, do it yourself.

Yes, with AI help. I should have tagged it Assisted-by, and v6 will.

> They are way too overloaded with unneeded noise and details. Make them
> to be straight to the point.

Agreed. I have cut them down.

The testing is my own. I wired a DPS310 to a BeagleBone Black,
reproduced the -ERANGE at oversampling 16, 32 and 64 by swapping the
buggy and fixed defines on the same kernel, and exercised the FIFO and
trigger paths on that board.

I will wait for more review before sending v6.

Thanks,
Rupesh


On Mon, Aug 17, 2026 at 9:47 PM Andy Shevchenko
<andriy.shevchenko@intel.com> wrote:
>
> On Mon, Aug 17, 2026 at 08:07:22PM +0300, Rupesh Majhi wrote:
> > The dps310 has no buffer support today. This series adds it, with the
> > hardware FIFO used when no external trigger is attached and the FIFO
> > left disabled in favor of the trigger when one is, so the switch between
> > the two modes can be reviewed together rather than in two submissions.
> >
> > Patch 1 fixes the CFG_REG bit definitions and replaces the standalone
> > fix I sent on 27 July, which Jonathan asked me to fold in here instead:
> >
> >   Link: https://lore.kernel.org/linux-iio/20260728223009.0cb86996@jic23-huawei/
> >
> > All three of those defines have been wrong since the driver was added,
> > but only P_SHIFT has a user and only that one misbehaves, so the patch
> > carries a Fixes tag for the original driver and one for the commit that
> > added the first user of P_SHIFT, along with Cc: stable. The FIFO enable
> > is needed by patch 3.
> >
> > The three INT_SEL interrupt enables at bits 6 to 4 are still not
> > defined. Nothing uses them, the driver has no interrupt path, and the
> > binding has no interrupts property, so adding unused defines to a fix
> > did not seem worth it. David also asked for the register defines to be
> > sorted low to high. That is a cleanup series of its own once this lands.
> >
> > Patch 2 adds the triggered buffer path.
> >
> > Patch 3 adds the hardware FIFO and the selection between it and an
> > attached trigger. Those started out as separate patches, but the branch
> > on iio_device_get_current_mode() is four lines and the FIFO patch is
> > wrong without it, since postenable would otherwise start the FIFO while
> > a trigger was driving the buffer. Splitting them would only have left a
> > broken commit in between, so they are one patch.
> >
> > Verified on an Infineon DPS310 breakout wired to a BeagleBone Black,
> > running this series on 7.2.0-rc2. Two modules built from the same tree,
> > differing only in the three CFG_REG defines corrected here, loaded
> > seconds apart. Three reads of in_pressure_input per oversampling ratio,
> > ambient 98.4 kPa and 27.2 degC:
> >
> >   OSR    before              after
> >     1    98.433  98.428      98.445  98.446
> >     8    98.460  98.460      98.477  98.479
> >    16    -ERANGE             98.566  98.564
> >    32    -ERANGE             98.428  98.427
> >    64    -ERANGE             98.464  98.463
> >   128    98.439  98.440      98.434  98.434
> >
> > Pressure oversampling 16, 32 and 64 return -ERANGE before the fix.
> > P_SHIFT is never enabled, so the result register no longer matches the
> > scale factor the compensation divides by, and
> > dps310_calculate_pressure() ends up negative. 128 is not affected in
> > practice. Temperature is unaffected throughout, since TMP_SHIFT_EN was
> > already defined correctly.
> >
> > Everything else was checked with checkpatch --strict and a W=1 build,
> > plus an arm build for aspeed_g5 and a boot under qemu-system-arm -M
> > rainier-bmc, which covers probe, the sysfs values, raw times scale
> > matching processed, EBUSY on sysfs reads while the buffer is enabled,
> > and all three scan mask combinations. QEMU's dps310 model implements
> > neither the FIFO nor the interrupt, so patch 3 was tested on the
> > BeagleBone Black above only.
> >
> > On hardware, patch 3 was checked with both channels enabled, temperature
> > only and pressure only, at 8 Hz and at 128 Hz. A blocking read returns
> > in every case, which is the part that needs the timer: with no
> > interrupt, hwfifo_flush_to_buffer alone would leave a reader asleep on
> > rb->pollq. At 128 Hz, 100 scans arrive in 0.81 s, so the batching is
> > real. Timestamps are monotonic throughout and land on the configured
> > period, 125.0000 ms at 8 Hz, except where a drain collected more than
> > the rate accounts for and the batch is compressed to stay ordered. With
> > a sysfs trigger attached the FIFO stays disabled and the trigger drives
> > the buffer, at the rate trigger_now is written.
>
> Are the commit messages are written with AI? Please, do it yourself.
> They are way too overloaded with unneeded noise and details. Make them
> to be straight to the point.
>
> --
> With Best Regards,
> Andy Shevchenko
>
>