[U-Boot] [PATCH] riscv: cache: Implement i/dcache [status, enable, disable]

[Auer, Lukas]

Hi Rick,

On Mon, 2018-10-22 at 16:16 +0800, Andes wrote:
> From: Rick Chen <rick at andestech.com>
> 
> AndeStar V5 provide mcache_ctl register which can configure
> I/D cache as enabled or disabled.
> 
> This CSR will be encapsulated by CONFIG_NDS_V5.
> If you want to configure cache on AndeStar V5
> AE350 platform. YOu can enable [*] AndeStar V5 ISA support
> by make menuconfig.
> 
> Signed-off-by: Rick Chen <rick at andestech.com>
> Cc: Greentime Hu <greentime at andestech.com>
> ---
>  arch/riscv/Kconfig             |   8 +++
>  arch/riscv/cpu/ax25/cpu.c      |   4 ++
>  arch/riscv/cpu/start.S         |  19 ++++++
>  arch/riscv/include/asm/cache.h |   9 +++
>  arch/riscv/lib/cache.c         | 131
> ++++++++++++++++++++++++++++++++++++++---
>  5 files changed, 163 insertions(+), 8 deletions(-)
> 
> diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
> index 168ca3d..de7fd9e 100644
> --- a/arch/riscv/Kconfig
> +++ b/arch/riscv/Kconfig
> @@ -43,4 +43,12 @@ config 32BIT
>  config 64BIT
>  	bool
>  
> +config NDS_V5
> +	bool "AndeStar V5 ISA support"
> +	def_bool n

You can use just "default n" instead of "def_bool n" here, since you
already have a "bool" above it.

> +	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.
> +
>  endmenu
> 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();

I don't think you need to flush the data cache here. This should be
handled by bootm_load_os() in common/bootm.c.

> +	icache_disable();
> +	dcache_disable();
>  
>  	return 0;
>  }
> diff --git a/arch/riscv/cpu/start.S b/arch/riscv/cpu/start.S
> index 7cd7755..a8764df 100644
> --- a/arch/riscv/cpu/start.S
> +++ b/arch/riscv/cpu/start.S
> @@ -51,6 +51,23 @@ handle_reset:
>  	csrwi mstatus, 0
>  	csrwi mie, 0
>  
> +/* Enable cache */
> +#ifndef CONFIG_SYS_ICACHE_OFF
> +#ifdef CONFIG_NDS_V5
> +	csrr t1, mcache_ctl
> +	ori t0, t1, 0x1
> +	csrw mcache_ctl, t0
> +#endif
> +#endif
> +
> +#ifndef CONFIG_SYS_DCACHE_OFF
> +#ifdef CONFIG_NDS_V5
> +	csrr t1, mcache_ctl
> +	ori t0, t1, 0x2
> +	csrw mcache_ctl, t0
> +#endif
> +#endif
> +
>  /*
>   * Do CPU critical regs init only at reboot,
>   * not when booting from ram
> @@ -191,6 +208,8 @@ clbss_l:
>   * initialization, now running from RAM.
>   */
>  call_board_init_r:
> +	jal	invalidate_icache_all
> +	jal	flush_dcache_all
>  	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..e76ca13 100644
> --- a/arch/riscv/include/asm/cache.h
> +++ b/arch/riscv/include/asm/cache.h
> @@ -7,6 +7,15 @@
>  #ifndef _ASM_RISCV_CACHE_H
>  #define _ASM_RISCV_CACHE_H
>  
> +/* cache */
> +int	icache_status(void);
> +void	icache_enable(void);
> +void	icache_disable(void);
> +int	dcache_status(void);
> +void	dcache_enable(void);
> +void	dcache_disable(void);
> +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 1d67c49..1837fb8 100644
> --- a/arch/riscv/lib/cache.c
> +++ b/arch/riscv/lib/cache.c
> @@ -6,19 +6,39 @@
>  
>  #include <common.h>
>  
> -void flush_dcache_range(unsigned long start, unsigned long end)
> +#ifndef CONFIG_SYS_ICACHE_OFF
> +void invalidate_icache_all(void)
>  {
> +	asm volatile("FENCE.I":::"memory");
>  }

fence.i will likely still be needed even if the instruction cache is
disabled. It should therefore not be removed using the #ifndef.

>  
> -void invalidate_icache_range(unsigned long start, unsigned long end)
> +void icache_enable(void)
>  {
> +#ifndef CONFIG_SYS_ICACHE_OFF

This #ifndef (also the ones below) is redundant since it is already
tested above.

> +#ifdef CONFIG_NDS_V5
> +	asm volatile (
> +		"csrr t1, mcache_ctl\n\t"
> +		"ori t0, t1, 0x1\n\t"
> +		"csrw mcache_ctl, t0\n\t"
> +	);
> +#endif
> +#endif
>  }
>  

I think the cpu-specific functions should go into a separate file under
cpu/ax25. In this case, all functions in this file (lib/cache.c) should
be declared weak, so that they can be overwritten. In addition, we
could also move the enable cache functions into that file. The
functions could then be called, like with ARM, from board_init_r() by
defining the function enable_caches / initr_caches.
What do you think?

Thanks,
Lukas

> -void invalidate_dcache_range(unsigned long start, unsigned long end)
> +void icache_disable(void)
>  {
> +#ifndef CONFIG_SYS_ICACHE_OFF
> +#ifdef CONFIG_NDS_V5
> +	asm volatile (
> +		"csrr t1, mcache_ctl\n\t"
> +		"andi t0, t1, ~0x1\n\t"
> +		"csrw mcache_ctl, t0\n\t"
> +	);
> +#endif
> +#endif
>  }
> -
> -void flush_cache(unsigned long addr, unsigned long size)
> +#else
> +void invalidate_icache_all(void)
>  {
>  }
>  
> @@ -30,9 +50,44 @@ void icache_disable(void)
>  {
>  }
>  
> -int icache_status(void)
> +#endif
> +
> +#ifndef CONFIG_SYS_DCACHE_OFF
> +void flush_dcache_all(void)
> +{
> +	asm volatile("FENCE":::"memory");
> +}
> +
> +void dcache_enable(void)
> +{
> +#ifndef CONFIG_SYS_DCACHE_OFF
> +#ifdef CONFIG_NDS_V5
> +	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_NDS_V5
> +	asm volatile (
> +		"csrr t1, mcache_ctl\n\t"
> +		"andi t0, t1, ~0x2\n\t"
> +		"csrw mcache_ctl, t0\n\t"
> +	);
> +#endif
> +#endif
> +}
> +
> +
> +#else
> +void flush_dcache_all(void)
>  {
> -	return 0;
>  }
>  
>  void dcache_enable(void)
> @@ -42,8 +97,68 @@ void dcache_enable(void)
>  void dcache_disable(void)
>  {
>  }
> +#endif
> +
> +int icache_status(void)
> +{
> +#ifndef CONFIG_SYS_ICACHE_OFF
> +	int ret = 1;
> +#else
> +	int ret = 0;
> +#endif
> +
> +#ifdef CONFIG_NDS_V5
> +	asm volatile (
> +		"csrr t1, mcache_ctl\n\t"
> +		"andi	%0, t1, 0x01\n\t"
> +		: "=r" (ret)
> +		:
> +		: "memory"
> +	);
> +#endif
> +
> +	return ret;
> +}
>  
>  int dcache_status(void)
>  {
> -	return 0;
> +#ifndef CONFIG_SYS_DCACHE_OFF
> +		int ret = 1;
> +#else
> +		int ret = 0;
> +#endif
> +
> +#ifdef CONFIG_NDS_V5
> +	asm volatile (
> +		"csrr t1, mcache_ctl\n\t"
> +		"andi	%0, t1, 0x02\n\t"
> +		: "=r" (ret)
> +		:
> +		: "memory"
> +	);
> +#endif
> +
> +	return ret;
> +}
> +
> +void invalidate_icache_range(unsigned long start, unsigned long end)
> +{
> +}
> +
> +void invalidate_dcache_range(unsigned long start, unsigned long end)
> +{
> +}
> +
> +void flush_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)
> +{
>  }