Kever, [Sorry for the delay, I am switching laptops and this got stuck in my Drafts folder on the old machine — which I noticed only today]
> On 3 Sep 2018, at 05:21, Kever Yang <kever.y...@rock-chips.com> wrote: > > Hi Philipp, > > > On 08/30/2018 05:11 PM, Philipp Tomsich wrote: >> >> >> On Wed, 18 Apr 2018, Kever Yang wrote: >> >>> Most of Rockchip SoCs have ARM arch/generic timer whose clock source >>> is from one of secure timer(if the soc supports Trust environment). >>> >>> STIMER can only access in secure mode, so it should be init before >>> the proper U-Boot(usually in non-secure mode). >>> Add a MACRO for timer base addr so that we can init with a common >>> function in SPL/TPL. >>> >>> Signed-off-by: Kever Yang <kever.y...@rock-chips.com> >> >> See below for required changes. >> >>> --- >>> >>> arch/arm/mach-rockchip/Kconfig | 16 ++++++++++++++++ >>> 1 file changed, 16 insertions(+) >>> >>> diff --git a/arch/arm/mach-rockchip/Kconfig >>> b/arch/arm/mach-rockchip/Kconfig >>> index 0adaed4..55d3d5c 100644 >>> --- a/arch/arm/mach-rockchip/Kconfig >>> +++ b/arch/arm/mach-rockchip/Kconfig >>> @@ -191,6 +191,22 @@ config ROCKCHIP_BOOT_MODE_REG >>> The Soc will enter to different boot mode(defined in >>> asm/arch/boot_mode.h) >>> according to the value from this register. >>> >>> +config ROCKCHIP_STIMER_BASE >>> + hex "Rockchip Secure timer base address" >>> + default 0x200440a0 if ROCKCHIP_RK3036 >>> + default 0x20018020 if ROCKCHIP_RK3126 >>> + default 0x200440a0 if ROCKCHIP_RK3128 >>> + default 0x110d0020 if ROCKCHIP_RK322X >>> + default 0xff810020 if ROCKCHIP_RK3288 >>> + default 0xff1d0020 if ROCKCHIP_RK3328 >>> + default 0xff830020 if ROCKCHIP_RK3368 >>> + default 0xff8680a0 if ROCKCHIP_RK3399 >>> + default 0x10350020 if ROCKCHIP_RV1108 >>> + default 0 >>> + help >>> + The secure timer inited in SPL/TPL in secure word, ARM generic >>> timer >>> + works after this timer work. >> >> NAK. >> This is not a user-configurable/selectable option, but rather a >> function of the chip used. >> This belongs into a header file in arch/arm/include/asm/arch-rockchip. > > Yes, you are correct in one way, but I think if this goes to header > file, it will separate in different > header file for different SoCs, or with a lot if MACRO like "#if > defined(CONFIG_ROCKCHIP_RK3288) #elif" > in one header file. I don’t care whether we make this different files (my preferred choice in the long run … i.e. after we clean up the header-file directory) or put it into single file: at the moment both variants are common in our asm/arch-rockchip directory … > Make ROCKCHIP_STIMER_BASE in Kconfig and use like ROCKCHIP_BOOT_MODE_REG > seems like a better idea to make things simple. > > Actually there are many other configs like this: > - DEBUG_UART_BASE This is an excellent example and really needs to go into Kconfig, as the UART selection is done via this (e.g. we use a different DEBUG_UART on the RK3399-Q7 than on Rockchip’s reference design) and it’s not something the chip designs but rather a selection from a list encoded as an address. > - IRAM_START > - DRAM_START > > One more consideration is, I don't think make SoC configs into too much > place is a good idea, > now we have 3 places for configs: > - Kconfig default value > - configs/soc_defconfig > - include/configs/soc_header.h (this is moving to Kconfig one by one now) > > so I would not like to move it into arch/arm/include/asm/arch-rockchip, > if you insist this should go to header file instead of Kconfig, I would > prefer > to use header file in 'include/configs/' folder, how do you think? All of this should eventually go into asm/arch-rockchip, unless it’s a an board-specific overrride/configuration item (include/configs was historically meant for boards and not for SOCs). I believe that include/configs/ will eventually be almost completely replaced by Kconfig entries and that any SOC-related things will need to go into asm/arch-rockchip. If you could put it into asm/arch-rockchip, I’d prefer this. However, if you want to have it in include/configs, I won’t delay the series because of it and we’ll deal with the clean-up when the time comes. Thanks, Philipp. > > Thanks, > - Kever >> >>> + >>> config ROCKCHIP_SPL_RESERVE_IRAM >>> hex "Size of IRAM reserved in SPL" >>> default 0 _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
