[U-Boot] [PATCH v3 2/5] ARM64: rockchip: add support for rk3399 SoC based evb-board

Andreas Färber afaerber at suse.de
Mon Jul 18 16:13:41 CEST 2016


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
>>
>>> 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"
>>
>>> 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.
>>
>>> +    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
>>
>>> +      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
>>
>>> +    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.
>>
>>>     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?
>>
>>> +
>>> +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?
>>
>>> 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.
>>
>>> +#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?

>>> +#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.
>>
>>> +#define CONFIG_FAT_WRITE
>>> +#define CONFIG_PARTITION_UUIDS
>>> +#define CONFIG_CMD_PART
>> These two are selected by config_distro_bootcmd.h already.
>>
>>> +
>>> +/* 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?
>>
>>> +
>>> +#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.

How much RAM does the DDR blob enable? If it's more than 128 MB then we
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

-- 
SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)


More information about the U-Boot mailing list