[riot-notifications] [RIOT-OS/RIOT] drivers/include: add header definition for wdt (#11527)
notifications at github.com
Tue May 21 10:02:23 CEST 2019
@aabadie @benpicco @kaspar030
I pushed new changes taking your comments into account.
Regarding naming, Changed `wdt_setup_callback()` to `wdt_setup_reboot_with_callback()` so its clear it is doing both operations.
For the WDT callback I think the clearer way is adding a paramter `cb_time` to `wdt_setup_reboot_with_callback()`. This will make the api clearer for all use cases.
* If fixed value: `assert(cb_time==0)`
* If configurable value it can be set as `cb_time`
* If configurable within certain values `assert(cb_time==OPTION_A || cb_time==OPTION_B ...)`
* If conflicts with min_time (samd): `assert(min_time != cb_time!=0)`
* @brief Set up the wdt timer, only use max_time if NORMAL operation
* set min_time and max_time for WINDOWED timer.
* @param[in] min_time lower bound for WINDOWED watchdog in ms, has to be 0
* for NORMAL watchdog.
* @param[in] max_time upper bound for WINDOWED watchdog in ms, time before
* reset for NORMAL watchdog.
* @param[in] cb _time time before wdt_cb is executed, must.
* @param[in] wdt_cb wdt callback, can be NULL
* @param[in] arg optional argument which is passed to the
* callback, can be NULL
void wdt_setup_reboot_with_callback(uint32_t min_time, uint32_t max_time,
uint32_t cb_time, wdt_cb_t wdt_cb, void* arg);
> on nrf52, one can disable or keep the watchdog running during sleep/debug sessions. I guess other architectures also provide this kind of behaviors. How could this be handled by the current design ? Maybe using overridable defines at CPU levels and let the peripheral driver deal with it ? or at HAL level ?
@aabadie This can indeed be the case. Overridable defines should do the trick but lets leave this for a future PR to try to limit the discussion on this PR, what do you think?
> my second comment is about multiple watchdog management on the same CPU. still on nrf52, one can configure up to 8 watchdog "reload requests". STM32 provides 2 types of watchdog on the same CPU (independent and windowed). If I'm not mistaken, the current design doesn't allow to support such features.
@aabadie A choice has to be made between supporting all features and a simple API. From discussions in #7374 the choice was going for a simple one and not supporting all features. So I think a single WDT has to the way to go (unless in the feature someone puts forward a use case for multiple ones). What do you think?
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
-------------- next part --------------
An HTML attachment was scrubbed...
More information about the notifications