Linux IIO development
 help / color / mirror / Atom feed
From: Jonathan Cameron <jic23@kernel.org>
To: Stepan Ionichev <sozdayvek@gmail.com>
Cc: dlechner@baylibre.com, nuno.sa@analog.com, andy@kernel.org,
	gregkh@linuxfoundation.org, hcazarim@yahoo.com,
	joshua.crofts1@gmail.com, linux-iio@vger.kernel.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2] iio: trigger: iio-trig-interrupt: use devm_* helpers
Date: Mon, 11 May 2026 15:56:00 +0100	[thread overview]
Message-ID: <20260511155600.13fc63ab@jic23-huawei> (raw)
In-Reply-To: <20260511063229.1433-1-sozdayvek@gmail.com>

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


  parent reply	other threads:[~2026-05-11 14:56 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-05-11  6:07 [PATCH] iio: trigger: iio-trig-interrupt: use devm_* helpers Stepan Ionichev
2026-05-11  6:32 ` [PATCH v2] " Stepan Ionichev
2026-05-11 13:53   ` Joshua Crofts
2026-05-11 14:56   ` Jonathan Cameron [this message]
2026-05-11 15:05   ` Andy Shevchenko
2026-05-11  6:36 ` [PATCH] " Stepan Ionichev
2026-05-11  6:44 ` Stepan Ionichev
2026-05-11 13:33 ` Joshua Crofts
2026-05-11  6:27   ` Stepan Ionichev
2026-05-11 13:48     ` Joshua Crofts
2026-05-11 13:46   ` Andy Shevchenko
2026-05-11 13:38 ` Andy Shevchenko

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260511155600.13fc63ab@jic23-huawei \
    --to=jic23@kernel.org \
    --cc=andy@kernel.org \
    --cc=dlechner@baylibre.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=hcazarim@yahoo.com \
    --cc=joshua.crofts1@gmail.com \
    --cc=linux-iio@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=nuno.sa@analog.com \
    --cc=sozdayvek@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox