[PATCH] iio: adc: qcom-spmi-iadc: balance enable_irq_wake() on driver unbind

[PATCH] iio: adc: qcom-spmi-iadc: balance enable_irq_wake() on driver unbind

[Stepan Ionichev]

iadc_probe() calls enable_irq_wake() after a successful
devm_request_irq(), but the driver has no remove callback or
matching disable_irq_wake(), so the wake reference count on the
IRQ is leaked on module unload or driver unbind.

Check the IRQ request error first, then register a devm action
that calls disable_irq_wake() so the wake reference is released
in the same scope as the enable. While here, drop the inverted
"if (!ret) ... else return ret" in favour of the standard
"if (ret) return ret;" pattern.

Signed-off-by: Stepan Ionichev <sozdayvek@gmail.com>
---
 drivers/iio/adc/qcom-spmi-iadc.c | 18 +++++++++++++++---
 1 file changed, 15 insertions(+), 3 deletions(-)

diff --git a/drivers/iio/adc/qcom-spmi-iadc.c b/drivers/iio/adc/qcom-spmi-iadc.c
index b64a8a407..0ec3a0c4b 100644
--- a/drivers/iio/adc/qcom-spmi-iadc.c
+++ b/drivers/iio/adc/qcom-spmi-iadc.c
@@ -481,6 +481,11 @@ static const struct iio_chan_spec iadc_channels[] = {
 	},
 };
 
+static void iadc_disable_irq_wake(void *data)
+{
+	disable_irq_wake((unsigned long)data);
+}
+
 static int iadc_probe(struct platform_device *pdev)
 {
 	struct device_node *node = pdev->dev.of_node;
@@ -538,9 +543,16 @@ static int iadc_probe(struct platform_device *pdev)
 	if (!iadc->poll_eoc) {
 		ret = devm_request_irq(dev, irq_eoc, iadc_isr, 0,
 					"spmi-iadc", iadc);
-		if (!ret)
-			enable_irq_wake(irq_eoc);
-		else
+		if (ret)
+			return ret;
+
+		ret = enable_irq_wake(irq_eoc);
+		if (ret)
+			return ret;
+
+		ret = devm_add_action_or_reset(dev, iadc_disable_irq_wake,
+					       (void *)(unsigned long)irq_eoc);
+		if (ret)
 			return ret;
 	} else {
 		ret = devm_device_init_wakeup(iadc->dev);
-- 
2.43.0

Re: [PATCH] iio: adc: qcom-spmi-iadc: balance enable_irq_wake() on driver unbind

[Jonathan Cameron]

On Thu, 21 May 2026 00:09:24 +0500
Stepan Ionichev <sozdayvek@gmail.com> wrote:

> iadc_probe() calls enable_irq_wake() after a successful
> devm_request_irq(), but the driver has no remove callback or
> matching disable_irq_wake(), so the wake reference count on the
> IRQ is leaked on module unload or driver unbind.
> 
> Check the IRQ request error first, then register a devm action
> that calls disable_irq_wake() so the wake reference is released
> in the same scope as the enable. While here, drop the inverted
> "if (!ret) ... else return ret" in favour of the standard
> "if (ret) return ret;" pattern.
> 
> Signed-off-by: Stepan Ionichev <sozdayvek@gmail.com>

Looks mostly good - one trivial question inline.

> ---
>  drivers/iio/adc/qcom-spmi-iadc.c | 18 +++++++++++++++---
>  1 file changed, 15 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/iio/adc/qcom-spmi-iadc.c b/drivers/iio/adc/qcom-spmi-iadc.c
> index b64a8a407..0ec3a0c4b 100644
> --- a/drivers/iio/adc/qcom-spmi-iadc.c
> +++ b/drivers/iio/adc/qcom-spmi-iadc.c
> @@ -481,6 +481,11 @@ static const struct iio_chan_spec iadc_channels[] = {
>  	},
>  };
>  
> +static void iadc_disable_irq_wake(void *data)
> +{
> +	disable_irq_wake((unsigned long)data);
Why not cast it to an int given that's what disable_irq_wake() takes

> +}
> +
>  static int iadc_probe(struct platform_device *pdev)
>  {
>  	struct device_node *node = pdev->dev.of_node;
> @@ -538,9 +543,16 @@ static int iadc_probe(struct platform_device *pdev)
>  	if (!iadc->poll_eoc) {
>  		ret = devm_request_irq(dev, irq_eoc, iadc_isr, 0,
>  					"spmi-iadc", iadc);
> -		if (!ret)
> -			enable_irq_wake(irq_eoc);
> -		else
> +		if (ret)
> +			return ret;
> +
> +		ret = enable_irq_wake(irq_eoc);
> +		if (ret)
> +			return ret;
> +
> +		ret = devm_add_action_or_reset(dev, iadc_disable_irq_wake,
> +					       (void *)(unsigned long)irq_eoc);

What is the unsigned long bit of this cast giving us? Just casting to
void * should be enough I think.


> +		if (ret)
>  			return ret;
>  	} else {
>  		ret = devm_device_init_wakeup(iadc->dev);

Re: [PATCH] iio: adc: qcom-spmi-iadc: balance enable_irq_wake() on driver unbind

[Jonathan Cameron]

On Fri, 22 May 2026 15:58:49 +0100
Jonathan Cameron <jic23@kernel.org> wrote:

> On Thu, 21 May 2026 00:09:24 +0500
> Stepan Ionichev <sozdayvek@gmail.com> wrote:
> 
> > iadc_probe() calls enable_irq_wake() after a successful
> > devm_request_irq(), but the driver has no remove callback or
> > matching disable_irq_wake(), so the wake reference count on the
> > IRQ is leaked on module unload or driver unbind.
> > 
> > Check the IRQ request error first, then register a devm action
> > that calls disable_irq_wake() so the wake reference is released
> > in the same scope as the enable. While here, drop the inverted
> > "if (!ret) ... else return ret" in favour of the standard
> > "if (ret) return ret;" pattern.
> > 
> > Signed-off-by: Stepan Ionichev <sozdayvek@gmail.com>  
> 
> Looks mostly good - one trivial question inline.
> 
> > ---
> >  drivers/iio/adc/qcom-spmi-iadc.c | 18 +++++++++++++++---
> >  1 file changed, 15 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/iio/adc/qcom-spmi-iadc.c b/drivers/iio/adc/qcom-spmi-iadc.c
> > index b64a8a407..0ec3a0c4b 100644
> > --- a/drivers/iio/adc/qcom-spmi-iadc.c
> > +++ b/drivers/iio/adc/qcom-spmi-iadc.c
> > @@ -481,6 +481,11 @@ static const struct iio_chan_spec iadc_channels[] = {
> >  	},
> >  };
> >  
> > +static void iadc_disable_irq_wake(void *data)
> > +{
> > +	disable_irq_wake((unsigned long)data);  
> Why not cast it to an int given that's what disable_irq_wake() takes
> 
> > +}
> > +
> >  static int iadc_probe(struct platform_device *pdev)
> >  {
> >  	struct device_node *node = pdev->dev.of_node;
> > @@ -538,9 +543,16 @@ static int iadc_probe(struct platform_device *pdev)
> >  	if (!iadc->poll_eoc) {
> >  		ret = devm_request_irq(dev, irq_eoc, iadc_isr, 0,
> >  					"spmi-iadc", iadc);
> > -		if (!ret)
> > -			enable_irq_wake(irq_eoc);
> > -		else
> > +		if (ret)
> > +			return ret;
> > +
> > +		ret = enable_irq_wake(irq_eoc);
> > +		if (ret)
> > +			return ret;
> > +
> > +		ret = devm_add_action_or_reset(dev, iadc_disable_irq_wake,
> > +					       (void *)(unsigned long)irq_eoc);  
> 
> What is the unsigned long bit of this cast giving us? Just casting to
> void * should be enough I think.
The answer to this was size mismatch - so I was completely wrong ;(

So as I just replied to v2 this was correct, but could do with
a Fixes tag.

Thanks

Jonathan

> 
> 
> > +		if (ret)
> >  			return ret;
> >  	} else {
> >  		ret = devm_device_init_wakeup(iadc->dev);  
> 
>