Re: [PATCH 1/4] watchdog: qcom: Propagate errors from optional IRQ lookup

[email protected]
Newsgroups org.kernel.vger.linux-watchdog
Message-ID <[email protected]>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The probe function aborts and returns an invalid positive value if an optional IRQ is defined but the hardware variant does not support pretimeout.
--

--- Patch [1]: [PATCH 1/4] watchdog: qcom: Propagate errors from optional IRQ lookup ---
commit 29f7b7cd8d0f4e915e9e20f6974535c663f1a3b4
Author: bui duc phuc <[email protected]>

watchdog: qcom: Propagate errors from optional IRQ lookup

This commit updates the error handling for an optional interrupt lookup
in the Qualcomm watchdog probe. Instead of only checking for -EPROBE_DEFER,
it attempts to propagate all errors except -ENXIO to better report failures.

> diff --git a/drivers/watchdog/qcom-wdt.c b/drivers/watchdog/qcom-wdt.c
> index a425902b9f68d..2d9f79101b494 100644
> --- a/drivers/watchdog/qcom-wdt.c
> +++ b/drivers/watchdog/qcom-wdt.c
> @@ -314,8 +314,8 @@ static int qcom_wdt_probe(struct platform_device *pdev)
>  		wdt->wdd.info = &qcom_wdt_pt_info;
>  		wdt->wdd.pretimeout = 1;
>  	} else {
> -		if (irq == -EPROBE_DEFER)
> -			return -EPROBE_DEFER;
> +		if (irq != -ENXIO)
> +			return irq;

[Severity: High]
Will this cause the probe to fail on platforms where a valid IRQ is found
but pretimeout is disabled?

If platform_get_irq_optional() successfully finds an IRQ, it returns a
positive number. If the platform (such as APQ8064 or IPQ8064) provides an
interrupt in the device tree but the match data does not support pretimeout,
qcom_wdt_probe() execution falls into this else block.

Since the positive IRQ number does not equal -ENXIO, this condition evaluates
to true and the driver prematurely returns the positive IRQ number. 

The driver core treats any non-zero return from probe as an error, which
aborts the watchdog initialization completely leaving the system without
watchdog protection.

Should this condition be scoped to only catch negative error codes, for
example by checking if (irq < 0 && irq != -ENXIO) before returning?

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

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.