[PATCH 0/3] iio: mpl3115: add support for DRDY interrupt

[PATCH 0/3] iio: mpl3115: add support for DRDY interrupt

[Antoni Pokusinski]

Hello,
This set of patches adds support for the DRDY interrupt in the MPL3115
pressure sensor. The device has 2 interrupt lines, hence the new
binding. I also added support for the sampling frequency which
determines the time interval between subsequent measurements (in the
continuous measurements mode) so it's obiously tied to the DRDY
interrupt feature.

Kind regards,
Antoni Pokusinski

Antoni Pokusinski (3):
  dt-bindings: iio: pressure: add binding for mpl3115
  iio: mpl3115: add support for DRDY interrupt
  iio: mpl3115: add support for sampling frequency

 .../bindings/iio/pressure/fsl,mpl3115.yaml    |  63 +++++
 .../devicetree/bindings/trivial-devices.yaml  |   2 -
 drivers/iio/pressure/mpl3115.c                | 247 +++++++++++++++++-
 3 files changed, 305 insertions(+), 7 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/iio/pressure/fsl,mpl3115.yaml

-- 
2.25.1

[PATCH 1/3] dt-bindings: iio: pressure: add binding for mpl3115

[Antoni Pokusinski]

MPL3115 is an I2C pressure and temperature sensor. It features 2
interrupt lines which can be configured to indicate events such as data
ready or pressure/temperature threshold reached.

Signed-off-by: Antoni Pokusinski <apokusinski01@gmail.com>
---
 .../bindings/iio/pressure/fsl,mpl3115.yaml    | 63 +++++++++++++++++++
 .../devicetree/bindings/trivial-devices.yaml  |  2 -
 2 files changed, 63 insertions(+), 2 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/iio/pressure/fsl,mpl3115.yaml

diff --git a/Documentation/devicetree/bindings/iio/pressure/fsl,mpl3115.yaml b/Documentation/devicetree/bindings/iio/pressure/fsl,mpl3115.yaml
new file mode 100644
index 000000000000..9e21eae7acc7
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/pressure/fsl,mpl3115.yaml
@@ -0,0 +1,63 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/iio/pressure/fsl,mpl3115.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: MPL3115 precision pressure sensor with altimetry
+
+maintainers:
+  - Antoni Pokusinski <apokusinski01@gmail.com>
+
+description: |
+  MPL3115 is a pressure/altitude and temperature sensor with I2C interface.
+  It features two programmable interrupt lines which indicate events such as
+  data ready or pressure/temperature threshold reached.
+  https://www.nxp.com/docs/en/data-sheet/MPL3115A2.pdf
+
+properties:
+  compatible:
+    const: fsl,mpl3115
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    minItems: 1
+    maxItems: 2
+
+  interrupt-names:
+    minItems: 1
+    maxItems: 2
+    items:
+      enum:
+        - INT1
+        - INT2
+
+  drive-open-drain:
+    type: boolean
+    description:
+      set if the specified interrupt pins should be configured as
+      open drain. If not set, defaults to push-pull.
+
+required:
+  - compatible
+  - reg
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/irq.h>
+    i2c {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        pressure@60 {
+            compatible = "fsl,mpl3115";
+            reg = <0x60>;
+            interrupt-parent = <&gpio1>;
+            interrupts = <4 IRQ_TYPE_EDGE_FALLING>;
+            interrupt-names = "INT2";
+        };
+    };
diff --git a/Documentation/devicetree/bindings/trivial-devices.yaml b/Documentation/devicetree/bindings/trivial-devices.yaml
index f3dd18681aa6..918d4a12d61c 100644
--- a/Documentation/devicetree/bindings/trivial-devices.yaml
+++ b/Documentation/devicetree/bindings/trivial-devices.yaml
@@ -113,8 +113,6 @@ properties:
           - fsl,mma7660
             # MMA8450Q: Xtrinsic Low-power, 3-axis Xtrinsic Accelerometer
           - fsl,mma8450
-            # MPL3115: Absolute Digital Pressure Sensor
-          - fsl,mpl3115
             # MPR121: Proximity Capacitive Touch Sensor Controller
           - fsl,mpr121
             # Honeywell Humidicon HIH-6130 humidity/temperature sensor
-- 
2.25.1

[PATCH 2/3] iio: mpl3115: add support for DRDY interrupt

[Antoni Pokusinski]

MPL3115 sensor features a "data ready" interrupt which indicates the
presence of new measurements.

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

diff --git a/drivers/iio/pressure/mpl3115.c b/drivers/iio/pressure/mpl3115.c
index 579da60ef441..cf34de8f0d7e 100644
--- a/drivers/iio/pressure/mpl3115.c
+++ b/drivers/iio/pressure/mpl3115.c
@@ -7,7 +7,7 @@
  * (7-bit I2C slave address 0x60)
  *
  * TODO: FIFO buffer, altimeter mode, oversampling, continuous mode,
- * interrupts, user offset correction, raw mode
+ * user offset correction, raw mode
  */
 
 #include <linux/module.h>
@@ -17,26 +17,45 @@
 #include <linux/iio/trigger_consumer.h>
 #include <linux/iio/buffer.h>
 #include <linux/iio/triggered_buffer.h>
+#include <linux/iio/trigger.h>
 #include <linux/delay.h>
+#include <linux/property.h>
 
 #define MPL3115_STATUS 0x00
 #define MPL3115_OUT_PRESS 0x01 /* MSB first, 20 bit */
 #define MPL3115_OUT_TEMP 0x04 /* MSB first, 12 bit */
 #define MPL3115_WHO_AM_I 0x0c
+#define MPL3115_INT_SOURCE 0x12
+#define MPL3115_PT_DATA_CFG 0x13
 #define MPL3115_CTRL_REG1 0x26
+#define MPL3115_CTRL_REG3 0x28
+#define MPL3115_CTRL_REG4 0x29
+#define MPL3115_CTRL_REG5 0x2a
 
 #define MPL3115_DEVICE_ID 0xc4
 
 #define MPL3115_STATUS_PRESS_RDY BIT(2)
 #define MPL3115_STATUS_TEMP_RDY BIT(1)
 
+#define MPL3115_CTRL_INT_SRC_DRDY BIT(7)
+
+#define MPL3115_PT_DATA_EVENT_ALL (BIT(2) | BIT(1) | BIT(0))
+
 #define MPL3115_CTRL_RESET BIT(2) /* software reset */
 #define MPL3115_CTRL_OST BIT(1) /* initiate measurement */
 #define MPL3115_CTRL_ACTIVE BIT(0) /* continuous measurement */
 #define MPL3115_CTRL_OS_258MS (BIT(5) | BIT(4)) /* 64x oversampling */
 
+#define MPL3115_CTRL_IPOL1 BIT(5)
+#define MPL3115_CTRL_IPOL2 BIT(1)
+
+#define MPL3115_CTRL_INT_EN_DRDY BIT(7)
+
+#define MPL3115_CTRL_INT_CFG_DRDY BIT(7)
+
 struct mpl3115_data {
 	struct i2c_client *client;
+	struct iio_trigger *drdy_trig;
 	struct mutex lock;
 	u8 ctrl_reg1;
 };
@@ -164,10 +183,12 @@ static irqreturn_t mpl3115_trigger_handler(int irq, void *p)
 	int ret, pos = 0;
 
 	mutex_lock(&data->lock);
-	ret = mpl3115_request(data);
-	if (ret < 0) {
-		mutex_unlock(&data->lock);
-		goto done;
+	if (!(data->ctrl_reg1 & MPL3115_CTRL_ACTIVE)) {
+		ret = mpl3115_request(data);
+		if (ret < 0) {
+			mutex_unlock(&data->lock);
+			goto done;
+		}
 	}
 
 	if (test_bit(0, indio_dev->active_scan_mask)) {
@@ -228,10 +249,142 @@ static const struct iio_chan_spec mpl3115_channels[] = {
 	IIO_CHAN_SOFT_TIMESTAMP(2),
 };
 
+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;
+
+	ret = i2c_smbus_read_byte_data(data->client, MPL3115_INT_SOURCE);
+	if (ret < 0)
+		return IRQ_HANDLED;
+
+	if (!(ret & MPL3115_CTRL_INT_SRC_DRDY))
+		return IRQ_NONE;
+
+	iio_trigger_poll_nested(data->drdy_trig);
+
+	return IRQ_HANDLED;
+}
+
+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);
+	int ret;
+	u8 ctrl_reg1 = data->ctrl_reg1;
+
+	if (state)
+		ctrl_reg1 |= MPL3115_CTRL_ACTIVE;
+	else
+		ctrl_reg1 &= ~MPL3115_CTRL_ACTIVE;
+
+	guard(mutex)(&data->lock);
+
+	ret = i2c_smbus_write_byte_data(data->client, MPL3115_CTRL_REG1,
+					ctrl_reg1);
+	if (ret < 0)
+		return ret;
+
+	ret = i2c_smbus_write_byte_data(data->client, MPL3115_CTRL_REG4,
+					state ? MPL3115_CTRL_INT_EN_DRDY : 0);
+	if (ret < 0)
+		goto reg1_cleanup;
+
+	data->ctrl_reg1 = ctrl_reg1;
+
+	return 0;
+
+reg1_cleanup:
+	i2c_smbus_write_byte_data(data->client, MPL3115_CTRL_REG1,
+				  data->ctrl_reg1);
+	return ret;
+}
+
+static const struct iio_trigger_ops mpl3115_trigger_ops = {
+	.set_trigger_state = mpl3115_set_trigger_state,
+};
+
 static const struct iio_info mpl3115_info = {
 	.read_raw = &mpl3115_read_raw,
 };
 
+static int mpl3115_trigger_probe(struct mpl3115_data *data,
+				 struct iio_dev *indio_dev)
+{
+	struct fwnode_handle *fwnode;
+	int ret, irq, irq_type;
+	bool act_high, is_int2 = false;
+
+	fwnode = dev_fwnode(&data->client->dev);
+	if (!fwnode)
+		return -ENODEV;
+
+	irq = fwnode_irq_get_byname(fwnode, "INT1");
+	if (irq < 0) {
+		irq = fwnode_irq_get_byname(fwnode, "INT2");
+		if (irq < 0)
+			return 0;
+
+		is_int2 = true;
+	}
+
+	irq_type = irq_get_trigger_type(irq);
+	switch (irq_type) {
+	case IRQF_TRIGGER_RISING:
+		act_high = true;
+		break;
+	case IRQF_TRIGGER_FALLING:
+		act_high = false;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	ret = i2c_smbus_write_byte_data(data->client, MPL3115_PT_DATA_CFG,
+					MPL3115_PT_DATA_EVENT_ALL);
+	if (ret < 0)
+		return ret;
+
+	if (!is_int2) {
+		ret = i2c_smbus_write_byte_data(data->client,
+						MPL3115_CTRL_REG5,
+						MPL3115_CTRL_INT_CFG_DRDY);
+		if (ret)
+			return ret;
+	}
+	if (act_high) {
+		ret = i2c_smbus_write_byte_data(data->client,
+						MPL3115_CTRL_REG3,
+						is_int2 ? MPL3115_CTRL_IPOL2 :
+							  MPL3115_CTRL_IPOL1);
+		if (ret)
+			return ret;
+	}
+
+	data->drdy_trig = devm_iio_trigger_alloc(&data->client->dev,
+						 "%s-dev%d",
+						 indio_dev->name,
+						 iio_device_id(indio_dev));
+	if (!data->drdy_trig)
+		return -ENOMEM;
+
+	data->drdy_trig->ops = &mpl3115_trigger_ops;
+	iio_trigger_set_drvdata(data->drdy_trig, indio_dev);
+	ret = iio_trigger_register(data->drdy_trig);
+	if (ret)
+		return ret;
+
+	indio_dev->trig = iio_trigger_get(data->drdy_trig);
+
+	return devm_request_threaded_irq(&data->client->dev, irq,
+					 NULL,
+					 mpl3115_interrupt_handler,
+					 IRQF_ONESHOT,
+					 "mpl3115_irq",
+					 indio_dev);
+}
+
 static int mpl3115_probe(struct i2c_client *client)
 {
 	const struct i2c_device_id *id = i2c_client_get_device_id(client);
@@ -271,6 +424,10 @@ static int mpl3115_probe(struct i2c_client *client)
 	if (ret < 0)
 		return ret;
 
+	ret = mpl3115_trigger_probe(data, indio_dev);
+	if (ret)
+		return ret;
+
 	ret = iio_triggered_buffer_setup(indio_dev, NULL,
 		mpl3115_trigger_handler, NULL);
 	if (ret < 0)
-- 
2.25.1

[PATCH 3/3] iio: mpl3115: add support for sampling frequency

[Antoni Pokusinski]

When the device is in ACTIVE mode the temperature and pressure measurements
are collected with a frequency determined by the ST[3:0] bits of CTRL_REG2
register.

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

diff --git a/drivers/iio/pressure/mpl3115.c b/drivers/iio/pressure/mpl3115.c
index cf34de8f0d7e..2f1860ca1f32 100644
--- a/drivers/iio/pressure/mpl3115.c
+++ b/drivers/iio/pressure/mpl3115.c
@@ -28,6 +28,7 @@
 #define MPL3115_INT_SOURCE 0x12
 #define MPL3115_PT_DATA_CFG 0x13
 #define MPL3115_CTRL_REG1 0x26
+#define MPL3115_CTRL_REG2 0x27
 #define MPL3115_CTRL_REG3 0x28
 #define MPL3115_CTRL_REG4 0x29
 #define MPL3115_CTRL_REG5 0x2a
@@ -46,6 +47,8 @@
 #define MPL3115_CTRL_ACTIVE BIT(0) /* continuous measurement */
 #define MPL3115_CTRL_OS_258MS (BIT(5) | BIT(4)) /* 64x oversampling */
 
+#define MPL3115_CTRL_ST (BIT(3) | BIT(2) | BIT(1) | BIT(0))
+
 #define MPL3115_CTRL_IPOL1 BIT(5)
 #define MPL3115_CTRL_IPOL2 BIT(1)
 
@@ -53,6 +56,25 @@
 
 #define MPL3115_CTRL_INT_CFG_DRDY BIT(7)
 
+static const unsigned int mpl3115_samp_freq_table[][2] = {
+	{ 1,      0},
+	{ 0, 500000},
+	{ 0, 250000},
+	{ 0, 125000},
+	{ 0,  62500},
+	{ 0,  31250},
+	{ 0,  15625},
+	{ 0,   7812},
+	{ 0,   3906},
+	{ 0,   1953},
+	{ 0,    976},
+	{ 0,    488},
+	{ 0,    244},
+	{ 0,    122},
+	{ 0,     61},
+	{ 0,     30},
+};
+
 struct mpl3115_data {
 	struct i2c_client *client;
 	struct iio_trigger *drdy_trig;
@@ -163,10 +185,60 @@ static int mpl3115_read_raw(struct iio_dev *indio_dev,
 		default:
 			return -EINVAL;
 		}
+	case IIO_CHAN_INFO_SAMP_FREQ:
+		ret = i2c_smbus_read_byte_data(data->client, MPL3115_CTRL_REG2);
+		if (ret < 0)
+			return ret;
+
+		ret &= MPL3115_CTRL_ST;
+
+		*val = mpl3115_samp_freq_table[ret][0];
+		*val2 = mpl3115_samp_freq_table[ret][1];
+		return IIO_VAL_INT_PLUS_MICRO;
 	}
 	return -EINVAL;
 }
 
+static int mpl3115_read_avail(struct iio_dev *indio_dev,
+			      struct iio_chan_spec const *chan,
+			      const int **vals, int *type, int *length,
+			      long mask)
+{
+	if (mask != IIO_CHAN_INFO_SAMP_FREQ)
+		return -EINVAL;
+
+	*type = IIO_VAL_INT_PLUS_MICRO;
+	*length = ARRAY_SIZE(mpl3115_samp_freq_table) * 2;
+	*vals = (int *)mpl3115_samp_freq_table;
+	return IIO_AVAIL_LIST;
+}
+
+static int mpl3115_write_raw(struct iio_dev *indio_dev,
+			     const struct iio_chan_spec *chan,
+			     int val, int val2, long mask)
+{
+	struct mpl3115_data *data = iio_priv(indio_dev);
+	int i, ret;
+
+	if (mask != IIO_CHAN_INFO_SAMP_FREQ)
+		return -EINVAL;
+
+	for (i = 0; i < ARRAY_SIZE(mpl3115_samp_freq_table); i++)
+		if (val == mpl3115_samp_freq_table[i][0] &&
+		    val2 == mpl3115_samp_freq_table[i][1])
+			break;
+
+	if (i == ARRAY_SIZE(mpl3115_samp_freq_table))
+		return -EINVAL;
+
+	if (!iio_device_claim_direct(indio_dev))
+		return -EBUSY;
+
+	ret = i2c_smbus_write_byte_data(data->client, MPL3115_CTRL_REG2, i);
+	iio_device_release_direct(indio_dev);
+	return ret;
+}
+
 static irqreturn_t mpl3115_trigger_handler(int irq, void *p)
 {
 	struct iio_poll_func *pf = p;
@@ -224,6 +296,9 @@ static const struct iio_chan_spec mpl3115_channels[] = {
 		.type = IIO_PRESSURE,
 		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),
 		.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE),
+		.info_mask_shared_by_all = BIT(IIO_CHAN_INFO_SAMP_FREQ),
+		.info_mask_shared_by_all_available =
+			BIT(IIO_CHAN_INFO_SAMP_FREQ),
 		.scan_index = 0,
 		.scan_type = {
 			.sign = 'u',
@@ -237,6 +312,9 @@ static const struct iio_chan_spec mpl3115_channels[] = {
 		.type = IIO_TEMP,
 		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),
 		.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE),
+		.info_mask_shared_by_all = BIT(IIO_CHAN_INFO_SAMP_FREQ),
+		.info_mask_shared_by_all_available =
+			BIT(IIO_CHAN_INFO_SAMP_FREQ),
 		.scan_index = 1,
 		.scan_type = {
 			.sign = 's',
@@ -307,6 +385,8 @@ static const struct iio_trigger_ops mpl3115_trigger_ops = {
 
 static const struct iio_info mpl3115_info = {
 	.read_raw = &mpl3115_read_raw,
+	.read_avail = &mpl3115_read_avail,
+	.write_raw = &mpl3115_write_raw,
 };
 
 static int mpl3115_trigger_probe(struct mpl3115_data *data,
-- 
2.25.1

Re: [PATCH 2/3] iio: mpl3115: add support for DRDY interrupt

[Andy Shevchenko]

On Sun, Sep 21, 2025 at 4:34 PM Antoni Pokusinski
<apokusinski01@gmail.com> wrote:
>
> MPL3115 sensor features a "data ready" interrupt which indicates the
> presence of new measurements.

...

>  #include <linux/module.h>

>  #include <linux/iio/trigger_consumer.h>
>  #include <linux/iio/buffer.h>
>  #include <linux/iio/triggered_buffer.h>
> +#include <linux/iio/trigger.h>
>  #include <linux/delay.h>

> +#include <linux/property.h>

This is like delay.h is misplaced. What we do here, we group generic
ones followed by subsystem (IIO) related ones, and this seems wrong in
this driver.

Can you rather move delay.h to be the first, and add property.h after
i2c.h followed by a blank line, so in the result it will be like

delay.h
i2c.h
module.h
property.h
...blank.line...
iio/*.h

...

> +#define MPL3115_CTRL_INT_SRC_DRDY BIT(7)
> +
> +#define MPL3115_PT_DATA_EVENT_ALL (BIT(2) | BIT(1) | BIT(0))

Not sure I understand this definition in the following aspects:
1) why is this disrupting the _CTRL_ definitions?
2) why is this using BIT(x) and not respective definitions?
3) why can't you use GENMASK() if you just select all bits in a
certain bitfield?


>  #define MPL3115_CTRL_RESET BIT(2) /* software reset */
>  #define MPL3115_CTRL_OST BIT(1) /* initiate measurement */
>  #define MPL3115_CTRL_ACTIVE BIT(0) /* continuous measurement */
>  #define MPL3115_CTRL_OS_258MS (BIT(5) | BIT(4)) /* 64x oversampling */

...

>         mutex_lock(&data->lock);
> -       ret = mpl3115_request(data);
> -       if (ret < 0) {
> -               mutex_unlock(&data->lock);
> -               goto done;
> +       if (!(data->ctrl_reg1 & MPL3115_CTRL_ACTIVE)) {
> +               ret = mpl3115_request(data);
> +               if (ret < 0) {

> +                       mutex_unlock(&data->lock);

Instead, I suggest adding a prerequisite that moves the driver to use
cleanup.h, in particular scoped_guard(). This will reduce a churn
here,

> +                       goto done;
> +               }
>         }

...

> +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);
> +       int ret;
> +       u8 ctrl_reg1 = data->ctrl_reg1;
> +
> +       if (state)
> +               ctrl_reg1 |= MPL3115_CTRL_ACTIVE;
> +       else
> +               ctrl_reg1 &= ~MPL3115_CTRL_ACTIVE;

> +       guard(mutex)(&data->lock);

Oh, and you already use this! Definitely, it misses the prerequisite patch.

> +       ret = i2c_smbus_write_byte_data(data->client, MPL3115_CTRL_REG1,
> +                                       ctrl_reg1);
> +       if (ret < 0)
> +               return ret;
> +
> +       ret = i2c_smbus_write_byte_data(data->client, MPL3115_CTRL_REG4,
> +                                       state ? MPL3115_CTRL_INT_EN_DRDY : 0);
> +       if (ret < 0)
> +               goto reg1_cleanup;
> +
> +       data->ctrl_reg1 = ctrl_reg1;
> +
> +       return 0;
> +
> +reg1_cleanup:
> +       i2c_smbus_write_byte_data(data->client, MPL3115_CTRL_REG1,
> +                                 data->ctrl_reg1);
> +       return ret;
> +}

...

> +static int mpl3115_trigger_probe(struct mpl3115_data *data,
> +                                struct iio_dev *indio_dev)
> +{
> +       struct fwnode_handle *fwnode;
> +       int ret, irq, irq_type;
> +       bool act_high, is_int2 = false;

> +       fwnode = dev_fwnode(&data->client->dev);
> +       if (!fwnode)
> +               return -ENODEV;


Why is this fatal? Also, do we have a board file for users of this right now?

> +       irq = fwnode_irq_get_byname(fwnode, "INT1");
> +       if (irq < 0) {
> +               irq = fwnode_irq_get_byname(fwnode, "INT2");
> +               if (irq < 0)
> +                       return 0;
> +
> +               is_int2 = true;
> +       }
> +
> +       irq_type = irq_get_trigger_type(irq);
> +       switch (irq_type) {
> +       case IRQF_TRIGGER_RISING:
> +               act_high = true;
> +               break;
> +       case IRQF_TRIGGER_FALLING:
> +               act_high = false;
> +               break;
> +       default:
> +               return -EINVAL;
> +       }
> +
> +       ret = i2c_smbus_write_byte_data(data->client, MPL3115_PT_DATA_CFG,
> +                                       MPL3115_PT_DATA_EVENT_ALL);
> +       if (ret < 0)
> +               return ret;

> +       if (!is_int2) {
> +               ret = i2c_smbus_write_byte_data(data->client,
> +                                               MPL3115_CTRL_REG5,
> +                                               MPL3115_CTRL_INT_CFG_DRDY);
> +               if (ret)
> +                       return ret;
> +       }
> +       if (act_high) {
> +               ret = i2c_smbus_write_byte_data(data->client,
> +                                               MPL3115_CTRL_REG3,
> +                                               is_int2 ? MPL3115_CTRL_IPOL2 :
> +                                                         MPL3115_CTRL_IPOL1);
> +               if (ret)
> +                       return ret;
> +       }

This if (!is_int2) and ternary with the same argument is kinda hard to
read, can we refactor it somehow?

For example, if these two booleans are represented by a common enum, we can do

switch (cfg_flags) {
case INT2_ACTIVE_HIGH:
    _write_byte_data(REG3);
    break;
case INT2_ACTIVE_LOW:
    break;
case INT1_ACTIVE_HIGH:
   _write_byte_data(REG5);
   _write_byte_data(REG3);
  break;
case INT1_ACTIVE_LOW:
   _write_byte_data(REG5);
   break;
default:
    return -EINVAL;
}

Yes, it's more verbose, but I find this better to read and understand.

Note, you may drop the switch case for IRQ with this approach as you
can use a few bits together (separate bits for raising and falling to
make the default case working here).

> +       data->drdy_trig = devm_iio_trigger_alloc(&data->client->dev,
> +                                                "%s-dev%d",
> +                                                indio_dev->name,
> +                                                iio_device_id(indio_dev));
> +       if (!data->drdy_trig)
> +               return -ENOMEM;
> +
> +       data->drdy_trig->ops = &mpl3115_trigger_ops;
> +       iio_trigger_set_drvdata(data->drdy_trig, indio_dev);
> +       ret = iio_trigger_register(data->drdy_trig);
> +       if (ret)
> +               return ret;
> +
> +       indio_dev->trig = iio_trigger_get(data->drdy_trig);
> +
> +       return devm_request_threaded_irq(&data->client->dev, irq,
> +                                        NULL,
> +                                        mpl3115_interrupt_handler,
> +                                        IRQF_ONESHOT,
> +                                        "mpl3115_irq",
> +                                        indio_dev);
> +}

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 3/3] iio: mpl3115: add support for sampling frequency

[Nuno Sá]

On Sun, 2025-09-21 at 15:33 +0200, Antoni Pokusinski wrote:
> When the device is in ACTIVE mode the temperature and pressure measurements
> are collected with a frequency determined by the ST[3:0] bits of CTRL_REG2
> register.
> 
> Signed-off-by: Antoni Pokusinski <apokusinski01@gmail.com>
> ---

LGTM,

Reviewed-by: Nuno Sá <nuno.sa@analog.com>

>  drivers/iio/pressure/mpl3115.c | 80 ++++++++++++++++++++++++++++++++++
>  1 file changed, 80 insertions(+)
> 
> diff --git a/drivers/iio/pressure/mpl3115.c b/drivers/iio/pressure/mpl3115.c
> index cf34de8f0d7e..2f1860ca1f32 100644
> --- a/drivers/iio/pressure/mpl3115.c
> +++ b/drivers/iio/pressure/mpl3115.c
> @@ -28,6 +28,7 @@
>  #define MPL3115_INT_SOURCE 0x12
>  #define MPL3115_PT_DATA_CFG 0x13
>  #define MPL3115_CTRL_REG1 0x26
> +#define MPL3115_CTRL_REG2 0x27
>  #define MPL3115_CTRL_REG3 0x28
>  #define MPL3115_CTRL_REG4 0x29
>  #define MPL3115_CTRL_REG5 0x2a
> @@ -46,6 +47,8 @@
>  #define MPL3115_CTRL_ACTIVE BIT(0) /* continuous measurement */
>  #define MPL3115_CTRL_OS_258MS (BIT(5) | BIT(4)) /* 64x oversampling */
>  
> +#define MPL3115_CTRL_ST (BIT(3) | BIT(2) | BIT(1) | BIT(0))
> +
>  #define MPL3115_CTRL_IPOL1 BIT(5)
>  #define MPL3115_CTRL_IPOL2 BIT(1)
>  
> @@ -53,6 +56,25 @@
>  
>  #define MPL3115_CTRL_INT_CFG_DRDY BIT(7)
>  
> +static const unsigned int mpl3115_samp_freq_table[][2] = {
> +	{ 1,      0},
> +	{ 0, 500000},
> +	{ 0, 250000},
> +	{ 0, 125000},
> +	{ 0,  62500},
> +	{ 0,  31250},
> +	{ 0,  15625},
> +	{ 0,   7812},
> +	{ 0,   3906},
> +	{ 0,   1953},
> +	{ 0,    976},
> +	{ 0,    488},
> +	{ 0,    244},
> +	{ 0,    122},
> +	{ 0,     61},
> +	{ 0,     30},
> +};
> +
>  struct mpl3115_data {
>  	struct i2c_client *client;
>  	struct iio_trigger *drdy_trig;
> @@ -163,10 +185,60 @@ static int mpl3115_read_raw(struct iio_dev *indio_dev,
>  		default:
>  			return -EINVAL;
>  		}
> +	case IIO_CHAN_INFO_SAMP_FREQ:
> +		ret = i2c_smbus_read_byte_data(data->client,
> MPL3115_CTRL_REG2);
> +		if (ret < 0)
> +			return ret;
> +
> +		ret &= MPL3115_CTRL_ST;
> +
> +		*val = mpl3115_samp_freq_table[ret][0];
> +		*val2 = mpl3115_samp_freq_table[ret][1];
> +		return IIO_VAL_INT_PLUS_MICRO;
>  	}
>  	return -EINVAL;
>  }
>  
> +static int mpl3115_read_avail(struct iio_dev *indio_dev,
> +			      struct iio_chan_spec const *chan,
> +			      const int **vals, int *type, int *length,
> +			      long mask)
> +{
> +	if (mask != IIO_CHAN_INFO_SAMP_FREQ)
> +		return -EINVAL;
> +
> +	*type = IIO_VAL_INT_PLUS_MICRO;
> +	*length = ARRAY_SIZE(mpl3115_samp_freq_table) * 2;
> +	*vals = (int *)mpl3115_samp_freq_table;
> +	return IIO_AVAIL_LIST;
> +}
> +
> +static int mpl3115_write_raw(struct iio_dev *indio_dev,
> +			     const struct iio_chan_spec *chan,
> +			     int val, int val2, long mask)
> +{
> +	struct mpl3115_data *data = iio_priv(indio_dev);
> +	int i, ret;
> +
> +	if (mask != IIO_CHAN_INFO_SAMP_FREQ)
> +		return -EINVAL;
> +
> +	for (i = 0; i < ARRAY_SIZE(mpl3115_samp_freq_table); i++)
> +		if (val == mpl3115_samp_freq_table[i][0] &&
> +		    val2 == mpl3115_samp_freq_table[i][1])
> +			break;
> +
> +	if (i == ARRAY_SIZE(mpl3115_samp_freq_table))
> +		return -EINVAL;
> +
> +	if (!iio_device_claim_direct(indio_dev))
> +		return -EBUSY;
> +
> +	ret = i2c_smbus_write_byte_data(data->client, MPL3115_CTRL_REG2, i);
> +	iio_device_release_direct(indio_dev);
> +	return ret;
> +}
> +
>  static irqreturn_t mpl3115_trigger_handler(int irq, void *p)
>  {
>  	struct iio_poll_func *pf = p;
> @@ -224,6 +296,9 @@ static const struct iio_chan_spec mpl3115_channels[] = {
>  		.type = IIO_PRESSURE,
>  		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),
>  		.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE),
> +		.info_mask_shared_by_all = BIT(IIO_CHAN_INFO_SAMP_FREQ),
> +		.info_mask_shared_by_all_available =
> +			BIT(IIO_CHAN_INFO_SAMP_FREQ),
>  		.scan_index = 0,
>  		.scan_type = {
>  			.sign = 'u',
> @@ -237,6 +312,9 @@ static const struct iio_chan_spec mpl3115_channels[] = {
>  		.type = IIO_TEMP,
>  		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),
>  		.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE),
> +		.info_mask_shared_by_all = BIT(IIO_CHAN_INFO_SAMP_FREQ),
> +		.info_mask_shared_by_all_available =
> +			BIT(IIO_CHAN_INFO_SAMP_FREQ),
>  		.scan_index = 1,
>  		.scan_type = {
>  			.sign = 's',
> @@ -307,6 +385,8 @@ static const struct iio_trigger_ops mpl3115_trigger_ops =
> {
>  
>  static const struct iio_info mpl3115_info = {
>  	.read_raw = &mpl3115_read_raw,
> +	.read_avail = &mpl3115_read_avail,
> +	.write_raw = &mpl3115_write_raw,
>  };
>  
>  static int mpl3115_trigger_probe(struct mpl3115_data *data,

Re: [PATCH 2/3] iio: mpl3115: add support for DRDY interrupt

[Nuno Sá]

On Sun, 2025-09-21 at 15:33 +0200, Antoni Pokusinski wrote:
> MPL3115 sensor features a "data ready" interrupt which indicates the
> presence of new measurements.
> 
> Signed-off-by: Antoni Pokusinski <apokusinski01@gmail.com>
> ---
>  drivers/iio/pressure/mpl3115.c | 167 ++++++++++++++++++++++++++++++++-
>  1 file changed, 162 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/iio/pressure/mpl3115.c b/drivers/iio/pressure/mpl3115.c
> index 579da60ef441..cf34de8f0d7e 100644
> --- a/drivers/iio/pressure/mpl3115.c
> +++ b/drivers/iio/pressure/mpl3115.c
> @@ -7,7 +7,7 @@
>   * (7-bit I2C slave address 0x60)
>   *
>   * TODO: FIFO buffer, altimeter mode, oversampling, continuous mode,
> - * interrupts, user offset correction, raw mode
> + * user offset correction, raw mode
>   */
>  
>  #include <linux/module.h>
> @@ -17,26 +17,45 @@
>  #include <linux/iio/trigger_consumer.h>
>  #include <linux/iio/buffer.h>
>  #include <linux/iio/triggered_buffer.h>
> +#include <linux/iio/trigger.h>
>  #include <linux/delay.h>
> +#include <linux/property.h>
>  
>  #define MPL3115_STATUS 0x00
>  #define MPL3115_OUT_PRESS 0x01 /* MSB first, 20 bit */
>  #define MPL3115_OUT_TEMP 0x04 /* MSB first, 12 bit */
>  #define MPL3115_WHO_AM_I 0x0c
> +#define MPL3115_INT_SOURCE 0x12
> +#define MPL3115_PT_DATA_CFG 0x13
>  #define MPL3115_CTRL_REG1 0x26
> +#define MPL3115_CTRL_REG3 0x28
> +#define MPL3115_CTRL_REG4 0x29
> +#define MPL3115_CTRL_REG5 0x2a
>  
>  #define MPL3115_DEVICE_ID 0xc4
>  
>  #define MPL3115_STATUS_PRESS_RDY BIT(2)
>  #define MPL3115_STATUS_TEMP_RDY BIT(1)
>  
> +#define MPL3115_CTRL_INT_SRC_DRDY BIT(7)
> +
> +#define MPL3115_PT_DATA_EVENT_ALL (BIT(2) | BIT(1) | BIT(0))
> +
>  #define MPL3115_CTRL_RESET BIT(2) /* software reset */
>  #define MPL3115_CTRL_OST BIT(1) /* initiate measurement */
>  #define MPL3115_CTRL_ACTIVE BIT(0) /* continuous measurement */
>  #define MPL3115_CTRL_OS_258MS (BIT(5) | BIT(4)) /* 64x oversampling */
>  
> +#define MPL3115_CTRL_IPOL1 BIT(5)
> +#define MPL3115_CTRL_IPOL2 BIT(1)
> +
> +#define MPL3115_CTRL_INT_EN_DRDY BIT(7)
> +
> +#define MPL3115_CTRL_INT_CFG_DRDY BIT(7)
> +
>  struct mpl3115_data {
>  	struct i2c_client *client;
> +	struct iio_trigger *drdy_trig;
>  	struct mutex lock;
>  	u8 ctrl_reg1;
>  };
> @@ -164,10 +183,12 @@ static irqreturn_t mpl3115_trigger_handler(int irq, void
> *p)
>  	int ret, pos = 0;
>  
>  	mutex_lock(&data->lock);
> -	ret = mpl3115_request(data);
> -	if (ret < 0) {
> -		mutex_unlock(&data->lock);
> -		goto done;
> +	if (!(data->ctrl_reg1 & MPL3115_CTRL_ACTIVE)) {
> +		ret = mpl3115_request(data);
> +		if (ret < 0) {
> +			mutex_unlock(&data->lock);
> +			goto done;
> +		}
>  	}
>  
>  	if (test_bit(0, indio_dev->active_scan_mask)) {
> @@ -228,10 +249,142 @@ static const struct iio_chan_spec mpl3115_channels[] =
> {
>  	IIO_CHAN_SOFT_TIMESTAMP(2),
>  };
>  
> +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;
> +
> +	ret = i2c_smbus_read_byte_data(data->client, MPL3115_INT_SOURCE);
> +	if (ret < 0)
> +		return IRQ_HANDLED;
> +
> +	if (!(ret & MPL3115_CTRL_INT_SRC_DRDY))
> +		return IRQ_NONE;
> +
> +	iio_trigger_poll_nested(data->drdy_trig);
> +
> +	return IRQ_HANDLED;
> +}
> +
> +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);
> +	int ret;
> +	u8 ctrl_reg1 = data->ctrl_reg1;
> +
> +	if (state)
> +		ctrl_reg1 |= MPL3115_CTRL_ACTIVE;
> +	else
> +		ctrl_reg1 &= ~MPL3115_CTRL_ACTIVE;
> +
> +	guard(mutex)(&data->lock);
> +

As Andy pointed out, you should have a precursor patch converting the complete
driver to use the cleanup logic.

Another nice cleanup you could do (if you want of course) would be to get rid of
mpl3115_remove().

> +	ret = i2c_smbus_write_byte_data(data->client, MPL3115_CTRL_REG1,
> +					ctrl_reg1);
> +	if (ret < 0)
> +		return ret;
> +
> +	ret = i2c_smbus_write_byte_data(data->client, MPL3115_CTRL_REG4,
> +					state ? MPL3115_CTRL_INT_EN_DRDY :
> 0);
> +	if (ret < 0)
> +		goto reg1_cleanup;
> +
> +	data->ctrl_reg1 = ctrl_reg1;
> +
> +	return 0;
> +
> +reg1_cleanup:
> +	i2c_smbus_write_byte_data(data->client, MPL3115_CTRL_REG1,
> +				  data->ctrl_reg1);
> +	return ret;
> +}
> +
> +static const struct iio_trigger_ops mpl3115_trigger_ops = {
> +	.set_trigger_state = mpl3115_set_trigger_state,
> +};
> +
>  static const struct iio_info mpl3115_info = {
>  	.read_raw = &mpl3115_read_raw,
>  };
>  
> +static int mpl3115_trigger_probe(struct mpl3115_data *data,
> +				 struct iio_dev *indio_dev)
> +{
> +	struct fwnode_handle *fwnode;
> +	int ret, irq, irq_type;
> +	bool act_high, is_int2 = false;
> +
> +	fwnode = dev_fwnode(&data->client->dev);
> +	if (!fwnode)
> +		return -ENODEV;
> +

And to add to Andy's review, fwnode_irq_get_byname() will give you an error
anyways if !fwnode.

> +	irq = fwnode_irq_get_byname(fwnode, "INT1");
> +	if (irq < 0) {
> +		irq = fwnode_irq_get_byname(fwnode, "INT2");
> +		if (irq < 0)
> +			return 0;
> +
> +		is_int2 = true;
> +	}
> +
> +	irq_type = irq_get_trigger_type(irq);
> +	switch (irq_type) {
> +	case IRQF_TRIGGER_RISING:
> +		act_high = true;
> +		break;
> +	case IRQF_TRIGGER_FALLING:
> +		act_high = false;
> +		break;
> +	default:
> +		return -EINVAL;
> +	}
> +
> +	ret = i2c_smbus_write_byte_data(data->client, MPL3115_PT_DATA_CFG,
> +					MPL3115_PT_DATA_EVENT_ALL);
> +	if (ret < 0)
> +		return ret;
> +
> +	if (!is_int2) {
> +		ret = i2c_smbus_write_byte_data(data->client,
> +						MPL3115_CTRL_REG5,
> +						MPL3115_CTRL_INT_CFG_DRDY);
> +		if (ret)
> +			return ret;
> +	}
> +	if (act_high) {
> +		ret = i2c_smbus_write_byte_data(data->client,
> +						MPL3115_CTRL_REG3,
> +						is_int2 ? MPL3115_CTRL_IPOL2
> :
> +							 
> MPL3115_CTRL_IPOL1);
> +		if (ret)
> +			return ret;
> +	}
> +
> +	data->drdy_trig = devm_iio_trigger_alloc(&data->client->dev,
> +						 "%s-dev%d",
> +						 indio_dev->name,
> +						 iio_device_id(indio_dev));
> +	if (!data->drdy_trig)
> +		return -ENOMEM;
> +
> +	data->drdy_trig->ops = &mpl3115_trigger_ops;
> +	iio_trigger_set_drvdata(data->drdy_trig, indio_dev);
> +	ret = iio_trigger_register(data->drdy_trig);

devm_iio_trigger_register()

- Nuno Sá

> +	if (ret)
> +		return ret;
> +
> +	indio_dev->trig = iio_trigger_get(data->drdy_trig);
> +
> +	return devm_request_threaded_irq(&data->client->dev, irq,
> +					 NULL,
> +					 mpl3115_interrupt_handler,
> +					 IRQF_ONESHOT,
> +					 "mpl3115_irq",
> +					 indio_dev);
> +}
> +
>  static int mpl3115_probe(struct i2c_client *client)
>  {
>  	const struct i2c_device_id *id = i2c_client_get_device_id(client);
> @@ -271,6 +424,10 @@ static int mpl3115_probe(struct i2c_client *client)
>  	if (ret < 0)
>  		return ret;
>  
> +	ret = mpl3115_trigger_probe(data, indio_dev);
> +	if (ret)
> +		return ret;
> +
>  	ret = iio_triggered_buffer_setup(indio_dev, NULL,
>  		mpl3115_trigger_handler, NULL);
>  	if (ret < 0)

Re: [PATCH 2/3] iio: mpl3115: add support for DRDY interrupt

[Antoni Pokusinski]

On Sun, Sep 21, 2025 at 10:29:28PM +0300, Andy Shevchenko wrote:
> On Sun, Sep 21, 2025 at 4:34 PM Antoni Pokusinski
> <apokusinski01@gmail.com> wrote:
> >
> > MPL3115 sensor features a "data ready" interrupt which indicates the
> > presence of new measurements.
> 
> ...
> 
> >  #include <linux/module.h>
> 
> >  #include <linux/iio/trigger_consumer.h>
> >  #include <linux/iio/buffer.h>
> >  #include <linux/iio/triggered_buffer.h>
> > +#include <linux/iio/trigger.h>
> >  #include <linux/delay.h>
> 
> > +#include <linux/property.h>
> 
> This is like delay.h is misplaced. What we do here, we group generic
> ones followed by subsystem (IIO) related ones, and this seems wrong in
> this driver.
> 
> Can you rather move delay.h to be the first, and add property.h after
> i2c.h followed by a blank line, so in the result it will be like
> 
> delay.h
> i2c.h
> module.h
> property.h
> ...blank.line...
> iio/*.h
> 
> ...
> 
Sure, will fix this in v2.

> > +#define MPL3115_CTRL_INT_SRC_DRDY BIT(7)
> > +
> > +#define MPL3115_PT_DATA_EVENT_ALL (BIT(2) | BIT(1) | BIT(0))
> 
> Not sure I understand this definition in the following aspects:
> 1) why is this disrupting the _CTRL_ definitions?
> 2) why is this using BIT(x) and not respective definitions?
> 3) why can't you use GENMASK() if you just select all bits in a
> certain bitfield?
> 
> 
1) I placed the definitions of the bits/masks in the order of the registers
that they correspond to, i.e.
  CTRL_INT_SRC_DRDY // bit in reg 0x12
  PT_DATA_EVENT_ALL // bits in reg 0x13
  CTRL_RESET        // bit in reg 0x14

Actually, the wrong name here is the INT_SRC_DRDY definition because it is a
bit in the INT_SOURCE register, not a control register. Therefore, the
name should be MPL3115_INT_SRC_DRDY instead of MPL3115_CTRL_INT_SRC_DRDY

2) I saw that e.g. CTRL_OS_258MS is defined using BIT(x), so I thought
that this is the convention in this driver that I did not want to
disrupt by using GENMASK()

3) Sure, I'd even prefer GENMASK(), the only reason why I didn't use it
is explained in 2)

> >  #define MPL3115_CTRL_RESET BIT(2) /* software reset */
> >  #define MPL3115_CTRL_OST BIT(1) /* initiate measurement */
> >  #define MPL3115_CTRL_ACTIVE BIT(0) /* continuous measurement */
> >  #define MPL3115_CTRL_OS_258MS (BIT(5) | BIT(4)) /* 64x oversampling */
> 
> ...
> 
> >         mutex_lock(&data->lock);
> > -       ret = mpl3115_request(data);
> > -       if (ret < 0) {
> > -               mutex_unlock(&data->lock);
> > -               goto done;
> > +       if (!(data->ctrl_reg1 & MPL3115_CTRL_ACTIVE)) {
> > +               ret = mpl3115_request(data);
> > +               if (ret < 0) {
> 
> > +                       mutex_unlock(&data->lock);
> 
> Instead, I suggest adding a prerequisite that moves the driver to use
> cleanup.h, in particular scoped_guard(). This will reduce a churn
> here,
>
Will add in v2.
> > +                       goto done;
> > +               }
> >         }
> 
> ...
> 
> > +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);
> > +       int ret;
> > +       u8 ctrl_reg1 = data->ctrl_reg1;
> > +
> > +       if (state)
> > +               ctrl_reg1 |= MPL3115_CTRL_ACTIVE;
> > +       else
> > +               ctrl_reg1 &= ~MPL3115_CTRL_ACTIVE;
> 
> > +       guard(mutex)(&data->lock);
> 
> Oh, and you already use this! Definitely, it misses the prerequisite patch.
> 
> > +       ret = i2c_smbus_write_byte_data(data->client, MPL3115_CTRL_REG1,
> > +                                       ctrl_reg1);
> > +       if (ret < 0)
> > +               return ret;
> > +
> > +       ret = i2c_smbus_write_byte_data(data->client, MPL3115_CTRL_REG4,
> > +                                       state ? MPL3115_CTRL_INT_EN_DRDY : 0);
> > +       if (ret < 0)
> > +               goto reg1_cleanup;
> > +
> > +       data->ctrl_reg1 = ctrl_reg1;
> > +
> > +       return 0;
> > +
> > +reg1_cleanup:
> > +       i2c_smbus_write_byte_data(data->client, MPL3115_CTRL_REG1,
> > +                                 data->ctrl_reg1);
> > +       return ret;
> > +}
> 
> ...
> 
> > +static int mpl3115_trigger_probe(struct mpl3115_data *data,
> > +                                struct iio_dev *indio_dev)
> > +{
> > +       struct fwnode_handle *fwnode;
> > +       int ret, irq, irq_type;
> > +       bool act_high, is_int2 = false;
> 
> > +       fwnode = dev_fwnode(&data->client->dev);
> > +       if (!fwnode)
> > +               return -ENODEV;
> 
> 
> Why is this fatal? Also, do we have a board file for users of this right now?
> 
Actually it seems it does not have to be fatal. If we get rid of this
if, then we'd simply return without setting up the trigger and with no
interrupt support, which is ok I guess.

As for the board file, do you mean some PCB schematics? I don't know
about any, I've only used the following datasheet from NXP:
https://www.nxp.com/docs/en/data-sheet/MPL3115A2S.pdf

> > +       irq = fwnode_irq_get_byname(fwnode, "INT1");
> > +       if (irq < 0) {
> > +               irq = fwnode_irq_get_byname(fwnode, "INT2");
> > +               if (irq < 0)
> > +                       return 0;
> > +
> > +               is_int2 = true;
> > +       }
> > +
> > +       irq_type = irq_get_trigger_type(irq);
> > +       switch (irq_type) {
> > +       case IRQF_TRIGGER_RISING:
> > +               act_high = true;
> > +               break;
> > +       case IRQF_TRIGGER_FALLING:
> > +               act_high = false;
> > +               break;
> > +       default:
> > +               return -EINVAL;
> > +       }
> > +
> > +       ret = i2c_smbus_write_byte_data(data->client, MPL3115_PT_DATA_CFG,
> > +                                       MPL3115_PT_DATA_EVENT_ALL);
> > +       if (ret < 0)
> > +               return ret;
> 
> > +       if (!is_int2) {
> > +               ret = i2c_smbus_write_byte_data(data->client,
> > +                                               MPL3115_CTRL_REG5,
> > +                                               MPL3115_CTRL_INT_CFG_DRDY);
> > +               if (ret)
> > +                       return ret;
> > +       }
> > +       if (act_high) {
> > +               ret = i2c_smbus_write_byte_data(data->client,
> > +                                               MPL3115_CTRL_REG3,
> > +                                               is_int2 ? MPL3115_CTRL_IPOL2 :
> > +                                                         MPL3115_CTRL_IPOL1);
> > +               if (ret)
> > +                       return ret;
> > +       }
> 
> This if (!is_int2) and ternary with the same argument is kinda hard to
> read, can we refactor it somehow?
> 
> For example, if these two booleans are represented by a common enum, we can do
> 
> switch (cfg_flags) {
> case INT2_ACTIVE_HIGH:
>     _write_byte_data(REG3);
>     break;
> case INT2_ACTIVE_LOW:
>     break;
> case INT1_ACTIVE_HIGH:
>    _write_byte_data(REG5);
>    _write_byte_data(REG3);
>   break;
> case INT1_ACTIVE_LOW:
>    _write_byte_data(REG5);
>    break;
> default:
>     return -EINVAL;
> }
> 
> Yes, it's more verbose, but I find this better to read and understand.
> 
> Note, you may drop the switch case for IRQ with this approach as you
> can use a few bits together (separate bits for raising and falling to
> make the default case working here).
> 
Ok, your suggestion looks nice. I think to define maybe the enums like this:

  #define INT2 BIT(2) 
  enum {
    INT2_ACTIVE_LOW = INT2 | IRQF_TRIGGER_FALLING,
    INT2_ACTIVE_HIGH = INT2 | IRQF_TRIGGER_RISING,
    INT1_ACTIVE_LOW = !INT2 | IRQF_TRIGGER_FALLING,
    INT1_ACTIVE_HIGH = !INT2 | IRQF_TRIGGER_RISING, 
  };

This way the cfg_flags could be first |= INT_2 (after the call to the
fwnode_irq_get_byname) and then |= irq_type (after the call to the irq_get_trigger_type)

> > +       data->drdy_trig = devm_iio_trigger_alloc(&data->client->dev,
> > +                                                "%s-dev%d",
> > +                                                indio_dev->name,
> > +                                                iio_device_id(indio_dev));
> > +       if (!data->drdy_trig)
> > +               return -ENOMEM;
> > +
> > +       data->drdy_trig->ops = &mpl3115_trigger_ops;
> > +       iio_trigger_set_drvdata(data->drdy_trig, indio_dev);
> > +       ret = iio_trigger_register(data->drdy_trig);
> > +       if (ret)
> > +               return ret;
> > +
> > +       indio_dev->trig = iio_trigger_get(data->drdy_trig);
> > +
> > +       return devm_request_threaded_irq(&data->client->dev, irq,
> > +                                        NULL,
> > +                                        mpl3115_interrupt_handler,
> > +                                        IRQF_ONESHOT,
> > +                                        "mpl3115_irq",
> > +                                        indio_dev);
> > +}
> 
> -- 
> With Best Regards,
> Andy Shevchenko

Kind regards,
Antoni Pokusinski

Re: [PATCH 2/3] iio: mpl3115: add support for DRDY interrupt

[Antoni Pokusinski]

On Mon, Sep 22, 2025 at 10:15:19AM +0100, Nuno Sá wrote:
> On Sun, 2025-09-21 at 15:33 +0200, Antoni Pokusinski wrote:
> > MPL3115 sensor features a "data ready" interrupt which indicates the
> > presence of new measurements.
> > 
> > Signed-off-by: Antoni Pokusinski <apokusinski01@gmail.com>
> > ---
> >  drivers/iio/pressure/mpl3115.c | 167 ++++++++++++++++++++++++++++++++-
> >  1 file changed, 162 insertions(+), 5 deletions(-)
> > 
> > diff --git a/drivers/iio/pressure/mpl3115.c b/drivers/iio/pressure/mpl3115.c
> > index 579da60ef441..cf34de8f0d7e 100644
> > --- a/drivers/iio/pressure/mpl3115.c
> > +++ b/drivers/iio/pressure/mpl3115.c
> > @@ -7,7 +7,7 @@
> >   * (7-bit I2C slave address 0x60)
> >   *
> >   * TODO: FIFO buffer, altimeter mode, oversampling, continuous mode,
> > - * interrupts, user offset correction, raw mode
> > + * user offset correction, raw mode
> >   */
> >  
> >  #include <linux/module.h>
> > @@ -17,26 +17,45 @@
> >  #include <linux/iio/trigger_consumer.h>
> >  #include <linux/iio/buffer.h>
> >  #include <linux/iio/triggered_buffer.h>
> > +#include <linux/iio/trigger.h>
> >  #include <linux/delay.h>
> > +#include <linux/property.h>
> >  
> >  #define MPL3115_STATUS 0x00
> >  #define MPL3115_OUT_PRESS 0x01 /* MSB first, 20 bit */
> >  #define MPL3115_OUT_TEMP 0x04 /* MSB first, 12 bit */
> >  #define MPL3115_WHO_AM_I 0x0c
> > +#define MPL3115_INT_SOURCE 0x12
> > +#define MPL3115_PT_DATA_CFG 0x13
> >  #define MPL3115_CTRL_REG1 0x26
> > +#define MPL3115_CTRL_REG3 0x28
> > +#define MPL3115_CTRL_REG4 0x29
> > +#define MPL3115_CTRL_REG5 0x2a
> >  
> >  #define MPL3115_DEVICE_ID 0xc4
> >  
> >  #define MPL3115_STATUS_PRESS_RDY BIT(2)
> >  #define MPL3115_STATUS_TEMP_RDY BIT(1)
> >  
> > +#define MPL3115_CTRL_INT_SRC_DRDY BIT(7)
> > +
> > +#define MPL3115_PT_DATA_EVENT_ALL (BIT(2) | BIT(1) | BIT(0))
> > +
> >  #define MPL3115_CTRL_RESET BIT(2) /* software reset */
> >  #define MPL3115_CTRL_OST BIT(1) /* initiate measurement */
> >  #define MPL3115_CTRL_ACTIVE BIT(0) /* continuous measurement */
> >  #define MPL3115_CTRL_OS_258MS (BIT(5) | BIT(4)) /* 64x oversampling */
> >  
> > +#define MPL3115_CTRL_IPOL1 BIT(5)
> > +#define MPL3115_CTRL_IPOL2 BIT(1)
> > +
> > +#define MPL3115_CTRL_INT_EN_DRDY BIT(7)
> > +
> > +#define MPL3115_CTRL_INT_CFG_DRDY BIT(7)
> > +
> >  struct mpl3115_data {
> >  	struct i2c_client *client;
> > +	struct iio_trigger *drdy_trig;
> >  	struct mutex lock;
> >  	u8 ctrl_reg1;
> >  };
> > @@ -164,10 +183,12 @@ static irqreturn_t mpl3115_trigger_handler(int irq, void
> > *p)
> >  	int ret, pos = 0;
> >  
> >  	mutex_lock(&data->lock);
> > -	ret = mpl3115_request(data);
> > -	if (ret < 0) {
> > -		mutex_unlock(&data->lock);
> > -		goto done;
> > +	if (!(data->ctrl_reg1 & MPL3115_CTRL_ACTIVE)) {
> > +		ret = mpl3115_request(data);
> > +		if (ret < 0) {
> > +			mutex_unlock(&data->lock);
> > +			goto done;
> > +		}
> >  	}
> >  
> >  	if (test_bit(0, indio_dev->active_scan_mask)) {
> > @@ -228,10 +249,142 @@ static const struct iio_chan_spec mpl3115_channels[] =
> > {
> >  	IIO_CHAN_SOFT_TIMESTAMP(2),
> >  };
> >  
> > +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;
> > +
> > +	ret = i2c_smbus_read_byte_data(data->client, MPL3115_INT_SOURCE);
> > +	if (ret < 0)
> > +		return IRQ_HANDLED;
> > +
> > +	if (!(ret & MPL3115_CTRL_INT_SRC_DRDY))
> > +		return IRQ_NONE;
> > +
> > +	iio_trigger_poll_nested(data->drdy_trig);
> > +
> > +	return IRQ_HANDLED;
> > +}
> > +
> > +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);
> > +	int ret;
> > +	u8 ctrl_reg1 = data->ctrl_reg1;
> > +
> > +	if (state)
> > +		ctrl_reg1 |= MPL3115_CTRL_ACTIVE;
> > +	else
> > +		ctrl_reg1 &= ~MPL3115_CTRL_ACTIVE;
> > +
> > +	guard(mutex)(&data->lock);
> > +
> 
> As Andy pointed out, you should have a precursor patch converting the complete
> driver to use the cleanup logic.
> 
> Another nice cleanup you could do (if you want of course) would be to get rid of
> mpl3115_remove().
> 
Will add the precursor patch in v2
> > +	ret = i2c_smbus_write_byte_data(data->client, MPL3115_CTRL_REG1,
> > +					ctrl_reg1);
> > +	if (ret < 0)
> > +		return ret;
> > +
> > +	ret = i2c_smbus_write_byte_data(data->client, MPL3115_CTRL_REG4,
> > +					state ? MPL3115_CTRL_INT_EN_DRDY :
> > 0);
> > +	if (ret < 0)
> > +		goto reg1_cleanup;
> > +
> > +	data->ctrl_reg1 = ctrl_reg1;
> > +
> > +	return 0;
> > +
> > +reg1_cleanup:
> > +	i2c_smbus_write_byte_data(data->client, MPL3115_CTRL_REG1,
> > +				  data->ctrl_reg1);
> > +	return ret;
> > +}
> > +
> > +static const struct iio_trigger_ops mpl3115_trigger_ops = {
> > +	.set_trigger_state = mpl3115_set_trigger_state,
> > +};
> > +
> >  static const struct iio_info mpl3115_info = {
> >  	.read_raw = &mpl3115_read_raw,
> >  };
> >  
> > +static int mpl3115_trigger_probe(struct mpl3115_data *data,
> > +				 struct iio_dev *indio_dev)
> > +{
> > +	struct fwnode_handle *fwnode;
> > +	int ret, irq, irq_type;
> > +	bool act_high, is_int2 = false;
> > +
> > +	fwnode = dev_fwnode(&data->client->dev);
> > +	if (!fwnode)
> > +		return -ENODEV;
> > +
> 
> And to add to Andy's review, fwnode_irq_get_byname() will give you an error
> anyways if !fwnode.
> 
> > +	irq = fwnode_irq_get_byname(fwnode, "INT1");
> > +	if (irq < 0) {
> > +		irq = fwnode_irq_get_byname(fwnode, "INT2");
> > +		if (irq < 0)
> > +			return 0;
> > +
> > +		is_int2 = true;
> > +	}
> > +
> > +	irq_type = irq_get_trigger_type(irq);
> > +	switch (irq_type) {
> > +	case IRQF_TRIGGER_RISING:
> > +		act_high = true;
> > +		break;
> > +	case IRQF_TRIGGER_FALLING:
> > +		act_high = false;
> > +		break;
> > +	default:
> > +		return -EINVAL;
> > +	}
> > +
> > +	ret = i2c_smbus_write_byte_data(data->client, MPL3115_PT_DATA_CFG,
> > +					MPL3115_PT_DATA_EVENT_ALL);
> > +	if (ret < 0)
> > +		return ret;
> > +
> > +	if (!is_int2) {
> > +		ret = i2c_smbus_write_byte_data(data->client,
> > +						MPL3115_CTRL_REG5,
> > +						MPL3115_CTRL_INT_CFG_DRDY);
> > +		if (ret)
> > +			return ret;
> > +	}
> > +	if (act_high) {
> > +		ret = i2c_smbus_write_byte_data(data->client,
> > +						MPL3115_CTRL_REG3,
> > +						is_int2 ? MPL3115_CTRL_IPOL2
> > :
> > +							 
> > MPL3115_CTRL_IPOL1);
> > +		if (ret)
> > +			return ret;
> > +	}
> > +
> > +	data->drdy_trig = devm_iio_trigger_alloc(&data->client->dev,
> > +						 "%s-dev%d",
> > +						 indio_dev->name,
> > +						 iio_device_id(indio_dev));
> > +	if (!data->drdy_trig)
> > +		return -ENOMEM;
> > +
> > +	data->drdy_trig->ops = &mpl3115_trigger_ops;
> > +	iio_trigger_set_drvdata(data->drdy_trig, indio_dev);
> > +	ret = iio_trigger_register(data->drdy_trig);
> 
> devm_iio_trigger_register()
> 
> - Nuno Sá
>
Will fix in v2
> > +	if (ret)
> > +		return ret;
> > +
> > +	indio_dev->trig = iio_trigger_get(data->drdy_trig);
> > +
> > +	return devm_request_threaded_irq(&data->client->dev, irq,
> > +					 NULL,
> > +					 mpl3115_interrupt_handler,
> > +					 IRQF_ONESHOT,
> > +					 "mpl3115_irq",
> > +					 indio_dev);
> > +}
> > +
> >  static int mpl3115_probe(struct i2c_client *client)
> >  {
> >  	const struct i2c_device_id *id = i2c_client_get_device_id(client);
> > @@ -271,6 +424,10 @@ static int mpl3115_probe(struct i2c_client *client)
> >  	if (ret < 0)
> >  		return ret;
> >  
> > +	ret = mpl3115_trigger_probe(data, indio_dev);
> > +	if (ret)
> > +		return ret;
> > +
> >  	ret = iio_triggered_buffer_setup(indio_dev, NULL,
> >  		mpl3115_trigger_handler, NULL);
> >  	if (ret < 0)

Re: [PATCH 1/3] dt-bindings: iio: pressure: add binding for mpl3115

[Rob Herring (Arm)]

On Sun, 21 Sep 2025 15:33:26 +0200, Antoni Pokusinski wrote:
> MPL3115 is an I2C pressure and temperature sensor. It features 2
> interrupt lines which can be configured to indicate events such as data
> ready or pressure/temperature threshold reached.
> 
> Signed-off-by: Antoni Pokusinski <apokusinski01@gmail.com>
> ---
>  .../bindings/iio/pressure/fsl,mpl3115.yaml    | 63 +++++++++++++++++++
>  .../devicetree/bindings/trivial-devices.yaml  |  2 -
>  2 files changed, 63 insertions(+), 2 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/iio/pressure/fsl,mpl3115.yaml
> 

Reviewed-by: Rob Herring (Arm) <robh@kernel.org>

Re: [PATCH 3/3] iio: mpl3115: add support for sampling frequency

[David Lechner]

On 9/21/25 8:33 AM, Antoni Pokusinski wrote:
> When the device is in ACTIVE mode the temperature and pressure measurements
> are collected with a frequency determined by the ST[3:0] bits of CTRL_REG2
> register.
> 
> Signed-off-by: Antoni Pokusinski <apokusinski01@gmail.com>
> ---
>  drivers/iio/pressure/mpl3115.c | 80 ++++++++++++++++++++++++++++++++++
>  1 file changed, 80 insertions(+)
> 
> diff --git a/drivers/iio/pressure/mpl3115.c b/drivers/iio/pressure/mpl3115.c
> index cf34de8f0d7e..2f1860ca1f32 100644
> --- a/drivers/iio/pressure/mpl3115.c
> +++ b/drivers/iio/pressure/mpl3115.c
> @@ -28,6 +28,7 @@
>  #define MPL3115_INT_SOURCE 0x12
>  #define MPL3115_PT_DATA_CFG 0x13
>  #define MPL3115_CTRL_REG1 0x26
> +#define MPL3115_CTRL_REG2 0x27
>  #define MPL3115_CTRL_REG3 0x28
>  #define MPL3115_CTRL_REG4 0x29
>  #define MPL3115_CTRL_REG5 0x2a
> @@ -46,6 +47,8 @@
>  #define MPL3115_CTRL_ACTIVE BIT(0) /* continuous measurement */
>  #define MPL3115_CTRL_OS_258MS (BIT(5) | BIT(4)) /* 64x oversampling */
>  
> +#define MPL3115_CTRL_ST (BIT(3) | BIT(2) | BIT(1) | BIT(0))

Can be simplified with GENMASK(3, 0).

Would also be more clear to call it MPL3115_CTRL_REG2_ST.
Otherwise it looks like it overlaps with the bits above
and below.

> +
>  #define MPL3115_CTRL_IPOL1 BIT(5)
>  #define MPL3115_CTRL_IPOL2 BIT(1)
>  
> @@ -53,6 +56,25 @@
>  
>  #define MPL3115_CTRL_INT_CFG_DRDY BIT(7)
>  
> +static const unsigned int mpl3115_samp_freq_table[][2] = {
> +	{ 1,      0},
> +	{ 0, 500000},
> +	{ 0, 250000},
> +	{ 0, 125000},
> +	{ 0,  62500},
> +	{ 0,  31250},
> +	{ 0,  15625},
> +	{ 0,   7812},
> +	{ 0,   3906},
> +	{ 0,   1953},
> +	{ 0,    976},
> +	{ 0,    488},
> +	{ 0,    244},
> +	{ 0,    122},
> +	{ 0,     61},
> +	{ 0,     30},

Didn't check these all, but got 30.5... for the last one, which
would round to 31. Not that it matters much in this case. ;-)

> +};
> +
>  struct mpl3115_data {
>  	struct i2c_client *client;
>  	struct iio_trigger *drdy_trig;
> @@ -163,10 +185,60 @@ static int mpl3115_read_raw(struct iio_dev *indio_dev,
>  		default:
>  			return -EINVAL;
>  		}
> +	case IIO_CHAN_INFO_SAMP_FREQ:
> +		ret = i2c_smbus_read_byte_data(data->client, MPL3115_CTRL_REG2);
> +		if (ret < 0)
> +			return ret;
> +
> +		ret &= MPL3115_CTRL_ST;

FIELD_GET() would be appropriate here.

> +
> +		*val = mpl3115_samp_freq_table[ret][0];
> +		*val2 = mpl3115_samp_freq_table[ret][1];
> +		return IIO_VAL_INT_PLUS_MICRO;
>  	}
>  	return -EINVAL;
>  }
>  
> +static int mpl3115_read_avail(struct iio_dev *indio_dev,
> +			      struct iio_chan_spec const *chan,
> +			      const int **vals, int *type, int *length,
> +			      long mask)
> +{
> +	if (mask != IIO_CHAN_INFO_SAMP_FREQ)
> +		return -EINVAL;
> +
> +	*type = IIO_VAL_INT_PLUS_MICRO;
> +	*length = ARRAY_SIZE(mpl3115_samp_freq_table) * 2;
> +	*vals = (int *)mpl3115_samp_freq_table;
> +	return IIO_AVAIL_LIST;
> +}
> +
> +static int mpl3115_write_raw(struct iio_dev *indio_dev,
> +			     const struct iio_chan_spec *chan,
> +			     int val, int val2, long mask)
> +{
> +	struct mpl3115_data *data = iio_priv(indio_dev);
> +	int i, ret;
> +
> +	if (mask != IIO_CHAN_INFO_SAMP_FREQ)
> +		return -EINVAL;
> +
> +	for (i = 0; i < ARRAY_SIZE(mpl3115_samp_freq_table); i++)
> +		if (val == mpl3115_samp_freq_table[i][0] &&
> +		    val2 == mpl3115_samp_freq_table[i][1])
> +			break;
> +
> +	if (i == ARRAY_SIZE(mpl3115_samp_freq_table))
> +		return -EINVAL;
> +
> +	if (!iio_device_claim_direct(indio_dev))
> +		return -EBUSY;
> +
> +	ret = i2c_smbus_write_byte_data(data->client, MPL3115_CTRL_REG2, i);

This potentially clears some unrelated bits. I guess it is not a problem for
now since those bit aren't used, but could be easy to overlook in the future.

Also, FIELD_PREP() would be appropriate.

> +	iio_device_release_direct(indio_dev);
> +	return ret;
> +}
> +
>  static irqreturn_t mpl3115_trigger_handler(int irq, void *p)
>  {
>  	struct iio_poll_func *pf = p;
> @@ -224,6 +296,9 @@ static const struct iio_chan_spec mpl3115_channels[] = {
>  		.type = IIO_PRESSURE,
>  		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),
>  		.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE),
> +		.info_mask_shared_by_all = BIT(IIO_CHAN_INFO_SAMP_FREQ),
> +		.info_mask_shared_by_all_available =
> +			BIT(IIO_CHAN_INFO_SAMP_FREQ),
>  		.scan_index = 0,
>  		.scan_type = {
>  			.sign = 'u',
> @@ -237,6 +312,9 @@ static const struct iio_chan_spec mpl3115_channels[] = {
>  		.type = IIO_TEMP,
>  		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),
>  		.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE),
> +		.info_mask_shared_by_all = BIT(IIO_CHAN_INFO_SAMP_FREQ),
> +		.info_mask_shared_by_all_available =
> +			BIT(IIO_CHAN_INFO_SAMP_FREQ),
>  		.scan_index = 1,
>  		.scan_type = {
>  			.sign = 's',
> @@ -307,6 +385,8 @@ static const struct iio_trigger_ops mpl3115_trigger_ops = {
>  
>  static const struct iio_info mpl3115_info = {
>  	.read_raw = &mpl3115_read_raw,
> +	.read_avail = &mpl3115_read_avail,
> +	.write_raw = &mpl3115_write_raw,
>  };
>  
>  static int mpl3115_trigger_probe(struct mpl3115_data *data,

Re: [PATCH 1/3] dt-bindings: iio: pressure: add binding for mpl3115

[Marcelo Schmitt]

Hi Antoni,

Some inline suggestions for making this binding better comply with the
'attempt to make bindings complete' guideline.

On 09/21, Antoni Pokusinski wrote:
> MPL3115 is an I2C pressure and temperature sensor. It features 2
> interrupt lines which can be configured to indicate events such as data
> ready or pressure/temperature threshold reached.
> 
> Signed-off-by: Antoni Pokusinski <apokusinski01@gmail.com>
> ---
...
> +description: |
> +  MPL3115 is a pressure/altitude and temperature sensor with I2C interface.
> +  It features two programmable interrupt lines which indicate events such as
> +  data ready or pressure/temperature threshold reached.
> +  https://www.nxp.com/docs/en/data-sheet/MPL3115A2.pdf
> +
> +properties:
> +  compatible:
> +    const: fsl,mpl3115
> +
> +  reg:
> +    maxItems: 1

For completeness, could also add the power supplies.

  vdd-supply: true

  vddio-supply: true

> +
> +  interrupts:
> +    minItems: 1
> +    maxItems: 2
> +
> +  interrupt-names:
> +    minItems: 1
> +    maxItems: 2
> +    items:
> +      enum:
> +        - INT1
> +        - INT2
> +
> +  drive-open-drain:
> +    type: boolean
> +    description:
> +      set if the specified interrupt pins should be configured as
> +      open drain. If not set, defaults to push-pull.
> +
> +required:
> +  - compatible
> +  - reg
  - vdd-supply
  - vddio-supply

and also add the supplies to the example below.

> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/interrupt-controller/irq.h>
> +    i2c {
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +
> +        pressure@60 {
> +            compatible = "fsl,mpl3115";
> +            reg = <0x60>;
> +            interrupt-parent = <&gpio1>;
> +            interrupts = <4 IRQ_TYPE_EDGE_FALLING>;
> +            interrupt-names = "INT2";
> +        };
> +    };

Best regards,
Marcelo

Re: [PATCH 2/3] iio: mpl3115: add support for DRDY interrupt

[Jonathan Cameron]

> ...
> 
> >         mutex_lock(&data->lock);
> > -       ret = mpl3115_request(data);
> > -       if (ret < 0) {
> > -               mutex_unlock(&data->lock);
> > -               goto done;
> > +       if (!(data->ctrl_reg1 & MPL3115_CTRL_ACTIVE)) {
> > +               ret = mpl3115_request(data);
> > +               if (ret < 0) {  
> 
> > +                       mutex_unlock(&data->lock);  
> 
> Instead, I suggest adding a prerequisite that moves the driver to use
> cleanup.h, in particular scoped_guard(). This will reduce a churn
> here,

I'll comment on this in version 3, but I'm not sure scoped_guard() is
necessarily a good idea here.

Also note that there is no requirement to use one style universally in
a driver (guard / vs explicit unlocks) as often they work best in different
usecases with the same locks.
> 
> > +                       goto done;
> > +               }
> >         }