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.
Thanks,
Stefan
+ }
#endif
#endif
#else
@@ -74,11 +81,21 @@ int init_func_watchdog_reset(void);
#if defined(__ASSEMBLY__)
#define WATCHDOG_RESET /*XXX DO_NOT_DEL_THIS_COMMENT*/
#else
- #define WATCHDOG_RESET() {}
+ #define WATCHDOG_RESET() { \
+ cyclic_run(); \
+ }
#endif /* __ASSEMBLY__ */
#endif /* CONFIG_WATCHDOG && !__ASSEMBLY__ */
#endif /* CONFIG_HW_WATCHDOG */
+#if !defined(__ASSEMBLY__)
+/* Currently only needed for fs/cramfs/uncompress.c */
+static inline void watchdog_reset_func(void)
+{
+ WATCHDOG_RESET();
+}
+#endif
+
/*
* Prototypes from $(CPU)/cpu.c.
*/
--
2.37.1
Regards,
Simon
Viele Grüße,
Stefan Roese
--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: s...@denx.de
