[PATCH v7 0/8] io: accel: mma8452: Allow open drain interrupt pin configuration

[PATCH v7 0/8] io: accel: mma8452: Allow open drain interrupt pin configuration

[Esben Haabendal]

Extend the mma8452 driver with support for configuration of the
interrupt line in open-drain mode, which is needed for hardware designs
where the interrupt line is shared with other chips.

Adding drive-open-drain property to mma8452 device-tree node for such
designs to enable switching pin configuration to open-drain mode.

Signed-off-by: Esben Haabendal <esben@geanix.com>
---
Changes in v7:
- Use dev pointer directly in mma8452_runtime_suspend() also.
- Move struct i2c_client *client definition to patch 4.
- Drop patch 7 (Drop unneeded lock acquire on read).
- Link to v6: https://patch.msgid.link/20260825-mma8452-open-drain-v6-0-9b252804ee80@geanix.com

Changes in v6:
- Prevent interrupt storm if runtime suspend fails to set the device in
  standby mode by not setting suspended flag to true when failing to
  activate standby mode and by ensuring PM counter is not leaked in
  mma8452_interrupt().
- Don't acquire data->lock in mma8452_read_raw().
- Add fix for use-after-free in mma8452_probe() error path when CONFIG_PM
  is enabled.
- Fix (brown paper bug) build error introduced in v5.
- Renamed label in mma8452_interrupt().
- Add patch to return -ENODATA when missing device model information.
- Link to v5: https://patch.msgid.link/20260819-mma8452-open-drain-v5-0-d8aa590d7c36@geanix.com

Changes in v5:
- Squashed dev_err_probe() call to one line.
- Fixed typo in patch 2 title.
- Added synchronization between runtime suspend and interrupt handler.
- Link to v4: https://patch.msgid.link/20260812-mma8452-open-drain-v4-0-bfca15d02b59@geanix.com

Changes in v4:
- Fixed interrupt handler to check runtime PM status before trying to
  access the chip.
- Split open-drain support and interrupt sharing into separate patches.
- Added new patch to reuse existing struct device * through mma8452_probe()
  function.
- Print warning message when irq type is not set by firmware.
- Link to v3: https://patch.msgid.link/20260805-mma8452-open-drain-v3-0-6149f406a409@geanix.com

Changes in v3:
- Reordered patches, swapping #2 and #3.
- Always add IRQF_SHARED flag.
- New patch to change it so IQRF_TRIGGER_LOW flag is only added when no
  trigger type is set by firmware.
- Link to v2: https://patch.msgid.link/20260715-mma8452-open-drain-v2-0-95be9f5f4795@geanix.com

Changes in v2:
- Commit message of patch 2 updated.
- Operator precedence bug fixed in flags argument to
  request_threaded_irq().
- Always check return value of mma8452_set_interrupt_pin_mode(), and just
  check for non-zero value.
- Added new patch with optimization of struct mma8452_data ordering.
- Link to v1: https://patch.msgid.link/20260715-mma8452-open-drain-v1-0-b1dd2a440c60@geanix.com

To: Jonathan Cameron <jic23@kernel.org>
To: Lars-Peter Clausen <lars@metafoo.de>
To: Rob Herring <robh@kernel.org>
To: Krzysztof Kozlowski <krzk+dt@kernel.org>
To: Conor Dooley <conor+dt@kernel.org>
To: Martin Kepplinger <martink@posteo.de>
To: Sean Nyekjaer <sean@geanix.com>
To: David Lechner <dlechner@baylibre.com>
To: Nuno Sá <nuno.sa@analog.com>
To: Andy Shevchenko <andy@kernel.org>
To: Martin Kepplinger <martin.kepplinger@theobroma-systems.com>
To: Christoph Muellner <christoph.muellner@theobroma-systems.com>
Cc: linux-iio@vger.kernel.org
Cc: devicetree@vger.kernel.org
Cc: linux-kernel@vger.kernel.org

---
Esben Haabendal (8):
      dt-bindings: iio: accel: mma8452: Add drive-open-drain
      iio: accel: mma8452: Optimize struct mma8452_data member orders
      iio: accel: mma8452: Only apply trigger type when not set by firmware
      iio: accel: mma8452: Support interrupt sharing
      iio: accel: mma8452: Allow open drain interrupt pin configuration
      iio: accel: mma8452: Reuse existing dev pointer in mma8452_probe()
      iio: accel: mma8452: Fix use-after-free bug in error error path
      iio: accel: mma8452: Use proper error code when missing device model

 .../devicetree/bindings/iio/accel/fsl,mma8452.yaml |   6 ++
 drivers/iio/accel/mma8452.c                        | 115 ++++++++++++++++-----
 2 files changed, 94 insertions(+), 27 deletions(-)
---
base-commit: cee9395acd8043be0644b25c34bfa86623f2b935
change-id: 20250401-mma8452-open-drain-81577c41375c

Best regards,
--  
Esben Haabendal <esben@geanix.com>

[PATCH v7 1/8] dt-bindings: iio: accel: mma8452: Add drive-open-drain

[Esben Haabendal]

Add new boolean to configure selected interrupt pin to open drain instead
of the default push-pull mode.

Acked-by: Rob Herring (Arm) <robh@kernel.org>
Signed-off-by: Esben Haabendal <esben@geanix.com>
---
 Documentation/devicetree/bindings/iio/accel/fsl,mma8452.yaml | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/Documentation/devicetree/bindings/iio/accel/fsl,mma8452.yaml b/Documentation/devicetree/bindings/iio/accel/fsl,mma8452.yaml
index b0dd2b4e116a..20701aa725d0 100644
--- a/Documentation/devicetree/bindings/iio/accel/fsl,mma8452.yaml
+++ b/Documentation/devicetree/bindings/iio/accel/fsl,mma8452.yaml
@@ -39,6 +39,12 @@ properties:
     minItems: 1
     maxItems: 2
 
+  drive-open-drain:
+    $ref: /schemas/types.yaml#/definitions/flag
+    description: the interrupt line will be configured as open drain, which is
+      useful if several sensors share the same interrupt line. (This binding is
+      taken from pinctrl.)
+
   vdd-supply: true
   vddio-supply: true
 

-- 
2.55.0

[PATCH v7 2/8] iio: accel: mma8452: Optimize struct mma8452_data member orders

[Esben Haabendal]

Reorder struct mma8452_data members to avoid holes.

Reviewed-by: Joshua Crofts <joshua.crofts1@gmail.com>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com>
Signed-off-by: Esben Haabendal <esben@geanix.com>
---
 drivers/iio/accel/mma8452.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/iio/accel/mma8452.c b/drivers/iio/accel/mma8452.c
index 7d683686dd9d..f645a5c6fd1c 100644
--- a/drivers/iio/accel/mma8452.c
+++ b/drivers/iio/accel/mma8452.c
@@ -106,10 +106,7 @@ struct mma8452_data {
 	struct i2c_client *client;
 	struct mutex lock;
 	struct iio_mount_matrix orientation;
-	u8 ctrl_reg1;
-	u8 data_cfg;
 	const struct mma_chip_info *chip_info;
-	int sleep_val;
 	struct regulator *vdd_reg;
 	struct regulator *vddio_reg;
 
@@ -118,6 +115,10 @@ struct mma8452_data {
 		__be16 channels[3];
 		aligned_s64 ts;
 	} buffer;
+
+	int sleep_val;
+	u8 ctrl_reg1;
+	u8 data_cfg;
 };
 
  /**

-- 
2.55.0

[PATCH v7 3/8] iio: accel: mma8452: Only apply trigger type when not set by firmware

[Esben Haabendal]

Instead of unconditionally overriding the trigger type, it is better to
only apply a default when no trigger type is set by firmware. This should
be reasonably backward compatible, and should only potentially cause
problems if systems exist where firmware specifies an incorrect trigger
type. With a bit of luck, there are no such systems.

Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com>
Signed-off-by: Esben Haabendal <esben@geanix.com>
---
 drivers/iio/accel/mma8452.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/drivers/iio/accel/mma8452.c b/drivers/iio/accel/mma8452.c
index f645a5c6fd1c..1fb43c5b0b72 100644
--- a/drivers/iio/accel/mma8452.c
+++ b/drivers/iio/accel/mma8452.c
@@ -1683,9 +1683,16 @@ static int mma8452_probe(struct i2c_client *client)
 		goto trigger_cleanup;
 
 	if (client->irq) {
+		unsigned long irq_flags;
+
+		irq_flags = irq_get_trigger_type(client->irq);
+		if (irq_flags == IRQ_TYPE_NONE) {
+			dev_info(dev, "invalid irq type, setting default active low\n");
+			irq_flags = IRQF_TRIGGER_LOW;
+		}
+		irq_flags |= IRQF_ONESHOT;
 		ret = request_threaded_irq(client->irq, NULL, mma8452_interrupt,
-					   IRQF_TRIGGER_LOW | IRQF_ONESHOT,
-					   client->name, indio_dev);
+					   irq_flags, client->name, indio_dev);
 		if (ret)
 			goto buffer_cleanup;
 	}

-- 
2.55.0

[PATCH v7 4/8] iio: accel: mma8452: Support interrupt sharing

[Esben Haabendal]

Adding handling of rutnime PM suspension in the interrupt handler allows
sharing interrupt with other devices.

Keep in mind that the device by default is using push-pull for the irq pin,
which might require additional hardware design to allow interrupt sharing.

The suspended flag is added together with synchronize_irq() in order to
protect against race conditions when doing runtime suspend and device
removal. This way we ensure that interrupt handler does not try to access
the device while regulators are disabled.

Signed-off-by: Esben Haabendal <esben@geanix.com>
---
 drivers/iio/accel/mma8452.c | 31 +++++++++++++++++++++++++++----
 1 file changed, 27 insertions(+), 4 deletions(-)

diff --git a/drivers/iio/accel/mma8452.c b/drivers/iio/accel/mma8452.c
index 1fb43c5b0b72..936eeb2efb2d 100644
--- a/drivers/iio/accel/mma8452.c
+++ b/drivers/iio/accel/mma8452.c
@@ -119,6 +119,7 @@ struct mma8452_data {
 	int sleep_val;
 	u8 ctrl_reg1;
 	u8 data_cfg;
+	bool suspended;
 };
 
  /**
@@ -1056,14 +1057,24 @@ static irqreturn_t mma8452_interrupt(int irq, void *p)
 	struct iio_dev *indio_dev = p;
 	struct mma8452_data *data = iio_priv(indio_dev);
 	irqreturn_t ret = IRQ_NONE;
+	int pm_status;
 	int src;
 
+	pm_status = pm_runtime_get_if_active(&data->client->dev);
+	if (pm_status == 0)
+		return IRQ_NONE; /* device is powered down */
+	if (READ_ONCE(data->suspended)) {
+		/* device is being removed */
+		ret = IRQ_NONE;
+		goto out_runtime_put;
+	}
+
 	src = i2c_smbus_read_byte_data(data->client, MMA8452_INT_SRC);
 	if (src < 0)
-		return IRQ_NONE;
+		goto out_runtime_put;
 
 	if (!(src & (data->chip_info->enabled_events | MMA8452_INT_DRDY)))
-		return IRQ_NONE;
+		goto out_runtime_put;
 
 	if (src & MMA8452_INT_DRDY) {
 		iio_trigger_poll_nested(indio_dev->trig);
@@ -1089,6 +1100,10 @@ static irqreturn_t mma8452_interrupt(int irq, void *p)
 		ret = IRQ_HANDLED;
 	}
 
+out_runtime_put:
+	if (pm_status > 0)
+		pm_runtime_put_autosuspend(&data->client->dev);
+
 	return ret;
 }
 
@@ -1690,7 +1705,7 @@ static int mma8452_probe(struct i2c_client *client)
 			dev_info(dev, "invalid irq type, setting default active low\n");
 			irq_flags = IRQF_TRIGGER_LOW;
 		}
-		irq_flags |= IRQF_ONESHOT;
+		irq_flags |= IRQF_ONESHOT | IRQF_SHARED;
 		ret = request_threaded_irq(client->irq, NULL, mma8452_interrupt,
 					   irq_flags, client->name, indio_dev);
 		if (ret)
@@ -1762,7 +1777,8 @@ static void mma8452_remove(struct i2c_client *client)
 #ifdef CONFIG_PM
 static int mma8452_runtime_suspend(struct device *dev)
 {
-	struct iio_dev *indio_dev = i2c_get_clientdata(to_i2c_client(dev));
+	struct i2c_client *client = to_i2c_client(dev);
+	struct iio_dev *indio_dev = i2c_get_clientdata(client);
 	struct mma8452_data *data = iio_priv(indio_dev);
 	int ret;
 
@@ -1774,6 +1790,10 @@ static int mma8452_runtime_suspend(struct device *dev)
 		return -EAGAIN;
 	}
 
+	WRITE_ONCE(data->suspended, true);
+
+	synchronize_irq(client->irq);
+
 	ret = regulator_disable(data->vddio_reg);
 	if (ret) {
 		dev_err(dev, "failed to disable VDDIO regulator\n");
@@ -1808,6 +1828,8 @@ static int mma8452_runtime_resume(struct device *dev)
 		return ret;
 	}
 
+	WRITE_ONCE(data->suspended, false);
+
 	ret = mma8452_active(data);
 	if (ret < 0)
 		goto runtime_resume_failed;
@@ -1822,6 +1844,7 @@ static int mma8452_runtime_resume(struct device *dev)
 	return 0;
 
 runtime_resume_failed:
+	WRITE_ONCE(data->suspended, true);
 	regulator_disable(data->vddio_reg);
 	regulator_disable(data->vdd_reg);
 

-- 
2.55.0

[PATCH v7 5/8] iio: accel: mma8452: Allow open drain interrupt pin configuration

[Esben Haabendal]

When designing systems sharing the interrupt for mma8452 chips, it is
helpful to be able to configure the irq pin in open-drain mode (default is
push-pull).

Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com>
Signed-off-by: Esben Haabendal <esben@geanix.com>
---
 drivers/iio/accel/mma8452.c | 28 ++++++++++++++++++++++++++++
 1 file changed, 28 insertions(+)

diff --git a/drivers/iio/accel/mma8452.c b/drivers/iio/accel/mma8452.c
index 936eeb2efb2d..2ba6e2c44cb9 100644
--- a/drivers/iio/accel/mma8452.c
+++ b/drivers/iio/accel/mma8452.c
@@ -81,6 +81,8 @@
 #define  MMA8452_CTRL_REG2_RST			BIT(6)
 #define  MMA8452_CTRL_REG2_MODS_SHIFT		3
 #define  MMA8452_CTRL_REG2_MODS_MASK		0x1b
+#define MMA8452_CTRL_REG3			0x2c
+#define  MMA8452_CTRL_REG3_PP_OD		BIT(0)
 #define MMA8452_CTRL_REG4			0x2d
 #define MMA8452_CTRL_REG5			0x2e
 #define MMA8452_OFF_X				0x2f
@@ -120,6 +122,7 @@ struct mma8452_data {
 	u8 ctrl_reg1;
 	u8 data_cfg;
 	bool suspended;
+	bool open_drain;
 };
 
  /**
@@ -648,6 +651,22 @@ static int mma8452_set_power_mode(struct mma8452_data *data, u8 mode)
 	return mma8452_change_config(data, MMA8452_CTRL_REG2, reg);
 }
 
+static int mma8452_set_interrupt_pin_mode(struct mma8452_data *data)
+{
+	int reg;
+
+	reg = i2c_smbus_read_byte_data(data->client, MMA8452_CTRL_REG3);
+	if (reg < 0)
+		return reg;
+
+	if (data->open_drain)
+		reg |= MMA8452_CTRL_REG3_PP_OD;
+	else
+		reg &= ~MMA8452_CTRL_REG3_PP_OD;
+
+	return i2c_smbus_write_byte_data(data->client, MMA8452_CTRL_REG3, reg);
+}
+
 /* returns >0 if in freefall mode, 0 if not or <0 if an error occurred */
 static int mma8452_freefall_mode_enabled(struct mma8452_data *data)
 {
@@ -1682,6 +1701,11 @@ static int mma8452_probe(struct i2c_client *client)
 			goto disable_regulators;
 	}
 
+	data->open_drain = device_property_read_bool(dev, "drive-open-drain");
+	ret = mma8452_set_interrupt_pin_mode(data);
+	if (ret)
+		goto trigger_cleanup;
+
 	data->ctrl_reg1 = MMA8452_CTRL_ACTIVE |
 			  (MMA8452_CTRL_DR_DEFAULT << MMA8452_CTRL_DR_SHIFT);
 
@@ -1830,6 +1854,10 @@ static int mma8452_runtime_resume(struct device *dev)
 
 	WRITE_ONCE(data->suspended, false);
 
+	ret = mma8452_set_interrupt_pin_mode(data);
+	if (ret)
+		goto runtime_resume_failed;
+
 	ret = mma8452_active(data);
 	if (ret < 0)
 		goto runtime_resume_failed;

-- 
2.55.0

[PATCH v7 6/8] iio: accel: mma8452: Reuse existing dev pointer in mma8452_probe()

[Esben Haabendal]

In commit 32a5c04d4575 ("iio: accel: mma8452: Use dev_err_probe()") the
struct device * pointer was assigned to local variable dev, so we can just
as well reuse that throughout the function for sligthly more readable code.

Reviewed-by: Joshua Crofts <joshua.crofts1@gmail.com>
Signed-off-by: Esben Haabendal <esben@geanix.com>
---
 drivers/iio/accel/mma8452.c | 34 ++++++++++++++++------------------
 1 file changed, 16 insertions(+), 18 deletions(-)

diff --git a/drivers/iio/accel/mma8452.c b/drivers/iio/accel/mma8452.c
index 2ba6e2c44cb9..bf56ebd6e091 100644
--- a/drivers/iio/accel/mma8452.c
+++ b/drivers/iio/accel/mma8452.c
@@ -1587,7 +1587,7 @@ static int mma8452_probe(struct i2c_client *client)
 	struct iio_dev *indio_dev;
 	int ret;
 
-	indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*data));
+	indio_dev = devm_iio_device_alloc(dev, sizeof(*data));
 	if (!indio_dev)
 		return -ENOMEM;
 
@@ -1597,21 +1597,20 @@ static int mma8452_probe(struct i2c_client *client)
 
 	data->chip_info = i2c_get_match_data(client);
 	if (!data->chip_info)
-		return dev_err_probe(&client->dev, -ENODEV,
-				     "unknown device model\n");
+		return dev_err_probe(dev, -ENODEV, "unknown device model\n");
 
-	ret = iio_read_mount_matrix(&client->dev, &data->orientation);
+	ret = iio_read_mount_matrix(dev, &data->orientation);
 	if (ret)
 		return ret;
 
-	data->vdd_reg = devm_regulator_get(&client->dev, "vdd");
+	data->vdd_reg = devm_regulator_get(dev, "vdd");
 	if (IS_ERR(data->vdd_reg))
-		return dev_err_probe(&client->dev, PTR_ERR(data->vdd_reg),
+		return dev_err_probe(dev, PTR_ERR(data->vdd_reg),
 				     "failed to get VDD regulator!\n");
 
-	data->vddio_reg = devm_regulator_get(&client->dev, "vddio");
+	data->vddio_reg = devm_regulator_get(dev, "vddio");
 	if (IS_ERR(data->vddio_reg))
-		return dev_err_probe(&client->dev, PTR_ERR(data->vddio_reg),
+		return dev_err_probe(dev, PTR_ERR(data->vddio_reg),
 				     "failed to get VDDIO regulator!\n");
 
 	ret = regulator_enable(data->vdd_reg);
@@ -1643,7 +1642,7 @@ static int mma8452_probe(struct i2c_client *client)
 		goto disable_regulators;
 	}
 
-	dev_info(&client->dev, "registering %s accelerometer; ID 0x%x\n",
+	dev_info(dev, "registering %s accelerometer; ID 0x%x\n",
 		 data->chip_info->name, data->chip_info->chip_id);
 
 	i2c_set_clientdata(client, indio_dev);
@@ -1676,10 +1675,10 @@ static int mma8452_probe(struct i2c_client *client)
 	if (client->irq) {
 		int irq2;
 
-		irq2 = fwnode_irq_get_byname(dev_fwnode(&client->dev), "INT2");
+		irq2 = fwnode_irq_get_byname(dev_fwnode(dev), "INT2");
 
 		if (irq2 == client->irq) {
-			dev_dbg(&client->dev, "using interrupt line INT2\n");
+			dev_dbg(dev, "using interrupt line INT2\n");
 		} else {
 			ret = i2c_smbus_write_byte_data(client,
 							MMA8452_CTRL_REG5,
@@ -1687,7 +1686,7 @@ static int mma8452_probe(struct i2c_client *client)
 			if (ret < 0)
 				goto disable_regulators;
 
-			dev_dbg(&client->dev, "using interrupt line INT1\n");
+			dev_dbg(dev, "using interrupt line INT1\n");
 		}
 
 		ret = i2c_smbus_write_byte_data(client,
@@ -1736,14 +1735,13 @@ static int mma8452_probe(struct i2c_client *client)
 			goto buffer_cleanup;
 	}
 
-	ret = pm_runtime_set_active(&client->dev);
+	ret = pm_runtime_set_active(dev);
 	if (ret < 0)
 		goto free_irq;
 
-	pm_runtime_enable(&client->dev);
-	pm_runtime_set_autosuspend_delay(&client->dev,
-					 MMA8452_AUTO_SUSPEND_DELAY_MS);
-	pm_runtime_use_autosuspend(&client->dev);
+	pm_runtime_enable(dev);
+	pm_runtime_set_autosuspend_delay(dev, MMA8452_AUTO_SUSPEND_DELAY_MS);
+	pm_runtime_use_autosuspend(dev);
 
 	ret = iio_device_register(indio_dev);
 	if (ret < 0)
@@ -1810,7 +1808,7 @@ static int mma8452_runtime_suspend(struct device *dev)
 	ret = mma8452_standby(data);
 	mutex_unlock(&data->lock);
 	if (ret < 0) {
-		dev_err(&data->client->dev, "powering off device failed\n");
+		dev_err(dev, "powering off device failed\n");
 		return -EAGAIN;
 	}
 

-- 
2.55.0

[PATCH v7 7/8] iio: accel: mma8452: Fix use-after-free bug in error error path

[Esben Haabendal]

If mma8452_probe() fails in iio_device_register() or later, we could end up
with runtime suspend callback being called with a now freed device pointer.

Fixes: 96c0cb2bbfe0 ("iio: mma8452: add support for runtime power management")
Cc: stable@vger.kernel.org
Signed-off-by: Esben Haabendal <esben@geanix.com>
---
 drivers/iio/accel/mma8452.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/iio/accel/mma8452.c b/drivers/iio/accel/mma8452.c
index bf56ebd6e091..9619b200fa5a 100644
--- a/drivers/iio/accel/mma8452.c
+++ b/drivers/iio/accel/mma8452.c
@@ -1745,7 +1745,7 @@ static int mma8452_probe(struct i2c_client *client)
 
 	ret = iio_device_register(indio_dev);
 	if (ret < 0)
-		goto free_irq;
+		goto runtime_suspend;
 
 	ret = mma8452_set_freefall_mode(data, false);
 	if (ret < 0)
@@ -1756,6 +1756,10 @@ static int mma8452_probe(struct i2c_client *client)
 unregister_device:
 	iio_device_unregister(indio_dev);
 
+runtime_suspend:
+	pm_runtime_disable(dev);
+	pm_runtime_set_suspended(dev);
+
 free_irq:
 	if (client->irq)
 		free_irq(client->irq, indio_dev);

-- 
2.55.0

[PATCH v7 8/8] iio: accel: mma8452: Use proper error code when missing device model

[Esben Haabendal]

Switch -ENODEV error on i2c_get_match_data() failure to -ENODATA to
satisfy the IIO coding style.

Reviewed-by: Joshua Crofts <joshua.crofts1@gmail.com>
Signed-off-by: Esben Haabendal <esben@geanix.com>
---
 drivers/iio/accel/mma8452.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/iio/accel/mma8452.c b/drivers/iio/accel/mma8452.c
index 9619b200fa5a..9eaced6bf242 100644
--- a/drivers/iio/accel/mma8452.c
+++ b/drivers/iio/accel/mma8452.c
@@ -1597,7 +1597,7 @@ static int mma8452_probe(struct i2c_client *client)
 
 	data->chip_info = i2c_get_match_data(client);
 	if (!data->chip_info)
-		return dev_err_probe(dev, -ENODEV, "unknown device model\n");
+		return dev_err_probe(dev, -ENODATA, "unknown device model\n");
 
 	ret = iio_read_mount_matrix(dev, &data->orientation);
 	if (ret)

-- 
2.55.0

Re: [PATCH v7 8/8] iio: accel: mma8452: Use proper error code when missing device model

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Pre-existing memory leak of iio_trigger in mma8452_probe error paths.
--

commit 19ad2f8e418619267dc62918b9e4fe522127cae2
Author: Esben Haabendal <esben@geanix.com>

iio: accel: mma8452: Use proper error code when missing device model
    
Switch -ENODEV error on i2c_get_match_data() failure to -ENODATA to
satisfy the IIO coding style.

> diff --git a/drivers/iio/accel/mma8452.c b/drivers/iio/accel/mma8452.c
> index 9619b200fa5a9..9eaced6bf2429 100644
> --- a/drivers/iio/accel/mma8452.c
> +++ b/drivers/iio/accel/mma8452.c
> @@ -1597,7 +1597,7 @@ static int mma8452_probe(struct i2c_client *client)
>  
>  	data->chip_info = i2c_get_match_data(client);
>  	if (!data->chip_info)
> -		return dev_err_probe(dev, -ENODEV, "unknown device model\n");
> +		return dev_err_probe(dev, -ENODATA, "unknown device model\n");
>  
>  	ret = iio_read_mount_matrix(dev, &data->orientation);
>  	if (ret)

[Severity: High]
This isn't a bug introduced by this patch, but there is a pre-existing 
issue in the error handling of mma8452_probe further down in this function.

When mma8452_trigger_setup is called during probe, it elevates the trigger
reference count:

drivers/iio/accel/mma8452.c:mma8452_trigger_setup() {
    ...
    indio_dev->trig = iio_trigger_get(trig);
    ...
}

If a subsequent setup step fails, such as mma8452_set_interrupt_pin_mode,
the code jumps to the cleanup label:

drivers/iio/accel/mma8452.c:mma8452_probe() {
    ...
    ret = mma8452_set_interrupt_pin_mode(data);
    if (ret)
        goto trigger_cleanup;
    ...
}

The error path leaps directly to the final mma8452_trigger_cleanup. Because 
the triggered buffer mode flag is not yet set at the time of failure, the 
IIO core omits bridging cleanup logic and skips the iio_trigger_put call.

Does this result in permanently leaking the trigger device memory?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260831-mma8452-open-drain-v7-0-22946812c928@geanix.com?part=8

Re: [PATCH v7 4/8] iio: accel: mma8452: Support interrupt sharing

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Missing rollback of `data->suspended` on runtime suspend failure permanently disables interrupt handling and breaks device operation.
- [Medium] Unconditional call to `synchronize_irq(client->irq)` may incorrectly synchronize with IRQ 0 if the device operates in polling mode.
--

commit a488fa629f355995847b3d4e006a55add5fc9ce7
Author: Esben Haabendal <esben@geanix.com>

iio: accel: mma8452: Support interrupt sharing

Adding handling of rutnime PM suspension in the interrupt handler allows
sharing interrupt with other devices.

> diff --git a/drivers/iio/accel/mma8452.c b/drivers/iio/accel/mma8452.c
> index 1fb43c5b0b72f..936eeb2efb2dd 100644
> --- a/drivers/iio/accel/mma8452.c
> +++ b/drivers/iio/accel/mma8452.c
> @@ -1056,14 +1057,24 @@ static irqreturn_t mma8452_interrupt(int irq, void *p)
>  	struct iio_dev *indio_dev = p;
>  	struct mma8452_data *data = iio_priv(indio_dev);
>  	irqreturn_t ret = IRQ_NONE;
> +	int pm_status;
>  	int src;
>  
> +	pm_status = pm_runtime_get_if_active(&data->client->dev);
> +	if (pm_status == 0)
> +		return IRQ_NONE; /* device is powered down */
> +	if (READ_ONCE(data->suspended)) {
> +		/* device is being removed */
> +		ret = IRQ_NONE;
> +		goto out_runtime_put;
> +	}

[Severity: High]
If mma8452_runtime_suspend() fails and leaves data->suspended set to true,
won't this cause all subsequent interrupts to be dropped here?

[ ... ]

> @@ -1774,6 +1790,10 @@ static int mma8452_runtime_suspend(struct device *dev)
>  		return -EAGAIN;
>  	}
>  
> +	WRITE_ONCE(data->suspended, true);
> +
> +	synchronize_irq(client->irq);

[Severity: Medium]
If the device operates in polling mode and is instantiated without a hardware
interrupt, client->irq will be 0. Calling synchronize_irq(0) on architectures
where IRQ 0 is a valid system interrupt (like the legacy timer on x86) might
needlessly block execution waiting for an unrelated system handler.

Could we check if client->irq is non-zero before calling synchronize_irq()?

> +
>  	ret = regulator_disable(data->vddio_reg);
>  	if (ret) {
>  		dev_err(dev, "failed to disable VDDIO regulator\n");
>  		return ret;
>  	}

[Severity: High]
If regulator_disable() fails here and returns an error, the PM core will abort
the suspend transition leaving the device in the RPM_ACTIVE state. However,
data->suspended is left as true, and the hardware is left in standby.

As mentioned above, since data->suspended remains true, won't the interrupt
handler unconditionally return IRQ_NONE for all future interrupts? This seems
like it would break the device's event handling until the module is reloaded.
Should we revert data->suspended to false and restore the hardware state on
this error path?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260831-mma8452-open-drain-v7-0-22946812c928@geanix.com?part=4

Re: [PATCH v7 7/8] iio: accel: mma8452: Fix use-after-free bug in error error path

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:

Pre-existing issues:
- [High] Missing runtime PM reference during probe() allows concurrent suspend and unbalanced regulator disables.
- [High] Unpowered I2C access and unbalanced regulator disables in mma8452_remove().
- [High] Runtime PM reference leak in mma8452_read() on I2C error path.
- [High] Runtime PM reference leak in mma8452_write_event_config() and mma8452_data_rdy_trigger_set_state() on I2C error paths.
--

commit f144db47a6c84b2bd25c6e5cc7cf38650461861a
Author: Esben Haabendal <esben@geanix.com>

iio: accel: mma8452: Fix use-after-free bug in error error path

If mma8452_probe() fails in iio_device_register() or later, we could end up
with runtime suspend callback being called with a now freed device pointer.

Cc: stable@vger.kernel.org

> diff --git a/drivers/iio/accel/mma8452.c b/drivers/iio/accel/mma8452.c
> index bf56ebd6e091a..9619b200fa5a9 100644
> --- a/drivers/iio/accel/mma8452.c
> +++ b/drivers/iio/accel/mma8452.c
> @@ -1745,7 +1745,7 @@ static int mma8452_probe(struct i2c_client *client)
>  
>  	ret = iio_device_register(indio_dev);
>  	if (ret < 0)
> -		goto free_irq;
> +		goto runtime_suspend;
>  
>  	ret = mma8452_set_freefall_mode(data, false);
>  	if (ret < 0)
> @@ -1756,6 +1756,10 @@ static int mma8452_probe(struct i2c_client *client)
>  unregister_device:
>  	iio_device_unregister(indio_dev);
>  
> +runtime_suspend:
> +	pm_runtime_disable(dev);
> +	pm_runtime_set_suspended(dev);
> +
>  free_irq:
>  	if (client->irq)
>  		free_irq(client->irq, indio_dev);

[Severity: High]
This isn't a bug introduced by this patch, but looking at the error paths here
in mma8452_probe(), is it possible to hit an unbalanced regulator disable if
the device suspends concurrently?

Because the shared IRQ is requested earlier in mma8452_probe(), if the
interrupt fires, mma8452_interrupt() increments and then drops the usage count
via pm_runtime_put_autosuspend():

drivers/iio/accel/mma8452.c:mma8452_interrupt() {
    ...
    if (pm_status > 0)
        pm_runtime_put_autosuspend(&data->client->dev);
    ...
}

Since pm_runtime_use_autosuspend() hasn't been called yet during this early
phase in mma8452_probe():

drivers/iio/accel/mma8452.c:mma8452_probe() {
    ...
    pm_runtime_enable(dev);
    pm_runtime_set_autosuspend_delay(dev, MMA8452_AUTO_SUSPEND_DELAY_MS);
    pm_runtime_use_autosuspend(dev);
    ...
}

Dropping the count schedules an immediate asynchronous suspend via the
workqueue. The device could suspend mid-probe, and mma8452_runtime_suspend()
would disable the hardware regulators. Subsequent I2C accesses in the
remainder of probe would hit unpowered hardware, leading to bus errors or
hangs.

If mma8452_probe() subsequently fails, the error path unconditionally calls
regulator_disable():

drivers/iio/accel/mma8452.c:mma8452_probe() {
    ...
disable_regulators:
    regulator_disable(data->vddio_reg);
    ...
}

Could this cause an unbalanced disable WARN? Should probe hold a PM usage
reference via pm_runtime_get_noresume() before enabling runtime PM?

[Severity: High]
This is a pre-existing issue, but does mma8452_remove() safely access I2C
hardware and disable regulators during driver unbind?

drivers/iio/accel/mma8452.c:mma8452_remove() {
    ...
    pm_runtime_disable(&client->dev);
    pm_runtime_set_suspended(&client->dev);

    if (client->irq)
        free_irq(client->irq, indio_dev);

    iio_triggered_buffer_cleanup(indio_dev);
    mma8452_trigger_cleanup(indio_dev);
    mma8452_standby(iio_priv(indio_dev));

    regulator_disable(data->vddio_reg);
    regulator_disable(data->vdd_reg);
    ...
}

If the device was runtime suspended (which happens automatically after 2
seconds of inactivity), the hardware is unpowered and regulators are already
disabled. mma8452_standby() performs I2C accesses on unpowered hardware,
which will fail or hang the bus. The subsequent regulator_disable() calls
drop the enable count below zero, triggering a kernel WARN. Should
mma8452_remove() wake the device via pm_runtime_get_sync() before performing
hardware cleanup?

[Severity: High]
This isn't a bug introduced by this patch, but does mma8452_read() leak a
runtime PM reference on the I2C error path?

drivers/iio/accel/mma8452.c:mma8452_read() {
    ...
    ret = mma8452_set_runtime_pm_state(data->client, true);
    if (ret)
        return ret;

    ret = i2c_smbus_read_i2c_block_data(data->client, MMA8452_OUT_X,
                        3 * sizeof(__be16), (u8 *)buf);
    if (ret < 0)
        return ret;
    ...
}

If i2c_smbus_read_i2c_block_data() fails, it returns the error immediately
without calling mma8452_set_runtime_pm_state(..., false). This leaves the PM
usage count unbalanced, preventing the device from entering runtime suspend
and causing a permanent power drain.

[Severity: High]
This isn't a bug introduced by this patch, but there appear to be similar PM
reference leaks in mma8452_write_event_config() and
mma8452_data_rdy_trigger_set_state() on I2C error paths:

drivers/iio/accel/mma8452.c:mma8452_write_event_config() {
    ...
        val = i2c_smbus_read_byte_data(data->client, ev_regs->ev_cfg);
        if (val < 0)
            return val;
    ...
}

drivers/iio/accel/mma8452.c:mma8452_data_rdy_trigger_set_state() {
    ...
    reg = i2c_smbus_read_byte_data(data->client, MMA8452_CTRL_REG4);
    if (reg < 0)
        return reg;
    ...
}

Both functions acquire a PM reference. If the subsequent hardware
configuration via I2C read fails, they return the error immediately without
rolling back the PM state, permanently breaking power management for the
device.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260831-mma8452-open-drain-v7-0-22946812c928@geanix.com?part=7

Re: [PATCH v7 3/8] iio: accel: mma8452: Only apply trigger type when not set by firmware

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

New issues:
- [High] Hardware Configuration Mismatch for Interrupt Trigger Type

Pre-existing issues:
- [High] Unbalanced regulator disables and unpowered I2C access on driver remove if runtime PM is suspended
--

commit 4e5790a8b828438ae278194b6cc6f95e4fa39c50
Author: Esben Haabendal <esben@geanix.com>

iio: accel: mma8452: Only apply trigger type when not set by firmware

Instead of unconditionally overriding the trigger type, it is better to
only apply a default when no trigger type is set by firmware. This should
be reasonably backward compatible, and should only potentially cause
problems if systems exist where firmware specifies an incorrect trigger type.

> diff --git a/drivers/iio/accel/mma8452.c b/drivers/iio/accel/mma8452.c
> index f645a5c6fd1c2..1fb43c5b0b72f 100644
> --- a/drivers/iio/accel/mma8452.c
> +++ b/drivers/iio/accel/mma8452.c
> @@ -1683,9 +1683,16 @@ static int mma8452_probe(struct i2c_client *client)
>  		goto trigger_cleanup;
>  
>  	if (client->irq) {
> +		unsigned long irq_flags;
> +
> +		irq_flags = irq_get_trigger_type(client->irq);
> +		if (irq_flags == IRQ_TYPE_NONE) {
> +			dev_info(dev, "invalid irq type, setting default active low\n");
> +			irq_flags = IRQF_TRIGGER_LOW;
> +		}
> +		irq_flags |= IRQF_ONESHOT;
>  		ret = request_threaded_irq(client->irq, NULL, mma8452_interrupt,
> -					   IRQF_TRIGGER_LOW | IRQF_ONESHOT,
> -					   client->name, indio_dev);

[Severity: High]
Does this code configure the sensor's hardware polarity to match the requested
trigger type?

If the firmware configures the interrupt with an active-high trigger (such as
IRQ_TYPE_LEVEL_HIGH), the patch reads this via irq_get_trigger_type() and
requests the interrupt from the host controller using this flag. However, the
driver never sets the IPOL bit (Interrupt Polarity, bit 1 of CTRL_REG3) to
match this configuration.

Since the sensor remains in its default active-low state, can this hardware
configuration mismatch result in the interrupt signal going unrecognized or
causing an interrupt storm on systems with an active-high trigger?

While reviewing this, I also noticed a pre-existing issue in mma8452_remove():

drivers/iio/accel/mma8452.c:mma8452_remove() {
	...
	pm_runtime_disable(&client->dev);
	pm_runtime_set_suspended(&client->dev);

	if (client->irq)
		free_irq(client->irq, indio_dev);

	iio_triggered_buffer_cleanup(indio_dev);
	mma8452_trigger_cleanup(indio_dev);
	mma8452_standby(iio_priv(indio_dev));

	regulator_disable(data->vddio_reg);
	regulator_disable(data->vdd_reg);
}

[Severity: High]
This is a pre-existing issue, but can this cause a regulator enable count
underflow and I2C timeouts?

If the module is removed while the device is in a runtime-suspended state
(where mma8452_runtime_suspend() has already disabled both regulators),
mma8452_remove() calls pm_runtime_disable() which does not resume the device.

It then calls mma8452_standby(), which performs I2C writes to the unpowered
device, and unconditionally disables the regulators again, causing an underflow.

Is there a way to ensure the device is woken up or the regulator states are
checked before proceeding with removal?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260831-mma8452-open-drain-v7-0-22946812c928@geanix.com?part=3

Re: [PATCH v7 5/8] iio: accel: mma8452: Allow open drain interrupt pin configuration

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Pre-existing issues:
- [High] The MMA8452 driver permanently loses volatile hardware configuration upon runtime PM resume because it fails to restore registers wiped by the regulator power-down.
- [High] Sysfs read and write handlers execute hardware I2C operations without runtime PM protection, causing transactions to fail against an unpowered chip.
--

commit 3785b0b6d22b638ef18d797d66c6f15f51fc0132
Author: Esben Haabendal <esben@geanix.com>

iio: accel: mma8452: Allow open drain interrupt pin configuration
    
When designing systems sharing the interrupt for mma8452 chips, it is
helpful to be able to configure the irq pin in open-drain mode (default is
push-pull).
    
Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com>
Signed-off-by: Esben Haabendal <esben@geanix.com>

> diff --git a/drivers/iio/accel/mma8452.c b/drivers/iio/accel/mma8452.c
> index 936eeb2efb2dd..2ba6e2c44cb9e 100644
> --- a/drivers/iio/accel/mma8452.c
> +++ b/drivers/iio/accel/mma8452.c

[ ... ]

> @@ -1830,6 +1854,10 @@ static int mma8452_runtime_resume(struct device *dev)
>  
>  	WRITE_ONCE(data->suspended, false);
>  
> +	ret = mma8452_set_interrupt_pin_mode(data);
> +	if (ret)
> +		goto runtime_resume_failed;
> +
>  	ret = mma8452_active(data);
>  	if (ret < 0)
>  		goto runtime_resume_failed;

[Severity: High]
This isn't a bug introduced by this patch, but does mma8452_runtime_resume()
fail to restore the full device configuration?

Disabling the VDD and VDDIO regulators in mma8452_runtime_suspend() causes the
chip to lose power and reset to defaults. However, mma8452_runtime_resume()
only restores CTRL_REG1 and CTRL_REG3.

It seems to omit restoring operational parameters established during probe or
via sysfs, such as DATA_CFG (scale) and event thresholds. Does this lead to
silent data corruption, where the software-cached state (like data->data_cfg
representing an 8g scale) becomes desynchronized from the hardware (which
resets to 2g), causing subsequent sensor readings to be misinterpreted by
userspace?

[Severity: High]
This is a pre-existing issue, but do the sysfs read and write handlers execute
hardware I2C operations without runtime PM protection?

The driver implements pm_runtime_autosuspend to disable regulators after 2
seconds of inactivity. However, mma8452_read() calls mma8452_drdy(), which
executes an I2C read on MMA8452_STATUS before invoking
mma8452_set_runtime_pm_state(..., true) to wake the device:

static int mma8452_read(struct mma8452_data *data, __be16 buf[3])
{
	int ret = mma8452_drdy(data);

	if (ret < 0)
		return ret;

	ret = mma8452_set_runtime_pm_state(data->client, true);
        ...

Similarly, mma8452_write_raw() and mma8452_write_event_value() execute I2C
transactions without calling pm_runtime_resume_and_get().

Since the chip is unpowered during autosuspend, will these sysfs operations
routinely timeout or NACK when the device is suspended, rendering the
interface unusable for polling?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260831-mma8452-open-drain-v7-0-22946812c928@geanix.com?part=5

Re: [PATCH v7 7/8] iio: accel: mma8452: Fix use-after-free bug in error error path

[Joshua Crofts]

On Mon, 31 Aug 2026 14:17:10 +0200
Esben Haabendal <esben@geanix.com> wrote:

> If mma8452_probe() fails in iio_device_register() or later, we could end up
> with runtime suspend callback being called with a now freed device pointer.
> 
> Fixes: 96c0cb2bbfe0 ("iio: mma8452: add support for runtime power management")
> Cc: stable@vger.kernel.org
> Signed-off-by: Esben Haabendal <esben@geanix.com>
> ---

Reviewed-by: Joshua Crofts <joshua.crofts1@gmail.com>

-- 
Kind regards,
Joshua Crofts

Re: [PATCH v7 4/8] iio: accel: mma8452: Support interrupt sharing

[Andy Shevchenko]

On Mon, Aug 31, 2026 at 02:17:07PM +0200, Esben Haabendal wrote:
> Adding handling of rutnime PM suspension in the interrupt handler allows
> sharing interrupt with other devices.
> 
> Keep in mind that the device by default is using push-pull for the irq pin,
> which might require additional hardware design to allow interrupt sharing.
> 
> The suspended flag is added together with synchronize_irq() in order to
> protect against race conditions when doing runtime suspend and device
> removal. This way we ensure that interrupt handler does not try to access
> the device while regulators are disabled.

...

> static int mma8452_runtime_resume(struct device *dev)

>  		return ret;
>  	}
>  
> +	WRITE_ONCE(data->suspended, false);
> +
>  	ret = mma8452_active(data);
>  	if (ret < 0)
>  		goto runtime_resume_failed;

>  	return 0;
>  
>  runtime_resume_failed:
> +	WRITE_ONCE(data->suspended, true);
>  	regulator_disable(data->vddio_reg);
>  	regulator_disable(data->vdd_reg);

But with this, what's the point in having WRITE_ONCE()? It can be read
just in the middle as true and be immediately changed afterwards. It
may be that I am missing something, but I think WRITE_ONCE() should be
done once in this function.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v7 6/8] iio: accel: mma8452: Reuse existing dev pointer in mma8452_probe()

[Andy Shevchenko]

On Mon, Aug 31, 2026 at 02:17:09PM +0200, Esben Haabendal wrote:
> In commit 32a5c04d4575 ("iio: accel: mma8452: Use dev_err_probe()") the
> struct device * pointer was assigned to local variable dev, so we can just
> as well reuse that throughout the function for sligthly more readable code.

Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com>

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v7 8/8] iio: accel: mma8452: Use proper error code when missing device model

[Andy Shevchenko]

On Mon, Aug 31, 2026 at 02:17:11PM +0200, Esben Haabendal wrote:
> Switch -ENODEV error on i2c_get_match_data() failure to -ENODATA to
> satisfy the IIO coding style.

Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com>

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v7 7/8] iio: accel: mma8452: Fix use-after-free bug in error error path

[Andy Shevchenko]

On Mon, Aug 31, 2026 at 02:17:10PM +0200, Esben Haabendal wrote:
> If mma8452_probe() fails in iio_device_register() or later, we could end up
> with runtime suspend callback being called with a now freed device pointer.

> Fixes: 96c0cb2bbfe0 ("iio: mma8452: add support for runtime power management")

Can this be popped up in the series? I think previous couple of patches are not
fixes...

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v7 4/8] iio: accel: mma8452: Support interrupt sharing

[Esben Haabendal]

"Andy Shevchenko" <andriy.shevchenko@intel.com> writes:

> On Mon, Aug 31, 2026 at 02:17:07PM +0200, Esben Haabendal wrote:
>> Adding handling of rutnime PM suspension in the interrupt handler allows
>> sharing interrupt with other devices.
>>
>> Keep in mind that the device by default is using push-pull for the irq pin,
>> which might require additional hardware design to allow interrupt sharing.
>>
>> The suspended flag is added together with synchronize_irq() in order to
>> protect against race conditions when doing runtime suspend and device
>> removal. This way we ensure that interrupt handler does not try to access
>> the device while regulators are disabled.
>
> ...
>
>> static int mma8452_runtime_resume(struct device *dev)
>
>>  		return ret;
>>  	}
>>
>> +	WRITE_ONCE(data->suspended, false);
>> +
>>  	ret = mma8452_active(data);
>>  	if (ret < 0)
>>  		goto runtime_resume_failed;
>
>>  	return 0;
>>
>>  runtime_resume_failed:
>> +	WRITE_ONCE(data->suspended, true);
>>  	regulator_disable(data->vddio_reg);
>>  	regulator_disable(data->vdd_reg);
>
> But with this, what's the point in having WRITE_ONCE()? It can be read
> just in the middle as true and be immediately changed afterwards. It
> may be that I am missing something, but I think WRITE_ONCE() should be
> done once in this function.

Yes, there does look like there is still a race condition after adding
this data->suspended flag. An irq handler could just have read
data->suspended, gotten false, and thereafter proceeeded with handling
the irq, and then we write data->suspended=true and the irq handler
would just continue with accessing the device, even though we are now
(if possible) powering down the device.

I did go through all the pre-existing runtime pm and other race
condition issues raised by sashiko-bot during this review, and worked
through it all. The result is a quite a bit larger than what I would
like to add on top of this series. Among other things, it converts the
driver to use regmap for accessing the i2c registers, and after various
fixes the data->suspended flag is removed again.

So I am a bit hesitant to pull all those changes into this series, the
combined series would blow up quite a bit. But if required, I guess I
can do that, although I fear that it will not make reviewing easier to
mix things more than maybe needed.

Could we find a way to merge this series first in some way, or should I
post a new version with all the other fixes added on top?

/Esben

Re: [PATCH v7 7/8] iio: accel: mma8452: Fix use-after-free bug in error error path

[Esben Haabendal]

"Andy Shevchenko" <andriy.shevchenko@intel.com> writes:

> On Mon, Aug 31, 2026 at 02:17:10PM +0200, Esben Haabendal wrote:
>> If mma8452_probe() fails in iio_device_register() or later, we could end up
>> with runtime suspend callback being called with a now freed device pointer.
>
>> Fixes: 96c0cb2bbfe0 ("iio: mma8452: add support for runtime power management")
>
> Can this be popped up in the series? I think previous couple of patches are not
> fixes...

Yes, popping it in front of all the other code patches in the series
does not seem to cause problems. I have scheduled that for v8.

/Esben

Re: [PATCH v7 8/8] iio: accel: mma8452: Use proper error code when missing device model

[Esben Haabendal]

<sashiko-bot@kernel.org> writes:

> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
>
> Pre-existing issues:
> - [High] Pre-existing memory leak of iio_trigger in mma8452_probe error paths.

I have a fix for this queued up for the follow-up series that I have
ready to send once this series is merged.

/Esben

Re: [PATCH v7 4/8] iio: accel: mma8452: Support interrupt sharing

[Andy Shevchenko]

On Mon, Aug 31, 2026 at 05:43:06PM +0200, Esben Haabendal wrote:
> "Andy Shevchenko" <andriy.shevchenko@intel.com> writes:
> > On Mon, Aug 31, 2026 at 02:17:07PM +0200, Esben Haabendal wrote:

...

> >> +	WRITE_ONCE(data->suspended, false);
> >> +
> >>  	ret = mma8452_active(data);
> >>  	if (ret < 0)
> >>  		goto runtime_resume_failed;
> >
> >>  	return 0;
> >>
> >>  runtime_resume_failed:
> >> +	WRITE_ONCE(data->suspended, true);
> >>  	regulator_disable(data->vddio_reg);
> >>  	regulator_disable(data->vdd_reg);
> >
> > But with this, what's the point in having WRITE_ONCE()? It can be read
> > just in the middle as true and be immediately changed afterwards. It
> > may be that I am missing something, but I think WRITE_ONCE() should be
> > done once in this function.
> 
> Yes, there does look like there is still a race condition after adding
> this data->suspended flag. An irq handler could just have read
> data->suspended, gotten false, and thereafter proceeeded with handling
> the irq, and then we write data->suspended=true and the irq handler
> would just continue with accessing the device, even though we are now
> (if possible) powering down the device.
> 
> I did go through all the pre-existing runtime pm and other race
> condition issues raised by sashiko-bot during this review, and worked
> through it all. The result is a quite a bit larger than what I would
> like to add on top of this series. Among other things, it converts the
> driver to use regmap for accessing the i2c registers, and after various
> fixes the data->suspended flag is removed again.
> 
> So I am a bit hesitant to pull all those changes into this series, the
> combined series would blow up quite a bit. But if required, I guess I
> can do that, although I fear that it will not make reviewing easier to
> mix things more than maybe needed.

Personally I would go with it, or as a compromise, publish somewhere in Git
(like on GitHub) and share the link to the repo (in a format that one can run
with `git fetch ...`).

> Could we find a way to merge this series first in some way, or should I
> post a new version with all the other fixes added on top?

It's a Q to Jonathan.

-- 
With Best Regards,
Andy Shevchenko