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

Yu-Chien Peter Lin
2023-01-31T07:54:08Z

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