[PATCH v4 0/5] iio: accel: mma8452: improve coding style, pm and resource cleanup

[PATCH v4 0/5] iio: accel: mma8452: improve coding style, pm and resource cleanup

[Sanjay Chitroda]

Hi all,

This series contains a small fixes, cleanup and improvements to use
modern kernel helper API and coding style for mma8452 accel driver.

The changes modernize mutex with guard(), dev_err_probe usage,
resolve checkpatch CHECKS and pm_ptr macro usage.

Changes in v3:
- Following input from Andy and Jonathan added new changes as following
  0001: handle return value to have proper error propagation
  0002: use non-devm API to maintain resource management LIFO order
  0006: convert individual regulator using bulk regulator API
  0009: use IIO cleanup helper for DIRECT_MODE
- Address kernel coding stype specific review comment
- Reorder local struct device and dev_err_probe change
v2 series -> https://lore.kernel.org/all/20260422165643.2148195-1-sanjayembedded@gmail.com/
Changes in v2:
- 0005: address review comment from Andy and Geert
  and use DEFINE_RUNTIME_DEV_PM_OPS macro
- Added new cleanup channges in mma8452 driver

No functional behavior changes are intended.

Testing:
  - Compiled with W=1
  - Build-tested on QEMU x86_64

Feedback and reviews are very welcome.

---
Sanjay Chitroda (5):
      iio: accel: mma8452: convert to bulk regulator usage
      iio: accel: mma8452: use local struct device
      iio: accel: mma8452: use pm_ptr() and direct runtime PM calls
      iio: accel: mma8452: Use IIO cleanup helpers
      iio: accel: mma8452: use guard() to release mutexes

 drivers/iio/accel/mma8452.c | 247 +++++++++++++++++++-------------------------
 1 file changed, 109 insertions(+), 138 deletions(-)
---
base-commit: 7b84b1e9dd850a5c9b55e27daa4ecdc2dd5b3431
change-id: 20260601-15-apr-pm-iio-mma8452-v4-temp-e040489abec3

Best regards,
--  
Sanjay Chitroda <sanjayembeddedse@gmail.com>

[PATCH v4 1/5] iio: accel: mma8452: convert to bulk regulator usage

[Sanjay Chitroda]

From: Sanjay Chitroda <sanjayembeddedse@gmail.com>

The "vdd" and "vddio" regulators are always controlled together. Switch
to the regulator bulk API to handle setup, enable, and disable paths in
a single call.

No functional change intended.

Suggested-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
Signed-off-by: Sanjay Chitroda <sanjayembeddedse@gmail.com>

---
changes in v4:
- added explicit header with input from Joshua Crofts
- validate new member of structure with `pahole` and `bloat-o-meter`
  comment from Andy
  ./scripts/bloat-o-meter mma8452-before.o mma8452-after.o
  add/remove: 0/0 grow/shrink: 0/6 up/down: 0/-1013 (-1013)
  Function                                     old     new   delta
  _entry_ptr                                    72      56     -16
  _entry                                       396     308     -88
  mma8452_remove                               646     548     -98
  mma8452_runtime_suspend                      633     467    -166
  mma8452_runtime_resume                       939     639    -300
  mma8452_probe                               4247    3902    -345
  Total: Before=36561, After=35548, chg -2.77%

  pahole before:
    /* size: 288, cachelines: 5, members: 10 */
    /* sum members: 278, holes: 2, sum holes: 10 */

  pahole after:
    /* size: 320, cachelines: 5, members: 9 */
    /* sum members: 310, holes: 2, sum holes: 10 */
  summary: overall size of driver is optimized and all new member are in same cacheline
- v3 link -> https://lore.kernel.org/all/20260505174640.3998281-7-sanjayembedded@gmail.com/
---
 drivers/iio/accel/mma8452.c | 60 ++++++++++++---------------------------------
 1 file changed, 16 insertions(+), 44 deletions(-)

diff --git a/drivers/iio/accel/mma8452.c b/drivers/iio/accel/mma8452.c
index 1403b32e2b21..a44b2138951e 100644
--- a/drivers/iio/accel/mma8452.c
+++ b/drivers/iio/accel/mma8452.c
@@ -18,6 +18,7 @@
  * TODO: orientation events
  */
 
+#include <linux/array_size.h>
 #include <linux/delay.h>
 #include <linux/i2c.h>
 #include <linux/mod_devicetable.h>
@@ -111,8 +112,7 @@ struct mma8452_data {
 	u8 data_cfg;
 	const struct mma_chip_info *chip_info;
 	int sleep_val;
-	struct regulator *vdd_reg;
-	struct regulator *vddio_reg;
+	struct regulator_bulk_data regs[2];
 
 	/* Ensure correct alignment of time stamp when present */
 	struct {
@@ -1570,25 +1570,15 @@ static int mma8452_probe(struct i2c_client *client)
 	if (ret)
 		return ret;
 
-	data->vdd_reg = devm_regulator_get(&client->dev, "vdd");
-	if (IS_ERR(data->vdd_reg))
-		return dev_err_probe(&client->dev, PTR_ERR(data->vdd_reg),
-				     "failed to get VDD regulator!\n");
-
-	data->vddio_reg = devm_regulator_get(&client->dev, "vddio");
-	if (IS_ERR(data->vddio_reg))
-		return dev_err_probe(&client->dev, PTR_ERR(data->vddio_reg),
-				     "failed to get VDDIO regulator!\n");
-
-	ret = regulator_enable(data->vdd_reg);
+	data->regs[0].supply = "vdd";
+	data->regs[1].supply = "vddio";
+	ret = devm_regulator_bulk_get(dev, ARRAY_SIZE(data->regs), data->regs);
 	if (ret)
-		return dev_err_probe(dev, ret, "failed to enable VDD regulator!\n");
+		return dev_err_probe(dev, ret, "failed to get regulators\n");
 
-	ret = regulator_enable(data->vddio_reg);
-	if (ret) {
-		dev_err_probe(dev, ret, "failed to enable VDDIO regulator!\n");
-		goto disable_regulator_vdd;
-	}
+	ret = regulator_bulk_enable(ARRAY_SIZE(data->regs), data->regs);
+	if (ret)
+		return dev_err_probe(dev, ret, "failed to enable regulators\n");
 
 	ret = i2c_smbus_read_byte_data(client, MMA8452_WHO_AM_I);
 	if (ret < 0)
@@ -1723,10 +1713,7 @@ static int mma8452_probe(struct i2c_client *client)
 	mma8452_trigger_cleanup(indio_dev);
 
 disable_regulators:
-	regulator_disable(data->vddio_reg);
-
-disable_regulator_vdd:
-	regulator_disable(data->vdd_reg);
+	regulator_bulk_disable(ARRAY_SIZE(data->regs), data->regs);
 
 	return ret;
 }
@@ -1748,8 +1735,7 @@ static void mma8452_remove(struct i2c_client *client)
 	mma8452_trigger_cleanup(indio_dev);
 	mma8452_standby(iio_priv(indio_dev));
 
-	regulator_disable(data->vddio_reg);
-	regulator_disable(data->vdd_reg);
+	regulator_bulk_disable(ARRAY_SIZE(data->regs), data->regs);
 }
 
 #ifdef CONFIG_PM
@@ -1767,15 +1753,9 @@ static int mma8452_runtime_suspend(struct device *dev)
 		return -EAGAIN;
 	}
 
-	ret = regulator_disable(data->vddio_reg);
-	if (ret) {
-		dev_err(dev, "failed to disable VDDIO regulator\n");
-		return ret;
-	}
-
-	ret = regulator_disable(data->vdd_reg);
+	ret = regulator_bulk_disable(ARRAY_SIZE(data->regs), data->regs);
 	if (ret) {
-		dev_err(dev, "failed to disable VDD regulator\n");
+		dev_err(dev, "failed to disable regulators\n");
 		return ret;
 	}
 
@@ -1788,16 +1768,9 @@ static int mma8452_runtime_resume(struct device *dev)
 	struct mma8452_data *data = iio_priv(indio_dev);
 	int ret, sleep_val;
 
-	ret = regulator_enable(data->vdd_reg);
-	if (ret) {
-		dev_err(dev, "failed to enable VDD regulator\n");
-		return ret;
-	}
-
-	ret = regulator_enable(data->vddio_reg);
+	ret = regulator_bulk_enable(ARRAY_SIZE(data->regs), data->regs);
 	if (ret) {
-		dev_err(dev, "failed to enable VDDIO regulator\n");
-		regulator_disable(data->vdd_reg);
+		dev_err(dev, "failed to enable regulators\n");
 		return ret;
 	}
 
@@ -1815,8 +1788,7 @@ static int mma8452_runtime_resume(struct device *dev)
 	return 0;
 
 runtime_resume_failed:
-	regulator_disable(data->vddio_reg);
-	regulator_disable(data->vdd_reg);
+	regulator_bulk_disable(ARRAY_SIZE(data->regs), data->regs);
 
 	return ret;
 }

-- 
2.34.1

Re: [PATCH v4 1/5] iio: accel: mma8452: convert to bulk regulator usage

[Andy Shevchenko]

On Tue, Jun 02, 2026 at 06:59:22PM +0530, Sanjay Chitroda wrote:
> 
> The "vdd" and "vddio" regulators are always controlled together. Switch
> to the regulator bulk API to handle setup, enable, and disable paths in
> a single call.
> 
> No functional change intended.

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

-- 
With Best Regards,
Andy Shevchenko

[PATCH v4 2/5] iio: accel: mma8452: use local struct device

[Sanjay Chitroda]

From: Sanjay Chitroda <sanjayembeddedse@gmail.com>

Introduce a local struct device pointer derived from &client->dev.
This avoids repeated &client->dev usage and improves readability.

Signed-off-by: Sanjay Chitroda <sanjayembeddedse@gmail.com>

---
changes in v4:
- address coding style comment given by Andy
- v3 link -> https://lore.kernel.org/all/20260505174640.3998281-8-sanjayembedded@gmail.com/
---
 drivers/iio/accel/mma8452.c | 39 +++++++++++++++++++--------------------
 1 file changed, 19 insertions(+), 20 deletions(-)

diff --git a/drivers/iio/accel/mma8452.c b/drivers/iio/accel/mma8452.c
index a44b2138951e..1d4ea614cb57 100644
--- a/drivers/iio/accel/mma8452.c
+++ b/drivers/iio/accel/mma8452.c
@@ -223,15 +223,15 @@ static int mma8452_drdy(struct mma8452_data *data)
 static int mma8452_set_runtime_pm_state(struct i2c_client *client, bool on)
 {
 #ifdef CONFIG_PM
+	struct device *dev = &client->dev;
 	int ret;
 
 	if (on)
-		ret = pm_runtime_resume_and_get(&client->dev);
+		ret = pm_runtime_resume_and_get(dev);
 	else
-		ret = pm_runtime_put_autosuspend(&client->dev);
+		ret = pm_runtime_put_autosuspend(dev);
 	if (ret < 0) {
-		dev_err(&client->dev,
-			"failed to change power state to %d\n", on);
+		dev_err(dev, "failed to change power state to %d\n", on);
 
 		return ret;
 	}
@@ -1553,7 +1553,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;
 
@@ -1563,10 +1563,9 @@ 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;
 
@@ -1599,7 +1598,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);
@@ -1632,10 +1631,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,
@@ -1643,7 +1642,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,
@@ -1680,14 +1679,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)
@@ -1722,11 +1720,12 @@ static void mma8452_remove(struct i2c_client *client)
 {
 	struct iio_dev *indio_dev = i2c_get_clientdata(client);
 	struct mma8452_data *data = iio_priv(indio_dev);
+	struct device *dev = &client->dev;
 
 	iio_device_unregister(indio_dev);
 
-	pm_runtime_disable(&client->dev);
-	pm_runtime_set_suspended(&client->dev);
+	pm_runtime_disable(dev);
+	pm_runtime_set_suspended(dev);
 
 	if (client->irq)
 		free_irq(client->irq, indio_dev);
@@ -1749,7 +1748,7 @@ static int mma8452_runtime_suspend(struct device *dev)
 	ret = mma8452_standby(data);
 	mutex_unlock(&data->lock);
 	if (ret < 0) {
-		dev_err(&data->client->dev, "powering off device failed\n");
+		dev_err(dev, "powering off device failed\n");
 		return -EAGAIN;
 	}
 

-- 
2.34.1

Re: [PATCH v4 2/5] iio: accel: mma8452: use local struct device

[Andy Shevchenko]

On Tue, Jun 02, 2026 at 06:59:23PM +0530, Sanjay Chitroda wrote:

> Introduce a local struct device pointer derived from &client->dev.
> This avoids repeated &client->dev usage and improves readability.

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

-- 
With Best Regards,
Andy Shevchenko

[PATCH v4 3/5] iio: accel: mma8452: use pm_ptr() and direct runtime PM calls

[Sanjay Chitroda]

From: Sanjay Chitroda <sanjayembeddedse@gmail.com>

Use pm_ptr() together with DEFINE_RUNTIME_DEV_PM_OPS() so the PM ops
pointer is automatically handled when CONFIG_PM is enabled or disabled.

Switch to direct PM runtime calls and drop
mma8452_set_runtime_pm_state() wrapper, which is no longer needed.

While at it, fix runtime PM handling in several paths to ensure proper
balancing of pm_runtime_resume_and_get() and pm_runtime_put(), avoiding
usage count leaks on error paths.

This follows modern kernel power-management conventions.

Signed-off-by: Sanjay Chitroda <sanjayembeddedse@gmail.com>

---
changes in v4:
- fix runtime PM handling by balancing the refcount with input from Jonathan
- v3 link -> https://lore.kernel.org/all/20260505174640.3998281-9-sanjayembedded@gmail.com/
changes in v3:
- Use direct PM runtime calls and drop redundant wrapper along with CONFIG_PM
- v2 link -> https://lore.kernel.org/all/20260422165643.2148195-6-sanjayembedded@gmail.com/
changes in v2:
- Use DEFINE_RUNTIME_DEV_PM_OPS to address review comment and resolve 0-day bot warning
- v1 link -> https://lore.kernel.org/all/20260414192045.3598010-1-sanjayembedded@gmail.com/
---
 drivers/iio/accel/mma8452.c | 110 ++++++++++++++++++++++++--------------------
 1 file changed, 59 insertions(+), 51 deletions(-)

diff --git a/drivers/iio/accel/mma8452.c b/drivers/iio/accel/mma8452.c
index 1d4ea614cb57..467e42c2c0dd 100644
--- a/drivers/iio/accel/mma8452.c
+++ b/drivers/iio/accel/mma8452.c
@@ -220,43 +220,21 @@ static int mma8452_drdy(struct mma8452_data *data)
 	return -EIO;
 }
 
-static int mma8452_set_runtime_pm_state(struct i2c_client *client, bool on)
-{
-#ifdef CONFIG_PM
-	struct device *dev = &client->dev;
-	int ret;
-
-	if (on)
-		ret = pm_runtime_resume_and_get(dev);
-	else
-		ret = pm_runtime_put_autosuspend(dev);
-	if (ret < 0) {
-		dev_err(dev, "failed to change power state to %d\n", on);
-
-		return ret;
-	}
-#endif
-
-	return 0;
-}
-
 static int mma8452_read(struct mma8452_data *data, __be16 buf[3])
 {
+	struct device *dev = &data->client->dev;
 	int ret = mma8452_drdy(data);
 
 	if (ret < 0)
 		return ret;
 
-	ret = mma8452_set_runtime_pm_state(data->client, true);
-	if (ret)
+	ret = pm_runtime_resume_and_get(dev);
+	if (ret < 0)
 		return ret;
 
 	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);
+	pm_runtime_put_autosuspend(dev);
 
 	return ret;
 }
@@ -975,6 +953,7 @@ static int mma8452_write_event_config(struct iio_dev *indio_dev,
 				      bool state)
 {
 	struct mma8452_data *data = iio_priv(indio_dev);
+	struct device *dev = &data->client->dev;
 	int val, ret;
 	const struct mma8452_event_regs *ev_regs;
 
@@ -982,17 +961,22 @@ static int mma8452_write_event_config(struct iio_dev *indio_dev,
 	if (ret)
 		return ret;
 
-	ret = mma8452_set_runtime_pm_state(data->client, state);
-	if (ret)
-		return ret;
+	if (state) {
+		ret = pm_runtime_resume_and_get(dev);
+		if (ret < 0)
+			return ret;
+	}
 
 	switch (dir) {
 	case IIO_EV_DIR_FALLING:
-		return mma8452_set_freefall_mode(data, state);
+		ret = mma8452_set_freefall_mode(data, state);
+		goto pm_runtime_put;
 	case IIO_EV_DIR_RISING:
 		val = i2c_smbus_read_byte_data(data->client, ev_regs->ev_cfg);
-		if (val < 0)
-			return val;
+		if (val < 0) {
+			ret = val;
+			goto pm_runtime_put;
+		}
 
 		if (state) {
 			if (mma8452_freefall_mode_enabled(data)) {
@@ -1004,19 +988,31 @@ static int mma8452_write_event_config(struct iio_dev *indio_dev,
 			val |= BIT(chan->scan_index +
 					ev_regs->ev_cfg_chan_shift);
 		} else {
-			if (mma8452_freefall_mode_enabled(data))
-				return 0;
-
+			if (mma8452_freefall_mode_enabled(data)) {
+				ret = 0;
+				goto pm_runtime_put;
+			}
 			val &= ~BIT(chan->scan_index +
 					ev_regs->ev_cfg_chan_shift);
 		}
 
 		val |= ev_regs->ev_cfg_ele;
 
-		return mma8452_change_config(data, ev_regs->ev_cfg, val);
+		ret = mma8452_change_config(data, ev_regs->ev_cfg, val);
+		goto pm_runtime_put;
+
 	default:
-		return -EINVAL;
+		ret = -EINVAL;
+		goto pm_runtime_put;
 	}
+
+pm_runtime_put:
+	if (state)
+		pm_runtime_put(dev);
+	else
+		pm_runtime_put_autosuspend(dev);
+
+	return ret;
 }
 
 static void mma8452_transient_interrupt(struct iio_dev *indio_dev)
@@ -1453,22 +1449,39 @@ static int mma8452_data_rdy_trigger_set_state(struct iio_trigger *trig,
 {
 	struct iio_dev *indio_dev = iio_trigger_get_drvdata(trig);
 	struct mma8452_data *data = iio_priv(indio_dev);
+	struct device *dev = &data->client->dev;
 	int reg, ret;
 
-	ret = mma8452_set_runtime_pm_state(data->client, state);
-	if (ret)
-		return ret;
+	if (state) {
+		ret = pm_runtime_resume_and_get(dev);
+		if (ret < 0)
+			return ret;
+	}
 
 	reg = i2c_smbus_read_byte_data(data->client, MMA8452_CTRL_REG4);
-	if (reg < 0)
-		return reg;
+	if (reg < 0) {
+		ret = reg;
+		goto pm_runtime_put;
+	}
 
 	if (state)
 		reg |= MMA8452_INT_DRDY;
 	else
 		reg &= ~MMA8452_INT_DRDY;
 
-	return mma8452_change_config(data, MMA8452_CTRL_REG4, reg);
+	ret = mma8452_change_config(data, MMA8452_CTRL_REG4, reg);
+	if (ret < 0)
+		goto pm_runtime_put;
+
+	if (!state)
+		return pm_runtime_put_autosuspend(dev);
+
+	return 0;
+
+pm_runtime_put:
+	if (state)
+		pm_runtime_put(dev);
+	return ret;
 }
 
 static const struct iio_trigger_ops mma8452_trigger_ops = {
@@ -1737,7 +1750,6 @@ static void mma8452_remove(struct i2c_client *client)
 	regulator_bulk_disable(ARRAY_SIZE(data->regs), data->regs);
 }
 
-#ifdef CONFIG_PM
 static int mma8452_runtime_suspend(struct device *dev)
 {
 	struct iio_dev *indio_dev = i2c_get_clientdata(to_i2c_client(dev));
@@ -1791,13 +1803,9 @@ static int mma8452_runtime_resume(struct device *dev)
 
 	return ret;
 }
-#endif
 
-static const struct dev_pm_ops mma8452_pm_ops = {
-	SET_SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend, pm_runtime_force_resume)
-	SET_RUNTIME_PM_OPS(mma8452_runtime_suspend,
-			   mma8452_runtime_resume, NULL)
-};
+static DEFINE_RUNTIME_DEV_PM_OPS(mma8452_pm_ops,
+				 mma8452_runtime_suspend, mma8452_runtime_resume, NULL);
 
 static const struct i2c_device_id mma8452_id[] = {
 	{ .name = "fxls8471", .driver_data = (kernel_ulong_t)&mma_chip_info_table[fxls8471] },
@@ -1814,7 +1822,7 @@ static struct i2c_driver mma8452_driver = {
 	.driver = {
 		.name	= "mma8452",
 		.of_match_table = mma8452_dt_ids,
-		.pm	= &mma8452_pm_ops,
+		.pm	= pm_ptr(&mma8452_pm_ops),
 	},
 	.probe = mma8452_probe,
 	.remove = mma8452_remove,

-- 
2.34.1

Re: [PATCH v4 3/5] iio: accel: mma8452: use pm_ptr() and direct runtime PM calls

[Jonathan Cameron]

On Tue, 02 Jun 2026 18:59:24 +0530
Sanjay Chitroda <sanjayembeddedse@gmail.com> wrote:

> From: Sanjay Chitroda <sanjayembeddedse@gmail.com>
> 
> Use pm_ptr() together with DEFINE_RUNTIME_DEV_PM_OPS() so the PM ops
> pointer is automatically handled when CONFIG_PM is enabled or disabled.
> 
> Switch to direct PM runtime calls and drop
> mma8452_set_runtime_pm_state() wrapper, which is no longer needed.
> 
> While at it, fix runtime PM handling in several paths to ensure proper
> balancing of pm_runtime_resume_and_get() and pm_runtime_put(), avoiding
> usage count leaks on error paths.

"While at it" comments cause alarm bells to ring, even more so when they
are fixes. This should be broken up and the fixes moved to the start
of the patch set followed only later by rework.

I'm fairly sure this introduces some bugs.. What testing can you do on this
one?  I'd be very nervous touching this without some form of verification
that the state is correct and the power remains on when events are in use
(right not it doesn't)

The final problem of disabling events on remove() also needs to be covered.
It is up to the driver to figure out what events are enabled and turn
them off.

> 
> This follows modern kernel power-management conventions.
> 
> Signed-off-by: Sanjay Chitroda <sanjayembeddedse@gmail.com>
> 
> ---
> changes in v4:
> - fix runtime PM handling by balancing the refcount with input from Jonathan
> - v3 link -> https://lore.kernel.org/all/20260505174640.3998281-9-sanjayembedded@gmail.com/
> changes in v3:
> - Use direct PM runtime calls and drop redundant wrapper along with CONFIG_PM
> - v2 link -> https://lore.kernel.org/all/20260422165643.2148195-6-sanjayembedded@gmail.com/
> changes in v2:
> - Use DEFINE_RUNTIME_DEV_PM_OPS to address review comment and resolve 0-day bot warning
> - v1 link -> https://lore.kernel.org/all/20260414192045.3598010-1-sanjayembedded@gmail.com/
> ---
>  drivers/iio/accel/mma8452.c | 110 ++++++++++++++++++++++++--------------------
>  1 file changed, 59 insertions(+), 51 deletions(-)
> 
> diff --git a/drivers/iio/accel/mma8452.c b/drivers/iio/accel/mma8452.c
> index 1d4ea614cb57..467e42c2c0dd 100644
> --- a/drivers/iio/accel/mma8452.c
> +++ b/drivers/iio/accel/mma8452.c

> @@ -975,6 +953,7 @@ static int mma8452_write_event_config(struct iio_dev *indio_dev,
>  				      bool state)
>  {
>  	struct mma8452_data *data = iio_priv(indio_dev);
> +	struct device *dev = &data->client->dev;
>  	int val, ret;
>  	const struct mma8452_event_regs *ev_regs;
>  
> @@ -982,17 +961,22 @@ static int mma8452_write_event_config(struct iio_dev *indio_dev,

This one is fiddlier. In the case of turning an event on the aim
is to leave the pm counter raised, the decrement only comes when turning it off.

>  	if (ret)
>  		return ret;
>  
> -	ret = mma8452_set_runtime_pm_state(data->client, state);
> -	if (ret)
> -		return ret;
> +	if (state) {
> +		ret = pm_runtime_resume_and_get(dev);
> +		if (ret < 0)
> +			return ret;
> +	}
>  
>  	switch (dir) {
>  	case IIO_EV_DIR_FALLING:
> -		return mma8452_set_freefall_mode(data, state);
> +		ret = mma8452_set_freefall_mode(data, state);
> +		goto pm_runtime_put;

see below for more discussion but break might be appropriate here.

>  	case IIO_EV_DIR_RISING:
>  		val = i2c_smbus_read_byte_data(data->client, ev_regs->ev_cfg);
> -		if (val < 0)
> -			return val;
> +		if (val < 0) {
> +			ret = val;
> +			goto pm_runtime_put;
> +		}
>  
>  		if (state) {
>  			if (mma8452_freefall_mode_enabled(data)) {
> @@ -1004,19 +988,31 @@ static int mma8452_write_event_config(struct iio_dev *indio_dev,
>  			val |= BIT(chan->scan_index +
>  					ev_regs->ev_cfg_chan_shift);
>  		} else {
> -			if (mma8452_freefall_mode_enabled(data))
> -				return 0;
> -
> +			if (mma8452_freefall_mode_enabled(data)) {
> +				ret = 0;
I'm not 100% sure what intent is here. This is in a rising path
which has nothing directly to do with freefall yet the configuration
is not modified.

In this path state == false, and we aren't in an error case so maybe we just want
to do nothing other than decrementing the counter.



> +				goto pm_runtime_put;

> +			}
>  			val &= ~BIT(chan->scan_index +
>  					ev_regs->ev_cfg_chan_shift);
>  		}
>  
>  		val |= ev_regs->ev_cfg_ele;
>  
> -		return mma8452_change_config(data, ev_regs->ev_cfg, val);
> +		ret = mma8452_change_config(data, ev_regs->ev_cfg, val);
> +		goto pm_runtime_put;
> +
>  	default:
> -		return -EINVAL;
> +		ret = -EINVAL;
> +		goto pm_runtime_put;
>  	}
> +



> +pm_runtime_put:
> +	if (state)
> +		pm_runtime_put(dev);
> +	else
> +		pm_runtime_put_autosuspend(dev);
This is wrong as the counter is universally decremented
whether or not we were turning something on.

Anyhow, Split this an do an earlier return in good path.

Mind you there is a fun question of what the right thing to do on
a failure is.  Maybe we should assume a retry is on the way and
do a put_autosuspend.

If you do keep the combined, you can just break in the switch
rather than gotos.


> +
> +	return ret;
>  }
>  
>  static void mma8452_transient_interrupt(struct iio_dev *indio_dev)
> @@ -1453,22 +1449,39 @@ static int mma8452_data_rdy_trigger_set_state(struct iio_trigger *trig,
>  {
>  	struct iio_dev *indio_dev = iio_trigger_get_drvdata(trig);
>  	struct mma8452_data *data = iio_priv(indio_dev);
> +	struct device *dev = &data->client->dev;
>  	int reg, ret;
>  
> -	ret = mma8452_set_runtime_pm_state(data->client, state);
> -	if (ret)
> -		return ret;
> +	if (state) {
> +		ret = pm_runtime_resume_and_get(dev);
> +		if (ret < 0)
> +			return ret;
> +	}
>  
>  	reg = i2c_smbus_read_byte_data(data->client, MMA8452_CTRL_REG4);
> -	if (reg < 0)
> -		return reg;
> +	if (reg < 0) {
> +		ret = reg;
> +		goto pm_runtime_put;
> +	}
>  
>  	if (state)
>  		reg |= MMA8452_INT_DRDY;
>  	else
>  		reg &= ~MMA8452_INT_DRDY;
>  
> -	return mma8452_change_config(data, MMA8452_CTRL_REG4, reg);
> +	ret = mma8452_change_config(data, MMA8452_CTRL_REG4, reg);
> +	if (ret < 0)
> +		goto pm_runtime_put;
> +
> +	if (!state)
> +		return pm_runtime_put_autosuspend(dev);
> +
> +	return 0;
> +
> +pm_runtime_put:
> +	if (state)
> +		pm_runtime_put(dev);
> +	return ret;
After the cleanup there isn't a lot of shared code in here and the
flow is confusing I'd break it into two helpers along the lines of:

static int mma8452_data_rdy_trigger_enable(struct mma8452_data *data)
{
	struct device *dev = &data->client->dev;
	int ret;

	ret = pm_runtime_resume_and_get(dev);
	if (ret < 0)
		return ret;

	ret = i2c_smbus_read_byte_data(data->client, MMA8452_CTRL_REG4);
	if (ret < 0)
		goto powerdown;

	ret = mma8452_change_config(data, MMA8452_CTRL_REG4, ret | MMA8452_INT_DRDY);
	if (ret < 0)
		goto powerdown;

	return 0;

powerdown:
	pm_runtime_put(dev);
	return ret;
}

static int mma8452_data_ready_trigger_disable(struct mma8452_data *data)
{
	struct device *dev = &data->client->dev;
	int reg, ret;

	ret = i2c_smbus_read_byte_data(data->client, MMA8452_CTRL_REG4);
	if (ret < 0)
		return ret;
	
	ret = mma8452_change_config(data, MMA8452_CTRL_REG4, ret & ~MMA8452_INT_DRDY);
	if (ret < 0)
		return ret;

	pm_runtime_put_autosuspend(dev);

	return 0;
}

Then pick between them based on state.

While that is a fairly significant refactor I think that is fine as part of
the fix as it makes the code more readable.


>  }
>  
>  static const struct iio_trigger_ops mma8452_trigger_ops = {
> @@ -1737,7 +1750,6 @@ static void mma8452_remove(struct i2c_client *client)
>  	regulator_bulk_disable(ARRAY_SIZE(data->regs), data->regs);
>  }
>  
> -#ifdef CONFIG_PM
Unrelated to the fixes.

>  static int mma8452_runtime_suspend(struct device *dev)
>  {
>  	struct iio_dev *indio_dev = i2c_get_clientdata(to_i2c_client(dev));
> @@ -1791,13 +1803,9 @@ static int mma8452_runtime_resume(struct device *dev)
>  
>  	return ret;
>  }
> -#endif
>  
> -static const struct dev_pm_ops mma8452_pm_ops = {
> -	SET_SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend, pm_runtime_force_resume)
> -	SET_RUNTIME_PM_OPS(mma8452_runtime_suspend,
> -			   mma8452_runtime_resume, NULL)
> -};
> +static DEFINE_RUNTIME_DEV_PM_OPS(mma8452_pm_ops,
> +				 mma8452_runtime_suspend, mma8452_runtime_resume, NULL);

This is unrelated to the fix aspect above.


>  
>  static const struct i2c_device_id mma8452_id[] = {
>  	{ .name = "fxls8471", .driver_data = (kernel_ulong_t)&mma_chip_info_table[fxls8471] },
> @@ -1814,7 +1822,7 @@ static struct i2c_driver mma8452_driver = {
>  	.driver = {
>  		.name	= "mma8452",
>  		.of_match_table = mma8452_dt_ids,
> -		.pm	= &mma8452_pm_ops,
> +		.pm	= pm_ptr(&mma8452_pm_ops),
>  	},
>  	.probe = mma8452_probe,
>  	.remove = mma8452_remove,
> 

[PATCH v4 4/5] iio: accel: mma8452: Use IIO cleanup helpers

[Sanjay Chitroda]

From: Sanjay Chitroda <sanjayembeddedse@gmail.com>

Use IIO_DEV_ACQUIRE_DIRECT_MODE() helper to automatically release
direct mode.

Suggested-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
Signed-off-by: Sanjay Chitroda <sanjayembeddedse@gmail.com>
---
 drivers/iio/accel/mma8452.c | 16 +++++++---------
 1 file changed, 7 insertions(+), 9 deletions(-)

diff --git a/drivers/iio/accel/mma8452.c b/drivers/iio/accel/mma8452.c
index 467e42c2c0dd..85dfd854e2b6 100644
--- a/drivers/iio/accel/mma8452.c
+++ b/drivers/iio/accel/mma8452.c
@@ -474,14 +474,14 @@ static int mma8452_read_raw(struct iio_dev *indio_dev,
 	int i, ret;
 
 	switch (mask) {
-	case IIO_CHAN_INFO_RAW:
-		if (!iio_device_claim_direct(indio_dev))
+	case IIO_CHAN_INFO_RAW: {
+		IIO_DEV_ACQUIRE_DIRECT_MODE(indio_dev, claim);
+		if (IIO_DEV_ACQUIRE_FAILED(claim))
 			return -EBUSY;
 
 		mutex_lock(&data->lock);
 		ret = mma8452_read(data, buffer);
 		mutex_unlock(&data->lock);
-		iio_device_release_direct(indio_dev);
 		if (ret < 0)
 			return ret;
 
@@ -490,6 +490,7 @@ static int mma8452_read_raw(struct iio_dev *indio_dev,
 			chan->scan_type.realbits - 1);
 
 		return IIO_VAL_INT;
+	}
 	case IIO_CHAN_INFO_SCALE:
 		i = data->data_cfg & MMA8452_DATA_CFG_FS_MASK;
 		*val = data->chip_info->mma_scales[i][0];
@@ -756,14 +757,11 @@ static int mma8452_write_raw(struct iio_dev *indio_dev,
 			     struct iio_chan_spec const *chan,
 			     int val, int val2, long mask)
 {
-	int ret;
-
-	if (!iio_device_claim_direct(indio_dev))
+	IIO_DEV_ACQUIRE_DIRECT_MODE(indio_dev, claim);
+	if (IIO_DEV_ACQUIRE_FAILED(claim))
 		return -EBUSY;
 
-	ret = __mma8452_write_raw(indio_dev, chan, val, val2, mask);
-	iio_device_release_direct(indio_dev);
-	return ret;
+	return __mma8452_write_raw(indio_dev, chan, val, val2, mask);
 }
 
 static int mma8452_get_event_regs(struct mma8452_data *data,

-- 
2.34.1

Re: [PATCH v4 4/5] iio: accel: mma8452: Use IIO cleanup helpers

[Andy Shevchenko]

On Tue, Jun 02, 2026 at 06:59:25PM +0530, Sanjay Chitroda wrote:

> Use IIO_DEV_ACQUIRE_DIRECT_MODE() helper to automatically release
> direct mode.

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

-- 
With Best Regards,
Andy Shevchenko

[PATCH v4 5/5] iio: accel: mma8452: use guard() to release mutexes

[Sanjay Chitroda]

From: Sanjay Chitroda <sanjayembeddedse@gmail.com>

Replace explicit mutex_lock() and mutex_unlock() with the guard() and
scoped_guard() macro for cleaner and safer mutex handling.

Signed-off-by: Sanjay Chitroda <sanjayembeddedse@gmail.com>

---
changes in v4:
- add blank line with input from Andy
- v3 link -> https://lore.kernel.org/all/20260505174640.3998281-11-sanjayembedded@gmail.com/
changes in v3:
- Following input from Jonathan extended mutex scope for
  IIO_CHAN_INFO_RAW case to include math operation under lock
- v2 link -> https://lore.kernel.org/all/20260422165643.2148195-7-sanjayembedded@gmail.com/
---
 drivers/iio/accel/mma8452.c | 30 ++++++++++++------------------
 1 file changed, 12 insertions(+), 18 deletions(-)

diff --git a/drivers/iio/accel/mma8452.c b/drivers/iio/accel/mma8452.c
index 85dfd854e2b6..7d0b4560ba2a 100644
--- a/drivers/iio/accel/mma8452.c
+++ b/drivers/iio/accel/mma8452.c
@@ -19,6 +19,7 @@
  */
 
 #include <linux/array_size.h>
+#include <linux/cleanup.h>
 #include <linux/delay.h>
 #include <linux/i2c.h>
 #include <linux/mod_devicetable.h>
@@ -479,9 +480,9 @@ static int mma8452_read_raw(struct iio_dev *indio_dev,
 		if (IIO_DEV_ACQUIRE_FAILED(claim))
 			return -EBUSY;
 
-		mutex_lock(&data->lock);
+		guard(mutex)(&data->lock);
+
 		ret = mma8452_read(data, buffer);
-		mutex_unlock(&data->lock);
 		if (ret < 0)
 			return ret;
 
@@ -579,36 +580,30 @@ static int mma8452_change_config(struct mma8452_data *data, u8 reg, u8 val)
 	int ret;
 	int is_active;
 
-	mutex_lock(&data->lock);
+	guard(mutex)(&data->lock);
 
 	is_active = mma8452_is_active(data);
-	if (is_active < 0) {
-		ret = is_active;
-		goto fail;
-	}
+	if (is_active < 0)
+		return is_active;
 
 	/* config can only be changed when in standby */
 	if (is_active > 0) {
 		ret = mma8452_standby(data);
 		if (ret < 0)
-			goto fail;
+			return ret;
 	}
 
 	ret = i2c_smbus_write_byte_data(data->client, reg, val);
 	if (ret < 0)
-		goto fail;
+		return ret;
 
 	if (is_active > 0) {
 		ret = mma8452_active(data);
 		if (ret < 0)
-			goto fail;
+			return ret;
 	}
 
-	ret = 0;
-fail:
-	mutex_unlock(&data->lock);
-
-	return ret;
+	return 0;
 }
 
 static int mma8452_set_power_mode(struct mma8452_data *data, u8 mode)
@@ -1754,9 +1749,8 @@ static int mma8452_runtime_suspend(struct device *dev)
 	struct mma8452_data *data = iio_priv(indio_dev);
 	int ret;
 
-	mutex_lock(&data->lock);
-	ret = mma8452_standby(data);
-	mutex_unlock(&data->lock);
+	scoped_guard(mutex, &data->lock)
+		ret = mma8452_standby(data);
 	if (ret < 0) {
 		dev_err(dev, "powering off device failed\n");
 		return -EAGAIN;

-- 
2.34.1

Re: [PATCH v4 5/5] iio: accel: mma8452: use guard() to release mutexes

[Andy Shevchenko]

On Tue, Jun 02, 2026 at 06:59:26PM +0530, Sanjay Chitroda wrote:

> Replace explicit mutex_lock() and mutex_unlock() with the guard() and
> scoped_guard() macro for cleaner and safer mutex handling.

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

-- 
With Best Regards,
Andy Shevchenko