On Tue, 27 Mar 2018, Kever Yang wrote:

The configure_l2ctlr() is used only by rk3288, do not need to locate in sys_proto.h

Please elaborate on what the function does and why it is not needed by any of the other SOCs (after all: it has been available to all SOCs so far).

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

This should be a standalone patch and doesn't need to be part of the series it is in. This series has way too many different things happening at once and needs to be broken up into individual series that do one well-defined thing each.

See below for requested changes.

---

arch/arm/include/asm/arch-rockchip/sys_proto.h | 22 ---------------------- arch/arm/mach-rockchip/rk3288/rk3288.c | 26 +++++++++++++++++++++++++- 2 files changed, 25 insertions(+), 23 deletions(-)

diff --git a/arch/arm/include/asm/arch-rockchip/sys_proto.h b/arch/arm/include/asm/arch-rockchip/sys_proto.h index e428d59..3617ac2 100644 --- a/arch/arm/include/asm/arch-rockchip/sys_proto.h +++ b/arch/arm/include/asm/arch-rockchip/sys_proto.h @@ -7,27 +7,5 @@ #ifndef _ASM_ARCH_SYS_PROTO_H #define _ASM_ARCH_SYS_PROTO_H

-#ifdef CONFIG_ROCKCHIP_RK3288 -#include <asm/armv7.h>

-static void configure_l2ctlr(void) -{

• uint32_t l2ctlr;
• l2ctlr = read_l2ctlr();
• l2ctlr &= 0xfffc0000; /* clear bit0~bit17 */
• /*
• • Data RAM write latency: 2 cycles
• • Data RAM read latency: 2 cycles
• • Data RAM setup latency: 1 cycle
• • Tag RAM write latency: 1 cycle
• • Tag RAM read latency: 1 cycle
• • Tag RAM setup latency: 1 cycle
• */
• l2ctlr |= (1 << 3 | 1 << 0);
• write_l2ctlr(l2ctlr);

-} -#endif /* CONFIG_ROCKCHIP_RK3288 */

#endif /* _ASM_ARCH_SYS_PROTO_H */ diff --git a/arch/arm/mach-rockchip/rk3288/rk3288.c b/arch/arm/mach-rockchip/rk3288/rk3288.c index acc3b79..1e1c6be 100644 --- a/arch/arm/mach-rockchip/rk3288/rk3288.c +++ b/arch/arm/mach-rockchip/rk3288/rk3288.c @@ -3,15 +3,39 @@

• SPDX-License-Identifier: GPL-2.0+

*/ +#include <asm/armv7.h> #include <asm/io.h> #include <asm/arch/hardware.h>

#define GRF_SOC_CON2 0xff77024c

Please make this a const-declaration in the function it is needed in.

+#ifdef CONFIG_SPL_BUILD

Should this really happen both for TPL and SPL?

+static void configure_l2ctlr(void) +{

• u32 l2ctlr;
• l2ctlr = read_l2ctlr();
• l2ctlr &= 0xfffc0000; /* clear bit0~bit17 */

What are bits 0...17?

• /*

• * Data RAM write latency: 2 cycles
  

• * Data RAM read latency: 2 cycles
  

• * Data RAM setup latency: 1 cycle
  

• * Tag RAM write latency: 1 cycle
  

• * Tag RAM read latency: 1 cycle
  

• * Tag RAM setup latency: 1 cycle
  

• */
  

Please add a symbolic way to assemble these (i.e. something that makes it easy for the casual reader to see what values you are writing to which bitfields).

• l2ctlr |= (1 << 3 | 1 << 0);

From the "clear bit0 ~ bit17" and this, I assume you actually want to do a

clrsetbits_le32...

• write_l2ctlr(l2ctlr);

+} +#endif

int arch_cpu_init(void) { /* We do some SoC one time setting here. */

+#ifdef CONFIG_SPL_BUILD

• configure_l2ctlr();

+#else /* Use rkpwm by default */ rk_setreg(GRF_SOC_CON2, 1 << 0);

Please use a symbolic way to write the (1 << 0), wo it is easy for the casual reader to see what gets enabled/disabled here.

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@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'?

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

On Tue, 27 Mar 2018, Kever Yang wrote:

All rockchip SoCs can use ns16550 driver, enable it for all and set SYS_NS16550_MEM32 for all SoCs.

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

Reviewed-by: Philipp Tomsich philipp.tomsich@theobroma-systems.com

See below for a question.

Note that I believe that the NS16550 changes should go into a separate series and be split off this huge series (which honestly is trying to do too many things at once).

arch/arm/Kconfig | 1 + arch/arm/mach-rockchip/Kconfig | 1 - include/configs/rk3036_common.h | 3 --- include/configs/rk3128_common.h | 2 -- include/configs/rk3188_common.h | 2 -- include/configs/rk322x_common.h | 1 - include/configs/rk3288_common.h | 2 -- include/configs/rk3328_common.h | 2 -- include/configs/rk3368_common.h | 2 -- include/configs/rk3399_common.h | 2 -- include/configs/rockchip-common.h | 2 ++ include/configs/rv1108_common.h | 3 --- 12 files changed, 3 insertions(+), 20 deletions(-)

diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig index 2c52ff0..3e9a80d 100644 --- a/arch/arm/Kconfig +++ b/arch/arm/Kconfig @@ -1167,6 +1167,7 @@ config ARCH_ROCKCHIP select DM_PWM select DM_REGULATOR select ENABLE_ARM_SOC_BOOT0_HOOK

• select SYS_NS16550

Just wondering: shouldn't this be an 'imply'?

imply CMD_FASTBOOT imply FASTBOOT imply FAT_WRITE diff --git a/arch/arm/mach-rockchip/Kconfig b/arch/arm/mach-rockchip/Kconfig index 0adaed4..007cb22 100644 --- a/arch/arm/mach-rockchip/Kconfig +++ b/arch/arm/mach-rockchip/Kconfig @@ -103,7 +103,6 @@ config ROCKCHIP_RK3368 imply SPL_SERIAL_SUPPORT imply TPL_SERIAL_SUPPORT select DEBUG_UART_BOARD_INIT

• select SYS_NS16550 help The Rockchip RK3368 is a ARM-based SoC with a octa-core (organised into a big and little cluster with 4 cores each) Cortex-A53 including

diff --git a/include/configs/rk3036_common.h b/include/configs/rk3036_common.h index f39a272..c5ec864 100644 --- a/include/configs/rk3036_common.h +++ b/include/configs/rk3036_common.h @@ -18,9 +18,6 @@ #define CONFIG_SYS_TIMER_BASE 0x200440a0 /* TIMER5 */ #define CONFIG_SYS_TIMER_COUNTER (CONFIG_SYS_TIMER_BASE + 8)

-#define CONFIG_SYS_NS16550 -#define CONFIG_SYS_NS16550_MEM32

#define CONFIG_SYS_INIT_SP_ADDR 0x60100000 #define CONFIG_SYS_LOAD_ADDR 0x60800800 #define CONFIG_SPL_STACK 0x10081fff diff --git a/include/configs/rk3128_common.h b/include/configs/rk3128_common.h index bd8019c..c593f18 100644 --- a/include/configs/rk3128_common.h +++ b/include/configs/rk3128_common.h @@ -19,8 +19,6 @@ #define CONFIG_SYS_TIMER_BASE 0x200440a0 /* TIMER5 */ #define CONFIG_SYS_TIMER_COUNTER (CONFIG_SYS_TIMER_BASE + 8)

-#define CONFIG_SYS_NS16550_MEM32

#define CONFIG_SYS_INIT_SP_ADDR 0x60100000 #define CONFIG_SYS_LOAD_ADDR 0x60800800

diff --git a/include/configs/rk3188_common.h b/include/configs/rk3188_common.h index 94f8cda..30c150e 100644 --- a/include/configs/rk3188_common.h +++ b/include/configs/rk3188_common.h @@ -22,8 +22,6 @@ #define CONFIG_SYS_TIMER_COUNTER (CONFIG_SYS_TIMER_BASE + 8) #define CONFIG_SYS_TIMER_COUNTS_DOWN

-#define CONFIG_SYS_NS16550_MEM32

#ifdef CONFIG_SPL_ROCKCHIP_BACK_TO_BROM /* Bootrom will load u-boot binary to 0x60000000 once return from SPL */ #endif diff --git a/include/configs/rk322x_common.h b/include/configs/rk322x_common.h index 7f9c7fb..832f037 100644 --- a/include/configs/rk322x_common.h +++ b/include/configs/rk322x_common.h @@ -18,7 +18,6 @@ #define CONFIG_SYS_TIMER_BASE 0x110c00a0 /* TIMER5 */ #define CONFIG_SYS_TIMER_COUNTER (CONFIG_SYS_TIMER_BASE + 8)

-#define CONFIG_SYS_NS16550_MEM32 #define CONFIG_SYS_INIT_SP_ADDR 0x60100000 #define CONFIG_SYS_LOAD_ADDR 0x60800800 #define CONFIG_SPL_STACK 0x10088000 diff --git a/include/configs/rk3288_common.h b/include/configs/rk3288_common.h index 78595b8..44d5c2a 100644 --- a/include/configs/rk3288_common.h +++ b/include/configs/rk3288_common.h @@ -19,8 +19,6 @@ #define CONFIG_SYS_TIMER_BASE 0xff810020 /* TIMER7 */ #define CONFIG_SYS_TIMER_COUNTER (CONFIG_SYS_TIMER_BASE + 8)

-#define CONFIG_SYS_NS16550_MEM32

#ifdef CONFIG_SPL_ROCKCHIP_BACK_TO_BROM /* Bootrom will load u-boot binary to 0x0 once return from SPL */ #endif diff --git a/include/configs/rk3328_common.h b/include/configs/rk3328_common.h index 7018668..ce650c8 100644 --- a/include/configs/rk3328_common.h +++ b/include/configs/rk3328_common.h @@ -13,8 +13,6 @@ #define CONFIG_SYS_CBSIZE 1024 #define CONFIG_SKIP_LOWLEVEL_INIT

-#define CONFIG_SYS_NS16550_MEM32

#define CONFIG_SYS_INIT_SP_ADDR 0x00300000 #define CONFIG_SYS_LOAD_ADDR 0x00800800

diff --git a/include/configs/rk3368_common.h b/include/configs/rk3368_common.h index a7fe4ca..866383d 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 CONFIG_SYS_NS16550_MEM32

#define CONFIG_SYS_INIT_SP_ADDR 0x00300000 #define CONFIG_SYS_LOAD_ADDR 0x00280000

diff --git a/include/configs/rk3399_common.h b/include/configs/rk3399_common.h index fe8c675..53691e5 100644 --- a/include/configs/rk3399_common.h +++ b/include/configs/rk3399_common.h @@ -17,8 +17,6 @@ #define CONFIG_SPL_SPI_LOAD #endif

-#define CONFIG_SYS_NS16550_MEM32

#define CONFIG_SYS_INIT_SP_ADDR 0x00300000 #define CONFIG_SYS_LOAD_ADDR 0x00800800 #define CONFIG_SPL_STACK 0xff8effff diff --git a/include/configs/rockchip-common.h b/include/configs/rockchip-common.h index 24651ce..4062480 100644 --- a/include/configs/rockchip-common.h +++ b/include/configs/rockchip-common.h @@ -12,6 +12,8 @@ #define CONFIG_SYS_ARCH_TIMER #define CONFIG_SYS_HZ_CLOCK 24000000

+#define CONFIG_SYS_NS16550_MEM32

#ifndef CONFIG_SPL_BUILD

/* First try to boot from SD (index 0), then eMMC (index 1) */ diff --git a/include/configs/rv1108_common.h b/include/configs/rv1108_common.h index 349c53c..cd204e9 100644 --- a/include/configs/rv1108_common.h +++ b/include/configs/rv1108_common.h @@ -18,9 +18,6 @@ #define CONFIG_SYS_TIMER_BASE 0x10350020 #define CONFIG_SYS_TIMER_COUNTER (CONFIG_SYS_TIMER_BASE + 8)

-#define CONFIG_SYS_NS16550 -#define CONFIG_SYS_NS16550_MEM32

#define CONFIG_SYS_SDRAM_BASE 0x60000000 #define CONFIG_NR_DRAM_BANKS 1 #define CONFIG_SYS_INIT_SP_ADDR (CONFIG_SYS_TEXT_BASE + 0x100000)

On Tue, 27 Mar 2018, Kever Yang wrote:

STIMER is can only access in secure mode if the SoCs supports trust, and it locate in alive power domain, as the source of ARM arch/generic timer, we add a base addr for all SoCs so that we can init with a common function.

The commit message does not really tell what the source changes are (although it seems to describe part of the motivation for this change).

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

See below for requested changes.

---

arch/arm/mach-rockchip/Kconfig | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+)

diff --git a/arch/arm/mach-rockchip/Kconfig b/arch/arm/mach-rockchip/Kconfig index 007cb22..5dfe452 100644 --- a/arch/arm/mach-rockchip/Kconfig +++ b/arch/arm/mach-rockchip/Kconfig @@ -190,6 +190,25 @@ 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 0xff220020 if ROCKCHIP_PX30

We don't have support for the PX30 in the U-Boot code base yet. Until a series to add support for the PX30 comes in, you should not have this here yet (and then add this specific case when the PX30 support is added).

• default 0x200440a0 if ROCKCHIP_RK3036
• default 0x2000e000 if ROCKCHIP_RK3066
• default 0x20018020 if ROCKCHIP_RK3126
• default 0x200440a0 if ROCKCHIP_RK3128
• default 0x2000e000 if ROCKCHIP_RK3188
• 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.
  

This should not be in Kconfig and rather in a header-file. With what you do here, a user (e.g. via 'make menuconfig') or a defconfig file (e.g. due to a typo) could accidentially change overwrite this.

config ROCKCHIP_SPL_RESERVE_IRAM hex "Size of IRAM reserved in SPL" default 0

Hi U-Boot Maintainers:

I would like to close this topic and update all related source code, you can find the previous discussion at [0][1]. Philipp object this patch and his original reason is:

This should not be in Kconfig and rather in a header-file. With what you do here, a user (e.g. via 'make menuconfig') or a defconfig file (e.g. due to a typo) could accidentially change overwrite this.

and Philipp also think:

asm/arch-rockchip would be preferable over include/config as a header

There are many ways for feature configure on rockchip platform now: - board dts; - header file in include/config - Kconfig in $(BOARD)_defconfig - Kconfig default value in 'Kconfig' file Already too many place to config, we should make it simple rather than complex.

I think people are migrating more and more configurations from header file to Kconfig, including: - option for enable/disable modules; - option for module parameter which is numerical value; - option for module parameter which is string; I think the target is that most of the configs goes to Kconfig and user can config options with menuconfig and no need to touch header file(Please tell me if this is not true).

And for all those config options, I think at lease can be separate into two kind: - options per-board; - options per-soc (some of them may per-vendor); I think the idealized model would be per-soc options goes to 'Kconfig' file with default value combine with SOC, and per-board options at $(BOARD)_defconfig.

ROCKCHIP_STIMER_REG(another case is BOOT_MODE_REG) is per-soc config option, just like SYS_TEXT_BASE, TPL_TEXT_BASE, TPL_MAX_SIZE, SYS_SOC and so on. Driver for different SOCs to use this reg are just the same, and this reg needs to be used very early in TPL/SPL which means no DM, no dts available.

Here are the solutions from previous discussion: - dts (by Simon) extra dtb code needed, setting separate into individual board dts; not available and not suggestion to use in TPL/SPL; - move to header in 'asm/arch-rockchip' (by Philipp) one more place to config, make config options more complicate; may separate in more than 10+ files for different soc/board; - RMII setting like, (by Philipp) reference to 'drivers/net/gmac_rockchip.c', driver for different SoCs are not re-usable, driver needs to update for each new SoC support; not available with DM in early TPL/SPL; - Kconfig with default value in this patch Keep driver clean and no need to update with new SoC; One place rather than 10+ individual files, same usage like SYS_TEXT_BASE, reference to: 341c058654 sunxi: move CONFIG_SYS_TEXT_BASE out of defconfigs

I think the default value in Kconfig is the best solution, the SPL/TPL should be small, fast and with enough functionality for use.

Thanks, - Kever [0] https://patchwork.ozlabs.org/patch/1004148/ [1] http://patchwork.ozlabs.org/patch/891462/

Kever Yang kever.yang@rock-chips.com 于2018年3月27日周二 下午5:30写道：

STIMER is can only access in secure mode if the SoCs supports trust, and it locate in alive power domain, as the source of ARM arch/generic timer, we add a base addr for all SoCs so that we can init with a common function.

Signed-off-by: Kever Yang kever.yang@rock-chips.com

arch/arm/mach-rockchip/Kconfig | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+)

diff --git a/arch/arm/mach-rockchip/Kconfig b/arch/arm/mach-rockchip/Kconfig index 007cb22..5dfe452 100644 --- a/arch/arm/mach-rockchip/Kconfig +++ b/arch/arm/mach-rockchip/Kconfig @@ -190,6 +190,25 @@ 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 0xff220020 if ROCKCHIP_PX30
  

•   default 0x200440a0 if ROCKCHIP_RK3036
  

•   default 0x2000e000 if ROCKCHIP_RK3066
  

•   default 0x20018020 if ROCKCHIP_RK3126
  

•   default 0x200440a0 if ROCKCHIP_RK3128
  

•   default 0x2000e000 if ROCKCHIP_RK3188
  

•   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 -- 1.9.1

On 04/02/2018 05:00 AM, Philipp Tomsich wrote:

On Tue, 27 Mar 2018, Kever Yang wrote:

...

We add this for get the location for boot device of bootrom.

Your commit message should be specific enough, so the motivation for the change, where it fits into the overall architecture and what is changed are apparent.

...

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

See below for requested changes.

...

---

arch/arm/mach-rockchip/Kconfig | 17 +++++++++++++++++ 1 file changed, 17 insertions(+)

diff --git a/arch/arm/mach-rockchip/Kconfig b/arch/arm/mach-rockchip/Kconfig index 5dfe452..98bf935 100644 --- a/arch/arm/mach-rockchip/Kconfig +++ b/arch/arm/mach-rockchip/Kconfig @@ -209,6 +209,23 @@ config ROCKCHIP_STIMER_BASE       The secure timer inited in SPL/TPL in secure word, ARM generic timer       works after this timer work.

+config ROCKCHIP_IRAM_START_ADDR +    hex "Rockchip Secure timer base address"

Huh: copy-and-paste?

Sorry, my fault, will fix in next version.

Thanks, - Kever

...

+    default 0xff0e0000 if ROCKCHIP_PX30

See my earlier comment regarding changes specific to the PX30.

...

+    default 0x10080000 if ROCKCHIP_RK3036 +    default 0x10080000 if ROCKCHIP_RK3128 +    default 0x10080000 if ROCKCHIP_RK3188 +    default 0x10080000 if ROCKCHIP_RK322X +    default 0xff700000 if ROCKCHIP_RK3288 +    default 0xff091000 if ROCKCHIP_RK3328 +    default 0xff8c0000 if ROCKCHIP_RK3368 +    default 0xff8c0000 if ROCKCHIP_RK3399 +    default 0x10080000 if ROCKCHIP_RV1108 +    default 0 +    help +      The IRAM start addr is to locate variant of the boot device from +      bootrom.

The same comment as for the secure timer-base applies here. This is way to important to have it in Kconfig, where a user can easily override.

...

config ROCKCHIP_SPL_RESERVE_IRAM     hex "Size of IRAM reserved in SPL"     default 0

On Tue, 27 Mar 2018, Kever Yang wrote:

We can use arch timer instead of rockchip timer.

A slightly more enlightening commit message would be great.

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

These changes for (for the timer) should go into a separate series. Also: you should refactor these patches in such a way that the tree (after every patch that is applied) this builds. A good way to do so is to do one device after another per patch.

---

include/configs/rk322x_common.h | 3 --- 1 file changed, 3 deletions(-)

diff --git a/include/configs/rk322x_common.h b/include/configs/rk322x_common.h index 832f037..29e222a 100644 --- a/include/configs/rk322x_common.h +++ b/include/configs/rk322x_common.h @@ -14,9 +14,6 @@ #define CONFIG_SYS_CBSIZE 1024 #define CONFIG_SYS_BOOTM_LEN (64 << 20) /* 64M */

-#define CONFIG_SYS_TIMER_RATE (24 * 1000 * 1000) -#define CONFIG_SYS_TIMER_BASE 0x110c00a0 /* TIMER5 */ -#define CONFIG_SYS_TIMER_COUNTER (CONFIG_SYS_TIMER_BASE + 8)

#define CONFIG_SYS_INIT_SP_ADDR 0x60100000 #define CONFIG_SYS_LOAD_ADDR 0x60800800

On Tue, 27 Mar 2018, Kever Yang wrote:

Move soc spec setting into rk322x.c and remove rk322x-board/board-spl.c

A clear description of what the patch is attempting to achieve.

Signed-off-by: Kever Yang kever.yang@rock-chips.com

See below for requested changes.

---

arch/arm/mach-rockchip/rk322x-board.c | 155 --------------------- arch/arm/mach-rockchip/rk322x/Makefile | 2 +- .../{rk322x-board-spl.c => rk322x/rk322x.c} | 86 ++++-------- 3 files changed, 29 insertions(+), 214 deletions(-) delete mode 100644 arch/arm/mach-rockchip/rk322x-board.c rename arch/arm/mach-rockchip/{rk322x-board-spl.c => rk322x/rk322x.c} (51%)

diff --git a/arch/arm/mach-rockchip/rk322x-board.c b/arch/arm/mach-rockchip/rk322x-board.c deleted file mode 100644 index 8642a90..0000000 --- a/arch/arm/mach-rockchip/rk322x-board.c +++ /dev/null @@ -1,155 +0,0 @@ -/*

• • (C) Copyright 2017 Rockchip Electronics Co., Ltd.
• • SPDX-License-Identifier: GPL-2.0+
• */

-#include <common.h> -#include <clk.h> -#include <dm.h> -#include <ram.h> -#include <syscon.h> -#include <asm/io.h> -#include <asm/arch/clock.h> -#include <asm/arch/periph.h> -#include <asm/arch/grf_rk322x.h> -#include <asm/arch/boot_mode.h>

-DECLARE_GLOBAL_DATA_PTR;

-__weak int rk_board_late_init(void) -{

• return 0;

-}

-int board_late_init(void) -{

• setup_boot_mode();
• return rk_board_late_init();

-}

-int board_init(void) -{ -#include <asm/arch/grf_rk322x.h>

• /* Enable early UART2 channel 1 on the RK322x */

-#define GRF_BASE 0x11000000

• struct rk322x_grf * const grf = (void *)GRF_BASE;
• enum {

• GPIO1B2_SHIFT		= 4,
  

• GPIO1B2_MASK		= 3 << GPIO1B2_SHIFT,
  

• GPIO1B2_GPIO		= 0,
  

• GPIO1B2_UART21_SIN,
  

• GPIO1B1_SHIFT		= 2,
  

• GPIO1B1_MASK		= 3 << GPIO1B1_SHIFT,
  

• GPIO1B1_GPIO            = 0,
  

• GPIO1B1_UART1_SOUT,
  

• GPIO1B1_UART21_SOUT,
  

• };
• enum {

• CON_IOMUX_UART2SEL_SHIFT= 8,
  

• CON_IOMUX_UART2SEL_MASK	= 1 << CON_IOMUX_UART2SEL_SHIFT,
  

• CON_IOMUX_UART2SEL_2	= 0,
  

• CON_IOMUX_UART2SEL_21,
  

• };
• rk_clrsetreg(&grf->gpio1b_iomux,

•      GPIO1B1_MASK | GPIO1B2_MASK,
  

•      GPIO1B2_UART21_SIN << GPIO1B2_SHIFT |
  

•      GPIO1B1_UART21_SOUT << GPIO1B1_SHIFT);
  

• /* Set channel C as UART2 input */
• rk_clrsetreg(&grf->con_iomux,

•      CON_IOMUX_UART2SEL_MASK,
  

•      CON_IOMUX_UART2SEL_21 << CON_IOMUX_UART2SEL_SHIFT);
  

• /*
• • The integrated macphy is enabled by default, disable it
• • for saving power consuming.
• */
• rk_clrsetreg(&grf->macphy_con[0],

•      MACPHY_CFG_ENABLE_MASK,
  

•      0 << MACPHY_CFG_ENABLE_SHIFT);
  

• return 0;

-}

-int dram_init_banksize(void) -{

• gd->bd->bi_dram[0].start = CONFIG_SYS_SDRAM_BASE;
• gd->bd->bi_dram[0].size = 0x8400000;
• /* Reserve 0x200000 for OPTEE */
• gd->bd->bi_dram[1].start = CONFIG_SYS_SDRAM_BASE

• 		+ gd->bd->bi_dram[0].size + 0x200000;
  

• gd->bd->bi_dram[1].size = gd->bd->bi_dram[0].start

• 		+ gd->ram_size - gd->bd->bi_dram[1].start;
  

• return 0;

-}

-#ifndef CONFIG_SYS_DCACHE_OFF -void enable_caches(void) -{

• /* Enable D-cache. I-cache is already enabled in start.S */
• dcache_enable();

-} -#endif

-#if defined(CONFIG_USB_GADGET) && defined(CONFIG_USB_GADGET_DWC2_OTG) -#include <usb.h> -#include <usb/dwc2_udc.h>

-static struct dwc2_plat_otg_data rk322x_otg_data = {

• .rx_fifo_sz = 512,
• .np_tx_fifo_sz = 16,
• .tx_fifo_sz = 128,

-};

-int board_usb_init(int index, enum usb_init_type init) -{

• int node;
• const char *mode;
• bool matched = false;
• const void *blob = gd->fdt_blob;
• /* find the usb_otg node */
• node = fdt_node_offset_by_compatible(blob, -1,

• 			"rockchip,rk3288-usb");
  

• while (node > 0) {

• mode = fdt_getprop(blob, node, "dr_mode", NULL);
  

• if (mode && strcmp(mode, "otg") == 0) {
  

• 	matched = true;
  

• 	break;
  

• }
  

• node = fdt_node_offset_by_compatible(blob, node,
  

• 			"rockchip,rk3288-usb");
  

• }
• if (!matched) {

• debug("Not found usb_otg device\n");
  

• return -ENODEV;
  

• }
• rk322x_otg_data.regs_otg = fdtdec_get_addr(blob, node, "reg");
• return dwc2_udc_probe(&rk322x_otg_data);

-}

-int board_usb_cleanup(int index, enum usb_init_type init) -{

• return 0;

-} -#endif

-#if defined(CONFIG_USB_FUNCTION_FASTBOOT) -int fb_set_reboot_flag(void) -{

• struct rk322x_grf *grf;
• printf("Setting reboot to fastboot flag ...\n");
• grf = syscon_get_first_range(ROCKCHIP_SYSCON_GRF);
• /* Set boot mode to fastboot */
• writel(BOOT_FASTBOOT, &grf->os_reg[0]);
• return 0;

-} -#endif diff --git a/arch/arm/mach-rockchip/rk322x/Makefile b/arch/arm/mach-rockchip/rk322x/Makefile index ecb3e8d..89b0fed 100644 --- a/arch/arm/mach-rockchip/rk322x/Makefile +++ b/arch/arm/mach-rockchip/rk322x/Makefile @@ -4,6 +4,6 @@ # SPDX-License-Identifier: GPL-2.0+ #

obj-y += clk_rk322x.o +obj-y += rk322x.o obj-y += syscon_rk322x.o diff --git a/arch/arm/mach-rockchip/rk322x-board-spl.c b/arch/arm/mach-rockchip/rk322x/rk322x.c similarity index 51% rename from arch/arm/mach-rockchip/rk322x-board-spl.c rename to arch/arm/mach-rockchip/rk322x/rk322x.c index 206abfa..98b6ec3 100644 --- a/arch/arm/mach-rockchip/rk322x-board-spl.c +++ b/arch/arm/mach-rockchip/rk322x/rk322x.c @@ -1,32 +1,41 @@ /*

• • (C) Copyright 2017 Rockchip Electronics Co., Ltd

• • Copyright (c) 2017 Rockchip Electronics Co., Ltd

• SPDX-License-Identifier: GPL-2.0+

*/

-#include <common.h> -#include <debug_uart.h> -#include <dm.h> -#include <ram.h> -#include <spl.h> #include <asm/io.h> #include <asm/arch/bootrom.h> -#include <asm/arch/cru_rk322x.h> -#include <asm/arch/grf_rk322x.h> #include <asm/arch/hardware.h> -#include <asm/arch/timer.h> -#include <asm/arch/uart.h> +#include <asm/arch/grf_rk322x.h>

-u32 spl_boot_device(void) +#define GRF_BASE 0x11000000 +#define CRU_MISC_CON 0x110e0134 +#define SGRF_DDR_CON0 0x10150000

Please use 'const' and declare within functions, if used only by a single function.

+const char * const boot_devices[BROM_LAST_BOOTSOURCE + 1] = {

• [BROM_BOOTSOURCE_EMMC] = "/sdhci@fe330000",
• [BROM_BOOTSOURCE_SD] = "/dwmmc@fe320000",

+};

+#ifdef CONFIG_SPL_BUILD +int arch_cpu_init(void) {

• return BOOT_DEVICE_MMC1;

-} -DECLARE_GLOBAL_DATA_PTR;

• static struct rk322x_grf * const grf = (void *)GRF_BASE;
• /* We do some SoC one time setting here. */

-#define GRF_BASE 0x11000000 -#define SGRF_BASE 0x10140000

• /* Disable the ddr secure region setting to make it non-secure */
• rk_clrreg(SGRF_DDR_CON0, 0x4000);
• /*

• * The integrated macphy is enabled by default, disable it
  

• * for saving power consuming.
  

• */
  

• rk_clrsetreg(&grf->macphy_con[0], MACPHY_CFG_ENABLE_MASK,

•      0 << MACPHY_CFG_ENABLE_SHIFT);
  
  

-#define DEBUG_UART_BASE 0x11030000

• return 0;

+} +#endif

void board_debug_uart_init(void) { @@ -34,8 +43,7 @@ void board_debug_uart_init(void) enum { GPIO1B2_SHIFT = 4, GPIO1B2_MASK = 3 << GPIO1B2_SHIFT,

• GPIO1B2_GPIO            = 0,
  

• GPIO1B2_UART1_SIN,
  

• GPIO1B2_GPIO		= 0,
  

  GPIO1B2_UART21_SIN,

This looks odd: are you shure this is 'GPIO1B2_UART21_SIN'?

GPIO1B1_SHIFT		= 2,

@@ -61,41 +69,3 @@ void board_debug_uart_init(void) CON_IOMUX_UART2SEL_MASK, CON_IOMUX_UART2SEL_21 << CON_IOMUX_UART2SEL_SHIFT); }

-#define SGRF_DDR_CON0 0x10150000 -void board_init_f(ulong dummy) -{

• struct udevice *dev;
• int ret;
• /*

• * Debug UART can be used from here if required:
  

• *
  

• * debug_uart_init();
  

• * printch('a');
  

• * printhex8(0x1234);
  

• * printascii("string");
  

• */
  

• debug_uart_init();
• printascii("SPL Init");
• ret = spl_early_init();
• if (ret) {

• debug("spl_early_init() failed: %d\n", ret);
  

• hang();
  

• }
• rockchip_timer_init();
• printf("timer init done\n");
• ret = uclass_get_device(UCLASS_RAM, 0, &dev);
• if (ret) {

• printf("DRAM init failed: %d\n", ret);
  

• return;
  

• }
• /* Disable the ddr secure region setting to make it non-secure */
• rk_clrreg(SGRF_DDR_CON0, 0x4000);

-#if defined(CONFIG_ROCKCHIP_SPL_BACK_TO_BROM) && !defined(CONFIG_SPL_BOARD_INIT)

• back_to_bootrom(BROM_BOOT_NEXTSTAGE);

-#endif -}

On Tue, 27 Mar 2018, Kever Yang wrote:

We are going to remove rockchip_udelay after enable arch timer.

Please use commit message that better describes what this commit is changing.

Signed-off-by: Kever Yang kever.yang@rock-chips.com

arch/arm/mach-rockchip/rk3036/sdram_rk3036.c | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-)

diff --git a/arch/arm/mach-rockchip/rk3036/sdram_rk3036.c b/arch/arm/mach-rockchip/rk3036/sdram_rk3036.c index e5393ec..ce3f09a 100644 --- a/arch/arm/mach-rockchip/rk3036/sdram_rk3036.c +++ b/arch/arm/mach-rockchip/rk3036/sdram_rk3036.c @@ -10,7 +10,6 @@ #include <asm/arch/grf_rk3036.h> #include <asm/arch/hardware.h> #include <asm/arch/sdram_rk3036.h> -#include <asm/arch/timer.h> #include <asm/arch/uart.h>

/* @@ -346,7 +345,7 @@ static void rkdclk_init(struct rk3036_sdram_priv *priv)

/* waiting for pll lock */ while (readl(&pll->con1) & (1 << PLL_LOCK_STATUS_SHIFT))

• rockchip_udelay(1);
  

• udelay(1);
  
  

  /* PLL enter normal-mode */ rk_clrsetreg(&priv->cru->cru_mode_con, DPLL_MODE_MASK,

@@ -374,25 +373,25 @@ void phy_pctrl_reset(struct rk3036_sdram_priv *priv) 1 << DDRCTRL_PSRST_SHIFT | 1 << DDRCTRL_SRST_SHIFT | 1 << DDRPHY_PSRST_SHIFT | 1 << DDRPHY_SRST_SHIFT);

• rockchip_udelay(10);

• udelay(10);

  rk_clrreg(&priv->cru->cru_softrst_con[5], 1 << DDRPHY_PSRST_SHIFT | 1 << DDRPHY_SRST_SHIFT);

• rockchip_udelay(10);

• udelay(10);

  rk_clrreg(&priv->cru->cru_softrst_con[5], 1 << DDRCTRL_PSRST_SHIFT | 1 << DDRCTRL_SRST_SHIFT);

• rockchip_udelay(10);

• udelay(10);

  clrsetbits_le32(&ddr_phy->ddrphy_reg1, SOFT_RESET_MASK << SOFT_RESET_SHIFT, 0 << SOFT_RESET_SHIFT);

• rockchip_udelay(10);

• udelay(10); clrsetbits_le32(&ddr_phy->ddrphy_reg1, SOFT_RESET_MASK << SOFT_RESET_SHIFT, 3 << SOFT_RESET_SHIFT);

• rockchip_udelay(1);

• udelay(1);

}

void phy_dll_bypass_set(struct rk3036_sdram_priv *priv, unsigned int freq) @@ -445,7 +444,7 @@ static void send_command(struct rk3036_ddr_pctl *pctl, u32 rank, u32 cmd, u32 arg) { writel((START_CMD | (rank << 20) | arg | cmd), &pctl->mcmd);

• rockchip_udelay(1);

• udelay(1); while (readl(&pctl->mcmd) & START_CMD) ;

} @@ -455,7 +454,7 @@ static void memory_init(struct rk3036_sdram_priv *priv) struct rk3036_ddr_pctl *pctl = priv->pctl;

send_command(pctl, 3, DESELECT_CMD, 0);

• rockchip_udelay(1);

• udelay(1); send_command(pctl, 3, PREA_CMD, 0); send_command(pctl, 3, MRS_CMD, (0x02 & BANK_ADDR_MASK) << BANK_ADDR_SHIFT |

@@ -493,7 +492,7 @@ static void data_training(struct rk3036_sdram_priv *priv) clrsetbits_le32(&ddr_phy->ddrphy_reg2, 0x03, DQS_SQU_CAL_NORMAL_MODE | DQS_SQU_CAL_START);

• rockchip_udelay(1);

• udelay(1); while ((readl(&ddr_phy->ddrphy_reg62) & CAL_DONE_MASK) != (HIGH_8BIT_CAL_DONE | LOW_8BIT_CAL_DONE)) { ;

On Tue, 27 Mar 2018, Kever Yang wrote:

Move some soc spec setting into rk3036.c

Please provide a summary of the changes in your commit message.

Signed-off-by: Kever Yang kever.yang@rock-chips.com

arch/arm/mach-rockchip/rk3036-board.c | 104 --------------------- arch/arm/mach-rockchip/rk3036/Makefile | 2 +- .../{rk3036-board-spl.c => rk3036/rk3036.c} | 41 ++------ arch/arm/mach-rockchip/rk3036/sdram_rk3036.c | 14 +++ 4 files changed, 24 insertions(+), 137 deletions(-) delete mode 100644 arch/arm/mach-rockchip/rk3036-board.c rename arch/arm/mach-rockchip/{rk3036-board-spl.c => rk3036/rk3036.c} (50%)

diff --git a/arch/arm/mach-rockchip/rk3036-board.c b/arch/arm/mach-rockchip/rk3036-board.c deleted file mode 100644 index a5d2571..0000000 --- a/arch/arm/mach-rockchip/rk3036-board.c +++ /dev/null @@ -1,104 +0,0 @@ -/*

• • (C) Copyright 2015 Rockchip Electronics Co., Ltd
• • SPDX-License-Identifier: GPL-2.0+
• */

-#include <common.h> -#include <clk.h> -#include <dm.h> -#include <ram.h> -#include <asm/io.h> -#include <asm/arch/clock.h> -#include <asm/arch/periph.h> -#include <asm/arch/grf_rk3036.h> -#include <asm/arch/boot_mode.h> -#include <asm/arch/sdram_rk3036.h> -#include <asm/gpio.h> -#include <dm/pinctrl.h>

-DECLARE_GLOBAL_DATA_PTR;

-__weak int rk_board_late_init(void) -{

• return 0;

-}

-int board_late_init(void) -{

• setup_boot_mode();
• return rk_board_late_init();

-}

-int board_init(void) -{

• return 0;

-}

-#if !CONFIG_IS_ENABLED(RAM) -/*

• • When CONFIG_RAM is enabled, the dram_init() function is implemented
• • in sdram_common.c.
• */

-int dram_init(void) -{

• gd->ram_size = sdram_size();
• return 0;

-} -#endif

-#ifndef CONFIG_SYS_DCACHE_OFF -void enable_caches(void) -{

• /* Enable D-cache. I-cache is already enabled in start.S */
• dcache_enable();

-} -#endif

-#if defined(CONFIG_USB_GADGET) && defined(CONFIG_USB_GADGET_DWC2_OTG) -#include <usb.h> -#include <usb/dwc2_udc.h>

-static struct dwc2_plat_otg_data rk3036_otg_data = {

• .rx_fifo_sz = 512,
• .np_tx_fifo_sz = 16,
• .tx_fifo_sz = 128,

-};

-int board_usb_init(int index, enum usb_init_type init) -{

• int node;
• const char *mode;
• bool matched = false;
• const void *blob = gd->fdt_blob;
• /* find the usb_otg node */
• node = fdt_node_offset_by_compatible(blob, -1,

• 			"rockchip,rk3288-usb");
  

• while (node > 0) {

• mode = fdt_getprop(blob, node, "dr_mode", NULL);
  

• if (mode && strcmp(mode, "otg") == 0) {
  

• 	matched = true;
  

• 	break;
  

• }
  

• node = fdt_node_offset_by_compatible(blob, node,
  

• 			"rockchip,rk3288-usb");
  

• }
• if (!matched) {

• debug("Not found usb_otg device\n");
  

• return -ENODEV;
  

• }
• rk3036_otg_data.regs_otg = fdtdec_get_addr(blob, node, "reg");
• return dwc2_udc_probe(&rk3036_otg_data);

-}

-int board_usb_cleanup(int index, enum usb_init_type init) -{

• return 0;

-} -#endif diff --git a/arch/arm/mach-rockchip/rk3036/Makefile b/arch/arm/mach-rockchip/rk3036/Makefile index 20d28f7..8a144d1 100644 --- a/arch/arm/mach-rockchip/rk3036/Makefile +++ b/arch/arm/mach-rockchip/rk3036/Makefile @@ -4,7 +4,7 @@ # SPDX-License-Identifier: GPL-2.0+ #

-obj-y += clk_rk3036.o +obj-y += rk3036.o clk_rk3036.o

ifndef CONFIG_SPL_BUILD obj-y += syscon_rk3036.o diff --git a/arch/arm/mach-rockchip/rk3036-board-spl.c b/arch/arm/mach-rockchip/rk3036/rk3036.c similarity index 50% rename from arch/arm/mach-rockchip/rk3036-board-spl.c rename to arch/arm/mach-rockchip/rk3036/rk3036.c index 550e3a1..39cc43b 100644 --- a/arch/arm/mach-rockchip/rk3036-board-spl.c +++ b/arch/arm/mach-rockchip/rk3036/rk3036.c @@ -1,28 +1,22 @@ /*

• • (C) Copyright 2015 Rockchip Electronics Co., Ltd

• • Copyright (c) 2017 Rockchip Electronics Co., Ltd

• SPDX-License-Identifier: GPL-2.0+

*/

-#include <common.h> -#include <debug_uart.h> #include <asm/io.h> #include <asm/arch/bootrom.h> -#include <asm/arch/grf_rk3036.h> #include <asm/arch/hardware.h> -#include <asm/arch/sdram_rk3036.h> -#include <asm/arch/timer.h> -#include <asm/arch/uart.h>

-DECLARE_GLOBAL_DATA_PTR;

+#include <asm/arch/grf_rk3036.h> #define GRF_BASE 0x20008000

-#define DEBUG_UART_BASE 0x20068000

-void board_init_f(ulong dummy) +#ifdef CONFIG_SPL_BUILD +int arch_cpu_init(void) +{

• return 0;

+} +#endif +void board_debug_uart_init(void) { -#ifdef EARLY_DEBUG struct rk3036_grf * const grf = (void *)GRF_BASE; /* * NOTE: sd card and debug uart use same iomux in rk3036, @@ -34,22 +28,5 @@ void board_init_f(ulong dummy) GPIO1C2_MASK << GPIO1C2_SHIFT, GPIO1C3_UART2_SOUT << GPIO1C3_SHIFT | GPIO1C2_UART2_SIN << GPIO1C2_SHIFT);

• debug_uart_init();

-#endif

• rockchip_timer_init();
• sdram_init();
• /* return to maskrom */
• back_to_bootrom(BROM_BOOT_NEXTSTAGE);

-}

-/* Place Holders */ -void board_init_r(gd_t *id, ulong dest_addr) -{

• /*

• * Function attribute is no-return
  

• * This Function never executes
  

• */
  

• while (1)

• ;
  

} diff --git a/arch/arm/mach-rockchip/rk3036/sdram_rk3036.c b/arch/arm/mach-rockchip/rk3036/sdram_rk3036.c index ce3f09a..a4fb3ae 100644 --- a/arch/arm/mach-rockchip/rk3036/sdram_rk3036.c +++ b/arch/arm/mach-rockchip/rk3036/sdram_rk3036.c @@ -11,6 +11,7 @@ #include <asm/arch/hardware.h> #include <asm/arch/sdram_rk3036.h> #include <asm/arch/uart.h> +DECLARE_GLOBAL_DATA_PTR;

/*

• we can not fit the code to access the device tree in SPL

@@ -764,3 +765,16 @@ void sdram_init(void) move_to_access_state(&sdram_priv); dram_cfg_rbc(&sdram_priv); }

+#if !CONFIG_IS_ENABLED(RAM) +/*

• • When CONFIG_RAM is enabled, the dram_init() function is implemented
• • in sdram_common.c.
• */

+int dram_init(void) +{

• gd->ram_size = sdram_size();
• return 0;

+} +#endif

On Tue, 27 Mar 2018, Kever Yang wrote:

Trust OS decode this info like this: https://github.com/ARM-software/arm-trusted-firmware/blob/master/plat/rockch... We have to set a available value, or else we get error info from Trust OS like this: "ERROR: over or zero region, nr=3145987, max=10"

Please describe what changes the patch makes.

Signed-off-by: Kever Yang kever.yang@rock-chips.com

See below for requested changes.

---

arch/arm/include/asm/arch-rockchip/sdram_common.h | 4 ++++ arch/arm/mach-rockchip/sdram_common.c | 21 +++++++++++++++++++++ 2 files changed, 25 insertions(+)

diff --git a/arch/arm/include/asm/arch-rockchip/sdram_common.h b/arch/arm/include/asm/arch-rockchip/sdram_common.h index fec8586..55c6b81 100644 --- a/arch/arm/include/asm/arch-rockchip/sdram_common.h +++ b/arch/arm/include/asm/arch-rockchip/sdram_common.h @@ -55,4 +55,8 @@ size_t rockchip_sdram_size(phys_addr_t reg);

/* Called by U-Boot board_init_r for Rockchip SoCs */ int dram_init(void);

+/* Write ddr param to a known place for trustos */ +int rockchip_setup_ddr_param(struct ram_info *info);

#endif diff --git a/arch/arm/mach-rockchip/sdram_common.c b/arch/arm/mach-rockchip/sdram_common.c index 76dbdc8..3a71f09 100644 --- a/arch/arm/mach-rockchip/sdram_common.c +++ b/arch/arm/mach-rockchip/sdram_common.c @@ -12,6 +12,15 @@ #include <dm/uclass-internal.h>

DECLARE_GLOBAL_DATA_PTR; +struct ddr_param {

• u32 count;
• u32 reserved;
• u64 bank_addr;
• u64 bank_size;

+};

+#define PARAM_DRAM_INFO_OFFSET 0x2000000

Having this a fixed offset feels a bit hackish... especially as this is now implemented for all SOCs.

size_t rockchip_sdram_size(phys_addr_t reg) { u32 rank, col, bk, cs0_row, cs1_row, bw, row_3_4; @@ -81,3 +90,15 @@ ulong board_get_usable_ram_top(ulong total_size)

return (gd->ram_top > top) ? top : gd->ram_top; }

+int rockchip_setup_ddr_param(struct ram_info *info) +{

• struct ddr_param *dinfo = (struct ddr_param *)CONFIG_SYS_SDRAM_BASE +

• 			PARAM_DRAM_INFO_OFFSET;
  

• dinfo->count = 1;
• dinfo->bank_addr = info->base;
• dinfo->bank_size = info->size;
• return 0;

+}

This setup should only be performed when preparing the system to enter the ATF. So you will need to hook it into path where the ATF is prepared for being jumped into.

I would further prefer (but this is a personal preference) to eventually move to using the FDT we inject into the ATF to convey this information.

On Tue, 27 Mar 2018, Kever Yang wrote:

dram_init_banksize() can be common used by all SoCs, move it into sdram_common.c

Signed-off-by: Kever Yang kever.yang@rock-chips.com

arch/arm/mach-rockchip/sdram_common.c | 63 ++++++++++++++++++++++++++++++++++- 1 file changed, 62 insertions(+), 1 deletion(-)

diff --git a/arch/arm/mach-rockchip/sdram_common.c b/arch/arm/mach-rockchip/sdram_common.c index 3a71f09..ff86096 100644 --- a/arch/arm/mach-rockchip/sdram_common.c +++ b/arch/arm/mach-rockchip/sdram_common.c @@ -21,13 +21,74 @@ struct ddr_param {

#define PARAM_DRAM_INFO_OFFSET 0x2000000

+#define TRUST_PARAMETER_OFFSET (34 * 1024 * 1024)

This isn't covered by what you commit message states as content for this patch.

+struct tos_parameter_t {

• u32 version;
• u32 checksum;
• struct {

• char name[8];
  

• s64 phy_addr;
  

• u32 size;
  

• u32 flags;
  

• } tee_mem;
• struct {

• char name[8];
  

• s64 phy_addr;
  

• u32 size;
  

• u32 flags;
  

• } drm_mem;
• s64 reserve[8];

+};

+int dram_init_banksize(void) +{

• size_t top = min((unsigned long)(gd->ram_size + CONFIG_SYS_SDRAM_BASE),

• 	 gd->ram_top);
  

+#ifdef CONFIG_ARM64

• /* Reserve 0x200000 for ATF bl31 */
• gd->bd->bi_dram[0].start = 0x200000;
• gd->bd->bi_dram[0].size = top - gd->bd->bi_dram[0].start;

This should only be done when preparing to start a IH_OS_ARM_TRUSTED_FIRMWARE.

+#else +#ifdef CONFIG_SPL_OPTEE

I don't think that this CONFIG_SPL_OPTEE was what the comments in the OPTEE thread have arrived at... please check again.

Just as an unreleated comment/reminder: you still need to revise the other series as per the comments and final decision on how to implement it.

• struct tos_parameter_t *tos_parameter;
• tos_parameter = (struct tos_parameter_t *)(CONFIG_SYS_SDRAM_BASE +

• 	TRUST_PARAMETER_OFFSET);
  

• if (tos_parameter->tee_mem.flags == 1) {

Please describe what this 'flags' member means and how it's encoded.

• gd->bd->bi_dram[0].start = CONFIG_SYS_SDRAM_BASE;
  

• gd->bd->bi_dram[0].size = tos_parameter->tee_mem.phy_addr
  

• 			- CONFIG_SYS_SDRAM_BASE;
  

• gd->bd->bi_dram[1].start = tos_parameter->tee_mem.phy_addr +
  

• 			tos_parameter->tee_mem.size;
  

• gd->bd->bi_dram[1].size = gd->bd->bi_dram[0].start
  

• 			+ top - gd->bd->bi_dram[1].start;
  

• } else {

• gd->bd->bi_dram[0].start = CONFIG_SYS_SDRAM_BASE;
  

• gd->bd->bi_dram[0].size = 0x8400000;
  

• /* Reserve 32M for OPTEE with TA */
  

• gd->bd->bi_dram[1].start = CONFIG_SYS_SDRAM_BASE
  

• 			+ gd->bd->bi_dram[0].size + 0x2000000;
  

• gd->bd->bi_dram[1].size = gd->bd->bi_dram[0].start
  

• 			+ top - gd->bd->bi_dram[1].start;
  

• }

This should again only be done, when the appropriate IH_OS_* is entered.

+#else

• gd->bd->bi_dram[0].start = CONFIG_SYS_SDRAM_BASE;
• gd->bd->bi_dram[0].size = top - gd->bd->bi_dram[0].start;

This should be the default (so you can remove the #if paths) once you make sure that the other paths are actually called for entering the particular IH_OS_* types.

+#endif +#endif

• return 0;

+}

size_t rockchip_sdram_size(phys_addr_t reg) { u32 rank, col, bk, cs0_row, cs1_row, bw, row_3_4; size_t chipsize_mb = 0; size_t size_mb = 0; u32 ch;

Ok, but not really needed (unless you want to do a separate 'cosmetic' or 'whitespace' patch. Touching an unrelated function is usually a bad idea.

u32 sys_reg = readl(reg); u32 ch_num = 1 + ((sys_reg >> SYS_REG_NUM_CH_SHIFT) & SYS_REG_NUM_CH_MASK);

On Tue, 27 Mar 2018, Kever Yang wrote:

remoe rk3128 board file and move SoC spec setting into rk3128.c

'Remove'? Also: please elaborate on what this patch is doing.

Signed-off-by: Kever Yang kever.yang@rock-chips.com

Many comments agains the similar changes also apply here.

---

arch/arm/mach-rockchip/rk3128-board.c | 127 ---------------------------------- 1 file changed, 127 deletions(-) delete mode 100644 arch/arm/mach-rockchip/rk3128-board.c

diff --git a/arch/arm/mach-rockchip/rk3128-board.c b/arch/arm/mach-rockchip/rk3128-board.c deleted file mode 100644 index 2e8393d..0000000 --- a/arch/arm/mach-rockchip/rk3128-board.c +++ /dev/null @@ -1,127 +0,0 @@ -/*

• • (C) Copyright 2017 Rockchip Electronics Co., Ltd.
• • SPDX-License-Identifier: GPL-2.0+
• */

-#include <common.h> -#include <clk.h> -#include <dm.h> -#include <ram.h> -#include <syscon.h> -#include <asm/io.h> -#include <asm/arch/clock.h> -#include <asm/arch/periph.h> -#include <asm/arch/grf_rk3128.h> -#include <asm/arch/boot_mode.h> -#include <asm/arch/timer.h> -#include <power/regulator.h>

-DECLARE_GLOBAL_DATA_PTR;

-__weak int rk_board_late_init(void) -{

• return 0;

-}

-int board_late_init(void) -{

• setup_boot_mode();
• return rk_board_late_init();

-}

-int board_init(void) -{

• int ret = 0;
• rockchip_timer_init();
• ret = regulators_enable_boot_on(false);
• if (ret) {

• debug("%s: Cannot enable boot on regulator\n", __func__);
  

• return ret;
  

• }
• return 0;

-}

-int dram_init_banksize(void) -{

• gd->bd->bi_dram[0].start = CONFIG_SYS_SDRAM_BASE;
• gd->bd->bi_dram[0].size = 0x8400000;
• /* Reserve 0xe00000(14MB) for OPTEE with TA enabled, otherwise 2MB */
• gd->bd->bi_dram[1].start = CONFIG_SYS_SDRAM_BASE

• 		+ gd->bd->bi_dram[0].size + 0xe00000;
  

• gd->bd->bi_dram[1].size = gd->bd->bi_dram[0].start

• 		+ gd->ram_size - gd->bd->bi_dram[1].start;
  

• return 0;

-}

-#ifndef CONFIG_SYS_DCACHE_OFF -void enable_caches(void) -{

• /* Enable D-cache. I-cache is already enabled in start.S */
• dcache_enable();

-} -#endif

-#if defined(CONFIG_USB_GADGET) && defined(CONFIG_USB_GADGET_DWC2_OTG) -#include <usb.h> -#include <usb/dwc2_udc.h>

-static struct dwc2_plat_otg_data rk3128_otg_data = {

• .rx_fifo_sz = 512,
• .np_tx_fifo_sz = 16,
• .tx_fifo_sz = 128,

-};

-int board_usb_init(int index, enum usb_init_type init) -{

• int node;
• const char *mode;
• bool matched = false;
• const void *blob = gd->fdt_blob;
• /* find the usb_otg node */
• node = fdt_node_offset_by_compatible(blob, -1,

• 			     "rockchip,rk3128-usb");
  

• while (node > 0) {

• mode = fdt_getprop(blob, node, "dr_mode", NULL);
  

• if (mode && strcmp(mode, "otg") == 0) {
  

• 	matched = true;
  

• 	break;
  

• }
  

• node = fdt_node_offset_by_compatible(blob, node,
  

• 				     "rockchip,rk3128-usb");
  

• }
• if (!matched) {

• debug("Not found usb_otg device\n");
  

• return -ENODEV;
  

• }
• rk3128_otg_data.regs_otg = fdtdec_get_addr(blob, node, "reg");
• return dwc2_udc_probe(&rk3128_otg_data);

-}

-int board_usb_cleanup(int index, enum usb_init_type init) -{

• return 0;

-} -#endif

-#if defined(CONFIG_USB_FUNCTION_FASTBOOT) -int fb_set_reboot_flag(void) -{

• struct rk3128_grf *grf;
• printf("Setting reboot to fastboot flag ...\n");
• grf = syscon_get_first_range(ROCKCHIP_SYSCON_GRF);
• /* Set boot mode to fastboot */
• writel(BOOT_FASTBOOT, &grf->os_reg[0]);
• return 0;

-} -#endif

On Tue, Mar 27, 2018 at 10:29 AM, Kever Yang kever.yang@rock-chips.com wrote:

Use "uboot,spl-boot-orde" instead of "uboot,boot0".

I think these should be in *-u-boot.dtsi files, that enables easy sync between the linux kernel dts files and u-boot like done on a number of other configs, a couple of the rk3368 devices seem to already use this as does devices like AllWinner

Signed-off-by: Kever Yang kever.yang@rock-chips.com

arch/arm/dts/rk3288-phycore-rdk.dts | 6 +----- arch/arm/dts/rk3288-veyron.dtsi | 6 +----- 2 files changed, 2 insertions(+), 10 deletions(-)

diff --git a/arch/arm/dts/rk3288-phycore-rdk.dts b/arch/arm/dts/rk3288-phycore-rdk.dts index f2bb7b5..3bf8efb 100644 --- a/arch/arm/dts/rk3288-phycore-rdk.dts +++ b/arch/arm/dts/rk3288-phycore-rdk.dts @@ -53,11 +53,7 @@

    chosen {
            stdout-path = &uart2;

•   };
  

•   config {
  

•           u-boot,dm-pre-reloc;
  

•           u-boot,boot0 = &emmc;
  

•           u-boot,spl-boot-order = &emmc;
    };
  
    user_buttons: user-buttons {
  

diff --git a/arch/arm/dts/rk3288-veyron.dtsi b/arch/arm/dts/rk3288-veyron.dtsi index a314058..77b9bf8 100644 --- a/arch/arm/dts/rk3288-veyron.dtsi +++ b/arch/arm/dts/rk3288-veyron.dtsi @@ -17,11 +17,7 @@

    chosen {
            stdout-path = &uart2;

•   };
  

•   config {
  

•           u-boot,dm-pre-reloc;
  

•           u-boot,boot0 = &spi_flash;
  

•           u-boot,spl-boot-order = &spi_flash;
    };
  
    firmware {
  

-- 1.9.1

---

U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot

On Tue, 27 Mar 2018, Kever Yang wrote:

We need timer_get_boot_us() for boot stage if we use generic timer only.

Signed-off-by: Kever Yang kever.yang@rock-chips.com

See below for requested changes/questions.

---

arch/arm/cpu/armv8/generic_timer.c | 6 ++++++ 1 file changed, 6 insertions(+)

diff --git a/arch/arm/cpu/armv8/generic_timer.c b/arch/arm/cpu/armv8/generic_timer.c index a2dda33..d96217e 100644 --- a/arch/arm/cpu/armv8/generic_timer.c +++ b/arch/arm/cpu/armv8/generic_timer.c @@ -7,6 +7,7 @@

#include <common.h> #include <command.h> +#include <div64.h> #include <asm/system.h>

DECLARE_GLOBAL_DATA_PTR; @@ -52,6 +53,11 @@ uint64_t get_ticks(void) return ticks; }

+ulong timer_get_boot_us(void) +{

• return lldiv(get_ticks(), CONFIG_SYS_HZ_CLOCK / (CONFIG_SYS_HZ * 1000));

+}

Can we be sure that this does never conflict with what is added by another timer implementation (e.g. DM_TIMER)? If not, you may have to add some additional infrastructure to allow selection of what timer is the boot-timer.

unsigned long usec2ticks(unsigned long usec) { ulong ticks;

hi Kever:

s/rk1108/rv1108

On 2018年03月27日 17:29, Kever Yang wrote:

We use ARM arch timer instead.

Signed-off-by: Kever Yang kever.yang@rock-chips.com

include/configs/rv1108_common.h | 5 ----- 1 file changed, 5 deletions(-)

diff --git a/include/configs/rv1108_common.h b/include/configs/rv1108_common.h index cd204e9..7a9ec7b 100644 --- a/include/configs/rv1108_common.h +++ b/include/configs/rv1108_common.h @@ -13,11 +13,6 @@ #define CONFIG_SYS_CBSIZE 1024 #define CONFIG_SKIP_LOWLEVEL_INIT

-#define CONFIG_SYS_TIMER_RATE (24 * 1000 * 1000) -/* TIMER1,initialized by ddr initialize code */ -#define CONFIG_SYS_TIMER_BASE 0x10350020 -#define CONFIG_SYS_TIMER_COUNTER (CONFIG_SYS_TIMER_BASE + 8)

• #define CONFIG_SYS_SDRAM_BASE 0x60000000 #define CONFIG_NR_DRAM_BANKS 1 #define CONFIG_SYS_INIT_SP_ADDR (CONFIG_SYS_TEXT_BASE + 0x100000)

On Tue, 27 Mar 2018, Kever Yang wrote:

We use ARM generic timer.

A more enlightening commit message, please.

Signed-off-by: Kever Yang kever.yang@rock-chips.com

See below for comments.

---

configs/lion-rk3368_defconfig | 4 ---- 1 file changed, 4 deletions(-)

diff --git a/configs/lion-rk3368_defconfig b/configs/lion-rk3368_defconfig index 8a95ce3..89c4d76 100644 --- a/configs/lion-rk3368_defconfig +++ b/configs/lion-rk3368_defconfig @@ -88,10 +88,6 @@ CONFIG_DEBUG_UART_ANNOUNCE=y CONFIG_DEBUG_UART_SKIP_INIT=y CONFIG_ROCKCHIP_SPI=y CONFIG_SYSRESET=y -CONFIG_TIMER=y -CONFIG_SPL_TIMER=y -CONFIG_TPL_TIMER=y -CONFIG_ROCKCHIP_TIMER=y

NAK: The reason to not do this (and to have a DM time for the RK3368) has been discussed, when we originally added this: SPL should not be specific on the software stack.

Consider the following cases: 1. Boot to Linux (or U-Boot in EL2): this always includes an ATF as the next-stage... so no need to setup the secure time here, as ATF will take care of this anyway. 2. Boot to U-Boot in EL3 (e.g. from the Miniloader): we shouldn't rely on the secure time having been set up (but U-Boot can't do it either, as the same binary could either run at EL3 or at EL2).

So the consensus was to not have U-Boot rely on the secure timer to be initialised... especially, as it doesn't have to rely on this.

Note that this is also true for the RK3399, but I need to finish up further changes to the DRAM init code, as that currently relies on having a timebase before the DM timer is available.

CONFIG_USE_TINY_PRINTF=y CONFIG_SPL_TINY_MEMSET=y CONFIG_LZO=y

On Tue, 27 Mar 2018, Kever Yang wrote:

Use common board file and move SoC spec setting into rk3399.c

Signed-off-by: Kever Yang kever.yang@rock-chips.com

See below for a few comments. I'll review again, once this series is somewhat more mature...

---

arch/arm/mach-rockchip/rk3399-board-spl.c | 179 ---------------------- arch/arm/mach-rockchip/rk3399-board.c | 14 -- arch/arm/mach-rockchip/rk3399/rk3399.c | 86 +++++++++-- board/rockchip/evb_rk3399/evb-rk3399.c | 56 ++++--- board/theobroma-systems/puma_rk3399/puma-rk3399.c | 17 +- 5 files changed, 103 insertions(+), 249 deletions(-) delete mode 100644 arch/arm/mach-rockchip/rk3399-board-spl.c delete mode 100644 arch/arm/mach-rockchip/rk3399-board.c

diff --git a/arch/arm/mach-rockchip/rk3399-board-spl.c b/arch/arm/mach-rockchip/rk3399-board-spl.c deleted file mode 100644 index d35990e..0000000 --- a/arch/arm/mach-rockchip/rk3399-board-spl.c +++ /dev/null @@ -1,179 +0,0 @@ -/*

• • (C) Copyright 2016 Rockchip Electronics Co., Ltd
• • (C) Copyright 2017 Theobroma Systems Design und Consulting GmbH

Please make sure when moving code we contributed, that our copyright shows up in the new location you are moving this to.

• • SPDX-License-Identifier: GPL-2.0+
• */

-#include <common.h> -#include <asm/arch/bootrom.h> -#include <asm/arch/clock.h> -#include <asm/arch/grf_rk3399.h> -#include <asm/arch/hardware.h> -#include <asm/arch/periph.h> -#include <asm/io.h> -#include <debug_uart.h> -#include <dm.h> -#include <dm/pinctrl.h> -#include <ram.h> -#include <spl.h> -#include <syscon.h>

-DECLARE_GLOBAL_DATA_PTR;

-void board_return_to_bootrom(void) -{

• back_to_bootrom(BROM_BOOT_NEXTSTAGE);

-}

-static const char * const boot_devices[BROM_LAST_BOOTSOURCE + 1] = {

• [BROM_BOOTSOURCE_EMMC] = "/sdhci@fe330000",
• [BROM_BOOTSOURCE_SPINOR] = "/spi@ff1d0000",
• [BROM_BOOTSOURCE_SD] = "/dwmmc@fe320000",

-};

-const char *board_spl_was_booted_from(void) -{

• u32 bootdevice_brom_id = readl(RK3399_BROM_BOOTSOURCE_ID_ADDR);
• const char *bootdevice_ofpath = NULL;
• if (bootdevice_brom_id < ARRAY_SIZE(boot_devices))

• bootdevice_ofpath = boot_devices[bootdevice_brom_id];
  

• if (bootdevice_ofpath)

• debug("%s: brom_bootdevice_id %x maps to '%s'\n",
  

•       __func__, bootdevice_brom_id, bootdevice_ofpath);
  

• else

• debug("%s: failed to resolve brom_bootdevice_id %x\n",
  

•       __func__, bootdevice_brom_id);
  

• return bootdevice_ofpath;

-}

-u32 spl_boot_device(void) -{

• u32 boot_device = BOOT_DEVICE_MMC1;
• if (CONFIG_IS_ENABLED(ROCKCHIP_BACK_TO_BROM))

• return BOOT_DEVICE_BOOTROM;
  

• return boot_device;

-}

-#define TIMER_CHN10_BASE 0xff8680a0 -#define TIMER_END_COUNT_L 0x00 -#define TIMER_END_COUNT_H 0x04 -#define TIMER_INIT_COUNT_L 0x10 -#define TIMER_INIT_COUNT_H 0x14 -#define TIMER_CONTROL_REG 0x1c

-#define TIMER_EN 0x1 -#define TIMER_FMODE (0 << 1) -#define TIMER_RMODE (1 << 1)

-void secure_timer_init(void) -{

• writel(0xffffffff, TIMER_CHN10_BASE + TIMER_END_COUNT_L);
• writel(0xffffffff, TIMER_CHN10_BASE + TIMER_END_COUNT_H);
• writel(0, TIMER_CHN10_BASE + TIMER_INIT_COUNT_L);
• writel(0, TIMER_CHN10_BASE + TIMER_INIT_COUNT_H);
• writel(TIMER_EN | TIMER_FMODE, TIMER_CHN10_BASE + TIMER_CONTROL_REG);

-}

-void board_debug_uart_init(void) -{ -#define GRF_BASE 0xff770000

• struct rk3399_grf_regs * const grf = (void *)GRF_BASE;

-#if defined(CONFIG_DEBUG_UART_BASE) && (CONFIG_DEBUG_UART_BASE == 0xff180000)

• /* Enable early UART0 on the RK3399 */
• rk_clrsetreg(&grf->gpio2c_iomux,

•      GRF_GPIO2C0_SEL_MASK,
  

•      GRF_UART0BT_SIN << GRF_GPIO2C0_SEL_SHIFT);
  

• rk_clrsetreg(&grf->gpio2c_iomux,

•      GRF_GPIO2C1_SEL_MASK,
  

•      GRF_UART0BT_SOUT << GRF_GPIO2C1_SEL_SHIFT);
  

-#else

• /* Enable early UART2 channel C on the RK3399 */
• rk_clrsetreg(&grf->gpio4c_iomux,

•      GRF_GPIO4C3_SEL_MASK,
  

•      GRF_UART2DGBC_SIN << GRF_GPIO4C3_SEL_SHIFT);
  

• rk_clrsetreg(&grf->gpio4c_iomux,

•      GRF_GPIO4C4_SEL_MASK,
  

•      GRF_UART2DBGC_SOUT << GRF_GPIO4C4_SEL_SHIFT);
  

• /* Set channel C as UART2 input */
• rk_clrsetreg(&grf->soc_con7,

•      GRF_UART_DBG_SEL_MASK,
  

•      GRF_UART_DBG_SEL_C << GRF_UART_DBG_SEL_SHIFT);
  

-#endif -}

-void board_init_f(ulong dummy) -{

• struct udevice *pinctrl;
• struct udevice *dev;
• struct rk3399_pmusgrf_regs *sgrf;
• struct rk3399_grf_regs *grf;
• int ret;

-#define EARLY_UART -#ifdef EARLY_UART

• /*

• * Debug UART can be used from here if required:
  

• *
  

• * debug_uart_init();
  

• * printch('a');
  

• * printhex8(0x1234);
  

• * printascii("string");
  

• */
  

• debug_uart_init();
• printascii("U-Boot SPL board init");

-#endif

• ret = spl_early_init();
• if (ret) {

• debug("spl_early_init() failed: %d\n", ret);
  

• hang();
  

• }
• /*

• * Disable DDR and SRAM security regions.
  

• *
  

• * As we are entered from the BootROM, the region from
  

• * 0x0 through 0xfffff (i.e. the first MB of memory) will
  

• * be protected. This will cause issues with the DW_MMC
  

• * driver, which tries to DMA from/to the stack (likely)
  

• * located in this range.
  

• */
  

• sgrf = syscon_get_first_range(ROCKCHIP_SYSCON_PMUSGRF);
• rk_clrsetreg(&sgrf->ddr_rgn_con[16], 0x1ff, 0);
• rk_clrreg(&sgrf->slv_secure_con4, 0x2000);
• /* eMMC clock generator: disable the clock multipilier */
• grf = syscon_get_first_range(ROCKCHIP_SYSCON_GRF);
• rk_clrreg(&grf->emmccore_con[11], 0x0ff);
• secure_timer_init();
• ret = uclass_get_device(UCLASS_PINCTRL, 0, &pinctrl);
• if (ret) {

• debug("Pinctrl init failed: %d\n", ret);
  

• return;
  

• }
• ret = uclass_get_device(UCLASS_RAM, 0, &dev);
• if (ret) {

• debug("DRAM init failed: %d\n", ret);
  

• return;
  

• }

-}

-#ifdef CONFIG_SPL_LOAD_FIT -int board_fit_config_name_match(const char *name) -{

• /* Just empty function now - can't decide what to choose */
• debug("%s: %s\n", __func__, name);
• return 0;

-} -#endif diff --git a/arch/arm/mach-rockchip/rk3399-board.c b/arch/arm/mach-rockchip/rk3399-board.c deleted file mode 100644 index 9293843..0000000 --- a/arch/arm/mach-rockchip/rk3399-board.c +++ /dev/null @@ -1,14 +0,0 @@ -/*

• • Copyright (c) 2017 Rockchip Electronics Co., Ltd
• • SPDX-License-Identifier: GPL-2.0+
• */

-#include <common.h> -#include <asm/arch/boot_mode.h>

-int board_late_init(void) -{

• setup_boot_mode();
• return 0;

-} diff --git a/arch/arm/mach-rockchip/rk3399/rk3399.c b/arch/arm/mach-rockchip/rk3399/rk3399.c index dbc248f..6c89f25 100644 --- a/arch/arm/mach-rockchip/rk3399/rk3399.c +++ b/arch/arm/mach-rockchip/rk3399/rk3399.c @@ -6,8 +6,11 @@

#include <common.h> #include <asm/armv8/mmu.h> -#include <asm/io.h> +#include <asm/arch/bootrom.h> +#include <asm/arch/grf_rk3399.h> #include <asm/arch/hardware.h> +#include <asm/io.h> +#include <syscon.h>

DECLARE_GLOBAL_DATA_PTR;

@@ -35,23 +38,86 @@ static struct mm_region rk3399_mem_map[] = {

struct mm_region *mem_map = rk3399_mem_map;

-int dram_init_banksize(void) -{

• size_t max_size = min((unsigned long)gd->ram_size, gd->ram_top);

+const char * const boot_devices[BROM_LAST_BOOTSOURCE + 1] = {

• [BROM_BOOTSOURCE_EMMC] = "/sdhci@fe330000",
• [BROM_BOOTSOURCE_SPINOR] = "/spi@ff1d0000",
• [BROM_BOOTSOURCE_SD] = "/dwmmc@fe320000",

+};

• /* Reserve 0x200000 for ATF bl31 */
• gd->bd->bi_dram[0].start = 0x200000;
• gd->bd->bi_dram[0].size = max_size - gd->bd->bi_dram[0].start;

+#ifdef CONFIG_SPL_BUILD

• return 0;

+#define TIMER_CHN10_BASE 0xff8680a0 +#define TIMER_END_COUNT_L 0x00 +#define TIMER_END_COUNT_H 0x04 +#define TIMER_INIT_COUNT_L 0x10 +#define TIMER_INIT_COUNT_H 0x14 +#define TIMER_CONTROL_REG 0x1c

+#define TIMER_EN 0x1 +#define TIMER_FMODE (0 << 1) +#define TIMER_RMODE (1 << 1)

+void rockchip_stimer_init(void) +{

• writel(0xffffffff, TIMER_CHN10_BASE + TIMER_END_COUNT_L);
• writel(0xffffffff, TIMER_CHN10_BASE + TIMER_END_COUNT_H);
• writel(0, TIMER_CHN10_BASE + TIMER_INIT_COUNT_L);
• writel(0, TIMER_CHN10_BASE + TIMER_INIT_COUNT_H);
• writel(TIMER_EN | TIMER_FMODE, TIMER_CHN10_BASE + TIMER_CONTROL_REG);
• printf("%s\n", __func__);

}

+#define GRF_BASE 0xff770000 +#define PMUSGRF_BASE 0xff330000 int arch_cpu_init(void) {

• struct rk3399_pmusgrf_regs *sgrf = (void *)PMUSGRF_BASE;
• struct rk3399_grf_regs * const grf = (void *)GRF_BASE;
• /* We do some SoC one time setting here. */
• /*

• * Disable DDR and SRAM security regions.
  

• *
  

• * As we are entered from the BootROM, the region from
  

• * 0x0 through 0xfffff (i.e. the first MB of memory) will
  

• * be protected. This will cause issues with the DW_MMC
  

• * driver, which tries to DMA from/to the stack (likely)
  

• * located in this range.
  

• */
  

• rk_clrsetreg(&sgrf->ddr_rgn_con[16], 0x1ff, 0);
• rk_clrreg(&sgrf->slv_secure_con4, 0x2000);

• /* Emmc clock generator: disable the clock multipilier */
• rk_clrreg(GRF_EMMCCORE_CON11, 0x0ff);

• /* eMMC clock generator: disable the clock multipilier */

• rk_clrreg(&grf->emmccore_con[11], 0x0ff);

  return 0;

} +#endif

+void board_debug_uart_init(void) +{ +#define GRF_BASE 0xff770000

• struct rk3399_grf_regs * const grf = (void *)GRF_BASE;

+#if defined(CONFIG_DEBUG_UART_BASE) && (CONFIG_DEBUG_UART_BASE == 0xff180000)

• /* Enable early UART0 on the RK3399 */
• rk_clrsetreg(&grf->gpio2c_iomux,

•      GRF_GPIO2C0_SEL_MASK,
  

•      GRF_UART0BT_SIN << GRF_GPIO2C0_SEL_SHIFT);
  

• rk_clrsetreg(&grf->gpio2c_iomux,

•      GRF_GPIO2C1_SEL_MASK,
  

•      GRF_UART0BT_SOUT << GRF_GPIO2C1_SEL_SHIFT);
  

+#else

• /* Enable early UART2 channel C on the RK3399 */
• rk_clrsetreg(&grf->gpio4c_iomux,

•      GRF_GPIO4C3_SEL_MASK,
  

•      GRF_UART2DGBC_SIN << GRF_GPIO4C3_SEL_SHIFT);
  

• rk_clrsetreg(&grf->gpio4c_iomux,

•      GRF_GPIO4C4_SEL_MASK,
  

•      GRF_UART2DBGC_SOUT << GRF_GPIO4C4_SEL_SHIFT);
  

• /* Set channel C as UART2 input */
• rk_clrsetreg(&grf->soc_con7,

•      GRF_UART_DBG_SEL_MASK,
  

•      GRF_UART_DBG_SEL_C << GRF_UART_DBG_SEL_SHIFT);
  

+#endif +} diff --git a/board/rockchip/evb_rk3399/evb-rk3399.c b/board/rockchip/evb_rk3399/evb-rk3399.c index 502dec3..79b7436 100644 --- a/board/rockchip/evb_rk3399/evb-rk3399.c +++ b/board/rockchip/evb_rk3399/evb-rk3399.c @@ -6,23 +6,29 @@

#include <common.h> #include <dm.h> +#include <ram.h> #include <dm/pinctrl.h> #include <dm/uclass-internal.h> #include <asm/arch/periph.h> #include <power/regulator.h> +#include <usb.h> +#include <dwc3-uboot.h> #include <spl.h>

DECLARE_GLOBAL_DATA_PTR;

-int board_init(void) +#define RK3399_CPUID_OFF 0x7 +#define RK3399_CPUID_LEN 0x10

+int rk_board_init(void) { struct udevice *pinctrl, *regulator; int ret;

/*

• * The PWM do not have decicated interrupt number in dts and can
  

• * The PWM does not have decicated interrupt number in dts and can
  

  • not get periph_id by pinctrl framework, so let's init them here.

• * The PWM2 and PWM3 are for pwm regulater.
  

• * The PWM2 and PWM3 are for pwm regulators.
  

  */ ret = uclass_get_device(UCLASS_PINCTRL, 0, &pinctrl); if (ret) {

@@ -49,10 +55,6 @@ int board_init(void) goto out; }

• ret = regulators_enable_boot_on(false);
• if (ret)

• debug("%s: Cannot enable boot on regulator\n", __func__);
  

• ret = regulator_get_by_platname("vcc5v0_host", &regulator); if (ret) { debug("%s vcc5v0_host init fail! ret %d\n", __func__, ret);

@@ -69,29 +71,23 @@ out: return 0; }

-void spl_board_init(void) -{

• struct udevice *pinctrl;
• int ret;
• ret = uclass_get_device(UCLASS_PINCTRL, 0, &pinctrl);
• if (ret) {

• debug("%s: Cannot find pinctrl device\n", __func__);
  

• goto err;
  

• }
• /* Enable debug UART */
• ret = pinctrl_request_noflags(pinctrl, PERIPH_ID_UART_DBG);
• if (ret) {

• debug("%s: Failed to set up console UART\n", __func__);
  

• goto err;
  

• }

+#ifdef CONFIG_USB_DWC3 +static struct dwc3_device dwc3_device_data = {

• .maximum_speed = USB_SPEED_HIGH,
• .base = 0xfe800000,
• .dr_mode = USB_DR_MODE_PERIPHERAL,
• .index = 0,
• .dis_u2_susphy_quirk = 1,

+};

• preloader_console_init();
• return;

-err:

• printf("%s: Error %d\n", __func__, ret);

+int usb_gadget_handle_interrupts(void) +{

• dwc3_uboot_handle_interrupt(0);
• return 0;

+}

• /* No way to report error here */
• hang();

+int board_usb_init(int index, enum usb_init_type init) +{

• return dwc3_uboot_init(&dwc3_device_data);

} +#endif diff --git a/board/theobroma-systems/puma_rk3399/puma-rk3399.c b/board/theobroma-systems/puma_rk3399/puma-rk3399.c index c6690fa..9946311 100644 --- a/board/theobroma-systems/puma_rk3399/puma-rk3399.c +++ b/board/theobroma-systems/puma_rk3399/puma-rk3399.c @@ -25,21 +25,6 @@

DECLARE_GLOBAL_DATA_PTR;

-int board_init(void) -{

• int ret;
• /*

• * We need to call into regulators_enable_boot_on() again, as the call
  

• * during SPL may have not included all regulators.
  

• */
  

• ret = regulators_enable_boot_on(false);
• if (ret)

• debug("%s: Cannot enable boot on regulator\n", __func__);
  

This is critical during SPL for us. I don't see this being moved anywhere else.

• return 0;

-}

static void rk3399_force_power_on_reset(void) { ofnode node; @@ -62,7 +47,7 @@ static void rk3399_force_power_on_reset(void) dm_gpio_set_value(&sysreset_gpio, 1); }

-void spl_board_init(void) +void rk_spl_board_init(void)

I am very unhappy with this being renamed: as indicated in my review of the new common board-file, we need a more robust solution to this.

{ int ret; struct rk3399_cru *cru = rockchip_get_cru();

On Tue, Mar 27, 2018 at 10:29 AM, Kever Yang kever.yang@rock-chips.com wrote:

pre-loader console in SPL needs UART.

Signed-off-by: Kever Yang kever.yang@rock-chips.com

arch/arm/dts/rk3399-firefly.dts | 1 + 1 file changed, 1 insertion(+)

diff --git a/arch/arm/dts/rk3399-firefly.dts b/arch/arm/dts/rk3399-firefly.dts index f134c00..a007fa1 100644 --- a/arch/arm/dts/rk3399-firefly.dts +++ b/arch/arm/dts/rk3399-firefly.dts @@ -648,6 +648,7 @@ };

&uart2 {

•   u-boot,dm-pre-reloc;
  

if this is u-boot specific shouldn't it be in a rk3399-firefly-u-boot.dtsi ?

    status = "okay";

};

-- 1.9.1

---

U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot