[U-Boot] [PATCH 1/2 V2] arm926: Flush the data cache before disabling it.

[Heiko Schocher <hs@denx.de>]

Hello Christian,

Christian Riesch wrote:
> Hi Sughosh,
> I had a look at the patch and I tried to understand what's going on
> here (I must confess that I didn't know anything about this cache
> stuff).
> 
> On Tue, Jan 10, 2012 at 7:12 PM, Sughosh Ganu <urwithsughosh@gmail.com> wrote:
>> The current implementation invalidates the cache instead of flushing
>> it. This causes problems on platforms where the spl/u-boot is already
>> loaded to the RAM, with caches enabled by a first stage bootloader.
>>
>> The V bit of the cp15's control register c1 is set to the value of
>> VINITHI on reset. Do not clear this bit by default, as there are SOC's
>> with no valid memory region at 0x0.
> [...]
>> diff --git a/arch/arm/cpu/arm926ejs/start.S b/arch/arm/cpu/arm926ejs/start.S
>> index 6a09c02..6e261c2 100644
>> --- a/arch/arm/cpu/arm926ejs/start.S
>> +++ b/arch/arm/cpu/arm926ejs/start.S
>> @@ -355,17 +355,20 @@ _dynsym_start_ofs:
>>  */
>>  cpu_init_crit:
>>        /*
>> -        * flush v4 I/D caches
>> +        * flush D cache before disabling it
>>         */
>>        mov     r0, #0
>> -       mcr     p15, 0, r0, c7, c7, 0   /* flush v3/v4 cache */
>> +flush_dcache:
>> +       mrc     p15, 0, r15, c7, c10, 3
>> +       bne     flush_dcache
>> +
> 
> Ok, instead of invalidating the D-cache you are cleaning it. From the
> ARM926EJ-S Technical Reference Manual [1]: "To guarantee that memory
> coherency is maintained, the DCache must be cleaned of dirty data
> before it is disabled." So since we are disabling D-Cache a few lines
> later (bic r0, r0, #0x00000087), this must be the right way to do it.

Yes, thats sounds reasonable to me, Albert?

>>        mcr     p15, 0, r0, c8, c7, 0   /* flush v4 TLB */
>>
>>        /*
>>         * disable MMU stuff and caches
>>         */
>>        mrc     p15, 0, r0, c1, c0, 0
>> -       bic     r0, r0, #0x00002300     /* clear bits 13, 9:8 (--V- --RS) */
>> +       bic     r0, r0, #0x00000300     /* clear bits 9:8 ( --RS) */
> 
> Ok, I read your comment above.

Hmm.. what should we do with the V-Bit? Is it OK not to clear it for all
cases?

Tested this patch on the cam_enc_4xx and enbw_cmc board, works fine
on that boards.

> 
>>        bic     r0, r0, #0x00000087     /* clear bits 7, 2:0 (B--- -CAM) */
>>        orr     r0, r0, #0x00000002     /* set bit 2 (A) Align */
>>        orr     r0, r0, #0x00001000     /* set bit 12 (I) I-Cache */
> 
> Although this is not changed in your patch, the last line makes me
> wonder. The comment says "disable MMU stuff and cached", but actually
> the last line sets bit 12 (I), which means that I-Cache gets enabled
> according to [1].

Yes, the last line enables the I-Cache. So we should at least add a
better comment here.

bye,
Heiko
-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany