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@xxxxxxxxx> 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@xxxxxxxxx>

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