[PATCH v3 0/4] io: accel: mma8452: Allow open drain interrupt pin configuration

[PATCH v3 0/4] 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 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 (4):
      dt-bindings: iio: accel: mma8452: Add drive-open-drain
      iio: accel: mma8452: Optimize struct mm8452_data member orders
      iio: accel: mma8452: Only apply trigger type when not set by firmware
      iio: accel: mma8452: Allow open drain interrupt pin configuration

 .../devicetree/bindings/iio/accel/fsl,mma8452.yaml |  6 +++
 drivers/iio/accel/mma8452.c                        | 44 +++++++++++++++++++---
 2 files changed, 45 insertions(+), 5 deletions(-)
---
base-commit: 075b74841bd0065a3bda3440873c747938e69b68
change-id: 20250401-mma8452-open-drain-81577c41375c

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

[PATCH v3 1/4] 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 v3 2/4] iio: accel: mma8452: Optimize struct mm8452_data member orders

[Esben Haabendal]

Reorder struct mma8452_data members to avoid holes.

Reviewed-by: Joshua Crofts <joshua.crofts1@gmail.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 v3 3/4] 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.

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

diff --git a/drivers/iio/accel/mma8452.c b/drivers/iio/accel/mma8452.c
index f645a5c6fd1c..723ca3d9cb96 100644
--- a/drivers/iio/accel/mma8452.c
+++ b/drivers/iio/accel/mma8452.c
@@ -1683,9 +1683,14 @@ 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)
+			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 v3 4/4] iio: accel: mma8452: Allow open drain interrupt pin configuration

[Esben Haabendal]

When sharing interrupt line with other chips, the interrupt pin most
likely needs to be configured in open-drain mode instead of push-pull.

The PP_OD bit in CTRL_REG3 is set when open-drain mode is configured,
allowing sharing the interrupt line with other devices (with same interrupt
polarity).
The IRQF_SHARED flag is added to request_threaded_irq() flags, indicating
that the interrupt line can be shared with other devices.

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

diff --git a/drivers/iio/accel/mma8452.c b/drivers/iio/accel/mma8452.c
index 723ca3d9cb96..fa0915530f95 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
@@ -119,6 +121,7 @@ struct mma8452_data {
 	int sleep_val;
 	u8 ctrl_reg1;
 	u8 data_cfg;
+	bool open_drain;
 };
 
  /**
@@ -647,6 +650,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)
 {
@@ -1667,6 +1686,11 @@ static int mma8452_probe(struct i2c_client *client)
 			goto disable_regulators;
 	}
 
+	data->open_drain = device_property_read_bool(&client->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);
 
@@ -1688,7 +1712,7 @@ static int mma8452_probe(struct i2c_client *client)
 		irq_flags = irq_get_trigger_type(client->irq);
 		if (irq_flags == IRQ_TYPE_NONE)
 			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)
@@ -1806,6 +1830,10 @@ static int mma8452_runtime_resume(struct device *dev)
 		return ret;
 	}
 
+	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

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

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The driver applies firmware-specified IRQ trigger types without configuring the sensor's polarity to match, breaking active-high interrupt configurations.
--

--- Patch [3]: [PATCH v3 3/4] iio: accel: mma8452: Only apply trigger type when not set by firmware ---
commit 15bd5e2afb9b0b1a0a93c58982fed1331a0ecfe5
Author: Esben Haabendal <esben@geanix.com>

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

This commit updates the driver to respect the firmware-provided interrupt
trigger type, falling back to an active-low trigger if no type is
specified by the firmware.

> diff --git a/drivers/iio/accel/mma8452.c b/drivers/iio/accel/mma8452.c
> index f645a5c6fd1c2..723ca3d9cb96b 100644
> --- a/drivers/iio/accel/mma8452.c
> +++ b/drivers/iio/accel/mma8452.c
> @@ -1683,9 +1683,14 @@ 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)
> +			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);

[Severity: High]
Does this cause an interrupt mismatch if the firmware specifies an
active-high interrupt?

Looking at mma8452_probe, the driver requests the IRQ with the
firmware-specified polarity, but it doesn't seem to program the sensor
hardware to match. The MMA8452 sensor defaults to active-low interrupts
after a software reset.

Because the driver doesn't update the IPOL bit in MMA8452_CTRL_REG3 when an
active-high trigger is requested, the sensor will still drive an active-low
signal. This mismatch between the IRQ controller expectation and hardware
output could lead to missed interrupts or interrupt storms.

>  		if (ret)
>  			goto buffer_cleanup;
>  	}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260805-mma8452-open-drain-v3-0-6149f406a409@geanix.com?part=3

Re: [PATCH v3 4/4] 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] Shared IRQ handler unconditionally accesses I2C bus while the device may be runtime-suspended.
- [Medium] Incorrect shared interrupt handling falsely claims unrouted interrupts, masking spurious IRQs.

Pre-existing issues:
- [High] Volatile register context is not fully restored after PM runtime resume.
- [High] Unbalanced regulator disable and unpowered I2C access during device removal.
- [High] Sysfs callbacks modify cached state and access I2C without a PM runtime reference.
- [High] Reference count leak of the IIO trigger device due to unbalanced iio_trigger_get().
- [High] The driver ignores the requested IRQ polarity, leading to a hardware/software mismatch.
--

--- Patch [4]: [PATCH v3 4/4] iio: accel: mma8452: Allow open drain interrupt pin configuration ---
commit 6358f754e42af1003c7e9459771157fe3985f6bb
Author: Esben Haabendal <esben@geanix.com>

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

This patch allows configuring the MMA8452 interrupt pin in open-drain
mode. It reads the "drive-open-drain" device property and sets the PP_OD
bit in CTRL_REG3. It also adds the IRQF_SHARED flag to allow sharing the
interrupt line with other devices.

> diff --git a/drivers/iio/accel/mma8452.c b/drivers/iio/accel/mma8452.c
> index 723ca3d9cb96b..fa0915530f953 100644
> --- a/drivers/iio/accel/mma8452.c
> +++ b/drivers/iio/accel/mma8452.c

[ ... ]

> @@ -1688,7 +1712,7 @@ static int mma8452_probe(struct i2c_client *client)
>  		irq_flags = irq_get_trigger_type(client->irq);
>  		if (irq_flags == IRQ_TYPE_NONE)
>  			irq_flags = IRQF_TRIGGER_LOW;

[Severity: High]
This is a pre-existing issue, but the driver appears to ignore the requested
IRQ polarity.

While mma8452_probe() retrieves the requested IRQ trigger type to configure
the OS handler, does the driver ever set the hardware IPOL bit (bit 1) in
CTRL_REG3 to match it?

The chip defaults to active-low, which might prevent interrupts from being
delivered if the platform specifies an active-high interrupt.

> -		irq_flags |= IRQF_ONESHOT;
> +		irq_flags |= IRQF_ONESHOT | IRQF_SHARED;

[Severity: High]
Does adding IRQF_SHARED here make it possible for the interrupt handler to
run while the MMA8452 device is runtime-suspended?

When another device on the shared line triggers an interrupt,
mma8452_interrupt() is invoked and unconditionally reads MMA8452_INT_SRC
over I2C:

mma8452_interrupt()
    src = i2c_smbus_read_byte_data(data->client, MMA8452_INT_SRC);

If the device utilizes runtime PM and powers off its regulators when
suspended, would this unpowered I2C access cause transfer failures or lock
up the bus?

[Severity: Medium]
Additionally, is the interrupt handler equipped to properly reject spurious
interrupts on a shared line?

With IRQF_SHARED, mma8452_interrupt() will run for other devices'
interrupts. It unconditionally checks the DRDY bit:

mma8452_interrupt()
    if (src & MMA8452_INT_DRDY) {
        iio_trigger_poll_nested(indio_dev->trig);
        ret = IRQ_HANDLED;
    }

Since DRDY indicates data is ready even if the DRDY interrupt is not routed
to the pin in CTRL_REG4, could this falsely claim unrouted interrupts and
mask IRQ storms from a faulty shared line?

>  		ret = request_threaded_irq(client->irq, NULL, mma8452_interrupt,
>  					   irq_flags, client->name, indio_dev);
>  		if (ret)

[ ... ]

> @@ -1806,6 +1830,10 @@ static int mma8452_runtime_resume(struct device *dev)
>  		return ret;
>  	}
>  
> +	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 mma8452_runtime_resume() fully restore
the volatile register context after a PM runtime suspend cycle?

mma8452_runtime_suspend() disables regulators, cutting power to the chip.
When resuming, CTRL_REG3 and CTRL_REG1 are restored, but are other volatile
registers like DATA_CFG, TRANSIENT_THS, CTRL_REG4/5, and FF_MT_CFG left
unconfigured?

Could this cause the device to lose its sample rate, thresholds, and
interrupt routing after the first auto-suspend cycle?

While reviewing the PM runtime and cleanup paths, a few other pre-existing
issues were noticed:

[Severity: High]
This is a pre-existing issue, but does mma8452_remove() safely handle
removing a runtime-suspended device?

mma8452_remove()
    mma8452_standby(iio_priv(indio_dev));
    regulator_disable(data->vddio_reg);
    regulator_disable(data->vdd_reg);

This unconditionally writes to I2C and disables regulators without
guaranteeing the device is in an active PM state via pm_runtime_get_sync().
If the device is already runtime-suspended, the regulators were already
disabled. Could this double-disable regulators and cause unpowered I2C
access?

[Severity: High]
This is a pre-existing issue, but do sysfs callbacks like
__mma8452_write_raw() modify cached state and access I2C without a PM
runtime reference?

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

If the device is in auto-suspend (regulators off) when user-space writes to
sysfs attributes, would the I2C writes fail and corrupt the cached driver
state?

[Severity: High]
This is a pre-existing issue, but is there a reference count leak of the IIO
trigger device?

In mma8452_trigger_setup(), an extra reference is acquired:

    indio_dev->trig = iio_trigger_get(trig);

However, mma8452_trigger_cleanup() unregisters the trigger but does not
appear to drop the reference with iio_trigger_put(). Could this permanently
elevate the reference count and prevent the module from being unloaded?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260805-mma8452-open-drain-v3-0-6149f406a409@geanix.com?part=4

Re: [PATCH v3 4/4] iio: accel: mma8452: Allow open drain interrupt pin configuration

[Esben Haabendal]

<sashiko-bot@kernel.org> writes:

> Thank you for your contribution! Sashiko AI review found 7 potential issue(s) to consider:
>
> New issues:
> - [High] Shared IRQ handler unconditionally accesses I2C bus while the device may be runtime-suspended.
> - [Medium] Incorrect shared interrupt handling falsely claims unrouted interrupts, masking spurious IRQs.
>
> Pre-existing issues:
> - [High] Volatile register context is not fully restored after PM runtime resume.
> - [High] Unbalanced regulator disable and unpowered I2C access during device removal.
> - [High] Sysfs callbacks modify cached state and access I2C without a PM runtime reference.
> - [High] Reference count leak of the IIO trigger device due to unbalanced iio_trigger_get().
> - [High] The driver ignores the requested IRQ polarity, leading to a hardware/software mismatch.
> --
>
> --- Patch [4]: [PATCH v3 4/4] iio: accel: mma8452: Allow open drain interrupt pin configuration ---
> commit 6358f754e42af1003c7e9459771157fe3985f6bb
> Author: Esben Haabendal <esben@geanix.com>
>
> iio: accel: mma8452: Allow open drain interrupt pin configuration
>
> This patch allows configuring the MMA8452 interrupt pin in open-drain
> mode. It reads the "drive-open-drain" device property and sets the PP_OD
> bit in CTRL_REG3. It also adds the IRQF_SHARED flag to allow sharing the
> interrupt line with other devices.
>
>> diff --git a/drivers/iio/accel/mma8452.c b/drivers/iio/accel/mma8452.c
>> index 723ca3d9cb96b..fa0915530f953 100644
>> --- a/drivers/iio/accel/mma8452.c
>> +++ b/drivers/iio/accel/mma8452.c
>
> [ ... ]
>
>> @@ -1688,7 +1712,7 @@ static int mma8452_probe(struct i2c_client *client)
>>  		irq_flags = irq_get_trigger_type(client->irq);
>>  		if (irq_flags == IRQ_TYPE_NONE)
>>  			irq_flags = IRQF_TRIGGER_LOW;
>
> [Severity: High]
> This is a pre-existing issue, but the driver appears to ignore the requested
> IRQ polarity.
>
> While mma8452_probe() retrieves the requested IRQ trigger type to configure
> the OS handler, does the driver ever set the hardware IPOL bit (bit 1) in
> CTRL_REG3 to match it?
>
> The chip defaults to active-low, which might prevent interrupts from being
> delivered if the platform specifies an active-high interrupt.

This is per design. If IPOL bit is set based on the requested IRQ
polarity, support for system designs with additional hardware changing
the irq polarity (like a NOT gate) would not be possible.

If IPOL bit should be configurable, we probably need to add a dedicate
device-tree binding for it.

>> -		irq_flags |= IRQF_ONESHOT;
>> +		irq_flags |= IRQF_ONESHOT | IRQF_SHARED;
>
> [Severity: High]
> Does adding IRQF_SHARED here make it possible for the interrupt handler to
> run while the MMA8452 device is runtime-suspended?
>
> When another device on the shared line triggers an interrupt,
> mma8452_interrupt() is invoked and unconditionally reads MMA8452_INT_SRC
> over I2C:
>
> mma8452_interrupt()
>     src = i2c_smbus_read_byte_data(data->client, MMA8452_INT_SRC);
>
> If the device utilizes runtime PM and powers off its regulators when
> suspended, would this unpowered I2C access cause transfer failures or lock
> up the bus?

Oh. That does look like a problem.

I will add a fix for this for v4 of this series. And I will split the
shared interrupt support into a separate patch, as it is now supposed to
be usable without open-drain mode also.

> [Severity: Medium]
> Additionally, is the interrupt handler equipped to properly reject spurious
> interrupts on a shared line?
>
> With IRQF_SHARED, mma8452_interrupt() will run for other devices'
> interrupts. It unconditionally checks the DRDY bit:
>
> mma8452_interrupt()
>     if (src & MMA8452_INT_DRDY) {
>         iio_trigger_poll_nested(indio_dev->trig);
>         ret = IRQ_HANDLED;
>     }
>
> Since DRDY indicates data is ready even if the DRDY interrupt is not routed
> to the pin in CTRL_REG4, could this falsely claim unrouted interrupts and
> mask IRQ storms from a faulty shared line?

Maybe. I will send a patch with a fix for this when this series is merged.

>>  		ret = request_threaded_irq(client->irq, NULL, mma8452_interrupt,
>>  					   irq_flags, client->name, indio_dev);
>>  		if (ret)
>
> [ ... ]
>
>> @@ -1806,6 +1830,10 @@ static int mma8452_runtime_resume(struct device *dev)
>>  		return ret;
>>  	}
>>
>> +	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 mma8452_runtime_resume() fully restore
> the volatile register context after a PM runtime suspend cycle?
>
> mma8452_runtime_suspend() disables regulators, cutting power to the chip.
> When resuming, CTRL_REG3 and CTRL_REG1 are restored, but are other volatile
> registers like DATA_CFG, TRANSIENT_THS, CTRL_REG4/5, and FF_MT_CFG left
> unconfigured?
>
> Could this cause the device to lose its sample rate, thresholds, and
> interrupt routing after the first auto-suspend cycle?
>
> While reviewing the PM runtime and cleanup paths, a few other pre-existing
> issues were noticed:
>
> [Severity: High]
> This is a pre-existing issue, but does mma8452_remove() safely handle
> removing a runtime-suspended device?
>
> mma8452_remove()
>     mma8452_standby(iio_priv(indio_dev));
>     regulator_disable(data->vddio_reg);
>     regulator_disable(data->vdd_reg);
>
> This unconditionally writes to I2C and disables regulators without
> guaranteeing the device is in an active PM state via pm_runtime_get_sync().
> If the device is already runtime-suspended, the regulators were already
> disabled. Could this double-disable regulators and cause unpowered I2C
> access?
>
> [Severity: High]
> This is a pre-existing issue, but do sysfs callbacks like
> __mma8452_write_raw() modify cached state and access I2C without a PM
> runtime reference?
>
> __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);
>
> If the device is in auto-suspend (regulators off) when user-space writes to
> sysfs attributes, would the I2C writes fail and corrupt the cached driver
> state?


As I am actually not using runtime PM with this device, I prefer to not
investigate these 3 issues further for now.

> [Severity: High]
> This is a pre-existing issue, but is there a reference count leak of the IIO
> trigger device?
>
> In mma8452_trigger_setup(), an extra reference is acquired:
>
>     indio_dev->trig = iio_trigger_get(trig);
>
> However, mma8452_trigger_cleanup() unregisters the trigger but does not
> appear to drop the reference with iio_trigger_put(). Could this permanently
> elevate the reference count and prevent the module from being unloaded?

Maybe. But in that case, this is a problem for basically all iio
drivers, as there are no explicit calls to iio_trigger_put() in any iio
drivers as of now.

/Esben