[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

[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
>