Hi Simon,
On 15.08.22 19:37, Simon Glass wrote:
Hi Stefan,
On Mon, 15 Aug 2022 at 10:16, Stefan Roese <s...@denx.de> wrote:
Hi Simon,
On 05.08.22 18:48, Simon Glass wrote:
Hi Stefan,
On Fri, 5 Aug 2022 at 08:26, Stefan Roese <s...@denx.de> wrote:
This patch integrates the main function responsible for calling all
registered cyclic functions cyclic_run() into the common WATCHDOG_RESET
macro. This guarantees that cyclic_run() is executed very often, which
is necessary for the cyclic functions to get scheduled and executed at
their configured periods.
If CONFIG_WATCHDOG is not enabled, only cyclic_run() without calling
watchdog_reset(). This guarantees that the cyclic functionality does not
rely on CONFIG_WATCHDOG being enabled.
Signed-off-by: Stefan Roese <s...@denx.de>
---
v3:
- No change
v2:
- No change
fs/cramfs/uncompress.c | 2 +-
include/watchdog.h | 23 ++++++++++++++++++++---
2 files changed, 21 insertions(+), 4 deletions(-)
Reviewed-by: Simon Glass <s...@chromium.org>
diff --git a/fs/cramfs/uncompress.c b/fs/cramfs/uncompress.c
index f431cc46c1f7..38e10e2e4422 100644
--- a/fs/cramfs/uncompress.c
+++ b/fs/cramfs/uncompress.c
@@ -62,7 +62,7 @@ int cramfs_uncompress_init (void)
stream.avail_in = 0;
#if defined(CONFIG_HW_WATCHDOG) || defined(CONFIG_WATCHDOG)
- stream.outcb = (cb_func) WATCHDOG_RESET;
+ stream.outcb = (cb_func)watchdog_reset_func;
#else
stream.outcb = Z_NULL;
#endif /* CONFIG_HW_WATCHDOG */
diff --git a/include/watchdog.h b/include/watchdog.h
index 813cc8f2a5d3..0a9777edcbad 100644
--- a/include/watchdog.h
+++ b/include/watchdog.h
@@ -11,6 +11,8 @@
#define _WATCHDOG_H_
#if !defined(__ASSEMBLY__)
+#include <cyclic.h>
+
/*
* Reset the watchdog timer, always returns 0
*
@@ -60,11 +62,16 @@ int init_func_watchdog_reset(void);
/* Don't require the watchdog to be enabled in SPL */
#if defined(CONFIG_SPL_BUILD) && \
!defined(CONFIG_SPL_WATCHDOG)
- #define WATCHDOG_RESET() {}
+ #define WATCHDOG_RESET() { \
+ cyclic_run(); \
+ }
#else
extern void watchdog_reset(void);
- #define WATCHDOG_RESET watchdog_reset
+ #define WATCHDOG_RESET() { \
+ watchdog_reset(); \
+ cyclic_run(); \
Doesn't this create two function calls from every reset site? I worry
about code size.
Good point. Even though the world build did not trigger any new problems
with oversized images.
I suggest creating a new function like
check_watchdog() which you can define (in the C file) with
IS_ENABLED() as either empty, calling watchdog_reset() and/or calling
cyclic_run(). LTO should help here.
I tried a bit to get clean up this ugly #ifdef construct. And move this
as you suggested into a C file. Just now I noticed, that we don't have
a matching C file, which is compiled in all cases. wdt-uclass.c and
cyclic.c are both only compiled when actually enabled in Kconfig.
So do you have some other / better idea on how to improve this?
BTW: Moving the watchdog integration into the cyclic infrastructure in
some follow-up patches will make all this much cleaner AFAICT.
If you are going to do this soon, then I suggest not working about it too much.
But you could create wdt_common.c or similar. It should be OK to
always compile it since the code will be dropped if nothing calls it.
Yes, this would be one potential intermediate "solution". I'll try to
work on the WDT migration to cyclic IF soon and will keep the code
in this header as-is for now.
Thanks,
Stefan
