* [PATCH v1 1/1] iio: core: Simplify IIO core managed APIs
@ 2026-02-16 8:14 Andy Shevchenko
2026-02-16 14:31 ` Nuno Sá
2026-02-20 10:18 ` Jonathan Cameron
0 siblings, 2 replies; 10+ messages in thread
From: Andy Shevchenko @ 2026-02-16 8:14 UTC (permalink / raw)
To: Andy Shevchenko, linux-iio, linux-kernel
Cc: Jonathan Cameron, David Lechner, Nuno Sá, Andy Shevchenko
Use devm_add_action_or_reset() instead of devres_alloc() and
devres_add(), which works the same. This will simplify the
code. There is no functional changes.
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
drivers/iio/buffer/kfifo_buf.c | 29 +++++++++++++----------------
drivers/iio/industrialio-trigger.c | 24 ++++++++++--------------
2 files changed, 23 insertions(+), 30 deletions(-)
diff --git a/drivers/iio/buffer/kfifo_buf.c b/drivers/iio/buffer/kfifo_buf.c
index 38034c8bcc04..a126cc05fb38 100644
--- a/drivers/iio/buffer/kfifo_buf.c
+++ b/drivers/iio/buffer/kfifo_buf.c
@@ -224,9 +224,9 @@ void iio_kfifo_free(struct iio_buffer *r)
}
EXPORT_SYMBOL(iio_kfifo_free);
-static void devm_iio_kfifo_release(struct device *dev, void *res)
+static void devm_iio_kfifo_release(void *r)
{
- iio_kfifo_free(*(struct iio_buffer **)res);
+ iio_kfifo_free(r);
}
/**
@@ -234,23 +234,20 @@ static void devm_iio_kfifo_release(struct device *dev, void *res)
* @dev: Device to allocate kfifo buffer for
*
* RETURNS:
- * Pointer to allocated iio_buffer on success, NULL on failure.
+ * Pointer to allocated iio_buffer on success, error pointer on failure.
*/
static struct iio_buffer *devm_iio_kfifo_allocate(struct device *dev)
{
- struct iio_buffer **ptr, *r;
-
- ptr = devres_alloc(devm_iio_kfifo_release, sizeof(*ptr), GFP_KERNEL);
- if (!ptr)
- return NULL;
+ struct iio_buffer *r;
+ int ret;
r = iio_kfifo_allocate();
- if (r) {
- *ptr = r;
- devres_add(dev, ptr);
- } else {
- devres_free(ptr);
- }
+ if (!r)
+ return ERR_PTR(-ENOMEM);
+
+ ret = devm_add_action_or_reset(dev, devm_iio_kfifo_release, r);
+ if (ret)
+ return ERR_PTR(ret);
return r;
}
@@ -275,8 +272,8 @@ int devm_iio_kfifo_buffer_setup_ext(struct device *dev,
struct iio_buffer *buffer;
buffer = devm_iio_kfifo_allocate(dev);
- if (!buffer)
- return -ENOMEM;
+ if (IS_ERR(buffer))
+ return PTR_ERR(buffer);
indio_dev->modes |= INDIO_BUFFER_SOFTWARE;
indio_dev->setup_ops = setup_ops;
diff --git a/drivers/iio/industrialio-trigger.c b/drivers/iio/industrialio-trigger.c
index 54416a384232..00c8b7e5c8b9 100644
--- a/drivers/iio/industrialio-trigger.c
+++ b/drivers/iio/industrialio-trigger.c
@@ -634,9 +634,9 @@ void iio_trigger_free(struct iio_trigger *trig)
}
EXPORT_SYMBOL(iio_trigger_free);
-static void devm_iio_trigger_release(struct device *dev, void *res)
+static void devm_iio_trigger_release(void *trig)
{
- iio_trigger_free(*(struct iio_trigger **)res);
+ iio_trigger_free(trig);
}
/**
@@ -658,24 +658,20 @@ struct iio_trigger *__devm_iio_trigger_alloc(struct device *parent,
struct module *this_mod,
const char *fmt, ...)
{
- struct iio_trigger **ptr, *trig;
+ struct iio_trigger *trig;
va_list vargs;
-
- ptr = devres_alloc(devm_iio_trigger_release, sizeof(*ptr),
- GFP_KERNEL);
- if (!ptr)
- return NULL;
+ int ret;
/* use raw alloc_dr for kmalloc caller tracing */
va_start(vargs, fmt);
trig = viio_trigger_alloc(parent, this_mod, fmt, vargs);
va_end(vargs);
- if (trig) {
- *ptr = trig;
- devres_add(parent, ptr);
- } else {
- devres_free(ptr);
- }
+ if (!trig)
+ return NULL;
+
+ ret = devm_add_action_or_reset(parent, devm_iio_trigger_release, trig);
+ if (ret)
+ return NULL;
return trig;
}
--
2.50.1
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATCH v1 1/1] iio: core: Simplify IIO core managed APIs
2026-02-16 8:14 [PATCH v1 1/1] iio: core: Simplify IIO core managed APIs Andy Shevchenko
@ 2026-02-16 14:31 ` Nuno Sá
2026-02-17 7:45 ` Andy Shevchenko
2026-02-20 10:18 ` Jonathan Cameron
1 sibling, 1 reply; 10+ messages in thread
From: Nuno Sá @ 2026-02-16 14:31 UTC (permalink / raw)
To: Andy Shevchenko, linux-iio, linux-kernel
Cc: Jonathan Cameron, David Lechner, Nuno Sá, Andy Shevchenko
On Mon, 2026-02-16 at 09:14 +0100, Andy Shevchenko wrote:
> Use devm_add_action_or_reset() instead of devres_alloc() and
> devres_add(), which works the same. This will simplify the
> code. There is no functional changes.
>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
LGTM. Just one minor "complain". Anyways:
Reviewed-by: Nuno Sá <nuno.sa@analog.com>
> drivers/iio/buffer/kfifo_buf.c | 29 +++++++++++++----------------
> drivers/iio/industrialio-trigger.c | 24 ++++++++++--------------
> 2 files changed, 23 insertions(+), 30 deletions(-)
>
> diff --git a/drivers/iio/buffer/kfifo_buf.c b/drivers/iio/buffer/kfifo_buf.c
> index 38034c8bcc04..a126cc05fb38 100644
> --- a/drivers/iio/buffer/kfifo_buf.c
> +++ b/drivers/iio/buffer/kfifo_buf.c
> @@ -224,9 +224,9 @@ void iio_kfifo_free(struct iio_buffer *r)
> }
> EXPORT_SYMBOL(iio_kfifo_free);
>
> -static void devm_iio_kfifo_release(struct device *dev, void *res)
> +static void devm_iio_kfifo_release(void *r)
> {
> - iio_kfifo_free(*(struct iio_buffer **)res);
> + iio_kfifo_free(r);
> }
>
> /**
> @@ -234,23 +234,20 @@ static void devm_iio_kfifo_release(struct device *dev, void *res)
> * @dev: Device to allocate kfifo buffer for
> *
> * RETURNS:
> - * Pointer to allocated iio_buffer on success, NULL on failure.
> + * Pointer to allocated iio_buffer on success, error pointer on failure.
> */
> static struct iio_buffer *devm_iio_kfifo_allocate(struct device *dev)
> {
> - struct iio_buffer **ptr, *r;
> -
> - ptr = devres_alloc(devm_iio_kfifo_release, sizeof(*ptr), GFP_KERNEL);
> - if (!ptr)
> - return NULL;
> + struct iio_buffer *r;
> + int ret;
>
> r = iio_kfifo_allocate();
> - if (r) {
> - *ptr = r;
> - devres_add(dev, ptr);
> - } else {
> - devres_free(ptr);
> - }
> + if (!r)
> + return ERR_PTR(-ENOMEM);
> +
> + ret = devm_add_action_or_reset(dev, devm_iio_kfifo_release, r);
> + if (ret)
> + return ERR_PTR(ret);
>
> return r;
> }
> @@ -275,8 +272,8 @@ int devm_iio_kfifo_buffer_setup_ext(struct device *dev,
> struct iio_buffer *buffer;
>
> buffer = devm_iio_kfifo_allocate(dev);
> - if (!buffer)
> - return -ENOMEM;
> + if (IS_ERR(buffer))
> + return PTR_ERR(buffer);
Subtle change that could have been mentioned in the commit message.
- Nuno Sá
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH v1 1/1] iio: core: Simplify IIO core managed APIs
2026-02-16 14:31 ` Nuno Sá
@ 2026-02-17 7:45 ` Andy Shevchenko
2026-02-17 13:29 ` Nuno Sá
0 siblings, 1 reply; 10+ messages in thread
From: Andy Shevchenko @ 2026-02-17 7:45 UTC (permalink / raw)
To: Nuno Sá
Cc: linux-iio, linux-kernel, Jonathan Cameron, David Lechner,
Nuno Sá, Andy Shevchenko
On Mon, Feb 16, 2026 at 02:31:57PM +0000, Nuno Sá wrote:
> On Mon, 2026-02-16 at 09:14 +0100, Andy Shevchenko wrote:
> > Use devm_add_action_or_reset() instead of devres_alloc() and
> > devres_add(), which works the same. This will simplify the
> > code. There is no functional changes.
> LGTM. Just one minor "complain". Anyways:
My answer below.
> Reviewed-by: Nuno Sá <nuno.sa@analog.com>
Thanks!
...
> > - if (!buffer)
> > - return -ENOMEM;
> > + if (IS_ERR(buffer))
> > + return PTR_ERR(buffer);
>
> Subtle change that could have been mentioned in the commit message.
Actually not really a change. Currently the error code is shadowed to -ENOMEM,
but again currently the callee can't return anything else. So, it's just a use
of the regular pattern. TL;DR: there is no functional change currently. If
anything in the future comes returning different error codes here, that will
bring the difference.
Anyways, if Jonathan asks to elaborate and send a new version, I will do for
sure.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v1 1/1] iio: core: Simplify IIO core managed APIs
2026-02-17 7:45 ` Andy Shevchenko
@ 2026-02-17 13:29 ` Nuno Sá
2026-02-17 13:32 ` Andy Shevchenko
0 siblings, 1 reply; 10+ messages in thread
From: Nuno Sá @ 2026-02-17 13:29 UTC (permalink / raw)
To: Andy Shevchenko
Cc: linux-iio, linux-kernel, Jonathan Cameron, David Lechner,
Nuno Sá, Andy Shevchenko
On Tue, 2026-02-17 at 09:45 +0200, Andy Shevchenko wrote:
> On Mon, Feb 16, 2026 at 02:31:57PM +0000, Nuno Sá wrote:
> > On Mon, 2026-02-16 at 09:14 +0100, Andy Shevchenko wrote:
> > > Use devm_add_action_or_reset() instead of devres_alloc() and
> > > devres_add(), which works the same. This will simplify the
> > > code. There is no functional changes.
>
> > LGTM. Just one minor "complain". Anyways:
>
> My answer below.
>
> > Reviewed-by: Nuno Sá <nuno.sa@analog.com>
>
> Thanks!
>
> ...
>
> > > - if (!buffer)
> > > - return -ENOMEM;
> > > + if (IS_ERR(buffer))
> > > + return PTR_ERR(buffer);
> >
> > Subtle change that could have been mentioned in the commit message.
>
> Actually not really a change. Currently the error code is shadowed to -ENOMEM,
Well before that was explicit. Now it also depends on the internals of __devm_add_action().
That is why I said it was subtle. But yes, no functional change.
- Nuno Sá
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v1 1/1] iio: core: Simplify IIO core managed APIs
2026-02-17 13:29 ` Nuno Sá
@ 2026-02-17 13:32 ` Andy Shevchenko
2026-02-20 10:09 ` Nuno Sá
0 siblings, 1 reply; 10+ messages in thread
From: Andy Shevchenko @ 2026-02-17 13:32 UTC (permalink / raw)
To: Nuno Sá
Cc: linux-iio, linux-kernel, Jonathan Cameron, David Lechner,
Nuno Sá, Andy Shevchenko
On Tue, Feb 17, 2026 at 01:29:07PM +0000, Nuno Sá wrote:
> On Tue, 2026-02-17 at 09:45 +0200, Andy Shevchenko wrote:
> > On Mon, Feb 16, 2026 at 02:31:57PM +0000, Nuno Sá wrote:
> > > On Mon, 2026-02-16 at 09:14 +0100, Andy Shevchenko wrote:
...
> > > > - if (!buffer)
> > > > - return -ENOMEM;
> > > > + if (IS_ERR(buffer))
> > > > + return PTR_ERR(buffer);
> > >
> > > Subtle change that could have been mentioned in the commit message.
> >
> > Actually not really a change. Currently the error code is shadowed to -ENOMEM,
>
> Well before that was explicit. Now it also depends on the internals of __devm_add_action().
True, but if that ever changed, we will have a new error code coming from
->probe() and that's fine. ->probe() is not stricted to the certain error codes.
> That is why I said it was subtle. But yes, no functional change.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v1 1/1] iio: core: Simplify IIO core managed APIs
2026-02-17 13:32 ` Andy Shevchenko
@ 2026-02-20 10:09 ` Nuno Sá
0 siblings, 0 replies; 10+ messages in thread
From: Nuno Sá @ 2026-02-20 10:09 UTC (permalink / raw)
To: Andy Shevchenko
Cc: linux-iio, linux-kernel, Jonathan Cameron, David Lechner,
Nuno Sá, Andy Shevchenko
On Tue, 2026-02-17 at 15:32 +0200, Andy Shevchenko wrote:
> On Tue, Feb 17, 2026 at 01:29:07PM +0000, Nuno Sá wrote:
> > On Tue, 2026-02-17 at 09:45 +0200, Andy Shevchenko wrote:
> > > On Mon, Feb 16, 2026 at 02:31:57PM +0000, Nuno Sá wrote:
> > > > On Mon, 2026-02-16 at 09:14 +0100, Andy Shevchenko wrote:
>
> ...
>
> > > > > - if (!buffer)
> > > > > - return -ENOMEM;
> > > > > + if (IS_ERR(buffer))
> > > > > + return PTR_ERR(buffer);
> > > >
> > > > Subtle change that could have been mentioned in the commit message.
> > >
> > > Actually not really a change. Currently the error code is shadowed to -ENOMEM,
> >
> > Well before that was explicit. Now it also depends on the internals of __devm_add_action().
>
> True, but if that ever changed, we will have a new error code coming from
> ->probe() and that's fine. ->probe() is not stricted to the certain error codes.
Note that I'm not complaining about the change. Just that it could have been mentioned
in the commit :)
- Nuno Sá
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v1 1/1] iio: core: Simplify IIO core managed APIs
2026-02-16 8:14 [PATCH v1 1/1] iio: core: Simplify IIO core managed APIs Andy Shevchenko
2026-02-16 14:31 ` Nuno Sá
@ 2026-02-20 10:18 ` Jonathan Cameron
2026-02-20 10:27 ` Andy Shevchenko
1 sibling, 1 reply; 10+ messages in thread
From: Jonathan Cameron @ 2026-02-20 10:18 UTC (permalink / raw)
To: Andy Shevchenko
Cc: linux-iio, linux-kernel, David Lechner, Nuno Sá,
Andy Shevchenko
On Mon, 16 Feb 2026 09:14:12 +0100
Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:
> Use devm_add_action_or_reset() instead of devres_alloc() and
> devres_add(), which works the same. This will simplify the
> code. There is no functional changes.
>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Nice. There was a mass sweep up of these cases a few years
back but I guess these instances slipped through the scripting
that was used to find those.
> ---
> drivers/iio/buffer/kfifo_buf.c | 29 +++++++++++++----------------
> drivers/iio/industrialio-trigger.c | 24 ++++++++++--------------
> 2 files changed, 23 insertions(+), 30 deletions(-)
>
> diff --git a/drivers/iio/buffer/kfifo_buf.c b/drivers/iio/buffer/kfifo_buf.c
> index 38034c8bcc04..a126cc05fb38 100644
> --- a/drivers/iio/buffer/kfifo_buf.c
> +++ b/drivers/iio/buffer/kfifo_buf.c
> @@ -234,23 +234,20 @@ static void devm_iio_kfifo_release(struct device *dev, void *res)
> * @dev: Device to allocate kfifo buffer for
> *
> * RETURNS:
> - * Pointer to allocated iio_buffer on success, NULL on failure.
> + * Pointer to allocated iio_buffer on success, error pointer on failure.
This had me briefly confused as I failed to notice it was static
(no idea why I gave such a simple internal function kernel-doc!)
However that brings the obvious follow up question. Given your
simplification does having a helper make sense given only a single caller?
Let's just squash it and have the two calls inline + some resulting docs
updates to get rid of the references to this function.
Thanks,
Jonathan
> */
> static struct iio_buffer *devm_iio_kfifo_allocate(struct device *dev)
> {
> - struct iio_buffer **ptr, *r;
> -
> - ptr = devres_alloc(devm_iio_kfifo_release, sizeof(*ptr), GFP_KERNEL);
> - if (!ptr)
> - return NULL;
> + struct iio_buffer *r;
> + int ret;
>
> r = iio_kfifo_allocate();
> - if (r) {
> - *ptr = r;
> - devres_add(dev, ptr);
> - } else {
> - devres_free(ptr);
> - }
> + if (!r)
> + return ERR_PTR(-ENOMEM);
> +
> + ret = devm_add_action_or_reset(dev, devm_iio_kfifo_release, r);
> + if (ret)
> + return ERR_PTR(ret);
>
> return r;
> }
> @@ -275,8 +272,8 @@ int devm_iio_kfifo_buffer_setup_ext(struct device *dev,
> struct iio_buffer *buffer;
>
> buffer = devm_iio_kfifo_allocate(dev);
> - if (!buffer)
> - return -ENOMEM;
> + if (IS_ERR(buffer))
> + return PTR_ERR(buffer);
>
> indio_dev->modes |= INDIO_BUFFER_SOFTWARE;
> indio_dev->setup_ops = setup_ops;
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH v1 1/1] iio: core: Simplify IIO core managed APIs
2026-02-20 10:18 ` Jonathan Cameron
@ 2026-02-20 10:27 ` Andy Shevchenko
2026-02-20 12:14 ` Jonathan Cameron
0 siblings, 1 reply; 10+ messages in thread
From: Andy Shevchenko @ 2026-02-20 10:27 UTC (permalink / raw)
To: Jonathan Cameron
Cc: Andy Shevchenko, linux-iio, linux-kernel, David Lechner,
Nuno Sá, Andy Shevchenko
On Fri, Feb 20, 2026 at 12:18 PM Jonathan Cameron <jic23@kernel.org> wrote:
> On Mon, 16 Feb 2026 09:14:12 +0100
> Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:
...
> This had me briefly confused as I failed to notice it was static
> (no idea why I gave such a simple internal function kernel-doc!)
>
> However that brings the obvious follow up question. Given your
> simplification does having a helper make sense given only a single caller?
>
> Let's just squash it and have the two calls inline + some resulting docs
> updates to get rid of the references to this function.
Do you want to have this change inside the proposed patch? Or in a
separate (pre/post) one?
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v1 1/1] iio: core: Simplify IIO core managed APIs
2026-02-20 10:27 ` Andy Shevchenko
@ 2026-02-20 12:14 ` Jonathan Cameron
2026-02-20 13:29 ` Andy Shevchenko
0 siblings, 1 reply; 10+ messages in thread
From: Jonathan Cameron @ 2026-02-20 12:14 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Andy Shevchenko, linux-iio, linux-kernel, David Lechner,
Nuno Sá, Andy Shevchenko
On Fri, 20 Feb 2026 12:27:40 +0200
Andy Shevchenko <andy.shevchenko@gmail.com> wrote:
> On Fri, Feb 20, 2026 at 12:18 PM Jonathan Cameron <jic23@kernel.org> wrote:
> > On Mon, 16 Feb 2026 09:14:12 +0100
> > Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:
>
> ...
>
> > This had me briefly confused as I failed to notice it was static
> > (no idea why I gave such a simple internal function kernel-doc!)
> >
> > However that brings the obvious follow up question. Given your
> > simplification does having a helper make sense given only a single caller?
> >
> > Let's just squash it and have the two calls inline + some resulting docs
> > updates to get rid of the references to this function.
>
> Do you want to have this change inside the proposed patch? Or in a
> separate (pre/post) one?
Same patch would be fine I think. It's small and coupled to the code
getting simpler because of the main change here. Seems like a pointless
dance to refactor same code twice.
Jonathan
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v1 1/1] iio: core: Simplify IIO core managed APIs
2026-02-20 12:14 ` Jonathan Cameron
@ 2026-02-20 13:29 ` Andy Shevchenko
0 siblings, 0 replies; 10+ messages in thread
From: Andy Shevchenko @ 2026-02-20 13:29 UTC (permalink / raw)
To: Jonathan Cameron
Cc: Andy Shevchenko, linux-iio, linux-kernel, David Lechner,
Nuno Sá, Andy Shevchenko
On Fri, Feb 20, 2026 at 12:14:50PM +0000, Jonathan Cameron wrote:
> On Fri, 20 Feb 2026 12:27:40 +0200
> Andy Shevchenko <andy.shevchenko@gmail.com> wrote:
> > On Fri, Feb 20, 2026 at 12:18 PM Jonathan Cameron <jic23@kernel.org> wrote:
> > > On Mon, 16 Feb 2026 09:14:12 +0100
> > > Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:
...
> > > This had me briefly confused as I failed to notice it was static
> > > (no idea why I gave such a simple internal function kernel-doc!)
> > >
> > > However that brings the obvious follow up question. Given your
> > > simplification does having a helper make sense given only a single caller?
> > >
> > > Let's just squash it and have the two calls inline + some resulting docs
> > > updates to get rid of the references to this function.
> >
> > Do you want to have this change inside the proposed patch? Or in a
> > separate (pre/post) one?
> Same patch would be fine I think. It's small and coupled to the code
> getting simpler because of the main change here. Seems like a pointless
> dance to refactor same code twice.
v2 has just been sent.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2026-02-20 13:29 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-02-16 8:14 [PATCH v1 1/1] iio: core: Simplify IIO core managed APIs Andy Shevchenko
2026-02-16 14:31 ` Nuno Sá
2026-02-17 7:45 ` Andy Shevchenko
2026-02-17 13:29 ` Nuno Sá
2026-02-17 13:32 ` Andy Shevchenko
2026-02-20 10:09 ` Nuno Sá
2026-02-20 10:18 ` Jonathan Cameron
2026-02-20 10:27 ` Andy Shevchenko
2026-02-20 12:14 ` Jonathan Cameron
2026-02-20 13:29 ` Andy Shevchenko
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox