Re: [PATCH v2 1/5] cyclic: Add a symbol for SPL

[Devarsh Thakkar <devarsht@ti.com>]

Hi Simon,

On 02/12/23 21:03, Simon Glass wrote:
> The cyclic subsystem is currently enabled either in all build phases
> or none. For tools this should not be enabled, but since lib/shc256.c
> and other files include watchdog.h in the host build, we must make
> sure that it is not enabled there.
> 
> Add an SPL symbol so that there is more control of this.
> 
> Add an include into cyclic.h so that tools can include this file.
> 
> Also add the kconfig.h header so that CONFIG_IS_ENABLED() works. We
> could avoid this for now by moving the location of the watchdog.h
> inclusion to outside the USE_HOSTCC area. But at some point the #ifdefs
> from these files will likely be removed, so there is no benefit in
> going that way.
> 
> Signed-off-by: Simon Glass <sjg@chromium.org>

Reviewed-by: Devarsh Thakkar <devarsht@ti.com>

Regards
Devarsh
> ---
> 
> Changes in v2:
> - Add an SPL_CYCLIC symbol
> - Add a lot more explanation about the header files
> 
>  common/Kconfig                    | 8 ++++++++
>  common/Makefile                   | 2 +-
>  drivers/watchdog/Kconfig          | 1 +
>  include/asm-generic/global_data.h | 2 +-
>  include/cyclic.h                  | 6 ++++--
>  5 files changed, 15 insertions(+), 4 deletions(-)
> 
> diff --git a/common/Kconfig b/common/Kconfig
> index 0283701f1d05..5906a4af7c33 100644
> --- a/common/Kconfig
> +++ b/common/Kconfig
> @@ -626,6 +626,14 @@ config CYCLIC
>  
>  if CYCLIC
>  
> +config SPL_CYCLIC
> +	bool "General-purpose cyclic execution mechanism (SPL)"
> +	help
> +	  This enables a general-purpose cyclic execution infrastructure in SPL,
> +	  to allow "small" (run-time wise) functions to be executed at
> +	  a specified frequency. Things like LED blinking or watchdog
> +	  triggering are examples for such tasks.
> +
>  config CYCLIC_MAX_CPU_TIME_US
>  	int "Sets the max allowed time for a cyclic function in us"
>  	default 1000
> diff --git a/common/Makefile b/common/Makefile
> index 1495436d5d45..27443863bf9b 100644
> --- a/common/Makefile
> +++ b/common/Makefile
> @@ -77,7 +77,7 @@ obj-$(CONFIG_CROS_EC) += cros_ec.o
>  obj-y += dlmalloc.o
>  obj-$(CONFIG_$(SPL_TPL_)SYS_MALLOC_F) += malloc_simple.o
>  
> -obj-$(CONFIG_CYCLIC) += cyclic.o
> +obj-$(CONFIG_$(SPL_TPL_)CYCLIC) += cyclic.o
>  obj-$(CONFIG_$(SPL_TPL_)EVENT) += event.o
>  
>  obj-$(CONFIG_$(SPL_TPL_)HASH) += hash.o
> diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
> index 07fc4940e918..378435c55cc7 100644
> --- a/drivers/watchdog/Kconfig
> +++ b/drivers/watchdog/Kconfig
> @@ -395,6 +395,7 @@ config WDT_ARM_SMC
>  config SPL_WDT
>  	bool "Enable driver model for watchdog timer drivers in SPL"
>  	depends on SPL_DM
> +	select SPL_CYCLIC if CYCLIC
>  	help
>  	  Enable driver model for watchdog timer in SPL.
>  	  This is similar to CONFIG_WDT in U-Boot.
> diff --git a/include/asm-generic/global_data.h b/include/asm-generic/global_data.h
> index e8c6412e3f8d..77f11a4383c9 100644
> --- a/include/asm-generic/global_data.h
> +++ b/include/asm-generic/global_data.h
> @@ -485,7 +485,7 @@ struct global_data {
>  	 */
>  	struct event_state event_state;
>  #endif
> -#ifdef CONFIG_CYCLIC
> +#if CONFIG_IS_ENABLED(CYCLIC)
>  	/**
>  	 * @cyclic_list: list of registered cyclic functions
>  	 */
> diff --git a/include/cyclic.h b/include/cyclic.h
> index 44ad3cb6b803..d3b368dd90df 100644
> --- a/include/cyclic.h
> +++ b/include/cyclic.h
> @@ -11,6 +11,7 @@
>  #ifndef __cyclic_h
>  #define __cyclic_h
>  
> +#include <linux/kconfig.h>
>  #include <linux/list.h>
>  #include <asm/types.h>
>  
> @@ -44,7 +45,8 @@ struct cyclic_info {
>  /** Function type for cyclic functions */
>  typedef void (*cyclic_func_t)(void *ctx);
>  
> -#if defined(CONFIG_CYCLIC)
> +#if CONFIG_IS_ENABLED(CYCLIC)
> +
>  /**
>   * cyclic_register - Register a new cyclic function
>   *
> @@ -122,6 +124,6 @@ static inline int cyclic_unregister_all(void)
>  {
>  	return 0;
>  }
> -#endif
> +#endif /* CYCLIC */
>  
>  #endif