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

From: Kever Yang <kever.yang@rock-chips.com>
To: u-boot@lists.denx.de
Subject: [U-Boot] [U-Boot, 02/36] rockchip: add common MACRO to enable sys arch timer
Date: Mon, 2 Apr 2018 09:41:37 +0800	[thread overview]
Message-ID: <e149f05d-fe4b-556b-3387-c3891353f49e@rock-chips.com> (raw)
In-Reply-To: <alpine.OSX.2.21.1804012247490.30208@vpn-10-11-0-14.lan>

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) */
>>
>