ARM1176: Coexist with other ARM1176 platforms
PATCH 1/1 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.
--- cpu/arm1176/cpu.c | 1 - cpu/arm1176/start.S | 60 ++++++++++++++++++++++++++++++++++++++------ include/configs/smdk6400.h | 6 ++++ 3 files changed, 58 insertions(+), 9 deletions(-)
cpu/arm1176/cpu.c +0 -1
@@ -33,7 +33,6 @@#include <common.h> #include <command.h> -#include <asm/arch/s3c6400.h> #include <asm/system.h> static void cache_flush (void );
cpu/arm1176/start.S +52 -8
118 lines not on this page · raw message
include/configs/smdk6400.h +6 -0
@@ -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 */
Signature · 1 line
1.6.3.3
PATCH 1/1 ARM1176: Coexist with other ARM1176 platforms
Quoted 28 lines · Cyril Chemparathy
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 instart.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.
Quoted 8 lines
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 <[email protected]>
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 <[email protected]>, 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
Chemparathy
PATCH 1/1 ARM1176: Coexist with other ARM1176 platforms
Hi Tom,
Quoted 7 lines
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.cMakefilenew_soc_names3c64xxstart.Su-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.
Quoted 2 lines
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?
Quoted 2 lines
The SOC specific code must be in its own dir. An example of this may be the
lowlevel_initneeds to move fromstart.Sto <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).
Quoted 1 line
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.
Quoted 3 lines
The maintainer of the original s3c64xx SOC, Guennadi Liakhovetski <[email protected]>, 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.
PATCH 1/1 ARM1176: Coexist with other ARM1176 platforms
Quoted 5 lines · Cyril
Hi Tom,
Quoted 3 lines
This patch is premature.
I need to see this patch within the context of the new SOC.
Quoted 8 lines
Quoted 4 lines
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.cMakefilenew_soc_names3c64xxstart.Su-boot.ldsCorrect. 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.
Quoted 5 lines
Quoted 3 lines
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..
Quoted 9 lines
Quoted 3 lines
The SOC specific code must be in its own dir. An example of this may be the
lowlevel_initneeds to move fromstart.Sto <SOC>/lowlevel_init.S
lowlevel_initis _called_ fromstart.Sand is expected to be implemented by SOCs if needed (ifndefCONFIG_SKIP_LOWLEVEL_INIT).Quoted 1 line
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
Chemparathy
PATCH 1/1 ARM1176: Coexist with other ARM1176 platforms
Tom,
Quoted 2 lines
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=0ab8eaa7c4c4472d64f9f401fc2f091dca955abc (included a non-shortened URL this time).
Quoted 2 lines
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.
Quoted 4 lines
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?
Quoted 2 lines
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.
PATCH 1/1 ARM1176: Coexist with other ARM1176 platforms
In reply to Cyril ChemparathyQuoted 73 lines · Cyril Chemparathy
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 instart.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 <[email protected]>--- cpu/arm1176/cpu.c | 1 - cpu/arm1176/start.S | 60 ++++++++++++++++++++++++++++++++++++++------ include/configs/smdk6400.h | 6 ++++ 3 files changed, 58 insertions(+), 9 deletions(-)cpu/arm1176/cpu.c +0
-1@@ -33,7 +33,6 @@#include <common.h> #include <command.h> -#include <asm/arch/s3c6400.h> #include <asm/system.h> static void cache_flush (void );cpu/arm1176/start.S +1
-2@@ -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
cpu/arm1176/start.S reset: · line 147
Quoted 4 lines
cpu/arm1176/start.S +1
-0@@ -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..
cpu/arm1176/start.S cpu_init_crit: · line 174
Quoted 9 lines
cpu/arm1176/start.S +2
-0/* * 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
cpu/arm1176/start.S mmu_disable: · line 194
Quoted 10 lines
cpu/arm1176/start.S +3
-0/* 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?
Quoted 17 lines
cpu/arm1176/start.S +14
-1#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:
cpu/arm1176/start.S mmu_disable: · line 218
Quoted 10 lines
cpu/arm1176/start.S +6
-2+#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.
Quoted 52 lines
cpu/arm1176/start.S +24
-2+#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 + + /* ************************************************************************* *
cpu/arm1176/start.S phy_last_jump: · line 420
Quoted 5 lines
cpu/arm1176/start.S +1
-1@@ -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.
include/configs/smdk6400.h line 43
Quoted 12 lines
cpu/arm1176/start.S +0
-0sub r13, r13, #(CONFIG_SYS_GBL_DATA_SIZE + 8) /* save caller lr in position 0 of saved stack */include/configs/smdk6400.h +1
-0@@ -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.
include/configs/smdk6400.h line 51
Quoted 8 lines
include/configs/smdk6400.h +5
-0+ +#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
PATCH 1/1 ARM1176: Coexist with other ARM1176 platforms
Hi Tom,
[...]
Quoted 7 lines
Quoted 1 line
+#ifndef
CONFIG_SKIP_LOWLEVEL_INIT
CONFIG_SKIP_LOWLEVEL_INITis 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 reallowlevel_init..
Will remove.
Quoted 8 lines
Quoted 1 line
+#ifdef
CONFIG_ENABLE_MMUThis logic is may not be quite correct
Frominclude/configs/smdk6400.h
#if !defined(CONFIG_NAND_SPL) && (TEXT_BASE>=0xc0000000) #defineCONFIG_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?
[...]
Quoted 4 lines
Quoted 4 lines
cpu/arm1176/start.S +0
-0/* 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:
Quoted 8 lines
cpu/arm1176/start.S mmu_disable: · line 194
Quoted 6 lines
cpu/arm1176/start.S +3
-0nop nop mov pc, r2 +mmu_disable_phys: +#else + mcr p15, 0, r0, c1, c0, 0Are 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.
[...]
Quoted 3 lines
cpu/arm1176/start.S mmu_disable: · line 218
Quoted 1 line
cpu/arm1176/start.S +0
-0mcr p15,0,r0,c15,c2,4 @ 256M (0x70000000 - 0x7fffffff)This comment '@ 256 .. ' is no longer valid.
Thanks. Will fix.
[...]
Quoted 5 lines
cpu/arm1176/start.S phy_last_jump: · line 420
Quoted 2 lines
cpu/arm1176/start.S +1
-1- /* 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.
[...]
Quoted 5 lines
include/configs/smdk6400.h line 43
Quoted 1 line
include/configs/smdk6400.h +1
-0+#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.