On 04/02/2018 04:51 AM, Philipp Tomsich wrote:
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.
Well, will add this info.
It would be great to document why this will always remain 24M.
All the setting in header file for Rockchip is 24M, doesn't it already a common code and we should re-use it in common file?
Signed-off-by: Kever Yang kever.yang@rock-chips.com Acked-by: Philipp Tomsich philipp.tomsich@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'?
Rockchip timer always have a option of 24M, it may not default in SoC value, we need to select to use 24M in this case.
+#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.
Does this setting break anything in ARMv8? I don't think we need to add a rockchip_common_armv7.h, at least we can add macro for the definition is used for CONFIG_ARM64 or not.
Thanks, - Kever
+#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) */