[U-Boot] [PATCH v3 2/5] ARM64: rockchip: add support for rk3399 SoC based evb-board
Kever Yang
kever.yang at rock-chips.com
Tue Jul 19 15:18:12 CEST 2016
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.yang at 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
>
More information about the U-Boot
mailing list