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

[PATCH v3] 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.

Fixes: ce0694841ea6 ("iio: iadc: Qualcomm SPMI PMIC current ADC driver")
Signed-off-by: Stepan Ionichev <sozdayvek@gmail.com>
---
v3:
- Add Fixes tag (Jonathan)

v2: https://lore.kernel.org/all/20260523134613.4930-1-sozdayvek@gmail.com/
v1: https://lore.kernel.org/all/20260520190924.12774-1-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 b64a8a407168..88f6be2108e7 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((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 *)(long)irq_eoc);
+		if (ret)
 			return ret;
 	} else {
 		ret = devm_device_init_wakeup(iadc->dev);
-- 
2.43.0

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

[Jonathan Cameron]

On Fri, 29 May 2026 14:56:48 +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.
> 
> Fixes: ce0694841ea6 ("iio: iadc: Qualcomm SPMI PMIC current ADC driver")
> Signed-off-by: Stepan Ionichev <sozdayvek@gmail.com>

+CC Bjorn and Konrad as we don't seem to have a more specific
maintainer for this driver.

Seems low risk, but nice to run it past them.

Thanks,

Jonathan

> ---
> v3:
> - Add Fixes tag (Jonathan)
> 
> v2: https://lore.kernel.org/all/20260523134613.4930-1-sozdayvek@gmail.com/
> v1: https://lore.kernel.org/all/20260520190924.12774-1-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 b64a8a407168..88f6be2108e7 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((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 *)(long)irq_eoc);
> +		if (ret)
>  			return ret;
>  	} else {
>  		ret = devm_device_init_wakeup(iadc->dev);

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

[Konrad Dybcio]

On 7/4/26 1:04 AM, Jonathan Cameron wrote:
> On Fri, 29 May 2026 14:56:48 +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.
>>
>> Fixes: ce0694841ea6 ("iio: iadc: Qualcomm SPMI PMIC current ADC driver")
>> Signed-off-by: Stepan Ionichev <sozdayvek@gmail.com>
> 
> +CC Bjorn and Konrad as we don't seem to have a more specific
> maintainer for this driver.
> 
> Seems low risk, but nice to run it past them.

The robot pointed out to me that this makes probe fail if the
irqchip isn't wakeup-capable, but it seems like that's never the
case for the one that will be used (i.e. the QC SPMI bus arbiter)

so:

Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
 
Konrad

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

[Jonathan Cameron]

On Mon, 6 Jul 2026 13:25:48 +0200
Konrad Dybcio <konrad.dybcio@oss.qualcomm.com> wrote:

> On 7/4/26 1:04 AM, Jonathan Cameron wrote:
> > On Fri, 29 May 2026 14:56:48 +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.
> >>
> >> Fixes: ce0694841ea6 ("iio: iadc: Qualcomm SPMI PMIC current ADC driver")
> >> Signed-off-by: Stepan Ionichev <sozdayvek@gmail.com>  
> > 
> > +CC Bjorn and Konrad as we don't seem to have a more specific
> > maintainer for this driver.
> > 
> > Seems low risk, but nice to run it past them.  
> 
> The robot pointed out to me that this makes probe fail if the
> irqchip isn't wakeup-capable, but it seems like that's never the
> case for the one that will be used (i.e. the QC SPMI bus arbiter)
> 
> so:
> 
> Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
>  
I'm clearly going crazy.  Seems I picked up an earlier version and
failed to track that correctly.  It's already upstream and from a quick
eyeball is the same as this patch (I think I must have picked up v1
after v2 discussion showing that was right option).

Thanks anyway and at least this record will exist of that wakeup-capable
thing.

Jonathan

> Konrad