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

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

On 5/8/2021 11:14 PM, Sean Anderson wrote:
> On 5/8/21 12:57 AM, Tianrui Wei wrote:
>> 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
>
> Ok, so use FIT. I believe the standard boot flow is to have SPL load a 
> FIT with OpenSBI and U-Boot.


I just realized for qemu, SPL and U-Boot was actually produced in 
separate defconfigs. Would that work here?


>
>>
>>
>>>>
>>>>
>>>>>
>>>>>>
>>>>>>
>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>>
>>>>>>>>>> +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
>
> Hm, I don't know the specific cause of that. You would have to post 
> the errors.


The exact error message is

common/built-in.o: in function `malloc_trim':
/u-boot/common/dlmalloc.c:2236:(.text.malloc_trim+0x7e): relocation 
truncated to fit: R_RISCV_PCREL_HI20 against `.LANCHOR0'
common/built-in.o: in function `free':
/u-boot/common/dlmalloc.c:1603:(.text.free+0x62): relocation truncated 
to fit: R_RISCV_PCREL_HI20 against `.LANCHOR3'
common/built-in.o: in function `malloc_extend_top':
/u-boot/common/dlmalloc.c:1124:(.text.malloc+0x284): relocation 
truncated to fit: R_RISCV_PCREL_HI20 against `.LANCHOR3'
scripts/Makefile.spl:463: recipe for target 'spl/u-boot-spl' failed
make[1]: *** [spl/u-boot-spl] Error 1
Makefile:1904: recipe for target 'spl/u-boot-spl' failed
make: *** [spl/u-boot-spl] Error 2
make: *** Waiting for unfinished jobs....

With the qemu_riscv64_spl_defconfig on u-boot HEAD, only changing 
CONFIG_SPL_TEXT_BASE to 0xfff00000000.

Thanks,

Tianrui


>
> --Sean
>
>>
>>
>> Many thanks for your valuable feedbacks!
>>
>> Tianrui
>>
>>
>>>
>>> --Sean
>
>