[PATCH] watchdog: bcm7038: Add a restart callback

[PATCH] watchdog: bcm7038: Add a restart callback

[Linus Walleij]

The watchdog in BCM6846 can reset the system, but bcm7038_wdt
does not provide a restart callback. Add this.

The vendor kernel also uses a short watchdog timeout to reset
this SoC.

Implement the restart operation by stopping the watchdog, loading the
minimum count of two ticks and starting it again. A count below two
does not trigger a reset, as also documented by the U-Boot BCM6345
watchdog driver.

Disable local interrupts across the register sequence and
reset wait  so preemptible uniprocessor MIPS kernels cannot run
a competing watchdog operation.

Assisted-by: LLM
Signed-off-by: Linus Walleij <linusw@kernel.org>
---
 drivers/watchdog/bcm7038_wdt.c | 30 ++++++++++++++++++++++++++++++
 1 file changed, 30 insertions(+)

diff --git a/drivers/watchdog/bcm7038_wdt.c b/drivers/watchdog/bcm7038_wdt.c
index e038dd66b819..1637e32ac02e 100644
--- a/drivers/watchdog/bcm7038_wdt.c
+++ b/drivers/watchdog/bcm7038_wdt.c
@@ -5,8 +5,10 @@
  */
 
 #include <linux/clk.h>
+#include <linux/delay.h>
 #include <linux/init.h>
 #include <linux/io.h>
+#include <linux/irqflags.h>
 #include <linux/module.h>
 #include <linux/of.h>
 #include <linux/platform_device.h>
@@ -113,6 +115,32 @@ static unsigned int bcm7038_wdt_get_timeleft(struct watchdog_device *wdog)
 	return time_left / wdt->rate;
 }
 
+static int bcm7038_wdt_restart(struct watchdog_device *wdog,
+			       unsigned long action, void *data)
+{
+	struct bcm7038_watchdog *wdt = watchdog_get_drvdata(wdog);
+	unsigned long flags;
+
+	/*
+	 * Prevent runtime watchdog operations from interfering on
+	 * uniprocessor systems where restart handlers are preemptible.
+	 */
+	local_irq_save(flags);
+
+	/* Stop the timer before changing its reload value. */
+	bcm7038_wdt_stop(wdog);
+	/* The watchdog does not fire with a count below two. */
+	bcm7038_wdt_write(2, wdt->base + WDT_TIMEOUT_REG);
+	bcm7038_wdt_ping(wdog);
+
+	/* Flush the writes and wait for the reset to take effect. */
+	bcm7038_wdt_read(wdt->base + WDT_CMD_REG);
+	mdelay(500);
+	local_irq_restore(flags);
+
+	return 0;
+}
+
 static const struct watchdog_info bcm7038_wdt_info = {
 	.identity	= "Broadcom BCM7038 Watchdog Timer",
 	.options	= WDIOF_SETTIMEOUT | WDIOF_KEEPALIVEPING |
@@ -125,6 +153,7 @@ static const struct watchdog_ops bcm7038_wdt_ops = {
 	.stop		= bcm7038_wdt_stop,
 	.set_timeout	= bcm7038_wdt_set_timeout,
 	.get_timeleft	= bcm7038_wdt_get_timeleft,
+	.restart	= bcm7038_wdt_restart,
 };
 
 static int bcm7038_wdt_probe(struct platform_device *pdev)
@@ -167,6 +196,7 @@ static int bcm7038_wdt_probe(struct platform_device *pdev)
 	wdt->wdd.max_timeout	= 0xffffffff / wdt->rate;
 	wdt->wdd.parent		= dev;
 	watchdog_set_drvdata(&wdt->wdd, wdt);
+	watchdog_set_restart_priority(&wdt->wdd, 128);
 
 	watchdog_stop_on_reboot(&wdt->wdd);
 	watchdog_stop_on_unregister(&wdt->wdd);

---
base-commit: 164778eff84a0ee908db0b28136a6f7d7f785c33
change-id: 20261014-genexis-xg6846-watchdog-reset-d44c695a77e0

Best regards,
--  
Linus Walleij <linusw@kernel.org>

Re: [PATCH] watchdog: bcm7038: Add a restart callback

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] watchdog: bcm7038_wdt: missing spinlock causes restart failure on panic
--

commit 6002003a9fe5a5243c56c551564e6ccb85d21046
Author: Linus Walleij <linusw@kernel.org>

watchdog: bcm7038: Add a restart callback
    
The watchdog in BCM6846 can reset the system, but bcm7038_wdt
does not provide a restart callback. Add this. The vendor kernel
also uses a short watchdog timeout to reset this SoC.

> diff --git a/drivers/watchdog/bcm7038_wdt.c b/drivers/watchdog/bcm7038_wdt.c
> index e038dd66b8198..1637e32ac02e8 100644
> --- a/drivers/watchdog/bcm7038_wdt.c
> +++ b/drivers/watchdog/bcm7038_wdt.c
[ ... ]
> @@ -113,6 +115,32 @@ static unsigned int bcm7038_wdt_get_timeleft(struct watchdog_device *wdog)
>  	return time_left / wdt->rate;
>  }
>  
> +static int bcm7038_wdt_restart(struct watchdog_device *wdog,
> +			       unsigned long action, void *data)
> +{
> +	struct bcm7038_watchdog *wdt = watchdog_get_drvdata(wdog);
> +	unsigned long flags;
> +
> +	/*
> +	 * Prevent runtime watchdog operations from interfering on
> +	 * uniprocessor systems where restart handlers are preemptible.
> +	 */
> +	local_irq_save(flags);
> +
> +	/* Stop the timer before changing its reload value. */
> +	bcm7038_wdt_stop(wdog);

[Severity: High]
Can this restart handler execute out of sequence with other watchdog
operations?

If a panic or SysRq interrupt preempts a ping or stop operation exactly
between its two writes to WDT_CMD_REG, could the hardware state machine
become corrupted?

For example, if bcm7038_wdt_ping() is interrupted:

drivers/watchdog/bcm7038_wdt.c:bcm7038_wdt_ping() {
	...
	bcm7038_wdt_write(WDT_START_1, wdt->base + WDT_CMD_REG);
	<--- interrupted by panic/SysRq

And then bcm7038_wdt_restart() executes in the interrupt context, it will
call bcm7038_wdt_stop() which writes WDT_STOP_1 to the hardware that was
expecting WDT_START_2.

Would this cause the watchdog to ignore the reset command and hang the
system during reboot or panic? Additionally, on SMP, could an
smp_send_stop() IPI from a panicking CPU halt another CPU mid-sequence in
the same way?

> +	/* The watchdog does not fire with a count below two. */
> +	bcm7038_wdt_write(2, wdt->base + WDT_TIMEOUT_REG);
> +	bcm7038_wdt_ping(wdog);
> +
> +	/* Flush the writes and wait for the reset to take effect. */
> +	bcm7038_wdt_read(wdt->base + WDT_CMD_REG);
> +	mdelay(500);
> +	local_irq_restore(flags);
> +
> +	return 0;
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261015-genexis-xg6846-watchdog-reset-v1-1-9577d65223d5@kernel.org?part=1

Re: [PATCH] watchdog: bcm7038: Add a restart callback

[Florian Fainelli]

+CC Justin,

On 10/14/26 15:39, Linus Walleij wrote:
> The watchdog in BCM6846 can reset the system, but bcm7038_wdt
> does not provide a restart callback. Add this.
> 
> The vendor kernel also uses a short watchdog timeout to reset
> this SoC.
> 
> Implement the restart operation by stopping the watchdog, loading the
> minimum count of two ticks and starting it again. A count below two
> does not trigger a reset, as also documented by the U-Boot BCM6345
> watchdog driver.
> 
> Disable local interrupts across the register sequence and
> reset wait  so preemptible uniprocessor MIPS kernels cannot run
> a competing watchdog operation.
> 
> Assisted-by: LLM
> Signed-off-by: Linus Walleij <linusw@kernel.org>

Reviewed-by: Florian Fainelli <florian.fainelli@broadcom.com>

We could also implement the .bootstatus callback by reading the WDCR 
register at offset 0x10 and check for bit 0 being set to indicate a 
watchdog reset or not.
-- 
Florian