| Message ID | 1524046740-11233-2-git-send-email-kever.yang@rock-chips.com |
|---|---|
| State | Changes Requested |
| Delegated to: | Philipp Tomsich |
| Headers | show |
| Series | rockchip: conver to use ARM arch timer for armv7 SoCs | expand |
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.yang@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. > + > config ROCKCHIP_SPL_RESERVE_IRAM > hex "Size of IRAM reserved in SPL" > default 0 >
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.yang@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. 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 - 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? Thanks, - Kever > >> + >> config ROCKCHIP_SPL_RESERVE_IRAM >> hex "Size of IRAM reserved in SPL" >> default 0 >> >
Hi Philipp, ping... Could you reply this first before I send next patch set? Thanks, - Kever On 09/03/2018 11:21 AM, Kever Yang 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.yang@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. > > 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 > - 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? > > 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
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.yang@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.yang@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
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. + config ROCKCHIP_SPL_RESERVE_IRAM hex "Size of IRAM reserved in SPL" default 0
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.yang@rock-chips.com> --- arch/arm/mach-rockchip/Kconfig | 16 ++++++++++++++++ 1 file changed, 16 insertions(+)