watchdog: add periodic watchdog ping
PATCH 1/1 watchdog: add periodic watchdog ping
This patch should cover at least this use cases:
- For production, for example automotive: cover as match as possible of SoC life time with watchdog. Since some SoCs do not provide enough control over watchdog timer it is not enough to enable it in PBL and retrigger in linux. Barebox may need to retrigger it a few times as well.
- For remote development. If there is no way to power cycle the target except of manually unplug it.
--- drivers/watchdog/wd_core.c | 57 ++++++++++++++++++++++++++++++++++++++++++++++ include/watchdog.h | 1 + 2 files changed, 58 insertions(+)
drivers/watchdog/wd_core.c +57 -0
@@ -16,7 +16,11 @@#include <common.h> #include <command.h> #include <errno.h> +#include <globalvar.h> #include <linux/ctype.h> +#include <magicvar.h> +#include <poller.h> +#include <init.h> #include <watchdog.h> static LIST_HEAD (watchdog_list);
@@ -101,3 +105,56 @@ unsigned int of_get_watchdog_priority(struct device_node *node)return priority; } + +static int watchdog_ping_enable; +static unsigned int watchdog_ping_timeout; + +static void watchdog_ping_func (struct poller_struct *poller) +{ +struct watchdog *wd; +unsigned int timeout = watchdog_ping_timeout; + +if (!watchdog_ping_enable) +return ; + + wd =watchdog_get_default (); + +if (!wd || wd->ping_next_event >get_time_ns ()) +return ; + + wd->ping_next_event =get_time_ns () +500 *MSECOND ; + +pr_debug ("setting timeout on %s to %ds\n" ,watchdog_name (wd), timeout); + + wd->set_timeout (wd, timeout); +} + +static struct poller_struct watchdog_poller = { + .func = watchdog_ping_func, +}; + +static int init_watchdog_ping_timeout (void ) +{ +int err; + + err =globalvar_add_simple_bool ("watchdog.ping_enable" , + &watchdog_ping_enable); +if (err) +return err; + + err =globalvar_add_simple_int ("watchdog.ping_timeout" , + &watchdog_ping_timeout,"%u" ); +if (err) +return err; + +return poller_register (&watchdog_poller); +} +late_initcall (init_watchdog_ping_timeout); + +BAREBOX_MAGICVAR_NAMED (global_watchdog_ping_enable, + global.watchdog.ping_enable, +"Enable periodic watchdog pings" ); + +BAREBOX_MAGICVAR_NAMED (global_watchdog_ping_timeout, + global.watchdog.ping_timeout, +"Periodic watchdog timeout in seconds" );
include/watchdog.h +1 -0
@@ -19,6 +19,7 @@ struct watchdog {struct device_d *dev;unsigned int priority;struct list_head list; +uint64_t ping_next_event; };#ifdef CONFIG_WATCHDOG
Signature · 7 lines
2.14.1 _______________________________________________ barebox mailing list [email protected] http://lists.infradead.org/mailman/listinfo/barebox
Quoted 40 lines · Oleksij Rempel
This patch should cover at least this use cases:
- For production, for example automotive: cover as match as possible of SoC life time with watchdog. Since some SoCs do not provide enough control over watchdog timer it is not enough to enable it in PBL and retrigger in linux. Barebox may need to retrigger it a few times as well.
- For remote development. If there is no way to power cycle the target except of manually unplug it.Signed-off-by Oleksij Rempel <[email protected]>--- drivers/watchdog/wd_core.c | 57 ++++++++++++++++++++++++++++++++++++++++++++++ include/watchdog.h | 1 + 2 files changed, 58 insertions(+)drivers/watchdog/wd_core.c +10
-0@@ -16,7 +16,11 @@#include <common.h> #include <command.h> #include <errno.h> +#include <globalvar.h> #include <linux/ctype.h> +#include <magicvar.h> +#include <poller.h> +#include <init.h> #include <watchdog.h> static LIST_HEAD (watchdog_list);@@ -101,3 +105,56 @@ unsigned int of_get_watchdog_priority(struct device_node *node)return priority; } + +static int watchdog_ping_enable; +static unsigned int watchdog_ping_timeout; + +static void watchdog_ping_func (struct poller_struct *poller) +{
drivers/watchdog/wd_core.c unsigned int of_get_watchdog_priority(struct device_node *node) · line 123
Quoted 10 lines
drivers/watchdog/wd_core.c +10
-0+struct watchdog *wd; +unsigned int timeout = watchdog_ping_timeout; + +if (!watchdog_ping_enable) +return ; + + wd =watchdog_get_default (); + +if (!wd || wd->ping_next_event >get_time_ns ()) +return ;
What if for some bizarre reason (say legacy design that cannot be changed) I have a system with more than one watchdog that needs to be petted? I'd consider making pollers a per watchdog device thing and also converting the code to use "struct poller_async" since it'll already implement periodic execution.
drivers/watchdog/wd_core.c unsigned int of_get_watchdog_priority(struct device_node *node) · line 126
Quoted 3 lines
drivers/watchdog/wd_core.c +3
-0+ + wd->ping_next_event =get_time_ns () +500 *MSECOND ; +
I'd expect your petting frequency to be tied to watchdog timeout period (say half the period), and not just fixed to 0.5 seconds. Is this on purpose?
drivers/watchdog/wd_core.c unsigned int of_get_watchdog_priority(struct device_node *node) · line 129
Quoted 3 lines
drivers/watchdog/wd_core.c +3
-0+pr_debug ("setting timeout on %s to %ds\n" ,watchdog_name (wd), timeout); + + wd->set_timeout (wd, timeout);
This changes the semantics of "set_timeout" callback, before, when notion of "petting" a watchdog didn't exist, it only changed the timeout setting, whereas with your patch it also becomes responsible for petting as well. It depends on IP block's implementation, but it is not that uncommon for one to have those functions -- petting and changing the timeout -- to be implemented separately, oftentimes via different registers. Just as a suggestion, it might be better to introduce a separate "pet()"/"ping()" callback and have the framework print a warning when used against a driver that was not explicitly ported to support the notion of "petting".
Quoted 21 lines
drivers/watchdog/wd_core.c +21
-0+} + +static struct poller_struct watchdog_poller = { + .func = watchdog_ping_func, +}; + +static int init_watchdog_ping_timeout (void ) +{ +int err; + + err =globalvar_add_simple_bool ("watchdog.ping_enable" , + &watchdog_ping_enable); +if (err) +return err; + + err =globalvar_add_simple_int ("watchdog.ping_timeout" , + &watchdog_ping_timeout,"%u" ); +if (err) +return err; + +return poller_register (&watchdog_poller);
drivers/watchdog/wd_core.c unsigned int of_get_watchdog_priority(struct device_node *node) · line 160
Quoted 10 lines
drivers/watchdog/wd_core.c +10
-0+} +late_initcall (init_watchdog_ping_timeout); + +BAREBOX_MAGICVAR_NAMED (global_watchdog_ping_enable, + global.watchdog.ping_enable, +"Enable periodic watchdog pings" ); + +BAREBOX_MAGICVAR_NAMED (global_watchdog_ping_timeout, + global.watchdog.ping_timeout, +"Periodic watchdog timeout in seconds" );
As another suggestion, consider porting the notion of timeout boundaries (max_timeout, min_timeout) similar to what exists in Linux kernel to allow some bounds checking and avoiding setting bogus timeout values.
Thanks,
Andrey Smrinov
_______________________________________________
barebox mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/barebox