Hi Andreas,

On 07/18/2016 10:13 PM, Andreas Färber wrote:
Hi Kever,

Am 18.07.2016 um 06:54 schrieb Kever Yang:
Hi Andreas,

     Thanks for you comments, I will apply them one by one except some
confuse below.

On 07/18/2016 07:26 AM, Andreas Färber wrote:
Hi,

Isn't evb short for evaluation board? That makes board board then. ;)

Am 15.07.2016 um 10:42 schrieb Kever Yang:
RK3399 is a SoC from Rockchip with dual-core Cortex-A72
and qual-core Cortex-A53 CPU. It supports two USB3.0
quad-core

Fixed.


type-C ports and two USB2.0 EHCI ports. Other interfaces
are very like RK3288, the DRAM are 32bit width address
"very much like" or "very similar to"

Fixed.


and support address from 0 to 4GB-128MB range.

Signed-off-by: Kever Yang <kever.y...@rock-chips.com>
---

Changes in v3:
Rebase on patch from Andreas:
[PATCH] rockchip: Exclude rk_timer for ARM64
[PATCH] rockchip: Clean up CPU selection

Changes in v2:
fix description error on board Kconfig

   arch/arm/Kconfig                       |  2 -
   arch/arm/mach-rockchip/Kconfig         | 22 ++++++++-
   arch/arm/mach-rockchip/rk3399/Kconfig  | 14 ++++++
   arch/arm/mach-rockchip/rk3399/Makefile |  5 ++
   board/rockchip/evb_rk3399/Kconfig      | 15 ++++++
   board/rockchip/evb_rk3399/MAINTAINERS  |  0
   board/rockchip/evb_rk3399/Makefile     |  7 +++
   board/rockchip/evb_rk3399/evb-rk3399.c | 41 +++++++++++++++++
   include/configs/evb_rk3399.h           | 24 ++++++++++
   include/configs/rk3399_common.h        | 84
++++++++++++++++++++++++++++++++++
   10 files changed, 211 insertions(+), 3 deletions(-)
   create mode 100644 arch/arm/mach-rockchip/rk3399/Kconfig
   create mode 100644 arch/arm/mach-rockchip/rk3399/Makefile
   create mode 100644 board/rockchip/evb_rk3399/Kconfig
   create mode 100644 board/rockchip/evb_rk3399/MAINTAINERS
   create mode 100644 board/rockchip/evb_rk3399/Makefile
   create mode 100644 board/rockchip/evb_rk3399/evb-rk3399.c
   create mode 100644 include/configs/evb_rk3399.h
   create mode 100644 include/configs/rk3399_common.h

diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index a262145..6e4d78a 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -846,8 +846,6 @@ config STM32
     config ARCH_ROCKCHIP
       bool "Support Rockchip SoCs"
-    select SUPPORT_SPL
-    select SPL
       select OF_CONTROL
       select DM
   diff --git a/arch/arm/mach-rockchip/Kconfig
b/arch/arm/mach-rockchip/Kconfig
index d3f5ffd..8499692 100644
--- a/arch/arm/mach-rockchip/Kconfig
+++ b/arch/arm/mach-rockchip/Kconfig
@@ -9,6 +9,10 @@ config ROCKCHIP_RK3288
         video interfaces supporting HDMI and eDP, several DDR3 options
         and video codec support. Peripherals include Gigabit Ethernet,
         USB2 host and OTG, SDIO, I2S, UART,s, SPI, I2C and PWMs.
+    select CPU_V7
+    select RK_TIMER
You no longer define RK_TIMER - either please do (I liked it) or drop
the selection, it leads to warnings on, e.g., firefly-rk3288_defconfig
otherwise.

Fixed.


+    select SUPPORT_SPL
+    select SPL
     config ROCKCHIP_RK3036
       bool "Support Rockchip RK3036"
@@ -18,6 +22,21 @@ config ROCKCHIP_RK3036
         including NEON and GPU, Mali-400 graphics, several DDR3 options
         and video codec support. Peripherals include Gigabit Ethernet,
         USB2 host and OTG, SDIO, I2S, UART, SPI, I2C and PWMs.
+    select CPU_V7
+    select RK_TIMER
+    select SUPPORT_SPL
+    select SPL
+
+config ROCKCHIP_RK3399
+    bool "Support Rockchip RK3399"
+    help
+      The Rockchip RK3399 is a ARM-based SoC with a dual-core
Cortex-A72
+      and qual-core Cortex-A53.
quad-core

Fixed.


+      including NEON and GPU, 1MB L2 cache, Mali-T7 graphics, two
+      video interfaces supporting HDMI and eDP, several DDR3 options
+      and video codec support. Peripherals include Gigabit Ethernet,
+      USB2 host and OTG, SDIO, I2S, UART,s, SPI, I2C and PWMs.
UARTs

Fixed.


+    select ARM64
     config SYS_MALLOC_F
       default y
@@ -44,8 +63,9 @@ config DM_GPIO
       default y
     config BLK
-    default y
+    default y if CPU_V7
Needs rebasing onto u-boot-rockchip.git.
Will fix in V5.

     source "arch/arm/mach-rockchip/rk3288/Kconfig"
   source "arch/arm/mach-rockchip/rk3036/Kconfig"
+source "arch/arm/mach-rockchip/rk3399/Kconfig"
   endif
diff --git a/arch/arm/mach-rockchip/rk3399/Kconfig
b/arch/arm/mach-rockchip/rk3399/Kconfig
new file mode 100644
index 0000000..923a6de
--- /dev/null
+++ b/arch/arm/mach-rockchip/rk3399/Kconfig
@@ -0,0 +1,14 @@
+if ROCKCHIP_RK3399
+
+config TARGET_EVB_RK3399
+    bool "RK3399 evb board"
Should this be enclosed in a choice section for futureproofness?
Will fix in V5.

+
+config SYS_SOC
+    default "rockchip"
+
+config SYS_MALLOC_F_LEN
+    default 0x0800
+
+source "board/rockchip/evb_rk3399/Kconfig"
+
+endif
diff --git a/arch/arm/mach-rockchip/rk3399/Makefile
b/arch/arm/mach-rockchip/rk3399/Makefile
new file mode 100644
index 0000000..ca69207
--- /dev/null
+++ b/arch/arm/mach-rockchip/rk3399/Makefile
@@ -0,0 +1,5 @@
+#
+# Copyright (C) 2016 Rockchip Electronics Co., Ltd
+#
+# SPDX-License-Identifier:      GPL-2.0+
+#
Needed?

Fixed.


diff --git a/board/rockchip/evb_rk3399/Kconfig
b/board/rockchip/evb_rk3399/Kconfig
new file mode 100644
index 0000000..412b81c
--- /dev/null
+++ b/board/rockchip/evb_rk3399/Kconfig
@@ -0,0 +1,15 @@
+if TARGET_EVB_RK3399
+
+config SYS_BOARD
+    default "evb_rk3399"
+
+config SYS_VENDOR
+    default "rockchip"
+
+config SYS_CONFIG_NAME
+    default "evb_rk3399"
+
+config BOARD_SPECIFIC_OPTIONS # dummy
+    def_bool y
+
+endif
diff --git a/board/rockchip/evb_rk3399/MAINTAINERS
b/board/rockchip/evb_rk3399/MAINTAINERS
new file mode 100644
index 0000000..e69de29
diff --git a/board/rockchip/evb_rk3399/Makefile
b/board/rockchip/evb_rk3399/Makefile
new file mode 100644
index 0000000..aaa51c2
--- /dev/null
+++ b/board/rockchip/evb_rk3399/Makefile
@@ -0,0 +1,7 @@
+#
+# (C) Copyright 2016 Rockchip Electronics Co., Ltd
+#
+# SPDX-License-Identifier:     GPL-2.0+
+#
+
+obj-y    += evb-rk3399.o
diff --git a/board/rockchip/evb_rk3399/evb-rk3399.c
b/board/rockchip/evb_rk3399/evb-rk3399.c
new file mode 100644
index 0000000..357b08b
--- /dev/null
+++ b/board/rockchip/evb_rk3399/evb-rk3399.c
@@ -0,0 +1,41 @@
+/*
+ * (C) Copyright 2016 Rockchip Electronics Co., Ltd
+ *
+ * SPDX-License-Identifier:     GPL-2.0+
+ */
+#include <dm.h>
Needed? Order/spacing.

Fixed.


+#include <common.h>
+#include <asm/armv8/mmu.h>
+
+DECLARE_GLOBAL_DATA_PTR;
+
+static struct mm_region rk3399_mem_map[] = {
+    {
+        .base = 0x0UL,
+        .size = 0x80000000UL,
+        .attrs = PTE_BLOCK_MEMTYPE(MT_NORMAL) |
+             PTE_BLOCK_INNER_SHARE
+    }, {
+        .base = 0xf0000000UL,
+        .size = 0x10000000UL,
+        .attrs = PTE_BLOCK_MEMTYPE(MT_DEVICE_NGNRNE) |
+             PTE_BLOCK_NON_SHARE |
+             PTE_BLOCK_PXN | PTE_BLOCK_UXN
+    }, {
+        /* List terminator */
+        0,
+    }
+};
+
+struct mm_region *mem_map = rk3399_mem_map;
+
+int board_init(void)
+{
+    return 0;
+}
+
+int dram_init(void)
+{
+    gd->ram_size = 0x80000000;
+    return 0;
+}
You'll need to implement dram_init_banksize() to avoid "DRAM: 0 Bytes"?
I doesn't get "DRAM: 0 Bytes", I get "DRAM: 2 GiB"
Okay, not sure what the difference between our patches was then.

But please remember to leave a white line before and after your inline
comments or they are really hard to spot among a huge quote!

Note that this happens among others in Thunderbird when writing in HTML
mode but sending as text mode. (That's why I force my account to always
use text mode also for composing, which works around that.)

diff --git a/include/configs/rk3399_common.h
b/include/configs/rk3399_common.h
new file mode 100644
index 0000000..1c13e2e
--- /dev/null
+++ b/include/configs/rk3399_common.h
@@ -0,0 +1,84 @@
+/*
+ * (C) Copyright 2016 Rockchip Electronics Co., Ltd
+ *
+ * SPDX-License-Identifier:     GPL-2.0+
+ */
+
+#ifndef __CONFIG_RK3399_COMMON_H
+#define __CONFIG_RK3399_COMMON_H
+
+#define CONFIG_SYS_CACHELINE_SIZE    64
+
+#include <asm/arch/hardware.h>
+
+#define CONFIG_SYS_NO_FLASH
+#define CONFIG_NR_DRAM_BANKS        1
+#define CONFIG_ENV_SIZE            0x2000
+#define CONFIG_SYS_MAXARGS        16
+#define CONFIG_BAUDRATE            1500000
+#define CONFIG_SYS_MALLOC_LEN        (32 << 20)
+#define CONFIG_SYS_CBSIZE        1024
+#define CONFIG_SKIP_LOWLEVEL_INIT
+#define CONFIG_DISPLAY_BOARDINFO
+
+#define CONFIG_SYS_NS16550
Please make this a Kconfig selection to avoid confusion.
OK, actually, I want to know how to decide where to put the CONFIG_
MACROs, we have defconfig, common config for soc, and common config
for board.
I don't have a definitive answer, but defconfig is per board, so if
something is always needed for the SoC it should go into the
mach-rockchip or rk3399 Kconfig, I think.

Whether that applies here ... Simon?

People add "CONFIG_SYS_NS16550=y" in defconfig, so I will do that too.


+#define CONFIG_SYS_NS16550_MEM32
+
+#define CONFIG_SYS_TEXT_BASE        0x00200000
+#define CONFIG_SYS_INIT_SP_ADDR        0x00300000
+#define CONFIG_SYS_LOAD_ADDR        0x00800800
+
+#define CONFIG_ROCKCHIP_COMMON
This is defined by other boards, but no one seems to checks it. Suggest
to drop it here and elsewhere.
Yes, the content for CONFIG_ROCKCHIP_COMMON has been removed,
sot this MACRO is no use now.
+#define CONFIG_SYS_BOOTM_LEN    (64 << 20)    /* 64M */
+
+/* MMC/SD IP block */
+#define CONFIG_MMC
+#define CONFIG_GENERIC_MMC
+#define CONFIG_SDHCI
+#define CONFIG_BOUNCE_BUFFER
+#define CONFIG_ROCKCHIP_SDHCI_MAX_FREQ    200000000
+
+#define CONFIG_DOS_PARTITION
This one is selected by config_distro_defaults.h already.

Fixed


+#define CONFIG_FAT_WRITE
+#define CONFIG_PARTITION_UUIDS
+#define CONFIG_CMD_PART
These two are selected by config_distro_bootcmd.h already.

Fixed


+
+/* RAW SD card / eMMC locations. */
+#define CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_SECTOR    256
+#define CONFIG_SYS_SPI_U_BOOT_OFFS    (128 << 10)
+
+/* FAT sd card locations. */
+#define CONFIG_SYS_MMCSD_FS_BOOT_PARTITION    1
+#define CONFIG_SPL_FS_LOAD_PAYLOAD_NAME        "u-boot.img"
+
+#define CONFIG_SPL_PINCTRL_SUPPORT
+#define CONFIG_SPL_RAM_SUPPORT
+#define CONFIG_SPL_DRIVERS_MISC_SUPPORT
Given that you moved SPL to rk3288, these are quite a few unneeded SPL
symbols?

Fixed


+
+#define CONFIG_SYS_SDRAM_BASE        0
+#define CONFIG_NR_DRAM_BANKS        1
+#define SDRAM_BANK_SIZE            (2UL << 30)
Move size into evb config? (commit message says up to ~4GB)
Possibly move banks too or is it always 1?
Rockchip evb is is combined by a mainboard and a daughter board
which daughter board usually including rk3399, DRAM, eMMC, the
DRAM could be 2GB or 4GB. The rk3399 SoC can support up to 4G-128M,
but for U-Boot, we don't actually using so large space, maybe 128MB is
enough.
So we prefer to using a 2GB config for compatible, which also easy to setup
to MMU table.
I don't see a problem with configuring less DRAM than available, but it
will become an issue once there are boards or TV boxes with 1 GB or
less. Therefore it's safer to place the SDRAM_BANK_SIZE into the evb's
config to force new boards to set it to a sane value.

Moved the SDRAM_BANK_SIZE to evb's config in V5.
The CONFIG_NR_DRAM_BANKS is always 1 for Rockchip SoCs.


How much RAM does the DDR blob enable? If it's more than 128 MB then we

What do your mean by "How much RAM does the DDR blob enable?"?

Thanks,
- Kever

should use the full 2GB or whatever here, since it affects loading of
kernels and operation of EFI applications such as GRUB (which will
allocate from top of RAM).

Note to self: In my patch I seem not to set SDRAM_BANK_SIZE at all,
which may be why I was seeing "DRAM: 0 Bytes".

Thanks,
Andreas



_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Reply via email to