On Wed, Dec 28, 2022 at 2:39 PM yanhong wang
<yanhong.w...@starfivetech.com> wrote:
>
>
>
> On 2022/12/26 21:54, Bin Meng wrote:
> > On Mon, Dec 26, 2022 at 3:32 PM yanhong wang
> > <yanhong.w...@starfivetech.com> wrote:
> >>
> >>
> >>
> >> On 2022/12/22 17:22, Bin Meng wrote:
> >> > On Mon, Dec 12, 2022 at 10:51 AM Yanhong Wang
> >> > <yanhong.w...@starfivetech.com> wrote:
> >> >>
> >> >> Add StarFive JH7110 SoC to support RISC-V arch.
> >> >>
> >> >> Signed-off-by: Yanhong Wang <yanhong.w...@starfivetech.com>
> >> >> ---
> >> >> arch/riscv/cpu/jh7110/Makefile | 10 ++++
> >> >> arch/riscv/cpu/jh7110/cpu.c | 23 ++++++++
> >> >> arch/riscv/cpu/jh7110/dram.c | 38 +++++++++++++
> >> >> arch/riscv/cpu/jh7110/spl.c | 56 +++++++++++++++++++
> >> >> .../include/asm/arch-jh7110/jh7110-regs.h | 20 +++++++
> >> >> arch/riscv/include/asm/arch-jh7110/spl.h | 13 +++++
> >> >> 6 files changed, 160 insertions(+)
> >> >> create mode 100644 arch/riscv/cpu/jh7110/Makefile
> >> >> create mode 100644 arch/riscv/cpu/jh7110/cpu.c
> >> >> create mode 100644 arch/riscv/cpu/jh7110/dram.c
> >> >> create mode 100644 arch/riscv/cpu/jh7110/spl.c
> >> >> create mode 100644 arch/riscv/include/asm/arch-jh7110/jh7110-regs.h
> >> >> create mode 100644 arch/riscv/include/asm/arch-jh7110/spl.h
> >> >>
> >> >> diff --git a/arch/riscv/cpu/jh7110/Makefile
> >> >> b/arch/riscv/cpu/jh7110/Makefile
> >> >> new file mode 100644
> >> >> index 0000000000..951c95631e
> >> >> --- /dev/null
> >> >> +++ b/arch/riscv/cpu/jh7110/Makefile
> >> >> @@ -0,0 +1,10 @@
> >> >> +# SPDX-License-Identifier: GPL-2.0+
> >> >> +#
> >> >> +# Copyright (C) 2022 StarFive Technology Co., Ltd.
> >> >> +
> >> >> +ifeq ($(CONFIG_SPL_BUILD),y)
> >> >> +obj-y += spl.o
> >> >> +else
> >> >> +obj-y += cpu.o
> >> >> +obj-y += dram.o
> >> >> +endif
> >> >> diff --git a/arch/riscv/cpu/jh7110/cpu.c b/arch/riscv/cpu/jh7110/cpu.c
> >> >> new file mode 100644
> >> >> index 0000000000..5da71908c0
> >> >> --- /dev/null
> >> >> +++ b/arch/riscv/cpu/jh7110/cpu.c
> >> >> @@ -0,0 +1,23 @@
> >> >> +// SPDX-License-Identifier: GPL-2.0+
> >> >> +/*
> >> >> + * Copyright (C) 2022 StarFive Technology Co., Ltd.
> >> >> + * Author: Yanhong Wang <yanhong.w...@starfivetech.com>
> >> >
> >> > nits: use space instead of tab between "Author" and the name
> >> > please fix this globally.
> >> >
> >>
> >>
> >> I will fix.
> >> >> + */
> >> >> +
> >> >> +#include <asm/cache.h>
> >> >> +#include <irq_func.h>
> >> >> +
> >> >> +/*
> >> >> + * cleanup_before_linux() is called just before we call linux
> >> >> + * it prepares the processor for linux
> >> >> + *
> >> >> + * we disable interrupt and caches.
> >> >> + */
> >> >> +int cleanup_before_linux(void)
> >> >> +{
> >> >> + disable_interrupts();
> >> >> +
> >> >> + cache_flush();
> >> >> +
> >> >> + return 0;
> >> >> +}
> >> >> diff --git a/arch/riscv/cpu/jh7110/dram.c b/arch/riscv/cpu/jh7110/dram.c
> >> >> new file mode 100644
> >> >> index 0000000000..4ce91aee9d
> >> >> --- /dev/null
> >> >> +++ b/arch/riscv/cpu/jh7110/dram.c
> >> >> @@ -0,0 +1,38 @@
> >> >> +// SPDX-License-Identifier: GPL-2.0+
> >> >> +/*
> >> >> + * Copyright (C) 2022 StarFive Technology Co., Ltd.
> >> >> + * Author: Yanhong Wang <yanhong.w...@starfivetech.com>
> >> >> + */
> >> >> +
> >> >> +#include <common.h>
> >> >> +#include <fdtdec.h>
> >> >> +#include <init.h>
> >> >> +#include <linux/sizes.h>
> >> >> +
> >> >> +DECLARE_GLOBAL_DATA_PTR;
> >> >> +
> >> >> +int dram_init(void)
> >> >> +{
> >> >> + return fdtdec_setup_mem_size_base();
> >> >> +}
> >> >> +
> >> >> +int dram_init_banksize(void)
> >> >> +{
> >> >> + return fdtdec_setup_memory_banksize();
> >> >> +}
> >> >> +
> >> >> +phys_size_t board_get_usable_ram_top(phys_size_t total_size)
> >> >> +{
> >> >> + /*
> >> >> + * Ensure that we run from first 4GB so that all
> >> >> + * addresses used by U-Boot are 32bit addresses.
> >> >> + *
> >> >> + * This in-turn ensures that 32bit DMA capable
> >> >> + * devices work fine because DMA mapping APIs will
> >> >> + * provide 32bit DMA addresses only.
> >> >> + */
> >> >> + if (IS_ENABLED(CONFIG_64BIT) && gd->ram_top > SZ_4G)
> >> >> + return SZ_4G;
> >> >> +
> >> >> + return gd->ram_top;
> >> >> +}
> >> >> diff --git a/arch/riscv/cpu/jh7110/spl.c b/arch/riscv/cpu/jh7110/spl.c
> >> >> new file mode 100644
> >> >> index 0000000000..a0aacacf7b
> >> >> --- /dev/null
> >> >> +++ b/arch/riscv/cpu/jh7110/spl.c
> >> >> @@ -0,0 +1,56 @@
> >> >> +// SPDX-License-Identifier: GPL-2.0+
> >> >> +/*
> >> >> + * Copyright (C) 2022 StarFive Technology Co., Ltd.
> >> >> + * Author: Yanhong Wang<yanhong.w...@starfivetech.com>
> >> >> + *
> >> >
> >> > remove this line
> >> >
> >>
> >> I will fix.
> >>
> >> >> + */
> >> >> +
> >> >> +#include <asm/csr.h>
> >> >> +#include <dm.h>
> >> >> +#include <log.h>
> >> >> +
> >> >> +#define CSR_U74_FEATURE_DISABLE 0x7c1
> >> >> +
> >> >> +int spl_soc_init(void)
> >> >> +{
> >> >> + int ret;
> >> >> + struct udevice *dev;
> >> >> +
> >> >> + /* DDR init */
> >> >> + ret = uclass_get_device(UCLASS_RAM, 0, &dev);
> >> >> + if (ret) {
> >> >> + debug("DRAM init failed: %d\n", ret);
> >> >> + return ret;
> >> >> + }
> >> >> +
> >> >> + /* flash init */
> >> >> + ret = uclass_get_device(UCLASS_SPI_FLASH, 0, &dev);
> >> >
> >> > Why is this needed?
> >>
> >> This is not necessary. It will be removed in the next version.
> >>
> >> >
> >> >> + if (ret) {
> >> >> + debug("SPI init failed: %d\n", ret);
> >> >> + return ret;
> >> >> + }
> >> >> +
> >> >> + return 0;
> >> >> +}
> >> >> +
> >> >> +void harts_early_init(void)
> >> >> +{
> >> >> + /*
> >> >> + * Feature Disable CSR
> >> >> + *
> >> >> + * Clear feature disable CSR to '0' to turn on all features for
> >> >> + * each core. This operation must be in M-mode.
> >> >> + */
> >> >> + if (CONFIG_IS_ENABLED(RISCV_MMODE))
> >> >> + csr_write(CSR_U74_FEATURE_DISABLE, 0);
> >> >> +
> >> >> + /* clear L2 LIM memory
> >> >> + * set __bss_end to 0x81FFFFF region to zero
> >> >> + */
> >> >
> >> > Wrong multi-line comment format
> >> >
> >> > Why is the clear L2LIM needed?
> >>
> >> The L2 LIM memory will be used as an sram to run the program. You must
> >> clear 0 before using it, otherwise it will cause CPU hang up, which is a
> >> limitation of the hardware.
> >
> > Can you explain in detail about the limitation of the hardware? Is
> > there a published errata for this issue? If yes, please put the errata
> > in the comment before this code.
> >
>
> There is no published errata for this issue.
>
Can you explain in detail about the limitation of the hardware, and
put some comments here?
> >>
> >> >
> >> >> + asm volatile("la t1, __bss_end\n\t"
> >> >> + "li t2, 0x81FFFFF\n\t"
> >> >> + "spl_clear_l2im:\n\t"
> >> >> + "sd zero, 0(t1)\n\t"
> >> >> + "addi t1, t1, 8\n\t"
> >> >> + "blt t1, t2, spl_clear_l2im\n\t");
> >> >> +}
> >> >
> >> > Also why can't this be written in C?
> >> >
> >>
> >> It may be more efficient to use assembly.
> >>
> >
> > Put inline assembly in C if absolutely necessary. I don't think this
> > is absolutely necessary.
> >
>
> The clear 0 initialization is necessary.
> I will put inline assembly in C in the next version.
>
I mean only if absolutely necessary. Such clearing memory is not
absolutely necessary.
Regards,
Bin
