Patchwork [U-Boot,v2,01/10] arm: move flush_dcache_all() to just before disable cache

login
register
mail settings
Submitter Simon Glass
Date Nov. 30, 2012, 11:01 p.m.
Message ID <1354316484-23515-1-git-send-email-sjg@chromium.org>
Download mbox | patch
Permalink /patch/203071/
State Accepted, archived
Delegated to: Albert ARIBAUD
Headers show

Comments

Simon Glass - Nov. 30, 2012, 11:01 p.m.
From: Arun Mankuzhi <arun.m@samsung.com>

In Cortex-A15 architecture, when we run cache invalidate
the cache clean operation executes automatically.
So if there are any dirty cache lines before disabling the L2 cache
these will be synchronized with the main memory when
invalidate_dcache_all() runs in the last part of U-boot

The two functions after flush_dcache_all is using the stack. So this
data will be on the cache. After disable when invalidate is called the
data will be flushed from cache to memory. This corrupts the stack in
invalida_dcache_all. So this change is required to avoid the u-boot
hang.

So flush has to be done just before clearing CR_C bit

Signed-off-by: Arun Mankuzhi <arun.m@samsung.com>
Signed-off-by: Simon Glass <sjg@chromium.org>
---
Changes in v2: None

 arch/arm/lib/cache-cp15.c |    5 ++++-
 1 files changed, 4 insertions(+), 1 deletions(-)
Albert ARIBAUD - Jan. 10, 2013, 10:27 p.m.
Hi Simon,

On Fri, 30 Nov 2012 15:01:14 -0800, Simon Glass <sjg@chromium.org>
wrote:

> From: Arun Mankuzhi <arun.m@samsung.com>
> 
> In Cortex-A15 architecture, when we run cache invalidate
> the cache clean operation executes automatically.
> So if there are any dirty cache lines before disabling the L2 cache
> these will be synchronized with the main memory when
> invalidate_dcache_all() runs in the last part of U-boot
> 
> The two functions after flush_dcache_all is using the stack. So this
> data will be on the cache. After disable when invalidate is called the
> data will be flushed from cache to memory. This corrupts the stack in
> invalida_dcache_all. So this change is required to avoid the u-boot
> hang.
> 
> So flush has to be done just before clearing CR_C bit
> 
> Signed-off-by: Arun Mankuzhi <arun.m@samsung.com>
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
> Changes in v2: None
> 
>  arch/arm/lib/cache-cp15.c |    5 ++++-
>  1 files changed, 4 insertions(+), 1 deletions(-)
> 
> diff --git a/arch/arm/lib/cache-cp15.c b/arch/arm/lib/cache-cp15.c
> index 939de10..06119c8 100644
> --- a/arch/arm/lib/cache-cp15.c
> +++ b/arch/arm/lib/cache-cp15.c
> @@ -124,8 +124,11 @@ static void cache_disable(uint32_t cache_bit)
>  			return;
>  		/* if disabling data cache, disable mmu too */
>  		cache_bit |= CR_M;
> -		flush_dcache_all();
>  	}
> +	reg = get_cr();
> +	cp_delay();
> +	if (cache_bit == (CR_C | CR_M))
> +		flush_dcache_all();
>  	set_cr(reg & ~cache_bit);
>  }
>  #endif

Applied the whole series to u-boot-arm/master, thanks!

Amicalement,
SRICHARAN R - Jan. 11, 2013, 7:59 a.m.
Hi,

On Saturday 01 December 2012 04:31 AM, Simon Glass wrote:
> From: Arun Mankuzhi <arun.m@samsung.com>
>
> In Cortex-A15 architecture, when we run cache invalidate
> the cache clean operation executes automatically.
> So if there are any dirty cache lines before disabling the L2 cache
> these will be synchronized with the main memory when
> invalidate_dcache_all() runs in the last part of U-boot
>
> The two functions after flush_dcache_all is using the stack. So this
> data will be on the cache. After disable when invalidate is called the
> data will be flushed from cache to memory. This corrupts the stack in
> invalida_dcache_all. So this change is required to avoid the u-boot
> hang.
>
> So flush has to be done just before clearing CR_C bit
>
> Signed-off-by: Arun Mankuzhi <arun.m@samsung.com>
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
> Changes in v2: None
>
>   arch/arm/lib/cache-cp15.c |    5 ++++-
>   1 files changed, 4 insertions(+), 1 deletions(-)
>
> diff --git a/arch/arm/lib/cache-cp15.c b/arch/arm/lib/cache-cp15.c
> index 939de10..06119c8 100644
> --- a/arch/arm/lib/cache-cp15.c
> +++ b/arch/arm/lib/cache-cp15.c
> @@ -124,8 +124,11 @@ static void cache_disable(uint32_t cache_bit)
>   			return;
>   		/* if disabling data cache, disable mmu too */
>   		cache_bit |= CR_M;
> -		flush_dcache_all();
>   	}
> +	reg = get_cr();
> +	cp_delay();
> +	if (cache_bit == (CR_C | CR_M))
> +		flush_dcache_all();
>   	set_cr(reg & ~cache_bit);
  Sorry for the late question.
  But are the two functions that is after flush_dcache_all currently ?

  There is only set_cr, which does not use the stack.

Regards,
  Sricharan
Simon Glass - Jan. 12, 2013, 4:33 p.m.
Hi,

On Thu, Jan 10, 2013 at 11:59 PM, R Sricharan <r.sricharan@ti.com> wrote:
> Hi,
>
>
> On Saturday 01 December 2012 04:31 AM, Simon Glass wrote:
>>
>> From: Arun Mankuzhi <arun.m@samsung.com>
>>
>> In Cortex-A15 architecture, when we run cache invalidate
>> the cache clean operation executes automatically.
>> So if there are any dirty cache lines before disabling the L2 cache
>> these will be synchronized with the main memory when
>> invalidate_dcache_all() runs in the last part of U-boot
>>
>> The two functions after flush_dcache_all is using the stack. So this
>> data will be on the cache. After disable when invalidate is called the
>> data will be flushed from cache to memory. This corrupts the stack in
>> invalida_dcache_all. So this change is required to avoid the u-boot
>> hang.
>>
>> So flush has to be done just before clearing CR_C bit
>>
>> Signed-off-by: Arun Mankuzhi <arun.m@samsung.com>
>> Signed-off-by: Simon Glass <sjg@chromium.org>
>> ---
>> Changes in v2: None
>>
>>   arch/arm/lib/cache-cp15.c |    5 ++++-
>>   1 files changed, 4 insertions(+), 1 deletions(-)
>>
>> diff --git a/arch/arm/lib/cache-cp15.c b/arch/arm/lib/cache-cp15.c
>> index 939de10..06119c8 100644
>> --- a/arch/arm/lib/cache-cp15.c
>> +++ b/arch/arm/lib/cache-cp15.c
>> @@ -124,8 +124,11 @@ static void cache_disable(uint32_t cache_bit)
>>                         return;
>>                 /* if disabling data cache, disable mmu too */
>>                 cache_bit |= CR_M;
>> -               flush_dcache_all();
>>         }
>> +       reg = get_cr();
>> +       cp_delay();
>> +       if (cache_bit == (CR_C | CR_M))
>> +               flush_dcache_all();
>>         set_cr(reg & ~cache_bit);
>
>  Sorry for the late question.
>  But are the two functions that is after flush_dcache_all currently ?
>
>  There is only set_cr, which does not use the stack.

I'm not sure I understand that comment, but Arun is copied on this
email and may have some further comment.

>
> Regards,
>  Sricharan
>
>

Regards,
Simon

Patch

diff --git a/arch/arm/lib/cache-cp15.c b/arch/arm/lib/cache-cp15.c
index 939de10..06119c8 100644
--- a/arch/arm/lib/cache-cp15.c
+++ b/arch/arm/lib/cache-cp15.c
@@ -124,8 +124,11 @@  static void cache_disable(uint32_t cache_bit)
 			return;
 		/* if disabling data cache, disable mmu too */
 		cache_bit |= CR_M;
-		flush_dcache_all();
 	}
+	reg = get_cr();
+	cp_delay();
+	if (cache_bit == (CR_C | CR_M))
+		flush_dcache_all();
 	set_cr(reg & ~cache_bit);
 }
 #endif