From 3a11eada38efbbe1517662d196d11c2e20d2b5ca Mon Sep 17 00:00:00 2001 From: Rasmus Villemoes Date: Tue, 21 May 2024 10:46:50 +0200 Subject: [PATCH 01/10] cyclic: stop strdup'ing name in cyclic_register() We are not checking the return value of strdup(), nor freeing the string in cyclic_unregister(). However, all current users either pass a string literal or the dev->name of the client device. So in all cases the name string will live at least as long as the cyclic_info is registered, so just make that a requirement. Reviewed-by: Stefan Roese Signed-off-by: Rasmus Villemoes --- common/cyclic.c | 2 +- include/cyclic.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/common/cyclic.c b/common/cyclic.c index a49bfc88f5c..c62e7fa7d19 100644 --- a/common/cyclic.c +++ b/common/cyclic.c @@ -40,7 +40,7 @@ struct cyclic_info *cyclic_register(cyclic_func_t func, uint64_t delay_us, /* Store values in struct */ cyclic->func = func; cyclic->ctx = ctx; - cyclic->name = strdup(name); + cyclic->name = name; cyclic->delay_us = delay_us; cyclic->start_time_us = timer_get_us(); hlist_add_head(&cyclic->list, cyclic_get_list()); diff --git a/include/cyclic.h b/include/cyclic.h index 44ad3cb6b80..38946216fb8 100644 --- a/include/cyclic.h +++ b/include/cyclic.h @@ -31,7 +31,7 @@ struct cyclic_info { void (*func)(void *ctx); void *ctx; - char *name; + const char *name; uint64_t delay_us; uint64_t start_time_us; uint64_t cpu_time_us; From df2b3829c67fc059fb4b6d8d7dfdc5946d199674 Mon Sep 17 00:00:00 2001 From: Rasmus Villemoes Date: Tue, 21 May 2024 10:46:51 +0200 Subject: [PATCH 02/10] wdt-uclass: prevent multiple cyclic_register calls Currently, the cyclic_register() done in wdt_start() is not undone in wdt_stop(). Moreover, calling wdt_start multiple times (which is perfectly allowed on an already started device, e.g. to change the timeout value) will result in another struct cyclic_info being registered, referring to the same watchdog device. This can easily be seen on e.g. a wandboard: => cyclic list function: watchdog@20bc000, cpu-time: 22 us, frequency: 1.01 times/s => wdt list watchdog@20bc000 (imx_wdt) => wdt dev watchdog@20bc000 => wdt start 50000 WDT: Started watchdog@20bc000 with servicing every 1000ms (50s timeout) => cyclic list function: watchdog@20bc000, cpu-time: 37 us, frequency: 1.03 times/s function: watchdog@20bc000, cpu-time: 241 us, frequency: 1.01 times/s => wdt start 12345 WDT: Started watchdog@20bc000 with servicing every 1000ms (12s timeout) => cyclic list function: watchdog@20bc000, cpu-time: 36 us, frequency: 1.03 times/s function: watchdog@20bc000, cpu-time: 100 us, frequency: 1.04 times/s function: watchdog@20bc000, cpu-time: 299 us, frequency: 1.00 times/s So properly unregister the watchdog device from the cyclic framework in wdt_stop(). In wdt_start(), we cannot just skip the registration, as the (new) timeout value may mean that we have to ask the cyclic framework to call us more often. So if we're already running, first unregister the old cyclic instance. Reviewed-by: Stefan Roese Signed-off-by: Rasmus Villemoes --- drivers/watchdog/wdt-uclass.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/drivers/watchdog/wdt-uclass.c b/drivers/watchdog/wdt-uclass.c index c88312ec721..12850016c93 100644 --- a/drivers/watchdog/wdt-uclass.c +++ b/drivers/watchdog/wdt-uclass.c @@ -121,10 +121,11 @@ int wdt_start(struct udevice *dev, u64 timeout_ms, ulong flags) struct wdt_priv *priv = dev_get_uclass_priv(dev); char str[16]; - priv->running = true; - memset(str, 0, 16); if (IS_ENABLED(CONFIG_WATCHDOG)) { + if (priv->running) + cyclic_unregister(priv->cyclic); + /* Register the watchdog driver as a cyclic function */ priv->cyclic = cyclic_register(wdt_cyclic, priv->reset_period * 1000, @@ -139,6 +140,7 @@ int wdt_start(struct udevice *dev, u64 timeout_ms, ulong flags) } } + priv->running = true; printf("WDT: Started %s with%s servicing %s (%ds timeout)\n", dev->name, IS_ENABLED(CONFIG_WATCHDOG) ? "" : "out", str, (u32)lldiv(timeout_ms, 1000)); @@ -159,6 +161,9 @@ int wdt_stop(struct udevice *dev) if (ret == 0) { struct wdt_priv *priv = dev_get_uclass_priv(dev); + if (IS_ENABLED(CONFIG_WATCHDOG) && priv->running) + cyclic_unregister(priv->cyclic); + priv->running = false; } From 008c4b3c3115f7f95467773f12bad0db7649e786 Mon Sep 17 00:00:00 2001 From: Rasmus Villemoes Date: Tue, 21 May 2024 10:46:52 +0200 Subject: [PATCH 03/10] cyclic: make clients embed a struct cyclic_info in their own data structure There are of course not a whole lot of examples in-tree yet, but before they appear, let's make this API change: Instead of separately allocating a 'struct cyclic_info', make the users embed such an instance in their own structure, and make the convention that the callback simply receives the 'struct cyclic_info *', from which the clients can get their own data using the container_of() macro. This has a number of advantages. First, it means cyclic_register() simply cannot fail, simplifying the code. The necessary storage will simply be allocated automatically when the client's own structure is allocated (often via uclass_priv_auto or similar). Second, code for which CONFIG_CYCLIC is just an option can more easily be written without #ifdefs, if we just provide an empty struct cyclic_info {}. For example, the nested CONFIG_IS_ENABLED()s in https://lore.kernel.org/u-boot/20240316201416.211480-1-marek.vasut+renesas@mailbox.org/ are mostly due to the existence of the 'struct cyclic_info *' member being guarded by #ifdef CONFIG_CYCLIC. And we do probably want to avoid the extra memory overhead of that member when !CONFIG_CYCLIC. But that is automatic if, instead of a 'struct cyclic_info *', one simply embeds a 'struct cyclic_info', which will have size 0 when !CONFIG_CYCLIC. Also, the no-op cyclic_register() function can just unconditionally be called, and the compiler will see that (1) the callback is referenced, so not emit a warning for a maybe-unused function and (2) see that it can actually never be reached, so not emit any code for it. Reviewed-by: Stefan Roese Signed-off-by: Rasmus Villemoes --- board/Marvell/octeon_nic23/board.c | 11 ++++++---- cmd/cyclic.c | 12 +++++----- common/cyclic.c | 22 +++++-------------- doc/develop/cyclic.rst | 26 +++++++++++++--------- drivers/watchdog/wdt-uclass.c | 33 +++++++++++++--------------- include/cyclic.h | 35 +++++++++++++++--------------- test/common/cyclic.c | 19 ++++++++++------ 7 files changed, 78 insertions(+), 80 deletions(-) diff --git a/board/Marvell/octeon_nic23/board.c b/board/Marvell/octeon_nic23/board.c index bc9332cb74a..cf20c97684a 100644 --- a/board/Marvell/octeon_nic23/board.c +++ b/board/Marvell/octeon_nic23/board.c @@ -249,7 +249,7 @@ void board_configure_qlms(void) * read the incorrect device ID 0x9700 (reset value) instead of 0x9702 * (restored value). */ -static void octeon_board_restore_pf(void *ctx) +static void octeon_board_restore_pf(struct cyclic_info *c) { union cvmx_spemx_flr_pf_stopreq stopreq; static bool start_initialized[2] = {false, false}; @@ -357,10 +357,13 @@ int board_late_init(void) board_configure_qlms(); /* Register cyclic function for PCIe FLR fixup */ - cyclic = cyclic_register(octeon_board_restore_pf, 100, - "pcie_flr_fix", NULL); - if (!cyclic) + cyclic = calloc(1, sizeof(*cyclic)); + if (cyclic) { + cyclic_register(cyclic, octeon_board_restore_pf, 100, + "pcie_flr_fix"); + } else { printf("Registering of cyclic function failed\n"); + } return 0; } diff --git a/cmd/cyclic.c b/cmd/cyclic.c index 40e966de9aa..339dd4a7bce 100644 --- a/cmd/cyclic.c +++ b/cmd/cyclic.c @@ -15,14 +15,16 @@ #include #include #include +#include struct cyclic_demo_info { + struct cyclic_info cyclic; uint delay_us; }; -static void cyclic_demo(void *ctx) +static void cyclic_demo(struct cyclic_info *c) { - struct cyclic_demo_info *info = ctx; + struct cyclic_demo_info *info = container_of(c, struct cyclic_demo_info, cyclic); /* Just a small dummy delay here */ udelay(info->delay_us); @@ -32,7 +34,6 @@ static int do_cyclic_demo(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) { struct cyclic_demo_info *info; - struct cyclic_info *cyclic; uint time_ms; if (argc < 3) @@ -48,10 +49,7 @@ static int do_cyclic_demo(struct cmd_tbl *cmdtp, int flag, int argc, info->delay_us = simple_strtoul(argv[2], NULL, 0); /* Register demo cyclic function */ - cyclic = cyclic_register(cyclic_demo, time_ms * 1000, "cyclic_demo", - info); - if (!cyclic) - printf("Registering of cyclic_demo failed\n"); + cyclic_register(&info->cyclic, cyclic_demo, time_ms * 1000, "cyclic_demo"); printf("Registered function \"%s\" to be executed all %dms\n", "cyclic_demo", time_ms); diff --git a/common/cyclic.c b/common/cyclic.c index c62e7fa7d19..ec38fad6775 100644 --- a/common/cyclic.c +++ b/common/cyclic.c @@ -26,34 +26,22 @@ struct hlist_head *cyclic_get_list(void) return (struct hlist_head *)&gd->cyclic_list; } -struct cyclic_info *cyclic_register(cyclic_func_t func, uint64_t delay_us, - const char *name, void *ctx) +void cyclic_register(struct cyclic_info *cyclic, cyclic_func_t func, + uint64_t delay_us, const char *name) { - struct cyclic_info *cyclic; - - cyclic = calloc(1, sizeof(struct cyclic_info)); - if (!cyclic) { - pr_debug("Memory allocation error\n"); - return NULL; - } + memset(cyclic, 0, sizeof(*cyclic)); /* Store values in struct */ cyclic->func = func; - cyclic->ctx = ctx; cyclic->name = name; cyclic->delay_us = delay_us; cyclic->start_time_us = timer_get_us(); hlist_add_head(&cyclic->list, cyclic_get_list()); - - return cyclic; } -int cyclic_unregister(struct cyclic_info *cyclic) +void cyclic_unregister(struct cyclic_info *cyclic) { hlist_del(&cyclic->list); - free(cyclic); - - return 0; } void cyclic_run(void) @@ -76,7 +64,7 @@ void cyclic_run(void) if (time_after_eq64(now, cyclic->next_call)) { /* Call cyclic function and account it's cpu-time */ cyclic->next_call = now + cyclic->delay_us; - cyclic->func(cyclic->ctx); + cyclic->func(cyclic); cyclic->run_cnt++; cpu_time = timer_get_us() - now; cyclic->cpu_time_us += cpu_time; diff --git a/doc/develop/cyclic.rst b/doc/develop/cyclic.rst index 67831496a70..893c269099a 100644 --- a/doc/develop/cyclic.rst +++ b/doc/develop/cyclic.rst @@ -19,20 +19,26 @@ Registering a cyclic function To register a cyclic function, use something like this:: - static void cyclic_demo(void *ctx) + struct donkey { + struct cyclic_info cyclic; + void (*say)(const char *s); + }; + + static void cyclic_demo(struct cyclic_info *c) { - /* Just a small dummy delay here */ - udelay(10); + struct donkey *donkey = container_of(c, struct donkey, cyclic); + + donkey->say("Are we there yet?"); } - - int board_init(void) + + int donkey_init(void) { - struct cyclic_info *cyclic; - + struct donkey *donkey; + + /* Initialize donkey ... */ + /* Register demo cyclic function */ - cyclic = cyclic_register(cyclic_demo, 10 * 1000, "cyclic_demo", NULL); - if (!cyclic) - printf("Registering of cyclic_demo failed\n"); + cyclic_register(&donkey->cyclic, cyclic_demo, 10 * 1000, "cyclic_demo"); return 0; } diff --git a/drivers/watchdog/wdt-uclass.c b/drivers/watchdog/wdt-uclass.c index 12850016c93..e2e7f9ab84b 100644 --- a/drivers/watchdog/wdt-uclass.c +++ b/drivers/watchdog/wdt-uclass.c @@ -17,12 +17,15 @@ #include #include #include +#include DECLARE_GLOBAL_DATA_PTR; #define WATCHDOG_TIMEOUT_SECS (CONFIG_WATCHDOG_TIMEOUT_MSECS / 1000) struct wdt_priv { + /* The udevice owning this wdt_priv. */ + struct udevice *dev; /* Timeout, in seconds, to configure this device to. */ u32 timeout; /* @@ -40,18 +43,17 @@ struct wdt_priv { /* autostart */ bool autostart; - struct cyclic_info *cyclic; + struct cyclic_info cyclic; }; -static void wdt_cyclic(void *ctx) +static void wdt_cyclic(struct cyclic_info *c) { - struct udevice *dev = ctx; - struct wdt_priv *priv; + struct wdt_priv *priv = container_of(c, struct wdt_priv, cyclic); + struct udevice *dev = priv->dev; if (!device_active(dev)) return; - priv = dev_get_uclass_priv(dev); if (!priv->running) return; @@ -124,20 +126,14 @@ int wdt_start(struct udevice *dev, u64 timeout_ms, ulong flags) memset(str, 0, 16); if (IS_ENABLED(CONFIG_WATCHDOG)) { if (priv->running) - cyclic_unregister(priv->cyclic); + cyclic_unregister(&priv->cyclic); /* Register the watchdog driver as a cyclic function */ - priv->cyclic = cyclic_register(wdt_cyclic, - priv->reset_period * 1000, - dev->name, dev); - if (!priv->cyclic) { - printf("cyclic_register for %s failed\n", - dev->name); - return -ENODEV; - } else { - snprintf(str, 16, "every %ldms", - priv->reset_period); - } + cyclic_register(&priv->cyclic, wdt_cyclic, + priv->reset_period * 1000, + dev->name); + + snprintf(str, 16, "every %ldms", priv->reset_period); } priv->running = true; @@ -162,7 +158,7 @@ int wdt_stop(struct udevice *dev) struct wdt_priv *priv = dev_get_uclass_priv(dev); if (IS_ENABLED(CONFIG_WATCHDOG) && priv->running) - cyclic_unregister(priv->cyclic); + cyclic_unregister(&priv->cyclic); priv->running = false; } @@ -262,6 +258,7 @@ static int wdt_pre_probe(struct udevice *dev) autostart = true; } priv = dev_get_uclass_priv(dev); + priv->dev = dev; priv->timeout = timeout; priv->reset_period = reset_period; priv->autostart = autostart; diff --git a/include/cyclic.h b/include/cyclic.h index 38946216fb8..2c3d383c5ef 100644 --- a/include/cyclic.h +++ b/include/cyclic.h @@ -18,7 +18,6 @@ * struct cyclic_info - Information about cyclic execution function * * @func: Function to call periodically - * @ctx: Context pointer to get passed to this function * @name: Name of the cyclic function, e.g. shown in the commands * @delay_ns: Delay is ns after which this function shall get executed * @start_time_us: Start time in us, when this function started its execution @@ -27,10 +26,12 @@ * @next_call: Next time in us, when the function shall be executed again * @list: List node * @already_warned: Flag that we've warned about exceeding CPU time usage + * + * When !CONFIG_CYCLIC, this struct is empty. */ struct cyclic_info { - void (*func)(void *ctx); - void *ctx; +#if defined(CONFIG_CYCLIC) + void (*func)(struct cyclic_info *c); const char *name; uint64_t delay_us; uint64_t start_time_us; @@ -39,31 +40,34 @@ struct cyclic_info { uint64_t next_call; struct hlist_node list; bool already_warned; +#endif }; /** Function type for cyclic functions */ -typedef void (*cyclic_func_t)(void *ctx); +typedef void (*cyclic_func_t)(struct cyclic_info *c); #if defined(CONFIG_CYCLIC) /** * cyclic_register - Register a new cyclic function * + * @cyclic: Cyclic info structure * @func: Function to call periodically * @delay_us: Delay is us after which this function shall get executed * @name: Cyclic function name/id - * @ctx: Context to pass to the function - * @return: pointer to cyclic_struct if OK, NULL on error + * + * The function @func will be called with @cyclic as its + * argument. @cyclic will usually be embedded in some device-specific + * structure, which the callback can retrieve using container_of(). */ -struct cyclic_info *cyclic_register(cyclic_func_t func, uint64_t delay_us, - const char *name, void *ctx); +void cyclic_register(struct cyclic_info *cyclic, cyclic_func_t func, + uint64_t delay_us, const char *name); /** * cyclic_unregister - Unregister a cyclic function * * @cyclic: Pointer to cyclic_struct of the function that shall be removed - * @return: 0 if OK, -ve on error */ -int cyclic_unregister(struct cyclic_info *cyclic); +void cyclic_unregister(struct cyclic_info *cyclic); /** * cyclic_unregister_all() - Clean up cyclic functions @@ -97,17 +101,14 @@ void cyclic_run(void); */ void schedule(void); #else -static inline struct cyclic_info *cyclic_register(cyclic_func_t func, - uint64_t delay_us, - const char *name, - void *ctx) + +static inline void cyclic_register(struct cyclic_info *cyclic, cyclic_func_t func, + uint64_t delay_us, const char *name) { - return NULL; } -static inline int cyclic_unregister(struct cyclic_info *cyclic) +static inline void cyclic_unregister(struct cyclic_info *cyclic) { - return 0; } static inline void cyclic_run(void) diff --git a/test/common/cyclic.c b/test/common/cyclic.c index 461f8cf91f4..51a07c576b6 100644 --- a/test/common/cyclic.c +++ b/test/common/cyclic.c @@ -12,22 +12,27 @@ #include /* Test that cyclic function is called */ -static bool cyclic_active = false; +static struct cyclic_test { + struct cyclic_info cyclic; + bool called; +} cyclic_test; -static void cyclic_test(void *ctx) +static void test_cb(struct cyclic_info *c) { - cyclic_active = true; + struct cyclic_test *t = container_of(c, struct cyclic_test, cyclic); + t->called = true; } static int dm_test_cyclic_running(struct unit_test_state *uts) { - cyclic_active = false; - ut_assertnonnull(cyclic_register(cyclic_test, 10 * 1000, "cyclic_demo", - NULL)); + cyclic_test.called = false; + cyclic_register(&cyclic_test.cyclic, test_cb, 10 * 1000, "cyclic_test"); /* Execute all registered cyclic functions */ schedule(); - ut_asserteq(true, cyclic_active); + ut_asserteq(true, cyclic_test.called); + + cyclic_unregister(&cyclic_test.cyclic); return 0; } From 7a779a8d09234d1be1a85b3a49287d0512f24419 Mon Sep 17 00:00:00 2001 From: Rasmus Villemoes Date: Tue, 28 May 2024 13:13:19 +0200 Subject: [PATCH 04/10] m68k: remove dead code There are no calls of "watchdog_reset()" anymore anywhere in the tree since the WATCHDOG_RESET macro got removed in 942d07df0e79 ("watchdog: Remove WATCHDOG_RESET macro"). The only places the identifiers watchdog_disable and watchdog_init are called are in arch/arm/mach-omap2/, so those can obviously not refer to these instances. Hence these functions are not actually used at all and can be removed. As a bonus, this also removes two leftover references to WATCHDOG_RESET. Cc: Huan Wang Cc: Angelo Dureghello Signed-off-by: Rasmus Villemoes Reviewed-by: Stefan Roese --- arch/m68k/cpu/mcf52x2/cpu.c | 108 ------------------------------------ 1 file changed, 108 deletions(-) diff --git a/arch/m68k/cpu/mcf52x2/cpu.c b/arch/m68k/cpu/mcf52x2/cpu.c index 6bfde5e9bd7..d0a0a4500a9 100644 --- a/arch/m68k/cpu/mcf52x2/cpu.c +++ b/arch/m68k/cpu/mcf52x2/cpu.c @@ -108,26 +108,6 @@ int do_reset(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) return 0; }; -#if defined(CONFIG_WATCHDOG) -void watchdog_reset(void) -{ - mbar_writeShort(MCF_WTM_WSR, 0x5555); - mbar_writeShort(MCF_WTM_WSR, 0xAAAA); -} - -int watchdog_disable(void) -{ - mbar_writeShort(MCF_WTM_WCR, 0); - return (0); -} - -int watchdog_init(void) -{ - mbar_writeShort(MCF_WTM_WCR, MCF_WTM_WCR_EN); - return (0); -} -#endif /* #ifdef CONFIG_WATCHDOG */ - #endif #ifdef CONFIG_M5272 @@ -174,49 +154,6 @@ int print_cpuinfo(void) }; #endif /* CONFIG_DISPLAY_CPUINFO */ -#if defined(CONFIG_WATCHDOG) -/* Called by macro WATCHDOG_RESET */ -void watchdog_reset(void) -{ - wdog_t *wdt = (wdog_t *)(MMAP_WDOG); - - out_be16(&wdt->wdog_wcr, 0); -} - -int watchdog_disable(void) -{ - wdog_t *wdt = (wdog_t *)(MMAP_WDOG); - - /* reset watchdog counter */ - out_be16(&wdt->wdog_wcr, 0); - /* disable watchdog interrupt */ - out_be16(&wdt->wdog_wirr, 0); - /* disable watchdog timer */ - out_be16(&wdt->wdog_wrrr, 0); - - puts("WATCHDOG:disabled\n"); - return (0); -} - -int watchdog_init(void) -{ - wdog_t *wdt = (wdog_t *)(MMAP_WDOG); - - /* disable watchdog interrupt */ - out_be16(&wdt->wdog_wirr, 0); - - /* set timeout and enable watchdog */ - out_be16(&wdt->wdog_wrrr, - (CONFIG_WATCHDOG_TIMEOUT_MSECS * CONFIG_SYS_HZ) / (32768 * 1000) - 1); - - /* reset watchdog counter */ - out_be16(&wdt->wdog_wcr, 0); - - puts("WATCHDOG:enabled\n"); - return (0); -} -#endif /* #ifdef CONFIG_WATCHDOG */ - #endif /* #ifdef CONFIG_M5272 */ #ifdef CONFIG_M5275 @@ -243,51 +180,6 @@ int print_cpuinfo(void) }; #endif /* CONFIG_DISPLAY_CPUINFO */ -#if defined(CONFIG_WATCHDOG) -/* Called by macro WATCHDOG_RESET */ -void watchdog_reset(void) -{ - wdog_t *wdt = (wdog_t *)(MMAP_WDOG); - - out_be16(&wdt->wsr, 0x5555); - out_be16(&wdt->wsr, 0xaaaa); -} - -int watchdog_disable(void) -{ - wdog_t *wdt = (wdog_t *)(MMAP_WDOG); - - /* reset watchdog counter */ - out_be16(&wdt->wsr, 0x5555); - out_be16(&wdt->wsr, 0xaaaa); - - /* disable watchdog timer */ - out_be16(&wdt->wcr, 0); - - puts("WATCHDOG:disabled\n"); - return (0); -} - -int watchdog_init(void) -{ - wdog_t *wdt = (wdog_t *)(MMAP_WDOG); - - /* disable watchdog */ - out_be16(&wdt->wcr, 0); - - /* set timeout and enable watchdog */ - out_be16(&wdt->wmr, - (CONFIG_WATCHDOG_TIMEOUT_MSECS * CONFIG_SYS_HZ) / (32768 * 1000) - 1); - - /* reset watchdog counter */ - out_be16(&wdt->wsr, 0x5555); - out_be16(&wdt->wsr, 0xaaaa); - - puts("WATCHDOG:enabled\n"); - return (0); -} -#endif /* #ifdef CONFIG_WATCHDOG */ - #endif /* #ifdef CONFIG_M5275 */ #ifdef CONFIG_M5282 From 4388ada7690e977f34cdab78b0df5c185a9e3c67 Mon Sep 17 00:00:00 2001 From: Rasmus Villemoes Date: Tue, 28 May 2024 13:13:20 +0200 Subject: [PATCH 05/10] wdt-uclass: watchdog_reset cleanup watchdog_reset() is no longer called from anywhere, so we do not need to define a dummy no-op function. Remove that definition, and update references to say schedule() instead. Signed-off-by: Rasmus Villemoes Reviewed-by: Stefan Roese --- drivers/watchdog/wdt-uclass.c | 19 ++----------------- 1 file changed, 2 insertions(+), 17 deletions(-) diff --git a/drivers/watchdog/wdt-uclass.c b/drivers/watchdog/wdt-uclass.c index e2e7f9ab84b..10be334e9ed 100644 --- a/drivers/watchdog/wdt-uclass.c +++ b/drivers/watchdog/wdt-uclass.c @@ -30,7 +30,7 @@ struct wdt_priv { u32 timeout; /* * Time, in milliseconds, between calling the device's ->reset() - * method from watchdog_reset(). + * method from schedule(). */ ulong reset_period; /* @@ -222,21 +222,6 @@ int wdt_expire_now(struct udevice *dev, ulong flags) return ret; } -#if defined(CONFIG_WATCHDOG) -/* - * Called by macro WATCHDOG_RESET. This function be called *very* early, - * so we need to make sure, that the watchdog driver is ready before using - * it in this function. - */ -void watchdog_reset(void) -{ - /* - * Empty function for now. The actual WDT handling is now done in - * the cyclic function instead. - */ -} -#endif - static int wdt_pre_probe(struct udevice *dev) { u32 timeout = WATCHDOG_TIMEOUT_SECS; @@ -264,7 +249,7 @@ static int wdt_pre_probe(struct udevice *dev) priv->autostart = autostart; /* * Pretend this device was last reset "long" ago so the first - * watchdog_reset will actually call its ->reset method. + * schedule() will actually call its ->reset method. */ priv->next_reset = get_timer(0); From 945fc278226cf2de432ac9ccf56b23520300afce Mon Sep 17 00:00:00 2001 From: Rasmus Villemoes Date: Tue, 28 May 2024 13:13:21 +0200 Subject: [PATCH 06/10] serial: ns16550: fix comment to mention schedule instead of watchdog_reset watchdog_reset() is no more. Make the comments match the code and today's reality. Signed-off-by: Rasmus Villemoes Reviewed-by: Stefan Roese --- drivers/serial/ns16550.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/serial/ns16550.c b/drivers/serial/ns16550.c index 4963385dc1c..42b69719dd7 100644 --- a/drivers/serial/ns16550.c +++ b/drivers/serial/ns16550.c @@ -291,9 +291,9 @@ void ns16550_putc(struct ns16550 *com_port, char c) serial_out(c, &com_port->thr); /* - * Call watchdog_reset() upon newline. This is done here in putc + * Call schedule() upon newline. This is done here in putc * since the environment code uses a single puts() to print the complete - * environment upon "printenv". So we can't put this watchdog call + * environment upon "printenv". So we can't put this schedule call * in puts(). */ if (c == '\n') @@ -390,9 +390,9 @@ static int ns16550_serial_putc(struct udevice *dev, const char ch) serial_out(ch, &com_port->thr); /* - * Call watchdog_reset() upon newline. This is done here in putc + * Call schedule() upon newline. This is done here in putc * since the environment code uses a single puts() to print the complete - * environment upon "printenv". So we can't put this watchdog call + * environment upon "printenv". So we can't put this schedule call * in puts(). */ if (ch == '\n') From 167f841ed44ea0e09f1f24d1eb66aa4fd84ffa15 Mon Sep 17 00:00:00 2001 From: Rasmus Villemoes Date: Tue, 28 May 2024 13:13:22 +0200 Subject: [PATCH 07/10] sh4: move reset_cpu() from watchdog.c to cpu.c The next patch will remove all the other code from watchdog.c, which would leave just this function in there. It seems just as natural for this function to be defined in cpu.c, allowing us to delete watchdog.c completely. Cc: Nobuhiro Iwamatsu Signed-off-by: Rasmus Villemoes Reviewed-by: Stefan Roese --- arch/sh/cpu/sh4/cpu.c | 10 ++++++++++ arch/sh/cpu/sh4/watchdog.c | 9 --------- 2 files changed, 10 insertions(+), 9 deletions(-) diff --git a/arch/sh/cpu/sh4/cpu.c b/arch/sh/cpu/sh4/cpu.c index b0ad685a91b..47a8549beba 100644 --- a/arch/sh/cpu/sh4/cpu.c +++ b/arch/sh/cpu/sh4/cpu.c @@ -10,6 +10,16 @@ #include #include #include +#include + +void reset_cpu(void) +{ + /* Address error with SR.BL=1 first. */ + trigger_address_error(); + + while (1) + ; +} int checkcpu(void) { diff --git a/arch/sh/cpu/sh4/watchdog.c b/arch/sh/cpu/sh4/watchdog.c index c5974337465..d394ac0b69b 100644 --- a/arch/sh/cpu/sh4/watchdog.c +++ b/arch/sh/cpu/sh4/watchdog.c @@ -49,12 +49,3 @@ int watchdog_disable(void) return 0; } #endif - -void reset_cpu(void) -{ - /* Address error with SR.BL=1 first. */ - trigger_address_error(); - - while (1) - ; -} From 73a8cdeb5cb2d7682bac88cfad175e26a1dcabad Mon Sep 17 00:00:00 2001 From: Rasmus Villemoes Date: Tue, 28 May 2024 13:13:23 +0200 Subject: [PATCH 08/10] sh4: remove watchdog.c file The external functions defined here are not called from anywhere. So they, and consequently the whole file, can be dropped. Cc: Nobuhiro Iwamatsu Signed-off-by: Rasmus Villemoes Reviewed-by: Stefan Roese --- arch/sh/cpu/sh4/Makefile | 2 +- arch/sh/cpu/sh4/watchdog.c | 51 -------------------------------------- 2 files changed, 1 insertion(+), 52 deletions(-) delete mode 100644 arch/sh/cpu/sh4/watchdog.c diff --git a/arch/sh/cpu/sh4/Makefile b/arch/sh/cpu/sh4/Makefile index 7403a2c3047..6d7e05ebc29 100644 --- a/arch/sh/cpu/sh4/Makefile +++ b/arch/sh/cpu/sh4/Makefile @@ -6,4 +6,4 @@ # (C) Copyright 2007 # Nobuhiro Iwamatsu -obj-y = cpu.o interrupts.o watchdog.o cache.o +obj-y = cpu.o interrupts.o cache.o diff --git a/arch/sh/cpu/sh4/watchdog.c b/arch/sh/cpu/sh4/watchdog.c deleted file mode 100644 index d394ac0b69b..00000000000 --- a/arch/sh/cpu/sh4/watchdog.c +++ /dev/null @@ -1,51 +0,0 @@ -// SPDX-License-Identifier: GPL-2.0+ - -#include -#include -#include -#include - -#define WDT_BASE WTCNT - -#define WDT_WD (1 << 6) -#define WDT_RST_P (0) -#define WDT_RST_M (1 << 5) -#define WDT_ENABLE (1 << 7) - -#if defined(CONFIG_WATCHDOG) -static unsigned char csr_read(void) -{ - return inb(WDT_BASE + 0x04); -} - -static void cnt_write(unsigned char value) -{ - outl((unsigned short)value | 0x5A00, WDT_BASE + 0x00); -} - -static void csr_write(unsigned char value) -{ - outl((unsigned short)value | 0xA500, WDT_BASE + 0x04); -} - -void watchdog_reset(void) -{ - outl(0x55000000, WDT_BASE + 0x08); -} - -int watchdog_init(void) -{ - /* Set overflow time*/ - cnt_write(0); - /* Power on reset */ - csr_write(WDT_WD|WDT_RST_P|WDT_ENABLE); - - return 0; -} - -int watchdog_disable(void) -{ - csr_write(csr_read() & ~WDT_ENABLE); - return 0; -} -#endif From e7dbc25c3071dca023a3ac1009af4815dede8e9a Mon Sep 17 00:00:00 2001 From: Rasmus Villemoes Date: Tue, 28 May 2024 13:13:24 +0200 Subject: [PATCH 09/10] powerpc: mpc83xx: remove unused watchdog_reset() function There is no longer any code in tree that calls a watchdog_reset() function. The macro WATCHDOG_RESET, which used to emit a call to watchdog_reset(), got removed two years ago. Cc: Christophe Leroy Signed-off-by: Rasmus Villemoes Reviewed-by: Stefan Roese --- arch/powerpc/cpu/mpc83xx/cpu.c | 15 --------------- 1 file changed, 15 deletions(-) diff --git a/arch/powerpc/cpu/mpc83xx/cpu.c b/arch/powerpc/cpu/mpc83xx/cpu.c index e0be938ea98..3c8cbd4252d 100644 --- a/arch/powerpc/cpu/mpc83xx/cpu.c +++ b/arch/powerpc/cpu/mpc83xx/cpu.c @@ -164,21 +164,6 @@ unsigned long get_tbclk(void) } #endif -#if defined(CONFIG_WATCHDOG) && !defined(CONFIG_WDT) -void watchdog_reset (void) -{ - int re_enable = disable_interrupts(); - - /* Reset the 83xx watchdog */ - volatile immap_t *immr = (immap_t *) CONFIG_SYS_IMMR; - immr->wdt.swsrr = 0x556c; - immr->wdt.swsrr = 0xaa39; - - if (re_enable) - enable_interrupts(); -} -#endif - /* * Initializes on-chip MMC controllers. * to override, implement board_mmc_init() From 85c476759a42dfedb2d66e9734f8c05b7cfb62d5 Mon Sep 17 00:00:00 2001 From: Rasmus Villemoes Date: Tue, 28 May 2024 13:13:25 +0200 Subject: [PATCH 10/10] powerpc: mpc85xx: remove dead watchdog-related code Nothing in-tree calls watchdog_reset() anymore (that stopped two years ago with the removal of the WATCHDOG_RESET macro). So that function is dead code. That was the only caller of reset_85xx_watchdog(), so that can obviously also be removed. Finally, init_85xx_watchdog() is/was also not called from anywhere, so that can go away as well, which nicely also removes a bit of arch-specific code from the generic watchdog.h header. Cc: Christophe Leroy Signed-off-by: Rasmus Villemoes Reviewed-by: Stefan Roese --- arch/powerpc/cpu/mpc85xx/cpu.c | 31 ------------------------------- include/watchdog.h | 3 --- 2 files changed, 34 deletions(-) diff --git a/arch/powerpc/cpu/mpc85xx/cpu.c b/arch/powerpc/cpu/mpc85xx/cpu.c index 6356b021638..ebce2fe3935 100644 --- a/arch/powerpc/cpu/mpc85xx/cpu.c +++ b/arch/powerpc/cpu/mpc85xx/cpu.c @@ -349,37 +349,6 @@ __weak unsigned long get_tbclk(void) } -#ifndef CONFIG_WDT -#if defined(CONFIG_WATCHDOG) -#define WATCHDOG_MASK (TCR_WP(63) | TCR_WRC(3) | TCR_WIE) -void -init_85xx_watchdog(void) -{ - mtspr(SPRN_TCR, (mfspr(SPRN_TCR) & ~WATCHDOG_MASK) | - TCR_WP(CFG_WATCHDOG_PRESC) | TCR_WRC(CFG_WATCHDOG_RC)); -} - -void -reset_85xx_watchdog(void) -{ - /* - * Clear TSR(WIS) bit by writing 1 - */ - mtspr(SPRN_TSR, TSR_WIS); -} - -void -watchdog_reset(void) -{ - int re_enable = disable_interrupts(); - - reset_85xx_watchdog(); - if (re_enable) - enable_interrupts(); -} -#endif /* CONFIG_WATCHDOG */ -#endif - /* * Initializes on-chip MMC controllers. * to override, implement board_mmc_init() diff --git a/include/watchdog.h b/include/watchdog.h index ac5f11e376f..d1956fafca1 100644 --- a/include/watchdog.h +++ b/include/watchdog.h @@ -40,7 +40,4 @@ int init_func_watchdog_reset(void); void hw_watchdog_init(void); #endif -#if defined(CONFIG_MPC85xx) - void init_85xx_watchdog(void); -#endif #endif /* _WATCHDOG_H_ */