[PATCH RESEND v5 0/6] io: accel: mma8452: Allow open drain interrupt pin configuration

[PATCH RESEND v5 0/6] io: accel: mma8452: Allow open drain interrupt pin configuration

[Esben Haabendal]

This is a resend of v5 which was not fully sent first time for some unknown
reason.

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 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>
Cc: linux-iio@vger.kernel.org
Cc: devicetree@vger.kernel.org
Cc: linux-kernel@vger.kernel.org

---
Esben Haabendal (6):
      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()

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

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

[PATCH RESEND v5 1/6] 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 RESEND v5 2/6] 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 RESEND v5 3/6] 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 RESEND v5 4/6] 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 | 22 +++++++++++++++++++---
 1 file changed, 19 insertions(+), 3 deletions(-)

diff --git a/drivers/iio/accel/mma8452.c b/drivers/iio/accel/mma8452.c
index 1fb43c5b0b72..25bb2dd69381 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,19 @@ 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 || READ_ONCE(data->suspended))
+		return IRQ_NONE; /* device is powered down or being removed */
+
 	src = i2c_smbus_read_byte_data(data->client, MMA8452_INT_SRC);
 	if (src < 0)
-		return IRQ_NONE;
+		goto out;
 
 	if (!(src & (data->chip_info->enabled_events | MMA8452_INT_DRDY)))
-		return IRQ_NONE;
+		goto out;
 
 	if (src & MMA8452_INT_DRDY) {
 		iio_trigger_poll_nested(indio_dev->trig);
@@ -1089,6 +1095,10 @@ static irqreturn_t mma8452_interrupt(int irq, void *p)
 		ret = IRQ_HANDLED;
 	}
 
+out:
+	if (pm_status > 0)
+		pm_runtime_put_autosuspend(&data->client->dev);
+
 	return ret;
 }
 
@@ -1690,7 +1700,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)
@@ -1768,12 +1778,15 @@ static int mma8452_runtime_suspend(struct device *dev)
 
 	mutex_lock(&data->lock);
 	ret = mma8452_standby(data);
+	WRITE_ONCE(data->suspended, true);
 	mutex_unlock(&data->lock);
 	if (ret < 0) {
 		dev_err(&data->client->dev, "powering off device failed\n");
 		return -EAGAIN;
 	}
 
+	synchronize_irq(client->irq);
+
 	ret = regulator_disable(data->vddio_reg);
 	if (ret) {
 		dev_err(dev, "failed to disable VDDIO regulator\n");
@@ -1808,6 +1821,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 +1837,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 RESEND v5 5/6] 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 25bb2dd69381..060a6207e52d 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)
 {
@@ -1677,6 +1696,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);
 
@@ -1823,6 +1847,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 RESEND v5 6/6] 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.

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

diff --git a/drivers/iio/accel/mma8452.c b/drivers/iio/accel/mma8452.c
index 060a6207e52d..bf5cb1c4425a 100644
--- a/drivers/iio/accel/mma8452.c
+++ b/drivers/iio/accel/mma8452.c
@@ -1582,7 +1582,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;
 
@@ -1592,21 +1592,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);
@@ -1638,7 +1637,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);
@@ -1671,10 +1670,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,
@@ -1682,7 +1681,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,
@@ -1731,14 +1730,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)

-- 
2.55.0

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

[Joshua Crofts]

On Wed, 19 Aug 2026 10:15:33 +0200
Esben Haabendal <esben@geanix.com> 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.
> 
> Signed-off-by: Esben Haabendal <esben@geanix.com>
> ---

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

-- 
Kind regards,
Joshua Crofts

Re: [PATCH RESEND v5 4/6] iio: accel: mma8452: Support interrupt sharing

[sashiko-bot]

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

New issues:
- [High] Failed runtime suspend leaves the `suspended` flag set to `true`, causing an unrecoverable PM usage counter leak and a potential interrupt storm.
- [Low] Compilation Error: undeclared variable `client` used in `mma8452_runtime_suspend`.

Pre-existing issues:
- [High] Unbalanced `regulator_disable()` calls and I2C writes to a powered-off device in `mma8452_remove()`.
--

--- Patch [4]: [PATCH RESEND v5 4/6] iio: accel: mma8452: Support interrupt sharing ---
commit 79ad9089167d804daaa97086d058cd3a0e2f4199
Author: Esben Haabendal <esben@geanix.com>

iio: accel: mma8452: Support interrupt sharing

This commit adds handling of runtime PM suspension in the interrupt handler
to allow sharing interrupts with other devices. It introduces a suspended flag
and uses synchronize_irq() to protect against race conditions during runtime
suspend and device removal.

> diff --git a/drivers/iio/accel/mma8452.c b/drivers/iio/accel/mma8452.c
> index 1fb43c5b0b72f..25bb2dd69381e 100644
> --- a/drivers/iio/accel/mma8452.c
> +++ b/drivers/iio/accel/mma8452.c
[ ... ]
> @@ -1056,14 +1057,19 @@ 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 || READ_ONCE(data->suspended))
> +		return IRQ_NONE; /* device is powered down or being removed */
> +

[Severity: High]
If mma8452_runtime_suspend() in drivers/iio/accel/mma8452.c fails and
leaves data->suspended set to true, can this early return cause a PM
usage counter leak and an interrupt storm?

If the device is kept in the RPM_ACTIVE state after a suspend failure,
pm_runtime_get_if_active() will return 1. Because data->suspended is true,
the handler will return IRQ_NONE immediately.

This skips pm_runtime_put_autosuspend(), leaking the usage counter. Also,
by returning IRQ_NONE on a shared level-triggered IRQ without clearing the
hardware interrupt, wouldn't this trigger an interrupt storm?

[ ... ]
> @@ -1768,12 +1778,15 @@ static int mma8452_runtime_suspend(struct device *dev)
>  
>  	mutex_lock(&data->lock);
>  	ret = mma8452_standby(data);
> +	WRITE_ONCE(data->suspended, true);
>  	mutex_unlock(&data->lock);
>  	if (ret < 0) {
>  		dev_err(&data->client->dev, "powering off device failed\n");
>  		return -EAGAIN;
>  	}

[Severity: High]
If mma8452_standby() fails, should data->suspended be reverted to false?

When returning -EAGAIN on failure in drivers/iio/accel/mma8452.c,
leaving data->suspended set to true creates the inconsistent state that
breaks mma8452_interrupt() as noted above.

>  
> +	synchronize_irq(client->irq);
> +

[Severity: Low]
Does this code compile?

There is no local variable named client in mma8452_runtime_suspend() in
drivers/iio/accel/mma8452.c. The I2C client is available via
data->client or to_i2c_client(dev), so using client->irq directly
appears to cause a compilation failure.

[ ... ]

[Severity: High]
This is a pre-existing issue, but I noticed a potential problem during
device removal in mma8452_remove() in drivers/iio/accel/mma8452.c:

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

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

If the device is already in the RPM_SUSPENDED state when mma8452_remove() is
called, its regulators have already been disabled by
mma8452_runtime_suspend().

Because pm_runtime_disable() does not resume the device, calling
regulator_disable() again will underflow the regulator enable count. Could
this forcefully disable shared regulators and cause system instability?

Additionally, mma8452_standby() performs an I2C write. Is it safe to
unconditionally access the hardware when it might be powered off?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260819-mma8452-open-drain-v5-0-d8aa590d7c36@geanix.com?part=4

Re: [PATCH RESEND v5 2/6] iio: accel: mma8452: Optimize struct mma8452_data member orders

[sashiko-bot]

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

Pre-existing issues:
- [High] The mma8452_probe function fails to disable PM runtime on error paths, causing a Use-After-Free.
- [High] mma8452_remove() performs I2C accesses and disables regulators without first resuming the device, leading to timeouts and unbalanced regulator counts.
- [High] `mma8452_runtime_resume` does not hold `data->lock`, causing severe race conditions with `mma8452_change_config`.
- [High] I2C registers are accessed before the device is resumed from PM runtime suspend, leading to systematic I/O errors when regulators are off.
- [High] Kernel stack memory can be leaked to userspace due to unhandled short reads from the I2C bus.
- [Medium] Integer overflow in `val * USEC_PER_SEC` allows bypassing bounds checks when configuring the event period.
--

--- Patch [2]: [PATCH RESEND v5 2/6] iio: accel: mma8452: Optimize struct mma8452_data member orders ---
commit ad9de8da12adc66a058b85d96e7d8e823d6389d4
Author: Esben Haabendal <esben@geanix.com>

iio: accel: mma8452: Optimize struct mma8452_data member orders

This commit reorders the members of struct mma8452_data to eliminate padding
holes. It groups the sleep_val, ctrl_reg1, and data_cfg members at the end
of the structure.

> diff --git a/drivers/iio/accel/mma8452.c b/drivers/iio/accel/mma8452.c
> index 7d683686dd9d6..f645a5c6fd1c2 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;
>  };

[Severity: High]
This is a pre-existing issue, but does mma8452_probe() fail to disable PM
runtime on error paths?

If a step like iio_device_register() fails after PM runtime is enabled via
pm_runtime_enable() and pm_runtime_use_autosuspend(), the code jumps to the
error labels:

drivers/iio/accel/mma8452.c:mma8452_probe() {
...
free_irq:
	if (client->irq)
		free_irq(client->irq, indio_dev);
...

Since it misses calling pm_runtime_disable(&client->dev), a pending PM
autosuspend timer can still fire after the devres-managed data and indio_dev
are freed.

When mma8452_runtime_suspend() runs, it attempts to lock data->lock and
access regulators within the freed structure. Can this lead to a
use-after-free?


[Severity: High]
This isn't a bug introduced by this patch, but does mma8452_remove() tear
down resources without waking the device first?

If the device is in autosuspend, its regulators are disabled. During module
removal, mma8452_remove() prevents future state changes but does not wake up
the device:

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

It then calls mma8452_standby() which initiates an I2C transaction on the
powered-off chip, causing an I2C timeout.

It also calls regulator_disable() on the already disabled regulators,
triggering a WARN for unbalanced disables. Should there be a
pm_runtime_get_sync() at the beginning of the remove function?


[Severity: High]
This is a pre-existing issue, but does mma8452_runtime_resume() modify the
device active state without holding the data->lock?

The runtime resume handler performs an unprotected I2C write to
MMA8452_CTRL_REG1:

drivers/iio/accel/mma8452.c:mma8452_runtime_resume() {
...
	ret = mma8452_active(data);
	if (ret < 0)
		goto runtime_resume_failed;
...

If PM core resumes the device concurrently with a sysfs attribute write
(which relies on exclusive lock protection to keep the device in STANDBY),
their writes can interleave:

drivers/iio/accel/mma8452.c:mma8452_change_config() {
...
	/* config can only be changed when in standby */
	if (is_active > 0) {
		ret = mma8452_standby(data);
		if (ret < 0)
			goto fail;
	}
...

A concurrent unprotected mma8452_active() from the resume path will
prematurely switch the device to ACTIVE, violating the chip's requirement
that configurations only change during STANDBY. Can this cause sysfs
writes to be silently ignored?


[Severity: High]
This isn't a bug introduced by this patch, but does the driver attempt I2C
operations before the device is resumed from PM runtime suspend?

The driver supports PM autosuspend, which turns off power to the hardware
via the regulators. When reading data, data ready polling is performed before
setting the PM runtime state to active:

drivers/iio/accel/mma8452.c:mma8452_read() {
...
	int ret = mma8452_drdy(data);

	if (ret < 0)
		return ret;

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

If the device is suspended and unpowered, the I2C read in mma8452_drdy()
fails with -EIO, aborting the read before the device is woken up.

Similarly, sysfs writes via __mma8452_write_raw() perform I2C operations
without requesting a PM resume at all:

drivers/iio/accel/mma8452.c:__mma8452_write_raw() {
...
return mma8452_change_config(data, MMA8452_CTRL_REG1,
					     data->ctrl_reg1);
...

Does this lead to systematic I/O errors when the regulators are off?


[Severity: High]
This is a pre-existing issue, but does mma8452_read_raw() leak kernel stack
memory due to unhandled short reads?

When mma8452_read() performs the I2C block read, it overwrites the positive
return value (bytes read) with 0:

drivers/iio/accel/mma8452.c:mma8452_read() {
...
	ret = i2c_smbus_read_i2c_block_data(data->client, MMA8452_OUT_X,
					    3 * sizeof(__be16), (u8 *)buf);
	if (ret < 0)
		return ret;

	ret = mma8452_set_runtime_pm_state(data->client, false);

	return ret;
...

Because it erases the knowledge of a short read, mma8452_read_raw() blindly
accesses its local uninitialized array:

drivers/iio/accel/mma8452.c:mma8452_read_raw() {
...
		*val = sign_extend32(be16_to_cpu(
			buffer[chan->scan_index]) >> chan->scan_type.shift,
			chan->scan_type.realbits - 1);
...

If the block read returns fewer bytes than requested, could accessing the
unpopulated trailing portion of the array leak uninitialized stack memory to
userspace?


[Severity: Medium]
This isn't a bug introduced by this patch, but does the event period
configuration suffer from an integer overflow?

In mma8452_write_event_value(), a 32-bit multiplication is performed before
deriving and validating the hardware configuration steps:

drivers/iio/accel/mma8452.c:mma8452_write_event_value() {
...
		steps = (val * USEC_PER_SEC + val2) /
				mma8452_time_step_us[ret][
					mma8452_get_odr_index(data)];

		if (steps < 0 || steps > 0xff)
			return -EINVAL;
...

On 32-bit systems, USEC_PER_SEC is a 32-bit long. Since val is derived from
user input as a signed 32-bit integer, providing a large positive value
(e.g., 4300) can cause val * 1000000 to overflow and wrap around to a small
positive integer.

Because the wrapped value will then seamlessly pass the bounds check, does
this allow userspace to bypass validation and program an incorrect period
into the hardware?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260819-mma8452-open-drain-v5-0-d8aa590d7c36@geanix.com?part=2

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

[sashiko-bot]

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

New issues:
- [Low] Compilation Error in mma8452_runtime_suspend() due to undeclared identifier 'client'.

Pre-existing issues:
- [High] Missing pm_runtime_disable() in mma8452_probe() error paths leads to a guaranteed NULL pointer dereference in the autosuspend callback.
--

--- Patch [6]: [PATCH RESEND v5 6/6] iio: accel: mma8452: Reuse existing dev pointer in mma8452_probe() ---
commit 004c22b83c7e1b63662958bd7b1a838d3015cf83
Author: Esben Haabendal <esben@geanix.com>

iio: accel: mma8452: Reuse existing dev pointer in mma8452_probe()

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.

> diff --git a/drivers/iio/accel/mma8452.c b/drivers/iio/accel/mma8452.c
> index 060a6207e52d0..bf5cb1c4425a3 100644
> --- a/drivers/iio/accel/mma8452.c
> +++ b/drivers/iio/accel/mma8452.c

[ ... ]

> @@ -1731,14 +1730,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)

[Severity: High]
This is a pre-existing issue, but if mma8452_probe() fails here, is there a
missing pm_runtime_disable() in the error paths?

If mma8452_probe() fails after enabling PM runtime (for example, during
iio_device_register()), it returns an error without disabling PM runtime or
autosuspend. The driver core cleans up the device and clears its driver data
via dev_set_drvdata(dev, NULL), and queues an idle notification.

Because PM runtime is still active, this triggers the autosuspend timer.
When the timer expires, the autosuspend callback executes:

drivers/iio/accel/mma8452.c:mma8452_runtime_suspend() {
    struct iio_dev *indio_dev = i2c_get_clientdata(to_i2c_client(dev));
    struct mma8452_data *data = iio_priv(indio_dev);
    ...
}

Does this lead to a reliable NULL pointer dereference when iio_priv()
dereferences the cleared driver data?

[Severity: Low]
Also, looking at mma8452_runtime_suspend(), does the module fail to compile
when CONFIG_PM is enabled due to an undeclared identifier?

The code attempts to call synchronize_irq():

drivers/iio/accel/mma8452.c:mma8452_runtime_suspend() {
    ...
    synchronize_irq(client->irq);
    ...
}

Since client is not declared in the scope of mma8452_runtime_suspend(),
should this access data->client->irq instead?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260819-mma8452-open-drain-v5-0-d8aa590d7c36@geanix.com?part=6

Re: [PATCH RESEND v5 3/6] iio: accel: mma8452: Only apply trigger type when not set by firmware

[sashiko-bot]

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

New issues:
- [High] The patch dynamically trusts the firmware's IRQ trigger type but fails to configure the sensor hardware's interrupt polarity to match, causing an interrupt storm.

Pre-existing issues:
- [High] I2C register accesses are performed while the device is physically powered off in PM runtime suspend.
- [High] Unhandled latched motion events (`INT_FF_MT`) cause an infinite IRQ storm.
--

--- Patch [3]: [PATCH RESEND v5 3/6] iio: accel: mma8452: Only apply trigger type when not set by firmware ---
commit ad034b1db299a86a70db53d5a2b4a52f2a134eee
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.

> 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);

[Severity: High]
If the firmware specifies an active-high interrupt, will this cause an
interrupt storm?

The irq_get_trigger_type function fetches the interrupt polarity from the
firmware (e.g., active-high) and passes it to request_threaded_irq.
However, the driver forces the sensor into its power-on default state
via mma8452_reset, which hardcodes the interrupt pin to active-low.

Because the driver never writes to CTRL_REG3 to set the IPOL bit
to active-high to match the requested IRQ type, the sensor's idle-high
line could be seen as a permanently asserted interrupt.

> +		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;
>  	}

[Severity: High]
This isn't a bug introduced by this patch, but are there I2C register
accesses occurring while the device is physically powered off?

If userspace attempts to read or write sysfs attributes after the
autosuspend timeout, the regulators are disabled. Direct calls are
made without resuming the device:

drivers/iio/accel/mma8452.c:mma8452_read_raw() {
    ...
    ret = i2c_smbus_read_byte_data(data->client,
                                   MMA8452_OFF_X + chan->scan_index);
    ...
}

A similar pattern happens in __mma8452_write_raw calling
mma8452_change_config. Any unprivileged userspace read/write to the
configuration attributes 2 seconds after the last activity will result
in an I2C NACK (-EREMOTEIO or -ENXIO).

[Severity: High]
This is a pre-existing issue, but can unhandled latched motion events
cause an infinite IRQ storm?

For chips that lack the TRANS event (like mma8652), rising-edge motion
events are mapped to the ff_mt registers. When the latch is explicitly
enabled for rising events, the hardware will hold the interrupt until
read:

drivers/iio/accel/mma8452.c:mma8452_write_event_config() {
    ...
    val |= ev_regs->ev_cfg_ele;
    ...
}

When the interrupt fires, the handler evaluates freefall mode and skips
handling, never reading the MMA8452_FF_MT_SRC register to clear the latch:

drivers/iio/accel/mma8452.c:mma8452_interrupt() {
    ...
    if (src & MMA8452_INT_FF_MT) {
        if (mma8452_freefall_mode_enabled(data)) {
            ...
        }
        ret = IRQ_HANDLED;
    }
    ...
}

Remotely triggering a physical motion event after enabling the rising
threshold via sysfs could permanently lock up the interrupt line.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260819-mma8452-open-drain-v5-0-d8aa590d7c36@geanix.com?part=3

Re: [PATCH RESEND v5 5/6] iio: accel: mma8452: Allow open drain interrupt pin configuration

[sashiko-bot]

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

New issues:
- [High] Hardware interrupt polarity mismatch.
- [Low] Compilation failure due to undeclared `client` variable in `mma8452_runtime_suspend`.

Pre-existing issues:
- [High] Missing restore of critical device configuration registers during PM runtime resume.
- [High] AB-BA deadlock between driver lock and PM runtime lock.
- [High] Unpowered I2C access and data race on `ctrl_reg1` in `write_raw`.
- [High] Early interrupt registration risks NULL pointer dereference.
- [High] Unbalanced regulator disable and unpowered I2C access during driver removal.
--

--- Patch [5]: [PATCH RESEND v5 5/6] iio: accel: mma8452: Allow open drain interrupt pin configuration ---
commit 8b317beed417968ce06b604c39245c855aa368c3
Author: Esben Haabendal <esben@geanix.com>

iio: accel: mma8452: Allow open drain interrupt pin configuration

This patch adds support for configuring the mma8452 interrupt pin in
open-drain mode. It reads the "drive-open-drain" device property and sets
the appropriate bit in CTRL_REG3 during probe and runtime resume.

> diff --git a/drivers/iio/accel/mma8452.c b/drivers/iio/accel/mma8452.c
> index 25bb2dd69381e..060a6207e52d0 100644
> --- a/drivers/iio/accel/mma8452.c
> +++ b/drivers/iio/accel/mma8452.c

[ ... ]

> @@ -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);
> +}

[Severity: High]
Does this also need to configure the interrupt polarity bit (IPOL) to match
the requested trigger type?

The driver parses the IRQ trigger type from device properties using
irq_get_trigger_type() and registers the interrupt accordingly, but it
doesn't seem to program the hardware to match.

Since the hardware defaults to active-low, could this cause missed
interrupts or interrupt storms on systems that specify an active-high
trigger?

[ ... ]

> @@ -1677,6 +1696,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 |

[Severity: High]
This is a pre-existing issue, but is it safe to register the interrupt
before the iio event interfaces are allocated?

In mma8452_probe(), request_threaded_irq() is called before
iio_device_register().

If the hardware generates an interrupt immediately, the handler calls
iio_push_event(), which accesses indio_dev->event_interface. Since
event_interface is only allocated during iio_device_register(), could this
cause a NULL pointer dereference?

[ ... ]

> @@ -1823,6 +1847,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);

[Severity: High]
This is a pre-existing issue, but does the driver restore all necessary
hardware configurations during runtime resume?

When regulators are physically disabled during runtime suspend, the hardware
resets to defaults.

It appears the resume path only restores CTRL_REG1 and CTRL_REG3, leaving
configurations set during probe (like MMA8452_DATA_CFG, MMA8452_CTRL_REG4,
and MMA8452_CTRL_REG5) uninitialized.

Could this result in a permanent loss of critical sensor settings?

[ ... ]

[Severity: Low]
In mma8452_runtime_suspend(), there appears to be a compilation issue:

drivers/iio/accel/mma8452.c:mma8452_runtime_suspend()
    synchronize_irq(client->irq);

The 'client' identifier is used here without being declared in this scope.
Will this cause the driver to fail to compile when CONFIG_PM is enabled?

[Severity: High]
This is a pre-existing issue, but is there a potential AB-BA deadlock between
the driver lock and the PM runtime lock?

In mma8452_read_raw(), the driver acquires data->lock and then indirectly
calls pm_runtime_resume_and_get(), which waits for the device PM state to
change.

Concurrently, if the PM subsystem is executing mma8452_runtime_suspend(),
it holds the PM synchronization lock and attempts to acquire data->lock.

Could this lead to a system hang when user-space reads sysfs attributes while
the PM autosuspend worker runs?

[Severity: High]
This is a pre-existing issue, but is there an unpowered I2C access and data
race in the raw write path?

drivers/iio/accel/mma8452.c:__mma8452_write_raw()
    data->ctrl_reg1 &= ~MMA8452_CTRL_DR_MASK;
    data->ctrl_reg1 |= i << MMA8452_CTRL_DR_SHIFT;
    ...
    return mma8452_change_config(data, MMA8452_CTRL_REG1, data->ctrl_reg1);

This modifies data->ctrl_reg1 locklessly and accesses the hardware via
mma8452_change_config() without ensuring the device is powered on via
PM runtime.

If the device is runtime suspended, won't the I2C transaction fail?

Additionally, if a resume occurs concurrently, mma8452_runtime_resume()
reads data->ctrl_reg1, which seems to create a data race.

[Severity: High]
This is a pre-existing issue, but are regulators disabled safely during driver
removal?

drivers/iio/accel/mma8452.c:mma8452_remove()
    mma8452_standby(iio_priv(indio_dev));
    regulator_disable(data->vddio_reg);
    regulator_disable(data->vdd_reg);

If the device is already runtime-suspended, pm_runtime_disable() does not
resume the device.

Calling mma8452_standby() performs I2C writes to a powered-off sensor. Then,
regulator_disable() is called unconditionally on both regulators, decrementing
their use counts below zero.

Could this cause I2C transfer failures and kernel warnings for unbalanced
regulator usage counts?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260819-mma8452-open-drain-v5-0-d8aa590d7c36@geanix.com?part=5

Re: [PATCH RESEND v5 4/6] iio: accel: mma8452: Support interrupt sharing

[Esben Haabendal]

<sashiko-bot@kernel.org> writes:

> Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
>
> New issues:
> - [High] Failed runtime suspend leaves the `suspended` flag set to `true`,
> causing an unrecoverable PM usage counter leak and a potential interrupt storm.
> - [Low] Compilation Error: undeclared variable `client` used in `mma8452_runtime_suspend`.
>
> Pre-existing issues:
> - [High] Unbalanced `regulator_disable()` calls and I2C writes to a powered-off device in `mma8452_remove()`.
> --
>
> --- Patch [4]: [PATCH RESEND v5 4/6] iio: accel: mma8452: Support interrupt sharing ---
> commit 79ad9089167d804daaa97086d058cd3a0e2f4199
> Author: Esben Haabendal <esben@geanix.com>
>
> iio: accel: mma8452: Support interrupt sharing
>
> This commit adds handling of runtime PM suspension in the interrupt handler
> to allow sharing interrupts with other devices. It introduces a suspended flag
> and uses synchronize_irq() to protect against race conditions during runtime
> suspend and device removal.
>
>> diff --git a/drivers/iio/accel/mma8452.c b/drivers/iio/accel/mma8452.c
>> index 1fb43c5b0b72f..25bb2dd69381e 100644
>> --- a/drivers/iio/accel/mma8452.c
>> +++ b/drivers/iio/accel/mma8452.c
> [ ... ]
>> @@ -1056,14 +1057,19 @@ 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 || READ_ONCE(data->suspended))
>> +		return IRQ_NONE; /* device is powered down or being removed */
>> +
>
> [Severity: High]
> If mma8452_runtime_suspend() in drivers/iio/accel/mma8452.c fails and
> leaves data->suspended set to true, can this early return cause a PM
> usage counter leak and an interrupt storm?
>
> If the device is kept in the RPM_ACTIVE state after a suspend failure,
> pm_runtime_get_if_active() will return 1. Because data->suspended is true,
> the handler will return IRQ_NONE immediately.
>
> This skips pm_runtime_put_autosuspend(), leaking the usage counter. Also,
> by returning IRQ_NONE on a shared level-triggered IRQ without clearing the
> hardware interrupt, wouldn't this trigger an interrupt storm?

Yes, I make sure to call pm_runtime_put_autosuspend() when
pm_runtime_get_active() returns 0 and suspended flag is set.

Also, I guess we should not set the suspended flag until after checking
the return value from mma8452_standby() in mma8452_runtime_suspend().

In case one of the regulator_disable() calls fails, the suspended flag
will be left set to true. But as the device is in standby, no interrupt
storm should happen.

> [ ... ]
>> @@ -1768,12 +1778,15 @@ static int mma8452_runtime_suspend(struct device *dev)
>>
>>  	mutex_lock(&data->lock);
>>  	ret = mma8452_standby(data);
>> +	WRITE_ONCE(data->suspended, true);
>>  	mutex_unlock(&data->lock);
>>  	if (ret < 0) {
>>  		dev_err(&data->client->dev, "powering off device failed\n");
>>  		return -EAGAIN;
>>  	}
>
> [Severity: High]
> If mma8452_standby() fails, should data->suspended be reverted to false?
>
> When returning -EAGAIN on failure in drivers/iio/accel/mma8452.c,
> leaving data->suspended set to true creates the inconsistent state that
> breaks mma8452_interrupt() as noted above.

I will move the WRITE_ONCE() to after checking the ret value, so we
don't leave suspended set to true in this case.

>> +	synchronize_irq(client->irq);
>> +
>
> [Severity: Low]
> Does this code compile?
>
> There is no local variable named client in mma8452_runtime_suspend() in
> drivers/iio/accel/mma8452.c. The I2C client is available via
> data->client or to_i2c_client(dev), so using client->irq directly
> appears to cause a compilation failure.
>
> [ ... ]
>
> [Severity: High]
> This is a pre-existing issue, but I noticed a potential problem during
> device removal in mma8452_remove() in drivers/iio/accel/mma8452.c:
>
> drivers/iio/accel/mma8452.c:mma8452_remove() {
>     ...
> 	pm_runtime_disable(&client->dev);
> 	pm_runtime_set_suspended(&client->dev);
>     ...
> 	mma8452_standby(iio_priv(indio_dev));
>
> 	regulator_disable(data->vddio_reg);
> 	regulator_disable(data->vdd_reg);
> }
>
> If the device is already in the RPM_SUSPENDED state when mma8452_remove() is
> called, its regulators have already been disabled by
> mma8452_runtime_suspend().
>
> Because pm_runtime_disable() does not resume the device, calling
> regulator_disable() again will underflow the regulator enable count. Could
> this forcefully disable shared regulators and cause system instability?
>
> Additionally, mma8452_standby() performs an I2C write. Is it safe to
> unconditionally access the hardware when it might be powered off?
>
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/20260819-mma8452-open-drain-v5-0-d8aa590d7c36@geanix.com?part=4

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

[Esben Haabendal]

<sashiko-bot@kernel.org> writes:

> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
>
> New issues:
> - [Low] Compilation Error in mma8452_runtime_suspend() due to undeclared identifier 'client'.
>
> Pre-existing issues:
> - [High] Missing pm_runtime_disable() in mma8452_probe() error paths leads to a guaranteed NULL pointer dereference in the autosuspend callback.
> --
>
> --- Patch [6]: [PATCH RESEND v5 6/6] iio: accel: mma8452: Reuse existing dev pointer in mma8452_probe() ---
> commit 004c22b83c7e1b63662958bd7b1a838d3015cf83
> Author: Esben Haabendal <esben@geanix.com>
>
> iio: accel: mma8452: Reuse existing dev pointer in mma8452_probe()
>
> 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.
>
>> diff --git a/drivers/iio/accel/mma8452.c b/drivers/iio/accel/mma8452.c
>> index 060a6207e52d0..bf5cb1c4425a3 100644
>> --- a/drivers/iio/accel/mma8452.c
>> +++ b/drivers/iio/accel/mma8452.c
>
> [ ... ]
>
>> @@ -1731,14 +1730,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)
>
> [Severity: High]
> This is a pre-existing issue, but if mma8452_probe() fails here, is there a
> missing pm_runtime_disable() in the error paths?
>
> If mma8452_probe() fails after enabling PM runtime (for example, during
> iio_device_register()), it returns an error without disabling PM runtime or
> autosuspend. The driver core cleans up the device and clears its driver data
> via dev_set_drvdata(dev, NULL), and queues an idle notification.
>
> Because PM runtime is still active, this triggers the autosuspend timer.
> When the timer expires, the autosuspend callback executes:
>
> drivers/iio/accel/mma8452.c:mma8452_runtime_suspend() {
>     struct iio_dev *indio_dev = i2c_get_clientdata(to_i2c_client(dev));
>     struct mma8452_data *data = iio_priv(indio_dev);
>     ...
> }
>
> Does this lead to a reliable NULL pointer dereference when iio_priv()
> dereferences the cleared driver data?

I would think so. I will add a fix to the series, calling both
pm_runtime_disable() and pm_runtime_set_suspended().

> [Severity: Low]
> Also, looking at mma8452_runtime_suspend(), does the module fail to compile
> when CONFIG_PM is enabled due to an undeclared identifier?
>
> The code attempts to call synchronize_irq():
>
> drivers/iio/accel/mma8452.c:mma8452_runtime_suspend() {
>     ...
>     synchronize_irq(client->irq);
>     ...
> }
>
> Since client is not declared in the scope of mma8452_runtime_suspend(),
> should this access data->client->irq instead?

Ok. That is strange. I was very sure I tested this exact patch version,
but I must have made a mistake. I will fix, and make sure I test it
properly (obviously with CONFIG_PM enabled to actually test it).

/Esben

Re: [PATCH RESEND v5 4/6] iio: accel: mma8452: Support interrupt sharing

[Andy Shevchenko]

On Wed, Aug 19, 2026 at 10:15:31AM +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.

...

> +	pm_status = pm_runtime_get_if_active(&data->client->dev);
> +	if (pm_status == 0 || READ_ONCE(data->suspended))
> +		return IRQ_NONE; /* device is powered down or being removed */

This way it won't work (if pm_status != 0, you need to act accordingly).
You need to split this:

	if (READ_ONCE(data->suspended))
		return IRQ_NONE; /* device is being removed */

	pm_status = pm_runtime_get_if_active(&data->client->dev);
	if (pm_status == 0)
		return IRQ_NONE; /* device is powered down */

But I'm not sure this will work as expected.

...

> +out:

out_runtime_put:

Hint: Always name labels as an answer to the Q:
'What will happen if I goto $LABEL?'.

> +	if (pm_status > 0)
> +		pm_runtime_put_autosuspend(&data->client->dev);

-- 
With Best Regards,
Andy Shevchenko

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

[Andy Shevchenko]

On Wed, Aug 19, 2026 at 10:15:33AM +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.

...

>  	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");

I would also move to -ENODATA. This can be done in a separate patch with might
be unified with the 'dev' use.

-- 
With Best Regards,
Andy Shevchenko