hw/watchdog/cmsdk_apb_watchdog: Fix INTEN issues
Current watchdog is free running out of reset, this combined with the
fact that current implementation also ensures the counter is running
when programing WDOGLOAD creates issues when the firmware defer the
programing of WDOGCONTROL.INTEN much later after WDOGLOAD. Arm
Programmer's Model documentation states that INTEN is also the
counter enable:
> INTEN
>
> Enable the interrupt event, WDOGINT. Set HIGH to enable the counter
> and the interrupt, or LOW to disable the counter and interrupt.
> Reloads the counter from the value in WDOGLOAD when the interrupt
> is enabled, after previously being disabled.
Source of the time of writing:
https://developer.arm.com/documentation/ddi0479/d/apb-components/apb-watchdog/programmers-model
Signed-off-by: Roque Arcudia Hernandez <roqueh@google.com>
Reviewed-by: Stephen Longfield <slongfield@google.com>
Reviewed-by: Joe Komlodi <komlodi@google.com>
Message-id: 20241115160328.1650269-3-roqueh@google.com
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
diff --git a/hw/watchdog/cmsdk-apb-watchdog.c b/hw/watchdog/cmsdk-apb-watchdog.c
index e4d25a2..ed5ff42 100644
--- a/hw/watchdog/cmsdk-apb-watchdog.c
+++ b/hw/watchdog/cmsdk-apb-watchdog.c
@@ -196,16 +196,13 @@
switch (offset) {
case A_WDOGLOAD:
- /*
- * Reset the load value and the current count, and make sure
- * we're counting.
- */
+ /* Reset the load value and the current count. */
ptimer_transaction_begin(s->timer);
ptimer_set_limit(s->timer, value, 1);
- ptimer_run(s->timer, 0);
ptimer_transaction_commit(s->timer);
break;
- case A_WDOGCONTROL:
+ case A_WDOGCONTROL: {
+ uint32_t prev_control = s->control;
if (s->is_luminary && 0 != (R_WDOGCONTROL_INTEN_MASK & s->control)) {
/*
* The Luminary version of this device ignores writes to
@@ -215,8 +212,25 @@
break;
}
s->control = value & R_WDOGCONTROL_VALID_MASK;
+ if (R_WDOGCONTROL_INTEN_MASK & (s->control ^ prev_control)) {
+ ptimer_transaction_begin(s->timer);
+ if (R_WDOGCONTROL_INTEN_MASK & s->control) {
+ /*
+ * Set HIGH to enable the counter and the interrupt. Reloads
+ * the counter from the value in WDOGLOAD when the interrupt
+ * is enabled, after previously being disabled.
+ */
+ ptimer_set_count(s->timer, ptimer_get_limit(s->timer));
+ ptimer_run(s->timer, 0);
+ } else {
+ /* Or LOW to disable the counter and interrupt. */
+ ptimer_stop(s->timer);
+ }
+ ptimer_transaction_commit(s->timer);
+ }
cmsdk_apb_watchdog_update(s);
break;
+ }
case A_WDOGINTCLR:
s->intstatus = 0;
ptimer_transaction_begin(s->timer);
@@ -305,8 +319,14 @@
s->resetstatus = 0;
/* Set the limit and the count */
ptimer_transaction_begin(s->timer);
+ /*
+ * We need to stop the ptimer before setting its limit reset value. If the
+ * order is the opposite when the code executes the stop after setting a new
+ * limit it may want to recalculate the count based on the current time (if
+ * the timer was currently running) and it won't get the proper reset value.
+ */
+ ptimer_stop(s->timer);
ptimer_set_limit(s->timer, 0xffffffff, 1);
- ptimer_run(s->timer, 0);
ptimer_transaction_commit(s->timer);
}
