[PATCH v8 0/9] io: accel: mma8452: Allow open drain interrupt pin configuration

[PATCH v8 0/9] 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 v8:
- Move patch 7 (with Fixes: ...) to go before the non-fixes patches.
- Rebased from v7.3-rc1 to current iio/togreg branch.
- Drop patch 8 (identical patch from Sanjay Chitroda already in togreg
  branch).
- Add new patch fixing a strangely extra indentation of a comment block.
- Add new patch adding documentation for struct mma8452_data.
- Move patch 4 to last in series, and rework the synchronization between
  suspend/resume and interrupt handler.
- Link to v7: https://patch.msgid.link/20260831-mma8452-open-drain-v7-0-22946812c928@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 (9):
      dt-bindings: iio: accel: mma8452: Add drive-open-drain
      iio: accel: mma8452: Fix use-after-free bug in error error path
      iio: accel: mma8452: Optimize struct mma8452_data member orders
      iio: accel: mma8452: Only apply trigger type when not set by firmware
      iio: accel: mma8452: Fix unintended comment indent
      iio: accel: mma8452: Add comment block for struct mma8452_data
      iio: accel: mma8452: Allow open drain interrupt pin configuration
      iio: accel: mma8452: Use proper error code when missing device model
      iio: accel: mma8452: Support interrupt sharing

 .../devicetree/bindings/iio/accel/fsl,mma8452.yaml |   6 +
 drivers/iio/accel/mma8452.c                        | 165 +++++++++++++++++----
 2 files changed, 141 insertions(+), 30 deletions(-)
---
base-commit: 309bd01041636b587bcb424b243640b448bd62f7
change-id: 20250401-mma8452-open-drain-81577c41375c

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

[PATCH v8 1/9] 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 v8 2/9] 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
Reviewed-by: Joshua Crofts <joshua.crofts1@gmail.com>
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 fe62a903f0e2..a937cbd84f30 100644
--- a/drivers/iio/accel/mma8452.c
+++ b/drivers/iio/accel/mma8452.c
@@ -1681,7 +1681,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)
@@ -1692,6 +1692,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

Re: [PATCH v8 2/9] iio: accel: mma8452: Fix use-after-free bug in error error path

[Jonathan Cameron]

On Mon, 07 Sep 2026 16:50:57 +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
> Reviewed-by: Joshua Crofts <joshua.crofts1@gmail.com>
> Signed-off-by: Esben Haabendal <esben@geanix.com>

Sashiko calls out some preexisting stuff that is worth a look
https://sashiko.dev/#/patchset/20260907-mma8452-open-drain-v8-0-c17407e22118%40geanix.com

Why freefall mode is set after the iio_device_register() is indeed an interesting
question. Any idea?

As far as it goes this patch is fine. I'm not sure about the other sashiko
comment about making sure the device is suspended. Given pm_runtime_set_active()
is called I would assume that one of the register sequences has indeed
turned on the device (maybe the reset?) and we should be turning it off again.

Jonathan

> ---
>  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 fe62a903f0e2..a937cbd84f30 100644
> --- a/drivers/iio/accel/mma8452.c
> +++ b/drivers/iio/accel/mma8452.c
> @@ -1681,7 +1681,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)
> @@ -1692,6 +1692,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);
> 

Re: [PATCH v8 2/9] iio: accel: mma8452: Fix use-after-free bug in error error path

[Esben Haabendal]

"Jonathan Cameron" <jic23@kernel.org> writes:

> On Mon, 07 Sep 2026 16:50:57 +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
>> Reviewed-by: Joshua Crofts <joshua.crofts1@gmail.com>
>> Signed-off-by: Esben Haabendal <esben@geanix.com>
>
> Sashiko calls out some preexisting stuff that is worth a look
> https://sashiko.dev/#/patchset/20260907-mma8452-open-drain-v8-0-c17407e22118%40geanix.com

Yes. And I have a follow-up patch series where I try to address
basically everything sashiko-bot has raised concerns for.

Given the rather large number of issues, and the corresponding large
number of changes needed, I am not planning on adding them to this
series.

> Why freefall mode is set after the iio_device_register() is indeed an interesting
> question. Any idea?

I cannot find any good reason for doing it like that. I am moving the
iio_device_register() call to be the last thing done in .probe() in the
follow-up series, so that the device is fully ready before we expose
user-space API for it.

> As far as it goes this patch is fine. I'm not sure about the other sashiko
> comment about making sure the device is suspended. Given pm_runtime_set_active()
> is called I would assume that one of the register sequences has indeed
> turned on the device (maybe the reset?) and we should be turning it off again.

There is quite a number of issues with runtime pm in this driver. I look
forward to getting feedback to the changes I have made to them :)

/Esben

[PATCH v8 3/9] 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 a937cbd84f30..6c5a8d65c8a4 100644
--- a/drivers/iio/accel/mma8452.c
+++ b/drivers/iio/accel/mma8452.c
@@ -108,10 +108,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_bulk_data regs[2];
 
 	/* Ensure correct alignment of time stamp when present */
@@ -119,6 +116,10 @@ struct mma8452_data {
 		__be16 channels[3];
 		aligned_s64 ts;
 	} buffer;
+
+	int sleep_val;
+	u8 ctrl_reg1;
+	u8 data_cfg;
 };
 
  /**

-- 
2.55.0

[PATCH v8 4/9] 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 6c5a8d65c8a4..05851274596e 100644
--- a/drivers/iio/accel/mma8452.c
+++ b/drivers/iio/accel/mma8452.c
@@ -1665,9 +1665,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 v8 5/9] iio: accel: mma8452: Fix unintended comment indent

[Esben Haabendal]

The extra space this block was indented with looks quite untraditional, and
does not align with the common style used in the kernel.

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

diff --git a/drivers/iio/accel/mma8452.c b/drivers/iio/accel/mma8452.c
index 05851274596e..c6af68d2a297 100644
--- a/drivers/iio/accel/mma8452.c
+++ b/drivers/iio/accel/mma8452.c
@@ -122,21 +122,21 @@ struct mma8452_data {
 	u8 data_cfg;
 };
 
- /**
-  * struct mma8452_event_regs - chip specific data related to events
-  * @ev_cfg:			event config register address
-  * @ev_cfg_ele:			latch bit in event config register
-  * @ev_cfg_chan_shift:		number of the bit to enable events in X
-  *				direction; in event config register
-  * @ev_src:			event source register address
-  * @ev_ths:			event threshold register address
-  * @ev_ths_mask:		mask for the threshold value
-  * @ev_count:			event count (period) register address
-  *
-  * Since not all chips supported by the driver support comparing high pass
-  * filtered data for events (interrupts), different interrupt sources are
-  * used for different chips and the relevant registers are included here.
-  */
+/**
+ * struct mma8452_event_regs - chip specific data related to events
+ * @ev_cfg:			event config register address
+ * @ev_cfg_ele:			latch bit in event config register
+ * @ev_cfg_chan_shift:		number of the bit to enable events in X
+ *				direction; in event config register
+ * @ev_src:			event source register address
+ * @ev_ths:			event threshold register address
+ * @ev_ths_mask:		mask for the threshold value
+ * @ev_count:			event count (period) register address
+ *
+ * Since not all chips supported by the driver support comparing high pass
+ * filtered data for events (interrupts), different interrupt sources are
+ * used for different chips and the relevant registers are included here.
+ */
 struct mma8452_event_regs {
 	u8 ev_cfg;
 	u8 ev_cfg_ele;

-- 
2.55.0

Re: [PATCH v8 5/9] iio: accel: mma8452: Fix unintended comment indent

[Joshua Crofts]

On Mon, 07 Sep 2026 16:51:00 +0200
Esben Haabendal <esben@geanix.com> wrote:

> The extra space this block was indented with looks quite untraditional, and
> does not align with the common style used in the kernel.
> 
> Signed-off-by: Esben Haabendal <esben@geanix.com>
> ---

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

-- 
Kind regards,
Joshua Crofts

[PATCH v8 6/9] iio: accel: mma8452: Add comment block for struct mma8452_data

[Esben Haabendal]

The struct mma8452_data is central for this driver, and it makes sense to
have a description of the fields in it to make it easier to work with the
driver.

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

diff --git a/drivers/iio/accel/mma8452.c b/drivers/iio/accel/mma8452.c
index c6af68d2a297..2c1b97b77bc1 100644
--- a/drivers/iio/accel/mma8452.c
+++ b/drivers/iio/accel/mma8452.c
@@ -104,6 +104,22 @@
 
 #define MMA8452_AUTO_SUSPEND_DELAY_MS		2000
 
+/**
+ * struct mma8452_data - IIO device private data structure
+ * @client:			the I2C client object
+ * @lock:			mutex for synchronziation of register
+ *				read-modify-write and holding chip in STANDBY
+ *				mode while writing to registers
+ * @orientation:		mounting matrix, flipped axis etc
+ * @chip_info:			chip specific data
+ * @vdd_reg:			reference to VDD regulator
+ * @vddio_reg:			reference to VDDIO regulator
+ * @buffer:			triggered buffer
+ * @sleep_val:			time in ms to sleep while waiting for drdy
+ * @ctrl_reg1:			CTRL_REG1 register shadow value
+ * @data_cfg:			DATA_CFG register shadow value
+ * @open_drain:			true for irq pin in open-drain mode
+ */
 struct mma8452_data {
 	struct i2c_client *client;
 	struct mutex lock;

-- 
2.55.0

Re: [PATCH v8 6/9] iio: accel: mma8452: Add comment block for struct mma8452_data

[Joshua Crofts]

On Mon, 07 Sep 2026 16:51:01 +0200
Esben Haabendal <esben@geanix.com> wrote:

> The struct mma8452_data is central for this driver, and it makes sense to
> have a description of the fields in it to make it easier to work with the
> driver.
> 
> Signed-off-by: Esben Haabendal <esben@geanix.com>
> ---
>  drivers/iio/accel/mma8452.c | 16 ++++++++++++++++
>  1 file changed, 16 insertions(+)
> 
> diff --git a/drivers/iio/accel/mma8452.c b/drivers/iio/accel/mma8452.c
> index c6af68d2a297..2c1b97b77bc1 100644
> --- a/drivers/iio/accel/mma8452.c
> +++ b/drivers/iio/accel/mma8452.c
> @@ -104,6 +104,22 @@
>  
>  #define MMA8452_AUTO_SUSPEND_DELAY_MS		2000
>  
> +/**
> + * struct mma8452_data - IIO device private data structure
> + * @client:			the I2C client object
> + * @lock:			mutex for synchronziation of register
> + *				read-modify-write and holding chip in STANDBY
> + *				mode while writing to registers

Perhaps add the word sequences after read-modify-write (that
could be a personal preference though)?

> + * @orientation:		mounting matrix, flipped axis etc

Missing period in etc., but that's a small enough nit :)

> + * @chip_info:			chip specific data
> + * @vdd_reg:			reference to VDD regulator
> + * @vddio_reg:			reference to VDDIO regulator
> + * @buffer:			triggered buffer
> + * @sleep_val:			time in ms to sleep while waiting for drdy
> + * @ctrl_reg1:			CTRL_REG1 register shadow value
> + * @data_cfg:			DATA_CFG register shadow value
> + * @open_drain:			true for irq pin in open-drain mode
> + */
>  struct mma8452_data {
>  	struct i2c_client *client;
>  	struct mutex lock;
> 

Either way, with those added in a new version or fixed up
by Jonathan,

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

-- 
Kind regards,
Joshua Crofts

Re: [PATCH v8 6/9] iio: accel: mma8452: Add comment block for struct mma8452_data

[Esben Haabendal]

"Joshua Crofts" <joshua.crofts1@gmail.com> writes:

> On Mon, 07 Sep 2026 16:51:01 +0200
> Esben Haabendal <esben@geanix.com> wrote:
>
>> The struct mma8452_data is central for this driver, and it makes sense to
>> have a description of the fields in it to make it easier to work with the
>> driver.
>>
>> Signed-off-by: Esben Haabendal <esben@geanix.com>
>> ---
>>  drivers/iio/accel/mma8452.c | 16 ++++++++++++++++
>>  1 file changed, 16 insertions(+)
>>
>> diff --git a/drivers/iio/accel/mma8452.c b/drivers/iio/accel/mma8452.c
>> index c6af68d2a297..2c1b97b77bc1 100644
>> --- a/drivers/iio/accel/mma8452.c
>> +++ b/drivers/iio/accel/mma8452.c
>> @@ -104,6 +104,22 @@
>>
>>  #define MMA8452_AUTO_SUSPEND_DELAY_MS		2000
>>
>> +/**
>> + * struct mma8452_data - IIO device private data structure
>> + * @client:			the I2C client object
>> + * @lock:			mutex for synchronziation of register
>> + *				read-modify-write and holding chip in STANDBY
>> + *				mode while writing to registers
>
> Perhaps add the word sequences after read-modify-write (that
> could be a personal preference though)?

Added for next version.

>> + * @orientation:		mounting matrix, flipped axis etc
>
> Missing period in etc., but that's a small enough nit :)

Also added :)

>> + * @chip_info:			chip specific data
>> + * @vdd_reg:			reference to VDD regulator
>> + * @vddio_reg:			reference to VDDIO regulator
>> + * @buffer:			triggered buffer
>> + * @sleep_val:			time in ms to sleep while waiting for drdy
>> + * @ctrl_reg1:			CTRL_REG1 register shadow value
>> + * @data_cfg:			DATA_CFG register shadow value
>> + * @open_drain:			true for irq pin in open-drain mode
>> + */
>>  struct mma8452_data {
>>  	struct i2c_client *client;
>>  	struct mutex lock;
>>
>
> Either way, with those added in a new version or fixed up
> by Jonathan,
>
> Reviewed-by: Joshua Crofts <joshua.crofts1@gmail.com>

Thanks.

/Esben

Re: [PATCH v8 6/9] iio: accel: mma8452: Add comment block for struct mma8452_data

[Andy Shevchenko]

On Mon, Sep 07, 2026 at 04:51:01PM +0200, Esben Haabendal wrote:
> The struct mma8452_data is central for this driver, and it makes sense to
> have a description of the fields in it to make it easier to work with the
> driver.

...

> +/**
> + * struct mma8452_data - IIO device private data structure
> + * @client:			the I2C client object
> + * @lock:			mutex for synchronziation of register
> + *				read-modify-write and holding chip in STANDBY
> + *				mode while writing to registers
> + * @orientation:		mounting matrix, flipped axis etc
> + * @chip_info:			chip specific data
> + * @vdd_reg:			reference to VDD regulator
> + * @vddio_reg:			reference to VDDIO regulator
> + * @buffer:			triggered buffer
> + * @sleep_val:			time in ms to sleep while waiting for drdy
> + * @ctrl_reg1:			CTRL_REG1 register shadow value
> + * @data_cfg:			DATA_CFG register shadow value
> + * @open_drain:			true for irq pin in open-drain mode
> + */

Have you tried to generate *.html and *.pdf out of this? Does rendering look
okay in both cases?

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v8 6/9] iio: accel: mma8452: Add comment block for struct mma8452_data

[Jonathan Cameron]

On Mon, 07 Sep 2026 16:51:01 +0200
Esben Haabendal <esben@geanix.com> wrote:

> The struct mma8452_data is central for this driver, and it makes sense to
> have a description of the fields in it to make it easier to work with the
> driver.
> 
> Signed-off-by: Esben Haabendal <esben@geanix.com>
> ---
>  drivers/iio/accel/mma8452.c | 16 ++++++++++++++++
>  1 file changed, 16 insertions(+)
> 
> diff --git a/drivers/iio/accel/mma8452.c b/drivers/iio/accel/mma8452.c
> index c6af68d2a297..2c1b97b77bc1 100644
> --- a/drivers/iio/accel/mma8452.c
> +++ b/drivers/iio/accel/mma8452.c
> @@ -104,6 +104,22 @@
>  
>  #define MMA8452_AUTO_SUSPEND_DELAY_MS		2000
>  
> +/**
> + * struct mma8452_data - IIO device private data structure
> + * @client:			the I2C client object
> + * @lock:			mutex for synchronziation of register
> + *				read-modify-write and holding chip in STANDBY
> + *				mode while writing to registers
> + * @orientation:		mounting matrix, flipped axis etc
> + * @chip_info:			chip specific data
> + * @vdd_reg:			reference to VDD regulator
> + * @vddio_reg:			reference to VDDIO regulator

These don't exist any more.  Sanjay did a conversion to bulk regulators
that I picked up a few weeks back. Please rebase for v9 on top
of the togreg branch of iio.git.  Seems that sashiko figured out enough
to base this on a more recent tree than you have.


> + * @buffer:			triggered buffer
> + * @sleep_val:			time in ms to sleep while waiting for drdy
> + * @ctrl_reg1:			CTRL_REG1 register shadow value
> + * @data_cfg:			DATA_CFG register shadow value
> + * @open_drain:			true for irq pin in open-drain mode
> + */
>  struct mma8452_data {
>  	struct i2c_client *client;
>  	struct mutex lock;
> 

Re: [PATCH v8 6/9] iio: accel: mma8452: Add comment block for struct mma8452_data

[Esben Haabendal]

"Jonathan Cameron" <jic23@kernel.org> writes:

> On Mon, 07 Sep 2026 16:51:01 +0200
> Esben Haabendal <esben@geanix.com> wrote:
>
>> The struct mma8452_data is central for this driver, and it makes sense to
>> have a description of the fields in it to make it easier to work with the
>> driver.
>>
>> Signed-off-by: Esben Haabendal <esben@geanix.com>
>> ---
>>  drivers/iio/accel/mma8452.c | 16 ++++++++++++++++
>>  1 file changed, 16 insertions(+)
>>
>> diff --git a/drivers/iio/accel/mma8452.c b/drivers/iio/accel/mma8452.c
>> index c6af68d2a297..2c1b97b77bc1 100644
>> --- a/drivers/iio/accel/mma8452.c
>> +++ b/drivers/iio/accel/mma8452.c
>> @@ -104,6 +104,22 @@
>>
>>  #define MMA8452_AUTO_SUSPEND_DELAY_MS		2000
>>
>> +/**
>> + * struct mma8452_data - IIO device private data structure
>> + * @client:			the I2C client object
>> + * @lock:			mutex for synchronziation of register
>> + *				read-modify-write and holding chip in STANDBY
>> + *				mode while writing to registers
>> + * @orientation:		mounting matrix, flipped axis etc
>> + * @chip_info:			chip specific data
>> + * @vdd_reg:			reference to VDD regulator
>> + * @vddio_reg:			reference to VDDIO regulator
>
> These don't exist any more.  Sanjay did a conversion to bulk regulators
> that I picked up a few weeks back. Please rebase for v9 on top
> of the togreg branch of iio.git.

Already done and queued up to be sent :)

> Seems that sashiko figured out enough to base this on a more recent
> tree than you have.

/Esben

[PATCH v8 7/9] 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 2c1b97b77bc1..b5ab85a86e72 100644
--- a/drivers/iio/accel/mma8452.c
+++ b/drivers/iio/accel/mma8452.c
@@ -83,6 +83,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
@@ -136,6 +138,7 @@ struct mma8452_data {
 	int sleep_val;
 	u8 ctrl_reg1;
 	u8 data_cfg;
+	bool open_drain;
 };
 
 /**
@@ -659,6 +662,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)
 {
@@ -1665,6 +1684,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);
 
@@ -1792,6 +1816,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

[PATCH v8 8/9] 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>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.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 b5ab85a86e72..fda29df5d109 100644
--- a/drivers/iio/accel/mma8452.c
+++ b/drivers/iio/accel/mma8452.c
@@ -1591,7 +1591,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

[PATCH v8 9/9] iio: accel: mma8452: Support interrupt sharing

[Esben Haabendal]

Adding support for sharing interrupt line with other device requires the
interrupt handler to handle runtime PM suspension properly, ignoring the
irq if the device is suspended (maybe even off). And while at it, we use
the PM reference to ensure we do not get suspended while processing an irq.

In order to prevent the chip from raising irq while suspended (that is when
using fixed regulator, where suspend just means setting the device in
STANDBY mode), we disable all interrupt sources by clearing CTRL_REG4, and
then restores the value again when resuming.

With that in place, it is safe to add the IRQF_SHARED flag.

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.

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

diff --git a/drivers/iio/accel/mma8452.c b/drivers/iio/accel/mma8452.c
index fda29df5d109..e521dca37f76 100644
--- a/drivers/iio/accel/mma8452.c
+++ b/drivers/iio/accel/mma8452.c
@@ -120,6 +120,7 @@
  * @sleep_val:			time in ms to sleep while waiting for drdy
  * @ctrl_reg1:			CTRL_REG1 register shadow value
  * @data_cfg:			DATA_CFG register shadow value
+ * @ctrl_reg4:			CTRL_REG4 register value to restore on resume
  * @open_drain:			true for irq pin in open-drain mode
  */
 struct mma8452_data {
@@ -138,6 +139,7 @@ struct mma8452_data {
 	int sleep_val;
 	u8 ctrl_reg1;
 	u8 data_cfg;
+	u8 ctrl_reg4;
 	bool open_drain;
 };
 
@@ -1083,15 +1085,21 @@ static irqreturn_t mma8452_interrupt(int irq, void *p)
 {
 	struct iio_dev *indio_dev = p;
 	struct mma8452_data *data = iio_priv(indio_dev);
+	struct device *dev = &data->client->dev;
 	irqreturn_t ret = IRQ_NONE;
+	int pm_status;
 	int src;
 
+	pm_status = pm_runtime_get_if_active(dev);
+	if (pm_status == 0)
+		return IRQ_NONE; /* device is powered down */
+
 	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);
@@ -1117,6 +1125,10 @@ static irqreturn_t mma8452_interrupt(int irq, void *p)
 		ret = IRQ_HANDLED;
 	}
 
+out_runtime_put:
+	if (pm_status > 0)
+		pm_runtime_put_autosuspend(dev);
+
 	return ret;
 }
 
@@ -1712,7 +1724,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)
@@ -1784,29 +1796,62 @@ 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;
 
-	scoped_guard(mutex, &data->lock)
-		ret = mma8452_standby(data);
+	guard(mutex)(&data->lock);
+
+	ret = i2c_smbus_read_byte_data(client, MMA8452_CTRL_REG4);
 	if (ret < 0) {
-		dev_err(dev, "powering off device failed\n");
+		dev_warn(dev, "backing up CTRL_REG4 failed\n");
 		return -EAGAIN;
+	} else
+		data->ctrl_reg4 = ret;
+
+	ret = i2c_smbus_write_byte_data(client, MMA8452_CTRL_REG4, 0);
+	if (ret) {
+		dev_warn(dev, "disabling interrupt sources (CTRL_REG4) failed\n");
+		return -EAGAIN;
+	}
+
+	ret = mma8452_standby(data);
+	if (ret < 0) {
+		dev_err(dev, "transition to STANDBY mode failed\n");
+		ret = -EAGAIN;
+		goto out_restore_ctrl_reg4;
 	}
 
+	/*
+	 * Interrupt line should be deasserted now, so we just need ensure any
+	 * mid-flight irq is completed (will return IRQ_NONE due to
+	 * pm_status==0).
+	 */
+	if (client->irq)
+		synchronize_irq(client->irq);
+
 	ret = regulator_bulk_disable(ARRAY_SIZE(data->regs), data->regs);
 	if (ret) {
 		dev_err(dev, "failed to disable regulators\n");
-		return ret;
+		goto out_active;
 	}
 
 	return 0;
+
+out_active:
+	if (mma8452_active(data))
+		dev_warn(dev, "failed to switch back to ACTIVE mode\n");
+out_restore_ctrl_reg4:
+	if (i2c_smbus_write_byte_data(client, MMA8452_CTRL_REG4, data->ctrl_reg4))
+		dev_warn(dev, "restoring CTRL_REG4 failed\n");
+	return ret;
 }
 
 static int mma8452_runtime_resume(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, sleep_val;
 
@@ -1820,6 +1865,10 @@ static int mma8452_runtime_resume(struct device *dev)
 	if (ret)
 		goto runtime_resume_failed;
 
+	ret = i2c_smbus_write_byte_data(client, MMA8452_CTRL_REG4, data->ctrl_reg4);
+	if (ret)
+		goto runtime_resume_failed;
+
 	ret = mma8452_active(data);
 	if (ret < 0)
 		goto runtime_resume_failed;

-- 
2.55.0

Re: [PATCH v8 9/9] iio: accel: mma8452: Support interrupt sharing

[Joshua Crofts]

On Mon, 07 Sep 2026 16:51:04 +0200
Esben Haabendal <esben@geanix.com> wrote:

> Adding support for sharing interrupt line with other device requires the
> interrupt handler to handle runtime PM suspension properly, ignoring the
> irq if the device is suspended (maybe even off). And while at it, we use
> the PM reference to ensure we do not get suspended while processing an irq.
> 
> In order to prevent the chip from raising irq while suspended (that is when
> using fixed regulator, where suspend just means setting the device in
> STANDBY mode), we disable all interrupt sources by clearing CTRL_REG4, and
> then restores the value again when resuming.
> 
> With that in place, it is safe to add the IRQF_SHARED flag.
> 
> 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.
> 
> Signed-off-by: Esben Haabendal <esben@geanix.com>
> ---

...

> @@ -1784,29 +1796,62 @@ 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;
>  
> -	scoped_guard(mutex, &data->lock)
> -		ret = mma8452_standby(data);
> +	guard(mutex)(&data->lock);
> +

Hmm, this patch might benefit from breaking it into multiple patches
(the change from scoped_guard to guard, the local pointers etc. along
the fact that these aren't mentioned in the commit message), but
that's up to Jonathan.


-- 
Kind regards,
Joshua Crofts

Re: [PATCH v8 9/9] iio: accel: mma8452: Support interrupt sharing

[Esben Haabendal]

"Joshua Crofts" <joshua.crofts1@gmail.com> writes:

> On Mon, 07 Sep 2026 16:51:04 +0200
> Esben Haabendal <esben@geanix.com> wrote:
>
>> Adding support for sharing interrupt line with other device requires the
>> interrupt handler to handle runtime PM suspension properly, ignoring the
>> irq if the device is suspended (maybe even off). And while at it, we use
>> the PM reference to ensure we do not get suspended while processing an irq.
>>
>> In order to prevent the chip from raising irq while suspended (that is when
>> using fixed regulator, where suspend just means setting the device in
>> STANDBY mode), we disable all interrupt sources by clearing CTRL_REG4, and
>> then restores the value again when resuming.
>>
>> With that in place, it is safe to add the IRQF_SHARED flag.
>>
>> 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.
>>
>> Signed-off-by: Esben Haabendal <esben@geanix.com>
>> ---
>
> ...
>
>> @@ -1784,29 +1796,62 @@ 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;
>>
>> -	scoped_guard(mutex, &data->lock)
>> -		ret = mma8452_standby(data);
>> +	guard(mutex)(&data->lock);
>> +
>
> Hmm, this patch might benefit from breaking it into multiple patches
> (the change from scoped_guard to guard, the local pointers etc. along
> the fact that these aren't mentioned in the commit message), but
> that's up to Jonathan.

I will try and improve the commit message. I don't think that breaking
it up makes a lot of sense.

The change from guard() to scoped_guard() is closely tied to the rest of
the changes. At least I am not sure if it is even sane to make that
change without the rest of this commit. It might be fine. Probably is,
but is it worth to risk causing trouble for older kernels?

Regarding the local pointers, I assume you mean the added struct
i2c_client and struct device pointers. They are only used in the added
code, so I don't see how I can split that out in a meaningful way.

I don't mind making such changes, I just don't see what I am expected to
do.

/Esben

Re: [PATCH v8 9/9] iio: accel: mma8452: Support interrupt sharing

[Joshua Crofts]

On Mon, 07 Sep 2026 18:36:56 +0200
Esben Haabendal <esben@geanix.com> wrote:

> "Joshua Crofts" <joshua.crofts1@gmail.com> writes:
> 
> > On Mon, 07 Sep 2026 16:51:04 +0200
> > Esben Haabendal <esben@geanix.com> wrote:
> >  
> >> Adding support for sharing interrupt line with other device requires the
> >> interrupt handler to handle runtime PM suspension properly, ignoring the
> >> irq if the device is suspended (maybe even off). And while at it, we use
> >> the PM reference to ensure we do not get suspended while processing an irq.
> >>
> >> In order to prevent the chip from raising irq while suspended (that is when
> >> using fixed regulator, where suspend just means setting the device in
> >> STANDBY mode), we disable all interrupt sources by clearing CTRL_REG4, and
> >> then restores the value again when resuming.
> >>
> >> With that in place, it is safe to add the IRQF_SHARED flag.
> >>
> >> 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.
> >>
> >> Signed-off-by: Esben Haabendal <esben@geanix.com>
> >> ---  
> >
> > ...
> >  
> >> @@ -1784,29 +1796,62 @@ 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;
> >>
> >> -	scoped_guard(mutex, &data->lock)
> >> -		ret = mma8452_standby(data);
> >> +	guard(mutex)(&data->lock);
> >> +  
> >
> > Hmm, this patch might benefit from breaking it into multiple patches
> > (the change from scoped_guard to guard, the local pointers etc. along
> > the fact that these aren't mentioned in the commit message), but
> > that's up to Jonathan.  
> 
> I will try and improve the commit message. I don't think that breaking
> it up makes a lot of sense.
> 
> The change from guard() to scoped_guard() is closely tied to the rest of
> the changes. At least I am not sure if it is even sane to make that
> change without the rest of this commit. It might be fine. Probably is,
> but is it worth to risk causing trouble for older kernels?
> 
> Regarding the local pointers, I assume you mean the added struct
> i2c_client and struct device pointers. They are only used in the added
> code, so I don't see how I can split that out in a meaningful way.
> 

Ah fair enough, no need to split it then :)

-- 
Kind regards,
Joshua Crofts

Re: [PATCH v8 9/9] iio: accel: mma8452: Support interrupt sharing

[Jonathan Cameron]

On Mon, 07 Sep 2026 16:51:04 +0200
Esben Haabendal <esben@geanix.com> wrote:

> Adding support for sharing interrupt line with other device requires the
> interrupt handler to handle runtime PM suspension properly, ignoring the
> irq if the device is suspended (maybe even off). And while at it, we use
> the PM reference to ensure we do not get suspended while processing an irq.
> 
> In order to prevent the chip from raising irq while suspended (that is when
> using fixed regulator, where suspend just means setting the device in
> STANDBY mode), we disable all interrupt sources by clearing CTRL_REG4, and
> then restores the value again when resuming.
> 
> With that in place, it is safe to add the IRQF_SHARED flag.
> 
> 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.
> 
> Signed-off-by: Esben Haabendal <esben@geanix.com>
> ---
>  drivers/iio/accel/mma8452.c | 67 +++++++++++++++++++++++++++++++++++++++------
>  1 file changed, 58 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/iio/accel/mma8452.c b/drivers/iio/accel/mma8452.c
> index fda29df5d109..e521dca37f76 100644
> --- a/drivers/iio/accel/mma8452.c
> +++ b/drivers/iio/accel/mma8452.c
> @@ -120,6 +120,7 @@
>   * @sleep_val:			time in ms to sleep while waiting for drdy
>   * @ctrl_reg1:			CTRL_REG1 register shadow value
>   * @data_cfg:			DATA_CFG register shadow value
> + * @ctrl_reg4:			CTRL_REG4 register value to restore on resume
>   * @open_drain:			true for irq pin in open-drain mode
>   */
>  struct mma8452_data {
> @@ -138,6 +139,7 @@ struct mma8452_data {
>  	int sleep_val;
>  	u8 ctrl_reg1;
>  	u8 data_cfg;
> +	u8 ctrl_reg4;
>  	bool open_drain;
>  };
>  
> @@ -1083,15 +1085,21 @@ static irqreturn_t mma8452_interrupt(int irq, void *p)
>  {
>  	struct iio_dev *indio_dev = p;
>  	struct mma8452_data *data = iio_priv(indio_dev);
> +	struct device *dev = &data->client->dev;
>  	irqreturn_t ret = IRQ_NONE;
> +	int pm_status;
>  	int src;
>  
> +	pm_status = pm_runtime_get_if_active(dev);

Sashiko raises the question of what happens if you actually get an error
return from this. You may be deliberately ignoring those, but if
so add a comment.  The fun race around tear down is worth considering
in particular (see sashiko comment).


> +	if (pm_status == 0)
> +		return IRQ_NONE; /* device is powered down */

> @@ -1784,29 +1796,62 @@ 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;
>  
> -	scoped_guard(mutex, &data->lock)
> -		ret = mma8452_standby(data);
> +	guard(mutex)(&data->lock);
Mixing guards... 
> +
> +	ret = i2c_smbus_read_byte_data(client, MMA8452_CTRL_REG4);
>  	if (ret < 0) {
> -		dev_err(dev, "powering off device failed\n");
> +		dev_warn(dev, "backing up CTRL_REG4 failed\n");
>  		return -EAGAIN;
> +	} else
> +		data->ctrl_reg4 = ret;
> +
> +	ret = i2c_smbus_write_byte_data(client, MMA8452_CTRL_REG4, 0);
> +	if (ret) {
> +		dev_warn(dev, "disabling interrupt sources (CTRL_REG4) failed\n");
> +		return -EAGAIN;
> +	}
> +
> +	ret = mma8452_standby(data);
> +	if (ret < 0) {
> +		dev_err(dev, "transition to STANDBY mode failed\n");
> +		ret = -EAGAIN;
> +		goto out_restore_ctrl_reg4;

and goto is explicitly advised against in the docs in cleanup.h
You need to restructure the code to avoid that, potentially via
a helper function.

>  	}
>  
> +	/*
> +	 * Interrupt line should be deasserted now, so we just need ensure any
> +	 * mid-flight irq is completed (will return IRQ_NONE due to
> +	 * pm_status==0).
> +	 */
> +	if (client->irq)
> +		synchronize_irq(client->irq);
> +
>  	ret = regulator_bulk_disable(ARRAY_SIZE(data->regs), data->regs);
>  	if (ret) {
>  		dev_err(dev, "failed to disable regulators\n");
> -		return ret;
> +		goto out_active;
>  	}
>  
>  	return 0;
> +
> +out_active:
> +	if (mma8452_active(data))
> +		dev_warn(dev, "failed to switch back to ACTIVE mode\n");
> +out_restore_ctrl_reg4:
> +	if (i2c_smbus_write_byte_data(client, MMA8452_CTRL_REG4, data->ctrl_reg4))
> +		dev_warn(dev, "restoring CTRL_REG4 failed\n");
> +	return ret;
>  }
>  
>  static int mma8452_runtime_resume(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, sleep_val;
>  
> @@ -1820,6 +1865,10 @@ static int mma8452_runtime_resume(struct device *dev)
>  	if (ret)
>  		goto runtime_resume_failed;
>  
> +	ret = i2c_smbus_write_byte_data(client, MMA8452_CTRL_REG4, data->ctrl_reg4);
> +	if (ret)
> +		goto runtime_resume_failed;
> +
>  	ret = mma8452_active(data);
>  	if (ret < 0)
>  		goto runtime_resume_failed;
> 

Re: [PATCH v8 9/9] iio: accel: mma8452: Support interrupt sharing

[Esben Haabendal]

"Jonathan Cameron" <jic23@kernel.org> writes:

> On Mon, 07 Sep 2026 16:51:04 +0200
> Esben Haabendal <esben@geanix.com> wrote:
>
>> Adding support for sharing interrupt line with other device requires the
>> interrupt handler to handle runtime PM suspension properly, ignoring the
>> irq if the device is suspended (maybe even off). And while at it, we use
>> the PM reference to ensure we do not get suspended while processing an irq.
>>
>> In order to prevent the chip from raising irq while suspended (that is when
>> using fixed regulator, where suspend just means setting the device in
>> STANDBY mode), we disable all interrupt sources by clearing CTRL_REG4, and
>> then restores the value again when resuming.
>>
>> With that in place, it is safe to add the IRQF_SHARED flag.
>>
>> 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.
>>
>> Signed-off-by: Esben Haabendal <esben@geanix.com>
>> ---
>>  drivers/iio/accel/mma8452.c | 67 +++++++++++++++++++++++++++++++++++++++------
>>  1 file changed, 58 insertions(+), 9 deletions(-)
>>
>> diff --git a/drivers/iio/accel/mma8452.c b/drivers/iio/accel/mma8452.c
>> index fda29df5d109..e521dca37f76 100644
>> --- a/drivers/iio/accel/mma8452.c
>> +++ b/drivers/iio/accel/mma8452.c
>> @@ -120,6 +120,7 @@
>>   * @sleep_val:			time in ms to sleep while waiting for drdy
>>   * @ctrl_reg1:			CTRL_REG1 register shadow value
>>   * @data_cfg:			DATA_CFG register shadow value
>> + * @ctrl_reg4:			CTRL_REG4 register value to restore on resume
>>   * @open_drain:			true for irq pin in open-drain mode
>>   */
>>  struct mma8452_data {
>> @@ -138,6 +139,7 @@ struct mma8452_data {
>>  	int sleep_val;
>>  	u8 ctrl_reg1;
>>  	u8 data_cfg;
>> +	u8 ctrl_reg4;
>>  	bool open_drain;
>>  };
>>
>> @@ -1083,15 +1085,21 @@ static irqreturn_t mma8452_interrupt(int irq, void *p)
>>  {
>>  	struct iio_dev *indio_dev = p;
>>  	struct mma8452_data *data = iio_priv(indio_dev);
>> +	struct device *dev = &data->client->dev;
>>  	irqreturn_t ret = IRQ_NONE;
>> +	int pm_status;
>>  	int src;
>>
>> +	pm_status = pm_runtime_get_if_active(dev);
>
> Sashiko raises the question of what happens if you actually get an error
> return from this. You may be deliberately ignoring those, but if
> so add a comment.

Yes, sounds like a good idea.

> The fun race around tear down is worth considering in particular (see
> sashiko comment).

I will look into that. Although very unlikely, it does sound like a real
issue that *could* occur.

Adding a boolean flag (remove_in_progress = true) to the mma8452_data
struct, and set that in mma8452_remove() before disabling runtime pm
seems like a KISS solution. Should we be returning IRQ_HANDLED or
IRQ_NONE in that case?  There is no IRQ_MAYBE return value :D

>> +	if (pm_status == 0)
>> +		return IRQ_NONE; /* device is powered down */
>
>> @@ -1784,29 +1796,62 @@ 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;
>>
>> -	scoped_guard(mutex, &data->lock)
>> -		ret = mma8452_standby(data);
>> +	guard(mutex)(&data->lock);
> Mixing guards...

Yes, I know. Resolving that turned out to be a bit more painful than I
thought. I do have that resolved in the next series I keep on talking
about. But distilling that as a separate patch for adding to this series
turned out to be impossible, as it relies on some of the other work,
like refactoring of the ACTIVE/STANDBY state handling (synchronization),
and most importantly, the switch to using regmap caching to properly
handle restoring of register values on resume.

I am therefore hoping that we can find some way to agree on things here,
and then properly resolve it in the next series...

For what it is worth, in this very specific case, the only guard used is
first in the LIFO order, and will therefore be executed in exactly the
right order on exit, as it will run after all the goto cleanup, just as
the LIFO cleanup order requires.

Obviously a fragile and undesirable way of doing error handling, but it
will go away again in the next series where I will eliminate the goto
error handling in mma8452_runtime_suspend().

>> +
>> +	ret = i2c_smbus_read_byte_data(client, MMA8452_CTRL_REG4);
>>  	if (ret < 0) {
>> -		dev_err(dev, "powering off device failed\n");
>> +		dev_warn(dev, "backing up CTRL_REG4 failed\n");
>>  		return -EAGAIN;
>> +	} else
>> +		data->ctrl_reg4 = ret;
>> +
>> +	ret = i2c_smbus_write_byte_data(client, MMA8452_CTRL_REG4, 0);
>> +	if (ret) {
>> +		dev_warn(dev, "disabling interrupt sources (CTRL_REG4) failed\n");
>> +		return -EAGAIN;
>> +	}
>> +
>> +	ret = mma8452_standby(data);
>> +	if (ret < 0) {
>> +		dev_err(dev, "transition to STANDBY mode failed\n");
>> +		ret = -EAGAIN;
>> +		goto out_restore_ctrl_reg4;
>
> and goto is explicitly advised against in the docs in cleanup.h
> You need to restructure the code to avoid that, potentially via
> a helper function.

I agree. See above.

>>  	}
>>
>> +	/*
>> +	 * Interrupt line should be deasserted now, so we just need ensure any
>> +	 * mid-flight irq is completed (will return IRQ_NONE due to
>> +	 * pm_status==0).
>> +	 */
>> +	if (client->irq)
>> +		synchronize_irq(client->irq);
>> +
>>  	ret = regulator_bulk_disable(ARRAY_SIZE(data->regs), data->regs);
>>  	if (ret) {
>>  		dev_err(dev, "failed to disable regulators\n");
>> -		return ret;
>> +		goto out_active;
>>  	}
>>
>>  	return 0;
>> +
>> +out_active:
>> +	if (mma8452_active(data))
>> +		dev_warn(dev, "failed to switch back to ACTIVE mode\n");
>> +out_restore_ctrl_reg4:
>> +	if (i2c_smbus_write_byte_data(client, MMA8452_CTRL_REG4, data->ctrl_reg4))
>> +		dev_warn(dev, "restoring CTRL_REG4 failed\n");
>> +	return ret;
>>  }
>>
>>  static int mma8452_runtime_resume(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, sleep_val;
>>
>> @@ -1820,6 +1865,10 @@ static int mma8452_runtime_resume(struct device *dev)
>>  	if (ret)
>>  		goto runtime_resume_failed;
>>
>> +	ret = i2c_smbus_write_byte_data(client, MMA8452_CTRL_REG4, data->ctrl_reg4);
>> +	if (ret)
>> +		goto runtime_resume_failed;
>> +
>>  	ret = mma8452_active(data);
>>  	if (ret < 0)
>>  		goto runtime_resume_failed;
>>