diff mbox series

watchdog: rti: support SPL (or re-start)

Message ID 20241108211506.1348528-1-alexander.sverdlin@siemens.com
State Superseded
Delegated to: Stefan Roese
Headers show
Series watchdog: rti: support SPL (or re-start) | expand

Commit Message

Sverdlin, Alexander Nov. 8, 2024, 9:15 p.m. UTC
From: Alexander Sverdlin <alexander.sverdlin@siemens.com>

If the RTI watchdog has been enabled in SPL, enabling it in U-Boot proper
fails because it can only be enabled once in HW and never stopped. This
however leads to a situation that wdt_cyclic() watchdog trigger is not
being started any longer and the WDT fires at some point.

Allow for WDT re-start by not bailing out if the [previously] configured
period matches the one to be configured.

Enabling in [A53] SPL has been tested on AM62x-based HW (where [A53] SPL is
responsible for loading R5 DM firmware and not this driver).

Signed-off-by: Alexander Sverdlin <alexander.sverdlin@siemens.com>
---
 drivers/watchdog/rti_wdt.c | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

Comments

Jan Kiszka Nov. 9, 2024, 9:19 a.m. UTC | #1
On 08.11.24 22:15, A. Sverdlin wrote:
> From: Alexander Sverdlin <alexander.sverdlin@siemens.com>
> 
> If the RTI watchdog has been enabled in SPL, enabling it in U-Boot proper
> fails because it can only be enabled once in HW and never stopped. This
> however leads to a situation that wdt_cyclic() watchdog trigger is not
> being started any longer and the WDT fires at some point.

Nothing against this change, just pointing out that, when a watchdog is
supposed to monitor a complete boot, you normally don't want it to be
triggered anymore before that boot is complete, neither from the kernel
(watchdog.handle_boot_enabled=0) nor from U-Boot
(CONFIG_WATCHDOG_AUTOSTART=n + script-based starting, or skip in in
proper when started by SPL). Anything else always comes with the risk of
running into a logically stuck boot that happily pets the watchdog until
eternity - and you have to ask someone to manually reset your device.

> 
> Allow for WDT re-start by not bailing out if the [previously] configured
> period matches the one to be configured.
> 
> Enabling in [A53] SPL has been tested on AM62x-based HW (where [A53] SPL is
> responsible for loading R5 DM firmware and not this driver).
> 
> Signed-off-by: Alexander Sverdlin <alexander.sverdlin@siemens.com>
> ---
>  drivers/watchdog/rti_wdt.c | 13 +++++++------
>  1 file changed, 7 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/watchdog/rti_wdt.c b/drivers/watchdog/rti_wdt.c
> index 99168d0cad03..320c5ca19e0a 100644
> --- a/drivers/watchdog/rti_wdt.c
> +++ b/drivers/watchdog/rti_wdt.c
> @@ -131,18 +131,19 @@ static int rti_wdt_start(struct udevice *dev, u64 timeout_ms, ulong flags)
>  	u32 timer_margin;
>  	int ret;
>  
> -	if (readl(priv->regs + RTIDWDCTRL) == WDENABLE_KEY)
> +	timer_margin = timeout_ms * priv->clk_hz / 1000;
> +	timer_margin >>= WDT_PRELOAD_SHIFT;
> +	if (timer_margin > WDT_PRELOAD_MAX)
> +		timer_margin = WDT_PRELOAD_MAX;
> +
> +	if (readl(priv->regs + RTIDWDCTRL) == WDENABLE_KEY &&
> +	    readl(priv->regs + RTIDWDPRLD) != timer_margin)
>  		return -EBUSY;
>  
>  	ret = rti_wdt_load_fw(dev);
>  	if (ret < 0)
>  		return ret;
>  
> -	timer_margin = timeout_ms * priv->clk_hz / 1000;
> -	timer_margin >>= WDT_PRELOAD_SHIFT;
> -	if (timer_margin > WDT_PRELOAD_MAX)
> -		timer_margin = WDT_PRELOAD_MAX;
> -
>  	writel(timer_margin, priv->regs + RTIDWDPRLD);
>  	writel(RTIWWDRX_NMI, priv->regs + RTIWWDRXCTRL);
>  	writel(RTIWWDSIZE_50P, priv->regs + RTIWWDSIZECTRL);

Reviewed-by: Jan Kiszka <jan.kiszka@siemens.com>

Jan
diff mbox series

Patch

diff --git a/drivers/watchdog/rti_wdt.c b/drivers/watchdog/rti_wdt.c
index 99168d0cad03..320c5ca19e0a 100644
--- a/drivers/watchdog/rti_wdt.c
+++ b/drivers/watchdog/rti_wdt.c
@@ -131,18 +131,19 @@  static int rti_wdt_start(struct udevice *dev, u64 timeout_ms, ulong flags)
 	u32 timer_margin;
 	int ret;
 
-	if (readl(priv->regs + RTIDWDCTRL) == WDENABLE_KEY)
+	timer_margin = timeout_ms * priv->clk_hz / 1000;
+	timer_margin >>= WDT_PRELOAD_SHIFT;
+	if (timer_margin > WDT_PRELOAD_MAX)
+		timer_margin = WDT_PRELOAD_MAX;
+
+	if (readl(priv->regs + RTIDWDCTRL) == WDENABLE_KEY &&
+	    readl(priv->regs + RTIDWDPRLD) != timer_margin)
 		return -EBUSY;
 
 	ret = rti_wdt_load_fw(dev);
 	if (ret < 0)
 		return ret;
 
-	timer_margin = timeout_ms * priv->clk_hz / 1000;
-	timer_margin >>= WDT_PRELOAD_SHIFT;
-	if (timer_margin > WDT_PRELOAD_MAX)
-		timer_margin = WDT_PRELOAD_MAX;
-
 	writel(timer_margin, priv->regs + RTIDWDPRLD);
 	writel(RTIWWDRX_NMI, priv->regs + RTIWWDRXCTRL);
 	writel(RTIWWDSIZE_50P, priv->regs + RTIWWDSIZECTRL);