Re: [U-Boot] [PATCH] ARM1176: Coexist with other ARM1176 platforms
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 >= 0xc000) > #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, =0xfff0 - 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 (0x7000 - 0x7fff) > > 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. ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] ARM1176: Coexist with other ARM1176 platforms
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. > > 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 > --- > 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 > #include > -#include > #include > > 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) 2007Samsung Electronics > * > @@ -35,7 +35,6 @@ > #ifdef CONFIG_ENABLE_MMU > #include > #endif > -#include > > #if !defined(CONFIG_ENABLE_MMU) && !defined(CONFIG_SYS_PHY_UBOOT_BASE) > #define CONFIG_SYS_PHY_UBOOT_BASECONFIG_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, #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 > + > +#ifdef CONFIG_ENABLE_MMU This logic is may not be quite correct From include/configs/smdk6400.h #if !defined(CONFIG_NAND_SPL) && (TEXT_BASE >= 0xc000) #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, =0x7000 > - orr r0, r0, #0x13 > + ldr r0, =CONFIG_PERIPORT_BASE > + orr r0, r0, #CONFIG_PERIPORT_SIZE > mcr p15,0,r0,c15,c2,4 @ 256M (0x7000 - 0x7fff) 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 */ > +
Re: [U-Boot] [PATCH] ARM1176: Coexist with other ARM1176 platforms
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=0ab8eaa7c4c4472d64f9f401fc2f091dca955abc (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. ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] ARM1176: Coexist with other ARM1176 platforms
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 /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 ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] ARM1176: Coexist with other ARM1176 platforms
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 /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 > , 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. ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] ARM1176: Coexist with other ARM1176 platforms
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. > > 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 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 /lowlevel_init.S I do not want one SOC if-def-ing up another SOC. The maintainer of the original s3c64xx SOC, Guennadi Liakhovetski , 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 ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
[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
---
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
#include
-#include
#include
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
#endif
-#include
#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, #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
+
+#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, =0x7000
- orr r0, r0, #0x13
+ ldr r0, =CONFIG_PERIPORT_BASE
+ orr r0, r0, #CONFIG_PERIPORT_SIZE
mcr p15,0,r0,c15,c2,4 @ 256M (0x7000 - 0x7fff)
+#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
