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

[PATCH v2 0/6] 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 cleanup and improvements to use modern kernel
helper API and coding style for mma8452 accelarator driver.

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

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 (6):
  iio: accel: mma8452: cleanup codestyle warning
  iio: accel: mma8452: sort headers alphabetically
  iio: accel: mma8452: use local struct device
  iio: accel: mma8452: Use dev_err_probe()
  iio: accel: mma8452: use pm_ptr() for dev_pm_ops
  iio: accel: mma8452: use guard() to release mutexes

 drivers/iio/accel/mma8452.c | 161 +++++++++++++++++-------------------
 1 file changed, 76 insertions(+), 85 deletions(-)


base-commit: eade2b843d9b1f668fc1775f15611bb0a1999cd9
-- 
2.34.1

[PATCH v2 1/6] 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 15172ba2972c..6d9091f696cf 100644
--- a/drivers/iio/accel/mma8452.c
+++ b/drivers/iio/accel/mma8452.c
@@ -704,8 +704,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;
@@ -786,8 +786,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;
@@ -816,11 +817,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;
@@ -879,11 +880,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;
@@ -953,8 +954,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;
 
@@ -1191,7 +1191,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);
 
@@ -1514,8 +1514,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);
@@ -1645,8 +1646,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;
 
@@ -1654,8 +1655,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 v2 2/6] 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>
---
 drivers/iio/accel/mma8452.c | 16 +++++++++-------
 1 file changed, 9 insertions(+), 7 deletions(-)

diff --git a/drivers/iio/accel/mma8452.c b/drivers/iio/accel/mma8452.c
index 6d9091f696cf..c54c0ea05ac1 100644
--- a/drivers/iio/accel/mma8452.c
+++ b/drivers/iio/accel/mma8452.c
@@ -18,21 +18,23 @@
  * 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/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 v2 3/6] 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 | 51 ++++++++++++++++++++-----------------
 1 file changed, 27 insertions(+), 24 deletions(-)

diff --git a/drivers/iio/accel/mma8452.c b/drivers/iio/accel/mma8452.c
index c54c0ea05ac1..99f7763b84ac 100644
--- a/drivers/iio/accel/mma8452.c
+++ b/drivers/iio/accel/mma8452.c
@@ -225,13 +225,14 @@ static int mma8452_set_runtime_pm_state(struct i2c_client *client, bool on)
 {
 #ifdef CONFIG_PM
 	int ret;
+	struct device *dev = &client->dev;
 
 	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,
+		dev_err(dev,
 			"failed to change power state to %d\n", on);
 
 		return ret;
@@ -1548,10 +1549,11 @@ MODULE_DEVICE_TABLE(of, mma8452_dt_ids);
 static int mma8452_probe(struct i2c_client *client)
 {
 	struct mma8452_data *data;
+	struct device *dev = &client->dev;
 	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;
 
@@ -1561,32 +1563,32 @@ 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;
 
-	data->vdd_reg = devm_regulator_get(&client->dev, "vdd");
+	data->vdd_reg = devm_regulator_get(dev, "vdd");
 	if (IS_ERR(data->vdd_reg))
-		return dev_err_probe(&client->dev, PTR_ERR(data->vdd_reg),
+		return dev_err_probe(dev, PTR_ERR(data->vdd_reg),
 				     "failed to get VDD regulator!\n");
 
-	data->vddio_reg = devm_regulator_get(&client->dev, "vddio");
+	data->vddio_reg = devm_regulator_get(dev, "vddio");
 	if (IS_ERR(data->vddio_reg))
-		return dev_err_probe(&client->dev, PTR_ERR(data->vddio_reg),
+		return dev_err_probe(dev, PTR_ERR(data->vddio_reg),
 				     "failed to get VDDIO regulator!\n");
 
 	ret = regulator_enable(data->vdd_reg);
 	if (ret) {
-		dev_err(&client->dev, "failed to enable VDD regulator!\n");
+		dev_err(dev, "failed to enable VDD regulator!\n");
 		return ret;
 	}
 
 	ret = regulator_enable(data->vddio_reg);
 	if (ret) {
-		dev_err(&client->dev, "failed to enable VDDIO regulator!\n");
+		dev_err(dev, "failed to enable VDDIO regulator!\n");
 		goto disable_regulator_vdd;
 	}
 
@@ -1609,7 +1611,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);
@@ -1642,10 +1644,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,
@@ -1653,7 +1655,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,
@@ -1683,7 +1685,7 @@ static int mma8452_probe(struct i2c_client *client)
 		goto trigger_cleanup;
 
 	if (client->irq) {
-		ret = devm_request_threaded_irq(&client->dev,
+		ret = devm_request_threaded_irq(dev,
 						client->irq,
 						NULL, mma8452_interrupt,
 						IRQF_TRIGGER_LOW | IRQF_ONESHOT,
@@ -1692,14 +1694,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 buffer_cleanup;
 
-	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)
@@ -1732,12 +1734,13 @@ static int mma8452_probe(struct i2c_client *client)
 static void mma8452_remove(struct i2c_client *client)
 {
 	struct iio_dev *indio_dev = i2c_get_clientdata(client);
+	struct device *dev = &client->dev;
 	struct mma8452_data *data = iio_priv(indio_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);
 
 	iio_triggered_buffer_cleanup(indio_dev);
 	mma8452_trigger_cleanup(indio_dev);
@@ -1758,7 +1761,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 v2 4/6] 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 | 11 ++++-------
 1 file changed, 4 insertions(+), 7 deletions(-)

diff --git a/drivers/iio/accel/mma8452.c b/drivers/iio/accel/mma8452.c
index 99f7763b84ac..d1ae2bf37409 100644
--- a/drivers/iio/accel/mma8452.c
+++ b/drivers/iio/accel/mma8452.c
@@ -1563,8 +1563,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, -ENODEV, "unknown device model\n");
 
 	ret = iio_read_mount_matrix(dev, &data->orientation);
 	if (ret)
@@ -1581,14 +1580,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(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(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 v2 5/6] iio: accel: mma8452: use pm_ptr() for dev_pm_ops

[Sanjay Chitroda]

From: Sanjay Chitroda <sanjayembeddedse@gmail.com>

Use pm_ptr() and DEFINE_RUNTIME_DEV_PM_OPS() so the PM ops pointer is
automatically handle when CONFIG_PM is enabled/disabled. This follows
modern kernel power-management conventions.

Signed-off-by: Sanjay Chitroda <sanjayembeddedse@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 | 11 +++--------
 1 file changed, 3 insertions(+), 8 deletions(-)

diff --git a/drivers/iio/accel/mma8452.c b/drivers/iio/accel/mma8452.c
index d1ae2bf37409..9983f76a8bcd 100644
--- a/drivers/iio/accel/mma8452.c
+++ b/drivers/iio/accel/mma8452.c
@@ -1747,7 +1747,6 @@ static void mma8452_remove(struct i2c_client *client)
 	regulator_disable(data->vdd_reg);
 }
 
-#ifdef CONFIG_PM
 static int mma8452_runtime_suspend(struct device *dev)
 {
 	struct iio_dev *indio_dev = i2c_get_clientdata(to_i2c_client(dev));
@@ -1815,13 +1814,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] },
@@ -1838,7 +1833,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 v2 6/6] 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>
---
 drivers/iio/accel/mma8452.c | 31 ++++++++++++-------------------
 1 file changed, 12 insertions(+), 19 deletions(-)

diff --git a/drivers/iio/accel/mma8452.c b/drivers/iio/accel/mma8452.c
index 9983f76a8bcd..1c284bdcc44a 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>
@@ -500,9 +501,8 @@ static int mma8452_read_raw(struct iio_dev *indio_dev,
 		if (!iio_device_claim_direct(indio_dev))
 			return -EBUSY;
 
-		mutex_lock(&data->lock);
-		ret = mma8452_read(data, buffer);
-		mutex_unlock(&data->lock);
+		scoped_guard(mutex, &data->lock)
+			ret = mma8452_read(data, buffer);
 		iio_device_release_direct(indio_dev);
 		if (ret < 0)
 			return ret;
@@ -600,36 +600,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)
@@ -1753,9 +1747,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 v2 2/6] iio: accel: mma8452: sort headers alphabetically

[Andy Shevchenko]

On Wed, Apr 22, 2026 at 10:26:39PM +0530, Sanjay Chitroda wrote:

> Sort include headers alphabetically to improve readability.

...

> -#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/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>

For this driver the regulator/consumer is a regular header and can be moved to
be between property.h and types.h.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2 3/6] iio: accel: mma8452: use local struct device

[Andy Shevchenko]

On Wed, Apr 22, 2026 at 10:26:40PM +0530, Sanjay Chitroda wrote:

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

...

> static int mma8452_set_runtime_pm_state(struct i2c_client *client, bool on)
>  {
>  #ifdef CONFIG_PM

So, this seems to be a missed in the PM patch.

>  	int ret;
> +	struct device *dev = &client->dev;

Keep it in reversed xmas tree order.

>  	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,
> +		dev_err(dev,
>  			"failed to change power state to %d\n", on);
>  
>  		return ret;

>  }

...

>  static int mma8452_probe(struct i2c_client *client)
>  {
>  	struct mma8452_data *data;
> +	struct device *dev = &client->dev;

Same here, reversed xmas tree order.

>  	struct iio_dev *indio_dev;
>  	int ret;

...

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

...

>  	ret = regulator_enable(data->vdd_reg);
>  	if (ret) {
> -		dev_err(&client->dev, "failed to enable VDD regulator!\n");
> +		dev_err(dev, "failed to enable VDD regulator!\n");
>  		return ret;

Convert this to dev_err_probe() before doing this patch. In that patch you
introduce temporary dev variable already. Here will use in the other places.

>  	}
>  
>  	ret = regulator_enable(data->vddio_reg);
>  	if (ret) {
> -		dev_err(&client->dev, "failed to enable VDDIO regulator!\n");
> +		dev_err(dev, "failed to enable VDDIO regulator!\n");
>  		goto disable_regulator_vdd;

Ditto.

>  	}

...

> +	pm_runtime_set_autosuspend_delay(dev,
>  					 MMA8452_AUTO_SUSPEND_DELAY_MS);

Now a single line.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2 4/6] iio: accel: mma8452: Use dev_err_probe()

[Andy Shevchenko]

On Wed, Apr 22, 2026 at 10:26:41PM +0530, Sanjay Chitroda wrote:

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

This patch should go earlier than dev temporary variable one.
And dev needs to be introduced here.

...

>  	ret = regulator_enable(data->vddio_reg);
>  	if (ret) {
> -		dev_err(dev, "failed to enable VDDIO regulator!\n");
> +		dev_err_probe(dev, ret, "failed to enable VDDIO regulator!\n");
>  		goto disable_regulator_vdd;

Before doing these patches, please fix the mess with the devm/non-devm ordering.
There shouldn't be goto after any devm calls.

>  	}

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2 5/6] iio: accel: mma8452: use pm_ptr() for dev_pm_ops

[Andy Shevchenko]

On Wed, Apr 22, 2026 at 10:26:42PM +0530, Sanjay Chitroda wrote:

> Use pm_ptr() and DEFINE_RUNTIME_DEV_PM_OPS() so the PM ops pointer is
> automatically handle when CONFIG_PM is enabled/disabled. This follows
> modern kernel power-management conventions.

After this patch there are still CONFIG_PM ifdeffery left, why?

-- 
With Best Regards,
Andy Shevchenko

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

[Andy Shevchenko]

On Wed, Apr 22, 2026 at 10:26:43PM +0530, Sanjay Chitroda wrote:

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

This one seems okay, but since it's not the majority of the changes that can
go, I won't give any tag right now, let's see v3 (but wait at least a couple of
days to give chances to others to review v2 in case I missed something).


-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2 2/6] iio: accel: mma8452: sort headers alphabetically

[Sanjay Chitroda]

On 23 April 2026 1:00:10 am IST, Andy Shevchenko <andriy.shevchenko@intel.com> wrote:
>On Wed, Apr 22, 2026 at 10:26:39PM +0530, Sanjay Chitroda wrote:
>
>> Sort include headers alphabetically to improve readability.
>
>...
>
>> -#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/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>
>
>For this driver the regulator/consumer is a regular header and can be moved to
>be between property.h and types.h.
>

Andy, Thank you for your review.
Along with alphabetic sort, I have also grouped different header(s), at top linux direct header, then iio headers and at last regulator headers for readability.

If you suggest, I would include this in commit description.

Thanks,
Sanjay Chitroda