Re: [PATCH v4 5/7] watchdog: mediatek: Add support for mt8167 toprgu/watchdog

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

Pre-existing issues:
- [High] Race condition in watchdog registration exposing uninitialized device properties.
--

--- Patch [5]: [PATCH v4 5/7] watchdog: mediatek: Add support for mt8167 toprgu/watchdog ---
commit 918b68ccd6f0fbc5fe13cfd642a3baccbeddc0ed
Author: Luca Leonardo Scorcia <[email protected]>

watchdog: mediatek: Add support for mt8167 toprgu/watchdog

This commit adds support for the Top Reset Generation Unit/Watchdog Timer
found on the mt8167 SoC. It uses a reset table to handle non-contiguous
reset bits and ensures the reset controller is registered with the correct
configuration.

> diff --git a/drivers/watchdog/mtk_wdt.c b/drivers/watchdog/mtk_wdt.c
> index f8208fb0f7234..fa522fd80dd45 100644
> --- a/drivers/watchdog/mtk_wdt.c
> +++ b/drivers/watchdog/mtk_wdt.c

[ ... ]

> @@ -457,12 +493,14 @@ static int mtk_wdt_probe(struct platform_device *pdev)
>  
>  	wdt_data = of_device_get_match_data(dev);
>  	if (wdt_data) {

[ ... ]

>  		err = toprgu_register_reset_controller(pdev,
>  						       wdt_data->toprgu_sw_rst_num);
>  		if (err)
>  			return err;
> -
> -		mtk_wdt->has_swsysrst_en = wdt_data->has_swsysrst_en;
>  	}
>  
>  	mtk_wdt->disable_wdt_extrst =

[Severity: High]
This is a pre-existing issue, but does the ordering in mtk_wdt_probe()
create a race condition with userspace?

Looking at the probe function, devm_watchdog_register_device() is called
and exposes the device to userspace before the hardware-specific properties
disable_wdt_extrst and reset_by_toprgu are populated.

If udev or another userspace process opens the watchdog device immediately
upon registration, could mtk_wdt_start() execute before these properties
are initialized?

If so, mtk_wdt_start() would evaluate the properties as false:

mtk_wdt_start() {
        ...
        if (mtk_wdt->disable_wdt_extrst)
                reg &= ~WDT_MODE_EXRST_EN;
        if (mtk_wdt->reset_by_toprgu)
                reg |= WDT_MODE_CNT_SEL;
        ...
}

Could this lead to incorrectly asserting external reset signals or failing
to trigger the proper TOPRGU reset path? Should the watchdog registration
be deferred until all device tree properties are fully parsed?

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