Re: [PATCH v2 2/7] watchdog: mediatek: Add wdt/toprgu resets for MT6589

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

Pre-existing issues:
- [High] Reset controller is synchronously registered before its structural configuration variable `has_swsysrst_en` is initialized.
--

--- Patch [2]: [PATCH v2 2/7] watchdog: mediatek: Add wdt/toprgu resets for MT6589 ---
commit 3b51cb7125d43b8f8ea3a7996ec1844775ec36b1
Author: Luca Leonardo Scorcia <[email protected]>

watchdog: mediatek: Add wdt/toprgu resets for MT6589

This commit populates the toprgu_sw_rst_num field for the MT6589 watchdog data.
It maps the 12 reset bits available in the WDT_SWSYSRST register to allow
toprgu resets via the device tree.

> diff --git a/drivers/watchdog/mtk_wdt.c b/drivers/watchdog/mtk_wdt.c
> index 91d110646e16f..e61f6ae743276 100644
> --- a/drivers/watchdog/mtk_wdt.c
> +++ b/drivers/watchdog/mtk_wdt.c
[ ... ]
> @@ -88,6 +89,10 @@ static const struct mtk_wdt_data mt2712_data = {
>  	.toprgu_sw_rst_num = MT2712_TOPRGU_SW_RST_NUM,
>  };
>  
> +static const struct mtk_wdt_data mt6589_data = {
> +	.toprgu_sw_rst_num = MT6589_TOPRGU_SW_RST_NUM,
> +};
> +
>  static const struct mtk_wdt_data mt6735_data = {
>  	.toprgu_sw_rst_num = MT6735_TOPRGU_RST_NUM,
>  };

[Severity: High]
This is a pre-existing issue, but registering the reset controller in
mtk_wdt_probe() happens before the structural configuration is fully
initialized:

drivers/watchdog/mtk_wdt.c:mtk_wdt_probe() {
    ...
	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;
	}
    ...
}

If a consumer driver probes concurrently and asserts the reset line, could it
execute before the has_swsysrst_en flag is set?

On SoCs requiring software reset enable, reading the uninitialized
has_swsysrst_en as false would cause the reset assertion to fail silently,
potentially leaving consumer devices in an undefined state.

Should mtk_wdt->has_swsysrst_en be initialized before calling
toprgu_register_reset_controller()?

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