Patchwork [U-Boot,4/8] arm: minor fixes for cache and mmu handling

login
register
mail settings
Submitter Aneesh V
Date Dec. 22, 2010, 11:54 a.m.
Message ID <1293018898-13253-5-git-send-email-aneesh@ti.com>
Download mbox | patch
Permalink /patch/76391/
State Changes Requested
Delegated to: Albert ARIBAUD
Headers show

Comments

Aneesh V - Dec. 22, 2010, 11:54 a.m.
1. make sure that page table setup is not done multiple times
2. flush_dcache_all() is more appropriate while disabling cache
   than a range flush on the entire memory(flush_cache())

   Provide a default implementation for flush_dcache_all()
   for backward compatibility and to avoid build issues.

Signed-off-by: Aneesh V <aneesh@ti.com>
---
 arch/arm/lib/cache-cp15.c |    9 +++++++--
 arch/arm/lib/cache.c      |   13 ++++++++++++-
 2 files changed, 19 insertions(+), 3 deletions(-)
Albert ARIBAUD - Jan. 8, 2011, 7:04 a.m.
Hi Aneesh,

Le 22/12/2010 12:54, Aneesh V a écrit :
> 1. make sure that page table setup is not done multiple times
> 2. flush_dcache_all() is more appropriate while disabling cache
>     than a range flush on the entire memory(flush_cache())
>
>     Provide a default implementation for flush_dcache_all()
>     for backward compatibility and to avoid build issues.
>
> Signed-off-by: Aneesh V<aneesh@ti.com>
> ---
>   arch/arm/lib/cache-cp15.c |    9 +++++++--
>   arch/arm/lib/cache.c      |   13 ++++++++++++-
>   2 files changed, 19 insertions(+), 3 deletions(-)
>
> diff --git a/arch/arm/lib/cache-cp15.c b/arch/arm/lib/cache-cp15.c
> index ca526fb..20aa993 100644
> --- a/arch/arm/lib/cache-cp15.c
> +++ b/arch/arm/lib/cache-cp15.c
> @@ -94,13 +94,18 @@ static inline void mmu_setup(void)
>   	set_cr(reg | CR_M);
>   }
>
> +static int mmu_enabled(void)
> +{
> +	return get_cr()&  CR_M;
> +}
> +
>   /* cache_bit must be either CR_I or CR_C */
>   static void cache_enable(uint32_t cache_bit)
>   {
>   	uint32_t reg;
>
>   	/* The data cache is not active unless the mmu is enabled too */
> -	if (cache_bit == CR_C)
> +	if ((cache_bit == CR_C)&&  !mmu_enabled())
>   		mmu_setup();
>   	reg = get_cr();	/* get control reg. */
>   	cp_delay();

Do you know why double MMU setups happen? Can we not fix the execution 
path and remove the second MMU setup call there, rather that catching t 
on the fly to ignore it?

> @@ -119,7 +124,7 @@ static void cache_disable(uint32_t cache_bit)
>   			return;
>   		/* if disabling data cache, disable mmu too */
>   		cache_bit |= CR_M;
> -		flush_cache(0, ~0);
> +		flush_dcache_all();
>   	}
>   	reg = get_cr();
>   	cp_delay();
> diff --git a/arch/arm/lib/cache.c b/arch/arm/lib/cache.c
> index 275b6e1..363609a 100644
> --- a/arch/arm/lib/cache.c
> +++ b/arch/arm/lib/cache.c
> @@ -25,7 +25,7 @@
>
>   #include<common.h>
>
> -void  __flush_cache(unsigned long dummy1, unsigned long dummy2)
> +void  __flush_cache(unsigned long start, unsigned long size)
>   {
>   #if defined(CONFIG_OMAP2420) || defined(CONFIG_ARM1136)
>   	void arm1136_cache_flush(void);
> @@ -42,3 +42,14 @@ void  __flush_cache(unsigned long dummy1, unsigned long dummy2)
>   }
>   void  flush_cache(unsigned long dummy1, unsigned long dummy2)

Please fix also parameters in this declaration.

>   	__attribute__((weak, alias("__flush_cache")));
> +
> +/*
> + * Default implementation:
> + * do a range flush for the entire range
> + */
> +void	__flush_dcache_all(void)
> +{
> +	flush_cache(0, ~0);
> +}
> +void	flush_dcache_all(void)
> +	__attribute__((weak, alias("__flush_dcache_all")));

Amicalement,
Aneesh V - Jan. 8, 2011, 9:13 a.m.
Hi Albert,

On Saturday 08 January 2011 12:34 PM, Albert ARIBAUD wrote:
> Hi Aneesh,
>
> Le 22/12/2010 12:54, Aneesh V a écrit :
>> 1. make sure that page table setup is not done multiple times
>> 2. flush_dcache_all() is more appropriate while disabling cache
>>      than a range flush on the entire memory(flush_cache())
>>
>>      Provide a default implementation for flush_dcache_all()
>>      for backward compatibility and to avoid build issues.
>>
>> Signed-off-by: Aneesh V<aneesh@ti.com>
>> ---
>>    arch/arm/lib/cache-cp15.c |    9 +++++++--
>>    arch/arm/lib/cache.c      |   13 ++++++++++++-
>>    2 files changed, 19 insertions(+), 3 deletions(-)
>>
>> diff --git a/arch/arm/lib/cache-cp15.c b/arch/arm/lib/cache-cp15.c
>> index ca526fb..20aa993 100644
>> --- a/arch/arm/lib/cache-cp15.c
>> +++ b/arch/arm/lib/cache-cp15.c
>> @@ -94,13 +94,18 @@ static inline void mmu_setup(void)
>>    	set_cr(reg | CR_M);
>>    }
>>
>> +static int mmu_enabled(void)
>> +{
>> +	return get_cr()&   CR_M;
>> +}
>> +
>>    /* cache_bit must be either CR_I or CR_C */
>>    static void cache_enable(uint32_t cache_bit)
>>    {
>>    	uint32_t reg;
>>
>>    	/* The data cache is not active unless the mmu is enabled too */
>> -	if (cache_bit == CR_C)
>> +	if ((cache_bit == CR_C)&&   !mmu_enabled())
>>    		mmu_setup();
>>    	reg = get_cr();	/* get control reg. */
>>    	cp_delay();
>
> Do you know why double MMU setups happen? Can we not fix the execution
> path and remove the second MMU setup call there, rather that catching t
> on the fly to ignore it?

Please note that mmu_setup() was getting called unconditionally from
dcache_enable(). I see that some drivers are calling dcache_enable()
and there are u-boot commands for enabling disabling cache.
Consequently mmu_setup() may get called multiple times.
Do we want to prevent dcache_enable() from being called multiple times?

Best regards,
Aneesh

Patch

diff --git a/arch/arm/lib/cache-cp15.c b/arch/arm/lib/cache-cp15.c
index ca526fb..20aa993 100644
--- a/arch/arm/lib/cache-cp15.c
+++ b/arch/arm/lib/cache-cp15.c
@@ -94,13 +94,18 @@  static inline void mmu_setup(void)
 	set_cr(reg | CR_M);
 }
 
+static int mmu_enabled(void)
+{
+	return get_cr() & CR_M;
+}
+
 /* cache_bit must be either CR_I or CR_C */
 static void cache_enable(uint32_t cache_bit)
 {
 	uint32_t reg;
 
 	/* The data cache is not active unless the mmu is enabled too */
-	if (cache_bit == CR_C)
+	if ((cache_bit == CR_C) && !mmu_enabled())
 		mmu_setup();
 	reg = get_cr();	/* get control reg. */
 	cp_delay();
@@ -119,7 +124,7 @@  static void cache_disable(uint32_t cache_bit)
 			return;
 		/* if disabling data cache, disable mmu too */
 		cache_bit |= CR_M;
-		flush_cache(0, ~0);
+		flush_dcache_all();
 	}
 	reg = get_cr();
 	cp_delay();
diff --git a/arch/arm/lib/cache.c b/arch/arm/lib/cache.c
index 275b6e1..363609a 100644
--- a/arch/arm/lib/cache.c
+++ b/arch/arm/lib/cache.c
@@ -25,7 +25,7 @@ 
 
 #include <common.h>
 
-void  __flush_cache(unsigned long dummy1, unsigned long dummy2)
+void  __flush_cache(unsigned long start, unsigned long size)
 {
 #if defined(CONFIG_OMAP2420) || defined(CONFIG_ARM1136)
 	void arm1136_cache_flush(void);
@@ -42,3 +42,14 @@  void  __flush_cache(unsigned long dummy1, unsigned long dummy2)
 }
 void  flush_cache(unsigned long dummy1, unsigned long dummy2)
 	__attribute__((weak, alias("__flush_cache")));
+
+/*
+ * Default implementation:
+ * do a range flush for the entire range
+ */
+void	__flush_dcache_all(void)
+{
+	flush_cache(0, ~0);
+}
+void	flush_dcache_all(void)
+	__attribute__((weak, alias("__flush_dcache_all")));