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

Leo Liang
2023-01-31T07:07:24Z

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