| Message ID | 1326549779-16475-1-git-send-email-urwithsughosh@gmail.com |
|---|---|
| State | Accepted |
| Headers | show |
Hi Sughosh, Le 14/01/2012 15:02, Sughosh Ganu a écrit : > 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. > > Also fix the comments to match code. > > Signed-off-by: Sughosh Ganu<urwithsughosh@gmail.com> > Cc: Albert Aribaud<albert.u.boot@aribaud.net> > Cc: Tom Rini<trini@ti.com> > --- > > Changes since V3 > * Removed tampering of the V bit setting. Would be done in a separate > patch on the lines of review comments by Albert. > > Changes since V2 > * Added code to invalidate I cache, based on review comment by Aneesh. > * Fixed comments to match the code. > > Changes since V1 > * Added arm926 keyword to the subject line > * Removed the superfluous setting of r0. > * Fixed the comment to reflect the fact that V is not being cleared > > arch/arm/cpu/arm926ejs/start.S | 12 ++++++++---- > 1 files changed, 8 insertions(+), 4 deletions(-) > > diff --git a/arch/arm/cpu/arm926ejs/start.S b/arch/arm/cpu/arm926ejs/start.S > index 6a09c02..d64165a 100644 > --- a/arch/arm/cpu/arm926ejs/start.S > +++ b/arch/arm/cpu/arm926ejs/start.S > @@ -355,14 +355,18 @@ _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 */ > - mcr p15, 0, r0, c8, c7, 0 /* flush v4 TLB */ Please add a comment explaining what the loop is waiting for exactly: > +flush_dcache: > + mrc p15, 0, r15, c7, c10, 3 > + bne flush_dcache > + > + mcr p15, 0, r0, c8, c7, 0 /* invalidate TLB */ > + mcr p15, 0, r0, c7, c5, 0 /* invalidate I Cache */ > > /* > - * disable MMU stuff and caches > + * disable MMU and D cache, and enable I cache. > */ > mrc p15, 0, r0, c1, c0, 0 > bic r0, r0, #0x00002300 /* clear bits 13, 9:8 (--V- --RS) */ Amicalement,
Hi Albert, On Saturday, February 18, 2012, Albert ARIBAUD <albert.u.boot@aribaud.net> wrote: > Le 14/01/2012 15:02, Sughosh Ganu a écrit : >> >> 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. >> >> Also fix the comments to match code. >> >> Signed-off-by: Sughosh Ganu<urwithsughosh@gmail.com> >> Cc: Albert Aribaud<albert.u.boot@aribaud.net> >> Cc: Tom Rini<trini@ti.com> >> --- >> >> Changes since V3 >> * Removed tampering of the V bit setting. Would be done in a separate >> patch on the lines of review comments by Albert. >> >> Changes since V2 >> * Added code to invalidate I cache, based on review comment by Aneesh. >> * Fixed comments to match the code. >> >> Changes since V1 >> * Added arm926 keyword to the subject line >> * Removed the superfluous setting of r0. >> * Fixed the comment to reflect the fact that V is not being cleared >> >> arch/arm/cpu/arm926ejs/start.S | 12 ++++++++---- >> 1 files changed, 8 insertions(+), 4 deletions(-) >> >> diff --git a/arch/arm/cpu/arm926ejs/start.S b/arch/arm/cpu/arm926ejs/start.S >> index 6a09c02..d64165a 100644 >> --- a/arch/arm/cpu/arm926ejs/start.S >> +++ b/arch/arm/cpu/arm926ejs/start.S >> @@ -355,14 +355,18 @@ _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 */ >> - mcr p15, 0, r0, c8, c7, 0 /* flush v4 TLB */ > > Please add a comment explaining what the loop is waiting for exactly: > >> +flush_dcache: >> + mrc p15, 0, r15, c7, c10, 3 >> + bne flush_dcache That's the flush dcache code that is given in ARM's TRM for the 926ejs. Anyway, that's already in mainline. Shall we prepare a patch that adds a comment? Regards, Christian
Hi Christian, Le 18/02/2012 19:51, Christian Riesch a écrit : > Hi Albert, > > On Saturday, February 18, 2012, Albert ARIBAUD<albert.u.boot@aribaud.net> > wrote: >> Le 14/01/2012 15:02, Sughosh Ganu a écrit : >>> >>> 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. >>> >>> Also fix the comments to match code. >>> >>> Signed-off-by: Sughosh Ganu<urwithsughosh@gmail.com> >>> Cc: Albert Aribaud<albert.u.boot@aribaud.net> >>> Cc: Tom Rini<trini@ti.com> >>> --- >>> >>> Changes since V3 >>> * Removed tampering of the V bit setting. Would be done in a separate >>> patch on the lines of review comments by Albert. >>> >>> Changes since V2 >>> * Added code to invalidate I cache, based on review comment by Aneesh. >>> * Fixed comments to match the code. >>> >>> Changes since V1 >>> * Added arm926 keyword to the subject line >>> * Removed the superfluous setting of r0. >>> * Fixed the comment to reflect the fact that V is not being cleared >>> >>> arch/arm/cpu/arm926ejs/start.S | 12 ++++++++---- >>> 1 files changed, 8 insertions(+), 4 deletions(-) >>> >>> diff --git a/arch/arm/cpu/arm926ejs/start.S > b/arch/arm/cpu/arm926ejs/start.S >>> index 6a09c02..d64165a 100644 >>> --- a/arch/arm/cpu/arm926ejs/start.S >>> +++ b/arch/arm/cpu/arm926ejs/start.S >>> @@ -355,14 +355,18 @@ _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 */ >>> - mcr p15, 0, r0, c8, c7, 0 /* flush v4 TLB */ >> >> Please add a comment explaining what the loop is waiting for exactly: >> >>> +flush_dcache: >>> + mrc p15, 0, r15, c7, c10, 3 >>> + bne flush_dcache > > That's the flush dcache code that is given in ARM's TRM for the 926ejs. Just because it comes from official ARM documentation does not make it understandable to new readers of the code. :) > Anyway, that's already in mainline. Shall we prepare a patch that adds a > comment? Sorry, I missed the fact it'd gone in already. No, that's ok, I'll just complain again, and earlier, when the file is next touched. > Regards, Christian Amicalement,
diff --git a/arch/arm/cpu/arm926ejs/start.S b/arch/arm/cpu/arm926ejs/start.S index 6a09c02..d64165a 100644 --- a/arch/arm/cpu/arm926ejs/start.S +++ b/arch/arm/cpu/arm926ejs/start.S @@ -355,14 +355,18 @@ _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 */ - mcr p15, 0, r0, c8, c7, 0 /* flush v4 TLB */ +flush_dcache: + mrc p15, 0, r15, c7, c10, 3 + bne flush_dcache + + mcr p15, 0, r0, c8, c7, 0 /* invalidate TLB */ + mcr p15, 0, r0, c7, c5, 0 /* invalidate I Cache */ /* - * disable MMU stuff and caches + * disable MMU and D cache, and enable I cache. */ mrc p15, 0, r0, c1, c0, 0 bic r0, r0, #0x00002300 /* clear bits 13, 9:8 (--V- --RS) */
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. Also fix the comments to match code. Signed-off-by: Sughosh Ganu <urwithsughosh@gmail.com> Cc: Albert Aribaud <albert.u.boot@aribaud.net> Cc: Tom Rini <trini@ti.com> --- Changes since V3 * Removed tampering of the V bit setting. Would be done in a separate patch on the lines of review comments by Albert. Changes since V2 * Added code to invalidate I cache, based on review comment by Aneesh. * Fixed comments to match the code. Changes since V1 * Added arm926 keyword to the subject line * Removed the superfluous setting of r0. * Fixed the comment to reflect the fact that V is not being cleared arch/arm/cpu/arm926ejs/start.S | 12 ++++++++---- 1 files changed, 8 insertions(+), 4 deletions(-)