Re: [PATCH 3/4] watchdog: dw_wdt: 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:

Pre-existing issues:
- [High] Hardware register access and shared IRQ registration occur before device is taken out of reset, which can cause an external abort or bus stall. This problem wasn't introduced by this patch, but it is a critical hardware lifecycle violation.
--

--- Patch [3]: [PATCH 3/4] watchdog: dw_wdt: Propagate errors from optional IRQ lookup ---
commit 415108e73d198e9211e81c51098b9fd10a005104
Author: bui duc phuc <[email protected]>

watchdog: dw_wdt: Propagate errors from optional IRQ lookup

This commit modifies dw_wdt_drv_probe() to propagate all error codes
returned by platform_get_irq_optional() other than -ENXIO. This ensures
failures are properly reported to the caller instead of only checking
for -EPROBE_DEFER.

> diff --git a/drivers/watchdog/dw_wdt.c b/drivers/watchdog/dw_wdt.c
> index c3fbb6068c520..39031445d754b 100644
> --- a/drivers/watchdog/dw_wdt.c
> +++ b/drivers/watchdog/dw_wdt.c
> @@ -611,7 +611,7 @@ static int dw_wdt_drv_probe(struct platform_device *pdev)
>  
>  		dw_wdt->wdd.info = &dw_wdt_pt_ident;
>  	} else {
> -		if (ret == -EPROBE_DEFER)
> +		if (ret != -ENXIO)
>  			return ret;
>  
>  		dw_wdt->wdd.info = &dw_wdt_ident;

[Severity: High]
This is a pre-existing issue, but does this function perform register accesses
and register a shared interrupt before the device is taken out of reset?

Looking at the sequence of operations in dw_wdt_drv_probe():

dw_wdt_drv_probe() {
        ...
        /* Enable normal reset without pre-timeout by default. */
        dw_wdt_update_mode(dw_wdt, DW_WDT_RMOD_RESET);
        ...
        ret = devm_request_irq(dev, ret, dw_wdt_irq, IRQF_SHARED ...);
        ...
        reset_control_deassert(dw_wdt->rst);
        ...
}

If the hardware starts in a reset state, could the register I/O inside
dw_wdt_update_mode() trigger a synchronous external abort or system hang
because it occurs before reset_control_deassert() is called?

Additionally, since devm_request_irq() is called with IRQF_SHARED before
the reset is deasserted, if another device on the same IRQ line triggers
an interrupt, would dw_wdt_irq() execute and attempt to read the watchdog
interrupt status register while the device is still held in reset?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=3
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.