From 3a11eada38efbbe1517662d196d11c2e20d2b5ca Mon Sep 17 00:00:00 2001 From: Rasmus Villemoes Date: Tue, 21 May 2024 10:46:50 +0200 Subject: 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 --- include/cyclic.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'include') 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; -- cgit v1.2.3 From 008c4b3c3115f7f95467773f12bad0db7649e786 Mon Sep 17 00:00:00 2001 From: Rasmus Villemoes Date: Tue, 21 May 2024 10:46:52 +0200 Subject: 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 --- include/cyclic.h | 35 ++++++++++++++++++----------------- 1 file changed, 18 insertions(+), 17 deletions(-) (limited to 'include') 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) -- cgit v1.2.3 From 85c476759a42dfedb2d66e9734f8c05b7cfb62d5 Mon Sep 17 00:00:00 2001 From: Rasmus Villemoes Date: Tue, 28 May 2024 13:13:25 +0200 Subject: 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 --- include/watchdog.h | 3 --- 1 file changed, 3 deletions(-) (limited to 'include') 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_ */ -- cgit v1.2.3