Re: [PATCH] watchdog: orion: Use of_device_get_match_data()

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The clock is explicitly disabled before the `devm`-managed interrupt is freed, leading to a potential bus fault or system hang. This problem wasn't introduced by this patch, but it is a pre-existing high-severity bug.
--

commit d9eb7a064a32ff51e0077728d87320936861ffad
Author: Rosen Penev <rosenp@gmail.com>

watchdog: orion: Use of_device_get_match_data()

Use of_device_get_match_data() to fetch watchdog match data directly
instead of open-coding an of_match_device() lookup.

> diff --git a/drivers/watchdog/orion_wdt.c b/drivers/watchdog/orion_wdt.c
> index 0e145f762f6f2..a92701ff26530 100644
> --- a/drivers/watchdog/orion_wdt.c
> +++ b/drivers/watchdog/orion_wdt.c
[ ... ]
> @@ -563,15 +561,14 @@ static int orion_wdt_probe(struct platform_device *pdev)
>  	if (!dev)
>  		return -ENOMEM;
>  
> -	match = of_match_device(orion_wdt_of_match_table, &pdev->dev);
> -	if (!match)
> +	dev->data = of_device_get_match_data(&pdev->dev);
> +	if (!dev->data)
>  		/* Default legacy match */
> -		match = &orion_wdt_of_match_table[0];
> +		dev->data = orion_wdt_of_match_table[0].data;
>  
>  	dev->wdt.info = &orion_wdt_info;
>  	dev->wdt.ops = &orion_wdt_ops;
>  	dev->wdt.min_timeout = 1;
> -	dev->data = match->data;
>  
>  	ret = orion_wdt_get_regs(pdev, dev);
>  	if (ret)

While reviewing this patch, I noticed a pre-existing issue in the probe
error path.

If watchdog_register_device() fails later in orion_wdt_probe(), the code
jumps to the disable_clk label:

        ret = watchdog_register_device(&dev->wdt);
        if (ret)
                goto disable_clk;

[ ... ]

disable_clk:
        clk_disable_unprepare(dev->clk);
        clk_put(dev->clk);
        return ret;

Since devm_request_irq() is used to allocate the interrupts, the devres
core will automatically free them only after the probe function returns.

Does this create a teardown window where the clock is explicitly disabled
but the interrupts remain active?

If a watchdog pre-timeout interrupt fires during this window, could the
handlers (orion_wdt_pre_irq or orion_wdt_irq) attempt to access
memory-mapped hardware registers while the clock is turned off, resulting
in a bus fault or system hang?

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