mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH v2 0/6] iio: adc: ti-ads1015: modernize resource management
@ 2026-07-27 19:20 Archit Anant
  2026-07-27 19:20 ` [PATCH v2 1/6] iio: adc: ti-ads1015: use DEFINE_RUNTIME_DEV_PM_OPS() Archit Anant
                   ` (5 more replies)
  0 siblings, 6 replies; 11+ messages in thread
From: Archit Anant @ 2026-07-27 19:20 UTC (permalink / raw)
  To: jic23
  Cc: dlechner, andy, nuno.sa, u.kleine-koenig, linux-iio,
	linux-kernel, 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.

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 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 (6):
  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

 drivers/iio/adc/ti-ads1015.c | 114 +++++++++++++++--------------------
 1 file changed, 50 insertions(+), 64 deletions(-)

-- 
2.39.5


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH v2 1/6] iio: adc: ti-ads1015: use DEFINE_RUNTIME_DEV_PM_OPS()
  2026-07-27 19:20 [PATCH v2 0/6] iio: adc: ti-ads1015: modernize resource management Archit Anant
@ 2026-07-27 19:20 ` Archit Anant
  2026-08-01 18:28   ` Jonathan Cameron
  2026-07-27 19:20 ` [PATCH v2 2/6] iio: adc: ti-ads1015: fix PM leak on probe failure Archit Anant
                   ` (4 subsequent siblings)
  5 siblings, 1 reply; 11+ messages in thread
From: Archit Anant @ 2026-07-27 19:20 UTC (permalink / raw)
  To: jic23
  Cc: dlechner, andy, nuno.sa, u.kleine-koenig, linux-iio,
	linux-kernel, 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


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH v2 2/6] iio: adc: ti-ads1015: fix PM leak on probe failure
  2026-07-27 19:20 [PATCH v2 0/6] iio: adc: ti-ads1015: modernize resource management Archit Anant
  2026-07-27 19:20 ` [PATCH v2 1/6] iio: adc: ti-ads1015: use DEFINE_RUNTIME_DEV_PM_OPS() Archit Anant
@ 2026-07-27 19:20 ` Archit Anant
  2026-07-27 19:20 ` [PATCH v2 3/6] iio: adc: ti-ads1015: sort headers alphabetically Archit Anant
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 11+ messages in thread
From: Archit Anant @ 2026-07-27 19:20 UTC (permalink / raw)
  To: jic23
  Cc: dlechner, andy, nuno.sa, u.kleine-koenig, linux-iio,
	linux-kernel, 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


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH v2 3/6] iio: adc: ti-ads1015: sort headers alphabetically
  2026-07-27 19:20 [PATCH v2 0/6] iio: adc: ti-ads1015: modernize resource management Archit Anant
  2026-07-27 19:20 ` [PATCH v2 1/6] iio: adc: ti-ads1015: use DEFINE_RUNTIME_DEV_PM_OPS() Archit Anant
  2026-07-27 19:20 ` [PATCH v2 2/6] iio: adc: ti-ads1015: fix PM leak on probe failure Archit Anant
@ 2026-07-27 19:20 ` Archit Anant
  2026-07-27 19:21 ` [PATCH v2 4/6] iio: adc: ti-ads1015: use local device pointer in probe Archit Anant
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 11+ messages in thread
From: Archit Anant @ 2026-07-27 19:20 UTC (permalink / raw)
  To: jic23
  Cc: dlechner, andy, nuno.sa, u.kleine-koenig, linux-iio,
	linux-kernel, 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


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH v2 4/6] iio: adc: ti-ads1015: use local device pointer in probe
  2026-07-27 19:20 [PATCH v2 0/6] iio: adc: ti-ads1015: modernize resource management Archit Anant
                   ` (2 preceding siblings ...)
  2026-07-27 19:20 ` [PATCH v2 3/6] iio: adc: ti-ads1015: sort headers alphabetically Archit Anant
@ 2026-07-27 19:21 ` Archit Anant
  2026-08-01 18:32   ` Jonathan Cameron
  2026-07-27 19:21 ` [PATCH v2 5/6] iio: adc: ti-ads1015: use dev_err_probe() for error handling Archit Anant
  2026-07-27 19:21 ` [PATCH v2 6/6] iio: adc: ti-ads1015: convert to fully managed resources Archit Anant
  5 siblings, 1 reply; 11+ messages in thread
From: Archit Anant @ 2026-07-27 19:21 UTC (permalink / raw)
  To: jic23
  Cc: dlechner, andy, nuno.sa, u.kleine-koenig, linux-iio,
	linux-kernel, 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 | 27 ++++++++++++++-------------
 1 file changed, 14 insertions(+), 13 deletions(-)

diff --git a/drivers/iio/adc/ti-ads1015.c b/drivers/iio/adc/ti-ads1015.c
index 293fdd49381f..59ce2f89daeb 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;
 
@@ -978,15 +979,15 @@ static int ads1015_probe(struct i2c_client *client)
 					    &ads1015_regmap_config :
 					    &tla2024_regmap_config);
 	if (IS_ERR(data->regmap)) {
-		dev_err(&client->dev, "Failed to allocate register map\n");
+		dev_err(dev, "Failed to allocate register map\n");
 		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) {
-		dev_err(&client->dev, "iio triggered buffer setup failed\n");
+		dev_err(dev, "iio triggered buffer setup failed\n");
 		return ret;
 	}
 
@@ -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);
+		dev_err(dev, "Failed to register IIO device\n");
+		pm_runtime_disable(dev);
+		pm_runtime_set_suspended(dev);
 		return ret;
 	}
 
-- 
2.39.5


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH v2 5/6] iio: adc: ti-ads1015: use dev_err_probe() for error handling
  2026-07-27 19:20 [PATCH v2 0/6] iio: adc: ti-ads1015: modernize resource management Archit Anant
                   ` (3 preceding siblings ...)
  2026-07-27 19:21 ` [PATCH v2 4/6] iio: adc: ti-ads1015: use local device pointer in probe Archit Anant
@ 2026-07-27 19:21 ` Archit Anant
  2026-08-01 18:39   ` Jonathan Cameron
  2026-07-27 19:21 ` [PATCH v2 6/6] iio: adc: ti-ads1015: convert to fully managed resources Archit Anant
  5 siblings, 1 reply; 11+ messages in thread
From: Archit Anant @ 2026-07-27 19:21 UTC (permalink / raw)
  To: jic23
  Cc: dlechner, andy, nuno.sa, u.kleine-koenig, linux-iio,
	linux-kernel, 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.

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 59ce2f89daeb..a8dba4e5aed3 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(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(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


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH v2 6/6] iio: adc: ti-ads1015: convert to fully managed resources
  2026-07-27 19:20 [PATCH v2 0/6] iio: adc: ti-ads1015: modernize resource management Archit Anant
                   ` (4 preceding siblings ...)
  2026-07-27 19:21 ` [PATCH v2 5/6] iio: adc: ti-ads1015: use dev_err_probe() for error handling Archit Anant
@ 2026-07-27 19:21 ` Archit Anant
  5 siblings, 0 replies; 11+ messages in thread
From: Archit Anant @ 2026-07-27 19:21 UTC (permalink / raw)
  To: jic23
  Cc: dlechner, andy, nuno.sa, u.kleine-koenig, linux-iio,
	linux-kernel, 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 | 45 ++++++++++++++----------------------
 1 file changed, 17 insertions(+), 28 deletions(-)

diff --git a/drivers/iio/adc/ti-ads1015.c b/drivers/iio/adc/ti-ads1015.c
index a8dba4e5aed3..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,40 +1036,23 @@ 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) {
-		dev_err(dev, "Failed to register IIO device\n");
-		pm_runtime_disable(dev);
-		pm_runtime_set_suspended(dev);
+	ret = devm_pm_runtime_set_active_enabled(dev);
+	if (ret)
 		return ret;
-	}
-
-	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);
+	ret = devm_iio_device_register(dev, indio_dev);
 	if (ret)
-		dev_warn(&client->dev, "Failed to power down (%pe)\n",
-			 ERR_PTR(ret));
+		return dev_err_probe(dev, ret,
+				     "Failed to register IIO device\n");
+
+	return 0;
 }
 
 static int ads1015_runtime_suspend(struct device *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


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 1/6] iio: adc: ti-ads1015: use DEFINE_RUNTIME_DEV_PM_OPS()
  2026-07-27 19:20 ` [PATCH v2 1/6] iio: adc: ti-ads1015: use DEFINE_RUNTIME_DEV_PM_OPS() Archit Anant
@ 2026-08-01 18:28   ` Jonathan Cameron
  2026-08-03  7:22     ` Archit Anant
  0 siblings, 1 reply; 11+ messages in thread
From: Jonathan Cameron @ 2026-08-01 18:28 UTC (permalink / raw)
  To: Archit Anant
  Cc: dlechner, andy, nuno.sa, u.kleine-koenig, linux-iio, linux-kernel

On Tue, 28 Jul 2026 00:50:57 +0530
Archit Anant <architanant5@gmail.com> wrote:

> 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>
Hi Archit,

This looks fine but did make me look at the code that was being protected
and in particular ads1015_set_conv_mode()

A few things jump out about that which might make sense for further improvement
if you want to take them on.


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

Use FIELD_PREP() here and drop the ADS1015_CFG_MOD_SHIFT macro.
That made me wonder how extensive _SHIFT macros are in this driver and the
answer is very!  Get rid of all of them in favour of FIELD_PREP()
and FIELD_GREP() and using the field masks.


> }
The other thing is the question of why this function exists at all
given with the value of mode inline it would be obvious what it is doing
without the wrapper and this is the only similar little helper function.

I'd squash it so we do the regmap_update_bits() calls directly instead
of via this helper function.

I did debate whether brining setting conv_invalid into the function made
sense but on balance I think not.

If you do make these changes, 1 patch for dropping all the _SHIFT
macros and replacing with FIELD_PREP() / FIELD_GET() and a second
patch to remove the helper function.

Thanks,

Jonathan


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


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 4/6] iio: adc: ti-ads1015: use local device pointer in probe
  2026-07-27 19:21 ` [PATCH v2 4/6] iio: adc: ti-ads1015: use local device pointer in probe Archit Anant
@ 2026-08-01 18:32   ` Jonathan Cameron
  0 siblings, 0 replies; 11+ messages in thread
From: Jonathan Cameron @ 2026-08-01 18:32 UTC (permalink / raw)
  To: Archit Anant
  Cc: dlechner, andy, nuno.sa, u.kleine-koenig, linux-iio, linux-kernel

On Tue, 28 Jul 2026 00:51:00 +0530
Archit Anant <architanant5@gmail.com> wrote:

> Introduce a local device pointer 'dev' in ads1015_probe to
> avoid accessing &client->dev repeatedly.
> 
> Signed-off-by: Archit Anant <architanant5@gmail.com>
Hi Archit,

To avoid churn in the series this should be done slightly different.
See below but in short it is a case of not updating everything in
this patch given you are going to touch the same code in the next
one.

> ---
>  drivers/iio/adc/ti-ads1015.c | 27 ++++++++++++++-------------
>  1 file changed, 14 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/iio/adc/ti-ads1015.c b/drivers/iio/adc/ti-ads1015.c
> index 293fdd49381f..59ce2f89daeb 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;
>  
> @@ -978,15 +979,15 @@ static int ads1015_probe(struct i2c_client *client)
>  					    &ads1015_regmap_config :
>  					    &tla2024_regmap_config);
>  	if (IS_ERR(data->regmap)) {
> -		dev_err(&client->dev, "Failed to allocate register map\n");
> +		dev_err(dev, "Failed to allocate register map\n");
>  		return PTR_ERR(data->regmap);

Leave updating this one for next patch (and ignore sashiko if it complains ;)

>  	}
>  
> -	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) {
> -		dev_err(&client->dev, "iio triggered buffer setup failed\n");
> +		dev_err(dev, "iio triggered buffer setup failed\n");
Likewise, do this as part of der_err_probe()
>  		return ret;
>  	}
>  
> @@ -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);
> +		dev_err(dev, "Failed to register IIO device\n");

Leave this one for dev_err_probe() patch.  Just this line not the ones around it.

> +		pm_runtime_disable(dev);
> +		pm_runtime_set_suspended(dev);
>  		return ret;
>  	}
>  


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 5/6] iio: adc: ti-ads1015: use dev_err_probe() for error handling
  2026-07-27 19:21 ` [PATCH v2 5/6] iio: adc: ti-ads1015: use dev_err_probe() for error handling Archit Anant
@ 2026-08-01 18:39   ` Jonathan Cameron
  0 siblings, 0 replies; 11+ messages in thread
From: Jonathan Cameron @ 2026-08-01 18:39 UTC (permalink / raw)
  To: Archit Anant
  Cc: dlechner, andy, nuno.sa, u.kleine-koenig, linux-iio, linux-kernel

On Tue, 28 Jul 2026 00:51:01 +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.
> 
> Suggested-by: Jonathan Cameron <jic23@kernel.org>
> Signed-off-by: Archit Anant <architanant5@gmail.com>

Ah. This doesn't pick up all the places I was expecting when reviewing
previous patch.  It should definitely include the dev_err_probe()
for iio_device_register() - that won't be change just because
you move to devm_iio_device_register() in the next patch.

> ---
>  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 59ce2f89daeb..a8dba4e5aed3 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(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(dev, "iio triggered buffer setup failed\n");

Leave the switch from client->dev for this patch then we don't end up changing
same line twice.  Just add a brief comment to say you are taking advantage
of the now available local dev pointer.

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


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 1/6] iio: adc: ti-ads1015: use DEFINE_RUNTIME_DEV_PM_OPS()
  2026-08-01 18:28   ` Jonathan Cameron
@ 2026-08-03  7:22     ` Archit Anant
  0 siblings, 0 replies; 11+ messages in thread
From: Archit Anant @ 2026-08-03  7:22 UTC (permalink / raw)
  To: Jonathan Cameron
  Cc: dlechner, andy, nuno.sa, u.kleine-koenig, linux-iio, linux-kernel

On Sat, Aug 1, 2026 at 11:58 PM Jonathan Cameron <jic23@kernel.org> wrote:
>
> On Tue, 28 Jul 2026 00:50:57 +0530
> Archit Anant <architanant5@gmail.com> wrote:
>
> > 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>
> Hi Archit,
>
> This looks fine but did make me look at the code that was being protected
> and in particular ads1015_set_conv_mode()
>
> A few things jump out about that which might make sense for further improvement
> if you want to take them on.
>
>
> > 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);#
>
> Use FIELD_PREP() here and drop the ADS1015_CFG_MOD_SHIFT macro.
> That made me wonder how extensive _SHIFT macros are in this driver and the
> answer is very!  Get rid of all of them in favour of FIELD_PREP()
> and FIELD_GREP() and using the field masks.
>
>
> > }
> The other thing is the question of why this function exists at all
> given with the value of mode inline it would be obvious what it is doing
> without the wrapper and this is the only similar little helper function.
>
> I'd squash it so we do the regmap_update_bits() calls directly instead
> of via this helper function.
>
> I did debate whether brining setting conv_invalid into the function made
> sense but on balance I think not.
>
> If you do make these changes, 1 patch for dropping all the _SHIFT
> macros and replacing with FIELD_PREP() / FIELD_GET() and a second
> patch to remove the helper function.

Understood. I'll include these two patches and send the new series shortly.

>
> Thanks,
>
> Jonathan
>

-- 
Sincerely,
Archit Anant

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2026-08-03  7:22 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-07-27 19:20 [PATCH v2 0/6] iio: adc: ti-ads1015: modernize resource management Archit Anant
2026-07-27 19:20 ` [PATCH v2 1/6] iio: adc: ti-ads1015: use DEFINE_RUNTIME_DEV_PM_OPS() Archit Anant
2026-08-01 18:28   ` Jonathan Cameron
2026-08-03  7:22     ` Archit Anant
2026-07-27 19:20 ` [PATCH v2 2/6] iio: adc: ti-ads1015: fix PM leak on probe failure Archit Anant
2026-07-27 19:20 ` [PATCH v2 3/6] iio: adc: ti-ads1015: sort headers alphabetically Archit Anant
2026-07-27 19:21 ` [PATCH v2 4/6] iio: adc: ti-ads1015: use local device pointer in probe Archit Anant
2026-08-01 18:32   ` Jonathan Cameron
2026-07-27 19:21 ` [PATCH v2 5/6] iio: adc: ti-ads1015: use dev_err_probe() for error handling Archit Anant
2026-08-01 18:39   ` Jonathan Cameron
2026-07-27 19:21 ` [PATCH v2 6/6] iio: adc: ti-ads1015: convert to fully managed resources Archit Anant

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®