| Message ID | 1503873560-1496-1-git-send-email-aford173@gmail.com |
|---|---|
| State | RFC |
| Delegated to: | Tom Rini |
| Headers | show |
| Series | [U-Boot,RFC] ARM: davinci: da850: Enable Caches for DA850-EVM | expand |
On Sun, Aug 27, 2017 at 5:39 PM, Adam Ford <aford173@gmail.com> wrote: > What starting up the DA850-EVM, U-Boot generates a warning: > WARNING: Caches not enabled > > Looking at other arm926 processors, this is an attempt > to enable the caches and remove the warning. > > I am notsure who the proper TI or ARM people are to review this. > For what It's worth, I ran some whetstone benchmarks. Using 1000 loops, the da850-evm increased from 14.3 MIPS without cache to 16.7 MIPS with cache > Signed-off-by: Adam Ford <aford173@gmail.com> > --- > board/davinci/da8xxevm/da850evm.c | 26 ++++++++++++++++++++++++++ > 1 file changed, 26 insertions(+) > > diff --git a/board/davinci/da8xxevm/da850evm.c b/board/davinci/da8xxevm/da850evm.c > index c2d2e8e..33a923c 100644 > --- a/board/davinci/da8xxevm/da850evm.c > +++ b/board/davinci/da8xxevm/da850evm.c > @@ -491,3 +491,29 @@ int board_eth_init(bd_t *bis) > return 0; > } > #endif /* CONFIG_DRIVER_TI_EMAC */ > + > +#ifndef CONFIG_SYS_ICACHE_OFF > +/* Invalidate entire I-cache and branch predictor array */ > +void invalidate_icache_all(void) > +{ > + unsigned long i = 0; > + > + asm ("mcr p15, 0, %0, c7, c5, 0" : : "r" (i)); > +} > +#else > +void invalidate_icache_all(void) > +{ > +} > +#endif > + > +#ifndef CONFIG_SYS_DCACHE_OFF > +void enable_caches(void) > +{ > + /* Enable D-cache. I-cache is already enabled in start.S */ > + dcache_enable(); > +} > +#else > +void enable_caches(void) > +{ > +} > +#endif /* CONFIG_SYS_DCACHE_OFF */ > -- > 2.7.4 >
On Sun, 2017-08-27 at 17:39 -0500, Adam Ford wrote: > What starting up the DA850-EVM, U-Boot generates a warning: > WARNING: Caches not enabled > > Looking at other arm926 processors, this is an attempt > to enable the caches and remove the warning. > > I am notsure who the proper TI or ARM people are to review this. > > Signed-off-by: Adam Ford <aford173@gmail.com> > --- > board/davinci/da8xxevm/da850evm.c | 26 ++++++++++++++++++++++++++ > 1 file changed, 26 insertions(+) > > diff --git a/board/davinci/da8xxevm/da850evm.c > b/board/davinci/da8xxevm/da850evm.c > index c2d2e8e..33a923c 100644 > --- a/board/davinci/da8xxevm/da850evm.c > +++ b/board/davinci/da8xxevm/da850evm.c > @@ -491,3 +491,29 @@ int board_eth_init(bd_t *bis) > return 0; > } > #endif /* CONFIG_DRIVER_TI_EMAC */ > + > +#ifndef CONFIG_SYS_ICACHE_OFF > +/* Invalidate entire I-cache and branch predictor array */ > +void invalidate_icache_all(void) > +{ > + unsigned long i = 0; > + > + asm ("mcr p15, 0, %0, c7, c5, 0" : : "r" (i)); > +} > +#else > +void invalidate_icache_all(void) > +{ > +} > +#endif > + > +#ifndef CONFIG_SYS_DCACHE_OFF > +void enable_caches(void) > +{ > + /* Enable D-cache. I-cache is already enabled in start.S */ > + dcache_enable(); > +} > +#else > +void enable_caches(void) > +{ > +} > +#endif /* CONFIG_SYS_DCACHE_OFF */ My 2c on this would be: 1) If it's going to be for the davinci specifically, I'd do it the same way as for the at91 - i.e. under mach-davinci/arm926ejs 2) Is there any good reason _not_ to add it to arch/arm/cpu/arm926ejs/cache.c rather than per-board? Given it's a feature of the arm core rather than anything specific to the davinci it would make sense to me.
On Tue, Aug 29, 2017 at 2:01 AM, Peter Howard <pjh@northern-ridge.com.au> wrote: > On Sun, 2017-08-27 at 17:39 -0500, Adam Ford wrote: >> What starting up the DA850-EVM, U-Boot generates a warning: >> WARNING: Caches not enabled >> >> Looking at other arm926 processors, this is an attempt >> to enable the caches and remove the warning. >> >> I am notsure who the proper TI or ARM people are to review this. >> >> Signed-off-by: Adam Ford <aford173@gmail.com> >> --- >> board/davinci/da8xxevm/da850evm.c | 26 ++++++++++++++++++++++++++ >> 1 file changed, 26 insertions(+) >> >> diff --git a/board/davinci/da8xxevm/da850evm.c >> b/board/davinci/da8xxevm/da850evm.c >> index c2d2e8e..33a923c 100644 >> --- a/board/davinci/da8xxevm/da850evm.c >> +++ b/board/davinci/da8xxevm/da850evm.c >> @@ -491,3 +491,29 @@ int board_eth_init(bd_t *bis) >> return 0; >> } >> #endif /* CONFIG_DRIVER_TI_EMAC */ >> + >> +#ifndef CONFIG_SYS_ICACHE_OFF >> +/* Invalidate entire I-cache and branch predictor array */ >> +void invalidate_icache_all(void) >> +{ >> + unsigned long i = 0; >> + >> + asm ("mcr p15, 0, %0, c7, c5, 0" : : "r" (i)); >> +} >> +#else >> +void invalidate_icache_all(void) >> +{ >> +} >> +#endif >> + >> +#ifndef CONFIG_SYS_DCACHE_OFF >> +void enable_caches(void) >> +{ >> + /* Enable D-cache. I-cache is already enabled in start.S */ >> + dcache_enable(); >> +} >> +#else >> +void enable_caches(void) >> +{ >> +} >> +#endif /* CONFIG_SYS_DCACHE_OFF */ > > My 2c on this would be: > > 1) If it's going to be for the davinci specifically, I'd do it the same > way as for the at91 - i.e. under mach-davinci/arm926ejs Looking at the at91, the enable_caches only enables icache, but that appears to be done in the start.S already, but I will be the first to admit, I didn't read through the assembly code to verify against the instruction set. I used the iMX arm926ejs as the basis for my patch, but even then there are differences between the different i.MX boards in how they implement enable_caches(); > > 2) Is there any good reason _not_ to add it to > arch/arm/cpu/arm926ejs/cache.c rather than per-board? Given it's a > feature of the arm core rather than anything specific to the davinci it > would make sense to me. I would normally agree, which is why I was hoping someone from ARM might chime in. I noticed we have several different implementations of the arm926 depending on the silicon vendor (as noted above), so I wasn't sure if there was some difference between their implementations. I don't have the other boards, so I wasn't too excited on trying to change a common ARM architecture without some input or guidance from those who better understand the overall architecture better. > > > > -- > Peter Howard <pjh@northern-ridge.com.au>
+ Sekhar On Monday 28 August 2017 04:09 AM, Adam Ford wrote: > What starting up the DA850-EVM, U-Boot generates a warning: > WARNING: Caches not enabled > > Looking at other arm926 processors, this is an attempt > to enable the caches and remove the warning. > > I am notsure who the proper TI or ARM people are to review this. > > Signed-off-by: Adam Ford <aford173@gmail.com> > --- > board/davinci/da8xxevm/da850evm.c | 26 ++++++++++++++++++++++++++ > 1 file changed, 26 insertions(+) > > diff --git a/board/davinci/da8xxevm/da850evm.c b/board/davinci/da8xxevm/da850evm.c > index c2d2e8e..33a923c 100644 > --- a/board/davinci/da8xxevm/da850evm.c > +++ b/board/davinci/da8xxevm/da850evm.c > @@ -491,3 +491,29 @@ int board_eth_init(bd_t *bis) > return 0; > } > #endif /* CONFIG_DRIVER_TI_EMAC */ > + > +#ifndef CONFIG_SYS_ICACHE_OFF > +/* Invalidate entire I-cache and branch predictor array */ > +void invalidate_icache_all(void) > +{ > + unsigned long i = 0; > + > + asm ("mcr p15, 0, %0, c7, c5, 0" : : "r" (i)); > +} > +#else > +void invalidate_icache_all(void) > +{ > +} > +#endif > + > +#ifndef CONFIG_SYS_DCACHE_OFF > +void enable_caches(void) > +{ > + /* Enable D-cache. I-cache is already enabled in start.S */ > + dcache_enable(); > +} > +#else > +void enable_caches(void) > +{ > +} > +#endif /* CONFIG_SYS_DCACHE_OFF */ >
diff --git a/board/davinci/da8xxevm/da850evm.c b/board/davinci/da8xxevm/da850evm.c index c2d2e8e..33a923c 100644 --- a/board/davinci/da8xxevm/da850evm.c +++ b/board/davinci/da8xxevm/da850evm.c @@ -491,3 +491,29 @@ int board_eth_init(bd_t *bis) return 0; } #endif /* CONFIG_DRIVER_TI_EMAC */ + +#ifndef CONFIG_SYS_ICACHE_OFF +/* Invalidate entire I-cache and branch predictor array */ +void invalidate_icache_all(void) +{ + unsigned long i = 0; + + asm ("mcr p15, 0, %0, c7, c5, 0" : : "r" (i)); +} +#else +void invalidate_icache_all(void) +{ +} +#endif + +#ifndef CONFIG_SYS_DCACHE_OFF +void enable_caches(void) +{ + /* Enable D-cache. I-cache is already enabled in start.S */ + dcache_enable(); +} +#else +void enable_caches(void) +{ +} +#endif /* CONFIG_SYS_DCACHE_OFF */
What starting up the DA850-EVM, U-Boot generates a warning: WARNING: Caches not enabled Looking at other arm926 processors, this is an attempt to enable the caches and remove the warning. I am notsure who the proper TI or ARM people are to review this. Signed-off-by: Adam Ford <aford173@gmail.com> --- board/davinci/da8xxevm/da850evm.c | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+)