[U-Boot] [U-Boot, 02/36] rockchip: add common MACRO to enable sys arch timer

[Philipp Tomsich]

On Tue, 27 Mar 2018, Kever Yang wrote:

> All rockchip SoCs can use ARM arch timer, let's enable it in
> common header file

Please provide a commit message that is more descriptive of what actually 
happens... i.e. that COUNTER_FREQUENCY gets moved to a common header.  It 
would be great to document why this will always remain 24M.

> Signed-off-by: Kever Yang <kever.yang at rock-chips.com>
> Acked-by: Philipp Tomsich <philipp.tomsich at theobroma-systems.com>

See below for requested changes.

> ---
>
> include/configs/rk3368_common.h   | 2 --
> include/configs/rk3399_common.h   | 2 --
> include/configs/rockchip-common.h | 4 ++++
> 3 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/include/configs/rk3368_common.h b/include/configs/rk3368_common.h
> index 10f643f..a7fe4ca 100644
> --- a/include/configs/rk3368_common.h
> +++ b/include/configs/rk3368_common.h
> @@ -22,8 +22,6 @@
> #define CONFIG_SYS_CBSIZE		1024
> #define CONFIG_SKIP_LOWLEVEL_INIT
>
> -#define COUNTER_FREQUENCY               24000000
> -
> #define CONFIG_SYS_NS16550_MEM32
>
> #define CONFIG_SYS_INIT_SP_ADDR		0x00300000
> diff --git a/include/configs/rk3399_common.h b/include/configs/rk3399_common.h
> index d700bf2..fe8c675 100644
> --- a/include/configs/rk3399_common.h
> +++ b/include/configs/rk3399_common.h
> @@ -17,8 +17,6 @@
> #define CONFIG_SPL_SPI_LOAD
> #endif
>
> -#define COUNTER_FREQUENCY               24000000
> -
> #define CONFIG_SYS_NS16550_MEM32
>
> #define CONFIG_SYS_INIT_SP_ADDR		0x00300000
> diff --git a/include/configs/rockchip-common.h b/include/configs/rockchip-common.h
> index 26d41b5..24651ce 100644
> --- a/include/configs/rockchip-common.h
> +++ b/include/configs/rockchip-common.h
> @@ -8,6 +8,10 @@
> #define _ROCKCHIP_COMMON_H_
> #include <linux/sizes.h>
>
> +#define COUNTER_FREQUENCY               24000000

Is this really safe for all past, current and future SOCs (after all: you 
are putting this into 'rockchip-common.h'?

> +#define CONFIG_SYS_ARCH_TIMER

I don't agree with putting this here, as the CONFIG_SYS_ARCH_TIMER 
definition is only used on ARMv7, but this file is also included by ARMv8 
SOCs.

> +#define CONFIG_SYS_HZ_CLOCK	24000000

You might want to have this refer back to COUNTER_FREQUENCY.

> +
> #ifndef CONFIG_SPL_BUILD
>
> /* First try to boot from SD (index 0), then eMMC (index 1) */
>