[PATCH] iio: trigger: iio-trig-interrupt: use devm_* helpers

[PATCH] iio: trigger: iio-trig-interrupt: use devm_* helpers

[Stepan Ionichev]

Convert to devm_iio_trigger_alloc(), devm_request_irq() and
devm_iio_trigger_register(). The driver-managed lifecycle removes
the manual error-cleanup ladder in probe and the entire .remove
callback.

Drop the now-unused iio_interrupt_trigger_info structure: its
only purpose was to hold the IRQ number for the manual free_irq()
call in .remove, which is no longer needed. The matching
linux/slab.h include is also dropped.

No functional change.

Signed-off-by: Stepan Ionichev <sozdayvek@gmail.com>
---
 drivers/iio/trigger/iio-trig-interrupt.c | 63 ++++--------------------
 1 file changed, 9 insertions(+), 54 deletions(-)

diff --git a/drivers/iio/trigger/iio-trig-interrupt.c b/drivers/iio/trigger/iio-trig-interrupt.c
index a100c2e3a..e7db61fed 100644
--- a/drivers/iio/trigger/iio-trig-interrupt.c
+++ b/drivers/iio/trigger/iio-trig-interrupt.c
@@ -9,16 +9,11 @@
 #include <linux/module.h>
 #include <linux/platform_device.h>
 #include <linux/interrupt.h>
-#include <linux/slab.h>
 
 #include <linux/iio/iio.h>
 #include <linux/iio/trigger.h>
 
 
-struct iio_interrupt_trigger_info {
-	unsigned int irq;
-};
-
 static irqreturn_t iio_interrupt_trigger_poll(int irq, void *private)
 {
 	iio_trigger_poll(private);
@@ -27,11 +22,10 @@ static irqreturn_t iio_interrupt_trigger_poll(int irq, void *private)
 
 static int iio_interrupt_trigger_probe(struct platform_device *pdev)
 {
-	struct iio_interrupt_trigger_info *trig_info;
 	struct iio_trigger *trig;
 	unsigned long irqflags;
 	struct resource *irq_res;
-	int irq, ret = 0;
+	int irq, ret;
 
 	irq_res = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
 
@@ -42,61 +36,22 @@ static int iio_interrupt_trigger_probe(struct platform_device *pdev)
 
 	irq = irq_res->start;
 
-	trig = iio_trigger_alloc(NULL, "irqtrig%d", irq);
-	if (!trig) {
-		ret = -ENOMEM;
-		goto error_ret;
-	}
+	trig = devm_iio_trigger_alloc(&pdev->dev, "irqtrig%d", irq);
+	if (!trig)
+		return -ENOMEM;
 
-	trig_info = kzalloc_obj(*trig_info);
-	if (!trig_info) {
-		ret = -ENOMEM;
-		goto error_free_trigger;
-	}
-	iio_trigger_set_drvdata(trig, trig_info);
-	trig_info->irq = irq;
-	ret = request_irq(irq, iio_interrupt_trigger_poll,
-			  irqflags, trig->name, trig);
+	ret = devm_request_irq(&pdev->dev, irq, iio_interrupt_trigger_poll,
+			       irqflags, trig->name, trig);
 	if (ret) {
-		dev_err(&pdev->dev,
-			"request IRQ-%d failed", irq);
-		goto error_free_trig_info;
+		dev_err(&pdev->dev, "request IRQ-%d failed", irq);
+		return ret;
 	}
 
-	ret = iio_trigger_register(trig);
-	if (ret)
-		goto error_release_irq;
-	platform_set_drvdata(pdev, trig);
-
-	return 0;
-
-/* First clean up the partly allocated trigger */
-error_release_irq:
-	free_irq(irq, trig);
-error_free_trig_info:
-	kfree(trig_info);
-error_free_trigger:
-	iio_trigger_free(trig);
-error_ret:
-	return ret;
-}
-
-static void iio_interrupt_trigger_remove(struct platform_device *pdev)
-{
-	struct iio_trigger *trig;
-	struct iio_interrupt_trigger_info *trig_info;
-
-	trig = platform_get_drvdata(pdev);
-	trig_info = iio_trigger_get_drvdata(trig);
-	iio_trigger_unregister(trig);
-	free_irq(trig_info->irq, trig);
-	kfree(trig_info);
-	iio_trigger_free(trig);
+	return devm_iio_trigger_register(&pdev->dev, trig);
 }
 
 static struct platform_driver iio_interrupt_trigger_driver = {
 	.probe = iio_interrupt_trigger_probe,
-	.remove = iio_interrupt_trigger_remove,
 	.driver = {
 		.name = "iio_interrupt_trigger",
 	},
-- 
2.43.0

[PATCH v2] iio: trigger: iio-trig-interrupt: use devm_* helpers

[Stepan Ionichev]

Convert to devm_iio_trigger_alloc(), devm_request_irq() and
devm_iio_trigger_register(). The driver-managed lifecycle removes
the manual error-cleanup ladder in probe and the entire .remove()
callback.

Drop the now-unused iio_interrupt_trigger_info structure: its
only purpose was to hold the IRQ number for the manual free_irq()
call in .remove(), which is no longer needed. The matching
linux/slab.h include is also dropped.

Signed-off-by: Stepan Ionichev <sozdayvek@gmail.com>
---
v2:
- Drop the dev_err() call after devm_request_irq(); really_probe()
  in the driver core already logs probe failures, so the message
  would be duplicate (per Andy)
- Use a local struct device *dev = &pdev->dev (per Joshua)
- Drop the "No functional change" claim from the commit message;
  the resource lifecycle model changes (per Joshua)
- Use .remove() function notation in the commit message (per Andy)

 drivers/iio/trigger/iio-trig-interrupt.c | 64 ++++--------------------
 1 file changed, 9 insertions(+), 55 deletions(-)

diff --git a/drivers/iio/trigger/iio-trig-interrupt.c b/drivers/iio/trigger/iio-trig-interrupt.c
index a100c2e3a..ddd80ff82 100644
--- a/drivers/iio/trigger/iio-trig-interrupt.c
+++ b/drivers/iio/trigger/iio-trig-interrupt.c
@@ -9,16 +9,11 @@
 #include <linux/module.h>
 #include <linux/platform_device.h>
 #include <linux/interrupt.h>
-#include <linux/slab.h>
 
 #include <linux/iio/iio.h>
 #include <linux/iio/trigger.h>
 
 
-struct iio_interrupt_trigger_info {
-	unsigned int irq;
-};
-
 static irqreturn_t iio_interrupt_trigger_poll(int irq, void *private)
 {
 	iio_trigger_poll(private);
@@ -27,11 +22,11 @@ static irqreturn_t iio_interrupt_trigger_poll(int irq, void *private)
 
 static int iio_interrupt_trigger_probe(struct platform_device *pdev)
 {
-	struct iio_interrupt_trigger_info *trig_info;
+	struct device *dev = &pdev->dev;
 	struct iio_trigger *trig;
 	unsigned long irqflags;
 	struct resource *irq_res;
-	int irq, ret = 0;
+	int irq, ret;
 
 	irq_res = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
 
@@ -42,61 +37,20 @@ static int iio_interrupt_trigger_probe(struct platform_device *pdev)
 
 	irq = irq_res->start;
 
-	trig = iio_trigger_alloc(NULL, "irqtrig%d", irq);
-	if (!trig) {
-		ret = -ENOMEM;
-		goto error_ret;
-	}
-
-	trig_info = kzalloc_obj(*trig_info);
-	if (!trig_info) {
-		ret = -ENOMEM;
-		goto error_free_trigger;
-	}
-	iio_trigger_set_drvdata(trig, trig_info);
-	trig_info->irq = irq;
-	ret = request_irq(irq, iio_interrupt_trigger_poll,
-			  irqflags, trig->name, trig);
-	if (ret) {
-		dev_err(&pdev->dev,
-			"request IRQ-%d failed", irq);
-		goto error_free_trig_info;
-	}
+	trig = devm_iio_trigger_alloc(dev, "irqtrig%d", irq);
+	if (!trig)
+		return -ENOMEM;
 
-	ret = iio_trigger_register(trig);
+	ret = devm_request_irq(dev, irq, iio_interrupt_trigger_poll,
+			       irqflags, trig->name, trig);
 	if (ret)
-		goto error_release_irq;
-	platform_set_drvdata(pdev, trig);
-
-	return 0;
-
-/* First clean up the partly allocated trigger */
-error_release_irq:
-	free_irq(irq, trig);
-error_free_trig_info:
-	kfree(trig_info);
-error_free_trigger:
-	iio_trigger_free(trig);
-error_ret:
-	return ret;
-}
-
-static void iio_interrupt_trigger_remove(struct platform_device *pdev)
-{
-	struct iio_trigger *trig;
-	struct iio_interrupt_trigger_info *trig_info;
+		return ret;
 
-	trig = platform_get_drvdata(pdev);
-	trig_info = iio_trigger_get_drvdata(trig);
-	iio_trigger_unregister(trig);
-	free_irq(trig_info->irq, trig);
-	kfree(trig_info);
-	iio_trigger_free(trig);
+	return devm_iio_trigger_register(dev, trig);
 }
 
 static struct platform_driver iio_interrupt_trigger_driver = {
 	.probe = iio_interrupt_trigger_probe,
-	.remove = iio_interrupt_trigger_remove,
 	.driver = {
 		.name = "iio_interrupt_trigger",
 	},
-- 
2.43.0

Re: [PATCH v2] iio: trigger: iio-trig-interrupt: use devm_* helpers

[Joshua Crofts]

On Mon, 11 May 2026 at 15:47, Stepan Ionichev <sozdayvek@gmail.com> wrote:
>
> Convert to devm_iio_trigger_alloc(), devm_request_irq() and
> devm_iio_trigger_register(). The driver-managed lifecycle removes
> the manual error-cleanup ladder in probe and the entire .remove()
> callback.
>
> Drop the now-unused iio_interrupt_trigger_info structure: its
> only purpose was to hold the IRQ number for the manual free_irq()
> call in .remove(), which is no longer needed. The matching
> linux/slab.h include is also dropped.
>
> Signed-off-by: Stepan Ionichev <sozdayvek@gmail.com>

I know they might be trivial changes, but please slow down when sending
new versions - I didn't even finish writing a follow up message to you and
already got a new version in my inbox. Ideally wait at least 24 hours before
sending, thanks.

(FYI Stepan, you will receive this message twice as I initially only sent it
to you).

--
Kind regards

CJD

On Mon, 11 May 2026 at 15:47, Stepan Ionichev <sozdayvek@gmail.com> wrote:
>
> Convert to devm_iio_trigger_alloc(), devm_request_irq() and
> devm_iio_trigger_register(). The driver-managed lifecycle removes
> the manual error-cleanup ladder in probe and the entire .remove()
> callback.
>
> Drop the now-unused iio_interrupt_trigger_info structure: its
> only purpose was to hold the IRQ number for the manual free_irq()
> call in .remove(), which is no longer needed. The matching
> linux/slab.h include is also dropped.
>
> Signed-off-by: Stepan Ionichev <sozdayvek@gmail.com>
> ---
> v2:
> - Drop the dev_err() call after devm_request_irq(); really_probe()
>   in the driver core already logs probe failures, so the message
>   would be duplicate (per Andy)
> - Use a local struct device *dev = &pdev->dev (per Joshua)
> - Drop the "No functional change" claim from the commit message;
>   the resource lifecycle model changes (per Joshua)
> - Use .remove() function notation in the commit message (per Andy)
>
>  drivers/iio/trigger/iio-trig-interrupt.c | 64 ++++--------------------
>  1 file changed, 9 insertions(+), 55 deletions(-)
>
> diff --git a/drivers/iio/trigger/iio-trig-interrupt.c b/drivers/iio/trigger/iio-trig-interrupt.c
> index a100c2e3a..ddd80ff82 100644
> --- a/drivers/iio/trigger/iio-trig-interrupt.c
> +++ b/drivers/iio/trigger/iio-trig-interrupt.c
> @@ -9,16 +9,11 @@
>  #include <linux/module.h>
>  #include <linux/platform_device.h>
>  #include <linux/interrupt.h>
> -#include <linux/slab.h>
>
>  #include <linux/iio/iio.h>
>  #include <linux/iio/trigger.h>
>
>
> -struct iio_interrupt_trigger_info {
> -       unsigned int irq;
> -};
> -
>  static irqreturn_t iio_interrupt_trigger_poll(int irq, void *private)
>  {
>         iio_trigger_poll(private);
> @@ -27,11 +22,11 @@ static irqreturn_t iio_interrupt_trigger_poll(int irq, void *private)
>
>  static int iio_interrupt_trigger_probe(struct platform_device *pdev)
>  {
> -       struct iio_interrupt_trigger_info *trig_info;
> +       struct device *dev = &pdev->dev;
>         struct iio_trigger *trig;
>         unsigned long irqflags;
>         struct resource *irq_res;
> -       int irq, ret = 0;
> +       int irq, ret;
>
>         irq_res = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
>
> @@ -42,61 +37,20 @@ static int iio_interrupt_trigger_probe(struct platform_device *pdev)
>
>         irq = irq_res->start;
>
> -       trig = iio_trigger_alloc(NULL, "irqtrig%d", irq);
> -       if (!trig) {
> -               ret = -ENOMEM;
> -               goto error_ret;
> -       }
> -
> -       trig_info = kzalloc_obj(*trig_info);
> -       if (!trig_info) {
> -               ret = -ENOMEM;
> -               goto error_free_trigger;
> -       }
> -       iio_trigger_set_drvdata(trig, trig_info);
> -       trig_info->irq = irq;
> -       ret = request_irq(irq, iio_interrupt_trigger_poll,
> -                         irqflags, trig->name, trig);
> -       if (ret) {
> -               dev_err(&pdev->dev,
> -                       "request IRQ-%d failed", irq);
> -               goto error_free_trig_info;
> -       }
> +       trig = devm_iio_trigger_alloc(dev, "irqtrig%d", irq);
> +       if (!trig)
> +               return -ENOMEM;
>
> -       ret = iio_trigger_register(trig);
> +       ret = devm_request_irq(dev, irq, iio_interrupt_trigger_poll,
> +                              irqflags, trig->name, trig);
>         if (ret)
> -               goto error_release_irq;
> -       platform_set_drvdata(pdev, trig);
> -
> -       return 0;
> -
> -/* First clean up the partly allocated trigger */
> -error_release_irq:
> -       free_irq(irq, trig);
> -error_free_trig_info:
> -       kfree(trig_info);
> -error_free_trigger:
> -       iio_trigger_free(trig);
> -error_ret:
> -       return ret;
> -}
> -
> -static void iio_interrupt_trigger_remove(struct platform_device *pdev)
> -{
> -       struct iio_trigger *trig;
> -       struct iio_interrupt_trigger_info *trig_info;
> +               return ret;
>
> -       trig = platform_get_drvdata(pdev);
> -       trig_info = iio_trigger_get_drvdata(trig);
> -       iio_trigger_unregister(trig);
> -       free_irq(trig_info->irq, trig);
> -       kfree(trig_info);
> -       iio_trigger_free(trig);
> +       return devm_iio_trigger_register(dev, trig);
>  }
>
>  static struct platform_driver iio_interrupt_trigger_driver = {
>         .probe = iio_interrupt_trigger_probe,
> -       .remove = iio_interrupt_trigger_remove,
>         .driver = {
>                 .name = "iio_interrupt_trigger",
>         },
> --
> 2.43.0
>

Re: [PATCH v2] iio: trigger: iio-trig-interrupt: use devm_* helpers

[Jonathan Cameron]

On Mon, 11 May 2026 11:32:29 +0500
Stepan Ionichev <sozdayvek@gmail.com> wrote:

> Convert to devm_iio_trigger_alloc(), devm_request_irq() and
> devm_iio_trigger_register(). The driver-managed lifecycle removes
> the manual error-cleanup ladder in probe and the entire .remove()
> callback.
> 
> Drop the now-unused iio_interrupt_trigger_info structure: its
> only purpose was to hold the IRQ number for the manual free_irq()
> call in .remove(), which is no longer needed. The matching
> linux/slab.h include is also dropped.
> 
> Signed-off-by: Stepan Ionichev <sozdayvek@gmail.com>

Silly question for you - how does this driver actually bind on a modern
platform?  That is - how do we get one of these? 


> ---
> v2:
> - Drop the dev_err() call after devm_request_irq(); really_probe()
>   in the driver core already logs probe failures, so the message
>   would be duplicate (per Andy)
> - Use a local struct device *dev = &pdev->dev (per Joshua)
> - Drop the "No functional change" claim from the commit message;
>   the resource lifecycle model changes (per Joshua)
> - Use .remove() function notation in the commit message (per Andy)
> 
>  drivers/iio/trigger/iio-trig-interrupt.c | 64 ++++--------------------
>  1 file changed, 9 insertions(+), 55 deletions(-)
> 
> diff --git a/drivers/iio/trigger/iio-trig-interrupt.c b/drivers/iio/trigger/iio-trig-interrupt.c
> index a100c2e3a..ddd80ff82 100644
> --- a/drivers/iio/trigger/iio-trig-interrupt.c
> +++ b/drivers/iio/trigger/iio-trig-interrupt.c
> @@ -9,16 +9,11 @@
>  #include <linux/module.h>
>  #include <linux/platform_device.h>
>  #include <linux/interrupt.h>
> -#include <linux/slab.h>
>  
>  #include <linux/iio/iio.h>
>  #include <linux/iio/trigger.h>
>  
>  
> -struct iio_interrupt_trigger_info {
> -	unsigned int irq;
> -};
> -
>  static irqreturn_t iio_interrupt_trigger_poll(int irq, void *private)
>  {
>  	iio_trigger_poll(private);
> @@ -27,11 +22,11 @@ static irqreturn_t iio_interrupt_trigger_poll(int irq, void *private)
>  
>  static int iio_interrupt_trigger_probe(struct platform_device *pdev)
>  {
> -	struct iio_interrupt_trigger_info *trig_info;
> +	struct device *dev = &pdev->dev;
>  	struct iio_trigger *trig;
>  	unsigned long irqflags;
>  	struct resource *irq_res;
> -	int irq, ret = 0;
> +	int irq, ret;
>  
>  	irq_res = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
>  
> @@ -42,61 +37,20 @@ static int iio_interrupt_trigger_probe(struct platform_device *pdev)
>  
>  	irq = irq_res->start;
>  
> -	trig = iio_trigger_alloc(NULL, "irqtrig%d", irq);
> -	if (!trig) {
> -		ret = -ENOMEM;
> -		goto error_ret;
> -	}
> -
> -	trig_info = kzalloc_obj(*trig_info);
> -	if (!trig_info) {
> -		ret = -ENOMEM;
> -		goto error_free_trigger;
> -	}
> -	iio_trigger_set_drvdata(trig, trig_info);
> -	trig_info->irq = irq;
> -	ret = request_irq(irq, iio_interrupt_trigger_poll,
> -			  irqflags, trig->name, trig);
> -	if (ret) {
> -		dev_err(&pdev->dev,
> -			"request IRQ-%d failed", irq);
> -		goto error_free_trig_info;
> -	}
> +	trig = devm_iio_trigger_alloc(dev, "irqtrig%d", irq);

Passing a parent to the underlying iio_trigger_alloc() is a functional change
that needs to be clearly laid out.  Do that as a precursor patch where you
can then describe the effects that has.  Honestly I can't remember so I'd
like to see some analysis presented before I look at it!


> +	if (!trig)
> +		return -ENOMEM;
>  
> -	ret = iio_trigger_register(trig);
> +	ret = devm_request_irq(dev, irq, iio_interrupt_trigger_poll,
> +			       irqflags, trig->name, trig);
>  	if (ret)
> -		goto error_release_irq;
> -	platform_set_drvdata(pdev, trig);
> -
> -	return 0;
> -
> -/* First clean up the partly allocated trigger */
> -error_release_irq:
> -	free_irq(irq, trig);
> -error_free_trig_info:
> -	kfree(trig_info);
> -error_free_trigger:
> -	iio_trigger_free(trig);
> -error_ret:
> -	return ret;
> -}
> -
> -static void iio_interrupt_trigger_remove(struct platform_device *pdev)
> -{
> -	struct iio_trigger *trig;
> -	struct iio_interrupt_trigger_info *trig_info;
> +		return ret;
>  
> -	trig = platform_get_drvdata(pdev);
> -	trig_info = iio_trigger_get_drvdata(trig);
> -	iio_trigger_unregister(trig);
> -	free_irq(trig_info->irq, trig);
> -	kfree(trig_info);
> -	iio_trigger_free(trig);
> +	return devm_iio_trigger_register(dev, trig);
>  }
>  
>  static struct platform_driver iio_interrupt_trigger_driver = {
>  	.probe = iio_interrupt_trigger_probe,
> -	.remove = iio_interrupt_trigger_remove,
>  	.driver = {
>  		.name = "iio_interrupt_trigger",
>  	},

Re: [PATCH v2] iio: trigger: iio-trig-interrupt: use devm_* helpers

[Andy Shevchenko]

On Mon, May 11, 2026 at 11:32:29AM +0500, Stepan Ionichev wrote:
> Convert to devm_iio_trigger_alloc(), devm_request_irq() and
> devm_iio_trigger_register(). The driver-managed lifecycle removes
> the manual error-cleanup ladder in probe and the entire .remove()
> callback.
> 
> Drop the now-unused iio_interrupt_trigger_info structure: its
> only purpose was to hold the IRQ number for the manual free_irq()
> call in .remove(), which is no longer needed. The matching
> linux/slab.h include is also dropped.

...

> Signed-off-by: Stepan Ionichev <sozdayvek@gmail.com>
> ---
> v2:
> - Drop the dev_err() call after devm_request_irq(); really_probe()
>   in the driver core already logs probe failures, so the message
>   would be duplicate (per Andy)

FWIW, it's IRQ core (not driver core!) that does printing.

> - Use a local struct device *dev = &pdev->dev (per Joshua)
> - Drop the "No functional change" claim from the commit message;
>   the resource lifecycle model changes (per Joshua)
> - Use .remove() function notation in the commit message (per Andy)

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH] iio: trigger: iio-trig-interrupt: use devm_* helpers

[Stepan Ionichev]

On Mon, May 11, 2026 at 18:46, Andy Shevchenko wrote:
> Looking at this and taking into account the above, have you checked the
> - trigger lifetime versus platform device
> - parent-child relationship in the sysfs
>
> The latter might lead to interesting side-effects.

You are right -- the parent change is the real concern here, not
just the cleanup ladder.

Checked the rest of IIO: there are 26 other devm_iio_trigger_alloc()
callers in drivers/iio/, and all of them pass a non-NULL parent
(typically &client->dev, &spi->dev or &indio_dev->dev). The
iio-trig-interrupt driver with NULL parent is the only exception.

devm_iio_trigger_alloc() requires a non-NULL parent for devres
tracking, so the conversion necessarily introduces one. The
trigger then becomes a child of the platform device both in the
device tree and in sysfs, and its lifetime is tied to pdev
unbind instead of the (former) manual .remove().

If preserving the parent-less behavior is required for userspace
or for any subtle reason I have not thought of, please drop both
v1 and v2 of this patch. I do not have the IIO trigger framework
background to argue that the parent change is safe.

If the conversion is acceptable (in line with the rest of IIO),
v2 already addresses the dev_err()/local-device/commit-message
nits.

Stepan

Re: [PATCH] iio: trigger: iio-trig-interrupt: use devm_* helpers

[Stepan Ionichev]

On Mon, May 11, 2026 at 18:54, Joshua Crofts wrote:
> I know they might be trivial changes, but please slow down when sending
> new versions - I didn't even finish writing a follow up message to you and
> already got a new version in my inbox. Ideally wait at least 24 hours before
> sending, thanks.

Sorry, you are right. I will wait at least 24 hours for further
review before sending the next version of this and any other
patch with open feedback. Noted for the future.

Stepan

Re: [PATCH] iio: trigger: iio-trig-interrupt: use devm_* helpers

[Joshua Crofts]

On Mon, 11 May 2026 at 15:23, Stepan Ionichev <sozdayvek@gmail.com> wrote:
>
> Convert to devm_iio_trigger_alloc(), devm_request_irq() and
> devm_iio_trigger_register(). The driver-managed lifecycle removes
> the manual error-cleanup ladder in probe and the entire .remove
> callback.
>
> Drop the now-unused iio_interrupt_trigger_info structure: its
> only purpose was to hold the IRQ number for the manual free_irq()
> call in .remove, which is no longer needed. The matching
> linux/slab.h include is also dropped.
>
> No functional change.

I would say that this is a functional change, you are changing
how resources are managed and how teardown works. (But
maybe I'm wrong).

> @@ -42,61 +36,22 @@ static int iio_interrupt_trigger_probe(struct platform_device *pdev)
>
>         irq = irq_res->start;
>
> -       trig = iio_trigger_alloc(NULL, "irqtrig%d", irq);
> -       if (!trig) {
> -               ret = -ENOMEM;
> -               goto error_ret;
> -       }
> +       trig = devm_iio_trigger_alloc(&pdev->dev, "irqtrig%d", irq);

You can deal with this in another patch, but consider introducing
a local struct device variable, so you don't have to use &pdev->dev
constantly. It makes the code easier to read and saves space!

> +       if (!trig)
> +               return -ENOMEM;
>
> -       trig_info = kzalloc_obj(*trig_info);
> -       if (!trig_info) {
> -               ret = -ENOMEM;
> -               goto error_free_trigger;
> -       }
> -       iio_trigger_set_drvdata(trig, trig_info);
> -       trig_info->irq = irq;
> -       ret = request_irq(irq, iio_interrupt_trigger_poll,
> -                         irqflags, trig->name, trig);
> +       ret = devm_request_irq(&pdev->dev, irq, iio_interrupt_trigger_poll,
> +                              irqflags, trig->name, trig);
>         if (ret) {
> -               dev_err(&pdev->dev,
> -                       "request IRQ-%d failed", irq);
> -               goto error_free_trig_info;
> +               dev_err(&pdev->dev, "request IRQ-%d failed", irq);
> +               return ret;

If I'm not mistaken, devm_request_irq automatically throws an error on
failure, so the dev_err() call is redundant, just keep the return.

-- 
Kind regards

CJD

Re: [PATCH] iio: trigger: iio-trig-interrupt: use devm_* helpers

[Stepan Ionichev]

On Mon, May 11, 2026 at 15:33 +0200, Joshua Crofts wrote:
> I would say that this is a functional change, you are changing
> how resources are managed and how teardown works. (But
> maybe I'm wrong).

Fair point -- the user-visible behavior on success or on module
unload is the same, but the lifetime/ownership model changes.
I will reword that in v2 ("convert to devm-managed lifecycle"
instead of "no functional change").

> consider introducing a local struct device variable, so you
> don't have to use &pdev->dev constantly. It makes the code
> easier to read and saves space!

OK, will add `struct device *dev = &pdev->dev;` in v2.

> If I'm not mistaken, devm_request_irq automatically throws
> an error on failure, so the dev_err() call is redundant,
> just keep the return.

I do not see this in the source. devm_request_irq() in
kernel/irq/devres.c wraps request_threaded_irq() with a
devres action; neither prints on failure. The caller still
needs the error message.

In v2 I can switch from dev_err() to dev_err_probe(), which
handles -EPROBE_DEFER cleanly:

    if (ret)
        return dev_err_probe(dev, ret,
                             "request IRQ-%d failed\n", irq);

If I missed where devm_request_irq() prints, please point me
to it.

Stepan

Re: [PATCH] iio: trigger: iio-trig-interrupt: use devm_* helpers

[Joshua Crofts]

On Mon, 11 May 2026 at 15:42, Stepan Ionichev <sozdayvek@gmail.com> wrote:
>
> On Mon, May 11, 2026 at 15:33 +0200, Joshua Crofts wrote:
> > I would say that this is a functional change, you are changing
> > how resources are managed and how teardown works. (But
> > maybe I'm wrong).
>
> Fair point -- the user-visible behavior on success or on module
> unload is the same, but the lifetime/ownership model changes.
> I will reword that in v2 ("convert to devm-managed lifecycle"
> instead of "no functional change").
>
> > consider introducing a local struct device variable, so you
> > don't have to use &pdev->dev constantly. It makes the code
> > easier to read and saves space!
>
> OK, will add `struct device *dev = &pdev->dev;` in v2.

You can do this in a separate patch though, maybe for the entire
driver.

> > If I'm not mistaken, devm_request_irq automatically throws
> > an error on failure, so the dev_err() call is redundant,
> > just keep the return.
>
> I do not see this in the source. devm_request_irq() in
> kernel/irq/devres.c wraps request_threaded_irq() with a
> devres action; neither prints on failure. The caller still
> needs the error message.
>
> In v2 I can switch from dev_err() to dev_err_probe(), which
> handles -EPROBE_DEFER cleanly:
>
>     if (ret)
>         return dev_err_probe(dev, ret,
>                              "request IRQ-%d failed\n", irq);
>
> If I missed where devm_request_irq() prints, please point me
> to it.

Link to the definition, it's described in the comments.
https://elixir.bootlin.com/linux/v7.1-rc2/source/kernel/irq/devres.c#L94

-- 
Kind regards

CJD

Re: [PATCH] iio: trigger: iio-trig-interrupt: use devm_* helpers

[Andy Shevchenko]

On Mon, May 11, 2026 at 03:33:06PM +0200, Joshua Crofts wrote:
> On Mon, 11 May 2026 at 15:23, Stepan Ionichev <sozdayvek@gmail.com> wrote:

...

> > No functional change.
> 
> I would say that this is a functional change, you are changing
> how resources are managed and how teardown works. (But
> maybe I'm wrong).

Actually yes, this is quite a change that might have some subtle differences.

...

> > -       trig = iio_trigger_alloc(NULL, "irqtrig%d", irq);
> > -       if (!trig) {
> > -               ret = -ENOMEM;
> > -               goto error_ret;
> > -       }
> > +       trig = devm_iio_trigger_alloc(&pdev->dev, "irqtrig%d", irq);

Looking at this and taking into account the above, have you checked the
- trigger lifetime versus platform device
- parent-child relationship in the sysfs

The latter might lead to interesting side-effects.

> > +       if (!trig)
> > +               return -ENOMEM;

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH] iio: trigger: iio-trig-interrupt: use devm_* helpers

[Andy Shevchenko]

On Mon, May 11, 2026 at 11:07:32AM +0500, Stepan Ionichev wrote:
> Convert to devm_iio_trigger_alloc(), devm_request_irq() and
> devm_iio_trigger_register(). The driver-managed lifecycle removes
> the manual error-cleanup ladder in probe and the entire .remove

.remove()

> callback.
> 
> Drop the now-unused iio_interrupt_trigger_info structure: its
> only purpose was to hold the IRQ number for the manual free_irq()
> call in .remove, which is no longer needed. The matching

.remove()

> linux/slab.h include is also dropped.
> 
> No functional change.

...

> +	ret = devm_request_irq(&pdev->dev, irq, iio_interrupt_trigger_poll,
> +			       irqflags, trig->name, trig);
>  	if (ret) {
> -		dev_err(&pdev->dev,
> -			"request IRQ-%d failed", irq);
> -		goto error_free_trig_info;

> +		dev_err(&pdev->dev, "request IRQ-%d failed", irq);

No, drop this now duplicate message.

> +		return ret;
>  	}

-- 
With Best Regards,
Andy Shevchenko