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

Antoni Pokusinski posted 3 patches 1 month, 2 weeks ago
There is a newer version of this series
[PATCH v3 2/3] iio: mpl3115: add threshold events support
Posted by Antoni Pokusinski 1 month, 2 weeks ago
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
Re: [PATCH v3 2/3] iio: mpl3115: add threshold events support
Posted by Marcelo Schmitt 1 month, 1 week ago
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 2/3] iio: mpl3115: add threshold events support
Posted by Antoni Pokusinski 1 month, 1 week ago
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
Posted by Marcelo Schmitt 1 month, 1 week ago
...
> > > +	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
Posted by Andy Shevchenko 1 month, 1 week ago
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 2/3] iio: mpl3115: add threshold events support
Posted by Andy Shevchenko 1 month, 1 week ago
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
Posted by Antoni Pokusinski 1 month, 1 week ago
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