Re: [PATCH 05/11] riscv: cpu: ax25: Simplify cache enabling logic in harts_early_init()

[Leo Liang <ycliang@andestech.com>]

Hi Peter,
On Thu, Jan 19, 2023 at 03:05:38PM +0800, Yu Chien Peter Lin wrote:
> This patch improves the cache enabling operation in harts_early_init(),
> also moves the CSR definition to include/asm/arch-andes/csr.h and drops
> unnecessary i/d-cache disable functions from cleanup_before_linux().
> 
> Signed-off-by: Yu Chien Peter Lin <peterlin@andestech.com>
> ---
>  arch/riscv/cpu/ax25/cpu.c               | 46 +++++++------------------
>  arch/riscv/include/asm/arch-andes/csr.h | 29 ++++++++++++++++
>  2 files changed, 42 insertions(+), 33 deletions(-)
>  create mode 100644 arch/riscv/include/asm/arch-andes/csr.h
> 
> diff --git a/arch/riscv/cpu/ax25/cpu.c b/arch/riscv/cpu/ax25/cpu.c
> index c4c2de2ef0..d335b8d0a8 100644
> --- a/arch/riscv/cpu/ax25/cpu.c
> +++ b/arch/riscv/cpu/ax25/cpu.c
> @@ -1,6 +1,6 @@
>  void harts_early_init(void)
>  {
> +	/* Enable I/D-cache in SPL */
>  	if (CONFIG_IS_ENABLED(RISCV_MMODE)) {
> -		unsigned long long mcache_ctl_val = csr_read(CSR_MCACHE_CTL);
> +		unsigned long mcache_ctl_val = csr_read(CSR_MCACHE_CTL);
> +
> +		mcache_ctl_val |= MCACHE_CTL_DC_COHEN;
> +		mcache_ctl_val |= MCACHE_CTL_IC_EN;
> +		mcache_ctl_val |= MCACHE_CTL_DC_EN;

These three could be combined into one statement.
With your consent, I could modify this when applying to u-boot-riscv/master.

Reviewed-by: Leo Yu-Chi Liang <ycliang@andestech.com>

>  
> -		if (!(mcache_ctl_val & V5_MCACHE_CTL_DC_COHEN_EN))
> -			mcache_ctl_val |= V5_MCACHE_CTL_DC_COHEN_EN;
> -		if (!(mcache_ctl_val & V5_MCACHE_CTL_IC_EN))
> -			mcache_ctl_val |= V5_MCACHE_CTL_IC_EN;
> -		if (!(mcache_ctl_val & V5_MCACHE_CTL_DC_EN))
> -			mcache_ctl_val |= V5_MCACHE_CTL_DC_EN;
>  		csr_write(CSR_MCACHE_CTL, mcache_ctl_val);
>  
>  		/*
> -		 * Check DC_COHEN_EN, if cannot write to mcache_ctl,
> -		 * we assume this bitmap not support L2 CM
> +		 * Check mcache_ctl.DC_COHEN, we assume this platform does
> +		 * not support CM if the bit is hard-wired to 0.
>  		 */
> -		mcache_ctl_val = csr_read(CSR_MCACHE_CTL);
> -		if ((mcache_ctl_val & V5_MCACHE_CTL_DC_COHEN_EN)) {
> -		/* Wait for DC_COHSTA bit be set */
> -			while (!(mcache_ctl_val & V5_MCACHE_CTL_DC_COHSTA_EN))
> -				mcache_ctl_val = csr_read(CSR_MCACHE_CTL);
> +		if (csr_read(CSR_MCACHE_CTL) & MCACHE_CTL_DC_COHEN) {
> +			/* Wait for DC_COHSTA bit to be set */
> +			while (!(csr_read(CSR_MCACHE_CTL)& MCACHE_CTL_DC_COHSTA));
>  		}
>  	}
>  }

Best regards,
Leo