Hi,
-----Original Message----- From: Bin Meng bmeng.cn@gmail.com Sent: 13 March 2020 19:19 To: Anup Patel anup@brainfault.org Cc: Pragnesh Patel pragnesh.patel@sifive.com; U-Boot Mailing List <u- boot@lists.denx.de>; Atish Patra atish.patra@wdc.com; Palmer Dabbelt palmerdabbelt@google.com; Paul Walmsley paul.walmsley@sifive.com; Jagan Teki jagan@amarulasolutions.com; Troy Benjegerdes troy.benjegerdes@sifive.com; Anup Patel anup.patel@wdc.com; Sagar Kadam sagar.kadam@sifive.com; Rick Chen rick@andestech.com; Palmer Dabbelt palmer@dabbelt.com Subject: Re: [PATCH v5 12/14] riscv: sifive: fu540: enable all cache ways from u-boot proper
Hi Anup,
On Fri, Mar 13, 2020 at 6:49 PM Anup Patel anup@brainfault.org wrote:
On Fri, Mar 13, 2020 at 3:52 PM Bin Meng bmeng.cn@gmail.com wrote:
Hi Anup,
On Fri, Mar 13, 2020 at 6:02 PM Anup Patel anup@brainfault.org wrote:
On Fri, Mar 13, 2020 at 2:31 PM Bin Meng bmeng.cn@gmail.com
wrote:
On Wed, Mar 11, 2020 at 3:04 PM Pragnesh Patel pragnesh.patel@sifive.com wrote:
Enable all cache ways from u-boot proper.
U-Boot
Signed-off-by: Pragnesh Patel pragnesh.patel@sifive.com
board/sifive/fu540/Makefile | 1 + board/sifive/fu540/cache.c | 20 ++++++++++++++++++++ board/sifive/fu540/cache.h | 13 +++++++++++++ board/sifive/fu540/fu540.c | 6 ++++-- 4 files changed, 38 insertions(+), 2 deletions(-) create mode 100644 board/sifive/fu540/cache.c create mode 100644 board/sifive/fu540/cache.h
diff --git a/board/sifive/fu540/Makefile b/board/sifive/fu540/Makefile index b05e2f5807..3b867bbd89 100644 --- a/board/sifive/fu540/Makefile +++ b/board/sifive/fu540/Makefile @@ -3,6 +3,7 @@ # Copyright (c) 2019 Western Digital Corporation or its affiliates.
obj-y += fu540.o +obj-y += cache.o
ifdef CONFIG_SPL_BUILD obj-y += spl.o diff --git a/board/sifive/fu540/cache.c b/board/sifive/fu540/cache.c new file mode 100644 index 0000000000..a0bcd2ba48 --- /dev/null +++ b/board/sifive/fu540/cache.c
This should be put into arch/riscv/cpu/fu540/cache.c
@@ -0,0 +1,20 @@ +// SPDX-License-Identifier: GPL-2.0+ +/*
- Copyright (c) 2019 SiFive, Inc */ #include <asm/io.h>
+/* Register offsets */ +#define CACHE_ENABLE 0x008
+/* Enable ways; allow cache to use these ways */ void +cache_enable_ways(u64 base_addr, u8 value) {
volatile u32 *enable = (volatile u32 *)(base_addr +CACHE_ENABLE);/* memory barrier */mb();(*enable) = value;/* memory barrier */mb();+} diff --git a/board/sifive/fu540/cache.h b/board/sifive/fu540/cache.h new file mode 100644 index 0000000000..425124a23b --- /dev/null +++ b/board/sifive/fu540/cache.h
arch/riscv/include/asm/arch-fu540/cache.h
Let's not entire FU540 directory under arch/riscv/cpu directory just to have cache functions. The arch/riscv/cpu/generic is perfectly suitable for FU540.
I doubt arch/riscv/cpu/generic can be generic if we consider U-Boot SPL phase. It can be generic for S-mode U-Boot though.
Its really very easy to do. We are already doing this in Xvisor.
As an example, of DT based operations in a generic board support refer: https://github.com/avpatel/xvisor-next/blob/master/arch/arm/board/gene ric/brd_main.c https://github.com/avpatel/xvisor-next/blob/master/arch/arm/board/gene ric/foundation-v8.c
Yes, this can be easy to do if we have everything written based on DT. But we cannot always assume DT is available in SPL. Some SoCs will not allow SPL with DT support to run due to constraint resources.
Even for DT, due to SPL initialization codes can be very low-level and specific to an SoC, not everything can be properly modeled by DT. Take a look at the u-boot/arch/arm directory. Things are not that easy.
Using the above approach, we are able to boot same Xvisor ARM/ARM64 binary on multiple boards.
Yes, I know. The same as what is done in the Linux kernel. Take x86 for example, the same kernel image can boot on almost every x86 desktop/laptop/server we have today. But we have to understand that this is built on top of BIOS which does all low-level processor / chipset initialization and hide that very well for OS.
IMHO just for cache, it's better not to add arch/riscv/cpu/fu540. If something that we can not handle with arch/riscv/cpu/generic then we will definitely add arch/riscv/cpu/fu540 dir.
Thanks Bin and Anup for the review.
If we re-use arch/riscv/cpu/generic as-much as possible then arch/riscv will be easy to maintain in future.
We can add arch/riscv/cpu/generic/cache.c which will do things FU540 specific based on "#ifdef" or "DT compatible string".
Regards, Bin