Hi Rick,

On Wed, 2018-11-07 at 09:54 +0800, Andes wrote:

From: Rick Chen rick@andestech.com

AndeStar RISC-V(V5) provide mcache_ctl register which can configure I/D cache as enabled or disabled.

This CSR will be encapsulated by CONFIG_RISCV_NDS. If you want to configure cache on AndeStar V5 AE350 platform. YOu can enable [*] AndeStar V5 ISA support by make menuconfig.

This approach also provide the expansion when the vender specific features are going to join in.

Signed-off-by: Rick Chen rick@andestech.com Cc: Greentime Hu greentime@andestech.com

---

The commit message does not fully describe all changes of this patch. It would be helpful to split it into separate patches. Having only one change in each patch makes debugging later easier, because you don't have to manually undo parts of a patch to find out what is going on.

Changes in v3

Bin suggested:

1. Move RISCV_NDS to arch/riscv/cpu/ax25/Kconfig

Lukas suggested:

1. Add CONFIG_SYS_ICACHE_OFF in i/dcache_disable()
2. Remove duplicate declaration in arch/riscv/include/asm/cache.h
3. Add s space after "volatile" and ":::" to match the style.
4. Implement flush_dcache_range() and flush_cache() in

arch/riscv/lib/cache.c

arch/riscv/Kconfig | 6 +++ arch/riscv/cpu/ax25/Kconfig | 7 ++++ arch/riscv/cpu/ax25/Makefile | 1 + arch/riscv/cpu/ax25/cache.c | 95 ++++++++++++++++++++++++++++++++++++++++++ arch/riscv/cpu/ax25/cpu.c | 4 ++ arch/riscv/cpu/qemu/cpu.c | 2 +- arch/riscv/cpu/start.S | 6 +++ arch/riscv/include/asm/cache.h | 3 ++ arch/riscv/lib/cache.c | 32 ++++++++++---- 9 files changed, 146 insertions(+), 10 deletions(-) create mode 100644 arch/riscv/cpu/ax25/Kconfig create mode 100644 arch/riscv/cpu/ax25/cache.c

diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig index 371921b..6a86e9c 100644 --- a/arch/riscv/Kconfig +++ b/arch/riscv/Kconfig @@ -16,9 +16,15 @@ config TARGET_QEMU_VIRT

endchoice

+# board-specific options below source "board/AndesTech/ax25-ae350/Kconfig" source "board/emulation/qemu-riscv/Kconfig"

+# platform-specific options below +source "arch/riscv/cpu/ax25/Kconfig"

+# architecture-specific options below

choice prompt "Base ISA" default ARCH_RV32I diff --git a/arch/riscv/cpu/ax25/Kconfig b/arch/riscv/cpu/ax25/Kconfig new file mode 100644 index 0000000..6c7022f --- /dev/null +++ b/arch/riscv/cpu/ax25/Kconfig @@ -0,0 +1,7 @@ +config RISCV_NDS

• bool "AndeStar V5 ISA support"
• default n
• help

• Say Y here if you plan to run U-Boot on AndeStar v5
  

• platforms and use some specific features which are
  

• provided by Andes Technology AndeStar V5 Families.
  

I think it would make sense to add a "depends on TARGET_AX25_AE350" to the RISCV_NDS config, so that it is not shown for other boards.

It would be great if we could enable cache support by default, to make sure it is tested by Travis-CI. Some time ago, it was suggested on the riscv-linux mailing list to use the CSR numbers instead of the names. This way, the generic GCC compiler will not complain about the unknown CSRs. I think, this would be a good idea. What do you think?

diff --git a/arch/riscv/cpu/ax25/Makefile b/arch/riscv/cpu/ax25/Makefile index 2ab0342..318bacc 100644 --- a/arch/riscv/cpu/ax25/Makefile +++ b/arch/riscv/cpu/ax25/Makefile @@ -4,3 +4,4 @@ # Rick Chen, Andes Technology Corporation rick@andestech.com

obj-y := cpu.o +obj-y += cache.o diff --git a/arch/riscv/cpu/ax25/cache.c b/arch/riscv/cpu/ax25/cache.c new file mode 100644 index 0000000..6600ac2 --- /dev/null +++ b/arch/riscv/cpu/ax25/cache.c @@ -0,0 +1,95 @@ +// SPDX-License-Identifier: GPL-2.0+ +/*

• • Copyright (C) 2017 Andes Technology Corporation
• • Rick Chen, Andes Technology Corporation rick@andestech.com
• */

+#include <common.h>

+void icache_enable(void) +{ +#ifndef CONFIG_SYS_ICACHE_OFF +#ifdef CONFIG_RISCV_NDS

• asm volatile (

• "csrr t1, mcache_ctl\n\t"
  

• "ori t0, t1, 0x1\n\t"
  

• "csrw mcache_ctl, t0\n\t"
  

• );

+#endif +#endif +}

+void icache_disable(void) +{ +#ifndef CONFIG_SYS_ICACHE_OFF +#ifdef CONFIG_RISCV_NDS

• asm volatile (

• "fence.i\n\t"
  

• "csrr t1, mcache_ctl\n\t"
  

• "andi t0, t1, ~0x1\n\t"
  

• "csrw mcache_ctl, t0\n\t"
  

• );

+#endif +#endif +}

+void dcache_enable(void) +{ +#ifndef CONFIG_SYS_DCACHE_OFF +#ifdef CONFIG_RISCV_NDS

• asm volatile (

• "csrr t1, mcache_ctl\n\t"
  

• "ori t0, t1, 0x2\n\t"
  

• "csrw mcache_ctl, t0\n\t"
  

• );

+#endif +#endif +}

+void dcache_disable(void) +{ +#ifndef CONFIG_SYS_DCACHE_OFF +#ifdef CONFIG_RISCV_NDS

• asm volatile (

• "fence\n\t"
  

• "csrr t1, mcache_ctl\n\t"
  

• "andi t0, t1, ~0x2\n\t"
  

• "csrw mcache_ctl, t0\n\t"
  

• );

+#endif +#endif +}

+int icache_status(void) +{

• int ret = 0;

+#ifdef CONFIG_RISCV_NDS

• asm volatile (

• "csrr t1, mcache_ctl\n\t"
  

• "andi	%0, t1, 0x01\n\t"
  

• : "=r" (ret)
  

• :
  

• : "memory"
  

• );

+#endif

• return ret;

+}

+int dcache_status(void) +{

• int ret = 0;

+#ifdef CONFIG_RISCV_NDS

• asm volatile (

• "csrr t1, mcache_ctl\n\t"
  

• "andi	%0, t1, 0x02\n\t"
  

• : "=r" (ret)
  

• :
  

• : "memory"
  

• );

+#endif

• return ret;

+} diff --git a/arch/riscv/cpu/ax25/cpu.c b/arch/riscv/cpu/ax25/cpu.c index fddcc15..76689b2 100644 --- a/arch/riscv/cpu/ax25/cpu.c +++ b/arch/riscv/cpu/ax25/cpu.c @@ -6,6 +6,7 @@

/* CPU specific code */ #include <common.h> +#include <asm/cache.h>

/*

• cleanup_before_linux() is called just before we call linux

@@ -18,6 +19,9 @@ int cleanup_before_linux(void) disable_interrupts();

/* turn off I/D-cache */

• cache_flush();

• icache_disable();

• dcache_disable();

  return 0;

} diff --git a/arch/riscv/cpu/qemu/cpu.c b/arch/riscv/cpu/qemu/cpu.c index 6c7a327..25d97d0 100644 --- a/arch/riscv/cpu/qemu/cpu.c +++ b/arch/riscv/cpu/qemu/cpu.c @@ -15,7 +15,7 @@ int cleanup_before_linux(void) { disable_interrupts();

• /* turn off I/D-cache */

• cache_flush();

  return 0;

} diff --git a/arch/riscv/cpu/start.S b/arch/riscv/cpu/start.S index 331a534..15e1b81 100644 --- a/arch/riscv/cpu/start.S +++ b/arch/riscv/cpu/start.S @@ -46,6 +46,10 @@ _start: /* mask all interrupts */ csrw mie, zero

• /* Enable cache */
• jal icache_enable
• jal dcache_enable

/*

• Set stackpointer in internal/ex RAM to call board_init_f

*/ @@ -181,6 +185,8 @@ clbss_l:

• initialization, now running from RAM.

*/ call_board_init_r:

• jal invalidate_icache_all
• jal flush_dcache_all

I previously read this as call_board_init_f, which is why I thought it should be moved. Of course this is already correct :)

la t0, board_init_r mv t4, t0 /* offset of board_init_r() */ add t4, t4, t6 /* real address of board_init_r() */ diff --git a/arch/riscv/include/asm/cache.h b/arch/riscv/include/asm/cache.h index ca83dd6..ec8fe20 100644 --- a/arch/riscv/include/asm/cache.h +++ b/arch/riscv/include/asm/cache.h @@ -7,6 +7,9 @@ #ifndef _ASM_RISCV_CACHE_H #define _ASM_RISCV_CACHE_H

+/* cache */ +void cache_flush(void);

/*

• The current upper bound for RISCV L1 data cache line sizes is 32

bytes.

• We use that value for aligning DMA buffers unless the board

config has diff --git a/arch/riscv/lib/cache.c b/arch/riscv/lib/cache.c index d642a38..ae5c607 100644 --- a/arch/riscv/lib/cache.c +++ b/arch/riscv/lib/cache.c @@ -6,8 +6,18 @@

#include <common.h>

+void invalidate_icache_all(void) +{

• asm volatile ("fence.i" ::: "memory");

+}

+void flush_dcache_all(void) +{

• asm volatile ("fence" :::"memory");

+}

I thought about this a bit more. I don't think we can flush the data cache with the fence instruction. It is only used to enforce ordering constraints. I think we should therefore remove the flush_dcache* functions.

Thanks, Lukas

void flush_dcache_range(unsigned long start, unsigned long end) {

• flush_dcache_all();

}

void invalidate_icache_range(unsigned long start, unsigned long end) @@ -19,41 +29,45 @@ void invalidate_icache_range(unsigned long start, unsigned long end) invalidate_icache_all(); }

-void invalidate_icache_all(void) +void invalidate_dcache_range(unsigned long start, unsigned long end) {

• asm volatile ("fence.i" ::: "memory");

• flush_dcache_all();

}

-void invalidate_dcache_range(unsigned long start, unsigned long end) +void cache_flush(void) {

• invalidate_icache_all();
• flush_dcache_all();

}

void flush_cache(unsigned long addr, unsigned long size) {

• invalidate_icache_all();
• flush_dcache_all();

}

-void icache_enable(void) +__weak void icache_enable(void) { }

-void icache_disable(void) +__weak void icache_disable(void) { }

-int icache_status(void) +__weak int icache_status(void) { return 0; }

-void dcache_enable(void) +__weak void dcache_enable(void) { }

-void dcache_disable(void) +__weak void dcache_disable(void) { }

-int dcache_status(void) +__weak int dcache_status(void) { return 0; }