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

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

Pre-existing issues:
- [High] The probe function silently ignores probe deferral and other errors from platform_get_irq_optional().
- [High] The driver incorrectly modifies a static global watchdog_info structure during device probe.
--

--- Patch [6]: [PATCH 6/9] watchdog: orion: Remove redundant dev_err() ---
commit 820e86c1dcc7da8eb296b1915c4c9eb8f9dc8666
Author: Pan Chuang <[email protected]>

watchdog: orion: Remove redundant dev_err()

This commit removes redundant dev_err() messages in the driver, as
devm_request_irq() automatically logs errors on failure.

> diff --git a/drivers/watchdog/orion_wdt.c b/drivers/watchdog/orion_wdt.c
> index 0e145f762f6f2..1802634cc7cda 100644
> --- a/drivers/watchdog/orion_wdt.c
> +++ b/drivers/watchdog/orion_wdt.c
> @@ -613,10 +613,8 @@ static int orion_wdt_probe(struct platform_device *pdev)
>  		 */
>  		ret = devm_request_irq(&pdev->dev, irq, orion_wdt_irq, 0,
>  				       pdev->name, dev);

[Severity: High]
This isn't a bug introduced by this patch, but does orion_wdt_probe() silently
ignore probe deferrals when getting these interrupts?

Earlier in this function, the primary and secondary interrupts are fetched:

    irq = platform_get_irq_optional(pdev, 0);
    if (irq > 0) {

If the interrupt controller is not yet ready, platform_get_irq_optional()
returns -EPROBE_DEFER. Because of the if (irq > 0) check, we silently skip
the interrupts instead of propagating the error and deferring the probe. This
can cause the watchdog to permanently drop its interrupt-driven capabilities on
boot.

> -		if (ret < 0) {
> -			dev_err(&pdev->dev, "failed to request IRQ\n");
> +		if (ret < 0)
>  			goto disable_clk;
> -		}
>  	}
>  
>  	/* Optional 2nd interrupt for pretimeout */
> @@ -625,10 +623,8 @@ static int orion_wdt_probe(struct platform_device *pdev)
>  		orion_wdt_info.options |= WDIOF_PRETIMEOUT;

[Severity: High]
This is a pre-existing issue, but does this permanently corrupt the static
global orion_wdt_info struct for the lifetime of the module?

If the driver successfully finds a pretimeout interrupt during probe, it
permanently modifies the shared static global orion_wdt_info structure by
setting WDIOF_PRETIMEOUT.

If the device is subsequently unbound and re-bound to a hardware configuration
without the interrupt, or if multiple SoC instances exist with differing
interrupt capabilities, the driver will inherit the corrupted global state and
falsely advertise the pretimeout feature to userspace.

>  		ret = devm_request_irq(&pdev->dev, irq, orion_wdt_pre_irq,
>  				       0, pdev->name, dev);
> -		if (ret < 0) {
> -			dev_err(&pdev->dev, "failed to request IRQ\n");
> +		if (ret < 0)
>  			goto disable_clk;
> -		}
>  	}

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