[PATCH v2 00/13] iio: pressure: mprls0025pa: driver code cleanup

[PATCH v2 00/13] iio: pressure: mprls0025pa: driver code cleanup

[Petre Rodan]

This series contains a collection of patches to the MPR sensor based
on feedback I received for other drivers.

major changes:
 - trigger flag fix (define edge direction only in the device tree)
 - fix SPI timing violation
 - fix scan_type struct
 - fix pressure calculation
    (does not affect users that define a sensor via the pressure-triplet)
 - stricter check for the status byte + better error return levels
 - drop the use of devm_kzalloc()
 - stick to the datasheet parameters while performing the measurement
    sequence

minor changes:
 - includes added and removed
 - rename generic 'buffer' variable to 'rx_buf'

I still included the memset patch for the reasons described in the v1 thread.
if you strongly consider that patch inadequate then please skip it.

Tested on two sensors - MPRLS0015PA0000SA and MPRLS0001BA00001A

Datasheet: https://prod-edam.honeywell.com/content/dam/honeywell-edam/sps/siot/en-us/products/sensors/pressure-sensors/board-mount-pressure-sensors/micropressure-mpr-series/documents/sps-siot-mpr-series-datasheet-32332628-ciid-172626.pdf?download=false
Signed-off-by: Petre Rodan <petre.rodan@subdimension.ro>
---
Changes in v2:
- reordered patches based on Marcelo's input
- implemented code changes based on Marcelo and Jonathan's reviews
- Link to v1: https://lore.kernel.org/r/20251218-mprls_cleanup-v1-0-b36a170f1a5c@subdimension.ro

---
Petre Rodan (13):
      iio: pressure: mprls0025pa: fix spi_transfer struct initialisation
      iio: pressure: mprls0025pa: fix SPI CS delay violation
      iio: pressure: mprls0025pa: fix interrupt flag
      iio: pressure: mprls0025pa: fix scan_type struct
      iio: pressure: mprls0025pa: fix pressure calculation
      iio: pressure: mprls0025pa: cleanup includes
      iio: pressure: mprls0025pa: remove redundant declarations
      iio: pressure: mprls0025pa: rename buffer variable
      iio: pressure: mprls0025pa: introduce tx buffer
      iio: pressure: mprls0025pa: move memset to core
      iio: pressure: mprls0025pa: stricter checks for the status byte
      iio: pressure: mprls0025pa: change measurement sequence
      iio: pressure: mprls0025pa: add copyright line

 drivers/iio/pressure/mprls0025pa.c     | 112 +++++++++++++++------------------
 drivers/iio/pressure/mprls0025pa.h     |  15 ++---
 drivers/iio/pressure/mprls0025pa_i2c.c |  13 +---
 drivers/iio/pressure/mprls0025pa_spi.c |  41 +++++-------
 4 files changed, 73 insertions(+), 108 deletions(-)
---
base-commit: f9e05791642810a0cf6237d39fafd6fec5e0b4bb
change-id: 20251215-mprls_cleanup-01de8971b439

Best regards,
-- 
Petre Rodan <petre.rodan@subdimension.ro>

[PATCH v2 01/13] iio: pressure: mprls0025pa: fix spi_transfer struct initialisation

[Petre Rodan]

Make sure that the spi_transfer struct is zeroed out before use.

Fixes: a0858f0cd28e ("iio: pressure: mprls0025pa add SPI driver")
Signed-off-by: Petre Rodan <petre.rodan@subdimension.ro>
---
v1 -> v2 no change
---
 drivers/iio/pressure/mprls0025pa_spi.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/iio/pressure/mprls0025pa_spi.c b/drivers/iio/pressure/mprls0025pa_spi.c
index d04102f8a4a0..e6bb75de3411 100644
--- a/drivers/iio/pressure/mprls0025pa_spi.c
+++ b/drivers/iio/pressure/mprls0025pa_spi.c
@@ -40,7 +40,7 @@ static int mpr_spi_xfer(struct mpr_data *data, const u8 cmd, const u8 pkt_len)
 {
 	struct spi_device *spi = to_spi_device(data->dev);
 	struct mpr_spi_buf *buf = spi_get_drvdata(spi);
-	struct spi_transfer xfer;
+	struct spi_transfer xfer = { };
 
 	if (pkt_len > MPR_MEASUREMENT_RD_SIZE)
 		return -EOVERFLOW;

-- 
2.52.0

[PATCH v2 02/13] iio: pressure: mprls0025pa: fix SPI CS delay violation

[Petre Rodan]

Based on the sensor datasheet in chapter 7.6 SPI timing, Table 20,
during the SPI transfer there is a minimum time interval requirement
between the CS being asserted and the first clock edge (tHDSS).
This minimum interval of 2.5us is being violated if two consecutive SPI
transfers are queued up.

Fixes: a0858f0cd28e ("iio: pressure: mprls0025pa add SPI driver")
Datasheet: https://prod-edam.honeywell.com/content/dam/honeywell-edam/sps/siot/en-us/products/sensors/pressure-sensors/board-mount-pressure-sensors/micropressure-mpr-series/documents/sps-siot-mpr-series-datasheet-32332628-ciid-172626.pdf?download=false
Signed-off-by: Petre Rodan <petre.rodan@subdimension.ro>
---
v1 -> v2 use xfer.delay.value as per Marcelo Schmitt's review
---
 drivers/iio/pressure/mprls0025pa_spi.c | 19 ++++++++++++++-----
 1 file changed, 14 insertions(+), 5 deletions(-)

diff --git a/drivers/iio/pressure/mprls0025pa_spi.c b/drivers/iio/pressure/mprls0025pa_spi.c
index e6bb75de3411..cf17eb2e7208 100644
--- a/drivers/iio/pressure/mprls0025pa_spi.c
+++ b/drivers/iio/pressure/mprls0025pa_spi.c
@@ -8,6 +8,7 @@
  *  https://prod-edam.honeywell.com/content/dam/honeywell-edam/sps/siot/en-us/products/sensors/pressure-sensors/board-mount-pressure-sensors/micropressure-mpr-series/documents/sps-siot-mpr-series-datasheet-32332628-ciid-172626.pdf
  */
 
+#include <linux/array_size.h>
 #include <linux/device.h>
 #include <linux/errno.h>
 #include <linux/mod_devicetable.h>
@@ -40,17 +41,25 @@ static int mpr_spi_xfer(struct mpr_data *data, const u8 cmd, const u8 pkt_len)
 {
 	struct spi_device *spi = to_spi_device(data->dev);
 	struct mpr_spi_buf *buf = spi_get_drvdata(spi);
-	struct spi_transfer xfer = { };
+	struct spi_transfer xfers[2] = { };
 
 	if (pkt_len > MPR_MEASUREMENT_RD_SIZE)
 		return -EOVERFLOW;
 
 	buf->tx[0] = cmd;
-	xfer.tx_buf = buf->tx;
-	xfer.rx_buf = data->buffer;
-	xfer.len = pkt_len;
 
-	return spi_sync_transfer(spi, &xfer, 1);
+	/*
+	 * Dummy transfer with no data, just cause a 2.5us+ delay between the CS assert
+	 * and the first clock edge as per the datasheet tHDSS timing requirement.
+	 */
+	xfers[0].delay.value = 2500;
+	xfers[0].delay.unit = SPI_DELAY_UNIT_NSECS;
+
+	xfers[1].tx_buf = buf->tx;
+	xfers[1].rx_buf = data->buffer;
+	xfers[1].len = pkt_len;
+
+	return spi_sync_transfer(spi, xfers, ARRAY_SIZE(xfers));
 }
 
 static const struct mpr_ops mpr_spi_ops = {

-- 
2.52.0

[PATCH v2 03/13] iio: pressure: mprls0025pa: fix interrupt flag

[Petre Rodan]

Interrupt falling/rising flags should only be defined in the device tree.

Fixes: 713337d9143e ("iio: pressure: Honeywell mprls0025pa pressure sensor")
Signed-off-by: Petre Rodan <petre.rodan@subdimension.ro>
---
v1 -> v2 use IRQF_TRIGGER_NONE as Jonathan requested
---
 drivers/iio/pressure/mprls0025pa.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/iio/pressure/mprls0025pa.c b/drivers/iio/pressure/mprls0025pa.c
index 2336f2760eae..80985c090aff 100644
--- a/drivers/iio/pressure/mprls0025pa.c
+++ b/drivers/iio/pressure/mprls0025pa.c
@@ -12,6 +12,7 @@
 #include <linux/array_size.h>
 #include <linux/bitfield.h>
 #include <linux/bits.h>
+#include <linux/interrupt.h>
 #include <linux/math64.h>
 #include <linux/mod_devicetable.h>
 #include <linux/module.h>
@@ -419,9 +420,7 @@ int mpr_common_probe(struct device *dev, const struct mpr_ops *ops, int irq)
 
 	if (data->irq > 0) {
 		ret = devm_request_irq(dev, data->irq, mpr_eoc_handler,
-				       IRQF_TRIGGER_RISING,
-				       dev_name(dev),
-				       data);
+				       IRQF_TRIGGER_NONE, dev_name(dev), data);
 		if (ret)
 			return dev_err_probe(dev, ret,
 					  "request irq %d failed\n", data->irq);

-- 
2.52.0

[PATCH v2 04/13] iio: pressure: mprls0025pa: fix scan_type struct

[Petre Rodan]

Fix the scan_type sign and realbits assignment.

The pressure is a 24bit unsigned int between output_min and output_max.

 transfer function A: 10%   to 90%   of 2^24
 transfer function B:  2.5% to 22.5% of 2^24
 transfer function C: 20%   to 80%   of 2^24
[MPR_FUNCTION_A] = { .output_min = 1677722, .output_max = 15099494 }
[MPR_FUNCTION_B] = { .output_min =  419430, .output_max =  3774874 }
[MPR_FUNCTION_C] = { .output_min = 3355443, .output_max = 13421773 }

Fixes: 713337d9143e ("iio: pressure: Honeywell mprls0025pa pressure sensor")
Signed-off-by: Petre Rodan <petre.rodan@subdimension.ro>
---
v1 -> v2 no change
---
 drivers/iio/pressure/mprls0025pa.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/iio/pressure/mprls0025pa.c b/drivers/iio/pressure/mprls0025pa.c
index 80985c090aff..cec0bb3fc16f 100644
--- a/drivers/iio/pressure/mprls0025pa.c
+++ b/drivers/iio/pressure/mprls0025pa.c
@@ -161,8 +161,8 @@ static const struct iio_chan_spec mpr_channels[] = {
 					BIT(IIO_CHAN_INFO_OFFSET),
 		.scan_index = 0,
 		.scan_type = {
-			.sign = 's',
-			.realbits = 32,
+			.sign = 'u',
+			.realbits = 24,
 			.storagebits = 32,
 			.endianness = IIO_CPU,
 		},

-- 
2.52.0

[PATCH v2 05/13] iio: pressure: mprls0025pa: fix pressure calculation

[Petre Rodan]

A sign change is needed for proper calculation of the pressure.

This is a minor fix since it only affects users that might have custom
silicon from Honeywell that has honeywell,pmin-pascal != 0.

Also due to the fact that raw pressure values can not be lower
than output_min (400k-3.3M) there is no need to calculate a decimal for
the offset.

Fixes: 713337d9143e ("iio: pressure: Honeywell mprls0025pa pressure sensor")
Signed-off-by: Petre Rodan <petre.rodan@subdimension.ro>
---
v1 -> v2 slightly changed commit message, added fixes tag
---
 drivers/iio/pressure/mprls0025pa.c | 26 +++++++++++---------------
 drivers/iio/pressure/mprls0025pa.h |  2 --
 2 files changed, 11 insertions(+), 17 deletions(-)

diff --git a/drivers/iio/pressure/mprls0025pa.c b/drivers/iio/pressure/mprls0025pa.c
index cec0bb3fc16f..2142df7932e8 100644
--- a/drivers/iio/pressure/mprls0025pa.c
+++ b/drivers/iio/pressure/mprls0025pa.c
@@ -60,7 +60,7 @@
  *
  * Values given to the userspace in sysfs interface:
  * * raw	- press_cnt
- * * offset	- (-1 * outputmin) - pmin / scale
+ * * offset	- (-1 * outputmin) + pmin / scale
  *                note: With all sensors from the datasheet pmin = 0
  *                which reduces the offset to (-1 * outputmin)
  */
@@ -314,8 +314,7 @@ static int mpr_read_raw(struct iio_dev *indio_dev,
 		return IIO_VAL_INT_PLUS_NANO;
 	case IIO_CHAN_INFO_OFFSET:
 		*val = data->offset;
-		*val2 = data->offset2;
-		return IIO_VAL_INT_PLUS_NANO;
+		return IIO_VAL_INT;
 	default:
 		return -EINVAL;
 	}
@@ -331,8 +330,9 @@ int mpr_common_probe(struct device *dev, const struct mpr_ops *ops, int irq)
 	struct mpr_data *data;
 	struct iio_dev *indio_dev;
 	const char *triplet;
-	s64 scale, offset;
 	u32 func;
+	s32 tmp;
+	s64 odelta, pdelta;
 
 	indio_dev = devm_iio_device_alloc(dev, sizeof(*data));
 	if (!indio_dev)
@@ -406,17 +406,13 @@ int mpr_common_probe(struct device *dev, const struct mpr_ops *ops, int irq)
 	data->outmin = mpr_func_spec[data->function].output_min;
 	data->outmax = mpr_func_spec[data->function].output_max;
 
-	/* use 64 bit calculation for preserving a reasonable precision */
-	scale = div_s64(((s64)(data->pmax - data->pmin)) * NANO,
-			data->outmax - data->outmin);
-	data->scale = div_s64_rem(scale, NANO, &data->scale2);
-	/*
-	 * multiply with NANO before dividing by scale and later divide by NANO
-	 * again.
-	 */
-	offset = ((-1LL) * (s64)data->outmin) * NANO -
-		  div_s64(div_s64((s64)data->pmin * NANO, scale), NANO);
-	data->offset = div_s64_rem(offset, NANO, &data->offset2);
+	odelta = data->outmax - data->outmin;
+	pdelta = data->pmax - data->pmin;
+
+	data->scale = div_s64_rem(div_s64(pdelta * NANO, odelta), NANO, &tmp);
+	data->scale2 = tmp;
+
+	data->offset = div_s64(odelta * data->pmin, pdelta) - data->outmin;
 
 	if (data->irq > 0) {
 		ret = devm_request_irq(dev, data->irq, mpr_eoc_handler,
diff --git a/drivers/iio/pressure/mprls0025pa.h b/drivers/iio/pressure/mprls0025pa.h
index d62a018eaff3..b6944b305126 100644
--- a/drivers/iio/pressure/mprls0025pa.h
+++ b/drivers/iio/pressure/mprls0025pa.h
@@ -53,7 +53,6 @@ enum mpr_func_id {
  * @scale: pressure scale
  * @scale2: pressure scale, decimal number
  * @offset: pressure offset
- * @offset2: pressure offset, decimal number
  * @gpiod_reset: reset
  * @irq: end of conversion irq. used to distinguish between irq mode and
  *       reading in a loop until data is ready
@@ -75,7 +74,6 @@ struct mpr_data {
 	int			scale;
 	int			scale2;
 	int			offset;
-	int			offset2;
 	struct gpio_desc	*gpiod_reset;
 	int			irq;
 	struct completion	completion;

-- 
2.52.0

[PATCH v2 06/13] iio: pressure: mprls0025pa: cleanup includes

[Petre Rodan]

Remove unused headers and add required headers as needed.

Signed-off-by: Petre Rodan <petre.rodan@subdimension.ro>
---
v1->v2 split based on Marcelo's request
---
 drivers/iio/pressure/mprls0025pa.c | 6 ++++++
 drivers/iio/pressure/mprls0025pa.h | 3 ---
 2 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/iio/pressure/mprls0025pa.c b/drivers/iio/pressure/mprls0025pa.c
index 2142df7932e8..b0f70c9b503c 100644
--- a/drivers/iio/pressure/mprls0025pa.c
+++ b/drivers/iio/pressure/mprls0025pa.c
@@ -12,7 +12,12 @@
 #include <linux/array_size.h>
 #include <linux/bitfield.h>
 #include <linux/bits.h>
+#include <linux/completion.h>
+#include <linux/delay.h>
+#include <linux/errno.h>
+#include <linux/export.h>
 #include <linux/interrupt.h>
+#include <linux/jiffies.h>
 #include <linux/math64.h>
 #include <linux/mod_devicetable.h>
 #include <linux/module.h>
@@ -22,6 +27,7 @@
 #include <linux/gpio/consumer.h>
 
 #include <linux/iio/buffer.h>
+#include <linux/iio/iio.h>
 #include <linux/iio/trigger_consumer.h>
 #include <linux/iio/triggered_buffer.h>
 
diff --git a/drivers/iio/pressure/mprls0025pa.h b/drivers/iio/pressure/mprls0025pa.h
index b6944b305126..eab877da3451 100644
--- a/drivers/iio/pressure/mprls0025pa.h
+++ b/drivers/iio/pressure/mprls0025pa.h
@@ -12,10 +12,7 @@
 #define _MPRLS0025PA_H
 
 #include <linux/completion.h>
-#include <linux/delay.h>
-#include <linux/device.h>
 #include <linux/mutex.h>
-#include <linux/stddef.h>
 #include <linux/types.h>
 
 #include <linux/iio/iio.h>

-- 
2.52.0

[PATCH v2 07/13] iio: pressure: mprls0025pa: remove redundant declarations

[Petre Rodan]

Remove the iio_chan_spec and iio_dev structs which are already defined in
the included iio.h header.

Signed-off-by: Petre Rodan <petre.rodan@subdimension.ro>
---
v1 -> v2 split based on Marcelo's request
---
 drivers/iio/pressure/mprls0025pa.h | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/iio/pressure/mprls0025pa.h b/drivers/iio/pressure/mprls0025pa.h
index eab877da3451..e34253af8e23 100644
--- a/drivers/iio/pressure/mprls0025pa.h
+++ b/drivers/iio/pressure/mprls0025pa.h
@@ -25,9 +25,6 @@
 
 struct device;
 
-struct iio_chan_spec;
-struct iio_dev;
-
 struct mpr_data;
 struct mpr_ops;
 

-- 
2.52.0

[PATCH v2 08/13] iio: pressure: mprls0025pa: rename buffer variable

[Petre Rodan]

For the reason of better naming consistency rename priv->buffer into
priv->rx_buf since tx_buf will get introduced in the next patch.

Signed-off-by: Petre Rodan <petre.rodan@subdimension.ro>
---
v1 -> v2 no functional change
---
 drivers/iio/pressure/mprls0025pa.c     | 10 +++++-----
 drivers/iio/pressure/mprls0025pa.h     |  4 ++--
 drivers/iio/pressure/mprls0025pa_i2c.c |  4 ++--
 drivers/iio/pressure/mprls0025pa_spi.c |  2 +-
 4 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/drivers/iio/pressure/mprls0025pa.c b/drivers/iio/pressure/mprls0025pa.c
index b0f70c9b503c..fd9c8ea61eb3 100644
--- a/drivers/iio/pressure/mprls0025pa.c
+++ b/drivers/iio/pressure/mprls0025pa.c
@@ -238,7 +238,7 @@ static int mpr_read_pressure(struct mpr_data *data, s32 *press)
 					ret);
 				return ret;
 			}
-			if (!(data->buffer[0] & MPR_ST_ERR_FLAG))
+			if (!(data->rx_buf[0] & MPR_ST_ERR_FLAG))
 				break;
 		}
 		if (i == nloops) {
@@ -251,15 +251,15 @@ static int mpr_read_pressure(struct mpr_data *data, s32 *press)
 	if (ret < 0)
 		return ret;
 
-	if (data->buffer[0] & MPR_ST_ERR_FLAG) {
+	if (data->rx_buf[0] & MPR_ST_ERR_FLAG) {
 		dev_err(data->dev,
-			"unexpected status byte %02x\n", data->buffer[0]);
+			"unexpected status byte %02x\n", data->rx_buf[0]);
 		return -ETIMEDOUT;
 	}
 
-	*press = get_unaligned_be24(&data->buffer[1]);
+	*press = get_unaligned_be24(&data->rx_buf[1]);
 
-	dev_dbg(dev, "received: %*ph cnt: %d\n", ret, data->buffer, *press);
+	dev_dbg(dev, "received: %*ph cnt: %d\n", ret, data->rx_buf, *press);
 
 	return 0;
 }
diff --git a/drivers/iio/pressure/mprls0025pa.h b/drivers/iio/pressure/mprls0025pa.h
index e34253af8e23..119ebb0ba567 100644
--- a/drivers/iio/pressure/mprls0025pa.h
+++ b/drivers/iio/pressure/mprls0025pa.h
@@ -54,7 +54,7 @@ enum mpr_func_id {
  * @chan: channel values for buffered mode
  * @chan.pres: pressure value
  * @chan.ts: timestamp
- * @buffer: raw conversion data
+ * @rx_buf: raw conversion data
  */
 struct mpr_data {
 	struct device		*dev;
@@ -75,7 +75,7 @@ struct mpr_data {
 		s32 pres;
 		aligned_s64 ts;
 	} chan;
-	u8	    buffer[MPR_MEASUREMENT_RD_SIZE] __aligned(IIO_DMA_MINALIGN);
+	u8 rx_buf[MPR_MEASUREMENT_RD_SIZE] __aligned(IIO_DMA_MINALIGN);
 };
 
 struct mpr_ops {
diff --git a/drivers/iio/pressure/mprls0025pa_i2c.c b/drivers/iio/pressure/mprls0025pa_i2c.c
index 79811fd4a02b..36da0059b19f 100644
--- a/drivers/iio/pressure/mprls0025pa_i2c.c
+++ b/drivers/iio/pressure/mprls0025pa_i2c.c
@@ -30,8 +30,8 @@ static int mpr_i2c_read(struct mpr_data *data, const u8 unused, const u8 cnt)
 	if (cnt > MPR_MEASUREMENT_RD_SIZE)
 		return -EOVERFLOW;
 
-	memset(data->buffer, 0, MPR_MEASUREMENT_RD_SIZE);
-	ret = i2c_master_recv(client, data->buffer, cnt);
+	memset(data->rx_buf, 0, MPR_MEASUREMENT_RD_SIZE);
+	ret = i2c_master_recv(client, data->rx_buf, cnt);
 	if (ret < 0)
 		return ret;
 	else if (ret != cnt)
diff --git a/drivers/iio/pressure/mprls0025pa_spi.c b/drivers/iio/pressure/mprls0025pa_spi.c
index cf17eb2e7208..247b65226bb9 100644
--- a/drivers/iio/pressure/mprls0025pa_spi.c
+++ b/drivers/iio/pressure/mprls0025pa_spi.c
@@ -56,7 +56,7 @@ static int mpr_spi_xfer(struct mpr_data *data, const u8 cmd, const u8 pkt_len)
 	xfers[0].delay.unit = SPI_DELAY_UNIT_NSECS;
 
 	xfers[1].tx_buf = buf->tx;
-	xfers[1].rx_buf = data->buffer;
+	xfers[1].rx_buf = data->rx_buf;
 	xfers[1].len = pkt_len;
 
 	return spi_sync_transfer(spi, xfers, ARRAY_SIZE(xfers));

-- 
2.52.0

[PATCH v2 09/13] iio: pressure: mprls0025pa: introduce tx buffer

[Petre Rodan]

Use a tx_buf that is part of the priv struct for transferring data to
the sensor instead of relying on a devm_kzalloc()-ed array.
Remove the .init operation in the process.

Reviewed-by: Marcelo Schmitt <marcelo.schmitt1@gmail.com>
Signed-off-by: Petre Rodan <petre.rodan@subdimension.ro>
---
v1 -> v2 no functional change
---
 drivers/iio/pressure/mprls0025pa.c     |  4 ----
 drivers/iio/pressure/mprls0025pa.h     |  3 ++-
 drivers/iio/pressure/mprls0025pa_i2c.c | 10 ++--------
 drivers/iio/pressure/mprls0025pa_spi.c | 24 ++----------------------
 4 files changed, 6 insertions(+), 35 deletions(-)

diff --git a/drivers/iio/pressure/mprls0025pa.c b/drivers/iio/pressure/mprls0025pa.c
index fd9c8ea61eb3..7532b2e74413 100644
--- a/drivers/iio/pressure/mprls0025pa.c
+++ b/drivers/iio/pressure/mprls0025pa.c
@@ -363,10 +363,6 @@ int mpr_common_probe(struct device *dev, const struct mpr_ops *ops, int irq)
 		return dev_err_probe(dev, ret,
 				     "can't get and enable vdd supply\n");
 
-	ret = data->ops->init(data->dev);
-	if (ret)
-		return ret;
-
 	ret = device_property_read_u32(dev,
 				       "honeywell,transfer-function", &func);
 	if (ret)
diff --git a/drivers/iio/pressure/mprls0025pa.h b/drivers/iio/pressure/mprls0025pa.h
index 119ebb0ba567..9f43273e635f 100644
--- a/drivers/iio/pressure/mprls0025pa.h
+++ b/drivers/iio/pressure/mprls0025pa.h
@@ -55,6 +55,7 @@ enum mpr_func_id {
  * @chan.pres: pressure value
  * @chan.ts: timestamp
  * @rx_buf: raw conversion data
+ * @tx_buf: output buffer
  */
 struct mpr_data {
 	struct device		*dev;
@@ -76,10 +77,10 @@ struct mpr_data {
 		aligned_s64 ts;
 	} chan;
 	u8 rx_buf[MPR_MEASUREMENT_RD_SIZE] __aligned(IIO_DMA_MINALIGN);
+	u8 tx_buf[MPR_MEASUREMENT_RD_SIZE];
 };
 
 struct mpr_ops {
-	int (*init)(struct device *dev);
 	int (*read)(struct mpr_data *data, const u8 cmd, const u8 cnt);
 	int (*write)(struct mpr_data *data, const u8 cmd, const u8 cnt);
 };
diff --git a/drivers/iio/pressure/mprls0025pa_i2c.c b/drivers/iio/pressure/mprls0025pa_i2c.c
index 36da0059b19f..a0bbc6af9283 100644
--- a/drivers/iio/pressure/mprls0025pa_i2c.c
+++ b/drivers/iio/pressure/mprls0025pa_i2c.c
@@ -17,11 +17,6 @@
 
 #include "mprls0025pa.h"
 
-static int mpr_i2c_init(struct device *unused)
-{
-	return 0;
-}
-
 static int mpr_i2c_read(struct mpr_data *data, const u8 unused, const u8 cnt)
 {
 	int ret;
@@ -44,9 +39,9 @@ static int mpr_i2c_write(struct mpr_data *data, const u8 cmd, const u8 unused)
 {
 	int ret;
 	struct i2c_client *client = to_i2c_client(data->dev);
-	u8 wdata[MPR_PKT_SYNC_LEN] = { cmd };
 
-	ret = i2c_master_send(client, wdata, MPR_PKT_SYNC_LEN);
+	data->tx_buf[0] = cmd;
+	ret = i2c_master_send(client, data->tx_buf, MPR_PKT_SYNC_LEN);
 	if (ret < 0)
 		return ret;
 	else if (ret != MPR_PKT_SYNC_LEN)
@@ -56,7 +51,6 @@ static int mpr_i2c_write(struct mpr_data *data, const u8 cmd, const u8 unused)
 }
 
 static const struct mpr_ops mpr_i2c_ops = {
-	.init = mpr_i2c_init,
 	.read = mpr_i2c_read,
 	.write = mpr_i2c_write,
 };
diff --git a/drivers/iio/pressure/mprls0025pa_spi.c b/drivers/iio/pressure/mprls0025pa_spi.c
index 247b65226bb9..8c8c726f703f 100644
--- a/drivers/iio/pressure/mprls0025pa_spi.c
+++ b/drivers/iio/pressure/mprls0025pa_spi.c
@@ -19,34 +19,15 @@
 
 #include "mprls0025pa.h"
 
-struct mpr_spi_buf {
-	u8 tx[MPR_MEASUREMENT_RD_SIZE] __aligned(IIO_DMA_MINALIGN);
-};
-
-static int mpr_spi_init(struct device *dev)
-{
-	struct spi_device *spi = to_spi_device(dev);
-	struct mpr_spi_buf *buf;
-
-	buf = devm_kzalloc(dev, sizeof(*buf), GFP_KERNEL);
-	if (!buf)
-		return -ENOMEM;
-
-	spi_set_drvdata(spi, buf);
-
-	return 0;
-}
-
 static int mpr_spi_xfer(struct mpr_data *data, const u8 cmd, const u8 pkt_len)
 {
 	struct spi_device *spi = to_spi_device(data->dev);
-	struct mpr_spi_buf *buf = spi_get_drvdata(spi);
 	struct spi_transfer xfers[2] = { };
 
 	if (pkt_len > MPR_MEASUREMENT_RD_SIZE)
 		return -EOVERFLOW;
 
-	buf->tx[0] = cmd;
+	data->tx_buf[0] = cmd;
 
 	/*
 	 * Dummy transfer with no data, just cause a 2.5us+ delay between the CS assert
@@ -55,7 +36,7 @@ static int mpr_spi_xfer(struct mpr_data *data, const u8 cmd, const u8 pkt_len)
 	xfers[0].delay.value = 2500;
 	xfers[0].delay.unit = SPI_DELAY_UNIT_NSECS;
 
-	xfers[1].tx_buf = buf->tx;
+	xfers[1].tx_buf = data->tx_buf;
 	xfers[1].rx_buf = data->rx_buf;
 	xfers[1].len = pkt_len;
 
@@ -63,7 +44,6 @@ static int mpr_spi_xfer(struct mpr_data *data, const u8 cmd, const u8 pkt_len)
 }
 
 static const struct mpr_ops mpr_spi_ops = {
-	.init = mpr_spi_init,
 	.read = mpr_spi_xfer,
 	.write = mpr_spi_xfer,
 };

-- 
2.52.0

[PATCH v2 10/13] iio: pressure: mprls0025pa: move memset to core

[Petre Rodan]

Move memset() from the bus specific code into core.

Zeroing out the buffer is performed because the sensor has noticeable
latch-up sensitivity and in some cases it clamps the MISO signal to GND
in sync with SCLK [1]. A raw conversion of zero is out of bounds since
valid values have to be between output_min and output_max (and the
smallest output_min is 2.5% of 2^24 = 419430).

The user is expected to discard out of bounds pressure values.

Given the fact that we can't follow the behaviour of all SPI controllers
when faced to this clamping of an output signal, a raw conversion of zero
is used as an early warning in case the low level SPI API reacts
unexpectedly.

Link: https://e2e.ti.com/support/processors-group/processors/f/processors-forum/1588325/am3358-spi-tx-data-corruption [1]
Signed-off-by: Petre Rodan <petre.rodan@subdimension.ro>
---
v1 -> v2 no change
---
 drivers/iio/pressure/mprls0025pa.c     | 2 ++
 drivers/iio/pressure/mprls0025pa_i2c.c | 1 -
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/iio/pressure/mprls0025pa.c b/drivers/iio/pressure/mprls0025pa.c
index 7532b2e74413..e4651d3f686d 100644
--- a/drivers/iio/pressure/mprls0025pa.c
+++ b/drivers/iio/pressure/mprls0025pa.c
@@ -22,6 +22,7 @@
 #include <linux/mod_devicetable.h>
 #include <linux/module.h>
 #include <linux/property.h>
+#include <linux/string.h>
 #include <linux/units.h>
 
 #include <linux/gpio/consumer.h>
@@ -247,6 +248,7 @@ static int mpr_read_pressure(struct mpr_data *data, s32 *press)
 		}
 	}
 
+	memset(data->rx_buf, 0, sizeof(data->rx_buf));
 	ret = data->ops->read(data, MPR_CMD_NOP, MPR_PKT_NOP_LEN);
 	if (ret < 0)
 		return ret;
diff --git a/drivers/iio/pressure/mprls0025pa_i2c.c b/drivers/iio/pressure/mprls0025pa_i2c.c
index a0bbc6af9283..0fe8cfe0d7e7 100644
--- a/drivers/iio/pressure/mprls0025pa_i2c.c
+++ b/drivers/iio/pressure/mprls0025pa_i2c.c
@@ -25,7 +25,6 @@ static int mpr_i2c_read(struct mpr_data *data, const u8 unused, const u8 cnt)
 	if (cnt > MPR_MEASUREMENT_RD_SIZE)
 		return -EOVERFLOW;
 
-	memset(data->rx_buf, 0, MPR_MEASUREMENT_RD_SIZE);
 	ret = i2c_master_recv(client, data->rx_buf, cnt);
 	if (ret < 0)
 		return ret;

-- 
2.52.0

[PATCH v2 11/13] iio: pressure: mprls0025pa: stricter checks for the status byte

[Petre Rodan]

Make sure a valid conversion comes with a status byte that only has
the MPR_ST_POWER bit set.
Return -EBUSY if also MPR_ST_BUSY is set or -EIO otherwise.

Reviewed-by: Marcelo Schmitt <marcelo.schmitt1@gmail.com>
Signed-off-by: Petre Rodan <petre.rodan@subdimension.ro>
---
v1 -> v2 no change
---
 drivers/iio/pressure/mprls0025pa.c | 28 ++++++++++++++++++++++------
 1 file changed, 22 insertions(+), 6 deletions(-)

diff --git a/drivers/iio/pressure/mprls0025pa.c b/drivers/iio/pressure/mprls0025pa.c
index e4651d3f686d..078d1d719023 100644
--- a/drivers/iio/pressure/mprls0025pa.c
+++ b/drivers/iio/pressure/mprls0025pa.c
@@ -198,9 +198,10 @@ static void mpr_reset(struct mpr_data *data)
  *
  * Context: The function can sleep and data->lock should be held when calling it
  * Return:
- * * 0		- OK, the pressure value could be read
- * * -ETIMEDOUT	- Timeout while waiting for the EOC interrupt or busy flag is
- *		  still set after nloops attempts of reading
+ * * 0          - OK, the pressure value could be read
+ * * -EBUSY     - Sensor does not have a new conversion ready
+ * * -ETIMEDOUT - Timeout while waiting for the EOC interrupt
+ * * -EIO       - Invalid status byte received from sensor
  */
 static int mpr_read_pressure(struct mpr_data *data, s32 *press)
 {
@@ -253,10 +254,25 @@ static int mpr_read_pressure(struct mpr_data *data, s32 *press)
 	if (ret < 0)
 		return ret;
 
-	if (data->rx_buf[0] & MPR_ST_ERR_FLAG) {
+	/*
+	 * Status byte flags
+	 *  bit7 SANITY_CHK   - must always be 0
+	 *  bit6 MPR_ST_POWER - 1 if device is powered
+	 *  bit5 MPR_ST_BUSY  - 1 if device has no new conversion ready
+	 *  bit4 SANITY_CHK   - must always be 0
+	 *  bit3 SANITY_CHK   - must always be 0
+	 *  bit2 MEMORY_ERR   - 1 if integrity test has failed
+	 *  bit1 SANITY_CHK   - must always be 0
+	 *  bit0 MATH_ERR     - 1 during internal math saturation error
+	 */
+
+	if (data->rx_buf[0] == (MPR_ST_POWER | MPR_ST_BUSY))
+		return -EBUSY;
+
+	if (data->rx_buf[0] != MPR_ST_POWER) {
 		dev_err(data->dev,
-			"unexpected status byte %02x\n", data->rx_buf[0]);
-		return -ETIMEDOUT;
+			"unexpected status byte 0x%02x\n", data->rx_buf[0]);
+		return -EIO;
 	}
 
 	*press = get_unaligned_be24(&data->rx_buf[1]);

-- 
2.52.0

[PATCH v2 12/13] iio: pressure: mprls0025pa: change measurement sequence

[Petre Rodan]

This patch implements a measurement sequence that does not involve a
one byte read of the status byte before reading the conversion.

The sensor's conversions should be read either once the EoC interrupt
has triggered or 5ms after the 0xaa command. See Options 1 and 2
respectively in Tables 16 (page 15) and 18 (page 18) of the datasheet.
Note that Honeywell's example code also covered in the datasheet follows
Option 2 for both i2c and SPI.

The datasheet does not specify any of the retry parameters that are
currently implemented in the driver. A simple 5+ms sleep as specified in
Option 2 is enough for a valid measurement sequence.

The change also gets rid of the code duplication tied to the verification
of the status byte.

This change only affects users that do not define the EOC interrupt in
the device tree.

Datasheet: https://prod-edam.honeywell.com/content/dam/honeywell-edam/sps/siot/en-us/products/sensors/pressure-sensors/board-mount-pressure-sensors/micropressure-mpr-series/documents/sps-siot-mpr-series-datasheet-32332628-ciid-172626.pdf?download=false
Signed-off-by: Petre Rodan <petre.rodan@subdimension.ro>
---
v1 -> v2 rewrote commit message
---
 drivers/iio/pressure/mprls0025pa.c | 32 +++-----------------------------
 1 file changed, 3 insertions(+), 29 deletions(-)

diff --git a/drivers/iio/pressure/mprls0025pa.c b/drivers/iio/pressure/mprls0025pa.c
index 078d1d719023..8218d931647b 100644
--- a/drivers/iio/pressure/mprls0025pa.c
+++ b/drivers/iio/pressure/mprls0025pa.c
@@ -23,6 +23,7 @@
 #include <linux/module.h>
 #include <linux/property.h>
 #include <linux/string.h>
+#include <linux/time.h>
 #include <linux/units.h>
 
 #include <linux/gpio/consumer.h>
@@ -41,10 +42,6 @@
 /* bits in status byte */
 #define MPR_ST_POWER  BIT(6) /* device is powered */
 #define MPR_ST_BUSY   BIT(5) /* device is busy */
-#define MPR_ST_MEMORY BIT(2) /* integrity test passed */
-#define MPR_ST_MATH   BIT(0) /* internal math saturation */
-
-#define MPR_ST_ERR_FLAG  (MPR_ST_BUSY | MPR_ST_MEMORY | MPR_ST_MATH)
 
 /*
  * support _RAW sysfs interface:
@@ -206,8 +203,7 @@ static void mpr_reset(struct mpr_data *data)
 static int mpr_read_pressure(struct mpr_data *data, s32 *press)
 {
 	struct device *dev = data->dev;
-	int ret, i;
-	int nloops = 10;
+	int ret;
 
 	reinit_completion(&data->completion);
 
@@ -224,29 +220,7 @@ static int mpr_read_pressure(struct mpr_data *data, s32 *press)
 			return -ETIMEDOUT;
 		}
 	} else {
-		/* wait until status indicates data is ready */
-		for (i = 0; i < nloops; i++) {
-			/*
-			 * datasheet only says to wait at least 5 ms for the
-			 * data but leave the maximum response time open
-			 * --> let's try it nloops (10) times which seems to be
-			 *     quite long
-			 */
-			usleep_range(5000, 10000);
-			ret = data->ops->read(data, MPR_CMD_NOP, 1);
-			if (ret < 0) {
-				dev_err(dev,
-					"error while reading, status: %d\n",
-					ret);
-				return ret;
-			}
-			if (!(data->rx_buf[0] & MPR_ST_ERR_FLAG))
-				break;
-		}
-		if (i == nloops) {
-			dev_err(dev, "timeout while reading\n");
-			return -ETIMEDOUT;
-		}
+		fsleep(5 * USEC_PER_MSEC);
 	}
 
 	memset(data->rx_buf, 0, sizeof(data->rx_buf));

-- 
2.52.0

[PATCH v2 13/13] iio: pressure: mprls0025pa: add copyright line

[Petre Rodan]

Add copyright line to the core driver.

Signed-off-by: Petre Rodan <petre.rodan@subdimension.ro>
---
v1 -> v2 no change
---
 drivers/iio/pressure/mprls0025pa.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/iio/pressure/mprls0025pa.c b/drivers/iio/pressure/mprls0025pa.c
index 8218d931647b..0122108f1f3e 100644
--- a/drivers/iio/pressure/mprls0025pa.c
+++ b/drivers/iio/pressure/mprls0025pa.c
@@ -3,6 +3,7 @@
  * MPRLS0025PA - Honeywell MicroPressure pressure sensor series driver
  *
  * Copyright (c) Andreas Klinger <ak@it-klinger.de>
+ * Copyright (c) 2023-2025 Petre Rodan <petre.rodan@subdimension.ro>
  *
  * Data sheet:
  *  https://prod-edam.honeywell.com/content/dam/honeywell-edam/sps/siot/en-us/products/sensors/pressure-sensors/board-mount-pressure-sensors/micropressure-mpr-series/documents/sps-siot-mpr-series-datasheet-32332628-ciid-172626.pdf

-- 
2.52.0

Re: [PATCH v2 03/13] iio: pressure: mprls0025pa: fix interrupt flag

[Andy Shevchenko]

On Wed, Jan 14, 2026 at 12:05:37PM +0200, Petre Rodan wrote:
> Interrupt falling/rising flags should only be defined in the device tree.

...

>  	if (data->irq > 0) {
>  		ret = devm_request_irq(dev, data->irq, mpr_eoc_handler,
> -				       IRQF_TRIGGER_RISING,
> -				       dev_name(dev),
> -				       data);
> +				       IRQF_TRIGGER_NONE, dev_name(dev), data);

No, this is still incorrect, should be plain 0 here.

>  		if (ret)
>  			return dev_err_probe(dev, ret,
>  					  "request irq %d failed\n", data->irq);

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2 05/13] iio: pressure: mprls0025pa: fix pressure calculation

[Andy Shevchenko]

On Wed, Jan 14, 2026 at 12:05:39PM +0200, Petre Rodan wrote:
> A sign change is needed for proper calculation of the pressure.
> 
> This is a minor fix since it only affects users that might have custom
> silicon from Honeywell that has honeywell,pmin-pascal != 0.
> 
> Also due to the fact that raw pressure values can not be lower
> than output_min (400k-3.3M) there is no need to calculate a decimal for
> the offset.

...

> int mpr_common_probe(struct device *dev, const struct mpr_ops *ops, int irq)

>  	struct mpr_data *data;
>  	struct iio_dev *indio_dev;
>  	const char *triplet;
> -	s64 scale, offset;
>  	u32 func;
> +	s32 tmp;
> +	s64 odelta, pdelta;

Please, preserve reversed xmas tree order. And in general try to stick with that one.


-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2 12/13] iio: pressure: mprls0025pa: change measurement sequence

[Andy Shevchenko]

On Wed, Jan 14, 2026 at 12:05:46PM +0200, Petre Rodan wrote:
> This patch implements a measurement sequence that does not involve a

s/This patch implements/Implement/

This is described in the kernel Documentation (imperative mode).

> one byte read of the status byte before reading the conversion.
> 
> The sensor's conversions should be read either once the EoC interrupt
> has triggered or 5ms after the 0xaa command. See Options 1 and 2
> respectively in Tables 16 (page 15) and 18 (page 18) of the datasheet.
> Note that Honeywell's example code also covered in the datasheet follows
> Option 2 for both i2c and SPI.
> 
> The datasheet does not specify any of the retry parameters that are
> currently implemented in the driver. A simple 5+ms sleep as specified in
> Option 2 is enough for a valid measurement sequence.
> 
> The change also gets rid of the code duplication tied to the verification
> of the status byte.
> 
> This change only affects users that do not define the EOC interrupt in
> the device tree.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2 03/13] iio: pressure: mprls0025pa: fix interrupt flag

[Jonathan Cameron]

On Wed, 14 Jan 2026 12:13:33 +0200
Andy Shevchenko <andriy.shevchenko@intel.com> wrote:

> On Wed, Jan 14, 2026 at 12:05:37PM +0200, Petre Rodan wrote:
> > Interrupt falling/rising flags should only be defined in the device tree.  
> 
> ...
> 
> >  	if (data->irq > 0) {
> >  		ret = devm_request_irq(dev, data->irq, mpr_eoc_handler,
> > -				       IRQF_TRIGGER_RISING,
> > -				       dev_name(dev),
> > -				       data);
> > +				       IRQF_TRIGGER_NONE, dev_name(dev), data);  
> 
> No, this is still incorrect, should be plain 0 here.

Yeah. My mistake in agreeing with your reply in v1.  

I'll randomly blame lack of coffee ;)
See the help text above IRQF_TRIGGER_NONE in interrupt.h which talks about not
specifying up at all (i.e. 0) meaning go with whatever is already there.

Jonathan
> 
> >  		if (ret)
> >  			return dev_err_probe(dev, ret,
> >  					  "request irq %d failed\n", data->irq);  
> 

Re: [PATCH v2 00/13] iio: pressure: mprls0025pa: driver code cleanup

[Jonathan Cameron]

On Wed, 14 Jan 2026 12:05:34 +0200
Petre Rodan <petre.rodan@subdimension.ro> wrote:

> This series contains a collection of patches to the MPR sensor based
> on feedback I received for other drivers.

Hi Petre,

I took a look at the whole series and didn't have anything to add to
Andy's review.

As we are somewhat near the end of the cycle, if you have time,
feel free to send a v3 sooner than the normal delay of 1ish weeks
(unless there is something you want to wait for more feedback on!)

I'll almost certainly take the lot for the merge window now rather
than trying to get the fixes in this cycle.

Thanks,

Jonathan



> 
> major changes:
>  - trigger flag fix (define edge direction only in the device tree)
>  - fix SPI timing violation
>  - fix scan_type struct
>  - fix pressure calculation
>     (does not affect users that define a sensor via the pressure-triplet)
>  - stricter check for the status byte + better error return levels
>  - drop the use of devm_kzalloc()
>  - stick to the datasheet parameters while performing the measurement
>     sequence
> 
> minor changes:
>  - includes added and removed
>  - rename generic 'buffer' variable to 'rx_buf'
> 
> I still included the memset patch for the reasons described in the v1 thread.
> if you strongly consider that patch inadequate then please skip it.
> 
> Tested on two sensors - MPRLS0015PA0000SA and MPRLS0001BA00001A
> 
> Datasheet: https://prod-edam.honeywell.com/content/dam/honeywell-edam/sps/siot/en-us/products/sensors/pressure-sensors/board-mount-pressure-sensors/micropressure-mpr-series/documents/sps-siot-mpr-series-datasheet-32332628-ciid-172626.pdf?download=false
> Signed-off-by: Petre Rodan <petre.rodan@subdimension.ro>
> ---
> Changes in v2:
> - reordered patches based on Marcelo's input
> - implemented code changes based on Marcelo and Jonathan's reviews
> - Link to v1: https://lore.kernel.org/r/20251218-mprls_cleanup-v1-0-b36a170f1a5c@subdimension.ro
> 
> ---
> Petre Rodan (13):
>       iio: pressure: mprls0025pa: fix spi_transfer struct initialisation
>       iio: pressure: mprls0025pa: fix SPI CS delay violation
>       iio: pressure: mprls0025pa: fix interrupt flag
>       iio: pressure: mprls0025pa: fix scan_type struct
>       iio: pressure: mprls0025pa: fix pressure calculation
>       iio: pressure: mprls0025pa: cleanup includes
>       iio: pressure: mprls0025pa: remove redundant declarations
>       iio: pressure: mprls0025pa: rename buffer variable
>       iio: pressure: mprls0025pa: introduce tx buffer
>       iio: pressure: mprls0025pa: move memset to core
>       iio: pressure: mprls0025pa: stricter checks for the status byte
>       iio: pressure: mprls0025pa: change measurement sequence
>       iio: pressure: mprls0025pa: add copyright line
> 
>  drivers/iio/pressure/mprls0025pa.c     | 112 +++++++++++++++------------------
>  drivers/iio/pressure/mprls0025pa.h     |  15 ++---
>  drivers/iio/pressure/mprls0025pa_i2c.c |  13 +---
>  drivers/iio/pressure/mprls0025pa_spi.c |  41 +++++-------
>  4 files changed, 73 insertions(+), 108 deletions(-)
> ---
> base-commit: f9e05791642810a0cf6237d39fafd6fec5e0b4bb
> change-id: 20251215-mprls_cleanup-01de8971b439
> 
> Best regards,