[U-Boot] [RFC] ARM: davinci: da850: Enable Caches for DA850-EVM

[U-Boot] [RFC] ARM: davinci: da850: Enable Caches for DA850-EVM

[Adam Ford]

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 */
-- 
2.7.4

[U-Boot] [RFC] ARM: davinci: da850: Enable Caches for DA850-EVM

[Adam Ford]

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
>

[U-Boot] [RFC] ARM: davinci: da850: Enable Caches for DA850-EVM

[Peter Howard]

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.



-- 
Peter Howard <pjh@northern-ridge.com.au>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 473 bytes
Desc: This is a digitally signed message part
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20170829/273c62d8/attachment.sig>

[U-Boot] [RFC] ARM: davinci: da850: Enable Caches for DA850-EVM

[Adam Ford]

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>

[U-Boot] [RFC] ARM: davinci: da850: Enable Caches for DA850-EVM

[Vignesh R]

+ 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 */
> 

-- 
Regards
Vignesh