On Fri, Jan 13, 2012 at 1:26 AM, Sughosh Ganu <urwithsugh...@gmail.com> wrote: > hi Christian, > > On Fri Jan 13, 2012 at 09:06:26AM +0100, 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). > > Ok, thanks for taking time off to understand this issue. > >> >> On Tue, Jan 10, 2012 at 7:12 PM, Sughosh Ganu <urwithsugh...@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. > > Right, so the problem here is that instead of flushing the cache, > the code currently invalidates it. This would not be a problem on > platforms where cache is not turned ON(which would be the majority > of cases), but on my board, it seems to be turned ON by the rbl, > due to which it does not boot. > > If you check the manual, the loop that i have introduced is one for > "Testing and Cleaning", which means that in case the lines are not > dirty, it won't be flushed. So this should not break any platforms > where the cache isn't enabled. > >> >> > 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. >> >> > 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]. > > Yeah, this seems to be copied code, with discrepancies in the code > and comments. You would see that the line i have removed has a > comment for flushing the cache, but instead it is invalidating the > cache. I have just fixed the comments for the lines that i made > changes to.
I think while we're in here and noticing these things we should fix the comments at least. -- Tom _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
