* [PATCH v3 0/4] iio: adc: ad799x: modernize resource management
@ 2026-03-02 13:06 Archit Anant
2026-03-02 13:06 ` [PATCH v3 1/4] iio: adc: ad799x: make rx_buf static and DMA-safe Archit Anant
` (4 more replies)
0 siblings, 5 replies; 14+ messages in thread
From: Archit Anant @ 2026-03-02 13:06 UTC (permalink / raw)
To: jic23, lars, Michael.Hennerich
Cc: dlechner, nuno.sa, andy, linux-iio, linux-kernel, Archit Anant
This series modernizes the ad799x driver by converting all resource
management to use the devm_ infrastructure, ultimately allowing for the
complete removal of the ad799x_remove() function and simplifying the
error paths in probe.
This series addresses the feedback from v2, where the changes were
squashed into a single patch, by splitting the logical steps into
individual, bisectable commits. It also incorporates feedback regarding
the safe ordering of regulator disablement and IIO device unregistration.
Changes in v3:
- Split the monolithic v2 patch into 4 logical commits as requested by
Andy Shevchenko.
Changes in v2:
- Incorporated feedback from David Lechner to eliminate dynamic buffer
allocation.
- Incorporated feedback from Jonathan Cameron regarding the ordering
hazards of mixing devm and manual teardown, utilizing devm_add_action
and voltage caching.
Archit Anant (4):
iio: adc: ad799x: make rx_buf static and DMA-safe
iio: adc: ad799x: cache regulator voltages during probe
iio: adc: ad799x: use devm_add_action_or_reset for regulators
iio: adc: ad799x: use devm_iio_device_register and drop remove
drivers/iio/adc/ad799x.c | 86 ++++++++++++++++++++--------------------
1 file changed, 43 insertions(+), 43 deletions(-)
--
2.39.5
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v3 1/4] iio: adc: ad799x: make rx_buf static and DMA-safe
2026-03-02 13:06 [PATCH v3 0/4] iio: adc: ad799x: modernize resource management Archit Anant
@ 2026-03-02 13:06 ` Archit Anant
2026-03-02 16:29 ` David Lechner
2026-03-02 16:31 ` David Lechner
2026-03-02 13:06 ` [PATCH v3 2/4] iio: adc: ad799x: cache regulator voltages during probe Archit Anant
` (3 subsequent siblings)
4 siblings, 2 replies; 14+ messages in thread
From: Archit Anant @ 2026-03-02 13:06 UTC (permalink / raw)
To: jic23, lars, Michael.Hennerich
Cc: dlechner, nuno.sa, andy, linux-iio, linux-kernel, Archit Anant
Currently, rx_buf is dynamically allocated using kmalloc() every time
ad799x_update_scan_mode() is called. This can lead to memory leaks if
the scan mask is updated multiple times.
Drop the dynamic allocation and replace it with a static, buffer at the
end of the state structure using IIO_DECLARE_DMA_BUFFER_WITH_TS().
This eliminates the allocation overhead, prevents leaks, and removes
the need for manual kfree() on driver removal.
Suggested-by: David Lechner <dlechner@baylibre.com>
Signed-off-by: Archit Anant <architanant5@gmail.com>
---
drivers/iio/adc/ad799x.c | 13 ++++---------
1 file changed, 4 insertions(+), 9 deletions(-)
diff --git a/drivers/iio/adc/ad799x.c b/drivers/iio/adc/ad799x.c
index 108bb22162ef..7775be874081 100644
--- a/drivers/iio/adc/ad799x.c
+++ b/drivers/iio/adc/ad799x.c
@@ -39,6 +39,7 @@
#include <linux/iio/triggered_buffer.h>
#define AD799X_CHANNEL_SHIFT 4
+#define AD799X_MAX_CHANNELS 8
/*
* AD7991, AD7995 and AD7999 defines
@@ -133,8 +134,8 @@ struct ad799x_state {
unsigned int id;
u16 config;
- u8 *rx_buf;
unsigned int transfer_size;
+ IIO_DECLARE_DMA_BUFFER_WITH_TS(__be16, rx_buf, AD799X_MAX_CHANNELS);
};
static int ad799x_write_config(struct ad799x_state *st, u16 val)
@@ -217,11 +218,11 @@ static irqreturn_t ad799x_trigger_handler(int irq, void *p)
}
b_sent = i2c_smbus_read_i2c_block_data(st->client,
- cmd, st->transfer_size, st->rx_buf);
+ cmd, st->transfer_size, (u8 *)st->rx_buf);
if (b_sent < 0)
goto out;
- iio_push_to_buffers_with_timestamp(indio_dev, st->rx_buf,
+ iio_push_to_buffers_with_timestamp(indio_dev, &st->rx_buf,
iio_get_time_ns(indio_dev));
out:
iio_trigger_notify_done(indio_dev->trig);
@@ -234,11 +235,6 @@ static int ad799x_update_scan_mode(struct iio_dev *indio_dev,
{
struct ad799x_state *st = iio_priv(indio_dev);
- kfree(st->rx_buf);
- st->rx_buf = kmalloc(indio_dev->scan_bytes, GFP_KERNEL);
- if (!st->rx_buf)
- return -ENOMEM;
-
st->transfer_size = bitmap_weight(scan_mask,
iio_get_masklength(indio_dev)) * 2;
@@ -895,7 +891,6 @@ static void ad799x_remove(struct i2c_client *client)
if (st->vref)
regulator_disable(st->vref);
regulator_disable(st->reg);
- kfree(st->rx_buf);
}
static int ad799x_suspend(struct device *dev)
--
2.39.5
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH v3 2/4] iio: adc: ad799x: cache regulator voltages during probe
2026-03-02 13:06 [PATCH v3 0/4] iio: adc: ad799x: modernize resource management Archit Anant
2026-03-02 13:06 ` [PATCH v3 1/4] iio: adc: ad799x: make rx_buf static and DMA-safe Archit Anant
@ 2026-03-02 13:06 ` Archit Anant
2026-03-02 13:13 ` Andy Shevchenko
2026-03-02 16:37 ` David Lechner
2026-03-02 13:06 ` [PATCH v3 3/4] iio: adc: ad799x: use devm_add_action_or_reset for regulators Archit Anant
` (2 subsequent siblings)
4 siblings, 2 replies; 14+ messages in thread
From: Archit Anant @ 2026-03-02 13:06 UTC (permalink / raw)
To: jic23, lars, Michael.Hennerich
Cc: dlechner, nuno.sa, andy, linux-iio, linux-kernel, Archit Anant
Reading the regulator voltage via regulator_get_voltage() can be a slow
operation. Since the reference voltages for this ADC are not expected to
change at runtime, it is inefficient to query the regulator API every
time userspace reads the IIO_CHAN_INFO_SCALE attribute.
Cache the VCC and VREF voltages in the state structure during probe().
This improves the performance of ad799x_read_raw() and removes the
dependency on the regulator pointers during fast-path reads.
Suggested-by: Jonathan Cameron <jic23@kernel.org>
Signed-off-by: Archit Anant <architanant5@gmail.com>
---
drivers/iio/adc/ad799x.c | 17 +++++++++++++++--
1 file changed, 15 insertions(+), 2 deletions(-)
diff --git a/drivers/iio/adc/ad799x.c b/drivers/iio/adc/ad799x.c
index 7775be874081..35e0589428d0 100644
--- a/drivers/iio/adc/ad799x.c
+++ b/drivers/iio/adc/ad799x.c
@@ -135,6 +135,10 @@ struct ad799x_state {
u16 config;
unsigned int transfer_size;
+
+ int vcc_uv;
+ int vref_uv;
+
IIO_DECLARE_DMA_BUFFER_WITH_TS(__be16, rx_buf, AD799X_MAX_CHANNELS);
};
@@ -303,9 +307,9 @@ static int ad799x_read_raw(struct iio_dev *indio_dev,
return IIO_VAL_INT;
case IIO_CHAN_INFO_SCALE:
if (st->vref)
- ret = regulator_get_voltage(st->vref);
+ ret = st->vref_uv;
else
- ret = regulator_get_voltage(st->reg);
+ ret = st->vcc_uv;
if (ret < 0)
return ret;
@@ -809,6 +813,10 @@ static int ad799x_probe(struct i2c_client *client)
ret = regulator_enable(st->reg);
if (ret)
return ret;
+ ret = regulator_get_voltage(st->reg);
+ if (ret < 0)
+ goto error_disable_reg;
+ st->vcc_uv = ret;
/* check if an external reference is supplied */
if (chip_info->has_vref) {
@@ -827,6 +835,11 @@ static int ad799x_probe(struct i2c_client *client)
ret = regulator_enable(st->vref);
if (ret)
goto error_disable_reg;
+
+ ret = regulator_get_voltage(st->vref);
+ if (ret < 0)
+ goto error_disable_vref;
+ st->vref_uv = ret;
}
}
--
2.39.5
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH v3 3/4] iio: adc: ad799x: use devm_add_action_or_reset for regulators
2026-03-02 13:06 [PATCH v3 0/4] iio: adc: ad799x: modernize resource management Archit Anant
2026-03-02 13:06 ` [PATCH v3 1/4] iio: adc: ad799x: make rx_buf static and DMA-safe Archit Anant
2026-03-02 13:06 ` [PATCH v3 2/4] iio: adc: ad799x: cache regulator voltages during probe Archit Anant
@ 2026-03-02 13:06 ` Archit Anant
2026-03-02 13:16 ` Andy Shevchenko
2026-03-02 16:35 ` David Lechner
2026-03-02 13:06 ` [PATCH v3 4/4] iio: adc: ad799x: use devm_iio_device_register and drop remove Archit Anant
2026-03-02 16:28 ` [PATCH v3 0/4] iio: adc: ad799x: modernize resource management David Lechner
4 siblings, 2 replies; 14+ messages in thread
From: Archit Anant @ 2026-03-02 13:06 UTC (permalink / raw)
To: jic23, lars, Michael.Hennerich
Cc: dlechner, nuno.sa, andy, linux-iio, linux-kernel, Archit Anant
Convert the manual regulator_disable() handling to use
devm_add_action_or_reset(). This guarantees that the regulators are
disabled in the correct reverse order of allocation during device
teardown or probe failure.
This removes the need to manually disable the regulators in the
remove() function.
Suggested-by: David Lechner <dlechner@baylibre.com>
Signed-off-by: Archit Anant <architanant5@gmail.com>
---
drivers/iio/adc/ad799x.c | 19 ++++++++++++++++---
1 file changed, 16 insertions(+), 3 deletions(-)
diff --git a/drivers/iio/adc/ad799x.c b/drivers/iio/adc/ad799x.c
index 35e0589428d0..1a96b5417ecf 100644
--- a/drivers/iio/adc/ad799x.c
+++ b/drivers/iio/adc/ad799x.c
@@ -781,6 +781,13 @@ static const struct ad799x_chip_info ad799x_chip_info_tbl[] = {
},
};
+static void ad799x_reg_disable(void *data)
+{
+ struct regulator *reg = data;
+
+ regulator_disable(reg);
+}
+
static int ad799x_probe(struct i2c_client *client)
{
const struct i2c_device_id *id = i2c_client_get_device_id(client);
@@ -818,6 +825,10 @@ static int ad799x_probe(struct i2c_client *client)
goto error_disable_reg;
st->vcc_uv = ret;
+ ret = devm_add_action_or_reset(&client->dev, ad799x_reg_disable, st->reg);
+ if (ret)
+ return ret;
+
/* check if an external reference is supplied */
if (chip_info->has_vref) {
st->vref = devm_regulator_get_optional(&client->dev, "vref");
@@ -840,6 +851,11 @@ static int ad799x_probe(struct i2c_client *client)
if (ret < 0)
goto error_disable_vref;
st->vref_uv = ret;
+
+ ret = devm_add_action_or_reset(&client->dev, ad799x_reg_disable,
+ st->vref);
+ if (ret)
+ goto error_disable_reg;
}
}
@@ -901,9 +917,6 @@ static void ad799x_remove(struct i2c_client *client)
iio_device_unregister(indio_dev);
iio_triggered_buffer_cleanup(indio_dev);
- if (st->vref)
- regulator_disable(st->vref);
- regulator_disable(st->reg);
}
static int ad799x_suspend(struct device *dev)
--
2.39.5
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH v3 4/4] iio: adc: ad799x: use devm_iio_device_register and drop remove
2026-03-02 13:06 [PATCH v3 0/4] iio: adc: ad799x: modernize resource management Archit Anant
` (2 preceding siblings ...)
2026-03-02 13:06 ` [PATCH v3 3/4] iio: adc: ad799x: use devm_add_action_or_reset for regulators Archit Anant
@ 2026-03-02 13:06 ` Archit Anant
2026-03-02 13:17 ` Andy Shevchenko
2026-03-02 16:28 ` [PATCH v3 0/4] iio: adc: ad799x: modernize resource management David Lechner
4 siblings, 1 reply; 14+ messages in thread
From: Archit Anant @ 2026-03-02 13:06 UTC (permalink / raw)
To: jic23, lars, Michael.Hennerich
Cc: dlechner, nuno.sa, andy, linux-iio, linux-kernel, Archit Anant
Convert the driver to use the device-managed versions of
iio_device_register() and iio_triggered_buffer_setup().
Because all resources (buffer, regulators, IRQs, IIO device) are now
managed by the devm core, the unwinding order is guaranteed to be
safe and correct. We can safely remove all manual error handling goto
labels in ad799x_probe() and delete the ad799x_remove() function
entirely.
This eliminates boilerplate code and prevents future resource leaks.
Suggested-by: David Lechner <dlechner@baylibre.com>
Signed-off-by: Archit Anant <architanant5@gmail.com>
---
drivers/iio/adc/ad799x.c | 43 ++++++++++------------------------------
1 file changed, 11 insertions(+), 32 deletions(-)
diff --git a/drivers/iio/adc/ad799x.c b/drivers/iio/adc/ad799x.c
index 1a96b5417ecf..2b40ef2da7a2 100644
--- a/drivers/iio/adc/ad799x.c
+++ b/drivers/iio/adc/ad799x.c
@@ -822,7 +822,7 @@ static int ad799x_probe(struct i2c_client *client)
return ret;
ret = regulator_get_voltage(st->reg);
if (ret < 0)
- goto error_disable_reg;
+ return ret;
st->vcc_uv = ret;
ret = devm_add_action_or_reset(&client->dev, ad799x_reg_disable, st->reg);
@@ -835,7 +835,7 @@ static int ad799x_probe(struct i2c_client *client)
ret = PTR_ERR_OR_ZERO(st->vref);
if (ret) {
if (ret != -ENODEV)
- goto error_disable_reg;
+ return ret;
st->vref = NULL;
dev_info(&client->dev, "Using VCC reference voltage\n");
}
@@ -845,17 +845,17 @@ static int ad799x_probe(struct i2c_client *client)
extra_config |= AD7991_REF_SEL;
ret = regulator_enable(st->vref);
if (ret)
- goto error_disable_reg;
+ return ret;
ret = regulator_get_voltage(st->vref);
if (ret < 0)
- goto error_disable_vref;
+ return ret;
st->vref_uv = ret;
ret = devm_add_action_or_reset(&client->dev, ad799x_reg_disable,
st->vref);
if (ret)
- goto error_disable_reg;
+ return ret;
}
}
@@ -870,12 +870,12 @@ static int ad799x_probe(struct i2c_client *client)
ret = ad799x_update_config(st, st->chip_config->default_config | extra_config);
if (ret)
- goto error_disable_vref;
+ return ret;
- ret = iio_triggered_buffer_setup(indio_dev, NULL,
+ ret = devm_iio_triggered_buffer_setup(&client->dev, indio_dev, NULL,
&ad799x_trigger_handler, NULL);
if (ret)
- goto error_disable_vref;
+ return ret;
if (client->irq > 0) {
ret = devm_request_threaded_irq(&client->dev,
@@ -887,36 +887,16 @@ static int ad799x_probe(struct i2c_client *client)
client->name,
indio_dev);
if (ret)
- goto error_cleanup_ring;
+ return ret;
}
mutex_init(&st->lock);
- ret = iio_device_register(indio_dev);
+ ret = devm_iio_device_register(&client->dev, indio_dev);
if (ret)
- goto error_cleanup_ring;
+ return ret;
return 0;
-
-error_cleanup_ring:
- iio_triggered_buffer_cleanup(indio_dev);
-error_disable_vref:
- if (st->vref)
- regulator_disable(st->vref);
-error_disable_reg:
- regulator_disable(st->reg);
-
- return ret;
-}
-
-static void ad799x_remove(struct i2c_client *client)
-{
- struct iio_dev *indio_dev = i2c_get_clientdata(client);
- struct ad799x_state *st = iio_priv(indio_dev);
-
- iio_device_unregister(indio_dev);
-
- iio_triggered_buffer_cleanup(indio_dev);
}
static int ad799x_suspend(struct device *dev)
@@ -986,7 +966,6 @@ static struct i2c_driver ad799x_driver = {
.pm = pm_sleep_ptr(&ad799x_pm_ops),
},
.probe = ad799x_probe,
- .remove = ad799x_remove,
.id_table = ad799x_id,
};
module_i2c_driver(ad799x_driver);
--
2.39.5
^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH v3 2/4] iio: adc: ad799x: cache regulator voltages during probe
2026-03-02 13:06 ` [PATCH v3 2/4] iio: adc: ad799x: cache regulator voltages during probe Archit Anant
@ 2026-03-02 13:13 ` Andy Shevchenko
2026-03-02 16:37 ` David Lechner
1 sibling, 0 replies; 14+ messages in thread
From: Andy Shevchenko @ 2026-03-02 13:13 UTC (permalink / raw)
To: Archit Anant
Cc: jic23, lars, Michael.Hennerich, dlechner, nuno.sa, andy,
linux-iio, linux-kernel
On Mon, Mar 02, 2026 at 06:36:30PM +0530, Archit Anant wrote:
> Reading the regulator voltage via regulator_get_voltage() can be a slow
> operation. Since the reference voltages for this ADC are not expected to
> change at runtime, it is inefficient to query the regulator API every
> time userspace reads the IIO_CHAN_INFO_SCALE attribute.
>
> Cache the VCC and VREF voltages in the state structure during probe().
> This improves the performance of ad799x_read_raw() and removes the
> dependency on the regulator pointers during fast-path reads.
...
> + int vcc_uv;
> + int vref_uv;
_uV in both cases, please. It's a unit suffix.
...
> case IIO_CHAN_INFO_SCALE:
> if (st->vref)
> - ret = regulator_get_voltage(st->vref);
> + ret = st->vref_uv;
> else
> - ret = regulator_get_voltage(st->reg);
> + ret = st->vcc_uv;
> if (ret < 0)
> return ret;
Isn't it a dead check now?
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v3 3/4] iio: adc: ad799x: use devm_add_action_or_reset for regulators
2026-03-02 13:06 ` [PATCH v3 3/4] iio: adc: ad799x: use devm_add_action_or_reset for regulators Archit Anant
@ 2026-03-02 13:16 ` Andy Shevchenko
2026-03-02 16:35 ` David Lechner
1 sibling, 0 replies; 14+ messages in thread
From: Andy Shevchenko @ 2026-03-02 13:16 UTC (permalink / raw)
To: Archit Anant
Cc: jic23, lars, Michael.Hennerich, dlechner, nuno.sa, andy,
linux-iio, linux-kernel
On Mon, Mar 02, 2026 at 06:36:31PM +0530, Archit Anant wrote:
> Convert the manual regulator_disable() handling to use
> devm_add_action_or_reset(). This guarantees that the regulators are
> disabled in the correct reverse order of allocation during device
> teardown or probe failure.
>
> This removes the need to manually disable the regulators in the
> remove() function.
...
> +static void ad799x_reg_disable(void *data)
> +{
> + struct regulator *reg = data;
Unneeded.
> + regulator_disable(reg);
> +}
static void ad799x_reg_disable(void *reg)
{
regulator_disable(reg);
}
...
> static int ad799x_probe(struct i2c_client *client)
With
struct device *dev = &client->dev;
it will look better.
...
> + ret = devm_add_action_or_reset(&client->dev, ad799x_reg_disable, st->reg);
ret = devm_add_action_or_reset(dev, ad799x_reg_disable, st->reg);
> + if (ret)
> + return ret;
...
> + ret = devm_add_action_or_reset(&client->dev, ad799x_reg_disable,
> + st->vref);
ret = devm_add_action_or_reset(dev, ad799x_reg_disable, st->vref);
> + if (ret)
> + goto error_disable_reg;
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v3 4/4] iio: adc: ad799x: use devm_iio_device_register and drop remove
2026-03-02 13:06 ` [PATCH v3 4/4] iio: adc: ad799x: use devm_iio_device_register and drop remove Archit Anant
@ 2026-03-02 13:17 ` Andy Shevchenko
0 siblings, 0 replies; 14+ messages in thread
From: Andy Shevchenko @ 2026-03-02 13:17 UTC (permalink / raw)
To: Archit Anant
Cc: jic23, lars, Michael.Hennerich, dlechner, nuno.sa, andy,
linux-iio, linux-kernel
On Mon, Mar 02, 2026 at 06:36:32PM +0530, Archit Anant wrote:
> Convert the driver to use the device-managed versions of
> iio_device_register() and iio_triggered_buffer_setup().
>
> Because all resources (buffer, regulators, IRQs, IIO device) are now
> managed by the devm core, the unwinding order is guaranteed to be
> safe and correct. We can safely remove all manual error handling goto
> labels in ad799x_probe() and delete the ad799x_remove() function
> entirely.
>
> This eliminates boilerplate code and prevents future resource leaks.
...
> mutex_init(&st->lock);
Missed devm_mutex_init().
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v3 0/4] iio: adc: ad799x: modernize resource management
2026-03-02 13:06 [PATCH v3 0/4] iio: adc: ad799x: modernize resource management Archit Anant
` (3 preceding siblings ...)
2026-03-02 13:06 ` [PATCH v3 4/4] iio: adc: ad799x: use devm_iio_device_register and drop remove Archit Anant
@ 2026-03-02 16:28 ` David Lechner
2026-03-03 5:11 ` Archit Anant
4 siblings, 1 reply; 14+ messages in thread
From: David Lechner @ 2026-03-02 16:28 UTC (permalink / raw)
To: Archit Anant, jic23, lars, Michael.Hennerich
Cc: nuno.sa, andy, linux-iio, linux-kernel
On 3/2/26 7:06 AM, Archit Anant wrote:
> This series modernizes the ad799x driver by converting all resource
> management to use the devm_ infrastructure, ultimately allowing for the
> complete removal of the ad799x_remove() function and simplifying the
> error paths in probe.
>
> This series addresses the feedback from v2, where the changes were
> squashed into a single patch, by splitting the logical steps into
> individual, bisectable commits. It also incorporates feedback regarding
> the safe ordering of regulator disablement and IIO device unregistration.
>
> Changes in v3:
> - Split the monolithic v2 patch into 4 logical commits as requested by
> Andy Shevchenko.
>
> Changes in v2:
> - Incorporated feedback from David Lechner to eliminate dynamic buffer
> allocation.
> - Incorporated feedback from Jonathan Cameron regarding the ordering
> hazards of mixing devm and manual teardown, utilizing devm_add_action
> and voltage caching.
>
I just reviewed v2 because I didn't see there was a v3 yet. This is why
we ask to wait ideally 1 week, but at a few days before sending a new
revision.
Some of those comment will still apply to v3.
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v3 1/4] iio: adc: ad799x: make rx_buf static and DMA-safe
2026-03-02 13:06 ` [PATCH v3 1/4] iio: adc: ad799x: make rx_buf static and DMA-safe Archit Anant
@ 2026-03-02 16:29 ` David Lechner
2026-03-02 16:31 ` David Lechner
1 sibling, 0 replies; 14+ messages in thread
From: David Lechner @ 2026-03-02 16:29 UTC (permalink / raw)
To: Archit Anant, jic23, lars, Michael.Hennerich
Cc: nuno.sa, andy, linux-iio, linux-kernel
On 3/2/26 7:06 AM, Archit Anant wrote:
> Currently, rx_buf is dynamically allocated using kmalloc() every time
> ad799x_update_scan_mode() is called. This can lead to memory leaks if
> the scan mask is updated multiple times.
>
> Drop the dynamic allocation and replace it with a static, buffer at the
> end of the state structure using IIO_DECLARE_DMA_BUFFER_WITH_TS().
> This eliminates the allocation overhead, prevents leaks, and removes
> the need for manual kfree() on driver removal.
>
See replies in v1 and v2. DMA-safe is not important because it is I2C.
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v3 1/4] iio: adc: ad799x: make rx_buf static and DMA-safe
2026-03-02 13:06 ` [PATCH v3 1/4] iio: adc: ad799x: make rx_buf static and DMA-safe Archit Anant
2026-03-02 16:29 ` David Lechner
@ 2026-03-02 16:31 ` David Lechner
1 sibling, 0 replies; 14+ messages in thread
From: David Lechner @ 2026-03-02 16:31 UTC (permalink / raw)
To: Archit Anant, jic23, lars, Michael.Hennerich
Cc: nuno.sa, andy, linux-iio, linux-kernel
On 3/2/26 7:06 AM, Archit Anant wrote:
> Currently, rx_buf is dynamically allocated using kmalloc() every time
> ad799x_update_scan_mode() is called. This can lead to memory leaks if
> the scan mask is updated multiple times.
>
> Drop the dynamic allocation and replace it with a static, buffer at the
> end of the state structure using IIO_DECLARE_DMA_BUFFER_WITH_TS().
> This eliminates the allocation overhead, prevents leaks, and removes
> the need for manual kfree() on driver removal.
>
> Suggested-by: David Lechner <dlechner@baylibre.com>
> Signed-off-by: Archit Anant <architanant5@gmail.com>
> ---
...
> @@ -217,11 +218,11 @@ static irqreturn_t ad799x_trigger_handler(int irq, void *p)
> }
>
> b_sent = i2c_smbus_read_i2c_block_data(st->client,
> - cmd, st->transfer_size, st->rx_buf);
> + cmd, st->transfer_size, (u8 *)st->rx_buf);
> if (b_sent < 0)
> goto out;
>
> - iio_push_to_buffers_with_timestamp(indio_dev, st->rx_buf,
> + iio_push_to_buffers_with_timestamp(indio_dev, &st->rx_buf,
Don't need & here since rx_buf is still an array.
> iio_get_time_ns(indio_dev));
> out:
> iio_trigger_notify_done(indio_dev->trig);
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v3 3/4] iio: adc: ad799x: use devm_add_action_or_reset for regulators
2026-03-02 13:06 ` [PATCH v3 3/4] iio: adc: ad799x: use devm_add_action_or_reset for regulators Archit Anant
2026-03-02 13:16 ` Andy Shevchenko
@ 2026-03-02 16:35 ` David Lechner
1 sibling, 0 replies; 14+ messages in thread
From: David Lechner @ 2026-03-02 16:35 UTC (permalink / raw)
To: Archit Anant, jic23, lars, Michael.Hennerich
Cc: nuno.sa, andy, linux-iio, linux-kernel
On 3/2/26 7:06 AM, Archit Anant wrote:
> Convert the manual regulator_disable() handling to use
> devm_add_action_or_reset(). This guarantees that the regulators are
> disabled in the correct reverse order of allocation during device
> teardown or probe failure.
>
> This removes the need to manually disable the regulators in the
> remove() function.
>
> Suggested-by: David Lechner <dlechner@baylibre.com>
> Signed-off-by: Archit Anant <architanant5@gmail.com>
> ---
> drivers/iio/adc/ad799x.c | 19 ++++++++++++++++---
> 1 file changed, 16 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/iio/adc/ad799x.c b/drivers/iio/adc/ad799x.c
> index 35e0589428d0..1a96b5417ecf 100644
> --- a/drivers/iio/adc/ad799x.c
> +++ b/drivers/iio/adc/ad799x.c
> @@ -781,6 +781,13 @@ static const struct ad799x_chip_info ad799x_chip_info_tbl[] = {
> },
> };
>
> +static void ad799x_reg_disable(void *data)
> +{
> + struct regulator *reg = data;
> +
> + regulator_disable(reg);
> +}
> +
> static int ad799x_probe(struct i2c_client *client)
> {
> const struct i2c_device_id *id = i2c_client_get_device_id(client);
> @@ -818,6 +825,10 @@ static int ad799x_probe(struct i2c_client *client)
> goto error_disable_reg;
> st->vcc_uv = ret;
>
> + ret = devm_add_action_or_reset(&client->dev, ad799x_reg_disable, st->reg);
> + if (ret)
> + return ret;
> +
> /* check if an external reference is supplied */
> if (chip_info->has_vref) {
> st->vref = devm_regulator_get_optional(&client->dev, "vref");
> @@ -840,6 +851,11 @@ static int ad799x_probe(struct i2c_client *client)
> if (ret < 0)
> goto error_disable_vref;
> st->vref_uv = ret;
> +
> + ret = devm_add_action_or_reset(&client->dev, ad799x_reg_disable,
> + st->vref);
> + if (ret)
> + goto error_disable_reg;
This goto and others should be removed in this patch. devm handles it now.
> }
> }
>
> @@ -901,9 +917,6 @@ static void ad799x_remove(struct i2c_client *client)
> iio_device_unregister(indio_dev);
>
> iio_triggered_buffer_cleanup(indio_dev);
> - if (st->vref)
> - regulator_disable(st->vref);
> - regulator_disable(st->reg);
> }
>
> static int ad799x_suspend(struct device *dev)
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v3 2/4] iio: adc: ad799x: cache regulator voltages during probe
2026-03-02 13:06 ` [PATCH v3 2/4] iio: adc: ad799x: cache regulator voltages during probe Archit Anant
2026-03-02 13:13 ` Andy Shevchenko
@ 2026-03-02 16:37 ` David Lechner
1 sibling, 0 replies; 14+ messages in thread
From: David Lechner @ 2026-03-02 16:37 UTC (permalink / raw)
To: Archit Anant, jic23, lars, Michael.Hennerich
Cc: nuno.sa, andy, linux-iio, linux-kernel
On 3/2/26 7:06 AM, Archit Anant wrote:
> Reading the regulator voltage via regulator_get_voltage() can be a slow
> operation. Since the reference voltages for this ADC are not expected to
> change at runtime, it is inefficient to query the regulator API every
> time userspace reads the IIO_CHAN_INFO_SCALE attribute.
>
> Cache the VCC and VREF voltages in the state structure during probe().
> This improves the performance of ad799x_read_raw() and removes the
> dependency on the regulator pointers during fast-path reads.
>
> Suggested-by: Jonathan Cameron <jic23@kernel.org>
> Signed-off-by: Archit Anant <architanant5@gmail.com>
> ---
> drivers/iio/adc/ad799x.c | 17 +++++++++++++++--
> 1 file changed, 15 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/iio/adc/ad799x.c b/drivers/iio/adc/ad799x.c
> index 7775be874081..35e0589428d0 100644
> --- a/drivers/iio/adc/ad799x.c
> +++ b/drivers/iio/adc/ad799x.c
> @@ -135,6 +135,10 @@ struct ad799x_state {
> u16 config;
>
> unsigned int transfer_size;
> +
> + int vcc_uv;
> + int vref_uv;
> +
> IIO_DECLARE_DMA_BUFFER_WITH_TS(__be16, rx_buf, AD799X_MAX_CHANNELS);
> };
>
> @@ -303,9 +307,9 @@ static int ad799x_read_raw(struct iio_dev *indio_dev,
> return IIO_VAL_INT;
> case IIO_CHAN_INFO_SCALE:
> if (st->vref)
> - ret = regulator_get_voltage(st->vref);
> + ret = st->vref_uv;
> else
> - ret = regulator_get_voltage(st->reg);
> + ret = st->vcc_uv;
>
> if (ret < 0)
> return ret;
This can be simplified even more. See reply to v2.
We could even move the if statement to probe and only
add one state variable.
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v3 0/4] iio: adc: ad799x: modernize resource management
2026-03-02 16:28 ` [PATCH v3 0/4] iio: adc: ad799x: modernize resource management David Lechner
@ 2026-03-03 5:11 ` Archit Anant
0 siblings, 0 replies; 14+ messages in thread
From: Archit Anant @ 2026-03-03 5:11 UTC (permalink / raw)
To: David Lechner
Cc: jic23, lars, Michael.Hennerich, nuno.sa, andy, linux-iio,
linux-kernel
Hi David,
On Mon, Mar 2, 2026 at 9:58 PM David Lechner <dlechner@baylibre.com> wrote:
>
> On 3/2/26 7:06 AM, Archit Anant wrote:
> > This series modernizes the ad799x driver by converting all resource
> > management to use the devm_ infrastructure, ultimately allowing for the
> > complete removal of the ad799x_remove() function and simplifying the
> > error paths in probe.
> >
> > This series addresses the feedback from v2, where the changes were
> > squashed into a single patch, by splitting the logical steps into
> > individual, bisectable commits. It also incorporates feedback regarding
> > the safe ordering of regulator disablement and IIO device unregistration.
> >
> > Changes in v3:
> > - Split the monolithic v2 patch into 4 logical commits as requested by
> > Andy Shevchenko.
> >
> > Changes in v2:
> > - Incorporated feedback from David Lechner to eliminate dynamic buffer
> > allocation.
> > - Incorporated feedback from Jonathan Cameron regarding the ordering
> > hazards of mixing devm and manual teardown, utilizing devm_add_action
> > and voltage caching.
> >
>
> I just reviewed v2 because I didn't see there was a v3 yet. This is why
> we ask to wait ideally 1 week, but at a few days before sending a new
> revision.
I agree, I should have waited longer before sending the next revision.
After Andy's feedback on v2, I realized that the monolithic patch was
difficult to review and wanted to get the split series out as quickly as
possible to save reviewers' time. However, I see now that sending it too
fast caused a different kind of confusion.
>
> Some of those comment will still apply to v3.
I will wait a few days to collect all feedback on v3 before preparing
and sending v4.
Thanks for the patience and the review!
--
Sincerely,
Archit Anant
^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2026-03-03 5:11 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-03-02 13:06 [PATCH v3 0/4] iio: adc: ad799x: modernize resource management Archit Anant
2026-03-02 13:06 ` [PATCH v3 1/4] iio: adc: ad799x: make rx_buf static and DMA-safe Archit Anant
2026-03-02 16:29 ` David Lechner
2026-03-02 16:31 ` David Lechner
2026-03-02 13:06 ` [PATCH v3 2/4] iio: adc: ad799x: cache regulator voltages during probe Archit Anant
2026-03-02 13:13 ` Andy Shevchenko
2026-03-02 16:37 ` David Lechner
2026-03-02 13:06 ` [PATCH v3 3/4] iio: adc: ad799x: use devm_add_action_or_reset for regulators Archit Anant
2026-03-02 13:16 ` Andy Shevchenko
2026-03-02 16:35 ` David Lechner
2026-03-02 13:06 ` [PATCH v3 4/4] iio: adc: ad799x: use devm_iio_device_register and drop remove Archit Anant
2026-03-02 13:17 ` Andy Shevchenko
2026-03-02 16:28 ` [PATCH v3 0/4] iio: adc: ad799x: modernize resource management David Lechner
2026-03-03 5:11 ` Archit Anant
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox