Re: [U-Boot] [PATCH 1/2 V3] arm926: Flush the data cache before disabling it
Hi Christian, Le 18/02/2012 19:51, Christian Riesch a écrit : Hi Albert, On Saturday, February 18, 2012, Albert ARIBAUDalbert.u.b...@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 Ganuurwithsugh...@gmail.com Cc: Albert Aribaudalbert.u.b...@aribaud.net Cc: Tom Rinitr...@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, -- Albert. ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 1/2 V3] arm926: Flush the data cache before disabling it
Hi Albert, On Saturday, February 18, 2012, Albert ARIBAUD albert.u.b...@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 Ganuurwithsugh...@gmail.com Cc: Albert Aribaudalbert.u.b...@aribaud.net Cc: Tom Rinitr...@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 ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 1/2 V3] arm926: Flush the data cache before disabling it
Hi Sughosh, Le 14/01/2012 08:49, 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. 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. Also fix the comments to match code. Signed-off-by: Sughosh Ganuurwithsugh...@gmail.com --- 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 | 14 +- 1 files changed, 9 insertions(+), 5 deletions(-) diff --git a/arch/arm/cpu/arm926ejs/start.S b/arch/arm/cpu/arm926ejs/start.S index 6a09c02..91a9325 100644 --- a/arch/arm/cpu/arm926ejs/start.S +++ b/arch/arm/cpu/arm926ejs/start.S @@ -355,17 +355,21 @@ _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, #0x2300 /* clear bits 13, 9:8 (--V- --RS) */ + bic r0, r0, #0x0300 /* clear bits 9:8 ( --RS) */ NAK--this alters the functioning of U-Boot for many boards in ways unpredictable. If you want to get this specific V change into ARM, then please also add code to set V in all relevant SoCs, or (better yet IMO) make do not set V in cpu_init_crit a config option and set it in the relevant SoCs or boards. bic r0, r0, #0x0087 /* clear bits 7, 2:0 (B--- -CAM) */ orr r0, r0, #0x0002 /* set bit 2 (A) Align */ orr r0, r0, #0x1000 /* set bit 12 (I) I-Cache */ Amicalement, -- Albert. ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 1/2 V3] arm926: Flush the data cache before disabling it
hi Albert, On Sat Jan 14, 2012 at 10:02:16AM +0100, Albert ARIBAUD wrote: snip /* - * disable MMU stuff and caches + * disable MMU and D cache, and enable I cache. */ mrc p15, 0, r0, c1, c0, 0 -bic r0, r0, #0x2300 /* clear bits 13, 9:8 (--V- --RS) */ +bic r0, r0, #0x0300 /* clear bits 9:8 ( --RS) */ NAK--this alters the functioning of U-Boot for many boards in ways unpredictable. If you want to get this specific V change into ARM, then please also add code to set V in all relevant SoCs, or (better yet IMO) make do not set V in cpu_init_crit a config option and set it in the relevant SoCs or boards. Ok, but the problem i have is that i don't have visibility into all the SoC's out there -- don't know what maps where. So i think it should be done by people using those specific SOC's. I can add a config option, and introduce it for my board/SOC. Will that be fine. In any case, i will split this patch into two, with the cache flushing part kept separate, as it fixes a real issue on my board. Will work out the setting of the V bit in a separate patch. -sughosh ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 1/2 V3] arm926: Flush the data cache before disabling it
Hi Sughosh Le 14/01/2012 10:21, Sughosh Ganu a écrit : hi Albert, On Sat Jan 14, 2012 at 10:02:16AM +0100, Albert ARIBAUD wrote: snip /* -* disable MMU stuff and caches +* disable MMU and D cache, and enable I cache. */ mrc p15, 0, r0, c1, c0, 0 - bic r0, r0, #0x2300 /* clear bits 13, 9:8 (--V- --RS) */ + bic r0, r0, #0x0300 /* clear bits 9:8 ( --RS) */ NAK--this alters the functioning of U-Boot for many boards in ways unpredictable. If you want to get this specific V change into ARM, then please also add code to set V in all relevant SoCs, or (better yet IMO) make do not set V in cpu_init_crit a config option and set it in the relevant SoCs or boards. Ok, but the problem i have is that i don't have visibility into all the SoC's out there -- don't know what maps where. So i think it should be done by people using those specific SOC's. I can add a config option, and introduce it for my board/SOC. Will that be fine. For me it is fine: this way, you won't affect code behavior for existing boards. In any case, i will split this patch into two, with the cache flushing part kept separate, as it fixes a real issue on my board. Will work out the setting of the V bit in a separate patch. This is a good idea too. -sughosh Amicalement, -- Albert. ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
