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

Tianrui Wei tianrui-wei at outlook.com
Wed May 12 18:37:39 CEST 2021


Hi Sean,

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 at 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 at outlook.com>
>>>>>>>>>> Signed-off-by: Jonathan Balkind <jbalkind at 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 at 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 at 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 at 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 at sifive.com>
>>>>>>>>>> + *   Tianrui Wei <tianrui-wei at 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.
>
>>
>>
>>>>
>>>>
>>>>>
>>>>>>
>>>>>>
>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>>
>>>>>>>>>> +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.


I was wondering if you have any idea what may cause the alignment 
problems, we're also hitting it constantly and adding nops seems to have 
no impact so far.


Thanks,

Tianrui


>>>>>>
>>>>>>
>>>>>> 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 at wdc.com>
>>>>>>>>>> + *   Tianrui Wei <tianrui-wei at 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.
>
> --Sean
>
>>
>>
>> Many thanks for your valuable feedbacks!
>>
>> Tianrui
>>
>>
>>>
>>> --Sean
>
>


More information about the U-Boot mailing list