Is Feeding a Watchdog Timer from an ISR a Bad Practice?
In an embedded system, I require a watchdog to be able to pass ESD qualifications (the main reason for the watchdog requirement is that the device wasn't able to pass the harshest tests of the IEC 61000-4-2 standard). Having no experience with watchdogs, I went through this Memfault article. I liked the events "registration" and "flags" since it easily allows us to do conditional ORing of events depending on the system's state. Plus it feels scalable.
For the feeding part, here's what I put in place:
/** @brief Raise the event flag and feed the watchdog if all required event flags are set.
*
* @param[in] event_flag Bitmask corresponding to the triggering event.
*/
static void watchdog_feed_if_all_events_set(watchdog_event_t event_flag)
{
/* Declare as volatile since this function could be called from concurrent ISRs. */
static volatile watchdog_event_t watchdog_fed_events;
enter_critical();
watchdog_fed_events |= event_flag;
if ((watchdog_fed_events & watchdog_required_events) == watchdog_required_events) {
bsp_watchdog_feed();
watchdog_fed_events = 0;
}
exit_critical();
}
This function can then be called from any part of the application, including from ISRs.
Now for the question:
I was told it was senseless to feed a watchdog from an ISR and that a redesign is required, but no rationale was provided. I see nothing wrong with my implementation and it works just fine. I feel like a watchdog design is closely related/dependent to the system on which it is implemented and that there are no one-size-fits-all designs.
Am I missing something? Why would someone want to avoid feeding a watchdog from an ISR?
2 answers
You are accessing this answer with a direct link, so it's being shown above all other answers regardless of its score. You can return to the normal view.
Feeding the watchdog from an interrupt service routine is indeed a bad idea, usually. It depends on what you are really trying to protect against.
The purpose of a hardware watchdog is usually to allow recovery if the processor stops doing what it's supposed to be doing. The question then becomes how do you measure "supposed to be doing".
If you are only concerned about the processor itself no longer executing code due to a physical reason, like an ESD event, then feeding the dog from an interrupt routine is OK. However, there are other possible reasons the process can stop doing what it's supposed to be doing that this method won't catch. If the processor takes a bad jump someplace, it could be executing garbage foreground code, but interrupts might still run normally. If you have multiple tasks running, then a single task could be wedged, but the interrupt method won't detect that.
By having only foreground code (as opposed to interrupt code) feeding the dog, more of the system has to be functional to not cause a hardware reset. In a cooperative multi-tasking system, you could have one task occasionally feed the dog. If that one task is running occasionally, then all tasks must be running occasionally.
An even more robust system, necessary in pre-emptive multi-tasking, is for each task to set a flag occasionally but faster than you need to feed the dog. One task periodically checks the flags and only feeds the dog if all flags are set. Of course it resets the flags then, ready for the next interval.
1 comment thread
The problem with refreshing the watchdog from an ISR is that in case the main program hangs for whatever the reason, the ISR will keep living and could keep the watchdog alive, effectively blocking the product from recovering from an error state, making the watchdog useless.
It could leave GPIO or other hardware peripherals in an active state, which in turn could cause larger system-level problems.
Similarly, memory corruption bugs could tear down the variables that the ISR relies on and maybe that's why main() got stuck in some exception handler in the first place. What if the problem is that runaway code or memory corruption bugs is overwriting .bss memory where the watchdog_fed_events variable is stored?
Best practices is not to mix up the watchdog with any application-tier functionality or error handling. Let it be an independent peripheral with the sole purpose to check if the program is still running.
The utopia of best practices, which isn't always possible, is to only kick the watchdog once, at the top of the main loop:
void main ()
{
for(;;)
{
kick_dog(); // the only point in the program where we kick the dog
/* rest of program */
}
}
But of course before you get to the for(;;) you need to init a bunch of stuff which might be time consuming. So you might need to refresh the wdog a couple of times during that.
In some cases you might need to do very time-consuming things in run-time like erasing flash and then you might need to refresh the dog before and after such an operation.
It's also common practice to refresh the dog immediately after initializing it. Watchdog initialization should happen very early on, normally in the reset interrupt vector before you even initialize system clock and run the CRT code.
1 comment thread