[PATCH v3 00/10] iio: accel: mma8452: improve coding style, pm and resource cleanup

[PATCH v3 00/10] iio: accel: mma8452: improve coding style, pm and resource cleanup

[Sanjay Chitroda]

From: Sanjay Chitroda <sanjayembeddedse@gmail.com>

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.

Thanks,
Sanjay Chitroda

Sanjay Chitroda (10):
  iio: accel: mma8452: handle I2C read error(s) in mma8452_read()
  iio: accel: mma8452: switch to non-devm request_threaded_irq()
  iio: accel: mma8452: cleanup codestyle warning
  iio: accel: mma8452: sort headers alphabetically
  iio: accel: mma8452: Use dev_err_probe()
  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 | 278 +++++++++++++++---------------------
 1 file changed, 116 insertions(+), 162 deletions(-)


base-commit: 39b80c5c9830d12d2d6531059001301c4265322a
-- 
2.34.1

[PATCH v3 01/10] iio: accel: mma8452: handle I2C read error(s) in mma8452_read()

[Sanjay Chitroda]

From: Sanjay Chitroda <sanjayembeddedse@gmail.com>

Currently, If i2c_smbus_read_i2c_block_data() fails but
mma8452_set_runtime_pm_state() succeeds, mma8452_read() returns 0.

As a result, the caller mma8452_read_raw() assumes the read was
successful and proceeds to use a buffer containing uninitialized
stack memory.

Add proper checking of the I2C read return value and propagate errors
to the caller.

Fixes: 96c0cb2bbfe0 ("iio: mma8452: add support for runtime power management")
Signed-off-by: Sanjay Chitroda <sanjayembeddedse@gmail.com>
---
 drivers/iio/accel/mma8452.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/iio/accel/mma8452.c b/drivers/iio/accel/mma8452.c
index 15172ba2972c..cefc7cf4bd83 100644
--- a/drivers/iio/accel/mma8452.c
+++ b/drivers/iio/accel/mma8452.c
@@ -252,6 +252,8 @@ static int mma8452_read(struct mma8452_data *data, __be16 buf[3])
 
 	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);
 
-- 
2.34.1

[PATCH v3 02/10] iio: accel: mma8452: switch to non-devm request_threaded_irq()

[Sanjay Chitroda]

From: Sanjay Chitroda <sanjayembeddedse@gmail.com>

Avoid using devm_request_threaded_irq() as the driver requires explicit
error-handling path(s). Using devm_* API together with goto-based
unwinding breaks the expected LIFO resource release model.

Add explicit IRQ cleanup in the driver teardown paths to follow kernel
resource management conventions.

Fixes: 28e3427824cc ("iio: mma8452: Basic support for transient events.")
Signed-off-by: Sanjay Chitroda <sanjayembeddedse@gmail.com>
---
 drivers/iio/accel/mma8452.c | 19 ++++++++++++-------
 1 file changed, 12 insertions(+), 7 deletions(-)

diff --git a/drivers/iio/accel/mma8452.c b/drivers/iio/accel/mma8452.c
index cefc7cf4bd83..279a9b364886 100644
--- a/drivers/iio/accel/mma8452.c
+++ b/drivers/iio/accel/mma8452.c
@@ -1682,18 +1682,16 @@ static int mma8452_probe(struct i2c_client *client)
 		goto trigger_cleanup;
 
 	if (client->irq) {
-		ret = devm_request_threaded_irq(&client->dev,
-						client->irq,
-						NULL, mma8452_interrupt,
-						IRQF_TRIGGER_LOW | IRQF_ONESHOT,
-						client->name, indio_dev);
+		ret = request_threaded_irq(client->irq, NULL, mma8452_interrupt,
+					   IRQF_TRIGGER_LOW | IRQF_ONESHOT,
+					   client->name, indio_dev);
 		if (ret)
 			goto buffer_cleanup;
 	}
 
 	ret = pm_runtime_set_active(&client->dev);
 	if (ret < 0)
-		goto buffer_cleanup;
+		goto free_irq;
 
 	pm_runtime_enable(&client->dev);
 	pm_runtime_set_autosuspend_delay(&client->dev,
@@ -1702,7 +1700,7 @@ static int mma8452_probe(struct i2c_client *client)
 
 	ret = iio_device_register(indio_dev);
 	if (ret < 0)
-		goto buffer_cleanup;
+		goto free_irq;
 
 	ret = mma8452_set_freefall_mode(data, false);
 	if (ret < 0)
@@ -1713,6 +1711,10 @@ static int mma8452_probe(struct i2c_client *client)
 unregister_device:
 	iio_device_unregister(indio_dev);
 
+free_irq:
+	if (client->irq)
+		free_irq(client->irq, indio_dev);
+
 buffer_cleanup:
 	iio_triggered_buffer_cleanup(indio_dev);
 
@@ -1738,6 +1740,9 @@ static void mma8452_remove(struct i2c_client *client)
 	pm_runtime_disable(&client->dev);
 	pm_runtime_set_suspended(&client->dev);
 
+	if (client->irq)
+		free_irq(client->irq, indio_dev);
+
 	iio_triggered_buffer_cleanup(indio_dev);
 	mma8452_trigger_cleanup(indio_dev);
 	mma8452_standby(iio_priv(indio_dev));
-- 
2.34.1

[PATCH v3 03/10] iio: accel: mma8452: cleanup codestyle warning

[Sanjay Chitroda]

From: Sanjay Chitroda <sanjayembeddedse@gmail.com>

Reported by checkpatch:
FILE: drivers/iio/accel/mma8452.c

CHECK: Alignment should match open parenthesis

Signed-off-by: Sanjay Chitroda <sanjayembeddedse@gmail.com>
---
 drivers/iio/accel/mma8452.c | 47 +++++++++++++++++++------------------
 1 file changed, 24 insertions(+), 23 deletions(-)

diff --git a/drivers/iio/accel/mma8452.c b/drivers/iio/accel/mma8452.c
index 279a9b364886..916631519d3f 100644
--- a/drivers/iio/accel/mma8452.c
+++ b/drivers/iio/accel/mma8452.c
@@ -706,8 +706,8 @@ static int mma8452_set_hp_filter_frequency(struct mma8452_data *data,
 }
 
 static int __mma8452_write_raw(struct iio_dev *indio_dev,
-			     struct iio_chan_spec const *chan,
-			     int val, int val2, long mask)
+			       struct iio_chan_spec const *chan,
+			       int val, int val2, long mask)
 {
 	struct mma8452_data *data = iio_priv(indio_dev);
 	int i, j, ret;
@@ -788,8 +788,9 @@ static int mma8452_write_raw(struct iio_dev *indio_dev,
 }
 
 static int mma8452_get_event_regs(struct mma8452_data *data,
-		const struct iio_chan_spec *chan, enum iio_event_direction dir,
-		const struct mma8452_event_regs **ev_reg)
+				  const struct iio_chan_spec *chan,
+				  enum iio_event_direction dir,
+				  const struct mma8452_event_regs **ev_reg)
 {
 	if (!chan)
 		return -EINVAL;
@@ -818,11 +819,11 @@ static int mma8452_get_event_regs(struct mma8452_data *data,
 }
 
 static int mma8452_read_event_value(struct iio_dev *indio_dev,
-			       const struct iio_chan_spec *chan,
-			       enum iio_event_type type,
-			       enum iio_event_direction dir,
-			       enum iio_event_info info,
-			       int *val, int *val2)
+				    const struct iio_chan_spec *chan,
+				    enum iio_event_type type,
+				    enum iio_event_direction dir,
+				    enum iio_event_info info,
+				    int *val, int *val2)
 {
 	struct mma8452_data *data = iio_priv(indio_dev);
 	int ret, us, power_mode;
@@ -881,11 +882,11 @@ static int mma8452_read_event_value(struct iio_dev *indio_dev,
 }
 
 static int mma8452_write_event_value(struct iio_dev *indio_dev,
-				const struct iio_chan_spec *chan,
-				enum iio_event_type type,
-				enum iio_event_direction dir,
-				enum iio_event_info info,
-				int val, int val2)
+				     const struct iio_chan_spec *chan,
+				     enum iio_event_type type,
+				     enum iio_event_direction dir,
+				     enum iio_event_info info,
+				     int val, int val2)
 {
 	struct mma8452_data *data = iio_priv(indio_dev);
 	int ret, reg, steps;
@@ -955,8 +956,7 @@ static int mma8452_read_event_config(struct iio_dev *indio_dev,
 	case IIO_EV_DIR_FALLING:
 		return mma8452_freefall_mode_enabled(data);
 	case IIO_EV_DIR_RISING:
-		ret = i2c_smbus_read_byte_data(data->client,
-				ev_regs->ev_cfg);
+		ret = i2c_smbus_read_byte_data(data->client, ev_regs->ev_cfg);
 		if (ret < 0)
 			return ret;
 
@@ -1193,7 +1193,7 @@ static const struct attribute_group mma8452_event_attribute_group = {
 
 static const struct iio_mount_matrix *
 mma8452_get_mount_matrix(const struct iio_dev *indio_dev,
-			   const struct iio_chan_spec *chan)
+			 const struct iio_chan_spec *chan)
 {
 	struct mma8452_data *data = iio_priv(indio_dev);
 
@@ -1516,8 +1516,9 @@ static int mma8452_reset(struct i2c_client *client)
 	 * The following code will read the reset register, and check whether
 	 * this reset works.
 	 */
-	i2c_smbus_write_byte_data(client, MMA8452_CTRL_REG2,
-					MMA8452_CTRL_REG2_RST);
+	i2c_smbus_write_byte_data(client,
+				  MMA8452_CTRL_REG2,
+				  MMA8452_CTRL_REG2_RST);
 
 	for (i = 0; i < 10; i++) {
 		usleep_range(100, 200);
@@ -1647,8 +1648,8 @@ static int mma8452_probe(struct i2c_client *client)
 			dev_dbg(&client->dev, "using interrupt line INT2\n");
 		} else {
 			ret = i2c_smbus_write_byte_data(client,
-						MMA8452_CTRL_REG5,
-						data->chip_info->all_events);
+							MMA8452_CTRL_REG5,
+							data->chip_info->all_events);
 			if (ret < 0)
 				goto disable_regulators;
 
@@ -1656,8 +1657,8 @@ static int mma8452_probe(struct i2c_client *client)
 		}
 
 		ret = i2c_smbus_write_byte_data(client,
-					MMA8452_CTRL_REG4,
-					data->chip_info->enabled_events);
+						MMA8452_CTRL_REG4,
+						data->chip_info->enabled_events);
 		if (ret < 0)
 			goto disable_regulators;
 
-- 
2.34.1

[PATCH v3 04/10] iio: accel: mma8452: sort headers alphabetically

[Sanjay Chitroda]

From: Sanjay Chitroda <sanjayembeddedse@gmail.com>

Sort include headers alphabetically to improve readability.

Signed-off-by: Sanjay Chitroda <sanjayembeddedse@gmail.com>
---
Changes in v3:
- Grouped regulator header with common linux header group
- Link to v2: https://lore.kernel.org/all/20260422165643.2148195-3-sanjayembedded@gmail.com/
---
 drivers/iio/accel/mma8452.c | 17 +++++++++--------
 1 file changed, 9 insertions(+), 8 deletions(-)

diff --git a/drivers/iio/accel/mma8452.c b/drivers/iio/accel/mma8452.c
index 916631519d3f..d227ce3d5f67 100644
--- a/drivers/iio/accel/mma8452.c
+++ b/drivers/iio/accel/mma8452.c
@@ -18,21 +18,22 @@
  * TODO: orientation events
  */
 
-#include <linux/module.h>
+#include <linux/delay.h>
+#include <linux/i2c.h>
 #include <linux/mod_devicetable.h>
+#include <linux/module.h>
+#include <linux/pm_runtime.h>
 #include <linux/property.h>
-#include <linux/i2c.h>
+#include <linux/types.h>
+#include <linux/regulator/consumer.h>
+
+#include <linux/iio/buffer.h>
+#include <linux/iio/events.h>
 #include <linux/iio/iio.h>
 #include <linux/iio/sysfs.h>
-#include <linux/iio/buffer.h>
 #include <linux/iio/trigger.h>
 #include <linux/iio/trigger_consumer.h>
 #include <linux/iio/triggered_buffer.h>
-#include <linux/iio/events.h>
-#include <linux/delay.h>
-#include <linux/pm_runtime.h>
-#include <linux/regulator/consumer.h>
-#include <linux/types.h>
 
 #define MMA8452_STATUS				0x00
 #define  MMA8452_STATUS_DRDY			(BIT(2) | BIT(1) | BIT(0))
-- 
2.34.1

[PATCH v3 05/10] iio: accel: mma8452: Use dev_err_probe()

[Sanjay Chitroda]

From: Sanjay Chitroda <sanjayembeddedse@gmail.com>

dev_err_probe() makes error code handling simpler and handle
deferred probe nicely (avoid spamming logs).

Signed-off-by: Sanjay Chitroda <sanjayembeddedse@gmail.com>
---
 drivers/iio/accel/mma8452.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/drivers/iio/accel/mma8452.c b/drivers/iio/accel/mma8452.c
index d227ce3d5f67..b49949792190 100644
--- a/drivers/iio/accel/mma8452.c
+++ b/drivers/iio/accel/mma8452.c
@@ -1548,6 +1548,7 @@ MODULE_DEVICE_TABLE(of, mma8452_dt_ids);
 
 static int mma8452_probe(struct i2c_client *client)
 {
+	struct device *dev = &client->dev;
 	struct mma8452_data *data;
 	struct iio_dev *indio_dev;
 	int ret;
@@ -1580,14 +1581,12 @@ static int mma8452_probe(struct i2c_client *client)
 				     "failed to get VDDIO regulator!\n");
 
 	ret = regulator_enable(data->vdd_reg);
-	if (ret) {
-		dev_err(&client->dev, "failed to enable VDD regulator!\n");
-		return ret;
-	}
+	if (ret)
+		return dev_err_probe(dev, ret, "failed to enable VDD regulator!\n");
 
 	ret = regulator_enable(data->vddio_reg);
 	if (ret) {
-		dev_err(&client->dev, "failed to enable VDDIO regulator!\n");
+		dev_err_probe(dev, ret, "failed to enable VDDIO regulator!\n");
 		goto disable_regulator_vdd;
 	}
 
-- 
2.34.1

[PATCH v3 06/10] 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>
---
 drivers/iio/accel/mma8452.c | 59 ++++++++++---------------------------
 1 file changed, 15 insertions(+), 44 deletions(-)

diff --git a/drivers/iio/accel/mma8452.c b/drivers/iio/accel/mma8452.c
index b49949792190..1c984c708ec3 100644
--- a/drivers/iio/accel/mma8452.c
+++ b/drivers/iio/accel/mma8452.c
@@ -111,8 +111,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 +1569,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 +1712,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 +1734,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 +1752,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 +1767,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 +1787,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

[PATCH v3 07/10] 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>
---
 drivers/iio/accel/mma8452.c | 37 +++++++++++++++++++------------------
 1 file changed, 19 insertions(+), 18 deletions(-)

diff --git a/drivers/iio/accel/mma8452.c b/drivers/iio/accel/mma8452.c
index 1c984c708ec3..2cd24b1543af 100644
--- a/drivers/iio/accel/mma8452.c
+++ b/drivers/iio/accel/mma8452.c
@@ -222,15 +222,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;
 	}
@@ -1552,7 +1552,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;
 
@@ -1562,10 +1562,10 @@ 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,
+		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;
 
@@ -1598,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);
@@ -1631,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,
@@ -1642,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,
@@ -1679,14 +1679,14 @@ 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,
+	pm_runtime_enable(dev);
+	pm_runtime_set_autosuspend_delay(dev,
 					 MMA8452_AUTO_SUSPEND_DELAY_MS);
-	pm_runtime_use_autosuspend(&client->dev);
+	pm_runtime_use_autosuspend(dev);
 
 	ret = iio_device_register(indio_dev);
 	if (ret < 0)
@@ -1721,11 +1721,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);
@@ -1748,7 +1749,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

[PATCH v3 08/10] 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.
This follows modern kernel power-management conventions.

Signed-off-by: Sanjay Chitroda <sanjayembeddedse@gmail.com>
---
Changes in v3:
- Use direct PM runtime calls and drop redundant wrapper along with CONFIG_PM
- Link to v2: 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
- Link to v1: https://lore.kernel.org/all/20260414192045.3598010-1-sanjayembedded@gmail.com/
---
 drivers/iio/accel/mma8452.c | 55 +++++++++++++------------------------
 1 file changed, 19 insertions(+), 36 deletions(-)

diff --git a/drivers/iio/accel/mma8452.c b/drivers/iio/accel/mma8452.c
index 2cd24b1543af..5ab981481599 100644
--- a/drivers/iio/accel/mma8452.c
+++ b/drivers/iio/accel/mma8452.c
@@ -219,34 +219,15 @@ 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);
+	ret = pm_runtime_resume_and_get(dev);
 	if (ret)
 		return ret;
 
@@ -255,9 +236,7 @@ static int mma8452_read(struct mma8452_data *data, __be16 buf[3])
 	if (ret < 0)
 		return ret;
 
-	ret = mma8452_set_runtime_pm_state(data->client, false);
-
-	return ret;
+	return pm_runtime_put_autosuspend(dev);
 }
 
 static ssize_t mma8452_show_int_plus_micros(char *buf, const int (*vals)[2],
@@ -974,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;
 
@@ -981,8 +961,11 @@ 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)
+	if (state)
+		ret = pm_runtime_resume_and_get(dev);
+	else
+		ret = pm_runtime_put_autosuspend(dev);
+	if (ret < 0)
 		return ret;
 
 	switch (dir) {
@@ -1452,10 +1435,15 @@ 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)
+	if (state)
+		ret = pm_runtime_resume_and_get(dev);
+	else
+		ret = pm_runtime_put_autosuspend(dev);
+	if (ret < 0)
 		return ret;
 
 	reg = i2c_smbus_read_byte_data(data->client, MMA8452_CTRL_REG4);
@@ -1738,7 +1726,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));
@@ -1792,13 +1779,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[] = {
 	{ "fxls8471", (kernel_ulong_t)&mma_chip_info_table[fxls8471] },
@@ -1815,7 +1798,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

[PATCH v3 09/10] 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 5ab981481599..3bc53cfdac24 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

[PATCH v3 10/10] 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 v3:
- Following input from Jonathan extended mutex scope for
  IIO_CHAN_INFO_RAW case to include math operation under lock
- Link to v2: https://lore.kernel.org/all/20260422165643.2148195-7-sanjayembedded@gmail.com/
---
 drivers/iio/accel/mma8452.c | 29 +++++++++++------------------
 1 file changed, 11 insertions(+), 18 deletions(-)

diff --git a/drivers/iio/accel/mma8452.c b/drivers/iio/accel/mma8452.c
index 3bc53cfdac24..1370674d71c6 100644
--- a/drivers/iio/accel/mma8452.c
+++ b/drivers/iio/accel/mma8452.c
@@ -18,6 +18,7 @@
  * TODO: orientation events
  */
 
+#include <linux/cleanup.h>
 #include <linux/delay.h>
 #include <linux/i2c.h>
 #include <linux/mod_devicetable.h>
@@ -478,10 +479,9 @@ static int mma8452_read_raw(struct iio_dev *indio_dev,
 		IIO_DEV_ACQUIRE_DIRECT_MODE(indio_dev, claim);
 		if (IIO_DEV_ACQUIRE_FAILED(claim))
 			return -EBUSY;
+		guard(mutex)(&data->lock);
 
-		mutex_lock(&data->lock);
 		ret = mma8452_read(data, buffer);
-		mutex_unlock(&data->lock);
 		if (ret < 0)
 			return ret;
 
@@ -579,36 +579,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)
@@ -1730,9 +1724,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 v3 05/10] iio: accel: mma8452: Use dev_err_probe()

[Joshua Crofts]

On Tue, 5 May 2026 at 19:49, Sanjay Chitroda <sanjayembeddedse@gmail.com> wrote:
>
> From: Sanjay Chitroda <sanjayembeddedse@gmail.com>
>
> dev_err_probe() makes error code handling simpler and handle
> deferred probe nicely (avoid spamming logs).
>
> Signed-off-by: Sanjay Chitroda <sanjayembeddedse@gmail.com>
> ---
>  drivers/iio/accel/mma8452.c | 9 ++++-----
>  1 file changed, 4 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/iio/accel/mma8452.c b/drivers/iio/accel/mma8452.c
> index d227ce3d5f67..b49949792190 100644
> --- a/drivers/iio/accel/mma8452.c
> +++ b/drivers/iio/accel/mma8452.c
> @@ -1548,6 +1548,7 @@ MODULE_DEVICE_TABLE(of, mma8452_dt_ids);
>
>  static int mma8452_probe(struct i2c_client *client)
>  {
> +       struct device *dev = &client->dev;

Stray change, I assume this belongs to patch 7 ("use local struct device") of
your series.

-- 
Kind regards

CJD

Re: [PATCH v3 06/10] iio: accel: mma8452: convert to bulk regulator usage

[Joshua Crofts]

On Tue, 5 May 2026 at 19:49, Sanjay Chitroda <sanjayembeddedse@gmail.com> wrote:
>
> 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>
> ---
>  drivers/iio/accel/mma8452.c | 59 ++++++++++---------------------------
>  1 file changed, 15 insertions(+), 44 deletions(-)
>
> diff --git a/drivers/iio/accel/mma8452.c b/drivers/iio/accel/mma8452.c
> index b49949792190..1c984c708ec3 100644
> --- a/drivers/iio/accel/mma8452.c
> +++ b/drivers/iio/accel/mma8452.c
> @@ -111,8 +111,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 +1569,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);

Since you're using ARRAY_SIZE(), it would be good to add a <linux/array_size.h>
include, as it's currently pulling the macro from elsewhere. Perhaps
run the IWYU
tool to check (any header additions/removals stemming from IWYU would go in a
separate patch however).

-- 
Kind regards

CJD

Re: [PATCH v3 07/10] iio: accel: mma8452: use local struct device

[Joshua Crofts]

On Tue, 5 May 2026 at 19:50, Sanjay Chitroda <sanjayembeddedse@gmail.com> wrote:
> @@ -1552,7 +1552,7 @@ static int mma8452_probe(struct i2c_client *client)
>         struct iio_dev *indio_dev;
>         int ret;

As mentioned, you added the local struct device in patch 5, making
this really confusing (and potentially breaking if only a partial set
of the patches would be picked up). Please add it here.

-- 
Kind regards

CJD

Re: [PATCH v3 05/10] iio: accel: mma8452: Use dev_err_probe()

[Andy Shevchenko]

On Tue, May 05, 2026 at 08:45:32PM +0200, Joshua Crofts wrote:
> On Tue, 5 May 2026 at 19:49, Sanjay Chitroda <sanjayembeddedse@gmail.com> wrote:

...

> >  static int mma8452_probe(struct i2c_client *client)
> >  {
> > +       struct device *dev = &client->dev;
> 
> Stray change, I assume this belongs to patch 7 ("use local struct device") of
> your series.

Not really. It switches to dev_err_probe() that uses it.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v3 03/10] iio: accel: mma8452: cleanup codestyle warning

[Joshua Crofts]

On Tue, 5 May 2026 at 19:48, Sanjay Chitroda <sanjayembeddedse@gmail.com> wrote:
>
> From: Sanjay Chitroda <sanjayembeddedse@gmail.com>
>
> Reported by checkpatch:
> FILE: drivers/iio/accel/mma8452.c
>
> CHECK: Alignment should match open parenthesis
>
> Signed-off-by: Sanjay Chitroda <sanjayembeddedse@gmail.com>
> ---
>  drivers/iio/accel/mma8452.c | 47 +++++++++++++++++++------------------
>  1 file changed, 24 insertions(+), 23 deletions(-)
>
> diff --git a/drivers/iio/accel/mma8452.c b/drivers/iio/accel/mma8452.c
> index 279a9b364886..916631519d3f 100644
> --- a/drivers/iio/accel/mma8452.c
> +++ b/drivers/iio/accel/mma8452.c
> @@ -706,8 +706,8 @@ static int mma8452_set_hp_filter_frequency(struct mma8452_data *data,
>  }
>
>  static int __mma8452_write_raw(struct iio_dev *indio_dev,
> -                            struct iio_chan_spec const *chan,
> -                            int val, int val2, long mask)
> +                              struct iio_chan_spec const *chan,
> +                              int val, int val2, long mask)

This is still not aligned by 1 character, it should be aligned with the first
character after the opening parenthesis.

>  {
>         struct mma8452_data *data = iio_priv(indio_dev);
>         int i, j, ret;
> @@ -788,8 +788,9 @@ static int mma8452_write_raw(struct iio_dev *indio_dev,
>  }
>
>  static int mma8452_get_event_regs(struct mma8452_data *data,
> -               const struct iio_chan_spec *chan, enum iio_event_direction dir,
> -               const struct mma8452_event_regs **ev_reg)
> +                                 const struct iio_chan_spec *chan,
> +                                 enum iio_event_direction dir,
> +                                 const struct mma8452_event_regs **ev_reg)

Same here.

>  {
>         if (!chan)
>                 return -EINVAL;
> @@ -818,11 +819,11 @@ static int mma8452_get_event_regs(struct mma8452_data *data,
>  }
>
>  static int mma8452_read_event_value(struct iio_dev *indio_dev,
> -                              const struct iio_chan_spec *chan,
> -                              enum iio_event_type type,
> -                              enum iio_event_direction dir,
> -                              enum iio_event_info info,
> -                              int *val, int *val2)
> +                                   const struct iio_chan_spec *chan,
> +                                   enum iio_event_type type,
> +                                   enum iio_event_direction dir,
> +                                   enum iio_event_info info,
> +                                   int *val, int *val2)

Same here.

>  {
>         struct mma8452_data *data = iio_priv(indio_dev);
>         int ret, us, power_mode;
> @@ -881,11 +882,11 @@ static int mma8452_read_event_value(struct iio_dev *indio_dev,
>  }
>
>  static int mma8452_write_event_value(struct iio_dev *indio_dev,
> -                               const struct iio_chan_spec *chan,
> -                               enum iio_event_type type,
> -                               enum iio_event_direction dir,
> -                               enum iio_event_info info,
> -                               int val, int val2)
> +                                    const struct iio_chan_spec *chan,
> +                                    enum iio_event_type type,
> +                                    enum iio_event_direction dir,
> +                                    enum iio_event_info info,
> +                                    int val, int val2)

Same here.

The rest is correct though.

-- 
Kind regards

CJD

Re: [PATCH v3 00/10] iio: accel: mma8452: improve coding style, pm and resource cleanup

[Andy Shevchenko]

On Tue, May 05, 2026 at 11:16:30PM +0530, Sanjay Chitroda wrote:

> 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.

...

> Feedback and reviews are very welcome.

Sorry I haven't asked this previously. Is any part of this analysis / commit
messages / et cetera AI assisted?

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v3 05/10] iio: accel: mma8452: Use dev_err_probe()

[Joshua Crofts]

On Wed, 6 May 2026 at 11:22, Andy Shevchenko
<andriy.shevchenko@intel.com> wrote:
>
> On Tue, May 05, 2026 at 08:45:32PM +0200, Joshua Crofts wrote:
> > On Tue, 5 May 2026 at 19:49, Sanjay Chitroda <sanjayembeddedse@gmail.com> wrote:
>
> ...
>
> > >  static int mma8452_probe(struct i2c_client *client)
> > >  {
> > > +       struct device *dev = &client->dev;
> >
> > Stray change, I assume this belongs to patch 7 ("use local struct device") of
> > your series.
>
> Not really. It switches to dev_err_probe() that uses it.

Perhaps I used stray change incorrectly. The series has a patch that
adds instances of struct device, so I'm not sure why this patch is adding
it as well. It took me a solid minute to realize what happened when reviewing
patch 7 of the series, I was ready to say it wouldn't compile (see my
comment in patch 7).

-- 
Kind regards

CJD

Re: [PATCH v3 04/10] iio: accel: mma8452: sort headers alphabetically

[Joshua Crofts]

On Tue, 5 May 2026 at 19:49, Sanjay Chitroda <sanjayembeddedse@gmail.com> wrote:
>
> From: Sanjay Chitroda <sanjayembeddedse@gmail.com>
>
> Sort include headers alphabetically to improve readability.
>
> Signed-off-by: Sanjay Chitroda <sanjayembeddedse@gmail.com>

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

-- 
Kind regards

CJD

Re: [PATCH v3 06/10] iio: accel: mma8452: convert to bulk regulator usage

[Andy Shevchenko]

On Tue, May 05, 2026 at 11:16:36PM +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.

...

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

When touching data structures, always confirm with `pahole` that the chosen
layout suits well. You can also consider doing another patch to rearrange
the members for that. But be careful about the code generation, rearrange of
such a data needs confirmation from both `pahole` and `bloat-o-meter`.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v3 07/10] iio: accel: mma8452: use local struct device

[Andy Shevchenko]

On Tue, May 05, 2026 at 11:16:37PM +0530, Sanjay Chitroda wrote:

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

...

>  	data->chip_info = i2c_get_match_data(client);
>  	if (!data->chip_info)
> -		return dev_err_probe(&client->dev, -ENODEV,
> +		return dev_err_probe(dev, -ENODEV,
>  				     "unknown device model\n");

Now it's a single line.

...

> +	pm_runtime_set_autosuspend_delay(dev,
>  					 MMA8452_AUTO_SUSPEND_DELAY_MS);

Ditto.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v3 05/10] iio: accel: mma8452: Use dev_err_probe()

[Andy Shevchenko]

On Wed, May 06, 2026 at 11:27:12AM +0200, Joshua Crofts wrote:
> On Wed, 6 May 2026 at 11:22, Andy Shevchenko
> <andriy.shevchenko@intel.com> wrote:
> > On Tue, May 05, 2026 at 08:45:32PM +0200, Joshua Crofts wrote:
> > > On Tue, 5 May 2026 at 19:49, Sanjay Chitroda <sanjayembeddedse@gmail.com> wrote:

...

> > > >  static int mma8452_probe(struct i2c_client *client)
> > > >  {
> > > > +       struct device *dev = &client->dev;
> > >
> > > Stray change, I assume this belongs to patch 7 ("use local struct device") of
> > > your series.
> >
> > Not really. It switches to dev_err_probe() that uses it.
> 
> Perhaps I used stray change incorrectly. The series has a patch that
> adds instances of struct device, so I'm not sure why this patch is adding
> it as well.

Because it makes sense here. OTOH, we only change a couple of lines, worth
it having the temporary variable or not is a good question.

> It took me a solid minute to realize what happened when reviewing
> patch 7 of the series, I was ready to say it wouldn't compile (see my
> comment in patch 7).

Yep, but it's okay for patches that have dependencies. These two doesn't seem
like backporting material, they are in the bucket of "cleaning up the things",
which may have dependencies and it's fine.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v3 08/10] iio: accel: mma8452: use pm_ptr() and direct runtime PM calls

[Andy Shevchenko]

On Tue, May 05, 2026 at 11:16:38PM +0530, Sanjay Chitroda wrote:

> 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.
> This follows modern kernel power-management conventions.

...

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

> +

Stray blank line.

>  	int reg, ret;
>  
> -	ret = mma8452_set_runtime_pm_state(data->client, state);
> -	if (ret)
> +	if (state)
> +		ret = pm_runtime_resume_and_get(dev);
> +	else
> +		ret = pm_runtime_put_autosuspend(dev);

Hmm... Do we care about autosuspend returned value? What is its meaning?

> +	if (ret < 0)
>  		return ret;

...

> +static DEFINE_RUNTIME_DEV_PM_OPS(mma8452_pm_ops, mma8452_runtime_suspend,
> +				 mma8452_runtime_resume, NULL);

Use logical split

static DEFINE_RUNTIME_DEV_PM_OPS(mma8452_pm_ops,
				 mma8452_runtime_suspend, mma8452_runtime_resume, NULL);

OR

static DEFINE_RUNTIME_DEV_PM_OPS(mma8452_pm_ops,
				 mma8452_runtime_suspend,
				 mma8452_runtime_resume,
				 NULL);

-- 
With Best Regards,
Andy Shevchenko

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

[Andy Shevchenko]

On Tue, May 05, 2026 at 11:16:40PM +0530, Sanjay Chitroda wrote:

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

...

> static int mma8452_read_raw(struct iio_dev *indio_dev,

>  		IIO_DEV_ACQUIRE_DIRECT_MODE(indio_dev, claim);
>  		if (IIO_DEV_ACQUIRE_FAILED(claim))
>  			return -EBUSY;

+ Blank line here.

> +		guard(mutex)(&data->lock);
>  
> -		mutex_lock(&data->lock);
>  		ret = mma8452_read(data, buffer);
> -		mutex_unlock(&data->lock);

Perhaps okay, also scoped_guard() can be used, but I have no strong opinion.

>  		if (ret < 0)
>  			return ret;


-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v3 02/10] iio: accel: mma8452: switch to non-devm request_threaded_irq()

[Jonathan Cameron]

On Tue,  5 May 2026 23:16:32 +0530
Sanjay Chitroda <sanjayembeddedse@gmail.com> wrote:

> From: Sanjay Chitroda <sanjayembeddedse@gmail.com>
> 
> Avoid using devm_request_threaded_irq() as the driver requires explicit
> error-handling path(s). Using devm_* API together with goto-based
> unwinding breaks the expected LIFO resource release model.
> 
> Add explicit IRQ cleanup in the driver teardown paths to follow kernel
> resource management conventions.
> 
> Fixes: 28e3427824cc ("iio: mma8452: Basic support for transient events.")
For a fixes tag we need to have an explicit issue rather than
it not following best practice.  Patch itself is correct, and if all
else is good and you don't have a specific ordering problem in mind
I'll just drop the fixes tag whilst applying.


> Signed-off-by: Sanjay Chitroda <sanjayembeddedse@gmail.com>
> ---
>  drivers/iio/accel/mma8452.c | 19 ++++++++++++-------
>  1 file changed, 12 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/iio/accel/mma8452.c b/drivers/iio/accel/mma8452.c
> index cefc7cf4bd83..279a9b364886 100644
> --- a/drivers/iio/accel/mma8452.c
> +++ b/drivers/iio/accel/mma8452.c
> @@ -1682,18 +1682,16 @@ static int mma8452_probe(struct i2c_client *client)
>  		goto trigger_cleanup;
>  
>  	if (client->irq) {
> -		ret = devm_request_threaded_irq(&client->dev,
> -						client->irq,
> -						NULL, mma8452_interrupt,
> -						IRQF_TRIGGER_LOW | IRQF_ONESHOT,
> -						client->name, indio_dev);
> +		ret = request_threaded_irq(client->irq, NULL, mma8452_interrupt,
> +					   IRQF_TRIGGER_LOW | IRQF_ONESHOT,
> +					   client->name, indio_dev);
>  		if (ret)
>  			goto buffer_cleanup;
>  	}
>  
>  	ret = pm_runtime_set_active(&client->dev);
>  	if (ret < 0)
> -		goto buffer_cleanup;
> +		goto free_irq;
>  
>  	pm_runtime_enable(&client->dev);
>  	pm_runtime_set_autosuspend_delay(&client->dev,
> @@ -1702,7 +1700,7 @@ static int mma8452_probe(struct i2c_client *client)
>  
>  	ret = iio_device_register(indio_dev);
>  	if (ret < 0)
> -		goto buffer_cleanup;
> +		goto free_irq;
>  
>  	ret = mma8452_set_freefall_mode(data, false);
>  	if (ret < 0)
> @@ -1713,6 +1711,10 @@ static int mma8452_probe(struct i2c_client *client)
>  unregister_device:
>  	iio_device_unregister(indio_dev);
>  
> +free_irq:
> +	if (client->irq)
> +		free_irq(client->irq, indio_dev);
> +
>  buffer_cleanup:
>  	iio_triggered_buffer_cleanup(indio_dev);
>  
> @@ -1738,6 +1740,9 @@ static void mma8452_remove(struct i2c_client *client)
>  	pm_runtime_disable(&client->dev);
>  	pm_runtime_set_suspended(&client->dev);
>  
> +	if (client->irq)
> +		free_irq(client->irq, indio_dev);
> +
>  	iio_triggered_buffer_cleanup(indio_dev);
>  	mma8452_trigger_cleanup(indio_dev);
>  	mma8452_standby(iio_priv(indio_dev));

Re: [PATCH v3 03/10] iio: accel: mma8452: cleanup codestyle warning

[Jonathan Cameron]

On Wed, 6 May 2026 11:24:25 +0200
Joshua Crofts <joshua.crofts1@gmail.com> wrote:

> On Tue, 5 May 2026 at 19:48, Sanjay Chitroda <sanjayembeddedse@gmail.com> wrote:
> >
> > From: Sanjay Chitroda <sanjayembeddedse@gmail.com>
> >
> > Reported by checkpatch:
> > FILE: drivers/iio/accel/mma8452.c
> >
> > CHECK: Alignment should match open parenthesis
> >
> > Signed-off-by: Sanjay Chitroda <sanjayembeddedse@gmail.com>
> > ---
> >  drivers/iio/accel/mma8452.c | 47 +++++++++++++++++++------------------
> >  1 file changed, 24 insertions(+), 23 deletions(-)
> >
> > diff --git a/drivers/iio/accel/mma8452.c b/drivers/iio/accel/mma8452.c
> > index 279a9b364886..916631519d3f 100644
> > --- a/drivers/iio/accel/mma8452.c
> > +++ b/drivers/iio/accel/mma8452.c
> > @@ -706,8 +706,8 @@ static int mma8452_set_hp_filter_frequency(struct mma8452_data *data,
> >  }
> >
> >  static int __mma8452_write_raw(struct iio_dev *indio_dev,
> > -                            struct iio_chan_spec const *chan,
> > -                            int val, int val2, long mask)
> > +                              struct iio_chan_spec const *chan,
> > +                              int val, int val2, long mask)  
> 
> This is still not aligned by 1 character, it should be aligned with the first
> character after the opening parenthesis.

Hi Joshua,

Trap of reading patches in emails is that tabs and space combinations don't
always look correct. Also your email client replaced them all with spaces
to add more confusion ;)

From origin patch

 static int __mma8452_write_raw(struct iio_dev *indio_dev,
-			     struct iio_chan_spec const *chan,
-			     int val, int val2, long mask)
+			       struct iio_chan_spec const *chan,
+			       int val, int val2, long mask)
 {

With the leading chars removed removes one space from top line
but nothing from the following ones as they have leading tabs.

So you get:

static int __mma8452_write_raw(struct iio_dev *indio_dev,
			       struct iio_chan_spec const *chan,
			       int val, int val2, long mask)
{

So this is fine.  I haven't checked the others but guess they are the same.

I frequently end up testing this by hand when I think indents are off
and often turns out I'm wrong!

...

> 
> The rest is correct though.
They all have leading tabs on all lines.

Jonathan

Re: [PATCH v3 08/10] iio: accel: mma8452: use pm_ptr() and direct runtime PM calls

[Jonathan Cameron]

On Tue,  5 May 2026 23:16:38 +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.
> This follows modern kernel power-management conventions.
> 
> Signed-off-by: Sanjay Chitroda <sanjayembeddedse@gmail.com>

Hi Sanjay,

The cleanup you have in here has made what i think are two nasty race
conditions much  more obvious. I think we need to fix those
(and that fix needs to go at the start of this series)

Thanks for you work on this driver - this problem has been
there a long time!

Jonathan

> ---
> Changes in v3:
> - Use direct PM runtime calls and drop redundant wrapper along with CONFIG_PM
> - Link to v2: 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
> - Link to v1: https://lore.kernel.org/all/20260414192045.3598010-1-sanjayembedded@gmail.com/
> ---
>  drivers/iio/accel/mma8452.c | 55 +++++++++++++------------------------
>  1 file changed, 19 insertions(+), 36 deletions(-)
> 
> diff --git a/drivers/iio/accel/mma8452.c b/drivers/iio/accel/mma8452.c
> index 2cd24b1543af..5ab981481599 100644


>  static ssize_t mma8452_show_int_plus_micros(char *buf, const int (*vals)[2],
> @@ -974,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;
>  
> @@ -981,8 +961,11 @@ static int mma8452_write_event_config(struct iio_dev *indio_dev,
>  	if (ret)
>  		return ret;
>  
> -	ret = mma8452_set_runtime_pm_state(data->client, state);
This smells like the same bug as below. 
> -	if (ret)
> +	if (state)
> +		ret = pm_runtime_resume_and_get(dev);
> +	else
> +		ret = pm_runtime_put_autosuspend(dev);
> +	if (ret < 0)
>  		return ret;
>  
>  	switch (dir) {
> @@ -1452,10 +1435,15 @@ 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);

This functions makes me very suspicious and I think has a nasty existing
race condition.  If the auto suspend were to suspend immediately
vddio would be powered down.  Unless they supplies have odd naming on this
device that means the bus would be down before the write that follows.

As such I think this needs s rewrite to makes sure we only call
pm_runtime_put_autosuspend() after the bus writes in this function are done.

I think that is safe to do without testing as all it will do is delay
when the power gets turned off a tiny bit.

> -	if (ret)
> +	if (state)
> +		ret = pm_runtime_resume_and_get(dev);
> +	else
> +		ret = pm_runtime_put_autosuspend(dev);
> +	if (ret < 0)
>  		return ret;
>  
>  	reg = i2c_smbus_read_byte_data(data->client, MMA8452_CTRL_REG4);
> @@ -1738,7 +1726,6 @@ static void mma8452_remove(struct i2c_client *client)
>  	regulator_bulk_disable(ARRAY_SIZE(data->regs), data->regs);
>  }