[PATCH v3 0/3] iio: mpl3115: support for events

[PATCH v3 0/3] iio: mpl3115: support for events

[Antoni Pokusinski]

Hello,
The mpl3115 device can raise interrupts when a pressure or temperature
threshold is crossed, this patchset adds support for them using IIO's
events interface.

In this v3 I updated pressure data retrieval, so that the measurements
are now stored into u8[3], thanks for the explanation in v2. Some other
small cosmetic changes as well.

Kind regards,
Antoni Pokusinski

---
Changes since v2:
(general)
* added the patch tidying up the pressure data retrieval (u8[3] used)
(patch 2/3 "add threshold support")
* includes: removed unused linux/units.h
* read_thresh: fixed comment formatting
* interrupt_handler: val_press is now u8[3] instead of __be32 

Changes since v1:
(general)
* squashed the cleanup patch
* added the patch with the documentation update
(patch 1/2 "add threshold event support")
* patch description: explained changes in locking
* read_event_config: replaced switch with ifs
* read_event_config: return as early as possible, got rid of int_en_mask
* read/write_thresh: pressure: calculation changes to comply with raw ABI
* interrupt_handler: reordered the INT_SRC_* bits in if condition
* read/write_thresh: used sizeof() and values from limits.h
* write_thresh: replaced `u8 tmp[2]` with `__be16 tmp`
* dropped the space between casting `(u8 *) &tmp`


Antoni Pokusinski (3):
  iio: mpl3115: use get_unaligned_be24 to retrieve pressure data
  iio: mpl3115: add threshold events support
  iio: ABI: document pressure event attributes

 Documentation/ABI/testing/sysfs-bus-iio |   2 +
 drivers/iio/pressure/mpl3115.c          | 225 ++++++++++++++++++++++--
 2 files changed, 214 insertions(+), 13 deletions(-)


base-commit: 1d09cf18cc91d29f650ad9811ed4868d9304d6c7
-- 
2.25.1

[PATCH v3 1/3] iio: mpl3115: use get_unaligned_be24 to retrieve pressure data

[Antoni Pokusinski]

The pressure measurement result is arranged as 20-bit unsigned value
residing in three 8-bit registers. Hence, it can be retrieved using
get_unaligned_be24 and by applying 4-bit shift.

Signed-off-by: Antoni Pokusinski <apokusinski01@gmail.com>
---
 drivers/iio/pressure/mpl3115.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/iio/pressure/mpl3115.c b/drivers/iio/pressure/mpl3115.c
index c212dfdf59ff..3f1fa9fe3c76 100644
--- a/drivers/iio/pressure/mpl3115.c
+++ b/drivers/iio/pressure/mpl3115.c
@@ -16,6 +16,7 @@
 #include <linux/i2c.h>
 #include <linux/module.h>
 #include <linux/property.h>
+#include <linux/unaligned.h>
 
 #include <linux/iio/buffer.h>
 #include <linux/iio/iio.h>
@@ -125,7 +126,7 @@ static int mpl3115_read_info_raw(struct mpl3115_data *data,
 
 	switch (chan->type) {
 	case IIO_PRESSURE: { /* in 0.25 pascal / LSB */
-		__be32 tmp = 0;
+		u8 tmp[3];
 
 		guard(mutex)(&data->lock);
 		ret = mpl3115_request(data);
@@ -134,11 +135,11 @@ static int mpl3115_read_info_raw(struct mpl3115_data *data,
 
 		ret = i2c_smbus_read_i2c_block_data(data->client,
 						    MPL3115_OUT_PRESS,
-						    3, (u8 *) &tmp);
+						    sizeof(tmp), tmp);
 		if (ret < 0)
 			return ret;
 
-		*val = be32_to_cpu(tmp) >> chan->scan_type.shift;
+		*val = get_unaligned_be24(tmp) >> 4;
 		return IIO_VAL_INT;
 	}
 	case IIO_TEMP: { /* in 0.0625 celsius / LSB */
-- 
2.25.1

[PATCH v3 2/3] iio: mpl3115: add threshold events support

[Antoni Pokusinski]

Add support for pressure and temperature rising threshold events. For
both channels *_en and *_value (in raw units) attributes are exposed.

Since in write_event_config() the ctrl_reg1.active and ctrl_reg4
are modified, accessing the data->ctrl_reg{1,4} in set_trigger_state()
and write_event_config() needs to be now guarded by data->lock.
Otherwise, it would be possible that 2 concurrent threads executing
these functions would access the data->ctrl_reg{1,4} at the same time
and then one would overwrite the other's result.

Signed-off-by: Antoni Pokusinski <apokusinski01@gmail.com>
---
 drivers/iio/pressure/mpl3115.c | 218 +++++++++++++++++++++++++++++++--
 1 file changed, 208 insertions(+), 10 deletions(-)

diff --git a/drivers/iio/pressure/mpl3115.c b/drivers/iio/pressure/mpl3115.c
index 3f1fa9fe3c76..d56f5fcd5900 100644
--- a/drivers/iio/pressure/mpl3115.c
+++ b/drivers/iio/pressure/mpl3115.c
@@ -14,11 +14,13 @@
 #include <linux/cleanup.h>
 #include <linux/delay.h>
 #include <linux/i2c.h>
+#include <linux/limits.h>
 #include <linux/module.h>
 #include <linux/property.h>
 #include <linux/unaligned.h>
 
 #include <linux/iio/buffer.h>
+#include <linux/iio/events.h>
 #include <linux/iio/iio.h>
 #include <linux/iio/sysfs.h>
 #include <linux/iio/triggered_buffer.h>
@@ -31,6 +33,8 @@
 #define MPL3115_WHO_AM_I 0x0c
 #define MPL3115_INT_SOURCE 0x12
 #define MPL3115_PT_DATA_CFG 0x13
+#define MPL3115_PRESS_TGT 0x16 /* MSB first, 16 bit */
+#define MPL3115_TEMP_TGT 0x18
 #define MPL3115_CTRL_REG1 0x26
 #define MPL3115_CTRL_REG2 0x27
 #define MPL3115_CTRL_REG3 0x28
@@ -43,6 +47,8 @@
 #define MPL3115_STATUS_TEMP_RDY BIT(1)
 
 #define MPL3115_INT_SRC_DRDY BIT(7)
+#define MPL3115_INT_SRC_PTH BIT(3)
+#define MPL3115_INT_SRC_TTH BIT(2)
 
 #define MPL3115_PT_DATA_EVENT_ALL GENMASK(2, 0)
 
@@ -57,6 +63,8 @@
 #define MPL3115_CTRL3_IPOL2 BIT(1)
 
 #define MPL3115_CTRL4_INT_EN_DRDY BIT(7)
+#define MPL3115_CTRL4_INT_EN_PTH BIT(3)
+#define MPL3115_CTRL4_INT_EN_TTH BIT(2)
 
 #define MPL3115_CTRL5_INT_CFG_DRDY BIT(7)
 
@@ -84,6 +92,7 @@ struct mpl3115_data {
 	struct iio_trigger *drdy_trig;
 	struct mutex lock;
 	u8 ctrl_reg1;
+	u8 ctrl_reg4;
 };
 
 enum mpl3115_irq_pin {
@@ -307,6 +316,15 @@ static irqreturn_t mpl3115_trigger_handler(int irq, void *p)
 	return IRQ_HANDLED;
 }
 
+static const struct iio_event_spec mpl3115_temp_press_event[] = {
+	{
+		.type = IIO_EV_TYPE_THRESH,
+		.dir = IIO_EV_DIR_RISING,
+		.mask_separate = BIT(IIO_EV_INFO_ENABLE) |
+				 BIT(IIO_EV_INFO_VALUE),
+	},
+};
+
 static const struct iio_chan_spec mpl3115_channels[] = {
 	{
 		.type = IIO_PRESSURE,
@@ -322,7 +340,9 @@ static const struct iio_chan_spec mpl3115_channels[] = {
 			.storagebits = 32,
 			.shift = 12,
 			.endianness = IIO_BE,
-		}
+		},
+		.event_spec = mpl3115_temp_press_event,
+		.num_event_specs = ARRAY_SIZE(mpl3115_temp_press_event),
 	},
 	{
 		.type = IIO_TEMP,
@@ -338,7 +358,9 @@ static const struct iio_chan_spec mpl3115_channels[] = {
 			.storagebits = 16,
 			.shift = 4,
 			.endianness = IIO_BE,
-		}
+		},
+		.event_spec = mpl3115_temp_press_event,
+		.num_event_specs = ARRAY_SIZE(mpl3115_temp_press_event),
 	},
 	IIO_CHAN_SOFT_TIMESTAMP(2),
 };
@@ -348,15 +370,45 @@ static irqreturn_t mpl3115_interrupt_handler(int irq, void *private)
 	struct iio_dev *indio_dev = private;
 	struct mpl3115_data *data = iio_priv(indio_dev);
 	int ret;
+	u8 val_press[3];
+	__be16 val_temp;
 
 	ret = i2c_smbus_read_byte_data(data->client, MPL3115_INT_SOURCE);
 	if (ret < 0)
 		return IRQ_HANDLED;
 
-	if (!(ret & MPL3115_INT_SRC_DRDY))
+	if (!(ret & (MPL3115_INT_SRC_TTH | MPL3115_INT_SRC_PTH |
+		     MPL3115_INT_SRC_DRDY)))
 		return IRQ_NONE;
 
-	iio_trigger_poll_nested(data->drdy_trig);
+	if (ret & MPL3115_INT_SRC_DRDY)
+		iio_trigger_poll_nested(data->drdy_trig);
+
+	if (ret & MPL3115_INT_SRC_PTH) {
+		iio_push_event(indio_dev,
+			       IIO_UNMOD_EVENT_CODE(IIO_PRESSURE, 0,
+						    IIO_EV_TYPE_THRESH,
+						    IIO_EV_DIR_RISING),
+						    iio_get_time_ns(indio_dev));
+
+		/* Reset the SRC_PTH bit in INT_SOURCE */
+		i2c_smbus_read_i2c_block_data(data->client,
+					      MPL3115_OUT_PRESS,
+					      sizeof(val_press), val_press);
+	}
+
+	if (ret & MPL3115_INT_SRC_TTH) {
+		iio_push_event(indio_dev,
+			       IIO_UNMOD_EVENT_CODE(IIO_TEMP, 0,
+						    IIO_EV_TYPE_THRESH,
+						    IIO_EV_DIR_RISING),
+						    iio_get_time_ns(indio_dev));
+
+		/* Reset the SRC_TTH bit in INT_SOURCE */
+		i2c_smbus_read_i2c_block_data(data->client,
+					      MPL3115_OUT_TEMP,
+					      2, (u8 *)&val_temp);
+	}
 
 	return IRQ_HANDLED;
 }
@@ -377,6 +429,7 @@ static int mpl3115_config_interrupt(struct mpl3115_data *data,
 		goto reg1_cleanup;
 
 	data->ctrl_reg1 = ctrl_reg1;
+	data->ctrl_reg4 = ctrl_reg4;
 
 	return 0;
 
@@ -390,15 +443,22 @@ static int mpl3115_set_trigger_state(struct iio_trigger *trig, bool state)
 {
 	struct iio_dev *indio_dev = iio_trigger_get_drvdata(trig);
 	struct mpl3115_data *data = iio_priv(indio_dev);
-	u8 ctrl_reg1 = data->ctrl_reg1;
-	u8 ctrl_reg4 = state ? MPL3115_CTRL4_INT_EN_DRDY : 0;
+	u8 ctrl_reg1, ctrl_reg4;
 
-	if (state)
+	guard(mutex)(&data->lock);
+
+	ctrl_reg1 = data->ctrl_reg1;
+	ctrl_reg4 = data->ctrl_reg4;
+
+	if (state) {
 		ctrl_reg1 |= MPL3115_CTRL1_ACTIVE;
-	else
-		ctrl_reg1 &= ~MPL3115_CTRL1_ACTIVE;
+		ctrl_reg4 |= MPL3115_CTRL4_INT_EN_DRDY;
+	} else {
+		ctrl_reg4 &= ~MPL3115_CTRL4_INT_EN_DRDY;
 
-	guard(mutex)(&data->lock);
+		if (!ctrl_reg4)
+			ctrl_reg1 &= ~MPL3115_CTRL1_ACTIVE;
+	}
 
 	return mpl3115_config_interrupt(data, ctrl_reg1, ctrl_reg4);
 }
@@ -407,10 +467,148 @@ static const struct iio_trigger_ops mpl3115_trigger_ops = {
 	.set_trigger_state = mpl3115_set_trigger_state,
 };
 
+static int mpl3115_read_event_config(struct iio_dev *indio_dev,
+				     const struct iio_chan_spec *chan,
+				     enum iio_event_type type,
+				     enum iio_event_direction dir)
+{
+	struct mpl3115_data *data = iio_priv(indio_dev);
+
+	if (chan->type == IIO_PRESSURE)
+		return !!(data->ctrl_reg4 & MPL3115_CTRL4_INT_EN_PTH);
+
+	if (chan->type == IIO_TEMP)
+		return !!(data->ctrl_reg4 & MPL3115_CTRL4_INT_EN_TTH);
+
+	return -EINVAL;
+}
+
+static int mpl3115_write_event_config(struct iio_dev *indio_dev,
+				      const struct iio_chan_spec *chan,
+				      enum iio_event_type type,
+				      enum iio_event_direction dir,
+				      bool state)
+{
+	struct mpl3115_data *data = iio_priv(indio_dev);
+	u8 int_en_mask;
+	u8 ctrl_reg1, ctrl_reg4;
+
+	switch (chan->type) {
+	case IIO_PRESSURE:
+		int_en_mask = MPL3115_CTRL4_INT_EN_PTH;
+		break;
+	case IIO_TEMP:
+		int_en_mask = MPL3115_CTRL4_INT_EN_TTH;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	guard(mutex)(&data->lock);
+
+	ctrl_reg1 = data->ctrl_reg1;
+	ctrl_reg4 = data->ctrl_reg4;
+
+	if (state) {
+		ctrl_reg1 |= MPL3115_CTRL1_ACTIVE;
+		ctrl_reg4 |= int_en_mask;
+	} else {
+		ctrl_reg4 &= ~int_en_mask;
+
+		if (!ctrl_reg4)
+			ctrl_reg1 &= ~MPL3115_CTRL1_ACTIVE;
+	}
+
+	return mpl3115_config_interrupt(data, ctrl_reg1, ctrl_reg4);
+}
+
+static int mpl3115_read_thresh(struct iio_dev *indio_dev,
+			       const struct iio_chan_spec *chan,
+			       enum iio_event_type type,
+			       enum iio_event_direction dir,
+			       enum iio_event_info info,
+			       int *val, int *val2)
+{
+	struct mpl3115_data *data = iio_priv(indio_dev);
+	int ret;
+	__be16 tmp;
+
+	if (info != IIO_EV_INFO_VALUE)
+		return -EINVAL;
+
+	switch (chan->type) {
+	case IIO_PRESSURE:
+		ret = i2c_smbus_read_i2c_block_data(data->client,
+						    MPL3115_PRESS_TGT,
+						    sizeof(tmp), (u8 *)&tmp);
+		if (ret < 0)
+			return ret;
+
+		/*
+		 * Target value for the pressure is 16-bit unsigned value,
+		 * expressed in 2 Pa units
+		 */
+		*val = be16_to_cpu(tmp) << 1;
+
+		return IIO_VAL_INT;
+	case IIO_TEMP:
+		ret = i2c_smbus_read_byte_data(data->client, MPL3115_TEMP_TGT);
+		if (ret < 0)
+			return ret;
+
+		/* Target value for the temperature is 8-bit 2's complement */
+		*val = sign_extend32(ret, 7);
+
+		return IIO_VAL_INT;
+	default:
+		return -EINVAL;
+	}
+}
+
+static int mpl3115_write_thresh(struct iio_dev *indio_dev,
+				const struct iio_chan_spec *chan,
+				enum iio_event_type type,
+				enum iio_event_direction dir,
+				enum iio_event_info info,
+				int val, int val2)
+{
+	struct mpl3115_data *data = iio_priv(indio_dev);
+	__be16 tmp;
+
+	if (info != IIO_EV_INFO_VALUE)
+		return -EINVAL;
+
+	switch (chan->type) {
+	case IIO_PRESSURE:
+		val >>= 1;
+
+		if (val < 0 || val > U16_MAX)
+			return -EINVAL;
+
+		tmp = cpu_to_be16(val);
+
+		return i2c_smbus_write_i2c_block_data(data->client,
+						      MPL3115_PRESS_TGT,
+						      sizeof(tmp), (u8 *)&tmp);
+	case IIO_TEMP:
+		if (val < S8_MIN || val > S8_MAX)
+			return -EINVAL;
+
+		return i2c_smbus_write_byte_data(data->client,
+						 MPL3115_TEMP_TGT, val);
+	default:
+		return -EINVAL;
+	}
+}
+
 static const struct iio_info mpl3115_info = {
 	.read_raw = &mpl3115_read_raw,
 	.read_avail = &mpl3115_read_avail,
 	.write_raw = &mpl3115_write_raw,
+	.read_event_config = mpl3115_read_event_config,
+	.write_event_config = mpl3115_write_event_config,
+	.read_event_value = mpl3115_read_thresh,
+	.write_event_value = mpl3115_write_thresh,
 };
 
 static int mpl3115_trigger_probe(struct mpl3115_data *data,
-- 
2.25.1

[PATCH v3 3/3] iio: ABI: document pressure event attributes

[Antoni Pokusinski]

Add sysfs pressure event attributes exposed by the mpl3115 driver. These
allow controlling the threshold value and the enable state.

Signed-off-by: Antoni Pokusinski <apokusinski01@gmail.com>
---
 Documentation/ABI/testing/sysfs-bus-iio | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/ABI/testing/sysfs-bus-iio b/Documentation/ABI/testing/sysfs-bus-iio
index 352ab7b8476c..5f87dcee78f7 100644
--- a/Documentation/ABI/testing/sysfs-bus-iio
+++ b/Documentation/ABI/testing/sysfs-bus-iio
@@ -898,6 +898,7 @@ What:		/sys/.../iio:deviceX/events/in_tempY_thresh_rising_en
 What:		/sys/.../iio:deviceX/events/in_tempY_thresh_falling_en
 What:		/sys/.../iio:deviceX/events/in_capacitanceY_thresh_rising_en
 What:		/sys/.../iio:deviceX/events/in_capacitanceY_thresh_falling_en
+What:		/sys/.../iio:deviceX/events/in_pressure_thresh_rising_en
 KernelVersion:	2.6.37
 Contact:	linux-iio@vger.kernel.org
 Description:
@@ -1047,6 +1048,7 @@ What:		/sys/.../events/in_capacitanceY_thresh_rising_value
 What:		/sys/.../events/in_capacitanceY_thresh_falling_value
 What:		/sys/.../events/in_capacitanceY_thresh_adaptive_rising_value
 What:		/sys/.../events/in_capacitanceY_thresh_falling_rising_value
+What:		/sys/.../events/in_pressure_thresh_rising_value
 KernelVersion:	2.6.37
 Contact:	linux-iio@vger.kernel.org
 Description:
-- 
2.25.1

Re: [PATCH v3 1/3] iio: mpl3115: use get_unaligned_be24 to retrieve pressure data

[Andy Shevchenko]

On Wed, Nov 05, 2025 at 10:56:13AM +0100, Antoni Pokusinski wrote:
> The pressure measurement result is arranged as 20-bit unsigned value
> residing in three 8-bit registers. Hence, it can be retrieved using
> get_unaligned_be24 and by applying 4-bit shift.

get_unaligned_be24()

Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com>
independently on the below.

...

> +		u8 tmp[3];

While at it, you also may rename it to something better

${foo}_be24;

where ${foo} should be replaced to the meaningful name.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v3 2/3] iio: mpl3115: add threshold events support

[Andy Shevchenko]

On Wed, Nov 05, 2025 at 10:56:14AM +0100, Antoni Pokusinski wrote:
> Add support for pressure and temperature rising threshold events. For
> both channels *_en and *_value (in raw units) attributes are exposed.
> 
> Since in write_event_config() the ctrl_reg1.active and ctrl_reg4
> are modified, accessing the data->ctrl_reg{1,4} in set_trigger_state()
> and write_event_config() needs to be now guarded by data->lock.
> Otherwise, it would be possible that 2 concurrent threads executing
> these functions would access the data->ctrl_reg{1,4} at the same time
> and then one would overwrite the other's result.

...

> static irqreturn_t mpl3115_interrupt_handler(int irq, void *private)

>  	struct iio_dev *indio_dev = private;
>  	struct mpl3115_data *data = iio_priv(indio_dev);
>  	int ret;
> +	u8 val_press[3];
> +	__be16 val_temp;

s/_temp/$SOMETHING meaningful/ ?

>  	ret = i2c_smbus_read_byte_data(data->client, MPL3115_INT_SOURCE);
>  	if (ret < 0)
>  		return IRQ_HANDLED;
>  
> -	if (!(ret & MPL3115_INT_SRC_DRDY))
> +	if (!(ret & (MPL3115_INT_SRC_TTH | MPL3115_INT_SRC_PTH |
> +		     MPL3115_INT_SRC_DRDY)))
>  		return IRQ_NONE;
>  
> -	iio_trigger_poll_nested(data->drdy_trig);
> +	if (ret & MPL3115_INT_SRC_DRDY)
> +		iio_trigger_poll_nested(data->drdy_trig);
> +
> +	if (ret & MPL3115_INT_SRC_PTH) {
> +		iio_push_event(indio_dev,
> +			       IIO_UNMOD_EVENT_CODE(IIO_PRESSURE, 0,
> +						    IIO_EV_TYPE_THRESH,
> +						    IIO_EV_DIR_RISING),
> +						    iio_get_time_ns(indio_dev));
> +
> +		/* Reset the SRC_PTH bit in INT_SOURCE */
> +		i2c_smbus_read_i2c_block_data(data->client,
> +					      MPL3115_OUT_PRESS,
> +					      sizeof(val_press), val_press);
> +	}
> +
> +	if (ret & MPL3115_INT_SRC_TTH) {
> +		iio_push_event(indio_dev,
> +			       IIO_UNMOD_EVENT_CODE(IIO_TEMP, 0,
> +						    IIO_EV_TYPE_THRESH,
> +						    IIO_EV_DIR_RISING),
> +						    iio_get_time_ns(indio_dev));
> +
> +		/* Reset the SRC_TTH bit in INT_SOURCE */
> +		i2c_smbus_read_i2c_block_data(data->client,
> +					      MPL3115_OUT_TEMP,
> +					      2, (u8 *)&val_temp);

Why not sizeof() here ?

> +	}
>  
>  	return IRQ_HANDLED;
>  }

...

> +static int mpl3115_read_thresh(struct iio_dev *indio_dev,
> +			       const struct iio_chan_spec *chan,
> +			       enum iio_event_type type,
> +			       enum iio_event_direction dir,
> +			       enum iio_event_info info,
> +			       int *val, int *val2)
> +{
> +	struct mpl3115_data *data = iio_priv(indio_dev);
> +	int ret;
> +	__be16 tmp;

Also name can be improved. And notice confusion between temp
(temporary?  temperature?) and tmp.

> +	if (info != IIO_EV_INFO_VALUE)
> +		return -EINVAL;
> +
> +	switch (chan->type) {
> +	case IIO_PRESSURE:
> +		ret = i2c_smbus_read_i2c_block_data(data->client,
> +						    MPL3115_PRESS_TGT,
> +						    sizeof(tmp), (u8 *)&tmp);
> +		if (ret < 0)
> +			return ret;
> +
> +		/*
> +		 * Target value for the pressure is 16-bit unsigned value,
> +		 * expressed in 2 Pa units
> +		 */
> +		*val = be16_to_cpu(tmp) << 1;
> +
> +		return IIO_VAL_INT;
> +	case IIO_TEMP:
> +		ret = i2c_smbus_read_byte_data(data->client, MPL3115_TEMP_TGT);
> +		if (ret < 0)
> +			return ret;
> +
> +		/* Target value for the temperature is 8-bit 2's complement */
> +		*val = sign_extend32(ret, 7);
> +
> +		return IIO_VAL_INT;
> +	default:
> +		return -EINVAL;
> +	}
> +}

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v3 2/3] iio: mpl3115: add threshold events support

[Antoni Pokusinski]

On Wed, Nov 05, 2025 at 05:25:17PM +0200, Andy Shevchenko wrote:
> On Wed, Nov 05, 2025 at 10:56:14AM +0100, Antoni Pokusinski wrote:
> > static irqreturn_t mpl3115_interrupt_handler(int irq, void *private)
> 
> >  	struct iio_dev *indio_dev = private;
> >  	struct mpl3115_data *data = iio_priv(indio_dev);
> >  	int ret;
> > +	u8 val_press[3];
> > +	__be16 val_temp;
> 
> s/_temp/$SOMETHING meaningful/ ?
> 
In this case I'd leave the "val_temp". We have "val_press" and
"val_temp" here so imo it indicates quite clearly that this variable
stores a temperature measurement.

The cases with "tmp" that you pointed out can be a bit confusing indeed, so
I'm going to replace them with something more meaningful (e.g.
"tmp" -> "press_tgt" in read_thresh() )  

> >  	ret = i2c_smbus_read_byte_data(data->client, MPL3115_INT_SOURCE);
> >  	if (ret < 0)
> >  		return IRQ_HANDLED;
> >  
> > -	if (!(ret & MPL3115_INT_SRC_DRDY))
> > +	if (!(ret & (MPL3115_INT_SRC_TTH | MPL3115_INT_SRC_PTH |
> > +		     MPL3115_INT_SRC_DRDY)))
> >  		return IRQ_NONE;
> >  
> > -	iio_trigger_poll_nested(data->drdy_trig);
> > +	if (ret & MPL3115_INT_SRC_DRDY)
> > +		iio_trigger_poll_nested(data->drdy_trig);
> > +
> > +	if (ret & MPL3115_INT_SRC_PTH) {
> > +		iio_push_event(indio_dev,
> > +			       IIO_UNMOD_EVENT_CODE(IIO_PRESSURE, 0,
> > +						    IIO_EV_TYPE_THRESH,
> > +						    IIO_EV_DIR_RISING),
> > +						    iio_get_time_ns(indio_dev));
> > +
> > +		/* Reset the SRC_PTH bit in INT_SOURCE */
> > +		i2c_smbus_read_i2c_block_data(data->client,
> > +					      MPL3115_OUT_PRESS,
> > +					      sizeof(val_press), val_press);
> > +	}
> > +
> > +	if (ret & MPL3115_INT_SRC_TTH) {
> > +		iio_push_event(indio_dev,
> > +			       IIO_UNMOD_EVENT_CODE(IIO_TEMP, 0,
> > +						    IIO_EV_TYPE_THRESH,
> > +						    IIO_EV_DIR_RISING),
> > +						    iio_get_time_ns(indio_dev));
> > +
> > +		/* Reset the SRC_TTH bit in INT_SOURCE */
> > +		i2c_smbus_read_i2c_block_data(data->client,
> > +					      MPL3115_OUT_TEMP,
> > +					      2, (u8 *)&val_temp);

Kind regards,
Antoni

Re: [PATCH v3 1/3] iio: mpl3115: use get_unaligned_be24 to retrieve pressure data

[Marcelo Schmitt]

On 11/05, Antoni Pokusinski wrote:
> The pressure measurement result is arranged as 20-bit unsigned value
> residing in three 8-bit registers. Hence, it can be retrieved using
> get_unaligned_be24 and by applying 4-bit shift.
> 
> Signed-off-by: Antoni Pokusinski <apokusinski01@gmail.com>
> ---
>  drivers/iio/pressure/mpl3115.c | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/iio/pressure/mpl3115.c b/drivers/iio/pressure/mpl3115.c
...
>  
> -		*val = be32_to_cpu(tmp) >> chan->scan_type.shift;
> +		*val = get_unaligned_be24(tmp) >> 4;
hmm, now the number of bits shifted is dissociated from the channel characteristics.
We can do
		*val = get_unaligned_be24(tmp) >> (24 - chan->scan_type.realbits);
or maybe
		*val = get_unaligned_be24(tmp) >> (sizeof(tmp) - chan->scan_type.realbits);
but it starts becoming too long IMO. Even longer if `tmp` gets a more meaningful
name. Ah well, any of the three forms should work the same at the end of day so
no strong opinion.

Reviewed-by: Marcelo Schmitt <marcelo.schmitt1@gmail.com>

>  		return IIO_VAL_INT;
>  	}
>  	case IIO_TEMP: { /* in 0.0625 celsius / LSB */
> -- 
> 2.25.1
> 

Re: [PATCH v3 2/3] iio: mpl3115: add threshold events support

[Marcelo Schmitt]

Hi Antoni,

v3 looks mostly good to me.
A couple of minor suggestions in addition to Andy's.

On 11/05, Antoni Pokusinski wrote:
> Add support for pressure and temperature rising threshold events. For
> both channels *_en and *_value (in raw units) attributes are exposed.
> 
> Since in write_event_config() the ctrl_reg1.active and ctrl_reg4
> are modified, accessing the data->ctrl_reg{1,4} in set_trigger_state()
> and write_event_config() needs to be now guarded by data->lock.
> Otherwise, it would be possible that 2 concurrent threads executing
> these functions would access the data->ctrl_reg{1,4} at the same time
> and then one would overwrite the other's result.
> 
> Signed-off-by: Antoni Pokusinski <apokusinski01@gmail.com>
> ---
...
> +
> +static int mpl3115_write_thresh(struct iio_dev *indio_dev,
> +				const struct iio_chan_spec *chan,
> +				enum iio_event_type type,
> +				enum iio_event_direction dir,
> +				enum iio_event_info info,
> +				int val, int val2)
> +{
> +	struct mpl3115_data *data = iio_priv(indio_dev);
> +	__be16 tmp;
> +
> +	if (info != IIO_EV_INFO_VALUE)
> +		return -EINVAL;
> +
> +	switch (chan->type) {
> +	case IIO_PRESSURE:
> +		val >>= 1;
> +
> +		if (val < 0 || val > U16_MAX)
Alternatively, could use in_range() for the check.

> +			return -EINVAL;
> +
> +		tmp = cpu_to_be16(val);
> +
> +		return i2c_smbus_write_i2c_block_data(data->client,
> +						      MPL3115_PRESS_TGT,
> +						      sizeof(tmp), (u8 *)&tmp);
> +	case IIO_TEMP:
> +		if (val < S8_MIN || val > S8_MAX)
this could also use in_range().

If you opt for the macro,
#include <linux/minmax.h>

> +			return -EINVAL;
> +
> +		return i2c_smbus_write_byte_data(data->client,
> +						 MPL3115_TEMP_TGT, val);
> +	default:
> +		return -EINVAL;
> +	}
> +}
> +

Re: [PATCH v3 3/3] iio: ABI: document pressure event attributes

[Marcelo Schmitt]

On 11/05, Antoni Pokusinski wrote:
> Add sysfs pressure event attributes exposed by the mpl3115 driver. These
> allow controlling the threshold value and the enable state.
> 
> Signed-off-by: Antoni Pokusinski <apokusinski01@gmail.com>
> ---
>  Documentation/ABI/testing/sysfs-bus-iio | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/Documentation/ABI/testing/sysfs-bus-iio b/Documentation/ABI/testing/sysfs-bus-iio
> index 352ab7b8476c..5f87dcee78f7 100644
> --- a/Documentation/ABI/testing/sysfs-bus-iio
> +++ b/Documentation/ABI/testing/sysfs-bus-iio
> @@ -898,6 +898,7 @@ What:		/sys/.../iio:deviceX/events/in_tempY_thresh_rising_en
>  What:		/sys/.../iio:deviceX/events/in_tempY_thresh_falling_en
>  What:		/sys/.../iio:deviceX/events/in_capacitanceY_thresh_rising_en
>  What:		/sys/.../iio:deviceX/events/in_capacitanceY_thresh_falling_en
> +What:		/sys/.../iio:deviceX/events/in_pressure_thresh_rising_en
>  KernelVersion:	2.6.37
>  Contact:	linux-iio@vger.kernel.org
>  Description:
> @@ -1047,6 +1048,7 @@ What:		/sys/.../events/in_capacitanceY_thresh_rising_value
>  What:		/sys/.../events/in_capacitanceY_thresh_falling_value
>  What:		/sys/.../events/in_capacitanceY_thresh_adaptive_rising_value
>  What:		/sys/.../events/in_capacitanceY_thresh_falling_rising_value
> +What:		/sys/.../events/in_pressure_thresh_rising_value
This is how it's currently appearing in sysfs, right?
If so, IIO event sysfs entry generation might need a tweak.

For what matters to mpl3115 event support patch set, I think this is okay.

Reviewed-by: Marcelo Schmitt <marcelo.schmitt1@gmail.com>

>  KernelVersion:	2.6.37
>  Contact:	linux-iio@vger.kernel.org
>  Description:
> -- 
> 2.25.1
> 

Re: [PATCH v3 3/3] iio: ABI: document pressure event attributes

[Antoni Pokusinski]

On Thu, Nov 06, 2025 at 11:32:07PM -0300, Marcelo Schmitt wrote:
> On 11/05, Antoni Pokusinski wrote:
> > Add sysfs pressure event attributes exposed by the mpl3115 driver. These
> > allow controlling the threshold value and the enable state.
> > 
> > Signed-off-by: Antoni Pokusinski <apokusinski01@gmail.com>
> > ---
> >  Documentation/ABI/testing/sysfs-bus-iio | 2 ++
> >  1 file changed, 2 insertions(+)
> > 
> > diff --git a/Documentation/ABI/testing/sysfs-bus-iio b/Documentation/ABI/testing/sysfs-bus-iio
> > index 352ab7b8476c..5f87dcee78f7 100644
> > --- a/Documentation/ABI/testing/sysfs-bus-iio
> > +++ b/Documentation/ABI/testing/sysfs-bus-iio
> > @@ -898,6 +898,7 @@ What:		/sys/.../iio:deviceX/events/in_tempY_thresh_rising_en
> >  What:		/sys/.../iio:deviceX/events/in_tempY_thresh_falling_en
> >  What:		/sys/.../iio:deviceX/events/in_capacitanceY_thresh_rising_en
> >  What:		/sys/.../iio:deviceX/events/in_capacitanceY_thresh_falling_en
> > +What:		/sys/.../iio:deviceX/events/in_pressure_thresh_rising_en
> >  KernelVersion:	2.6.37
> >  Contact:	linux-iio@vger.kernel.org
> >  Description:
> > @@ -1047,6 +1048,7 @@ What:		/sys/.../events/in_capacitanceY_thresh_rising_value
> >  What:		/sys/.../events/in_capacitanceY_thresh_falling_value
> >  What:		/sys/.../events/in_capacitanceY_thresh_adaptive_rising_value
> >  What:		/sys/.../events/in_capacitanceY_thresh_falling_rising_value
> > +What:		/sys/.../events/in_pressure_thresh_rising_value
> This is how it's currently appearing in sysfs, right?
Yes, that's right.

> If so, IIO event sysfs entry generation might need a tweak.
> 
> For what matters to mpl3115 event support patch set, I think this is okay.
> 
> Reviewed-by: Marcelo Schmitt <marcelo.schmitt1@gmail.com>
> 
> >  KernelVersion:	2.6.37
> >  Contact:	linux-iio@vger.kernel.org
> >  Description:
> > -- 
> > 2.25.1
> > 
Kind regards,
Antoni Pokusinski

Re: [PATCH v3 2/3] iio: mpl3115: add threshold events support

[Antoni Pokusinski]

On Thu, Nov 06, 2025 at 10:55:01PM -0300, Marcelo Schmitt wrote:
> Hi Antoni,
> 
> v3 looks mostly good to me.
> A couple of minor suggestions in addition to Andy's.
> 
> On 11/05, Antoni Pokusinski wrote:
> > +
> > +static int mpl3115_write_thresh(struct iio_dev *indio_dev,
> > +				const struct iio_chan_spec *chan,
> > +				enum iio_event_type type,
> > +				enum iio_event_direction dir,
> > +				enum iio_event_info info,
> > +				int val, int val2)
> > +{
> > +	struct mpl3115_data *data = iio_priv(indio_dev);
> > +	__be16 tmp;
> > +
> > +	if (info != IIO_EV_INFO_VALUE)
> > +		return -EINVAL;
> > +
> > +	switch (chan->type) {
> > +	case IIO_PRESSURE:
> > +		val >>= 1;
> > +
> > +		if (val < 0 || val > U16_MAX)
> Alternatively, could use in_range() for the check.
> 
> > +			return -EINVAL;
> > +
> > +		tmp = cpu_to_be16(val);
> > +
> > +		return i2c_smbus_write_i2c_block_data(data->client,
> > +						      MPL3115_PRESS_TGT,
> > +						      sizeof(tmp), (u8 *)&tmp);
> > +	case IIO_TEMP:
> > +		if (val < S8_MIN || val > S8_MAX)
> this could also use in_range().
> 
> If you opt for the macro,
> #include <linux/minmax.h>
>
I see that the in_range() macro operates only on unsigned values, so
placing it here would be wrong I guess. In order to keep the style
consistenc in this function, I'd keep both checks as "val < x || val > y"

> > +			return -EINVAL;
> > +
> > +		return i2c_smbus_write_byte_data(data->client,
> > +						 MPL3115_TEMP_TGT, val);
> > +	default:
> > +		return -EINVAL;
> > +	}
> > +}
> > +
Kind regards,
Antoni Pokusinski

Re: [PATCH v3 2/3] iio: mpl3115: add threshold events support

[Marcelo Schmitt]

...
> > > +	switch (chan->type) {
> > > +	case IIO_PRESSURE:
> > > +		val >>= 1;
> > > +
> > > +		if (val < 0 || val > U16_MAX)
> > Alternatively, could use in_range() for the check.
> > 
> > > +			return -EINVAL;
> > > +
> > > +		tmp = cpu_to_be16(val);
> > > +
> > > +		return i2c_smbus_write_i2c_block_data(data->client,
> > > +						      MPL3115_PRESS_TGT,
> > > +						      sizeof(tmp), (u8 *)&tmp);
> > > +	case IIO_TEMP:
> > > +		if (val < S8_MIN || val > S8_MAX)
> > this could also use in_range().
> > 
> > If you opt for the macro,
> > #include <linux/minmax.h>
> >
> I see that the in_range() macro operates only on unsigned values, so
> placing it here would be wrong I guess. In order to keep the style
> consistenc in this function, I'd keep both checks as "val < x || val > y"
> 
Ah, good point. Okay, no objection. 

> > > +			return -EINVAL;
> > > +
> > > +		return i2c_smbus_write_byte_data(data->client,
> > > +						 MPL3115_TEMP_TGT, val);
> > > +	default:
> > > +		return -EINVAL;
> > > +	}
> > > +}
> > > +
> Kind regards,
> Antoni Pokusinski
> 

Re: [PATCH v3 2/3] iio: mpl3115: add threshold events support

[Andy Shevchenko]

On Sat, Nov 08, 2025 at 04:05:34PM -0300, Marcelo Schmitt wrote:

...

> > > > +		if (val < 0 || val > U16_MAX)
> > > Alternatively, could use in_range() for the check.
> > > 
> > > > +			return -EINVAL;

,,,

> > > > +		if (val < S8_MIN || val > S8_MAX)
> > > this could also use in_range().
> > > 
> > > If you opt for the macro,
> > > #include <linux/minmax.h>
> > >
> > I see that the in_range() macro operates only on unsigned values, so
> > placing it here would be wrong I guess. In order to keep the style
> > consistenc in this function, I'd keep both checks as "val < x || val > y"
> > 
> Ah, good point. Okay, no objection. 


Actually we need something like in_the_range() or so which takes the min/max pair instead of start-end. And make it work for any signdness.

> > > > +			return -EINVAL;

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v3 1/3] iio: mpl3115: use get_unaligned_be24 to retrieve pressure data

[Jonathan Cameron]

On Thu, 6 Nov 2025 22:33:49 -0300
Marcelo Schmitt <marcelo.schmitt1@gmail.com> wrote:

> On 11/05, Antoni Pokusinski wrote:
> > The pressure measurement result is arranged as 20-bit unsigned value
> > residing in three 8-bit registers. Hence, it can be retrieved using
> > get_unaligned_be24 and by applying 4-bit shift.
> > 
> > Signed-off-by: Antoni Pokusinski <apokusinski01@gmail.com>
> > ---
> >  drivers/iio/pressure/mpl3115.c | 7 ++++---
> >  1 file changed, 4 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/iio/pressure/mpl3115.c b/drivers/iio/pressure/mpl3115.c  
> ...
> >  
> > -		*val = be32_to_cpu(tmp) >> chan->scan_type.shift;
> > +		*val = get_unaligned_be24(tmp) >> 4;  
> hmm, now the number of bits shifted is dissociated from the channel characteristics.
> We can do
> 		*val = get_unaligned_be24(tmp) >> (24 - chan->scan_type.realbits);
This encodes that the field is always aligned to the maximum bit. Whilst it might
be true, there is nothing inherent that says it must be.

I'm not sure why we aren't using chan->scan_type.shift though.

> or maybe
> 		*val = get_unaligned_be24(tmp) >> (sizeof(tmp) - chan->scan_type.realbits);

That one needs a BYTES_TO_BITS factor too.

> but it starts becoming too long IMO. Even longer if `tmp` gets a more meaningful
> name. Ah well, any of the three forms should work the same at the end of day so
> no strong opinion.
> 
> Reviewed-by: Marcelo Schmitt <marcelo.schmitt1@gmail.com>
> 
> >  		return IIO_VAL_INT;
> >  	}
> >  	case IIO_TEMP: { /* in 0.0625 celsius / LSB */
> > -- 
> > 2.25.1
> >   

Re: [PATCH v3 3/3] iio: ABI: document pressure event attributes

[Jonathan Cameron]

On Thu, 6 Nov 2025 23:32:07 -0300
Marcelo Schmitt <marcelo.schmitt1@gmail.com> wrote:

> On 11/05, Antoni Pokusinski wrote:
> > Add sysfs pressure event attributes exposed by the mpl3115 driver. These
> > allow controlling the threshold value and the enable state.
> > 
> > Signed-off-by: Antoni Pokusinski <apokusinski01@gmail.com>
> > ---
> >  Documentation/ABI/testing/sysfs-bus-iio | 2 ++
> >  1 file changed, 2 insertions(+)
> > 
> > diff --git a/Documentation/ABI/testing/sysfs-bus-iio b/Documentation/ABI/testing/sysfs-bus-iio
> > index 352ab7b8476c..5f87dcee78f7 100644
> > --- a/Documentation/ABI/testing/sysfs-bus-iio
> > +++ b/Documentation/ABI/testing/sysfs-bus-iio
> > @@ -898,6 +898,7 @@ What:		/sys/.../iio:deviceX/events/in_tempY_thresh_rising_en
> >  What:		/sys/.../iio:deviceX/events/in_tempY_thresh_falling_en
> >  What:		/sys/.../iio:deviceX/events/in_capacitanceY_thresh_rising_en
> >  What:		/sys/.../iio:deviceX/events/in_capacitanceY_thresh_falling_en
> > +What:		/sys/.../iio:deviceX/events/in_pressure_thresh_rising_en
> >  KernelVersion:	2.6.37
> >  Contact:	linux-iio@vger.kernel.org
> >  Description:
> > @@ -1047,6 +1048,7 @@ What:		/sys/.../events/in_capacitanceY_thresh_rising_value
> >  What:		/sys/.../events/in_capacitanceY_thresh_falling_value
> >  What:		/sys/.../events/in_capacitanceY_thresh_adaptive_rising_value
> >  What:		/sys/.../events/in_capacitanceY_thresh_falling_rising_value
> > +What:		/sys/.../events/in_pressure_thresh_rising_value  
> This is how it's currently appearing in sysfs, right?
> If so, IIO event sysfs entry generation might need a tweak.
Tell me more... Why might we need a tweak around this?

Re: [PATCH v3 1/3] iio: mpl3115: use get_unaligned_be24 to retrieve pressure data

[Antoni Pokusinski]

On Sun, Nov 09, 2025 at 04:38:40PM +0000, Jonathan Cameron wrote:
> On Thu, 6 Nov 2025 22:33:49 -0300
> Marcelo Schmitt <marcelo.schmitt1@gmail.com> wrote:
> 
> > On 11/05, Antoni Pokusinski wrote:
> > > The pressure measurement result is arranged as 20-bit unsigned value
> > > residing in three 8-bit registers. Hence, it can be retrieved using
> > > get_unaligned_be24 and by applying 4-bit shift.
> > > 
> > > Signed-off-by: Antoni Pokusinski <apokusinski01@gmail.com>
> > > ---
> > >  drivers/iio/pressure/mpl3115.c | 7 ++++---
> > >  1 file changed, 4 insertions(+), 3 deletions(-)
> > > 
> > > diff --git a/drivers/iio/pressure/mpl3115.c b/drivers/iio/pressure/mpl3115.c  
> > ...
> > >  
> > > -		*val = be32_to_cpu(tmp) >> chan->scan_type.shift;
> > > +		*val = get_unaligned_be24(tmp) >> 4;  
> > hmm, now the number of bits shifted is dissociated from the channel characteristics.
> > We can do
> > 		*val = get_unaligned_be24(tmp) >> (24 - chan->scan_type.realbits);
> This encodes that the field is always aligned to the maximum bit. Whilst it might
> be true, there is nothing inherent that says it must be.
> 
> I'm not sure why we aren't using chan->scan_type.shift though.
The chan->scan_type.shift is 12 for the pressure channel, because
.realbits is 32. In order to better reflect the actual data format,
the pressure .shift and .realbits should be changed to 4 and 24 respectively
and the we could use the chan->scan_type.shift in here indeed.

But then the `iio_generic_buffer` tool should also be updated so that it
can manage the scan_data with realbits not being in the form 2^n.
Currently it supports only scan sizes of 1,2,4,8 bytes [1].

[1]: https://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git/tree/tools/iio/iio_generic_buffer.c#n189
> 
> > or maybe
> > 		*val = get_unaligned_be24(tmp) >> (sizeof(tmp) - chan->scan_type.realbits);
> 
> That one needs a BYTES_TO_BITS factor too.
> 
> > but it starts becoming too long IMO. Even longer if `tmp` gets a more meaningful
> > name. Ah well, any of the three forms should work the same at the end of day so
> > no strong opinion.
> > 
> > Reviewed-by: Marcelo Schmitt <marcelo.schmitt1@gmail.com>
> > 
> > >  		return IIO_VAL_INT;
> > >  	}
> > >  	case IIO_TEMP: { /* in 0.0625 celsius / LSB */
> > > -- 
> > > 2.25.1
> > >   
>
Kind regards,
Antoni Pokusinski

Re: [PATCH v3 1/3] iio: mpl3115: use get_unaligned_be24 to retrieve pressure data

[Jonathan Cameron]

On Mon, 10 Nov 2025 16:59:32 +0100
Antoni Pokusinski <apokusinski01@gmail.com> wrote:

> On Sun, Nov 09, 2025 at 04:38:40PM +0000, Jonathan Cameron wrote:
> > On Thu, 6 Nov 2025 22:33:49 -0300
> > Marcelo Schmitt <marcelo.schmitt1@gmail.com> wrote:
> >   
> > > On 11/05, Antoni Pokusinski wrote:  
> > > > The pressure measurement result is arranged as 20-bit unsigned value
> > > > residing in three 8-bit registers. Hence, it can be retrieved using
> > > > get_unaligned_be24 and by applying 4-bit shift.
> > > > 
> > > > Signed-off-by: Antoni Pokusinski <apokusinski01@gmail.com>
> > > > ---
> > > >  drivers/iio/pressure/mpl3115.c | 7 ++++---
> > > >  1 file changed, 4 insertions(+), 3 deletions(-)
> > > > 
> > > > diff --git a/drivers/iio/pressure/mpl3115.c b/drivers/iio/pressure/mpl3115.c    
> > > ...  
> > > >  
> > > > -		*val = be32_to_cpu(tmp) >> chan->scan_type.shift;
> > > > +		*val = get_unaligned_be24(tmp) >> 4;    
> > > hmm, now the number of bits shifted is dissociated from the channel characteristics.
> > > We can do
> > > 		*val = get_unaligned_be24(tmp) >> (24 - chan->scan_type.realbits);  
> > This encodes that the field is always aligned to the maximum bit. Whilst it might
> > be true, there is nothing inherent that says it must be.
> > 
> > I'm not sure why we aren't using chan->scan_type.shift though.  
> The chan->scan_type.shift is 12 for the pressure channel, because
> .realbits is 32. In order to better reflect the actual data format,
> the pressure .shift and .realbits should be changed to 4 and 24 respectively
> and the we could use the chan->scan_type.shift in here indeed.
> 
> But then the `iio_generic_buffer` tool should also be updated so that it
> can manage the scan_data with realbits not being in the form 2^n.
> Currently it supports only scan sizes of 1,2,4,8 bytes [1].

> 
> [1]: https://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git/tree/tools/iio/iio_generic_buffer.c#n189

I think this is confusing storagebits and realbits.

storagebits is always power of 2 * 8 because we want them naturally aligned for
efficient accesses.  realbits is however many bits of actual data we have, so once we
shift off the bottom "shift" bits, how many to mask.
  
This confusion isn't helped by inconsistent names between that
tool and the kernel. 

Anyhow, I indeed now see why it is shifted by 4 here - thanks for talking me through it!

You could do the much messier
*val = get_unaligned_be24(tmp) >> (chan->scan_type.shift - (chan->scan_type_storage_bits - 24));
Which is hideous.  Perhaps a comment will do the job.

/*
 * Note that chan->scan_type.shift accounts for 24 bit big endian data being
 * read into the lower addresses of a 32 bit buffer - hence shift here is 4 rather
 * than 12.
 */

Or as another option. Could do in _fill_trig_buffer() do

ret = i2c_smbus_read_i2c_block_data(data->client,
			MPL3115_OUT_PRESS, 3, &buffer[pos + 1]);
Then set the shift for the pressure channel to 4.  That is, read the 3 bytes
after leave the most significant byte as 0. 

Whilst technically an ABI change, and correctly written software shouldn't notice.

Jonathan

 
> >   
> > > or maybe
> > > 		*val = get_unaligned_be24(tmp) >> (sizeof(tmp) - chan->scan_type.realbits);  
> > 
> > That one needs a BYTES_TO_BITS factor too.
> >   
> > > but it starts becoming too long IMO. Even longer if `tmp` gets a more meaningful
> > > name. Ah well, any of the three forms should work the same at the end of day so
> > > no strong opinion.
> > > 
> > > Reviewed-by: Marcelo Schmitt <marcelo.schmitt1@gmail.com>
> > >   
> > > >  		return IIO_VAL_INT;
> > > >  	}
> > > >  	case IIO_TEMP: { /* in 0.0625 celsius / LSB */
> > > > -- 
> > > > 2.25.1
> > > >     
> >  
> Kind regards,
> Antoni Pokusinski
>