[PATCH v2] watchdog: qcom: Propagate errors from optional IRQ lookup

[PATCH v2] watchdog: qcom: Propagate errors from optional IRQ lookup

[phucduc.bui]

From: bui duc phuc <phucduc.bui@gmail.com>

platform_get_irq_optional() returns a positive IRQ number on success or
a negative error code on failure. For an optional IRQ, -ENXIO indicates
that no IRQ is available, while other errors should be propagated.

Instead of only checking for -EPROBE_DEFER, propagate all error codes
returned by platform_get_irq_optional() other than -ENXIO, so that
failures are properly reported to the caller.

Signed-off-by: bui duc phuc <phucduc.bui@gmail.com>
---

Link v1 : 
https://lore.kernel.org/all/20260807081652.38692-1-phucduc.bui@gmail.com/

Changes in v2 : 
 - Update pretimeout handling based on Guenter's review.

 drivers/watchdog/qcom-wdt.c | 22 ++++++++++++----------
 1 file changed, 12 insertions(+), 10 deletions(-)

diff --git a/drivers/watchdog/qcom-wdt.c b/drivers/watchdog/qcom-wdt.c
index 49bd04841f0c..9fee18fda750 100644
--- a/drivers/watchdog/qcom-wdt.c
+++ b/drivers/watchdog/qcom-wdt.c
@@ -289,18 +289,20 @@ static int qcom_wdt_probe(struct platform_device *pdev)
 
 	/* check if there is pretimeout support */
 	irq = platform_get_irq_optional(pdev, 0);
-	if (data->pretimeout && irq > 0) {
-		ret = devm_request_irq(dev, irq, qcom_wdt_isr, 0,
-				       "wdt_bark", &wdt->wdd);
-		if (ret)
-			return ret;
+	if (data->pretimeout) {
+		irq = platform_get_irq_optional(pdev, 0);
+		if (irq < 0 && irq != -ENXIO)
+			return irq;
+		if (irq > 0) {
+			ret = devm_request_irq(dev, irq, qcom_wdt_isr, 0,
+					       "wdt_bark", &wdt->wdd);
+			if (ret)
+				return ret;
 
-		wdt->wdd.info = &qcom_wdt_pt_info;
-		wdt->wdd.pretimeout = 1;
+			wdt->wdd.info = &qcom_wdt_pt_info;
+			wdt->wdd.pretimeout = 1;
+		}
 	} else {
-		if (irq == -EPROBE_DEFER)
-			return -EPROBE_DEFER;
-
 		wdt->wdd.info = &qcom_wdt_info;
 	}
 
-- 
2.43.0

Re: [PATCH v2] watchdog: qcom: Propagate errors from optional IRQ lookup

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Missing initialization of `wdt->wdd.info` when the optional pretimeout IRQ is absent but pretimeout is supported.
- [Low] Redundant sequential calls to `platform_get_irq_optional()`.
--

--- Patch [1]: [PATCH v2] watchdog: qcom: Propagate errors from optional IRQ lookup ---
commit bf01bac4df56d04f8ab925257d4dfcb55e1e6adf
Author: bui duc phuc <phucduc.bui@gmail.com>

watchdog: qcom: Propagate errors from optional IRQ lookup

This commit modifies the qcom watchdog driver to propagate errors returned
by platform_get_irq_optional(), rather than only checking for -EPROBE_DEFER.

Link: https://lore.kernel.org/all/20260807081652.38692-1-phucduc.bui@gmail.com/

> diff --git a/drivers/watchdog/qcom-wdt.c b/drivers/watchdog/qcom-wdt.c
> index a425902b9f68d..5c16420751631 100644
> --- a/drivers/watchdog/qcom-wdt.c
> +++ b/drivers/watchdog/qcom-wdt.c
> @@ -305,18 +305,20 @@ static int qcom_wdt_probe(struct platform_device *pdev)
>  
>  	/* check if there is pretimeout support */
>  	irq = platform_get_irq_optional(pdev, 0);
> -	if (data->pretimeout && irq > 0) {
> +	if (data->pretimeout) {
> +		irq = platform_get_irq_optional(pdev, 0);

[Severity: Low]
This isn't a bug, but is the first call to platform_get_irq_optional() still
needed?

It appears that if data->pretimeout is true, the result is immediately 
overwritten by the second call. If data->pretimeout is false, the irq value is
ignored by the else branch.

> +		if (irq < 0 && irq != -ENXIO)
> +			return irq;
> +		if (irq > 0) {
> +			ret = devm_request_irq(dev, irq, qcom_wdt_isr, 0,
> +					       "wdt_bark", &wdt->wdd);
> +			if (ret)
> +				return ret;
> +
> +			wdt->wdd.info = &qcom_wdt_pt_info;
> +			wdt->wdd.pretimeout = 1;
> +		}

[Severity: High]
Does this code leave wdt->wdd.info uninitialized if data->pretimeout is true
but the optional IRQ is missing?

If platform_get_irq_optional() returns -ENXIO, the if (irq > 0) branch is
skipped. Since this all occurs inside the if (data->pretimeout) branch, the
outer else block is bypassed.

Could this cause devm_watchdog_register_device() to later fail with -EINVAL
because wdd->info is NULL?

>  	} else {
> -		if (irq == -EPROBE_DEFER)
> -			return -EPROBE_DEFER;
> -
>  		wdt->wdd.info = &qcom_wdt_info;
>  	}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260810102144.51663-1-phucduc.bui@gmail.com?part=1

Re: [PATCH v2] watchdog: qcom: Propagate errors from optional IRQ lookup

[Bui Duc Phuc]

>         /* check if there is pretimeout support */
>         irq = platform_get_irq_optional(pdev, 0);
> -       if (data->pretimeout && irq > 0) {
> -               ret = devm_request_irq(dev, irq, qcom_wdt_isr, 0,
> -                                      "wdt_bark", &wdt->wdd);
> -               if (ret)
> -                       return ret;
> +       if (data->pretimeout) {
> +               irq = platform_get_irq_optional(pdev, 0);

Sorry, I missed removing the old irq = platform_get_irq_optional(pdev,
0); line. I will send v3 shortly.

Re: [PATCH v2] watchdog: qcom: Propagate errors from optional IRQ lookup

[Bui Duc Phuc]

> [Severity: High]
> Does this code leave wdt->wdd.info uninitialized if data->pretimeout is true
> but the optional IRQ is missing?
>
> If platform_get_irq_optional() returns -ENXIO, the if (irq > 0) branch is
> skipped. Since this all occurs inside the if (data->pretimeout) branch, the
> outer else block is bypassed.
>
> Could this cause devm_watchdog_register_device() to later fail with -EINVAL
> because wdd->info is NULL?
>
> >       } else {
> > -             if (irq == -EPROBE_DEFER)
> > -                     return -EPROBE_DEFER;
> > -
> >               wdt->wdd.info = &qcom_wdt_info;
> >       }
>
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/20260810102144.51663-1-phucduc.bui@gmail.com?part=1


Oh, it looks like this approach is correct.

Option C (default-then-override, only look up the IRQ when pretimeout
is supported):

----------------------
wdt->wdd.info = &qcom_wdt_info;

if (data->pretimeout) {
        irq = platform_get_irq_optional(pdev, 0);
        if(irq < 0 && irq != -ENXIO)
                return irq;

        if (irq > 0){
                ret = devm_request_irq(dev, irq, qcom_wdt_isr, 0,
                                                       "wdt_bark", &wdt->wdd);
                if(ret)
                        return ret;

                wdt->wdd.info= &qcom_wdt_pt_info;
                wdt->wdd.pretimeout = 1;
        }
}
-----------------------