cyclic: get rid of cyclic_init()

Currently, we must call cyclic_init() at some point before
cyclic_register() becomes possible. That turns out to be somewhat
awkward, especially with SPL, and has resulted in a watchdog callback
not being registered, thus causing the board to prematurely reset.

We already rely on gd->cyclic reliably being set to NULL by the asm
code that clears all of gd. Now that the cyclic list is a hlist, and
thus an empty list is represented by a NULL head pointer, and struct
cyclic_drv has no other members, we can just as well drop a level of
indirection and put the hlist_head directly in struct
global_data. This doesn't increase the size of struct global_data,
gets rid of an early malloc(), and generates slightly smaller code.

But primarily, this avoids having to call cyclic_init() early; the cyclic
infrastructure is simply ready to register callbacks as soon as we
enter C code.

We can still end up with schedule() being called from asm very early,
so we still need to check that gd itself has been properly initialized
[*], but once it has, gd->cyclic_list is perfectly fine to access, and
will just be an empty list.

As for cyclic_uninit(), it was never really the opposite of
cyclic_init() since it didn't free the struct cyclic_drv nor set
gd->cyclic to NULL. Rename it to cyclic_unregister_all() and use that
in test/, and also insert a call at the end of the board_init_f
sequence so that gd->cyclic_list is a fresh empty list before we enter
board_init_r().

A small piece of ugliness is that I had to add a cast in
cyclic_get_list() to silence a "discards 'volatile' qualifier"
warning, but that is completely equivalent to the existing handling of
the uclass_root_s list_head member.

[*] I'm not really sure where we guarantee that the register used for
gd contains 0 until it gets explicitly initialized, but that must be
the case, otherwise testing gd for being NULL would not make much sense.

Signed-off-by: Rasmus Villemoes <rasmus.villemoes@prevas.dk>
Reviewed-by: Stefan Roese <sr@denx.de>
Tested-by: Stefan Roese <sr@denx.de>
Tested-by: Tim Harvey <tharvey@gateworks.com> # imx8mm-venice-*

diff --git a/common/board_f.c b/common/board_f.c
index faa096479b3d1c58035c37caec9be48582b29f9e..7c26e235fc078c126bcd2a9337246e99592e23bd 100644 (file)
--- a/common/board_f.c
+++ b/common/board_f.c
@@ -839,7 +839,6 @@ static const init_fnc_t init_sequence_f[] = {
        initf_malloc,
        log_init,
        initf_bootstage,        /* uses its own timer, so does not need DM */
-       cyclic_init,
        event_init,
 #ifdef CONFIG_BLOBLIST
        bloblist_init,
@@ -957,6 +956,16 @@ static const init_fnc_t init_sequence_f[] = {
        do_elf_reloc_fixups,
 #endif
        clear_bss,
+       /*
+        * Deregister all cyclic functions before relocation, so that
+        * gd->cyclic_list does not contain any references to pre-relocation
+        * devices. Drivers will register their cyclic functions anew when the
+        * devices are probed again.
+        *
+        * This should happen as late as possible so that the window where a
+        * watchdog device is not serviced is as small as possible.
+        */
+       cyclic_unregister_all,
 #if !defined(CONFIG_ARM) && !defined(CONFIG_SANDBOX) && \
                !CONFIG_IS_ENABLED(X86_64)
        jump_to_copy,

diff --git a/common/board_r.c b/common/board_r.c
index 828ad448668042087903044e9461ae2033b848a2..db9cfb33ad2a9e944dd7efc3543812de92eca939 100644 (file)
--- a/common/board_r.c
+++ b/common/board_r.c
@@ -617,7 +617,6 @@ static init_fnc_t init_sequence_r[] = {
 #endif
        initr_barrier,
        initr_malloc,
-       cyclic_init,
        log_init,
        initr_bootstage,        /* Needs malloc() but has its own timer */
 #if defined(CONFIG_CONSOLE_RECORD)

diff --git a/common/cyclic.c b/common/cyclic.c
index 32d9bf0d02be2536da9bfe802ff8e5a64154bd21..a49bfc88f5c0e96257b59d3b75033007a2b537ed 100644 (file)
--- a/common/cyclic.c
+++ b/common/cyclic.c
@@ -22,7 +22,8 @@ void hw_watchdog_reset(void);
 
 struct hlist_head *cyclic_get_list(void)
 {
-       return &gd->cyclic->cyclic_list;
+       /* Silence "discards 'volatile' qualifier" warning. */
+       return (struct hlist_head *)&gd->cyclic_list;
 }
 
 struct cyclic_info *cyclic_register(cyclic_func_t func, uint64_t delay_us,
@@ -30,11 +31,6 @@ struct cyclic_info *cyclic_register(cyclic_func_t func, uint64_t delay_us,
 {
        struct cyclic_info *cyclic;
 
-       if (!gd->cyclic) {
-               pr_debug("Cyclic IF not ready yet\n");
-               return NULL;
-       }
-
        cyclic = calloc(1, sizeof(struct cyclic_info));
        if (!cyclic) {
                pr_debug("Memory allocation error\n");
@@ -47,7 +43,7 @@ struct cyclic_info *cyclic_register(cyclic_func_t func, uint64_t delay_us,
        cyclic->name = strdup(name);
        cyclic->delay_us = delay_us;
        cyclic->start_time_us = timer_get_us();
-       hlist_add_head(&cyclic->list, &gd->cyclic->cyclic_list);
+       hlist_add_head(&cyclic->list, cyclic_get_list());
 
        return cyclic;
 }
@@ -71,7 +67,7 @@ void cyclic_run(void)
                return;
 
        gd->flags |= GD_FLG_CYCLIC_RUNNING;
-       hlist_for_each_entry_safe(cyclic, tmp, &gd->cyclic->cyclic_list, list) {
+       hlist_for_each_entry_safe(cyclic, tmp, cyclic_get_list(), list) {
                /*
                 * Check if this cyclic function needs to get called, e.g.
                 * do not call the cyclic func too often
@@ -113,31 +109,17 @@ void schedule(void)
         * schedule() might get called very early before the cyclic IF is
         * ready. Make sure to only call cyclic_run() when it's initalized.
         */
-       if (gd && gd->cyclic)
+       if (gd)
                cyclic_run();
 }
 
-int cyclic_uninit(void)
+int cyclic_unregister_all(void)
 {
        struct cyclic_info *cyclic;
        struct hlist_node *tmp;
 
-       hlist_for_each_entry_safe(cyclic, tmp, &gd->cyclic->cyclic_list, list)
+       hlist_for_each_entry_safe(cyclic, tmp, cyclic_get_list(), list)
                cyclic_unregister(cyclic);
 
        return 0;
 }
-
-int cyclic_init(void)
-{
-       int size = sizeof(struct cyclic_drv);
-
-       gd->cyclic = (struct cyclic_drv *)malloc(size);
-       if (!gd->cyclic)
-               return -ENOMEM;
-
-       memset(gd->cyclic, '\0', size);
-       INIT_HLIST_HEAD(&gd->cyclic->cyclic_list);
-
-       return 0;
-}

diff --git a/include/asm-generic/global_data.h b/include/asm-generic/global_data.h
index 8d348b0ec04397f2dc12e80925cd93c282bb4f8e..02ad8ca595da9f2de83928deda76da559f4e640e 100644 (file)
--- a/include/asm-generic/global_data.h
+++ b/include/asm-generic/global_data.h
@@ -481,9 +481,9 @@ struct global_data {
 #endif
 #ifdef CONFIG_CYCLIC
        /**
-        * @cyclic: cyclic driver data
+        * @cyclic_list: list of registered cyclic functions
         */
-       struct cyclic_drv *cyclic;
+       struct hlist_head cyclic_list;
 #endif
        /**
         * @dmtag_list: List of DM tags

diff --git a/include/cyclic.h b/include/cyclic.h
index 4a11f9b105ea8d40e1533fda71f44a9d07899ef1..44ad3cb6b8033bd105316ef00a93ef2233336546 100644 (file)
--- a/include/cyclic.h
+++ b/include/cyclic.h
@@ -14,15 +14,6 @@
 #include <linux/list.h>
 #include <asm/types.h>
 
-/**
- * struct cyclic_drv - Cyclic driver internal data
- *
- * @cyclic_list: Cylic list node
- */
-struct cyclic_drv {
-       struct hlist_head cyclic_list;
-};
-
 /**
  * struct cyclic_info - Information about cyclic execution function
  *
@@ -75,18 +66,11 @@ struct cyclic_info *cyclic_register(cyclic_func_t func, uint64_t delay_us,
 int cyclic_unregister(struct cyclic_info *cyclic);
 
 /**
- * cyclic_init() - Set up cyclic functions
- *
- * Init a list of cyclic functions, so that these can be added as needed
- */
-int cyclic_init(void);
-
-/**
- * cyclic_uninit() - Clean up cyclic functions
+ * cyclic_unregister_all() - Clean up cyclic functions
  *
  * This removes all cyclic functions
  */
-int cyclic_uninit(void);
+int cyclic_unregister_all(void);
 
 /**
  * cyclic_get_list() - Get cyclic list pointer
@@ -134,12 +118,7 @@ static inline void schedule(void)
 {
 }
 
-static inline int cyclic_init(void)
-{
-       return 0;
-}
-
-static inline int cyclic_uninit(void)
+static inline int cyclic_unregister_all(void)
 {
        return 0;
 }

diff --git a/test/test-main.c b/test/test-main.c
index ddfd89c089ad5d7944b31a839cf16ee680805907..fe3ef6daad63f9d53eb046fe816896b85caeedb2 100644 (file)
--- a/test/test-main.c
+++ b/test/test-main.c
@@ -287,7 +287,6 @@ static int dm_test_restore(struct device_node *of_root)
 static int test_pre_run(struct unit_test_state *uts, struct unit_test *test)
 {
        ut_assertok(event_init());
-       ut_assertok(cyclic_init());
 
        if (test->flags & UT_TESTF_DM)
                ut_assertok(dm_test_pre_run(uts));
@@ -347,7 +346,7 @@ static int test_post_run(struct unit_test_state *uts, struct unit_test *test)
        ut_unsilence_console(uts);
        if (test->flags & UT_TESTF_DM)
                ut_assertok(dm_test_post_run(uts));
-       ut_assertok(cyclic_uninit());
+       ut_assertok(cyclic_unregister_all());
        ut_assertok(event_uninit());
 
        free(uts->of_other);