[PATCH v3 0/8] iio: adc: ti-ads1015: modernize resource management

[PATCH v3 0/8] iio: adc: ti-ads1015: modernize resource management

[Archit Anant]

This series modernizes the ti-ads1015 by transitioning it
to a fully devm_ architecture.

By moving resource allocation, device registration, and runtime power
management to the devm_ infrastructure, the teardown sequence is now
guaranteed to execute safely in the reverse order of initialization.
This allows for the complete removal of the ads1015_remove() function,
eliminating boilerplate code and reducing the risk of future resource
leaks.

Also the driver's configuration logic has been updated to use
the modern FIELD_PREP() and FIELD_GET() macros, allowing the removal
of all custom _SHIFT macros and a redundant helper function.

Additionally, housekeeping patches are included to ensure header
includes remain alphabetically sorted, dev_err_probe() is used
consistently, and PM macros are updated to modern standards.

Changes in v3:
- Added Patch 7 to replace all custom _SHIFT macros with FIELD_PREP
  and FIELD_GET as requested by Jonathan Cameron.
- Added Patch 8 to inline ads1015_set_conv_mode() directly at call
  sites, allowing for the removal of the final _SHIFT macro.

Changes in v2:
- Split the monolithic v1 patch into a 6-patch series to isolate
  housekeeping, bug fixes, and API modernizations for easier review
  and backporting.
- Patch 1: Added to modernize PM ops and remove #ifdef CONFIG_PM.
- Patch 2: Added to fix a pre-existing PM leak on probe failure
  identified by Jonathan Cameron.
- Patch 3: Extracted header sorting into a prerequisite patch.
- Patch 4 & 5: Extracted the introduction of the local 'dev' pointer
  and dev_err_probe() conversions, suggested by Jonathan Cameron.
- Patch 6: Now contains only the devm_ conversions and the removal
  of ads1015_remove().

Archit Anant (8):
  iio: adc: ti-ads1015: use DEFINE_RUNTIME_DEV_PM_OPS()
  iio: adc: ti-ads1015: fix PM leak on probe failure
  iio: adc: ti-ads1015: sort headers alphabetically
  iio: adc: ti-ads1015: use local device pointer in probe
  iio: adc: ti-ads1015: use dev_err_probe() for error handling
  iio: adc: ti-ads1015: convert to fully managed resources
  iio: adc: ti-ads1015: replace custom shift macros with FIELD_PREP/GET
  iio: adc: ti-ads1015: inline ads1015_set_conv_mode()

 drivers/iio/adc/ti-ads1015.c | 176 ++++++++++++++++-------------------
 1 file changed, 81 insertions(+), 95 deletions(-)

-- 
2.39.5

[PATCH v3 1/8] iio: adc: ti-ads1015: use DEFINE_RUNTIME_DEV_PM_OPS()

[Archit Anant]

Replace the deprecated SET_RUNTIME_PM_OPS() with the modern
DEFINE_RUNTIME_DEV_PM_OPS() macro. This allows for the removal of the
macro automatically handles dropping unused functions when PM is
disabled.

Update the driver struct to use pm_ptr() to avoid unused variable
warnings.

Signed-off-by: Archit Anant <architanant5@gmail.com>
---
 drivers/iio/adc/ti-ads1015.c | 12 +++++-------
 1 file changed, 5 insertions(+), 7 deletions(-)

diff --git a/drivers/iio/adc/ti-ads1015.c b/drivers/iio/adc/ti-ads1015.c
index 8a272af69f7d..9cd620b71429 100644
--- a/drivers/iio/adc/ti-ads1015.c
+++ b/drivers/iio/adc/ti-ads1015.c
@@ -1066,7 +1066,6 @@ static void ads1015_remove(struct i2c_client *client)
 			 ERR_PTR(ret));
 }
 
-#ifdef CONFIG_PM
 static int ads1015_runtime_suspend(struct device *dev)
 {
 	struct iio_dev *indio_dev = i2c_get_clientdata(to_i2c_client(dev));
@@ -1087,12 +1086,11 @@ static int ads1015_runtime_resume(struct device *dev)
 
 	return ret;
 }
-#endif
 
-static const struct dev_pm_ops ads1015_pm_ops = {
-	SET_RUNTIME_PM_OPS(ads1015_runtime_suspend,
-			   ads1015_runtime_resume, NULL)
-};
+static DEFINE_RUNTIME_DEV_PM_OPS(ads1015_pm_ops,
+				 ads1015_runtime_suspend,
+				 ads1015_runtime_resume,
+				 NULL);
 
 static const struct ads1015_chip_data ads1015_data = {
 	.channels	= ads1015_channels,
@@ -1147,7 +1145,7 @@ static struct i2c_driver ads1015_driver = {
 	.driver = {
 		.name = ADS1015_DRV_NAME,
 		.of_match_table = ads1015_of_match,
-		.pm = &ads1015_pm_ops,
+		.pm = pm_ptr(&ads1015_pm_ops),
 	},
 	.probe		= ads1015_probe,
 	.remove		= ads1015_remove,
-- 
2.39.5

[PATCH v3 2/8] iio: adc: ti-ads1015: fix PM leak on probe failure

[Archit Anant]

If iio_device_register() fails during probe(), the runtime PM
subsystem is left enabled. This leads to a resource leak upon failure.

Add the missing pm_runtime_disable() and pm_runtime_set_suspended()
calls to the probe() error path to ensure proper cleanup.

Reported-by: Jonathan Cameron <jic23@kernel.org>
Link: https://lore.kernel.org/linux-iio/20260718225515.7e49a458@jic23-huawei/
Signed-off-by: Archit Anant <architanant5@gmail.com>
---
 drivers/iio/adc/ti-ads1015.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/iio/adc/ti-ads1015.c b/drivers/iio/adc/ti-ads1015.c
index 9cd620b71429..3a14fdab56d8 100644
--- a/drivers/iio/adc/ti-ads1015.c
+++ b/drivers/iio/adc/ti-ads1015.c
@@ -1042,6 +1042,8 @@ static int ads1015_probe(struct i2c_client *client)
 	ret = iio_device_register(indio_dev);
 	if (ret < 0) {
 		dev_err(&client->dev, "Failed to register IIO device\n");
+		pm_runtime_disable(&client->dev);
+		pm_runtime_set_suspended(&client->dev);
 		return ret;
 	}
 
-- 
2.39.5

[PATCH v3 3/8] iio: adc: ti-ads1015: sort headers alphabetically

[Archit Anant]

Reorder include headers alphabetically to comply with the preferred
coding style and improve maintainability.

No functional changes.

Signed-off-by: Archit Anant <architanant5@gmail.com>
---
 drivers/iio/adc/ti-ads1015.c | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/drivers/iio/adc/ti-ads1015.c b/drivers/iio/adc/ti-ads1015.c
index 3a14fdab56d8..293fdd49381f 100644
--- a/drivers/iio/adc/ti-ads1015.c
+++ b/drivers/iio/adc/ti-ads1015.c
@@ -11,24 +11,24 @@
  *	* 0x4B - ADDR connected to SCL
  */
 
-#include <linux/module.h>
 #include <linux/cleanup.h>
+#include <linux/delay.h>
+#include <linux/i2c.h>
 #include <linux/init.h>
 #include <linux/irq.h>
-#include <linux/i2c.h>
+#include <linux/module.h>
+#include <linux/mutex.h>
+#include <linux/pm_runtime.h>
 #include <linux/property.h>
 #include <linux/regmap.h>
-#include <linux/pm_runtime.h>
-#include <linux/mutex.h>
-#include <linux/delay.h>
 
+#include <linux/iio/buffer.h>
+#include <linux/iio/events.h>
 #include <linux/iio/iio.h>
-#include <linux/iio/types.h>
 #include <linux/iio/sysfs.h>
-#include <linux/iio/events.h>
-#include <linux/iio/buffer.h>
-#include <linux/iio/triggered_buffer.h>
 #include <linux/iio/trigger_consumer.h>
+#include <linux/iio/triggered_buffer.h>
+#include <linux/iio/types.h>
 
 #define ADS1015_DRV_NAME "ads1015"
 
-- 
2.39.5

[PATCH v3 4/8] iio: adc: ti-ads1015: use local device pointer in probe

[Archit Anant]

Introduce a local device pointer 'dev' in ads1015_probe to
avoid accessing &client->dev repeatedly.

Signed-off-by: Archit Anant <architanant5@gmail.com>
---
 drivers/iio/adc/ti-ads1015.c | 21 +++++++++++----------
 1 file changed, 11 insertions(+), 10 deletions(-)

diff --git a/drivers/iio/adc/ti-ads1015.c b/drivers/iio/adc/ti-ads1015.c
index 293fdd49381f..c88904d7190d 100644
--- a/drivers/iio/adc/ti-ads1015.c
+++ b/drivers/iio/adc/ti-ads1015.c
@@ -933,6 +933,7 @@ static int ads1015_set_conv_mode(struct ads1015_data *data, int mode)
 static int ads1015_probe(struct i2c_client *client)
 {
 	const struct ads1015_chip_data *chip;
+	struct device *dev = &client->dev;
 	struct iio_dev *indio_dev;
 	struct ads1015_data *data;
 	int ret;
@@ -940,9 +941,9 @@ static int ads1015_probe(struct i2c_client *client)
 
 	chip = i2c_get_match_data(client);
 	if (!chip)
-		return dev_err_probe(&client->dev, -EINVAL, "Unknown chip\n");
+		return dev_err_probe(dev, -EINVAL, "Unknown chip\n");
 
-	indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*data));
+	indio_dev = devm_iio_device_alloc(dev, sizeof(*data));
 	if (!indio_dev)
 		return -ENOMEM;
 
@@ -982,7 +983,7 @@ static int ads1015_probe(struct i2c_client *client)
 		return PTR_ERR(data->regmap);
 	}
 
-	ret = devm_iio_triggered_buffer_setup(&client->dev, indio_dev, NULL,
+	ret = devm_iio_triggered_buffer_setup(dev, indio_dev, NULL,
 					      ads1015_trigger_handler,
 					      &ads1015_buffer_setup_ops);
 	if (ret < 0) {
@@ -1018,7 +1019,7 @@ static int ads1015_probe(struct i2c_client *client)
 		if (ret)
 			return ret;
 
-		ret = devm_request_threaded_irq(&client->dev, client->irq,
+		ret = devm_request_threaded_irq(dev, client->irq,
 						NULL, ads1015_event_handler,
 						irq_trig | IRQF_ONESHOT,
 						client->name, indio_dev);
@@ -1032,18 +1033,18 @@ static int ads1015_probe(struct i2c_client *client)
 
 	data->conv_invalid = true;
 
-	ret = pm_runtime_set_active(&client->dev);
+	ret = pm_runtime_set_active(dev);
 	if (ret)
 		return ret;
-	pm_runtime_set_autosuspend_delay(&client->dev, ADS1015_SLEEP_DELAY_MS);
-	pm_runtime_use_autosuspend(&client->dev);
-	pm_runtime_enable(&client->dev);
+	pm_runtime_set_autosuspend_delay(dev, ADS1015_SLEEP_DELAY_MS);
+	pm_runtime_use_autosuspend(dev);
+	pm_runtime_enable(dev);
 
 	ret = iio_device_register(indio_dev);
 	if (ret < 0) {
 		dev_err(&client->dev, "Failed to register IIO device\n");
-		pm_runtime_disable(&client->dev);
-		pm_runtime_set_suspended(&client->dev);
+		pm_runtime_disable(dev);
+		pm_runtime_set_suspended(dev);
 		return ret;
 	}
 
-- 
2.39.5

[PATCH v3 5/8] iio: adc: ti-ads1015: use dev_err_probe() for error handling

[Archit Anant]

Simplify the error handling paths in ads1015_probe() and
ads1015_client_get_channels_config()  by converting dev_err()
calls that are immediately followed by a return statement
over to the modern dev_err_probe() helper.

Note: This patch takes advantage of the previously introduced
local 'dev' pointer when converting the legacy
dev_err(&client->dev, ...) calls, avoiding unnecessary
code churn.

Suggested-by: Jonathan Cameron <jic23@kernel.org>
Signed-off-by: Archit Anant <architanant5@gmail.com>
---
 drivers/iio/adc/ti-ads1015.c | 32 ++++++++++++++------------------
 1 file changed, 14 insertions(+), 18 deletions(-)

diff --git a/drivers/iio/adc/ti-ads1015.c b/drivers/iio/adc/ti-ads1015.c
index c88904d7190d..895bce7859d2 100644
--- a/drivers/iio/adc/ti-ads1015.c
+++ b/drivers/iio/adc/ti-ads1015.c
@@ -883,18 +883,16 @@ static int ads1015_client_get_channels_config(struct i2c_client *client)
 
 		if (!fwnode_property_read_u32(node, "ti,gain", &pval)) {
 			pga = pval;
-			if (pga > 5) {
-				dev_err(dev, "invalid gain on %pfw\n", node);
-				return -EINVAL;
-			}
+			if (pga > 5)
+				return dev_err_probe(dev, -EINVAL,
+						     "invalid gain on %pfw\n", node);
 		}
 
 		if (!fwnode_property_read_u32(node, "ti,datarate", &pval)) {
 			data_rate = pval;
-			if (data_rate > 7) {
-				dev_err(dev, "invalid data_rate on %pfw\n", node);
-				return -EINVAL;
-			}
+			if (data_rate > 7)
+				return dev_err_probe(dev, -EINVAL,
+						     "invalid data_rate on %pfw\n", node);
 		}
 
 		data->channel_data[channel].pga = pga;
@@ -978,18 +976,16 @@ static int ads1015_probe(struct i2c_client *client)
 	data->regmap = devm_regmap_init_i2c(client, chip->has_comparator ?
 					    &ads1015_regmap_config :
 					    &tla2024_regmap_config);
-	if (IS_ERR(data->regmap)) {
-		dev_err(&client->dev, "Failed to allocate register map\n");
-		return PTR_ERR(data->regmap);
-	}
+	if (IS_ERR(data->regmap))
+		return dev_err_probe(dev, PTR_ERR(data->regmap),
+				     "Failed to allocate register map\n");
 
 	ret = devm_iio_triggered_buffer_setup(dev, indio_dev, NULL,
 					      ads1015_trigger_handler,
 					      &ads1015_buffer_setup_ops);
-	if (ret < 0) {
-		dev_err(&client->dev, "iio triggered buffer setup failed\n");
-		return ret;
-	}
+	if (ret < 0)
+		return dev_err_probe(dev, ret,
+				     "iio triggered buffer setup failed\n");
 
 	if (client->irq && chip->has_comparator) {
 		unsigned long irq_trig = irq_get_trigger_type(client->irq);
@@ -1042,10 +1038,10 @@ static int ads1015_probe(struct i2c_client *client)
 
 	ret = iio_device_register(indio_dev);
 	if (ret < 0) {
-		dev_err(&client->dev, "Failed to register IIO device\n");
 		pm_runtime_disable(dev);
 		pm_runtime_set_suspended(dev);
-		return ret;
+		return dev_err_probe(dev, ret,
+				     "Failed to register IIO device\n");
 	}
 
 	return 0;
-- 
2.39.5

[PATCH v3 6/8] iio: adc: ti-ads1015: convert to fully managed resources

[Archit Anant]

Refactor the driver to use devm_ allocations and power
management, allowing for the complete removal of the
ads1015_remove()

Key changes:
- Use devm_add_action_or_reset() to ensure the ADC is safely powered
down upon driver removal.
- Move to devm_pm_runtime_set_active_enabled() to manage the runtime
PM lifecycle.
- Convert iio_device_register() and mutex_init() to their devm_
variants.
- Remove the thus obsolete ads1015_remove() function.

Signed-off-by: Archit Anant <architanant5@gmail.com>
---
 drivers/iio/adc/ti-ads1015.c | 43 ++++++++++++++----------------------
 1 file changed, 16 insertions(+), 27 deletions(-)

diff --git a/drivers/iio/adc/ti-ads1015.c b/drivers/iio/adc/ti-ads1015.c
index 895bce7859d2..f92ec5941c47 100644
--- a/drivers/iio/adc/ti-ads1015.c
+++ b/drivers/iio/adc/ti-ads1015.c
@@ -928,6 +928,11 @@ static int ads1015_set_conv_mode(struct ads1015_data *data, int mode)
 				  mode << ADS1015_CFG_MOD_SHIFT);
 }
 
+static void ads1015_power_off(void *st)
+{
+	ads1015_set_conv_mode(st, ADS1015_SINGLESHOT);
+}
+
 static int ads1015_probe(struct i2c_client *client)
 {
 	const struct ads1015_chip_data *chip;
@@ -948,7 +953,9 @@ static int ads1015_probe(struct i2c_client *client)
 	data = iio_priv(indio_dev);
 	i2c_set_clientdata(client, indio_dev);
 
-	mutex_init(&data->lock);
+	ret = devm_mutex_init(dev, &data->lock);
+	if (ret)
+		return ret;
 
 	indio_dev->name = ADS1015_DRV_NAME;
 	indio_dev->modes = INDIO_DIRECT_MODE;
@@ -1029,42 +1036,25 @@ static int ads1015_probe(struct i2c_client *client)
 
 	data->conv_invalid = true;
 
-	ret = pm_runtime_set_active(dev);
+	ret = devm_add_action_or_reset(dev, ads1015_power_off, data);
 	if (ret)
 		return ret;
+
 	pm_runtime_set_autosuspend_delay(dev, ADS1015_SLEEP_DELAY_MS);
 	pm_runtime_use_autosuspend(dev);
-	pm_runtime_enable(dev);
 
-	ret = iio_device_register(indio_dev);
-	if (ret < 0) {
-		pm_runtime_disable(dev);
-		pm_runtime_set_suspended(dev);
+	ret = devm_pm_runtime_set_active_enabled(dev);
+	if (ret)
+		return ret;
+
+	ret = devm_iio_device_register(dev, indio_dev);
+	if (ret)
 		return dev_err_probe(dev, ret,
 				     "Failed to register IIO device\n");
-	}
 
 	return 0;
 }
 
-static void ads1015_remove(struct i2c_client *client)
-{
-	struct iio_dev *indio_dev = i2c_get_clientdata(client);
-	struct ads1015_data *data = iio_priv(indio_dev);
-	int ret;
-
-	iio_device_unregister(indio_dev);
-
-	pm_runtime_disable(&client->dev);
-	pm_runtime_set_suspended(&client->dev);
-
-	/* power down single shot mode */
-	ret = ads1015_set_conv_mode(data, ADS1015_SINGLESHOT);
-	if (ret)
-		dev_warn(&client->dev, "Failed to power down (%pe)\n",
-			 ERR_PTR(ret));
-}
-
 static int ads1015_runtime_suspend(struct device *dev)
 {
 	struct iio_dev *indio_dev = i2c_get_clientdata(to_i2c_client(dev));
@@ -1147,7 +1137,6 @@ static struct i2c_driver ads1015_driver = {
 		.pm = pm_ptr(&ads1015_pm_ops),
 	},
 	.probe		= ads1015_probe,
-	.remove		= ads1015_remove,
 	.id_table	= ads1015_id,
 };
 
-- 
2.39.5

[PATCH v3 7/8] iio: adc: ti-ads1015: replace custom shift macros with FIELD_PREP/GET

[Archit Anant]

The driver currently uses custom _SHIFT macros and manual bitwise
operations to read and write to the configuration register.

Modernize the bitwise logic by replacing all manual shifts with the
standard FIELD_PREP() and FIELD_GET() macros from <linux/bitfield.h>.
This improves readability and prevents potential bitwise errors.

Remove the now-unused _SHIFT macros from the top of the file.

Suggested-by: Jonathan Cameron <jic23@kernel.org>
Signed-off-by: Archit Anant <architanant5@gmail.com>
---
 drivers/iio/adc/ti-ads1015.c | 39 ++++++++++++++++--------------------
 1 file changed, 17 insertions(+), 22 deletions(-)

diff --git a/drivers/iio/adc/ti-ads1015.c b/drivers/iio/adc/ti-ads1015.c
index f92ec5941c47..3c78d85321e0 100644
--- a/drivers/iio/adc/ti-ads1015.c
+++ b/drivers/iio/adc/ti-ads1015.c
@@ -11,6 +11,7 @@
  *	* 0x4B - ADDR connected to SCL
  */
 
+#include <linux/bitfield.h>
 #include <linux/cleanup.h>
 #include <linux/delay.h>
 #include <linux/i2c.h>
@@ -39,14 +40,7 @@
 #define ADS1015_LO_THRESH_REG	0x02
 #define ADS1015_HI_THRESH_REG	0x03
 
-#define ADS1015_CFG_COMP_QUE_SHIFT	0
-#define ADS1015_CFG_COMP_LAT_SHIFT	2
-#define ADS1015_CFG_COMP_POL_SHIFT	3
-#define ADS1015_CFG_COMP_MODE_SHIFT	4
-#define ADS1015_CFG_DR_SHIFT	5
 #define ADS1015_CFG_MOD_SHIFT	8
-#define ADS1015_CFG_PGA_SHIFT	9
-#define ADS1015_CFG_MUX_SHIFT	12
 
 #define ADS1015_CFG_COMP_QUE_MASK	GENMASK(1, 0)
 #define ADS1015_CFG_COMP_LAT_MASK	BIT(2)
@@ -408,15 +402,15 @@ int ads1015_get_adc_result(struct ads1015_data *data, int chan, int *val)
 	dr = data->channel_data[chan].data_rate;
 	mask = ADS1015_CFG_MUX_MASK | ADS1015_CFG_PGA_MASK |
 		ADS1015_CFG_DR_MASK;
-	cfg = chan << ADS1015_CFG_MUX_SHIFT | pga << ADS1015_CFG_PGA_SHIFT |
-		dr << ADS1015_CFG_DR_SHIFT;
+	cfg = FIELD_PREP(ADS1015_CFG_MUX_MASK, chan) | FIELD_PREP(ADS1015_CFG_PGA_MASK, pga) |
+		FIELD_PREP(ADS1015_CFG_DR_MASK, dr);
 
 	if (ads1015_event_channel_enabled(data)) {
 		mask |= ADS1015_CFG_COMP_QUE_MASK | ADS1015_CFG_COMP_MODE_MASK;
-		cfg |= data->thresh_data[chan].comp_queue <<
-				ADS1015_CFG_COMP_QUE_SHIFT |
-			data->comp_mode <<
-				ADS1015_CFG_COMP_MODE_SHIFT;
+		cfg |= FIELD_PREP(ADS1015_CFG_COMP_QUE_MASK,
+				  data->thresh_data[chan].comp_queue) |
+			FIELD_PREP(ADS1015_CFG_COMP_MODE_MASK,
+				   data->comp_mode);
 	}
 
 	cfg = (old & ~mask) | (cfg & mask);
@@ -427,7 +421,7 @@ int ads1015_get_adc_result(struct ads1015_data *data, int chan, int *val)
 		data->conv_invalid = true;
 	}
 	if (data->conv_invalid) {
-		dr_old = (old & ADS1015_CFG_DR_MASK) >> ADS1015_CFG_DR_SHIFT;
+		dr_old = FIELD_GET(ADS1015_CFG_DR_MASK, old);
 		conv_time = DIV_ROUND_UP(USEC_PER_SEC, data_rate[dr_old]);
 		conv_time += DIV_ROUND_UP(USEC_PER_SEC, data_rate[dr]);
 		conv_time += conv_time / 10; /* 10% internal clock inaccuracy */
@@ -758,8 +752,8 @@ static int ads1015_disable_event_config(struct ads1015_data *data,
 
 	ret = regmap_update_bits(data->regmap, ADS1015_CFG_REG,
 				ADS1015_CFG_COMP_QUE_MASK,
-				ADS1015_CFG_COMP_DISABLE <<
-					ADS1015_CFG_COMP_QUE_SHIFT);
+				FIELD_PREP(ADS1015_CFG_COMP_QUE_MASK,
+					   ADS1015_CFG_COMP_DISABLE));
 	if (ret)
 		return ret;
 
@@ -999,19 +993,20 @@ static int ads1015_probe(struct i2c_client *client)
 		unsigned int cfg_comp_mask = ADS1015_CFG_COMP_QUE_MASK |
 			ADS1015_CFG_COMP_LAT_MASK | ADS1015_CFG_COMP_POL_MASK;
 		unsigned int cfg_comp =
-			ADS1015_CFG_COMP_DISABLE << ADS1015_CFG_COMP_QUE_SHIFT |
-			1 << ADS1015_CFG_COMP_LAT_SHIFT;
+			FIELD_PREP(ADS1015_CFG_COMP_QUE_MASK,
+				   ADS1015_CFG_COMP_DISABLE) |
+			FIELD_PREP(ADS1015_CFG_COMP_LAT_MASK, 1);
 
 		switch (irq_trig) {
 		case IRQF_TRIGGER_FALLING:
 		case IRQF_TRIGGER_LOW:
-			cfg_comp |= ADS1015_CFG_COMP_POL_LOW <<
-					ADS1015_CFG_COMP_POL_SHIFT;
+			cfg_comp |= FIELD_PREP(ADS1015_CFG_COMP_POL_MASK,
+					       ADS1015_CFG_COMP_POL_LOW);
 			break;
 		case IRQF_TRIGGER_HIGH:
 		case IRQF_TRIGGER_RISING:
-			cfg_comp |= ADS1015_CFG_COMP_POL_HIGH <<
-					ADS1015_CFG_COMP_POL_SHIFT;
+			cfg_comp |= FIELD_PREP(ADS1015_CFG_COMP_POL_MASK,
+					       ADS1015_CFG_COMP_POL_HIGH);
 			break;
 		default:
 			return -EINVAL;
-- 
2.39.5

[PATCH v3 8/8] iio: adc: ti-ads1015: inline ads1015_set_conv_mode()

[Archit Anant]

The ads1015_set_conv_mode() helper function is a single-line wrapper
around regmap_update_bits().

With the transition to the FIELD_PREP() macro, the raw register updates
are now self-documenting and easy to read. Remove the wrapper function
and inline the regmap calls directly at the call sites.

This also allows for the removal of the final custom shift macro,
ADS1015_CFG_MOD_SHIFT.

Suggested-by: Jonathan Cameron <jic23@kernel.org>
Signed-off-by: Archit Anant <architanant5@gmail.com>
---
 drivers/iio/adc/ti-ads1015.c | 31 ++++++++++++++++++-------------
 1 file changed, 18 insertions(+), 13 deletions(-)

diff --git a/drivers/iio/adc/ti-ads1015.c b/drivers/iio/adc/ti-ads1015.c
index 3c78d85321e0..e6b6cf61489f 100644
--- a/drivers/iio/adc/ti-ads1015.c
+++ b/drivers/iio/adc/ti-ads1015.c
@@ -40,8 +40,6 @@
 #define ADS1015_LO_THRESH_REG	0x02
 #define ADS1015_HI_THRESH_REG	0x03
 
-#define ADS1015_CFG_MOD_SHIFT	8
-
 #define ADS1015_CFG_COMP_QUE_MASK	GENMASK(1, 0)
 #define ADS1015_CFG_COMP_LAT_MASK	BIT(2)
 #define ADS1015_CFG_COMP_POL_MASK	BIT(3)
@@ -915,16 +913,14 @@ static void ads1015_get_channels_config(struct i2c_client *client)
 	}
 }
 
-static int ads1015_set_conv_mode(struct ads1015_data *data, int mode)
+static void ads1015_power_off(void *data)
 {
-	return regmap_update_bits(data->regmap, ADS1015_CFG_REG,
-				  ADS1015_CFG_MOD_MASK,
-				  mode << ADS1015_CFG_MOD_SHIFT);
-}
+	struct ads1015_data *st = data;
 
-static void ads1015_power_off(void *st)
-{
-	ads1015_set_conv_mode(st, ADS1015_SINGLESHOT);
+	regmap_update_bits(st->regmap, ADS1015_CFG_REG,
+			   ADS1015_CFG_MOD_MASK,
+			   FIELD_PREP(ADS1015_CFG_MOD_MASK,
+				      ADS1015_SINGLESHOT));
 }
 
 static int ads1015_probe(struct i2c_client *client)
@@ -1025,7 +1021,10 @@ static int ads1015_probe(struct i2c_client *client)
 			return ret;
 	}
 
-	ret = ads1015_set_conv_mode(data, ADS1015_CONTINUOUS);
+	ret = regmap_update_bits(data->regmap, ADS1015_CFG_REG,
+				  ADS1015_CFG_MOD_MASK,
+				  FIELD_PREP(ADS1015_CFG_MOD_MASK,
+					     ADS1015_CONTINUOUS));
 	if (ret)
 		return ret;
 
@@ -1055,7 +1054,10 @@ static int ads1015_runtime_suspend(struct device *dev)
 	struct iio_dev *indio_dev = i2c_get_clientdata(to_i2c_client(dev));
 	struct ads1015_data *data = iio_priv(indio_dev);
 
-	return ads1015_set_conv_mode(data, ADS1015_SINGLESHOT);
+	return regmap_update_bits(data->regmap, ADS1015_CFG_REG,
+				  ADS1015_CFG_MOD_MASK,
+				  FIELD_PREP(ADS1015_CFG_MOD_MASK,
+					     ADS1015_SINGLESHOT));
 }
 
 static int ads1015_runtime_resume(struct device *dev)
@@ -1064,7 +1066,10 @@ static int ads1015_runtime_resume(struct device *dev)
 	struct ads1015_data *data = iio_priv(indio_dev);
 	int ret;
 
-	ret = ads1015_set_conv_mode(data, ADS1015_CONTINUOUS);
+	ret = regmap_update_bits(data->regmap, ADS1015_CFG_REG,
+				  ADS1015_CFG_MOD_MASK,
+				  FIELD_PREP(ADS1015_CFG_MOD_MASK,
+					     ADS1015_CONTINUOUS));
 	if (!ret)
 		data->conv_invalid = true;
 
-- 
2.39.5

Re: [PATCH v3 2/8] iio: adc: ti-ads1015: fix PM leak on probe failure

[Andy Shevchenko]

On Wed, Aug 12, 2026 at 10:14:27PM +0530, Archit Anant wrote:
> If iio_device_register() fails during probe(), the runtime PM
> subsystem is left enabled. This leads to a resource leak upon failure.
> 
> Add the missing pm_runtime_disable() and pm_runtime_set_suspended()
> calls to the probe() error path to ensure proper cleanup.

> Reported-by: Jonathan Cameron <jic23@kernel.org>

Was it public email?

> Link: https://lore.kernel.org/linux-iio/20260718225515.7e49a458@jic23-huawei/

Ah, this should be Closes: tag instead of Link.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v3 5/8] iio: adc: ti-ads1015: use dev_err_probe() for error handling

[Andy Shevchenko]

On Wed, Aug 12, 2026 at 10:14:30PM +0530, Archit Anant wrote:
> Simplify the error handling paths in ads1015_probe() and
> ads1015_client_get_channels_config()  by converting dev_err()
> calls that are immediately followed by a return statement
> over to the modern dev_err_probe() helper.
> 
> Note: This patch takes advantage of the previously introduced
> local 'dev' pointer when converting the legacy
> dev_err(&client->dev, ...) calls, avoiding unnecessary
> code churn.

...

>  	ret = iio_device_register(indio_dev);
>  	if (ret < 0) {
> -		dev_err(&client->dev, "Failed to register IIO device\n");
>  		pm_runtime_disable(dev);
>  		pm_runtime_set_suspended(dev);
> -		return ret;

This shows that runtime PM calls were put to the wrong place in the previous
patch. Just move them to be up, then this will be naturally accepted change.

> +		return dev_err_probe(dev, ret,
> +				     "Failed to register IIO device\n");
>  	}

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v3 6/8] iio: adc: ti-ads1015: convert to fully managed resources

[Andy Shevchenko]

On Wed, Aug 12, 2026 at 10:14:31PM +0530, Archit Anant wrote:
> Refactor the driver to use devm_ allocations and power
> management, allowing for the complete removal of the
> ads1015_remove()

...

> -	ret = iio_device_register(indio_dev);
> -	if (ret < 0) {

> +	ret = devm_pm_runtime_set_active_enabled(dev);
> +	if (ret)

There are more places with unneeded ' < 0' comparisons, can you add an
intermediate patch to drop those? With that being done, this, in particular,
will have less churn.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v3 7/8] iio: adc: ti-ads1015: replace custom shift macros with FIELD_PREP/GET

[Andy Shevchenko]

On Wed, Aug 12, 2026 at 10:14:32PM +0530, Archit Anant wrote:
> The driver currently uses custom _SHIFT macros and manual bitwise
> operations to read and write to the configuration register.
> 
> Modernize the bitwise logic by replacing all manual shifts with the
> standard FIELD_PREP() and FIELD_GET() macros from <linux/bitfield.h>.
> This improves readability and prevents potential bitwise errors.
> 
> Remove the now-unused _SHIFT macros from the top of the file.

...

>  	dr = data->channel_data[chan].data_rate;
>  	mask = ADS1015_CFG_MUX_MASK | ADS1015_CFG_PGA_MASK |
>  		ADS1015_CFG_DR_MASK;
> -	cfg = chan << ADS1015_CFG_MUX_SHIFT | pga << ADS1015_CFG_PGA_SHIFT |
> -		dr << ADS1015_CFG_DR_SHIFT;
> +	cfg = FIELD_PREP(ADS1015_CFG_MUX_MASK, chan) | FIELD_PREP(ADS1015_CFG_PGA_MASK, pga) |
> +		FIELD_PREP(ADS1015_CFG_DR_MASK, dr);

Wrong indentation. Please, make sure in the lines you touched the indentation
is also correct.

...

>  		unsigned int cfg_comp =
> -			ADS1015_CFG_COMP_DISABLE << ADS1015_CFG_COMP_QUE_SHIFT |
> -			1 << ADS1015_CFG_COMP_LAT_SHIFT;
> +			FIELD_PREP(ADS1015_CFG_COMP_QUE_MASK,
> +				   ADS1015_CFG_COMP_DISABLE) |
> +			FIELD_PREP(ADS1015_CFG_COMP_LAT_MASK, 1);

It might be better to keep a longer line.

		unsigned int cfg_comp =
			FIELD_PREP(ADS1015_CFG_COMP_QUE_MASK, ADS1015_CFG_COMP_DISABLE) |
			FIELD_PREP(ADS1015_CFG_COMP_LAT_MASK, 1);

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v3 8/8] iio: adc: ti-ads1015: inline ads1015_set_conv_mode()

[Andy Shevchenko]

On Wed, Aug 12, 2026 at 10:14:33PM +0530, Archit Anant wrote:
> The ads1015_set_conv_mode() helper function is a single-line wrapper
> around regmap_update_bits().
> 
> With the transition to the FIELD_PREP() macro, the raw register updates
> are now self-documenting and easy to read. Remove the wrapper function
> and inline the regmap calls directly at the call sites.
> 
> This also allows for the removal of the final custom shift macro,
> ADS1015_CFG_MOD_SHIFT.

...

> -static int ads1015_set_conv_mode(struct ads1015_data *data, int mode)
> +static void ads1015_power_off(void *data)
>  {
> -	return regmap_update_bits(data->regmap, ADS1015_CFG_REG,
> -				  ADS1015_CFG_MOD_MASK,
> -				  mode << ADS1015_CFG_MOD_SHIFT);
> -}
> +	struct ads1015_data *st = data;
>  
> -static void ads1015_power_off(void *st)
> -{
> -	ads1015_set_conv_mode(st, ADS1015_SINGLESHOT);
> +	regmap_update_bits(st->regmap, ADS1015_CFG_REG,
> +			   ADS1015_CFG_MOD_MASK,
> +			   FIELD_PREP(ADS1015_CFG_MOD_MASK,
> +				      ADS1015_SINGLESHOT));
>  }

Wouldn't be better to have it as

static void ads1015_power_off(void *data)
{
	unsigned int val = FIELD_PREP(ADS1015_CFG_MOD_MASK, ADS1015_SINGLESHOT);
	struct regmap *map = data->regmap;

	regmap_update_bits(map, ADS1015_CFG_REG, ADS1015_CFG_MOD_MASK, val);
}

In lieu of this, think if the similar approach can be applied elsewhere in this
driver.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v3 2/8] iio: adc: ti-ads1015: fix PM leak on probe failure

[Jonathan Cameron]

On Thu, 13 Aug 2026 11:51:28 +0300
Andy Shevchenko <andriy.shevchenko@intel.com> wrote:

> On Wed, Aug 12, 2026 at 10:14:27PM +0530, Archit Anant wrote:
> > If iio_device_register() fails during probe(), the runtime PM
> > subsystem is left enabled. This leads to a resource leak upon failure.
> > 
> > Add the missing pm_runtime_disable() and pm_runtime_set_suspended()
> > calls to the probe() error path to ensure proper cleanup.  
> 
> > Reported-by: Jonathan Cameron <jic23@kernel.org>  
> 
> Was it public email?
> 
> > Link: https://lore.kernel.org/linux-iio/20260718225515.7e49a458@jic23-huawei/  
> 
> Ah, this should be Closes: tag instead of Link.
> 

I never checked but this is already fixed in the iio tree and
the relevant devm rework done.  :(  So much going on at the moment
in IIO I didn't recall this one at all!

https://lore.kernel.org/all/20260529183708.17819-1-sozdayvek@gmail.com/

Please rebase the remainder of the series.

Jonathan

Re: [PATCH v3 2/8] iio: adc: ti-ads1015: fix PM leak on probe failure

[Archit Anant]

On Sat, Aug 22, 2026 at 6:08 AM Jonathan Cameron <jic23@kernel.org> wrote:
>
> On Thu, 13 Aug 2026 11:51:28 +0300
> Andy Shevchenko <andriy.shevchenko@intel.com> wrote:
>
> > On Wed, Aug 12, 2026 at 10:14:27PM +0530, Archit Anant wrote:
> > > If iio_device_register() fails during probe(), the runtime PM
> > > subsystem is left enabled. This leads to a resource leak upon failure.
> > >
> > > Add the missing pm_runtime_disable() and pm_runtime_set_suspended()
> > > calls to the probe() error path to ensure proper cleanup.
> >
> > > Reported-by: Jonathan Cameron <jic23@kernel.org>
> >
> > Was it public email?
> >
> > > Link: https://lore.kernel.org/linux-iio/20260718225515.7e49a458@jic23-huawei/
> >
> > Ah, this should be Closes: tag instead of Link.
> >
>
> I never checked but this is already fixed in the iio tree and
> the relevant devm rework done.  :(  So much going on at the moment
> in IIO I didn't recall this one at all!
>
> https://lore.kernel.org/all/20260529183708.17819-1-sozdayvek@gmail.com/
>
> Please rebase the remainder of the series.

Oh, I missed that as well, nvm. I'll rebase the rest!

-- 
Sincerely,
Archit Anant