[PATCH V4 2/2] riscv: board: Support OpenPiton SoC

[Tianrui Wei <tianrui-wei@outlook.com>]

Hi Sean,

On 5/7/2021 9:03 PM, Sean Anderson wrote:
> On 5/6/21 11:48 PM, Tianrui Wei wrote:
>>
>> On 5/7/2021 11:41 AM, Sean Anderson wrote:
>>> On 5/6/21 11:28 PM, Tianrui Wei wrote:
>>>>
>>>> On 5/7/2021 11:15 AM, Sean Anderson wrote:
>>>>> On 5/6/21 11:06 PM, Tianrui Wei wrote:
>>>>>> Hi Sean,
>>>>>>
>>>>>>
>>>>>> Many thanks again for reviewing our code! We really appreciate 
>>>>>> it. Will fix the things you're suggesting ;p Though I have a few 
>>>>>> questions in line in the comment. Also, checkpatch didn't catch 
>>>>>> any of the identation issues. I was wondering if there are some 
>>>>>> specific flags to enable some of the checks? I'm running 
>>>>>> ./utils/checkpatch.pl this.patch right now.
>>>>>>
>>>>>>
>>>>>> On 5/7/2021 10:32 AM, Sean Anderson wrote:
>>>>>>> On 5/5/21 11:42 PM, Tianrui Wei wrote:
>>>>>>>> From: Tianrui Wei <tianrui-wei@outlook.com>
>>>>>>>> Date: Thu, 6 May 2021 11:30:20 +0800
>>>>>>>> Subject: [PATCH V4 2/2] riscv: board: Support OpenPiton SoC
>>>>>>>>
>>>>>>>> This patch add board support for OpenPiton.
>>>>>>>
>>>>>>> Please add some information to the commit message describing the 
>>>>>>> board.
>>>>>>>
>>>>>>>>
>>>>>>>> Signed-off-by: Tianrui Wei <tianrui-wei@outlook.com>
>>>>>>>> Signed-off-by: Jonathan Balkind <jbalkind@ucsb.edu>
>>>>>>>> ---
>>>>>>>>
>>>>>>>> ? arch/riscv/Kconfig????????????????????? |?? 4 +
>>>>>>>> ? arch/riscv/dts/Makefile???????????????? |?? 1 +
>>>>>>>> ? arch/riscv/dts/openpiton-riscv64.dts??? | 159 +++++
>>>>>>>> ? board/openpiton/riscv/Kconfig?????????? |? 42 ++
>>>>>>>> ? board/openpiton/riscv/MAINTAINERS?????? |?? 6 +
>>>>>>>> ? board/openpiton/riscv/Makefile????????? |?? 5 +
>>>>>>>> ? board/openpiton/riscv/openpiton-riscv.c |? 41 ++
>>>>>>>> ? configs/openpiton_riscv64_defconfig???? | 132 ++++
>>>>>>>> ? doc/board/index.rst???????????????????? |?? 1 +
>>>>>>>> ? doc/board/openpiton/index.rst?????????? |?? 9 +
>>>>>>>> ? doc/board/openpiton/riscv64.rst???????? | 885 
>>>>>>>> ++++++++++++++++++++++++
>>>>>>>> ? include/configs/openpiton-riscv.h?????? |? 58 ++
>>>>>>>> ? 12 files changed, 1343 insertions(+)
>>>>>>>> ? create mode 100644 arch/riscv/dts/openpiton-riscv64.dts
>>>>>>>> ? create mode 100644 board/openpiton/riscv/Kconfig
>>>>>>>> ? create mode 100644 board/openpiton/riscv/MAINTAINERS
>>>>>>>> ? create mode 100644 board/openpiton/riscv/Makefile
>>>>>>>> ? create mode 100644 board/openpiton/riscv/openpiton-riscv.c
>>>>>>>> ? create mode 100644 configs/openpiton_riscv64_defconfig
>>>>>>>> ? create mode 100644 doc/board/openpiton/index.rst
>>>>>>>> ? create mode 100644 doc/board/openpiton/riscv64.rst
>>>>>>>> ? create mode 100644 include/configs/openpiton-riscv.h
>>>>>>>>
>>>>>>>> diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
>>>>>>>> index 30b05408..9e7deb34 100644
>>>>>>>> --- a/arch/riscv/Kconfig
>>>>>>>> +++ b/arch/riscv/Kconfig
>>>>>>>> @@ -23,6 +23,9 @@ config TARGET_SIFIVE_FU540
>>>>>>>> ? config TARGET_SIPEED_MAIX
>>>>>>>> ????? bool "Support Sipeed Maix Board"
>>>>>>>> ? +config TARGET_OPENPITON_RISCV
>>>>>>>> +? bool "Support riscv cores on openpiton SoC"
>>>>>>>> +
>>>>>>>> ? endchoice
>>>>>>>> ??? config SYS_ICACHE_OFF
>>>>>>>> @@ -55,6 +58,7 @@ config SPL_SYS_DCACHE_OFF
>>>>>>>> ? source "board/AndesTech/ax25-ae350/Kconfig"
>>>>>>>> ? source "board/emulation/qemu-riscv/Kconfig"
>>>>>>>> ? source "board/microchip/mpfs_icicle/Kconfig"
>>>>>>>> +source "board/openpiton/riscv/Kconfig"
>>>>>>>
>>>>>>> Fix indentation.
>>>>>>
>>>>>>
>>>>>> It shows correct formatting in my client, and there's no warning 
>>>>>> in checkpatch for some reason
>>>>>
>>>>> Ignore this, sorry.
>>>>>
>>>>>>
>>>>>>
>>>>>>>
>>>>>>>> ? source "board/sifive/fu540/Kconfig"
>>>>>>>> ? source "board/sipeed/maix/Kconfig"
>>>>>>>> ? diff --git a/arch/riscv/dts/Makefile b/arch/riscv/dts/Makefile
>>>>>>>> index 3a6f96c6..b511cd74 100644
>>>>>>>> --- a/arch/riscv/dts/Makefile
>>>>>>>> +++ b/arch/riscv/dts/Makefile
>>>>>>>> @@ -1,6 +1,7 @@
>>>>>>>> ? # SPDX-License-Identifier: GPL-2.0+
>>>>>>>> ??? dtb-$(CONFIG_TARGET_AX25_AE350) += ae350_32.dtb ae350_64.dtb
>>>>>>>> +dtb-$(CONFIG_TARGET_OPENPITON_RISCV) += openpiton-riscv64.dtb
>>>>>>>> ? dtb-$(CONFIG_TARGET_SIFIVE_FU540) += hifive-unleashed-a00.dtb
>>>>>>>> ? dtb-$(CONFIG_TARGET_SIPEED_MAIX) += k210-maix-bit.dtb
>>>>>>>> ? diff --git a/arch/riscv/dts/openpiton-riscv64.dts 
>>>>>>>> b/arch/riscv/dts/openpiton-riscv64.dts
>>>>>>>> new file mode 100644
>>>>>>>> index 00000000..ce732b92
>>>>>>>> --- /dev/null
>>>>>>>> +++ b/arch/riscv/dts/openpiton-riscv64.dts
>>>>>>>> @@ -0,0 +1,159 @@
>>>>>>>> +// SPDX-License-Identifier: (GPL-2.0 OR MIT)
>>>>>>>> +/* Copyright (c) 2021 Tianrui Wei <tianrui-wei@outlook.com> */
>>>>>>>> +
>>>>>>>> +/*
>>>>>>>> + * This dts is for a dual core instance of OpenPiton+Ariane built
>>>>>>>> + * to run on a Digilent Genesys 2 FPGA at 66.67MHz. These files
>>>>>>>> + * are automatically generated by the OpenPiton build system and
>>>>>>>> + * this configuration may not be what you need if your 
>>>>>>>> configuration
>>>>>>>> + * is different from the below.
>>>>>>>> + */
>>>>>>>> +
>>>>>>>> +/dts-v1/;
>>>>>>>> +
>>>>>>>> +/ {
>>>>>>>> +??? #address-cells = <2>;
>>>>>>>> +??? #size-cells = <2>;
>>>>>>>> +??? u-boot,dm-pre-reloc;
>>>>>>>> +??? compatible = "openpiton,ariane";
>>>>>>>> +
>>>>>>>> +??? chosen {
>>>>>>>> +?????? stdout-path = "uart0:115200";
>>>>>>>> +??? };
>>>>>>>> +
>>>>>>>> +??? aliases {
>>>>>>>> +??????? console = &uart0;
>>>>>>>> +??????? serial0 = &uart0;
>>>>>>>> +??? };
>>>>>>>> +
>>>>>>>> +??? cpus {
>>>>>>>> +??????? #address-cells = <1>;
>>>>>>>> +??????? #size-cells = <0>;
>>>>>>>> +??????? u-boot,dm-pre-reloc;
>>>>>>>
>>>>>>> This is unnecessary since riscv_cpu has DM_FLAG_PRE_RELOC.
>>>>>>>
>>>>>>>> +??????? timebase-frequency = <520835>;
>>>>>>>> +
>>>>>>>> +??????? CPU0: cpu at 0 {
>>>>>>>> +??????????? clock-frequency = <66667000>;
>>>>>>>
>>>>>>> Please add a clocks node. If you just have one clock for the 
>>>>>>> whole soc,
>>>>>>> you can always use a fixed-clock binding.
>>>>>>>
>>>>>>>> + u-boot,dm-pre-reloc;
>>>>>>>> +??????????? device_type = "cpu";
>>>>>>>> +??????????? reg = <0>;
>>>>>>>> +??????????? status = "okay";
>>>>>>>> +??????????? compatible = "eth, ariane", "riscv";
>>>>>>>
>>>>>>> There should be no space after the comma. And what is "eth"? 
>>>>>>> Should this
>>>>>>> be lowrisc?
>>>>
>>>>
>>>> Oh eth was ETH Zurich, who developed ariane riscv64 CPU.
>
> As I understand it, ETH is an abbreviation like 'UC' in 'UC Berkeley'.
> So perhaps a better compatible string would be 'zurich,ariane' or
> 'eth-zurich,ariane'.


Thank you for your feed back. We plan to change it to openhwgroup, cva6 
as it's new rebranded name


>
>>>>
>>>>
>>>>>>>
>>>>>>>> +??????????? riscv,isa = "rv64imafdc";
>>>>>>>> +??????????? mmu-type = "riscv,sv39";
>>>>>>>> +??????????? tlb-split;
>>>>>>>> +??????????? // HLIC - hart local interrupt controller
>>>>>>>> +??????????? CPU0_intc: interrupt-controller {
>>>>>>>> +??????????????? #interrupt-cells = <1>;
>>>>>>>> +??????????????? interrupt-controller;
>>>>>>>> +??????????????? compatible = "riscv,cpu-intc";
>>>>>>>> +??????????? };
>>>>>>>> +??????? };
>>>>>>>> +
>>>>>>>> +??????? CPU1: cpu at 1 {
>>>>>>>
>>>>>>> Same comments as above.
>>>>>>>
>>>>>>>> +??????????? clock-frequency = <66667000>;
>>>>>>>> +??????????? u-boot,dm-pre-reloc;
>>>>>>>> +??????????? device_type = "cpu";
>>>>>>>> +??????????? reg = <1>;
>>>>>>>> +??????????? status = "okay";
>>>>>>>> +??????????? compatible = "eth, ariane", "riscv";
>>>>>>>> +??????????? riscv,isa = "rv64imafdc";
>>>>>>>> +??????????? mmu-type = "riscv,sv39";
>>>>>>>> +??????????? tlb-split;
>>>>>>>> +??????????? // HLIC - hart local interrupt controller
>>>>>>>> +??????????? CPU1_intc: interrupt-controller {
>>>>>>>> +??????????????? #interrupt-cells = <1>;
>>>>>>>> +??????????????? interrupt-controller;
>>>>>>>> +??????????????? compatible = "riscv,cpu-intc";
>>>>>>>> +??????????? };
>>>>>>>> +??????? };
>>>>>>>> +
>>>>>>>> +??? };
>>>>>>>> +
>>>>>>>> +??? memory at 80000000 {
>>>>>>>> +??????? u-boot,dm-pre-reloc;
>>>>>>>> +??????? device_type = "memory";
>>>>>>>> +??????? reg = < 0x00000000 0x80000000 0x00000000 0x40000000 >;
>>>>>>>> +??? };
>>>>>>>> +
>>>>>>>
>>>>>>> Everything below this line should be under a soc node.
>>>>>>
>>>>>>
>>>>>> Will do
>>>>>>
>>>>>>
>>>>>>>
>>>>>>>> +??? uart0: uart at fff0c2c000 {
>>>>>>>> +??????? u-boot,dm-pre-reloc;
>>>>>>>
>>>>>>> This is unnecessary. ns16550_serial has DM_FLAG_PRE_RELOC.
>>>>>>>
>>>>>>>> +??????? compatible = "ns16550";
>>>>>>>> +??????? reg = < 0x000000ff 0xf0c2c000 0x00000000 0x000d4000 >;
>>>>>>>> +??????? clock-frequency = <66667000>;
>>>>>>>
>>>>>>> Please add a clocks 
>>>>>>> node.Documentation/devicetree/bindings/serial/serial.yaml
>>>>>>>
>>>>>>>> +??????? current-speed = <115200>;
>>>>>>>
>>>>>>> This is only necessary if the baud rate cannot otherwise be 
>>>>>>> determined.
>>>>>>> Please remove this.
>>>>>>
>>>>>>
>>>>>> Oh this was necessary to hard code
>>>>>
>>>>> Why is that? It should be possible to determine the current baud rate
>>>>> from the input clock and the divider.
>>>>
>>>>
>>>> That makes sense, will do.
>>>>
>>>>
>>>>>
>>>>>>
>>>>>>
>>>>>>>
>>>>>>>> +??????? interrupt-parent = <&PLIC0>;
>>>>>>>
>>>>>>> Please specify this under /soc.
>>>>>>>
>>>>>>>> +??????? interrupts = <1>;
>>>>>>>> +??????? reg-shift = <0>;
>>>>>>>> +??????? // regs are spaced on 8 bit boundary
>>>>>>>> +??? };
>>>>>>>> +
>>>>>>>> +??? eth: ethernet at fff0d00000 {
>>>>>>>> +??????? compatible = "xlnx,xps-ethernetlite-1.00.a";
>>>>>>>> +??????? device_type = "network";
>>>>>>>> +??????? reg = < 0x000000ff 0xf0d00000 0x00000000 0x00100000 >;
>>>>>>>> +??????? interrupt-parent = <&PLIC0>;
>>>>>>>> +??????? interrupts = <2>;
>>>>>>>> +??????? local-mac-address = [ 00 18 3E 02 E3 E5 ];
>>>>>>>> +??????? phy-handle = <&phy0>;
>>>>>>>> +??????? xlnx,duplex = <0x1>;
>>>>>>>> +??????? xlnx,include-global-buffers = <0x1>;
>>>>>>>> +??????? xlnx,include-internal-loopback = <0x0>;
>>>>>>>> +??????? xlnx,include-mdio = <0x1>;
>>>>>>>> +??????? xlnx,rx-ping-pong = <0x1>;
>>>>>>>> +??????? xlnx,s-axi-id-width = <0x1>;
>>>>>>>> +??????? xlnx,tx-ping-pong = <0x1>;
>>>>>>>> +??????? xlnx,use-internal = <0x0>;
>>>>>>>> +??????? axi_ethernetlite_0_mdio: mdio {
>>>>>>>> +??????????? #address-cells = <1>;
>>>>>>>> +??????????? #size-cells = <0>;
>>>>>>>> +??????????? phy0: phy at 1 {
>>>>>>>> +??????????????? compatible = "ethernet-phy-id001C.C915";
>>>>>>>> +??????????????? device_type = "ethernet-phy";
>>>>>>>> +??????????????? reg = <1>;
>>>>>>>> +??????????? };
>>>>>>>> +??????? };
>>>>>>>> +??? };
>>>>>>>> +
>>>>>>>> +??? debug-controller at fff1000000 {
>>>>>>>> +??????? compatible = "riscv,debug-013";
>>>>>>>
>>>>>>> Is this binding needed? Will U-Boot/Linux/whoever need to 
>>>>>>> initialize
>>>>>>> this device?
>>>>>>
>>>>>>
>>>>>> I think
>>>>>
>>>>> Well, for the moment there is no driver for this in U-Boot or Linux.
>>>>
>>>>
>>>> Oh then I must be mistaken, will remove.
>>>>
>>>>
>>>>>
>>>>>>
>>>>>>
>>>>>>>
>>>>>>>> +??????? interrupts-extended = < &CPU0_intc 65535
>>>>>>>> +??????????????????????????????? &CPU1_intc 65535 >;
>>>>>>>
>>>>>>> Please align these.
>>>>>>>
>>>>>>>> +??????? reg = < 0x000000ff 0xf1000000 0x00000000 0x00001000 >;
>>>>>>>> +??????? reg-names = "control";
>>>>>>>> +??? };
>>>>>>>> +
>>>>>>>> +??? sdhci_0: sdhci at 0xf000000000 {
>>>>>>>
>>>>>>> No 0x prefix please.
>>>>>>>
>>>>>>>> +??????? u-boot,dm-pre-reloc;
>>>>>>>
>>>>>>> Why does this need to be pre-reloc?
>>>>>>
>>>>>>
>>>>>> We're using it in SPL
>>>>>
>>>>> Then please use u-boot,dm-spl
>>>>
>>>>
>>>> Will do
>>>>
>>>>
>>>>>
>>>>>>
>>>>>>
>>>>>>>
>>>>>>>> +??????? status = "okay";
>>>>>>>> +??????? compatible = "openpiton,piton-mmc";
>>>>>>>> +??????? reg = < 0x000000f0 0x00000000 0x00000000 0x00300000 >;
>>>>>>>> +??? };
>>>>>>>
>>>>>>> Missing newline.
>>>>>>>
>>>>>>>> +??? clint at fff1020000 {
>>>>>>>> +??????? u-boot,dm-pre-reloc;
>>>>>>>
>>>>>>> Again, unnecessary since the driver has the flag set.
>>>>>>>
>>>>>>>> +??????? compatible = "sifive,clint0";
>>>>>>>> +??????? interrupts-extended = < &CPU0_intc 3
>>>>>>>> +??????????????????????????????? &CPU0_intc 7
>>>>>>>> +??????????????????????????????? &CPU1_intc 3
>>>>>>>> +??????????????????????????????? &CPU1_intc 7 >;
>>>>>>>
>>>>>>> Please align this.
>>>>>>>
>>>>>>>> +??????? reg = < 0x000000ff 0xf1020000 0x00000000 0x000c0000 >;
>>>>>>>> +??????? reg-names = "control";
>>>>>>>
>>>>>>> Please add a clocks property.
>>>>>>>
>>>>>>>> +??? };
>>>>>>>> +
>>>>>>>> +??? PLIC0: plic at fff1100000 {
>>>>>>>> +??????? u-boot,dm-pre-reloc;
>>>>>>>
>>>>>>> Again, unnecessary.
>>>>>>>
>>>>>>>> +??????? #address-cells = <0>;
>>>>>>>
>>>>>>> Remove this.
>>>>>>>
>>>>>>>> +??????? #interrupt-cells = <1>;
>>>>>>>> +??????? compatible = "sifive,plic-1.0.0";
>>>>>>>> +??????? interrupt-controller;
>>>>>>>> +??????? interrupts-extended = < &CPU0_intc 11
>>>>>>>> +??????????????????????????????? &CPU0_intc 9
>>>>>>>> +??????????????????????????????? &CPU1_intc 11
>>>>>>>> +??????????????????????????????? &CPU1_intc 9 >;
>>>>>>>
>>>>>>> Align these please.
>>>>>>>
>>>>>>>> +??????? reg = < 0x000000ff 0xf1100000 0x00000000 0x04000000 >;
>>>>>>>> +??????? riscv,max-priority = <7>;
>>>>>>>> +??????? riscv,ndev = <2>;
>>>>>>>> +??? };
>>>>>>>
>>>>>>> In general, please add soc-specific compatible strings (e.g.
>>>>>>> "openpiton,ns16550") to allow for forward compatibility.
>>>>>>>
>>>>>>>> +
>>>>>>>> +};
>>>>>>>> +
>>>>>>>> diff --git a/board/openpiton/riscv/Kconfig 
>>>>>>>> b/board/openpiton/riscv/Kconfig
>>>>>>>> new file mode 100644
>>>>>>>> index 00000000..31ae44d5
>>>>>>>> --- /dev/null
>>>>>>>> +++ b/board/openpiton/riscv/Kconfig
>>>>>>>> @@ -0,0 +1,42 @@
>>>>>>>> +if TARGET_OPENPITON_RISCV
>>>>>>>> +
>>>>>>>> +config SYS_BOARD
>>>>>>>> +??? default "riscv"
>>>>>>>> +
>>>>>>>> +config SYS_VENDOR
>>>>>>>> +??? default "openpiton"
>>>>>>>> +
>>>>>>>> +config SYS_CPU
>>>>>>>> +??? default "generic"
>>>>>>>> +
>>>>>>>> +config SYS_CONFIG_NAME
>>>>>>>> +??? default "openpiton-riscv"
>>>>>>>> +
>>>>>>>> +config SYS_TEXT_BASE
>>>>>>>> +??? default 0x81000000 if SPL
>>>>>>>> +??? default 0x80000000 if !RISCV_SMODE
>>>>>>>> +??? default 0x81000000 if RISCV_SMODE
>>>>>>>> +
>>>>>>>> +config SPL_TEXT_BASE
>>>>>>>> +??? default 0x80000000
>>>>>>>> +
>>>>>>>> +config SPL_OPENSBI_LOAD_ADDR
>>>>>>>> +??? default 0x81000000
>>>>>>>> +
>>>>>>>> +config BOARD_SPECIFIC_OPTIONS # dummy
>>>>>>>> +??? def_bool y
>>>>>>>> +??? select ARCH_EARLY_INIT_R
>>>>>>>> +??? select SUPPORT_SPL
>>>>>>>> +??? imply CPU_RISCV
>>>>>>>> +??? imply RISCV_TIMER if (RISCV_SMODE || SPL_RISCV_SMODE)
>>>>>>>> +??? imply SIFIVE_CLINT if (RISCV_MMODE || SPL_RISCV_MMODE)
>>>>>>>> +??? imply CMD_CPU
>>>>>>>> +??? imply SPL_CPU_SUPPORT
>>>>>>>> +??? imply SPL_OPENSBI
>>>>>>>> +??? imply SPL_LOAD_FIT
>>>>>>>> +??? imply SPL_SMP
>>>>>>>> +??? imply SPL_MMC
>>>>>>>
>>>>>>> Fix indentation please. Did you run checkpatch?
>>>>>>>
>>>>>>>> +??? imply SMP
>>>>>>>> +??? imply SPL_RISCV_MMODE
>>>>>>>> +
>>>>>>>> +endif
>>>>>>>> diff --git a/board/openpiton/riscv/MAINTAINERS 
>>>>>>>> b/board/openpiton/riscv/MAINTAINERS
>>>>>>>> new file mode 100644
>>>>>>>> index 00000000..1db6fb60
>>>>>>>> --- /dev/null
>>>>>>>> +++ b/board/openpiton/riscv/MAINTAINERS
>>>>>>>> @@ -0,0 +1,6 @@
>>>>>>>> +Openpiton Riscv Bootloader
>>>>>>>
>>>>>>> OpenPiton BOARD
>>>>>>>
>>>>>>>> +M:??? Tianrui Wei<tianrui-wei@outlook.com>
>>>>>>>> +S:??? Maintained
>>>>>>>> +F:??? board/openpiton/riscv/
>>>>>>>
>>>>>>> Will there be non-riscv openpiton boards?
>>>>>>>
>>>>>>> In general, the directory layout here is
>>>>>>>
>>>>>>> board/manufacturer/board_name
>>>>>>>
>>>>>>> So who manufactures OpenPiton?
>>>>>>
>>>>>>
>>>>>> There will probably be, yes.
>>>>>>
>>>>>>
>>>>>> OpenPiton is a community project, and will continue to support 
>>>>>> other isas in the future ( already a few other supported now )
>>>>>>
>>>>>>
>>>>>>>
>>>>>>>> +F: include/configs/openpiton-riscv.h
>>>>>>>> +F:??? configs/openpiton_riscv_defconfig
>>>>>>>> diff --git a/board/openpiton/riscv/Makefile 
>>>>>>>> b/board/openpiton/riscv/Makefile
>>>>>>>> new file mode 100644
>>>>>>>> index 00000000..8cc20e7c
>>>>>>>> --- /dev/null
>>>>>>>> +++ b/board/openpiton/riscv/Makefile
>>>>>>>> @@ -0,0 +1,5 @@
>>>>>>>> +# SPDX-License-Identifier: GPL-2.0+
>>>>>>>> +#
>>>>>>>> +# Copyright (C) 2021 Tianrui Wei
>>>>>>>> +# Tianrui Wei <tianrui-wei@outlook.com>
>>>>>>>> +obj-y += openpiton-riscv.o
>>>>>>>> diff --git a/board/openpiton/riscv/openpiton-riscv.c 
>>>>>>>> b/board/openpiton/riscv/openpiton-riscv.c
>>>>>>>> new file mode 100644
>>>>>>>> index 00000000..5be407e7
>>>>>>>> --- /dev/null
>>>>>>>> +++ b/board/openpiton/riscv/openpiton-riscv.c
>>>>>>>> @@ -0,0 +1,41 @@
>>>>>>>> +// SPDX-License-Identifier: GPL-2.0+
>>>>>>>> +/*
>>>>>>>> + * Copyright (c) 2019 SiFive, Inc
>>>>>>>> + * Copyright (c) 2021 Tianrui Wei
>>>>>>>> + *
>>>>>>>> + *
>>>>>>>> + * Authors:
>>>>>>>> + *?? Pragnesh Patel <pragnesh.patel@sifive.com>
>>>>>>>> + *?? Tianrui Wei <tianrui-wei@outlook.com>
>>>>>>>> + */
>>>>>>>> +#include <common.h>
>>>>>>>> +#include <init.h>
>>>>>>>> +#include <configs/openpiton-riscv.h>
>>>>>>>> +#include <dm.h>
>>>>>>>> +#include <spl.h>
>>>>>>>> +
>>>>>>>> +#ifdef CONFIG_SPL
>>>>>>>> +void board_boot_order(u32 *spl_boot_list)
>>>>>>>> +{
>>>>>>>> +??? u8 i;
>>>>>>>> +??? u32 boot_devices[] = {
>>>>>>>> +??????? BOOT_DEVICE_MMC1,
>>>>>>>> +??? };
>>>>>>>> +
>>>>>>>> +??? for (i = 0; i < ARRAY_SIZE(boot_devices); i++)
>>>>>>>> +??????? spl_boot_list[i] = boot_devices[i];
>>>>>>>> +}
>>>>>>>> +#endif
>>>>>>>> +
>>>>>>>> +#ifdef CONFIG_SPL_LOAD_FIT
>>>>>>>> +int board_fit_config_name_match(const char *name)
>>>>>>>> +{
>>>>>>>> +??? /* boot using first FIT config */
>>>>>>>> +??? return 0;
>>>>>>>> +}
>>>>>>>> +#endif
>>>>>>>> +
>>>>>>>> +int board_init(void)
>>>>>>>> +{
>>>>>>>> +??????? return 0;
>>>>>>>> +}
>>>>>>>
>>>>>>> Do you need get_effective_memsize?
>>>>>>>
>>>>>>>> diff --git a/configs/openpiton_riscv64_defconfig 
>>>>>>>> b/configs/openpiton_riscv64_defconfig
>>>>>>>> new file mode 100644
>>>>>>>> index 00000000..37aa3c80
>>>>>>>> --- /dev/null
>>>>>>>> +++ b/configs/openpiton_riscv64_defconfig
>>>>>>>> @@ -0,0 +1,132 @@
>>>>>>>> +CONFIG_RISCV=y
>>>>>>>> +CONFIG_NR_DRAM_BANKS=1
>>>>>>>> +CONFIG_SPL=y
>>>>>>>> +CONFIG_SPL_MMC_SUPPORT=y
>>>>>>>> +CONFIG_DEFAULT_DEVICE_TREE="openpiton-riscv64"
>>>>>>>> +CONFIG_TARGET_OPENPITON_RISCV=y
>>>>>>>> +CONFIG_ARCH_RV64I=y
>>>>>>>> +CONFIG_RISCV_SMODE=y
>>>>>>>> +CONFIG_MISC_INIT_R=n
>>>>>>>> +CONFIG_SYS_MALLOC_F_LEN=0x1000
>>>>>>>> +CONFIG_SPL_SYS_MALLOC_F_LEN=0x100000
>>>>>>>> +CONFIG_SPL_PAYLOAD=""
>>>>>>>> +CONFIG_LIBDISK_SUPPORT=y
>>>>>>>> +CONFIG_NR_CPUS=32
>>>>>>>> +CONFIG_CMODEL_MEDANY=y
>>>>>>>> +CONFIG_EXPORT=n
>>>>>>>> +CONFIG_HASH=n
>>>>>>>> +CONFIG_SPL_SYS_MALLOC_SIMPLE=y
>>>>>>>> +CONFIG_SPL_BANNER_PRINT=n
>>>>>>>> +CONFIG_SPL_CRC32_SUPPORT=n
>>>>>>>> +CONFIG_GENERATE_SMBIOS_TABLE=n
>>>>>>>> +CONFIG_EFI_LOADER=n
>>>>>>>> +CONFIG_LIB_DATA=y
>>>>>>>> +CONFIG_SPL_USE_TINY_PRINTF=y
>>>>>>>> +CONFIG_SPL_TINY_MEMSET=y
>>>>>>>> +CONFIG_FS_EXT4=y
>>>>>>>> +CONFIG_DM_RTC=y
>>>>>>>> +CONFIG_SYS_NS16550=y
>>>>>>>> +CONFIG_SPL_OPENSBI=n
>>>>>>>> +CONFIG_SPL_RTC_SUPPORT=y
>>>>>>>> +CONFIG_SPL_FS_EXT4=y
>>>>>>>> +CONFIG_SPL_LEGACY_IMAGE_SUPPORT=n
>>>>>>>> +CONFIG_CMD_NET=n
>>>>>>>> +CONFIG_EFI_PARTITION=y
>>>>>>>> +CONFIG_EFI_PARTITION_ENTRIES_NUMBERS=128
>>>>>>>> +CONFIG_EFI_PARTITION_ENTRIES_OFF=0
>>>>>>>> +CONFIG_SPL_EFI_PARTITION=y
>>>>>>>> +CONFIG_PARTITION_UUIDS=y
>>>>>>>> +CONFIG_OF_EMBED=y
>>>
>>> This is not allowed for mainline boards. Please use OF_SEPARATE (for
>>> M-mode) or OF_PRIOR_STAGE (for S-mode).
>>
>>
>> Interesting. Does it work for SPL also?
>
> For SPL two binaries are produced, one with a DTB and one without. You
> can use either in whatever build step follows.


Thank you. As long as SPL has an embedded dtb, we're fine.


>
>>
>>
>>>
>>>>>>>> +CONFIG_HAVE_BLOCK_DEVICE=y
>>>>>>>> +CONFIG_CPU=y
>>>>>>>> +CONFIG_CPU_RISCV=y
>>>>>>>> +CONFIG_MMC=y
>>>>>>>> +CONFIG_DM_MMC=y
>>>>>>>> +CONFIG_SPL_DM_MMC=y
>>>>>>>> +CONFIG_MMC_PITON=y
>>>>>>>> +CONFIG_MMC_QUIRKS=y
>>>>>>>> +CONFIG_DEBUG_UART=y
>>>>>>>> +CONFIG_DEBUG_UART_NS16550=y
>>>>>>>> +CONFIG_DEBUG_UART_SHIFT=0
>>>>>>>> +CONFIG_DEBUG_UART_ANNOUNCE=y
>>>>>>>> +CONFIG_DEBUG_UART_SKIP_INIT=y
>>>>>>>> +CONFIG_DEBUG_UART_BASE=0xfff0c2c000
>>>>>>>> +CONFIG_DEBUG_UART_CLOCK=66667000
>>>>>>>> +CONFIG_SPL_LIBDISK_SUPPORT=y
>>>>>>>> +CONFIG_MMC_VERBOSE=n
>>>>>>>> +CONFIG_MMC_WRITE=n
>>>>>>>> +CONFIG_MMC_HW_PARTITIONING=n
>>>>>>>> +CONFIG_SYS_RELOC_GD_ENV_ADDR=n
>>>>>>>> +CONFIG_SAVEENV=n
>>>>>>>> +CONFIG_NET=n
>>>>>>>> +CONFIG_SPL_PARTITION_UUIDS=n
>>>>>>>> +CONFIG_CMD_EXT4=y
>>>>>>>> +CONFIG_CMD_FS_GENERIC=y
>>>>>>>> +CONFIG_CMD_READ=y
>>>>>>>> +CONFIG_CMD_LSBLK=y
>>>>>>>> +CONFIG_CMD_MMC=y
>>>>>>>> +CONFIG_CMD_GPT=y
>>>>>>>> +CONFIG_CMD_MEMINFO=y
>>>>>>>> +CONFIG_EXPERT=n
>>>>>>>> +CONFIG_ENV_VARS_UBOOT_CONFIG=y
>>>>>>>> +CONFIG_LEGACY_IMAGE_FORMAT=n
>>>>>>>> +CONFIG_ARCH_FIXUP_FDT_MEMORY=n
>>>>>>>> +CONFIG_MENU=y
>>>>>>>> +CONFIG_CMD_BOOTZ=y
>>>>>>>> +CONFIG_CMD_PART=y
>>>>>>>> +CONFIG_SHOW_REGS=y
>>>>>>>> +CONFIG_LOG=y
>>>>>>>> +CONFIG_LOGLEVEL=9
>>>>>>>> +CONFIG_SPL_LOGLEVEL=9
>>>>>>>> +CONFIG_TPL_LOGLEVEL=9
>>>>>>>> +CONFIG_SPL_LOG=y
>>>>>>>> +CONFIG_SPL_LOG_MAX_LEVEL=9
>>>>>>>> +CONFIG_SPL_LOG_CONSOLE=y
>>>>>>>> +CONFIG_LOG_ERROR_RETURN=y
>>>>>>>> +CONFIG_CMD_CPU=n
>>>>>>>> +CONFIG_BOOTM_NETBSD=n
>>>>>>>> +CONFIG_BOOTM_PLAN9=n
>>>>>>>> +CONFIG_BOOTM_RTEMS=n
>>>>>>>> +CONFIG_BOOTM_VXWORKS=n
>>>>>>>> +CONFIG_CMD_RUN=n
>>>>>>>> +CONFIG_CMD_IMI=n
>>>>>>>> +CONFIG_CMD_XIMG=n
>>>>>>>> +CONFIG_CMD_EXPORTENV=n
>>>>>>>> +CONFIG_CMD_IMPORTENV=n
>>>>>>>> +CONFIG_CMD_EDITENV=n
>>>>>>>> +CONFIG_CMD_SAVEENV=n
>>>>>>>> +CONFIG_CMD_CRC32=n
>>>>>>>> +CONFIG_CMD_RANDOM=n
>>>>>>>> +CONFIG_CMD_LZMADEC=n
>>>>>>>> +CONFIG_CMD_UNLZ4=n
>>>>>>>> +CONFIG_CMD_UNZIP=n
>>>>>>>> +CONFIG_CMD_FLASH=n
>>>>>>>> +CONFIG_RANDOM_UUID=n
>>>>>>>> +CONFIG_CMD_LOADB=n
>>>>>>>> +CONFIG_CMD_LOADS=n
>>>>>>>> +CONFIG_CMD_ECHO=n
>>>>>>>> +CONFIG_CMD_ITEST=n
>>>>>>>> +CONFIG_CMD_SOURCE=n
>>>>>>>> +CONFIG_CMD_SETEXPR=n
>>>>>>>> +CONFIG_CMD_BLOCK_CACHE=n
>>>>>>>> +CONFIG_CMD_DATE=n
>>>>>>>> +CONFIG_CMD_SLEEP=n
>>>>>>>> +CONFIG_CMD_SYSBOOT=y
>>>>>>>> +CONFIG_CMD_FAT=y
>>>>>>>> +CONFIG_DOS_PARTITION=y
>>>>>>>> +CONFIG_ISO_PARTITION=y
>>>>>>>> +CONFIG_DM_ETH=y
>>>>>>>> +CONFIG_RAM=y
>>>>>>>> +CONFIG_SPL_RAM=y
>>>>>>>> +CONFIG_FS_FAT=y
>>>>>>>> +CONFIG_FS_FAT_MAX_CLUSTSIZE=65536
>>>>>>>> +CONFIG_FS_SQUASHFS=y
>>>>>>>> +CONFIG_SHA1=y
>>>>>>>> +CONFIG_SHA256=y
>>>>>>>> +CONFIG_MD5=y
>>>>>>>> +CONFIG_ZLIB_UNCOMPRESS=y
>>>>>>>> +CONFIG_SPL_GZIP=y
>>>>>>>> +CONFIG_SPL_ZLIB=y
>>>>>>>> +CONFIG_GETOPT=y
>>>>>>>> +CONFIG_OF_LIBFDT_OVERLAY=y
>>>>>>>> +CONFIG_RAM_SIFIVE=n
>>>>>>>
>>>>>>> Did you generate this with "make savedefconfig"?
>>>>>>>
>>>>>>>> diff --git a/doc/board/index.rst b/doc/board/index.rst
>>>>>>>> index 915f1be8..51e60ac4 100644
>>>>>>>> --- a/doc/board/index.rst
>>>>>>>> +++ b/doc/board/index.rst
>>>>>>>> @@ -17,6 +17,7 @@ Board-specific doc
>>>>>>>> ???? google/index
>>>>>>>> ???? intel/index
>>>>>>>> ???? kontron/index
>>>>>>>> +?? openpiton/index
>>>>>>>
>>>>>>> Fix indentation
>>>>>>>
>>>>>>>> ???? renesas/index
>>>>>>>> ???? rockchip/index
>>>>>>>> ???? sifive/index
>>>>>>>> diff --git a/doc/board/openpiton/index.rst 
>>>>>>>> b/doc/board/openpiton/index.rst
>>>>>>>> new file mode 100644
>>>>>>>> index 00000000..c469102c
>>>>>>>> --- /dev/null
>>>>>>>> +++ b/doc/board/openpiton/index.rst
>>>>>>>> @@ -0,0 +1,9 @@
>>>>>>>> +.. SPDX-License-Identifier: GPL-2.0+
>>>>>>>> +
>>>>>>>> +OpenPiton
>>>>>>>> +=========
>>>>>>>> +
>>>>>>>> +.. toctree::
>>>>>>>> +?? :maxdepth: 2
>>>>>>>> +
>>>>>>>> +?? riscv64
>>>>>>>> diff --git a/doc/board/openpiton/riscv64.rst 
>>>>>>>> b/doc/board/openpiton/riscv64.rst
>>>>>>>> new file mode 100644
>>>>>>>> index 00000000..dc934bb4
>>>>>>>> --- /dev/null
>>>>>>>> +++ b/doc/board/openpiton/riscv64.rst
>>>>>>>> @@ -0,0 +1,885 @@
>>>>>>>> +.. SPDX-License-Identifier: GPL-2.0+
>>>>>>>> +
>>>>>>>> +Openpiton RISC-V SoC
>>>>>>>> +====================
>>>>>>>> +
>>>>>>>> +OpenPiton RISC-V SoC
>>>>>>>> +--------------------
>>>>>>>> +OpenPiton is an open source, manycore processor and research 
>>>>>>>> platform. It is a tiled manycore framework scalable from
>>>>>>>> one to 1/2 billion cores. It supports a number of ISAs 
>>>>>>>> including RISC-V with its P-Mesh cache coherence protocol and
>>>>>>>> networks on chip. It is highly configurable in both core and 
>>>>>>>> uncore components. OpenPiton has been verified in both ASIC
>>>>>>>> and multiple Xilinx FPGA prototypes running full-stack Debian 
>>>>>>>> linux.
>>>>>>>> +
>>>>>>>> +RISCV-V Standard Bootflow
>>>>>>>> +-------------------------
>>>>>>>> +Currently, OpenPiton implements RISC-V standard bootflow in 
>>>>>>>> the following steps
>>>>>>>> +mover.S -> u-boot-spl -> opensbi -> u-boot -> Linux
>>>>>>>> +This board supports S-mode u-boot as well as M-mode SPL
>>>>>>>> +
>>>>>>>> +Building OpenPition
>>>>>>>> +---------------------
>>>>>>>> +If you'd like to build OpenPiton, please go to OpenPiton 
>>>>>>>> github repo to build from the latest changes
>>>>>>>
>>>>>>> Please link to the github.
>>>>>>
>>>>>>
>>>>>> Will do
>>>>>>
>>>>>>
>>>>>>>
>>>>>>>> +
>>>>>>>> +Building Images
>>>>>>>> +---------------------------
>>>>>>>> +
>>>>>>>> +SPL
>>>>>>>> +---
>>>>>>>> +
>>>>>>>> +1. Add the RISC-V toolchain to your PATH.
>>>>>>>> +2. Setup ARCH & cross compilation environment variable:
>>>>>>>> +
>>>>>>>> +.. code-block:: none
>>>>>>>> +
>>>>>>>> +?? export CROSS_COMPILE=<riscv64 toolchain prefix>
>>>>>>>> +?? export ARCH=riscv
>>>>>>>> +
>>>>>>>> +3. make openpiton_riscv64_defconfig
>>>>>>>> +4. make
>>>>>>>> +
>>>>>>>> +U-Boot
>>>>>>>> +------
>>>>>>>> +
>>>>>>>> +1. Add the RISC-V toolchain to your PATH.
>>>>>>>> +2. Setup ARCH & cross compilation environment variable:
>>>>>>>> +
>>>>>>>> +.. code-block:: none
>>>>>>>> +
>>>>>>>> +?? export CROSS_COMPILE=<riscv64 toolchain prefix>
>>>>>>>> +?? export ARCH=riscv
>>>>>>>> +
>>>>>>>> +3. make openpiton_riscv64_defconfig
>>>>>>>> +4. make menuconfig, then change CONFIG_SYS_TEXT_BASE to 
>>>>>>>> 0x81020000
>>>>>>>
>>>>>>> Why isn't this the default?
>>>>>>
>>>>>>
>>>>>> Because u-boot SPL will just to CONFIG)SYS_TEXT_BASE ( where 
>>>>>> opensbi is ). If I change that, it will just jump to the wrong 
>>>>>> location.
>>>>>
>>>>> Can you make this dependent on S-Mode?
>>>>
>>>>
>>>> I'm not sure making it dependent on S-mode helpful, because in 
>>>> essense we need to generate u-boot and u-boot spl in 2 passes, so 
>>>> it had to be different
>>>
>>> U-Boot and SPL should be produced in the same pass. See e.g. SiFive 
>>> Unleashed.
>>
>>
>> Hmm that's only possible because they're using FIT, whereas we aren't 
>> though.
>
> I don't see what that has to do with anything.
>

If we're using fit, SPL will jump to any address as indicated by fit.

If not however, SPL will jump to CONFIG_SYS_TEXT_BASE, which is the text 
base for u-boot proper


>>
>>
>>>
>>>>
>>>>
>>>>>
>>>>>>
>>>>>>
>>>>>>>
>>>>>>>> +5. make
>>>>>>>> +
>>>>>>>> +
>>>>>>>> +opensbi
>>>>>>>> +-------
>>>>>>>> +
>>>>>>>> +1. Add the RISC-V toolchain to your PATH.
>>>>>>>> +2. Setup ARCH & cross compilation environment variable:
>>>>>>>> +
>>>>>>>> +.. code-block:: none
>>>>>>>> +
>>>>>>>> +?? export CROSS_COMPILE=<riscv64 toolchain prefix>
>>>>>>>> +?? export ARCH=riscv
>>>>>>>> +
>>>>>>>> +3. Go to OpenSBI directory
>>>>>>>> +4. Edit platform/fpga/openpiton/config.mk, and change 
>>>>>>>> FW_TEXT_START to 0x81000000
>>>>>>>> +5. make PLATFORM=fpga/openpiton FW_PAYLOAD_PATH=<path to 
>>>>>>>> u-boot-nodtb.bin>
>>>>>>>> +
>>>>>>>> +
>>>>>>>> +Using fw_payload.bin with linux
>>>>>>>> +-------------------------------
>>>>>>>> +Put the generated fw_payload.bin into the /boot directory on 
>>>>>>>> the root filesystem, plug in the SD card, then flash the
>>>>>>>> bitstream. Linux will boot automatically.
>>>>>>>> +
>>>>>>>> +Booting
>>>>>>>> +-------
>>>>>>>> +Once you plugin the sdcard and power up, you should see the 
>>>>>>>> U-Boot prompt.
>>>>>>>> +
>>>>>>>> +Sample Dual-core Debian boot log from OpenPiton
>>>>>>>> +-----------------------------------------------
>>>>>>>> +
>>>>>>>> +.. code-block:: none
>>>>>>>> +
>>>>>>>> +? <debug_uart>
>>>>>>>
>>>>>>> Please use a log without debug uart.
>>>>>>
>>>>>>
>>>>>> So this is the part where it was a little confusing. Disabling 
>>>>>> debug uart acutally doesn't work for some reason, so we had to 
>>>>>> keep it open. Will submit another patch if we got it working with 
>>>>>> debug uart turned off.
>>>>>
>>>>> This is a bit of a strange request, but can you try adding some 
>>>>> nops()
>>>>> (around 10-30) to some function (e.g. board_init). I've been having
>>>>> alignment problems in k210, so it could be something similar.
>>>>
>>>>
>>>> Hmm that was a good idea. Thanks for the suggestion!
>>>>
>>>>
>>>>>
>>>>>>>
>>>>>>> diff --git a/include/configs/openpiton-riscv.h 
>>>>>>> b/include/configs/openpiton-riscv.h
>>>>>>>> new file mode 100644
>>>>>>>> index 00000000..0f30609b
>>>>>>>> --- /dev/null
>>>>>>>> +++ b/include/configs/openpiton-riscv.h
>>>>>>>> @@ -0,0 +1,58 @@
>>>>>>>> +/* SPDX-License-Identifier: GPL-2.0+ */
>>>>>>>> +/*
>>>>>>>> + * Copyright (c) 2019 Western Digital Corporation or its 
>>>>>>>> affiliates.
>>>>>>>> + * Copyright (c) 2021 Tianrui Wei
>>>>>>>> + *
>>>>>>>> + * Authors:
>>>>>>>> + *?? Anup Patel <anup.patel@wdc.com>
>>>>>>>> + *?? Tianrui Wei <tianrui-wei@outlook.com>
>>>>>>>> + */
>>>>>>>> +
>>>>>>>> +#ifndef __OPENPITON_RISCV_CONFIG_H
>>>>>>>> +#define __OPENPITON_RISCV_CONFIG_H
>>>>>>>> +
>>>>>>>> +#include <linux/sizes.h>
>>>>>>>> +#define DEBUG
>>>>>>>> +#ifdef CONFIG_SPL
>>>>>>>> +#define CONFIG_SPL_MAX_SIZE???? 0x00100000
>>>>>>>> +#define CONFIG_SPL_BSS_START_ADDR?? 0x82000000
>>>>>>>> +#define CONFIG_SPL_BSS_MAX_SIZE???? 0x00100000
>>>>>>>> +#define CONFIG_SYS_SPL_MALLOC_START (CONFIG_SPL_BSS_START_ADDR 
>>>>>>>> + \
>>>>>>>> +??????? CONFIG_SPL_BSS_MAX_SIZE)
>>>>>>>> +#define CONFIG_SYS_SPL_MALLOC_SIZE? 0x0100000
>>>>>>>> +#define CONFIG_SPL_STACK??? (0x80000000 + 0x04000000 - \
>>>>>>>
>>>>>>> Please use SZ_64M here, or just use an absolute address.
>>>>>>
>>>>>>
>>>>>> Will fix
>>>>>>
>>>>>>
>>>>>>>
>>>>>>>> + GENERATED_GBL_DATA_SIZE)
>>>>>>>> +
>>>>>>>> +#define CONFIG_SPL_FS_LOAD_PAYLOAD_NAME "boot/fw_payload.bin"
>>>>>>>> +#define CONFIG_SPL_GD_ADDR 0x85000000
>>>>>>>> +#endif
>>>>>>>> +
>>>>>>>> +/* Environment options */
>>>>>>>> +#define CONFIG_SYS_SDRAM_BASE 0x80000000
>>>>>>>> +#define CONFIG_SYS_INIT_SP_ADDR (CONFIG_SYS_SDRAM_BASE + SZ_2M)
>>>>>>>> +#define CONFIG_SYS_LOAD_ADDR (CONFIG_SYS_SDRAM_BASE + SZ_2M)
>>>>>>>> +#define CONFIG_SYS_MALLOC_LEN?????? SZ_256M
>>>>>>>> +#define CONFIG_SYS_BOOTM_LEN??????? SZ_256M
>>>>>>>> +
>>>>>>>> +/* -------------------------------------------------
>>>>>>>> + * Environment
>>>>>>>> + */
>>>>>>>> +//Disable persistent environment variable storage
>>>>>>>> +#define CONFIG_ENV_IS_NOWHERE?? 1
>>>>>>>> +
>>>>>>>> +/* 
>>>>>>>> --------------------------------------------------------------------- 
>>>>>>>>
>>>>>>>> + * Board boot configuration
>>>>>>>> + */
>>>>>>>> +
>>>>>>>> +#define CONFIG_EXTRA_ENV_SETTINGS "\0"
>>>>>>>
>>>>>>> Please define some of the raw addresses used below to make 
>>>>>>> modifying the
>>>>>>> boot process easier. For example,
>>>>>>>
>>>>>>> ??? fdt_addr_r=0x86000000
>>>>>>> ??? kernel_addr_r=0x80200000
>>>>>>> ??? image=Image
>>>>>>> ??? mmcdev=0
>>>>>>> ??? mmcpart=1
>>>>>>
>>>>>>
>>>>>> Good idea! Will do!
>>>>>>
>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>> +
>>>>>>>> +#define CONFIG_USE_BOOTCOMMAND
>>>>>>>> +#define CONFIG_BOOTCOMMAND \
>>>>>>>> +??? "fdt addr ${fdtcontroladdr}; " \
>>>>>>>> +??? "fdt move ${fdtcontroladdr} 0x86000000; " \
>>>>>>>> +??? "ext4load mmc 0:1 0x80200000 boot/Image; " \
>>>>>>>
>>>>>>> Can you use "load" from CONFIG_CMD_FS_GENERIC?
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>>>
>>>>>>>> +??? "booti 0x80200000 - 0x86000000; "
>>>>>>>
>>>>>>> Is there a bootm flow?
>>>>>>
>>>>>>
>>>>>> No we don't support fit yet for some hardware reasons.
>>>>>
>>>>> Can you elaborate on that? I wasn't aware of any restrictions in this
>>>>> area.
>>>>
>>>>
>>>> So OpenPiton is actually some kind of SoC generator that generates
>>>> different SoC on FPGA. The device tree was generated at bitstream
>>>> creation time to facilitate different configurations, so each board
>>>> can have different device trees. We're aware of any way to do this in
>>>> FIT.
>>>
>>> Oh, so you're saying that the devicetree is placed at a specific 
>>> address
>>> in hardware? Does it need any fixups?
>>
>>
>> No for now we just embed it in SPL, will be converted into the 
>> bootrom for the chip with a small ZSBL.
>
> Well, atm you are using the DTS embedded in U-Boot.
>
> I don't know what the correct way to do this is...
>
> +CC Bin, Heinrich: Do you have a comment on this?
>
>>
>> Also, now that you mention fixups u-boot spl doesn't work at high 
>> address like 0xffffff0000 for now. Is that the case?
>
> What happens why you try that?


It would simply fail to compile with some link errors


Many thanks for your valuable feedbacks!

Tianrui


>
> --Sean