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

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

[Andrew Gaylard]

sp_wdt_restart() returns immediately after writing WDT_RESUME. The
kernel continues executing and the watchdog kthread could pet the
watchdog before the short timeout expired, preventing reset.

Add a spin loop so no other CPU can pet the watchdog while waiting for
the reset. Set the restart timeout to ~1.5s.

Signed-off-by: Andrew Gaylard <ag@ffroot.co.za>
---
 drivers/watchdog/sunplus_wdt.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/watchdog/sunplus_wdt.c b/drivers/watchdog/sunplus_wdt.c
index e5212f3d2e24..b44dcc307634 100644
--- a/drivers/watchdog/sunplus_wdt.c
+++ b/drivers/watchdog/sunplus_wdt.c
@@ -61,10 +61,16 @@ static int sp_wdt_restart(struct watchdog_device *wdev,
 
 	writel(WDT_STOP, base + WDT_CTRL);
 	writel(WDT_UNLOCK, base + WDT_CTRL);
-	writel(0x0001, base + WDT_CNT);
+	writel(0x20F5, base + WDT_CNT);	/* ~1.5s at 90kHz */
 	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();
+
 	return 0;
 }
 
-- 
2.53.0

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

[sashiko-bot]

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 <ag@ffroot.co.za>
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/20260820125304.1583383-1-ag@ffroot.co.za?part=1

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

[Guenter Roeck]

On Thu, Aug 20, 2026 at 02:53:04PM +0200, Andrew Gaylard wrote:
> sp_wdt_restart() returns immediately after writing WDT_RESUME. The
> kernel continues executing and the watchdog kthread could pet the
> watchdog before the short timeout expired, preventing reset.

Is this observed behavior or just an assumption ?

> 
> Add a spin loop so no other CPU can pet the watchdog while waiting for
> the reset. Set the restart timeout to ~1.5s.

I understand the first part, but why delay the reset by 1.5s ?
That is a functional change which needs an explanation.

Guenter

> 
> Signed-off-by: Andrew Gaylard <ag@ffroot.co.za>
> ---
>  drivers/watchdog/sunplus_wdt.c | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/watchdog/sunplus_wdt.c b/drivers/watchdog/sunplus_wdt.c
> index e5212f3d2e24..b44dcc307634 100644
> --- a/drivers/watchdog/sunplus_wdt.c
> +++ b/drivers/watchdog/sunplus_wdt.c
> @@ -61,10 +61,16 @@ static int sp_wdt_restart(struct watchdog_device *wdev,
>  
>  	writel(WDT_STOP, base + WDT_CTRL);
>  	writel(WDT_UNLOCK, base + WDT_CTRL);
> -	writel(0x0001, base + WDT_CNT);
> +	writel(0x20F5, base + WDT_CNT);	/* ~1.5s at 90kHz */
>  	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();
> +
>  	return 0;
>  }
>  
> -- 
> 2.53.0
> 
> 

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

[Andrew Gaylard]

Guenter Roeck <linux@roeck-us.net> writes:

> On Thu, Aug 20, 2026 at 02:53:04PM +0200, Andrew Gaylard wrote:
>> sp_wdt_restart() returns immediately after writing WDT_RESUME. The
>> kernel continues executing and the watchdog kthread could pet the
>> watchdog before the short timeout expired, preventing reset.
>
> Is this observed behavior or just an assumption ?

It's an assumption.

>> Add a spin loop so no other CPU can pet the watchdog while waiting for
>> the reset. Set the restart timeout to ~1.5s.
>
> I understand the first part, but why delay the reset by 1.5s ?
> That is a functional change which needs an explanation.

I had a printf there to confirm that it actually reached that point,
so I needed the delay.

-- 
Andrew

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

[Guenter Roeck]

On Thu, Aug 20, 2026 at 05:12:11PM +0200, Andrew Gaylard wrote:
> Guenter Roeck <linux@roeck-us.net> writes:
> 
> > On Thu, Aug 20, 2026 at 02:53:04PM +0200, Andrew Gaylard wrote:
> >> sp_wdt_restart() returns immediately after writing WDT_RESUME. The
> >> kernel continues executing and the watchdog kthread could pet the
> >> watchdog before the short timeout expired, preventing reset.
> >
> > Is this observed behavior or just an assumption ?
> 
> It's an assumption.
> 
So you habve no idea if this is a real problem. I am not going to apply
this patch.

Guenter

> >> Add a spin loop so no other CPU can pet the watchdog while waiting for
> >> the reset. Set the restart timeout to ~1.5s.
> >
> > I understand the first part, but why delay the reset by 1.5s ?
> > That is a functional change which needs an explanation.
> 
> I had a printf there to confirm that it actually reached that point,
> so I needed the delay.
> 
> -- 
> Andrew
>