[PATCH] watchdog: rti: support SPL (or re-start)
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(-)
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);
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
Hi Stefan!
On Fri, 2024-11-08 at 22:15 +0100, 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.
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
I saw that this patch has been set to "Superseded" in the patchwork [1], but I'm not sure by which patch. Could it be that it happened by mistake?
My other patch for the rti watchdog [2] is addressing some unrelated issue...
1. https://patchwork.ozlabs.org/project/uboot/patch/20241108211506.1348528-1-al...
2. https://patchwork.ozlabs.org/project/uboot/patch/20241120222412.1873381-1-al... https://patchwork.ozlabs.org/project/uboot/patch/20241121080326.1913067-1-al...
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);
Hi Alexander,
On 10.12.24 13:45, Sverdlin, Alexander wrote:
Hi Stefan!
On Fri, 2024-11-08 at 22:15 +0100, 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.
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
I saw that this patch has been set to "Superseded" in the patchwork [1], but I'm not sure by which patch. Could it be that it happened by mistake?
Might be. I need to take a look...
My other patch for the rti watchdog [2] is addressing some unrelated issue...
https://patchwork.ozlabs.org/project/uboot/patch/20241108211506.1348528-1-al...
https://patchwork.ozlabs.org/project/uboot/patch/20241120222412.1873381-1-al... https://patchwork.ozlabs.org/project/uboot/patch/20241121080326.1913067-1-al...
Sorry, I missed handling the watchdog patches in this release cycle. And now we are pretty late in the release cycle and I would prefer to defer pulling these patches after the upcoming release beginning of January. Please excuse the inconvenience.
Thanks, Stefan
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);
Viele Grüße, Stefan Roese
Hi Stefan!
On Sat, 2024-12-14 at 14:36 +0100, Stefan Roese wrote:
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
I saw that this patch has been set to "Superseded" in the patchwork [1], but I'm not sure by which patch. Could it be that it happened by mistake?
Might be. I need to take a look...
My other patch for the rti watchdog [2] is addressing some unrelated issue...
https://patchwork.ozlabs.org/project/uboot/patch/20241108211506.1348528-1-al...
https://patchwork.ozlabs.org/project/uboot/patch/20241120222412.1873381-1-al... https://patchwork.ozlabs.org/project/uboot/patch/20241121080326.1913067-1-al...
Sorry, I missed handling the watchdog patches in this release cycle. And now we are pretty late in the release cycle and I would prefer to defer pulling these patches after the upcoming release beginning of January. Please excuse the inconvenience.
Thanks for the quick reply! No problem, thank you for your efforts!
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.
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
Reviewed-by: Stefan Roese sr@denx.de
Thanks, Stefan
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);
Viele Grüße, Stefan Roese
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.
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
Applied to u-boot-watchdog/master
Thanks, Stefan
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);
Viele Grüße, Stefan Roese
participants (4)
-
A. Sverdlin -
Jan Kiszka -
Stefan Roese -
Sverdlin, Alexander