Watchdog timer trait.

[xd009642]

Setting up a watchdog timer to reset a microcontroller if it isn't serviced at regular intervals is fairly common. Also, from my experience watchdogs tend to be fairly similar and minimalist making them a potentially easy inclusion for a trait. I haven't analysed a large number of controllers but the typical operation seems to be as follows:

1. Once enabled watchdog cannot be disabled
2. A bit must be set or cleared periodically or processor is reset
3. Period length may be configurable
4. May be able to see current timer value.

I think 1, 2 and 3 would be essential for a watchdog trait, via functions start, kick and set_period. Due to it on some microcontrollers only offering a subset of timer functionality I'm also not sure how much crossover there should be with the timer traits? Open to discussion this is just prompted by some work I've been doing recently with STM32 processors.

[ryankurte]

Love the idea of this, though ime it is possible and there are a number of reasons you might want to disable the watchdog (for example, rebooting from an app with the watchdog to a boot-loader without, during long sleep states depending on architecture).
I personally would like to see disable added so we're not making application level design decisions.

[austinbes]

@ryankurte -- We'd have to think carefully about adding disable functionality -- not because I disagree with your point, but because some hardware watchdog timers cannot be disabled.

I think it might sense to have a separate trait (DisableableWatchdog is a bit of a mouthful, but it's what I can come up with off the top o fmy head) which owns the disable() function.

[ryankurte]

Good call, I think that's inline with the discussion on composable gpio traits also.

[Serisium]

I'm also familiar with platforms where the recommended method for performing a software-reset was to enable the watchdog timer, then to stall until the timer overflowed.

Should this action be captured in the trait, or should it be performed by the application?

[austinbes]

@ggreenwood What platform(s) are you talking about, specifically?

I don't think that's necessary or desirable to capture that trait in the application. What I can see being valuable is eventually adding a Reset (or similar) trait, which could be implemented in terms of the watchdog timer on platforms where that makes sense.

[Serisium]

@austinbes Some of the lower and middle-end AVR chips require using the Watchdog to reset: https://www.microchip.com/webdoc/AVRLibcReferenceManual/FAQ_1faq_softreset.html

[Serisium]

One other application comes to mind: A computationally-heavy library, or one that blocks to handle communication, may hog the processor for long enough to cause a watchdog reset. In this case, it would make sense for the library developer to access the Watchdog trait in order to reset it.

In this case, it could be dangerous to allow a library to have full access to the trait, since it includes the start() and set_period() functions. Only the kick() would actually be necessary.

[therealprof]

@xd009642 I very much like this idea. I also agree with @ryankurte that a watchdog should be disable-able since there are some architectures around that start with watchdog enabled by default (or per configuration bits) so it needs be disabled at boot (or properly taken care of by the software) otherwise it will periodically restart the MCU.

Just out of curiosity: Which platforms automatically forbid disabling the WDT? Typically enabling and locking are two separate operations (or configuration settings) and only if in locked state they cannot be disabled again.

[austinglaser]

At least one example: The IWDG peripheral on the STM32 series (I'm referencing the STM32F303 specifically, but I believe most STM32s are the same).

[xd009642]

The WWDG peripheral on the STM32F469 I'm using also has the same behaviour, the watchdog is disabled at start and once enabled can't be disabled. Also, the period can be changed after it's started as setting the period to 0 causes an instant reset.

Given the differences in hardware implementations, how about splitting watchdog into two traits, WatchdogEnable which starts it and kicks it, and WatchdogDisable which only stops it. Changing the period isn't a common use case so my thoughts are it's probably out of the scope of the embedded-hal.

[austinglaser]

It seems like both use-cases (watchdogs that are automatically enabled at reset, but can be disabled; and those which are by default disabled and unstoppable when started) would be served by having a composable set of watchdog traits -- does that match your interpretation?

@xd009642 I almost think (given @ggreenwood's comment) it should be three traits:

trait Watchdog {
    fn kick(&mut self);
}

trait WatchdogConfigure: Watchdog {
    type Time;

    fn set_period<T>(&mut self, period: T) where T: Into<Self::Time>;

    fn start(&mut self);
}

trait WatchdogDisable: Watchdog {
    fn disable(&mut self);
}

Given the WWDG example, I see what you're going for with having a separate set_period() function.

[xd009642]

Ah I see, slightly misread that part. Not too far off what I've just changed it to so no issue 👍

One thing mentioned on the PR comments, which I was just looking into is whether start should return a Result as on some architectures there may be invalid period values, or whether that is handled by the Time type. I personally err towards the time type as for more complicated things like PLLs I can see benefit in the type ensuring it's correctness regarding scalars etc.

[therealprof]

Fair enough.