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

[Yu-Chien Peter Lin <peterlin@andestech.com>]

Hi Leo,

On Tue, Jan 31, 2023 at 07:07:02AM +0000, Leo Liang wrote:
> 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.

Thanks, I also want to improve the comment, I'll update this along with
the next patch.

Best regards,
Peter Lin

> 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