Re: [PATCH 9/9] watchdog: sprd: Remove redundant dev_err()

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Incorrect devres ordering leaves the IRQ handler active while device clocks are disabled, risking a Data Abort (kernel panic).
--

--- Patch [9]: [PATCH 9/9] watchdog: sprd: Remove redundant dev_err() ---
commit a68eda5f5b396a6d33ede72c1a4cc217df36d52f
Author: Pan Chuang <[email protected]>

watchdog: sprd: Remove redundant dev_err()

This commit removes a redundant driver-specific dev_err() call after
devm_request_irq(). The core already logs detailed error messages on
failure.

> diff --git a/drivers/watchdog/sprd_wdt.c b/drivers/watchdog/sprd_wdt.c
> index aacf04616fefe..616b4d7f21d78 100644
> --- a/drivers/watchdog/sprd_wdt.c
> +++ b/drivers/watchdog/sprd_wdt.c
> @@ -293,10 +293,8 @@ static int sprd_wdt_probe(struct platform_device *pdev)
>  
>  	ret = devm_request_irq(dev, wdt->irq, sprd_wdt_isr, IRQF_NO_SUSPEND,
>  			       "sprd-wdt", (void *)wdt);

[Severity: High]
This is a pre-existing issue, but I noticed a potential race condition
with the devres ordering.

Since devm_request_irq() is called before sprd_wdt_enable(), could a pending
interrupt (e.g., from the bootloader) trigger sprd_wdt_isr() while the device
clocks are still disabled?

Also, since devres teardown is LIFO, sprd_wdt_disable() (registered further
down via devm_add_action_or_reset()) will execute before the IRQ is freed
on probe failure or device removal. This leaves a window where the ISR could
fire and access registers with clocks disabled:

sprd_wdt_isr() {
	...
	sprd_wdt_unlock(wdt->base);
	writel_relaxed(SPRD_WDT_INT_CLEAR_BIT, wdt->base + SPRD_WDT_INT_CLR);
	...
}

Would it be safer to request the IRQ after the clocks are enabled and the
disable action is registered, to prevent a potential Data Abort?

> -	if (ret) {
> -		dev_err(dev, "failed to register irq\n");
> +	if (ret)
>  		return ret;
> -	}
>  
>  	wdt->wdd.info = &sprd_wdt_info;
>  	wdt->wdd.ops = &sprd_wdt_ops;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=9