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. 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. > + } > #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