[U-Boot] [PATCH] ARM1176: Coexist with other ARM1176 platforms
The current ARM1176 CPU specific code is too specific to the SMDK6400 architecture. The following changes were necessary prerequisites for the addition of other SoCs based on ARM1176.
Existing board's (SMDK6400) configuration has been modified to keep behavior unchanged despite these changes.
1. Peripheral port remap configurability The earlier code had hardcoded remap values specific to s3c64xx in start.S. This change makes the peripheral port remap addresses and sizes configurable.
2. Skip low level initialization Ability to skip low level initialization if necessary. Many other platforms have a similar capability, and this is quite useful during debug/bring-up.
3. U-Boot code relocation support Most architectures allow u-boot code to run initially at a different address (possibly in NOR) and then get relocated to its final resting place in RAM. Added support for this capability in ARM1176 architecture.
4. Disable TCM if necessary If a ROM based bootloader happened to have initialized TCM, we disable it here to keep things sane.
5. Remove unnecessary SoC specific includes ARM1176 code does not really need this SoC specific include. The presence of this include prevents builds on other ARM1176 archs.
6. ARM926 style MMU disable when !CONFIG_ENABLE_MMU The original MMU disable code masks out too many bits from the load address when it tries to figure out the physical address of the jump target label. Consequently, it ends up branching to the wrong address after disabling the MMU.
Signed-off-by: Cyril Chemparathy cyril@ti.com --- cpu/arm1176/cpu.c | 1 - cpu/arm1176/start.S | 60 ++++++++++++++++++++++++++++++++++++++------ include/configs/smdk6400.h | 6 ++++ 3 files changed, 58 insertions(+), 9 deletions(-)
diff --git a/cpu/arm1176/cpu.c b/cpu/arm1176/cpu.c index 2c0014f..c0fd114 100644 --- a/cpu/arm1176/cpu.c +++ b/cpu/arm1176/cpu.c @@ -33,7 +33,6 @@
#include <common.h> #include <command.h> -#include <asm/arch/s3c6400.h> #include <asm/system.h>
static void cache_flush (void); diff --git a/cpu/arm1176/start.S b/cpu/arm1176/start.S index 68a356d..beec574 100644 --- a/cpu/arm1176/start.S +++ b/cpu/arm1176/start.S @@ -1,5 +1,5 @@ /* - * armboot - Startup Code for S3C6400/ARM1176 CPU-core + * armboot - Startup Code for ARM1176 CPU-core * * Copyright (c) 2007 Samsung Electronics * @@ -35,7 +35,6 @@ #ifdef CONFIG_ENABLE_MMU #include <asm/proc/domain.h> #endif -#include <asm/arch/s3c6400.h>
#if !defined(CONFIG_ENABLE_MMU) && !defined(CONFIG_SYS_PHY_UBOOT_BASE) #define CONFIG_SYS_PHY_UBOOT_BASE CONFIG_SYS_UBOOT_BASE @@ -145,6 +144,7 @@ reset: * ************************************************************************* */ +#ifndef CONFIG_SKIP_LOWLEVEL_INIT /* * we do sys-critical inits only at reboot, * not when booting from ram! @@ -170,6 +170,8 @@ cpu_init_crit: 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 + +#ifdef CONFIG_ENABLE_MMU /* Prepare to disable the MMU */ adr r1, mmu_disable_phys /* We presume we're within the first 1024 bytes */ @@ -187,20 +189,60 @@ mmu_disable: nop nop mov pc, r2 +mmu_disable_phys: +#else + mcr p15, 0, r0, c1, c0, 0 #endif
-mmu_disable_phys: +#ifdef CONFIG_DISABLE_TCM + /* + * Disable the TCMs + */ + mrc p15, 0, r0, c0, c0, 2 /* Return TCM details */ + cmp r0, #0 + beq skip_tcmdisable + mov r1, #0 + mov r2, #1 + tst r0, r2 + mcrne p15, 0, r1, c9, c1, 1 /* Disable Instruction TCM if present*/ + tst r0, r2, LSL #16 + mcrne p15, 0, r1, c9, c1, 0 /* Disable Data TCM if present*/ +skip_tcmdisable: +#endif +#endif + +#ifdef CONFIG_PERIPORT_REMAP /* Peri port setup */ - ldr r0, =0x70000000 - orr r0, r0, #0x13 + ldr r0, =CONFIG_PERIPORT_BASE + orr r0, r0, #CONFIG_PERIPORT_SIZE mcr p15,0,r0,c15,c2,4 @ 256M (0x70000000 - 0x7fffffff) +#endif
/* * Go setup Memory and board specific bits prior to relocation. */ bl lowlevel_init /* go setup pll,mux,memory */ +#endif /* CONFIG_SKIP_LOWLEVEL_INIT */ + +#ifndef CONFIG_SKIP_RELOCATE_UBOOT +relocate: /* relocate U-Boot to RAM */ + adr r0, _start /* r0 <- current position of code */ + ldr r1, _TEXT_BASE /* test if we run from flash or RAM */ + cmp r0, r1 /* don't reloc during debug */ + beq stack_setup + + ldr r2, _armboot_start + ldr r3, _bss_start + sub r2, r3, r2 /* r2 <- size of armboot */ + add r2, r0, r2 /* r2 <- source end address */ + +copy_loop: + ldmia r0!, {r3-r10} /* copy from source address [r0] */ + stmia r1!, {r3-r10} /* copy to target address [r1] */ + cmp r0, r2 /* until source end addreee [r2] */ + ble copy_loop +#endif /* CONFIG_SKIP_RELOCATE_UBOOT */
-after_copy: #ifdef CONFIG_ENABLE_MMU enable_mmu: /* enable domain access */ @@ -236,9 +278,9 @@ mmu_enable: nop nop mov pc, r2 +skip_hw_init: #endif
-skip_hw_init: /* Set up the stack */ stack_setup: ldr r0, =CONFIG_SYS_UBOOT_BASE /* base of copy in DRAM */ @@ -306,6 +348,8 @@ phy_last_jump: mov r0, #0 mov pc, r9 #endif + + /* ************************************************************************* * @@ -373,7 +417,7 @@ phy_last_jump: ldr r13, _armboot_start /* move past malloc pool */ sub r13, r13, #(CONFIG_SYS_MALLOC_LEN) - /* move to reserved a couple spots for abort stack */ + /* reserved a couple spots for abort stack */ sub r13, r13, #(CONFIG_SYS_GBL_DATA_SIZE + 8)
/* save caller lr in position 0 of saved stack */ diff --git a/include/configs/smdk6400.h b/include/configs/smdk6400.h index f04feae..7b4501d 100644 --- a/include/configs/smdk6400.h +++ b/include/configs/smdk6400.h @@ -40,6 +40,12 @@ #define CONFIG_S3C64XX 1 /* in a SAMSUNG S3C64XX Family */ #define CONFIG_SMDK6400 1 /* on a SAMSUNG SMDK6400 Board */
+#define CONFIG_SKIP_RELOCATE_UBOOT + +#define CONFIG_PERIPORT_REMAP +#define CONFIG_PERIPORT_BASE 0x70000000 +#define CONFIG_PERIPORT_SIZE 0x13 + #define CONFIG_SYS_SDRAM_BASE 0x50000000
/* input clock of PLL: SMDK6400 has 12MHz input clock */
Cyril Chemparathy wrote:
The current ARM1176 CPU specific code is too specific to the SMDK6400 architecture. The following changes were necessary prerequisites for the addition of other SoCs based on ARM1176.
Existing board's (SMDK6400) configuration has been modified to keep behavior unchanged despite these changes.
- Peripheral port remap configurability
The earlier code had hardcoded remap values specific to s3c64xx in start.S. This change makes the peripheral port remap addresses and sizes configurable.
- Skip low level initialization
Ability to skip low level initialization if necessary. Many other platforms have a similar capability, and this is quite useful during debug/bring-up.
- U-Boot code relocation support
Most architectures allow u-boot code to run initially at a different address (possibly in NOR) and then get relocated to its final resting place in RAM. Added support for this capability in ARM1176 architecture.
- Disable TCM if necessary
If a ROM based bootloader happened to have initialized TCM, we disable it here to keep things sane.
- Remove unnecessary SoC specific includes
ARM1176 code does not really need this SoC specific include. The presence of this include prevents builds on other ARM1176 archs.
- ARM926 style MMU disable when !CONFIG_ENABLE_MMU
The original MMU disable code masks out too many bits from the load address when it tries to figure out the physical address of the jump target label. Consequently, it ends up branching to the wrong address after disabling the MMU.
Signed-off-by: Cyril Chemparathy cyril@ti.com
This patch is premature. I need to see this patch within the context of the new SOC.
For a new SOC, I would like it be added as a new sub dir off of cpu/arm1176. At the same level as s3c64xx. So this dir would look like.
config.mk cpu.c Makefile new_soc_name s3c64xx start.S u-boot.lds
The common code that is sharable should also be at this level. This may mean moving and generalizing some s3c64xx/*.c. The SOC specific code must be in its own dir. An example of this may be the lowlevel_init needs to move from start.S to <SOC>/lowlevel_init.S
I do not want one SOC if-def-ing up another SOC.
The maintainer of the original s3c64xx SOC, Guennadi Liakhovetski g.liakhovetski@gmx.de, should be cc-ed on at least the initial changes so he has a heads up that some of his code is being moved/generalized.
Tom
Hi Tom,
This patch is premature. I need to see this patch within the context of the new SOC.
For a new SOC, I would like it be added as a new sub dir off of cpu/arm1176. At the same level as s3c64xx. So this dir would look like.
config.mk cpu.c Makefile new_soc_name s3c64xx start.S u-boot.lds
Correct. The new SOC adds cpu/arm1176/tnetv107x/.
Would you prefer if I were to include this patch as part of the initial tnetv107x submission? You could take a peek at this future submission at http://bit.ly/b1F2qX.
The common code that is sharable should also be at this level. This may mean moving and generalizing some s3c64xx/*.c.
I have taken a look at the code inside s3c64xx, and found it specific to that SOC (memory interface initialization, reset, etc.). Therefore, I don't think any of that code can be generalized and pulled out into cpu/arm1176/. Guennadi, do you agree with this assessment?
The SOC specific code must be in its own dir. An example of this may be the lowlevel_init needs to move from start.S to <SOC>/lowlevel_init.S
lowlevel_init is _called_ from start.S and is expected to be implemented by SOCs if needed (ifndef CONFIG_SKIP_LOWLEVEL_INIT).
I do not want one SOC if-def-ing up another SOC.
Absolutely. I understand your concern, but this patch if-defs up arm1176 code, and not s3c64xx SOC code.
The maintainer of the original s3c64xx SOC, Guennadi Liakhovetski g.liakhovetski@gmx.de, should be cc-ed on at least the initial changes so he has a heads up that some of his code is being moved/generalized.
Thanks.
Cyril.
Chemparathy, Cyril wrote:
Hi Tom,
This patch is premature. I need to see this patch within the context of the new SOC.
For a new SOC, I would like it be added as a new sub dir off of cpu/arm1176. At the same level as s3c64xx. So this dir would look like.
config.mk cpu.c Makefile new_soc_name s3c64xx start.S u-boot.lds
Correct. The new SOC adds cpu/arm1176/tnetv107x/.
Would you prefer if I were to include this patch as part of the initial tnetv107x submission? You could take a peek at this future submission at http://bit.ly/b1F2qX.
I was not able to access this link But, yes, please include this patch as part of the tnetv107x patchset.
The common code that is sharable should also be at this level. This may mean moving and generalizing some s3c64xx/*.c.
I have taken a look at the code inside s3c64xx, and found it specific to that SOC (memory interface initialization, reset, etc.). Therefore, I don't think any of that code can be generalized and pulled out into cpu/arm1176/. Guennadi, do you agree with this assessment?
Ok. Obviously it is better to generalize than to make off-by-one copies but not if there are not enough similarities to make it work..
The SOC specific code must be in its own dir. An example of this may be the lowlevel_init needs to move from start.S to <SOC>/lowlevel_init.S
lowlevel_init is _called_ from start.S and is expected to be implemented by SOCs if needed (ifndef CONFIG_SKIP_LOWLEVEL_INIT).
I do not want one SOC if-def-ing up another SOC.
Absolutely. I understand your concern, but this patch if-defs up arm1176 code, and not s3c64xx SOC code.
My concern is that start.S, because it came originally from s3c64xx, may need to either generalized or split up. With s3c64xx parts moving to SOC layer. Assembly is complicated enough without having #if-def's and if can be done in C it should.
Please review http://www.denx.de/wiki/U-Boot/Patches, clean up the patchset and submit, having another arm1176 soc will be great.
Thanks Tom
Tom,
I was not able to access this link But, yes, please include this patch as part of the tnetv107x patchset.
Sure. I will include this with the larger TNETV107X patchset and submit.
Until then, a code preview can be found at http://arago-project.org/git/people/?p=cyril/u-boot-tnetv107x.git;a=commit;h... (included a non-shortened URL this time).
Ok. Obviously it is better to generalize than to make off-by-one copies but not if there are not enough similarities to make it work..
Absolutely. In this particular case, it just happens to be the latter.
My concern is that start.S, because it came originally from s3c64xx, may need to either generalized or split up. With s3c64xx parts moving to SOC layer. Assembly is complicated enough without having #if-def's and if can be done in C it should.
Indeed start.S needed to be generalized to an extent. Further, since similar generalizations existed on other ARM platforms, I figured it would be best to stick with the same scheme.
My only reservation is with the whole "peripheral port remap" thing. I think that this should ideally be moved into SOC specific lowlevel_init. However, I did not want to mangle up s3c64xx code too much by moving stuff around in this manner.
Any thoughts on this specific concern?
Please review http://www.denx.de/wiki/U-Boot/Patches, clean up the patchset and submit, having another arm1176 soc will be great.
Will do. Thanks.
Cyril.
Cyril Chemparathy wrote:
The current ARM1176 CPU specific code is too specific to the SMDK6400 architecture. The following changes were necessary prerequisites for the addition of other SoCs based on ARM1176.
Existing board's (SMDK6400) configuration has been modified to keep behavior unchanged despite these changes.
- Peripheral port remap configurability
The earlier code had hardcoded remap values specific to s3c64xx in start.S. This change makes the peripheral port remap addresses and sizes configurable.
- Skip low level initialization
Ability to skip low level initialization if necessary. Many other platforms have a similar capability, and this is quite useful during debug/bring-up.
- U-Boot code relocation support
Most architectures allow u-boot code to run initially at a different address (possibly in NOR) and then get relocated to its final resting place in RAM. Added support for this capability in ARM1176 architecture.
- Disable TCM if necessary
If a ROM based bootloader happened to have initialized TCM, we disable it here to keep things sane.
- Remove unnecessary SoC specific includes
ARM1176 code does not really need this SoC specific include. The presence of this include prevents builds on other ARM1176 archs.
- ARM926 style MMU disable when !CONFIG_ENABLE_MMU
The original MMU disable code masks out too many bits from the load address when it tries to figure out the physical address of the jump target label. Consequently, it ends up branching to the wrong address after disabling the MMU.
Signed-off-by: Cyril Chemparathy cyril@ti.com
cpu/arm1176/cpu.c | 1 - cpu/arm1176/start.S | 60 ++++++++++++++++++++++++++++++++++++++------ include/configs/smdk6400.h | 6 ++++ 3 files changed, 58 insertions(+), 9 deletions(-)
diff --git a/cpu/arm1176/cpu.c b/cpu/arm1176/cpu.c index 2c0014f..c0fd114 100644 --- a/cpu/arm1176/cpu.c +++ b/cpu/arm1176/cpu.c @@ -33,7 +33,6 @@
#include <common.h> #include <command.h> -#include <asm/arch/s3c6400.h> #include <asm/system.h>
static void cache_flush (void); diff --git a/cpu/arm1176/start.S b/cpu/arm1176/start.S index 68a356d..beec574 100644 --- a/cpu/arm1176/start.S +++ b/cpu/arm1176/start.S @@ -1,5 +1,5 @@ /*
- armboot - Startup Code for S3C6400/ARM1176 CPU-core
- armboot - Startup Code for ARM1176 CPU-core
- Copyright (c) 2007 Samsung Electronics
@@ -35,7 +35,6 @@ #ifdef CONFIG_ENABLE_MMU #include <asm/proc/domain.h> #endif -#include <asm/arch/s3c6400.h>
#if !defined(CONFIG_ENABLE_MMU) && !defined(CONFIG_SYS_PHY_UBOOT_BASE) #define CONFIG_SYS_PHY_UBOOT_BASE CONFIG_SYS_UBOOT_BASE @@ -145,6 +144,7 @@ reset:
*/ +#ifndef CONFIG_SKIP_LOWLEVEL_INIT
CONFIG_SKIP_LOWLEVEL_INIT is not used in the other patches. Why is this needed ? board/samsung/samsung/smdk6400 has a lowlevel_init.o function. It is confusing why this function is being if-def and not the real lowlevel_init..
/* * we do sys-critical inits only at reboot, * not when booting from ram! @@ -170,6 +170,8 @@ cpu_init_crit: 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
+#ifdef CONFIG_ENABLE_MMU
This logic is may not be quite correct From include/configs/smdk6400.h #if !defined(CONFIG_NAND_SPL) && (TEXT_BASE >= 0xc0000000) #define CONFIG_ENABLE_MMU #endif Please check
/* Prepare to disable the MMU */ adr r1, mmu_disable_phys /* We presume we're within the first 1024 bytes */ @@ -187,20 +189,60 @@ mmu_disable: nop nop mov pc, r2 +mmu_disable_phys: +#else
- mcr p15, 0, r0, c1, c0, 0
Are the noop's above needed here?
#endif
-mmu_disable_phys: +#ifdef CONFIG_DISABLE_TCM
- /*
* Disable the TCMs*/- mrc p15, 0, r0, c0, c0, 2 /* Return TCM details */
- cmp r0, #0
- beq skip_tcmdisable
- mov r1, #0
- mov r2, #1
- tst r0, r2
- mcrne p15, 0, r1, c9, c1, 1 /* Disable Instruction TCM if present*/
- tst r0, r2, LSL #16
- mcrne p15, 0, r1, c9, c1, 0 /* Disable Data TCM if present*/
+skip_tcmdisable: +#endif +#endif
+#ifdef CONFIG_PERIPORT_REMAP /* Peri port setup */
- ldr r0, =0x70000000
- orr r0, r0, #0x13
- ldr r0, =CONFIG_PERIPORT_BASE
- orr r0, r0, #CONFIG_PERIPORT_SIZE mcr p15,0,r0,c15,c2,4 @ 256M (0x70000000 - 0x7fffffff)
This comment '@ 256 .. ' is no longer valid.
+#endif
/* * Go setup Memory and board specific bits prior to relocation. */ bl lowlevel_init /* go setup pll,mux,memory */ +#endif /* CONFIG_SKIP_LOWLEVEL_INIT */
+#ifndef CONFIG_SKIP_RELOCATE_UBOOT +relocate: /* relocate U-Boot to RAM */
- adr r0, _start /* r0 <- current position of code */
- ldr r1, _TEXT_BASE /* test if we run from flash or RAM */
- cmp r0, r1 /* don't reloc during debug */
- beq stack_setup
- ldr r2, _armboot_start
- ldr r3, _bss_start
- sub r2, r3, r2 /* r2 <- size of armboot */
- add r2, r0, r2 /* r2 <- source end address */
+copy_loop:
- ldmia r0!, {r3-r10} /* copy from source address [r0] */
- stmia r1!, {r3-r10} /* copy to target address [r1] */
- cmp r0, r2 /* until source end addreee [r2] */
- ble copy_loop
+#endif /* CONFIG_SKIP_RELOCATE_UBOOT */
-after_copy: #ifdef CONFIG_ENABLE_MMU enable_mmu: /* enable domain access */ @@ -236,9 +278,9 @@ mmu_enable: nop nop mov pc, r2 +skip_hw_init: #endif
-skip_hw_init: /* Set up the stack */ stack_setup: ldr r0, =CONFIG_SYS_UBOOT_BASE /* base of copy in DRAM */ @@ -306,6 +348,8 @@ phy_last_jump: mov r0, #0 mov pc, r9 #endif
/*
@@ -373,7 +417,7 @@ phy_last_jump: ldr r13, _armboot_start /* move past malloc pool */ sub r13, r13, #(CONFIG_SYS_MALLOC_LEN)
- /* move to reserved a couple spots for abort stack */
- /* reserved a couple spots for abort stack */
The old comment makes more sense. Revert this.
sub r13, r13, #(CONFIG_SYS_GBL_DATA_SIZE + 8)
/* save caller lr in position 0 of saved stack */ diff --git a/include/configs/smdk6400.h b/include/configs/smdk6400.h index f04feae..7b4501d 100644 --- a/include/configs/smdk6400.h +++ b/include/configs/smdk6400.h @@ -40,6 +40,12 @@ #define CONFIG_S3C64XX 1 /* in a SAMSUNG S3C64XX Family */ #define CONFIG_SMDK6400 1 /* on a SAMSUNG SMDK6400 Board */
+#define CONFIG_SKIP_RELOCATE_UBOOT
There is logic later in this file to undef this value. This is likely an error. If it isn't, add a comment.
+#define CONFIG_PERIPORT_REMAP +#define CONFIG_PERIPORT_BASE 0x70000000 +#define CONFIG_PERIPORT_SIZE 0x13
#define CONFIG_SYS_SDRAM_BASE 0x50000000
/* input clock of PLL: SMDK6400 has 12MHz input clock */
Tom
Hi Tom,
[...]
+#ifndef CONFIG_SKIP_LOWLEVEL_INIT
CONFIG_SKIP_LOWLEVEL_INIT is not used in the other patches. Why is this needed ? board/samsung/samsung/smdk6400 has a lowlevel_init.o function. It is confusing why this function is being if-def and not the real lowlevel_init..
Will remove.
+#ifdef CONFIG_ENABLE_MMU
This logic is may not be quite correct From include/configs/smdk6400.h #if !defined(CONFIG_NAND_SPL) && (TEXT_BASE >= 0xc0000000) #define CONFIG_ENABLE_MMU #endif Please check
Thanks. This logic was broken.
I have reworked this logic as follows, and removed the ifdef:
- adr r1, mmu_disable_phys - /* We presume we're within the first 1024 bytes */ - and r1, r1, #0x3fc - ldr r2, _TEXT_PHY_BASE - ldr r3, =0xfff00000 - and r2, r2, r3 - orr r2, r2, r1 + adr r2, mmu_disable_phys + sub r2, r2, #(CONFIG_SYS_PHY_UBOOT_BASE - TEXT_BASE)
The earlier implementation was masking out too many bits in trying to convert the jump address from virtual to physical. Any comments on this change?
[...]
/* Prepare to disable the MMU */ adr r1, mmu_disable_phys /* We presume we're within the first 1024 bytes */ @@ -187,20 +189,60 @@ mmu_disable: nop nop mov pc, r2 +mmu_disable_phys: +#else
- mcr p15, 0, r0, c1, c0, 0
Are the noop's above needed here?
I think the original author's intent was to entirely occupy one cache line. I don't know enough about the s3c64xx architecture to comment.
[...]
mcr p15,0,r0,c15,c2,4 @ 256M (0x70000000 - 0x7fffffff)
This comment '@ 256 .. ' is no longer valid.
Thanks. Will fix.
[...]
- /* move to reserved a couple spots for abort stack */
- /* reserved a couple spots for abort stack */
The old comment makes more sense. Revert this.
Thanks. Will fix.
[...]
+#define CONFIG_SKIP_RELOCATE_UBOOT
There is logic later in this file to undef this value. This is likely an error. If it isn't, add a comment.
I have removed the subsequent undef and verified that the generated disassembly of start.S remains the same (with the exception of the address computation change above) for both usb and non-usb smdk6400 builds.
Thanks Cyril.
participants (4)
-
Chemparathy, Cyril -
Cyril Chemparathy
-
Tom -
Tom Rix