[PATCH v5 00/10] iio: adc: ti-ads1015: modernize resource management and bitfield logic

[PATCH v5 00/10] iio: adc: ti-ads1015: modernize resource management and bitfield logic

[Archit Anant]

This series modernizes the ti-ads1015 driver by updating its runtime PM
handling, error handling, resource management, and register bitfield
operations.

This series builds on top of Stepan Ionichev's devm error path fix [1] and:
- Replaces SET_RUNTIME_PM_OPS() with DEFINE_RUNTIME_DEV_PM_OPS().
- Sorts the driver headers alphabetically.
- Returns -ENODATA when match data is missing.
- Uses a local device pointer in probe.
- Converts suitable error paths to dev_err_probe().
- Drops unneeded '< 0' error checks for functions returning 0 or error.
- Converts mutex initialization to devm_mutex_init().
- Transitions runtime PM setup to devm_pm_runtime_set_active_enabled().
- Inlines the redundant ads1015_set_conv_mode() helper.
- Replaces custom shift macros with FIELD_PREP()/FIELD_GET().

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

---
v5:
- Reordered patches: inlined ads1015_set_conv_mode() before applying
  FIELD_PREP() to completely eliminate code churn.
- Extracted FIELD_PREP() evaluations into local variables for
  regmap_update_bits() calls to ensure clean line lengths and readability.
- Added a patch to return -ENODATA when i2c_get_match_data() fails, as
  requested by Andy Shevchenko.
- Split devm_mutex_init() into its own patch to decouple it from the
  runtime PM modernization, as requested by Andy Shevchenko.

v4: https://lore.kernel.org/linux-iio/20260827172100.39176-1-architanant5@gmail.com/

Archit Anant (10):
  iio: adc: ti-ads1015: use DEFINE_RUNTIME_DEV_PM_OPS()
  iio: adc: ti-ads1015: sort headers alphabetically
  iio: adc: ti-ads1015: return -ENODATA when match data is missing
  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: drop unneeded < 0 error checks
  iio: adc: ti-ads1015: use devm_mutex_init()
  iio: adc: ti-ads1015: use managed runtime PM setup
  iio: adc: ti-ads1015: inline ads1015_set_conv_mode()
  iio: adc: ti-ads1015: replace custom shift macros with FIELD_PREP/GET

 drivers/iio/adc/ti-ads1015.c | 160 +++++++++++++++++------------------
 1 file changed, 76 insertions(+), 84 deletions(-)

-- 
2.39.5

[PATCH v5 01/10] 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 0fbfa4e499aa..93a165c5416a 100644
--- a/drivers/iio/adc/ti-ads1015.c
+++ b/drivers/iio/adc/ti-ads1015.c
@@ -1059,7 +1059,6 @@ static int ads1015_probe(struct i2c_client *client)
 	return devm_iio_device_register(&client->dev, indio_dev);
 }
 
-#ifdef CONFIG_PM
 static int ads1015_runtime_suspend(struct device *dev)
 {
 	struct iio_dev *indio_dev = i2c_get_clientdata(to_i2c_client(dev));
@@ -1080,12 +1079,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,
@@ -1140,7 +1138,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,
 	.id_table	= ads1015_id,
-- 
2.39.5

[PATCH v5 02/10] 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 93a165c5416a..ef03b74bda3e 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

Re: [PATCH v5 02/10] iio: adc: ti-ads1015: sort headers alphabetically

[Joshua Crofts]

On Wed, 30 Sep 2026 22:41:56 +0530
Archit Anant <architanant5@gmail.com> wrote:

> 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 93a165c5416a..ef03b74bda3e 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>

Small nit, but I'd drop iio/types.h as it is included by iio/iio.h
(No need to send a new version though, this can be addressed in the
future).

>  #define ADS1015_DRV_NAME "ads1015"
>  



-- 
Kind regards,
Joshua Crofts

[PATCH v5 03/10] iio: adc: ti-ads1015: return -ENODATA when match data is missing

[Archit Anant]

When i2c_get_match_data() returns NULL, it indicates that driver match
data is absent rather than an invalid argument being provided.

Return -ENODATA instead of -EINVAL to more accurately reflect the failure.

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

diff --git a/drivers/iio/adc/ti-ads1015.c b/drivers/iio/adc/ti-ads1015.c
index ef03b74bda3e..9479f110448d 100644
--- a/drivers/iio/adc/ti-ads1015.c
+++ b/drivers/iio/adc/ti-ads1015.c
@@ -951,7 +951,7 @@ 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(&client->dev, -ENODATA, "Unknown chip\n");
 
 	indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*data));
 	if (!indio_dev)
-- 
2.39.5

Re: [PATCH v5 03/10] iio: adc: ti-ads1015: return -ENODATA when match data is missing

[Andy Shevchenko]

On Wed, Sep 30, 2026 at 10:41:57PM +0530, Archit Anant wrote:
> When i2c_get_match_data() returns NULL, it indicates that driver match
> data is absent rather than an invalid argument being provided.
> 
> Return -ENODATA instead of -EINVAL to more accurately reflect the failure.

If you arrange this patch after the next one, you may reduce the churn by
switching to local 'dev' variable at the same time. It might be even not
needed to describe as it's just a small detail that doesn't change anything
in the behaviour of the code.

But no need to resend just for that.

-- 
With Best Regards,
Andy Shevchenko

[PATCH v5 04/10] 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 9479f110448d..85280200d050 100644
--- a/drivers/iio/adc/ti-ads1015.c
+++ b/drivers/iio/adc/ti-ads1015.c
@@ -944,6 +944,7 @@ static void ads1015_power_down(void *p)
 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;
@@ -951,9 +952,9 @@ static int ads1015_probe(struct i2c_client *client)
 
 	chip = i2c_get_match_data(client);
 	if (!chip)
-		return dev_err_probe(&client->dev, -ENODATA, "Unknown chip\n");
+		return dev_err_probe(dev, -ENODATA, "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;
 
@@ -993,7 +994,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) {
@@ -1029,7 +1030,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);
@@ -1041,22 +1042,22 @@ static int ads1015_probe(struct i2c_client *client)
 	if (ret)
 		return ret;
 
-	ret = devm_add_action_or_reset(&client->dev, ads1015_power_down, data);
+	ret = devm_add_action_or_reset(dev, ads1015_power_down, data);
 	if (ret)
 		return ret;
 
 	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);
-	ret = devm_pm_runtime_enable(&client->dev);
+	pm_runtime_set_autosuspend_delay(dev, ADS1015_SLEEP_DELAY_MS);
+	pm_runtime_use_autosuspend(dev);
+	ret = devm_pm_runtime_enable(dev);
 	if (ret)
 		return ret;
 
-	return devm_iio_device_register(&client->dev, indio_dev);
+	return devm_iio_device_register(dev, indio_dev);
 }
 
 static int ads1015_runtime_suspend(struct device *dev)
-- 
2.39.5

[PATCH v5 05/10] 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 | 28 ++++++++++++----------------
 1 file changed, 12 insertions(+), 16 deletions(-)

diff --git a/drivers/iio/adc/ti-ads1015.c b/drivers/iio/adc/ti-ads1015.c
index 85280200d050..34d3d31b5ddd 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;
@@ -989,18 +987,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);
-- 
2.39.5

Re: [PATCH v5 05/10] iio: adc: ti-ads1015: use dev_err_probe() for error handling

[Joshua Crofts]

On Wed, 30 Sep 2026 22:41:59 +0530
Archit Anant <architanant5@gmail.com> 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.

This paragraph should be removed, no need to mention this.

-- 
Kind regards,
Joshua Crofts

[PATCH v5 06/10] iio: adc: ti-ads1015: drop unneeded < 0 error checks

[Archit Anant]

Several functions in the driver only return 0 on success
or a negative error code on failure.

Change the error checking to align with standard kernel
coding style.

Suggested-by: Andy Shevchenko <andriy.shevchenko@intel.com>
Signed-off-by: Archit Anant <architanant5@gmail.com>
---
 drivers/iio/adc/ti-ads1015.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/iio/adc/ti-ads1015.c b/drivers/iio/adc/ti-ads1015.c
index 34d3d31b5ddd..6637c42e0506 100644
--- a/drivers/iio/adc/ti-ads1015.c
+++ b/drivers/iio/adc/ti-ads1015.c
@@ -539,7 +539,7 @@ static int __ads1015_read_info_raw(struct ads1015_data *data,
 		return -EBUSY;
 
 	ret = ads1015_set_power_state(data, true);
-	if (ret < 0)
+	if (ret)
 		return ret;
 
 	ret = ads1015_get_adc_result(data, chan->address, val);
@@ -727,7 +727,7 @@ static int ads1015_enable_event_config(struct ads1015_data *data,
 		return ret;
 
 	ret = ads1015_set_power_state(data, true);
-	if (ret < 0)
+	if (ret)
 		return ret;
 
 	ads1015_event_channel_enable(data, chan->address, comp_mode);
@@ -994,7 +994,7 @@ static int ads1015_probe(struct i2c_client *client)
 	ret = devm_iio_triggered_buffer_setup(dev, indio_dev, NULL,
 					      ads1015_trigger_handler,
 					      &ads1015_buffer_setup_ops);
-	if (ret < 0)
+	if (ret)
 		return dev_err_probe(dev, ret,
 				     "iio triggered buffer setup failed\n");
 
-- 
2.39.5

[PATCH v5 07/10] iio: adc: ti-ads1015: use devm_mutex_init()

[Archit Anant]

Convert mutex_init() to devm_mutex_init(). This ensures that the
mutex is automatically destroyed when the device is unbound, and
simplifies error handling.

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

diff --git a/drivers/iio/adc/ti-ads1015.c b/drivers/iio/adc/ti-ads1015.c
index 6637c42e0506..a790f694a1a9 100644
--- a/drivers/iio/adc/ti-ads1015.c
+++ b/drivers/iio/adc/ti-ads1015.c
@@ -959,7 +959,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;
-- 
2.39.5

[PATCH v5 08/10] iio: adc: ti-ads1015: use managed runtime PM setup

[Archit Anant]

Use devm_pm_runtime_set_active_enabled() to combine setting the device
active and enabling runtime PM.

Previously, probe explicitly called pm_runtime_set_active() followed by
devm_pm_runtime_enable(). The managed helper changes the teardown
semantics by associating runtime PM enablement with the device-managed
lifetime.

The runtime suspend and resume callbacks only update the conversion-mode
bits through regmap. They do not manipulate runtime-PM usage counts or
other shared resources such as regulators or clocks, so this change does
not introduce an additional runtime-PM reference or resource-balance
operation in this driver.

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

diff --git a/drivers/iio/adc/ti-ads1015.c b/drivers/iio/adc/ti-ads1015.c
index a790f694a1a9..9312822218df 100644
--- a/drivers/iio/adc/ti-ads1015.c
+++ b/drivers/iio/adc/ti-ads1015.c
@@ -1046,12 +1046,9 @@ static int ads1015_probe(struct i2c_client *client)
 
 	data->conv_invalid = true;
 
-	ret = pm_runtime_set_active(dev);
-	if (ret)
-		return ret;
 	pm_runtime_set_autosuspend_delay(dev, ADS1015_SLEEP_DELAY_MS);
 	pm_runtime_use_autosuspend(dev);
-	ret = devm_pm_runtime_enable(dev);
+	ret = devm_pm_runtime_set_active_enabled(dev);
 	if (ret)
 		return ret;
 
-- 
2.39.5

[PATCH v5 09/10] 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().

Since the function is simple and only called during initialization and
power management state transitions, the wrapper is unnecessary. Remove
it and inline the regmap_update_bits() logic directly at the call sites
to improve code readability and traceabilty.

Where applicable, introduce a local 'map' pointer to keep the
inlined function calls concise.

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

diff --git a/drivers/iio/adc/ti-ads1015.c b/drivers/iio/adc/ti-ads1015.c
index 9312822218df..2e8f89d16586 100644
--- a/drivers/iio/adc/ti-ads1015.c
+++ b/drivers/iio/adc/ti-ads1015.c
@@ -744,6 +744,7 @@ static int ads1015_enable_event_config(struct ads1015_data *data,
 static int ads1015_disable_event_config(struct ads1015_data *data,
 	const struct iio_chan_spec *chan, int comp_mode)
 {
+	struct regmap *map = data->regmap;
 	int ret;
 
 	if (!ads1015_event_channel_enabled(data))
@@ -756,7 +757,7 @@ static int ads1015_disable_event_config(struct ads1015_data *data,
 			comp_mode == ADS1015_CFG_COMP_MODE_WINDOW)
 		return 0;
 
-	ret = regmap_update_bits(data->regmap, ADS1015_CFG_REG,
+	ret = regmap_update_bits(map, ADS1015_CFG_REG,
 				ADS1015_CFG_COMP_QUE_MASK,
 				ADS1015_CFG_COMP_DISABLE <<
 					ADS1015_CFG_COMP_QUE_SHIFT);
@@ -921,19 +922,16 @@ static void ads1015_get_channels_config(struct i2c_client *client)
 	}
 }
 
-static int ads1015_set_conv_mode(struct ads1015_data *data, int mode)
-{
-	return regmap_update_bits(data->regmap, ADS1015_CFG_REG,
-				  ADS1015_CFG_MOD_MASK,
-				  mode << ADS1015_CFG_MOD_SHIFT);
-}
-
 static void ads1015_power_down(void *p)
 {
 	struct ads1015_data *data = p;
+	struct regmap *map = data->regmap;
 	int ret;
 
-	ret = ads1015_set_conv_mode(data, ADS1015_SINGLESHOT);
+	ret = regmap_update_bits(map, ADS1015_CFG_REG,
+				 ADS1015_CFG_MOD_MASK,
+				 ADS1015_SINGLESHOT << ADS1015_CFG_MOD_SHIFT);
+
 	if (ret)
 		dev_warn(regmap_get_device(data->regmap),
 			 "Failed to power down (%pe)\n", ERR_PTR(ret));
@@ -1036,7 +1034,9 @@ 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,
+				 ADS1015_CONTINUOUS << ADS1015_CFG_MOD_SHIFT);
 	if (ret)
 		return ret;
 
@@ -1059,17 +1059,23 @@ 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);
+	struct regmap *map = data->regmap;
 
-	return ads1015_set_conv_mode(data, ADS1015_SINGLESHOT);
+	return regmap_update_bits(map, ADS1015_CFG_REG,
+				  ADS1015_CFG_MOD_MASK,
+				  ADS1015_SINGLESHOT << ADS1015_CFG_MOD_SHIFT);
 }
 
 static int ads1015_runtime_resume(struct device *dev)
 {
 	struct iio_dev *indio_dev = i2c_get_clientdata(to_i2c_client(dev));
 	struct ads1015_data *data = iio_priv(indio_dev);
+	struct regmap *map = data->regmap;
 	int ret;
 
-	ret = ads1015_set_conv_mode(data, ADS1015_CONTINUOUS);
+	ret = regmap_update_bits(map, ADS1015_CFG_REG,
+				 ADS1015_CFG_MOD_MASK,
+				 ADS1015_CONTINUOUS << ADS1015_CFG_MOD_SHIFT);
 	if (!ret)
 		data->conv_invalid = true;
 
-- 
2.39.5

Re: [PATCH v5 09/10] iio: adc: ti-ads1015: inline ads1015_set_conv_mode()

[Andy Shevchenko]

On Wed, Sep 30, 2026 at 10:42:03PM +0530, Archit Anant wrote:
> The ads1015_set_conv_mode() helper function is a single-line wrapper
> around regmap_update_bits().
> 
> Since the function is simple and only called during initialization and
> power management state transitions, the wrapper is unnecessary. Remove
> it and inline the regmap_update_bits() logic directly at the call sites
> to improve code readability and traceabilty.
> 
> Where applicable, introduce a local 'map' pointer to keep the
> inlined function calls concise.

...

> -	ret = regmap_update_bits(data->regmap, ADS1015_CFG_REG,
> +	ret = regmap_update_bits(map, ADS1015_CFG_REG,
>  				ADS1015_CFG_COMP_QUE_MASK,
>  				ADS1015_CFG_COMP_DISABLE <<
>  					ADS1015_CFG_COMP_QUE_SHIFT);

The indentation is broken even in the original code.

>  static void ads1015_power_down(void *p)
>  {
>  	struct ads1015_data *data = p;
> +	struct regmap *map = data->regmap;
>  	int ret;
>  
> -	ret = ads1015_set_conv_mode(data, ADS1015_SINGLESHOT);
> +	ret = regmap_update_bits(map, ADS1015_CFG_REG,
> +				 ADS1015_CFG_MOD_MASK,
> +				 ADS1015_SINGLESHOT << ADS1015_CFG_MOD_SHIFT);

>  	if (ret)
>  		dev_warn(regmap_get_device(data->regmap),

At some point (may be even in this patch) it makes sense to switch to use 'map'
in all applicable cases.

>  			 "Failed to power down (%pe)\n", ERR_PTR(ret));

...

> static int ads1015_probe(struct i2c_client *client)

>  
> -	ret = ads1015_set_conv_mode(data, ADS1015_CONTINUOUS);
> +	ret = regmap_update_bits(data->regmap, ADS1015_CFG_REG,

Can you also use 'map' here?

> +				 ADS1015_CFG_MOD_MASK,
> +				 ADS1015_CONTINUOUS << ADS1015_CFG_MOD_SHIFT);
>  	if (ret)
>  		return ret;

...

> +	ret = regmap_update_bits(map, ADS1015_CFG_REG,
> +				 ADS1015_CFG_MOD_MASK,

This is now perfectly a single line, same for the cases above.

> +				 ADS1015_CONTINUOUS << ADS1015_CFG_MOD_SHIFT);
>  	if (!ret)
>  		data->conv_invalid = true;

-- 
With Best Regards,
Andy Shevchenko

[PATCH v5 10/10] 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 | 68 ++++++++++++++++--------------------
 1 file changed, 30 insertions(+), 38 deletions(-)

diff --git a/drivers/iio/adc/ti-ads1015.c b/drivers/iio/adc/ti-ads1015.c
index 2e8f89d16586..cf94e9e6a6d2 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,15 +40,6 @@
 #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)
 #define ADS1015_CFG_COMP_POL_MASK	BIT(3)
@@ -408,15 +400,16 @@ 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 +420,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 */
@@ -745,6 +738,7 @@ static int ads1015_disable_event_config(struct ads1015_data *data,
 	const struct iio_chan_spec *chan, int comp_mode)
 {
 	struct regmap *map = data->regmap;
+	unsigned int val;
 	int ret;
 
 	if (!ads1015_event_channel_enabled(data))
@@ -757,10 +751,8 @@ static int ads1015_disable_event_config(struct ads1015_data *data,
 			comp_mode == ADS1015_CFG_COMP_MODE_WINDOW)
 		return 0;
 
-	ret = regmap_update_bits(map, ADS1015_CFG_REG,
-				ADS1015_CFG_COMP_QUE_MASK,
-				ADS1015_CFG_COMP_DISABLE <<
-					ADS1015_CFG_COMP_QUE_SHIFT);
+	val = FIELD_PREP(ADS1015_CFG_COMP_QUE_MASK, ADS1015_CFG_COMP_DISABLE);
+	ret = regmap_update_bits(map, ADS1015_CFG_REG, ADS1015_CFG_COMP_QUE_MASK, val);
 	if (ret)
 		return ret;
 
@@ -926,11 +918,11 @@ static void ads1015_power_down(void *p)
 {
 	struct ads1015_data *data = p;
 	struct regmap *map = data->regmap;
+	unsigned int val;
 	int ret;
 
-	ret = regmap_update_bits(map, ADS1015_CFG_REG,
-				 ADS1015_CFG_MOD_MASK,
-				 ADS1015_SINGLESHOT << ADS1015_CFG_MOD_SHIFT);
+	val = FIELD_PREP(ADS1015_CFG_MOD_MASK, ADS1015_SINGLESHOT);
+	ret = regmap_update_bits(map, ADS1015_CFG_REG, ADS1015_CFG_MOD_MASK, val);
 
 	if (ret)
 		dev_warn(regmap_get_device(data->regmap),
@@ -943,6 +935,7 @@ static int ads1015_probe(struct i2c_client *client)
 	struct device *dev = &client->dev;
 	struct iio_dev *indio_dev;
 	struct ads1015_data *data;
+	unsigned int cfg_mode;
 	int ret;
 	int i;
 
@@ -1003,19 +996,19 @@ 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;
@@ -1034,9 +1027,8 @@ static int ads1015_probe(struct i2c_client *client)
 			return ret;
 	}
 
-	ret = regmap_update_bits(data->regmap, ADS1015_CFG_REG,
-				 ADS1015_CFG_MOD_MASK,
-				 ADS1015_CONTINUOUS << ADS1015_CFG_MOD_SHIFT);
+	cfg_mode = FIELD_PREP(ADS1015_CFG_MOD_MASK, ADS1015_CONTINUOUS);
+	ret = regmap_update_bits(data->regmap, ADS1015_CFG_REG, ADS1015_CFG_MOD_MASK, cfg_mode);
 	if (ret)
 		return ret;
 
@@ -1060,10 +1052,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);
 	struct regmap *map = data->regmap;
+	unsigned int val;
 
-	return regmap_update_bits(map, ADS1015_CFG_REG,
-				  ADS1015_CFG_MOD_MASK,
-				  ADS1015_SINGLESHOT << ADS1015_CFG_MOD_SHIFT);
+	val = FIELD_PREP(ADS1015_CFG_MOD_MASK, ADS1015_SINGLESHOT);
+	return regmap_update_bits(map, ADS1015_CFG_REG, ADS1015_CFG_MOD_MASK, val);
 }
 
 static int ads1015_runtime_resume(struct device *dev)
@@ -1071,11 +1063,11 @@ static int ads1015_runtime_resume(struct device *dev)
 	struct iio_dev *indio_dev = i2c_get_clientdata(to_i2c_client(dev));
 	struct ads1015_data *data = iio_priv(indio_dev);
 	struct regmap *map = data->regmap;
+	unsigned int val;
 	int ret;
 
-	ret = regmap_update_bits(map, ADS1015_CFG_REG,
-				 ADS1015_CFG_MOD_MASK,
-				 ADS1015_CONTINUOUS << ADS1015_CFG_MOD_SHIFT);
+	val = FIELD_PREP(ADS1015_CFG_MOD_MASK, ADS1015_CONTINUOUS);
+	ret = regmap_update_bits(map, ADS1015_CFG_REG, ADS1015_CFG_MOD_MASK, val);
 	if (!ret)
 		data->conv_invalid = true;
 
-- 
2.39.5

Re: [PATCH v5 10/10] iio: adc: ti-ads1015: replace custom shift macros with FIELD_PREP/GET

[Andy Shevchenko]

On Wed, Sep 30, 2026 at 10:42:04PM +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.

...

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

Personally I would go with if-else with full initialisations of mask and cfg.

	if (ads1015_event_channel_enabled(data)) {
		mask = ADS1015_CFG_COMP_QUE_MASK | ADS1015_CFG_COMP_MODE_MASK |
		       ADS1015_CFG_MUX_MASK | ADS1015_CFG_PGA_MASK |
		       ADS1015_CFG_DR_MASK;
		cfg = FIELD_PREP(ADS1015_CFG_MUX_MASK, chan) |
		      FIELD_PREP(ADS1015_CFG_PGA_MASK, pga) |
		      FIELD_PREP(ADS1015_CFG_DR_MASK, dr) |
		      FIELD_PREP(ADS1015_CFG_COMP_QUE_MASK,
				  data->thresh_data[chan].comp_queue) |
		      FIELD_PREP(ADS1015_CFG_COMP_MODE_MASK,
				  data->comp_mode);
	} else {
		mask = ADS1015_CFG_MUX_MASK | ADS1015_CFG_PGA_MASK |
		       ADS1015_CFG_DR_MASK;
		cfg = FIELD_PREP(ADS1015_CFG_MUX_MASK, chan) |
		      FIELD_PREP(ADS1015_CFG_PGA_MASK, pga) |
		      FIELD_PREP(ADS1015_CFG_DR_MASK, dr);
	}

// also note the broken indentation

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v5 00/10] iio: adc: ti-ads1015: modernize resource management and bitfield logic

[Joshua Crofts]

On Wed, 30 Sep 2026 22:41:54 +0530
Archit Anant <architanant5@gmail.com> wrote:

> This series modernizes the ti-ads1015 driver by updating its runtime PM
> handling, error handling, resource management, and register bitfield
> operations.
> 
> This series builds on top of Stepan Ionichev's devm error path fix [1] and:
> - Replaces SET_RUNTIME_PM_OPS() with DEFINE_RUNTIME_DEV_PM_OPS().
> - Sorts the driver headers alphabetically.
> - Returns -ENODATA when match data is missing.
> - Uses a local device pointer in probe.
> - Converts suitable error paths to dev_err_probe().
> - Drops unneeded '< 0' error checks for functions returning 0 or error.
> - Converts mutex initialization to devm_mutex_init().
> - Transitions runtime PM setup to devm_pm_runtime_set_active_enabled().
> - Inlines the redundant ads1015_set_conv_mode() helper.
> - Replaces custom shift macros with FIELD_PREP()/FIELD_GET().
> 
> [1] https://lore.kernel.org/all/20260529183708.17819-1-sozdayvek@gmail.com/
> 
> ---
> v5:
> - Reordered patches: inlined ads1015_set_conv_mode() before applying
>   FIELD_PREP() to completely eliminate code churn.
> - Extracted FIELD_PREP() evaluations into local variables for
>   regmap_update_bits() calls to ensure clean line lengths and readability.
> - Added a patch to return -ENODATA when i2c_get_match_data() fails, as
>   requested by Andy Shevchenko.
> - Split devm_mutex_init() into its own patch to decouple it from the
>   runtime PM modernization, as requested by Andy Shevchenko.
> 
> v4: https://lore.kernel.org/linux-iio/20260827172100.39176-1-architanant5@gmail.com/
> 
> Archit Anant (10):
>   iio: adc: ti-ads1015: use DEFINE_RUNTIME_DEV_PM_OPS()
>   iio: adc: ti-ads1015: sort headers alphabetically
>   iio: adc: ti-ads1015: return -ENODATA when match data is missing
>   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: drop unneeded < 0 error checks
>   iio: adc: ti-ads1015: use devm_mutex_init()
>   iio: adc: ti-ads1015: use managed runtime PM setup
>   iio: adc: ti-ads1015: inline ads1015_set_conv_mode()
>   iio: adc: ti-ads1015: replace custom shift macros with FIELD_PREP/GET
> 
>  drivers/iio/adc/ti-ads1015.c | 160 +++++++++++++++++------------------
>  1 file changed, 76 insertions(+), 84 deletions(-)
> 

I have two *very* tiny nits that can be tweaked when applying. Otherwise
LGTM.

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

-- 
Kind regards,
Joshua Crofts