Re: [PATCH] watchdog: sunplus: fix sp_wdt_restart spin loop and timeout

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The infinite loop in the restart handler permanently deadlocks the restart notifier chain upon failure, preventing fallback reboot mechanisms, and introduces unreachable dead code.
--

--- Patch [1]: [PATCH] watchdog: sunplus: fix sp_wdt_restart spin loop and timeout ---
commit a82fb222c63d9d961d7ca0c200d75e43dd479310
Author: Andrew Gaylard <[email protected]>
watchdog: sunplus: fix sp_wdt_restart spin loop and timeout

This commit increases the watchdog restart timeout to ~1.5s and introduces a
spin loop in sp_wdt_restart() to prevent other CPUs from petting the watchdog
before the system resets.

> diff --git a/drivers/watchdog/sunplus_wdt.c b/drivers/watchdog/sunplus_wdt.c
> --- a/drivers/watchdog/sunplus_wdt.c
> +++ b/drivers/watchdog/sunplus_wdt.c
> @@ -56,10 +56,16 @@ static int sp_wdt_restart(struct watchdog_device *wdev,

[ ... ]

>  	writel(WDT_LOCK, base + WDT_CTRL);
>  	writel(WDT_RESUME, base + WDT_CTRL);
>  
> +	/* Spin until the watchdog resets us. This prevents any other CPU
> +	 * from petting the watchdog.
> +	 */
> +	while (1)
> +		cpu_relax();
> +

[Severity: High]
If the watchdog hardware fails to reset the system, does this infinite loop
permanently deadlock the atomic notifier chain and prevent machine_restart()
from falling back to other registered restart mechanisms like PSCI?

Since machine_restart() explicitly disables local interrupts and halts all
secondary CPUs via smp_send_stop() before executing restart handlers, is the
concern about concurrent petting from other CPUs applicable here?

Also, does this infinite loop leave the subsequent return statement as
unreachable dead code?

>  	return 0;
>  }

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