[U-Boot] [PATCH 0/3] ARM: use r9 for gd instead of r8
To be EABI compliant (r9 is a platform specific register) and as a prepration for building u-boot with clang/llvm (with does / will support r9 as reserved register), store the pointer to gd in r9.
Jeroen Hofstee (3): ARM: make reserving the gd register a make variable ARM,relocate: do not use r9 ARM: use r9 for gd
arch/arm/config.mk | 2 ++ arch/arm/cpu/arm1136/config.mk | 2 +- arch/arm/cpu/arm1176/config.mk | 2 +- arch/arm/cpu/arm720t/config.mk | 2 +- arch/arm/cpu/arm920t/config.mk | 2 +- arch/arm/cpu/arm925t/config.mk | 2 +- arch/arm/cpu/arm926ejs/config.mk | 2 +- arch/arm/cpu/arm946es/config.mk | 2 +- arch/arm/cpu/arm_intcm/config.mk | 2 +- arch/arm/cpu/armv7/config.mk | 2 +- arch/arm/cpu/armv7/rmobile/config.mk | 2 +- arch/arm/cpu/ixp/config.mk | 2 +- arch/arm/cpu/pxa/config.mk | 2 +- arch/arm/cpu/s3c44b0/config.mk | 2 +- arch/arm/cpu/sa1100/config.mk | 2 +- arch/arm/include/asm/global_data.h | 2 +- arch/arm/lib/crt0.S | 16 ++++++++-------- arch/arm/lib/relocate.S | 6 +++--- 18 files changed, 28 insertions(+), 26 deletions(-)
Currently all ARM targets spell out that r8 needs to be a reserved register, while using a common crt0.s. Move this to a common make variable so it is not repeated (and can be easily changed)
Signed-off-by: Jeroen Hofstee jeroen@myspectrum.nl --- arch/arm/config.mk | 2 ++ arch/arm/cpu/arm1136/config.mk | 2 +- arch/arm/cpu/arm1176/config.mk | 2 +- arch/arm/cpu/arm720t/config.mk | 2 +- arch/arm/cpu/arm920t/config.mk | 2 +- arch/arm/cpu/arm925t/config.mk | 2 +- arch/arm/cpu/arm926ejs/config.mk | 2 +- arch/arm/cpu/arm946es/config.mk | 2 +- arch/arm/cpu/arm_intcm/config.mk | 2 +- arch/arm/cpu/armv7/config.mk | 2 +- arch/arm/cpu/armv7/rmobile/config.mk | 2 +- arch/arm/cpu/ixp/config.mk | 2 +- arch/arm/cpu/pxa/config.mk | 2 +- arch/arm/cpu/s3c44b0/config.mk | 2 +- arch/arm/cpu/sa1100/config.mk | 2 +- 15 files changed, 16 insertions(+), 14 deletions(-)
diff --git a/arch/arm/config.mk b/arch/arm/config.mk index 540a119..5e382ab 100644 --- a/arch/arm/config.mk +++ b/arch/arm/config.mk @@ -98,3 +98,5 @@ endif ifneq ($(CONFIG_SPL_BUILD),y) ALL-y += checkarmreloc endif + +OPTION_FIXED_GD=$(call cc-option, -ffixed-r8) diff --git a/arch/arm/cpu/arm1136/config.mk b/arch/arm/cpu/arm1136/config.mk index 1ef6061..daca1bd 100644 --- a/arch/arm/cpu/arm1136/config.mk +++ b/arch/arm/cpu/arm1136/config.mk @@ -4,7 +4,7 @@ # # SPDX-License-Identifier: GPL-2.0+ # -PLATFORM_RELFLAGS += -fno-common -ffixed-r8 -msoft-float +PLATFORM_RELFLAGS += -fno-common $(OPTION_FIXED_GD) -msoft-float
# Make ARMv5 to allow more compilers to work, even though its v6. PLATFORM_CPPFLAGS += -march=armv5 diff --git a/arch/arm/cpu/arm1176/config.mk b/arch/arm/cpu/arm1176/config.mk index 917da03..163778a 100644 --- a/arch/arm/cpu/arm1176/config.mk +++ b/arch/arm/cpu/arm1176/config.mk @@ -4,7 +4,7 @@ # # SPDX-License-Identifier: GPL-2.0+ # -PLATFORM_RELFLAGS += -fno-common -ffixed-r8 -msoft-float +PLATFORM_RELFLAGS += -fno-common $(OPTION_FIXED_GD) -msoft-float
# Make ARMv5 to allow more compilers to work, even though its v6. PLATFORM_CPPFLAGS += -march=armv5t diff --git a/arch/arm/cpu/arm720t/config.mk b/arch/arm/cpu/arm720t/config.mk index 56b6280..f1fb6a8 100644 --- a/arch/arm/cpu/arm720t/config.mk +++ b/arch/arm/cpu/arm720t/config.mk @@ -6,7 +6,7 @@ # SPDX-License-Identifier: GPL-2.0+ #
-PLATFORM_RELFLAGS += -fno-common -ffixed-r8 -msoft-float +PLATFORM_RELFLAGS += -fno-common $(OPTION_FIXED_GD) -msoft-float
PLATFORM_CPPFLAGS += -march=armv4 -mtune=arm7tdmi # ========================================================================= diff --git a/arch/arm/cpu/arm920t/config.mk b/arch/arm/cpu/arm920t/config.mk index 58fd756..a6b2c6f 100644 --- a/arch/arm/cpu/arm920t/config.mk +++ b/arch/arm/cpu/arm920t/config.mk @@ -5,7 +5,7 @@ # SPDX-License-Identifier: GPL-2.0+ #
-PLATFORM_RELFLAGS += -fno-common -ffixed-r8 -msoft-float +PLATFORM_RELFLAGS += -fno-common $(OPTION_FIXED_GD) -msoft-float
PLATFORM_CPPFLAGS += -march=armv4 # ========================================================================= diff --git a/arch/arm/cpu/arm925t/config.mk b/arch/arm/cpu/arm925t/config.mk index 58fd756..a6b2c6f 100644 --- a/arch/arm/cpu/arm925t/config.mk +++ b/arch/arm/cpu/arm925t/config.mk @@ -5,7 +5,7 @@ # SPDX-License-Identifier: GPL-2.0+ #
-PLATFORM_RELFLAGS += -fno-common -ffixed-r8 -msoft-float +PLATFORM_RELFLAGS += -fno-common $(OPTION_FIXED_GD) -msoft-float
PLATFORM_CPPFLAGS += -march=armv4 # ========================================================================= diff --git a/arch/arm/cpu/arm926ejs/config.mk b/arch/arm/cpu/arm926ejs/config.mk index 917ff7e..a29634d 100644 --- a/arch/arm/cpu/arm926ejs/config.mk +++ b/arch/arm/cpu/arm926ejs/config.mk @@ -5,7 +5,7 @@ # SPDX-License-Identifier: GPL-2.0+ #
-PLATFORM_RELFLAGS += -fno-common -ffixed-r8 -msoft-float +PLATFORM_RELFLAGS += -fno-common $(OPTION_FIXED_GD) -msoft-float
PLATFORM_CPPFLAGS += -march=armv5te # ========================================================================= diff --git a/arch/arm/cpu/arm946es/config.mk b/arch/arm/cpu/arm946es/config.mk index 1e41c11..7465da1 100644 --- a/arch/arm/cpu/arm946es/config.mk +++ b/arch/arm/cpu/arm946es/config.mk @@ -5,7 +5,7 @@ # SPDX-License-Identifier: GPL-2.0+ #
-PLATFORM_RELFLAGS += -fno-common -ffixed-r8 -msoft-float +PLATFORM_RELFLAGS += -fno-common $(OPTION_FIXED_GD) -msoft-float
PLATFORM_CPPFLAGS += -march=armv4 # ========================================================================= diff --git a/arch/arm/cpu/arm_intcm/config.mk b/arch/arm/cpu/arm_intcm/config.mk index 1e41c11..7465da1 100644 --- a/arch/arm/cpu/arm_intcm/config.mk +++ b/arch/arm/cpu/arm_intcm/config.mk @@ -5,7 +5,7 @@ # SPDX-License-Identifier: GPL-2.0+ #
-PLATFORM_RELFLAGS += -fno-common -ffixed-r8 -msoft-float +PLATFORM_RELFLAGS += -fno-common $(OPTION_FIXED_GD) -msoft-float
PLATFORM_CPPFLAGS += -march=armv4 # ========================================================================= diff --git a/arch/arm/cpu/armv7/config.mk b/arch/arm/cpu/armv7/config.mk index ca4a9e7..22be8df 100644 --- a/arch/arm/cpu/armv7/config.mk +++ b/arch/arm/cpu/armv7/config.mk @@ -4,7 +4,7 @@ # # SPDX-License-Identifier: GPL-2.0+ # -PLATFORM_RELFLAGS += -fno-common -ffixed-r8 -msoft-float +PLATFORM_RELFLAGS += -fno-common -msoft-float $(OPTION_FIXED_GD)
# If armv7-a is not supported by GCC fall-back to armv5, which is # supported by more tool-chains diff --git a/arch/arm/cpu/armv7/rmobile/config.mk b/arch/arm/cpu/armv7/rmobile/config.mk index 4f01610..fbf4b61 100644 --- a/arch/arm/cpu/armv7/rmobile/config.mk +++ b/arch/arm/cpu/armv7/rmobile/config.mk @@ -4,7 +4,7 @@ # # SPDX-License-Identifier: GPL-2.0+ # -PLATFORM_RELFLAGS += -fno-common -ffixed-r8 -msoft-float +PLATFORM_RELFLAGS += -fno-common $(OPTION_FIXED_GD) -msoft-float
# Make ARMv5 to allow more compilers to work, even though its v7a. PLATFORM_CPPFLAGS += -march=armv5 diff --git a/arch/arm/cpu/ixp/config.mk b/arch/arm/cpu/ixp/config.mk index 0f12f8b..ca2c182 100644 --- a/arch/arm/cpu/ixp/config.mk +++ b/arch/arm/cpu/ixp/config.mk @@ -8,7 +8,7 @@
BIG_ENDIAN = y
-PLATFORM_RELFLAGS += -fno-common -ffixed-r8 -msoft-float -mbig-endian +PLATFORM_RELFLAGS += -fno-common $(OPTION_FIXED_GD) -msoft-float -mbig-endian
PLATFORM_CPPFLAGS += -mbig-endian -march=armv5te -mtune=strongarm1100
diff --git a/arch/arm/cpu/pxa/config.mk b/arch/arm/cpu/pxa/config.mk index 535bca3..c6b8f0b 100644 --- a/arch/arm/cpu/pxa/config.mk +++ b/arch/arm/cpu/pxa/config.mk @@ -6,7 +6,7 @@ # SPDX-License-Identifier: GPL-2.0+ #
-PLATFORM_RELFLAGS += -fno-common -ffixed-r8 -msoft-float +PLATFORM_RELFLAGS += -fno-common $(OPTION_FIXED_GD) -msoft-float
PLATFORM_CPPFLAGS += -mcpu=xscale # ========================================================================= diff --git a/arch/arm/cpu/s3c44b0/config.mk b/arch/arm/cpu/s3c44b0/config.mk index b902ca3..424584a 100644 --- a/arch/arm/cpu/s3c44b0/config.mk +++ b/arch/arm/cpu/s3c44b0/config.mk @@ -6,7 +6,7 @@ # SPDX-License-Identifier: GPL-2.0+ #
-PLATFORM_RELFLAGS += -fno-common -ffixed-r8 -msoft-float +PLATFORM_RELFLAGS += -fno-common $(OPTION_FIXED_GD) -msoft-float
PLATFORM_CPPFLAGS += -march=armv4 -mtune=arm7tdmi -msoft-float # ========================================================================= diff --git a/arch/arm/cpu/sa1100/config.mk b/arch/arm/cpu/sa1100/config.mk index 576f685..d94e855 100644 --- a/arch/arm/cpu/sa1100/config.mk +++ b/arch/arm/cpu/sa1100/config.mk @@ -6,7 +6,7 @@ # SPDX-License-Identifier: GPL-2.0+ #
-PLATFORM_RELFLAGS += -fno-common -ffixed-r8 -msoft-float +PLATFORM_RELFLAGS += -fno-common $(OPTION_FIXED_GD) -msoft-float
PLATFORM_CPPFLAGS += -march=armv4 -mtune=strongarm1100 # =========================================================================
r9 is a platform-specific register in ARM EABI and not per definition a general purpose register. Do not use it while relocating so it can be used for gd.
Signed-off-by: Jeroen Hofstee jeroen@myspectrum.nl --- arch/arm/lib/relocate.S | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/arch/arm/lib/relocate.S b/arch/arm/lib/relocate.S index ab90430..a62a556 100644 --- a/arch/arm/lib/relocate.S +++ b/arch/arm/lib/relocate.S @@ -22,7 +22,7 @@
ENTRY(relocate_code) ldr r1, =__image_copy_start /* r1 <- SRC &__image_copy_start */ - subs r9, r0, r1 /* r9 <- relocation offset */ + subs r4, r0, r1 /* r4 <- relocation offset */ beq relocate_done /* skip relocation */ ldr r2, =__image_copy_end /* r2 <- SRC &__image_copy_end */
@@ -44,9 +44,9 @@ fixloop: bne fixnext
/* relative fix: increase location by offset */ - add r0, r0, r9 + add r0, r0, r4 ldr r1, [r0] - add r1, r1, r9 + add r1, r1, r4 str r1, [r0] fixnext: cmp r2, r3
To be more EABI compliant and as a preparation for building with clang, use the platform-specific r9 register for gd instead of r8.
note: The FIQ is not updated since it is not used in u-boot, and under discussion for the time being.
The following checkpatch warning is ignoredL WARNING: Use of volatile is usually wrong: see Documentation/volatile-considered-harmful.txt
Signed-off-by: Jeroen Hofstee jeroen@myspectrum.nl --- arch/arm/config.mk | 2 +- arch/arm/include/asm/global_data.h | 2 +- arch/arm/lib/crt0.S | 16 ++++++++-------- 3 files changed, 10 insertions(+), 10 deletions(-)
diff --git a/arch/arm/config.mk b/arch/arm/config.mk index 5e382ab..5f6e032 100644 --- a/arch/arm/config.mk +++ b/arch/arm/config.mk @@ -99,4 +99,4 @@ ifneq ($(CONFIG_SPL_BUILD),y) ALL-y += checkarmreloc endif
-OPTION_FIXED_GD=$(call cc-option, -ffixed-r8) +OPTION_FIXED_GD=$(call cc-option, -ffixed-r9) diff --git a/arch/arm/include/asm/global_data.h b/arch/arm/include/asm/global_data.h index 79a9597..e126436 100644 --- a/arch/arm/include/asm/global_data.h +++ b/arch/arm/include/asm/global_data.h @@ -47,6 +47,6 @@ struct arch_global_data {
#include <asm-generic/global_data.h>
-#define DECLARE_GLOBAL_DATA_PTR register volatile gd_t *gd asm ("r8") +#define DECLARE_GLOBAL_DATA_PTR register volatile gd_t *gd asm ("r9")
#endif /* __ASM_GBL_DATA_H */ diff --git a/arch/arm/lib/crt0.S b/arch/arm/lib/crt0.S index 960d12e..ac54b93 100644 --- a/arch/arm/lib/crt0.S +++ b/arch/arm/lib/crt0.S @@ -69,7 +69,7 @@ ENTRY(_main) bic sp, sp, #7 /* 8-byte alignment for ABI compliance */ sub sp, #GD_SIZE /* allocate one GD above SP */ bic sp, sp, #7 /* 8-byte alignment for ABI compliance */ - mov r8, sp /* GD is above SP */ + mov r9, sp /* GD is above SP */ mov r0, #0 bl board_init_f
@@ -81,15 +81,15 @@ ENTRY(_main) * 'here' but relocated. */
- ldr sp, [r8, #GD_START_ADDR_SP] /* sp = gd->start_addr_sp */ + ldr sp, [r9, #GD_START_ADDR_SP] /* sp = gd->start_addr_sp */ bic sp, sp, #7 /* 8-byte alignment for ABI compliance */ - ldr r8, [r8, #GD_BD] /* r8 = gd->bd */ - sub r8, r8, #GD_SIZE /* new GD is below bd */ + ldr r9, [r9, #GD_BD] /* r9 = gd->bd */ + sub r9, r9, #GD_SIZE /* new GD is below bd */
adr lr, here - ldr r0, [r8, #GD_RELOC_OFF] /* r0 = gd->reloc_off */ + ldr r0, [r9, #GD_RELOC_OFF] /* r0 = gd->reloc_off */ add lr, lr, r0 - ldr r0, [r8, #GD_RELOCADDR] /* r0 = gd->relocaddr */ + ldr r0, [r9, #GD_RELOCADDR] /* r0 = gd->relocaddr */ b relocate_code here:
@@ -111,8 +111,8 @@ clbss_l:cmp r0, r1 /* while not at end of BSS */ bl red_led_on
/* call board_init_r(gd_t *id, ulong dest_addr) */ - mov r0, r8 /* gd_t */ - ldr r1, [r8, #GD_RELOCADDR] /* dest_addr */ + mov r0, r9 /* gd_t */ + ldr r1, [r9, #GD_RELOCADDR] /* dest_addr */ /* call board_init_r */ ldr pc, =board_init_r /* this is auto-relocated! */
Dear Jeroen Hofstee,
In message 1376254719-15594-4-git-send-email-jeroen@myspectrum.nl you wrote:
To be more EABI compliant and as a preparation for building with clang, use the platform-specific r9 register for gd instead of r8.
note: The FIQ is not updated since it is not used in u-boot, and under discussion for the time being.
The following checkpatch warning is ignoredL WARNING: Use of volatile is usually wrong: see Documentation/volatile-considered-harmful.txt
Signed-off-by: Jeroen Hofstee jeroen@myspectrum.nl
arch/arm/config.mk | 2 +- arch/arm/include/asm/global_data.h | 2 +- arch/arm/lib/crt0.S | 16 ++++++++-------- 3 files changed, 10 insertions(+), 10 deletions(-)
IIRC, r9 is used as GOT pointer ?
In any case, please also update the README section of register usage on ARM.
Best regards,
Wolfgang Denk
Hi Wolfgang,
On Mon, 12 Aug 2013 07:53:15 +0200, Wolfgang Denk wd@denx.de wrote:
Dear Jeroen Hofstee,
In message 1376254719-15594-4-git-send-email-jeroen@myspectrum.nl you wrote:
To be more EABI compliant and as a preparation for building with clang, use the platform-specific r9 register for gd instead of r8.
note: The FIQ is not updated since it is not used in u-boot, and under discussion for the time being.
The following checkpatch warning is ignoredL WARNING: Use of volatile is usually wrong: see Documentation/volatile-considered-harmful.txt
Signed-off-by: Jeroen Hofstee jeroen@myspectrum.nl
arch/arm/config.mk | 2 +- arch/arm/include/asm/global_data.h | 2 +- arch/arm/lib/crt0.S | 16 ++++++++-------- 3 files changed, 10 insertions(+), 10 deletions(-)
IIRC, r9 is used as GOT pointer ?
No, it is not, well, not any more since GOT-based relocation was replaced by ELF relocation, which requires no reserved register.
In any case, please also update the README section of register usage on ARM.
Best regards,
Wolfgang Denk
Amicalement,
Dear Albert,
In message 20130812164406.5a6807e7@lilith you wrote:
IIRC, r9 is used as GOT pointer ?
No, it is not, well, not any more since GOT-based relocation was replaced by ELF relocation, which requires no reserved register.
In any case, please also update the README section of register usage on ARM.
I see, thanks for the explanation - so this is one more item thatneeds to be fixed in the README.
Best regards,
Wolfgang Denk
Hi,
On 08/12/2013 05:23 PM, Wolfgang Denk wrote:
Dear Albert,
In message 20130812164406.5a6807e7@lilith you wrote:
IIRC, r9 is used as GOT pointer ?
No, it is not, well, not any more since GOT-based relocation was replaced by ELF relocation, which requires no reserved register.
In any case, please also update the README section of register usage on ARM.
I see, thanks for the explanation - so this is one more item thatneeds to be fixed in the README.
Damn: "The role of register r9 is platform specific. A virtual platform may assign any role to this register and _must document_ this usage" ;)
I will update the README for both.
Regards, Jeroen
Dear Jeroen Hofstee,
On Sunday, August 11, 2013 10:58:36 PM, Jeroen Hofstee wrote:
To be EABI compliant (r9 is a platform specific register) and as a prepration for building u-boot with clang/llvm (with does / will support r9 as reserved register), store the pointer to gd in r9.
If r9 is reserved, I understand that its current usage may conflict with clang's but why would gd have to be stored in r9 for clang? Moreover, if r9 is reserved for clang (reserved for what?), why can it be used for gd?
I'm also wondering if r9 as initialized by relocate.S is not sometimes used by GCC to handle position-independent code within generated code, i.e. like the static base feature described for r9 as a possible usage in the ARM EABI AAPCS document (5.1.1). If this is the case, changing r9 to gd would break GCC code at runtime.
Best regards, Benoît
Hi Benoît,
On Mon, 12 Aug 2013 00:08:59 +0200 (CEST), Benoît Thébaudeau benoit.thebaudeau@advansee.com wrote:
Dear Jeroen Hofstee,
On Sunday, August 11, 2013 10:58:36 PM, Jeroen Hofstee wrote:
To be EABI compliant (r9 is a platform specific register) and as a prepration for building u-boot with clang/llvm (with does / will support r9 as reserved register), store the pointer to gd in r9.
If r9 is reserved, I understand that its current usage may conflict with clang's but why would gd have to be stored in r9 for clang? Moreover, if r9 is reserved for clang (reserved for what?), why can it be used for gd?
Actually, you have to think about this the other way around:
1. In LLVM/clang, r9 can be reserved, i.e., LLVM/clang can be told to not touch it at all, because it is the AAPCS platform register.
2. Because r9 is the AAPCS platform register, it is the natural choice for GD.
I'm also wondering if r9 as initialized by relocate.S is not sometimes used by GCC to handle position-independent code within generated code, i.e. like the static base feature described for r9 as a possible usage in the ARM EABI AAPCS document (5.1.1). If this is the case, changing r9 to gd would break GCC code at runtime.
This would be useful when your code gets moved around several times during its lifetime, which is not a requirement in U-Boot. We get perfectly working U-Boot relocation without a base register.
Best regards, Benoît
Amicalement,
Hello Benoît,
On 08/12/2013 12:08 AM, Benoît Thébaudeau wrote:
On Sunday, August 11, 2013 10:58:36 PM, Jeroen Hofstee wrote:
To be EABI compliant (r9 is a platform specific register) and as a prepration for building u-boot with clang/llvm (with does / will support r9 as reserved register), store the pointer to gd in r9.
If r9 is reserved, I understand that its current usage may conflict with clang's but why would gd have to be stored in r9 for clang? Moreover, if r9 is reserved for clang (reserved for what?), why can it be used for gd?
I know Albert already responded to this, but for completeness: The reserved is from a compiler point of view, which is perhaps a bit llvm nomenclature, Reserved means the compiler should _not_ use it as a general purpose register; it is reserved for the platform, U-boot in this case.
I'm also wondering if r9 as initialized by relocate.S is not sometimes used by GCC to handle position-independent code within generated code, i.e. like the static base feature described for r9 as a possible usage in the ARM EABI AAPCS document (5.1.1). If this is the case, changing r9 to gd would break GCC code at runtime.
This is not needed / supported on U-boot arm. U-boot will actually error at compile time if there are any symbols which are not pc relative.
Regards, Jeroen
v2: update the README as requested by Wolfgang Denk
cc: wd@denx.de Jeroen Hofstee (4): ARM: make reserving the gd register a make variable ARM,relocate: do not use r9 ARM: use r9 for gd README: update ARM register usage
README | 8 +++++--- arch/arm/config.mk | 2 ++ arch/arm/cpu/arm1136/config.mk | 2 +- arch/arm/cpu/arm1176/config.mk | 2 +- arch/arm/cpu/arm720t/config.mk | 2 +- arch/arm/cpu/arm920t/config.mk | 2 +- arch/arm/cpu/arm925t/config.mk | 2 +- arch/arm/cpu/arm926ejs/config.mk | 2 +- arch/arm/cpu/arm946es/config.mk | 2 +- arch/arm/cpu/arm_intcm/config.mk | 2 +- arch/arm/cpu/armv7/config.mk | 2 +- arch/arm/cpu/armv7/rmobile/config.mk | 2 +- arch/arm/cpu/ixp/config.mk | 2 +- arch/arm/cpu/pxa/config.mk | 2 +- arch/arm/cpu/s3c44b0/config.mk | 2 +- arch/arm/cpu/sa1100/config.mk | 2 +- arch/arm/include/asm/global_data.h | 2 +- arch/arm/lib/crt0.S | 16 ++++++++-------- arch/arm/lib/relocate.S | 6 +++--- 19 files changed, 33 insertions(+), 29 deletions(-)
Currently all ARM targets spell out that r8 needs to be a reserved register, while using a common crt0.s. Move this to a common make variable so it is not repeated (and can be easily changed)
cc: Albert ARIBAUD albert.u.boot@aribaud.net Signed-off-by: Jeroen Hofstee jeroen@myspectrum.nl --- arch/arm/config.mk | 2 ++ arch/arm/cpu/arm1136/config.mk | 2 +- arch/arm/cpu/arm1176/config.mk | 2 +- arch/arm/cpu/arm720t/config.mk | 2 +- arch/arm/cpu/arm920t/config.mk | 2 +- arch/arm/cpu/arm925t/config.mk | 2 +- arch/arm/cpu/arm926ejs/config.mk | 2 +- arch/arm/cpu/arm946es/config.mk | 2 +- arch/arm/cpu/arm_intcm/config.mk | 2 +- arch/arm/cpu/armv7/config.mk | 2 +- arch/arm/cpu/armv7/rmobile/config.mk | 2 +- arch/arm/cpu/ixp/config.mk | 2 +- arch/arm/cpu/pxa/config.mk | 2 +- arch/arm/cpu/s3c44b0/config.mk | 2 +- arch/arm/cpu/sa1100/config.mk | 2 +- 15 files changed, 16 insertions(+), 14 deletions(-)
diff --git a/arch/arm/config.mk b/arch/arm/config.mk index 540a119..5e382ab 100644 --- a/arch/arm/config.mk +++ b/arch/arm/config.mk @@ -98,3 +98,5 @@ endif ifneq ($(CONFIG_SPL_BUILD),y) ALL-y += checkarmreloc endif + +OPTION_FIXED_GD=$(call cc-option, -ffixed-r8) diff --git a/arch/arm/cpu/arm1136/config.mk b/arch/arm/cpu/arm1136/config.mk index 1ef6061..daca1bd 100644 --- a/arch/arm/cpu/arm1136/config.mk +++ b/arch/arm/cpu/arm1136/config.mk @@ -4,7 +4,7 @@ # # SPDX-License-Identifier: GPL-2.0+ # -PLATFORM_RELFLAGS += -fno-common -ffixed-r8 -msoft-float +PLATFORM_RELFLAGS += -fno-common $(OPTION_FIXED_GD) -msoft-float
# Make ARMv5 to allow more compilers to work, even though its v6. PLATFORM_CPPFLAGS += -march=armv5 diff --git a/arch/arm/cpu/arm1176/config.mk b/arch/arm/cpu/arm1176/config.mk index 917da03..163778a 100644 --- a/arch/arm/cpu/arm1176/config.mk +++ b/arch/arm/cpu/arm1176/config.mk @@ -4,7 +4,7 @@ # # SPDX-License-Identifier: GPL-2.0+ # -PLATFORM_RELFLAGS += -fno-common -ffixed-r8 -msoft-float +PLATFORM_RELFLAGS += -fno-common $(OPTION_FIXED_GD) -msoft-float
# Make ARMv5 to allow more compilers to work, even though its v6. PLATFORM_CPPFLAGS += -march=armv5t diff --git a/arch/arm/cpu/arm720t/config.mk b/arch/arm/cpu/arm720t/config.mk index 56b6280..f1fb6a8 100644 --- a/arch/arm/cpu/arm720t/config.mk +++ b/arch/arm/cpu/arm720t/config.mk @@ -6,7 +6,7 @@ # SPDX-License-Identifier: GPL-2.0+ #
-PLATFORM_RELFLAGS += -fno-common -ffixed-r8 -msoft-float +PLATFORM_RELFLAGS += -fno-common $(OPTION_FIXED_GD) -msoft-float
PLATFORM_CPPFLAGS += -march=armv4 -mtune=arm7tdmi # ========================================================================= diff --git a/arch/arm/cpu/arm920t/config.mk b/arch/arm/cpu/arm920t/config.mk index 58fd756..a6b2c6f 100644 --- a/arch/arm/cpu/arm920t/config.mk +++ b/arch/arm/cpu/arm920t/config.mk @@ -5,7 +5,7 @@ # SPDX-License-Identifier: GPL-2.0+ #
-PLATFORM_RELFLAGS += -fno-common -ffixed-r8 -msoft-float +PLATFORM_RELFLAGS += -fno-common $(OPTION_FIXED_GD) -msoft-float
PLATFORM_CPPFLAGS += -march=armv4 # ========================================================================= diff --git a/arch/arm/cpu/arm925t/config.mk b/arch/arm/cpu/arm925t/config.mk index 58fd756..a6b2c6f 100644 --- a/arch/arm/cpu/arm925t/config.mk +++ b/arch/arm/cpu/arm925t/config.mk @@ -5,7 +5,7 @@ # SPDX-License-Identifier: GPL-2.0+ #
-PLATFORM_RELFLAGS += -fno-common -ffixed-r8 -msoft-float +PLATFORM_RELFLAGS += -fno-common $(OPTION_FIXED_GD) -msoft-float
PLATFORM_CPPFLAGS += -march=armv4 # ========================================================================= diff --git a/arch/arm/cpu/arm926ejs/config.mk b/arch/arm/cpu/arm926ejs/config.mk index 917ff7e..a29634d 100644 --- a/arch/arm/cpu/arm926ejs/config.mk +++ b/arch/arm/cpu/arm926ejs/config.mk @@ -5,7 +5,7 @@ # SPDX-License-Identifier: GPL-2.0+ #
-PLATFORM_RELFLAGS += -fno-common -ffixed-r8 -msoft-float +PLATFORM_RELFLAGS += -fno-common $(OPTION_FIXED_GD) -msoft-float
PLATFORM_CPPFLAGS += -march=armv5te # ========================================================================= diff --git a/arch/arm/cpu/arm946es/config.mk b/arch/arm/cpu/arm946es/config.mk index 1e41c11..7465da1 100644 --- a/arch/arm/cpu/arm946es/config.mk +++ b/arch/arm/cpu/arm946es/config.mk @@ -5,7 +5,7 @@ # SPDX-License-Identifier: GPL-2.0+ #
-PLATFORM_RELFLAGS += -fno-common -ffixed-r8 -msoft-float +PLATFORM_RELFLAGS += -fno-common $(OPTION_FIXED_GD) -msoft-float
PLATFORM_CPPFLAGS += -march=armv4 # ========================================================================= diff --git a/arch/arm/cpu/arm_intcm/config.mk b/arch/arm/cpu/arm_intcm/config.mk index 1e41c11..7465da1 100644 --- a/arch/arm/cpu/arm_intcm/config.mk +++ b/arch/arm/cpu/arm_intcm/config.mk @@ -5,7 +5,7 @@ # SPDX-License-Identifier: GPL-2.0+ #
-PLATFORM_RELFLAGS += -fno-common -ffixed-r8 -msoft-float +PLATFORM_RELFLAGS += -fno-common $(OPTION_FIXED_GD) -msoft-float
PLATFORM_CPPFLAGS += -march=armv4 # ========================================================================= diff --git a/arch/arm/cpu/armv7/config.mk b/arch/arm/cpu/armv7/config.mk index ca4a9e7..22be8df 100644 --- a/arch/arm/cpu/armv7/config.mk +++ b/arch/arm/cpu/armv7/config.mk @@ -4,7 +4,7 @@ # # SPDX-License-Identifier: GPL-2.0+ # -PLATFORM_RELFLAGS += -fno-common -ffixed-r8 -msoft-float +PLATFORM_RELFLAGS += -fno-common -msoft-float $(OPTION_FIXED_GD)
# If armv7-a is not supported by GCC fall-back to armv5, which is # supported by more tool-chains diff --git a/arch/arm/cpu/armv7/rmobile/config.mk b/arch/arm/cpu/armv7/rmobile/config.mk index 4f01610..fbf4b61 100644 --- a/arch/arm/cpu/armv7/rmobile/config.mk +++ b/arch/arm/cpu/armv7/rmobile/config.mk @@ -4,7 +4,7 @@ # # SPDX-License-Identifier: GPL-2.0+ # -PLATFORM_RELFLAGS += -fno-common -ffixed-r8 -msoft-float +PLATFORM_RELFLAGS += -fno-common $(OPTION_FIXED_GD) -msoft-float
# Make ARMv5 to allow more compilers to work, even though its v7a. PLATFORM_CPPFLAGS += -march=armv5 diff --git a/arch/arm/cpu/ixp/config.mk b/arch/arm/cpu/ixp/config.mk index 0f12f8b..ca2c182 100644 --- a/arch/arm/cpu/ixp/config.mk +++ b/arch/arm/cpu/ixp/config.mk @@ -8,7 +8,7 @@
BIG_ENDIAN = y
-PLATFORM_RELFLAGS += -fno-common -ffixed-r8 -msoft-float -mbig-endian +PLATFORM_RELFLAGS += -fno-common $(OPTION_FIXED_GD) -msoft-float -mbig-endian
PLATFORM_CPPFLAGS += -mbig-endian -march=armv5te -mtune=strongarm1100
diff --git a/arch/arm/cpu/pxa/config.mk b/arch/arm/cpu/pxa/config.mk index 535bca3..c6b8f0b 100644 --- a/arch/arm/cpu/pxa/config.mk +++ b/arch/arm/cpu/pxa/config.mk @@ -6,7 +6,7 @@ # SPDX-License-Identifier: GPL-2.0+ #
-PLATFORM_RELFLAGS += -fno-common -ffixed-r8 -msoft-float +PLATFORM_RELFLAGS += -fno-common $(OPTION_FIXED_GD) -msoft-float
PLATFORM_CPPFLAGS += -mcpu=xscale # ========================================================================= diff --git a/arch/arm/cpu/s3c44b0/config.mk b/arch/arm/cpu/s3c44b0/config.mk index b902ca3..424584a 100644 --- a/arch/arm/cpu/s3c44b0/config.mk +++ b/arch/arm/cpu/s3c44b0/config.mk @@ -6,7 +6,7 @@ # SPDX-License-Identifier: GPL-2.0+ #
-PLATFORM_RELFLAGS += -fno-common -ffixed-r8 -msoft-float +PLATFORM_RELFLAGS += -fno-common $(OPTION_FIXED_GD) -msoft-float
PLATFORM_CPPFLAGS += -march=armv4 -mtune=arm7tdmi -msoft-float # ========================================================================= diff --git a/arch/arm/cpu/sa1100/config.mk b/arch/arm/cpu/sa1100/config.mk index 576f685..d94e855 100644 --- a/arch/arm/cpu/sa1100/config.mk +++ b/arch/arm/cpu/sa1100/config.mk @@ -6,7 +6,7 @@ # SPDX-License-Identifier: GPL-2.0+ #
-PLATFORM_RELFLAGS += -fno-common -ffixed-r8 -msoft-float +PLATFORM_RELFLAGS += -fno-common $(OPTION_FIXED_GD) -msoft-float
PLATFORM_CPPFLAGS += -march=armv4 -mtune=strongarm1100 # =========================================================================
On 08/14/2013 08:25 PM, Jeroen Hofstee wrote:
diff --git a/arch/arm/config.mk b/arch/arm/config.mk index 540a119..5e382ab 100644 --- a/arch/arm/config.mk +++ b/arch/arm/config.mk @@ -98,3 +98,5 @@ endif ifneq ($(CONFIG_SPL_BUILD),y) ALL-y += checkarmreloc endif
+OPTION_FIXED_GD=$(call cc-option, -ffixed-r8)
I am leaking some debug code here, this can simply be an assignment by value instead of a cc-option check.
Regards, Jeroen
Hi Jeroen,
On Sat, 17 Aug 2013 15:55:16 +0200, Jeroen Hofstee jeroen@myspectrum.nl wrote:
On 08/14/2013 08:25 PM, Jeroen Hofstee wrote:
diff --git a/arch/arm/config.mk b/arch/arm/config.mk index 540a119..5e382ab 100644 --- a/arch/arm/config.mk +++ b/arch/arm/config.mk @@ -98,3 +98,5 @@ endif ifneq ($(CONFIG_SPL_BUILD),y) ALL-y += checkarmreloc endif
+OPTION_FIXED_GD=$(call cc-option, -ffixed-r8)
I am leaking some debug code here, this can simply be an assignment by value instead of a cc-option check.
Does this mean you'll repost a version of this patch?
Regards, Jeroen
Amicalement,
Hello Albert,
On 09/19/2013 08:57 AM, Albert ARIBAUD wrote:
On Sat, 17 Aug 2013 15:55:16 +0200, Jeroen Hofstee jeroen@myspectrum.nl wrote:
On 08/14/2013 08:25 PM, Jeroen Hofstee wrote:
diff --git a/arch/arm/config.mk b/arch/arm/config.mk index 540a119..5e382ab 100644 --- a/arch/arm/config.mk +++ b/arch/arm/config.mk @@ -98,3 +98,5 @@ endif ifneq ($(CONFIG_SPL_BUILD),y) ALL-y += checkarmreloc endif
+OPTION_FIXED_GD=$(call cc-option, -ffixed-r8)
I am leaking some debug code here, this can simply be an assignment by value instead of a cc-option check.
Does this mean you'll repost a version of this patch?
yup, you get a v3. I intend to rebase on http://patchwork.ozlabs.org/patch/273377/ as well.
Regards, Jeroen
r9 is a platform-specific register in ARM EABI and not per definition a general purpose register. Do not use it while relocating so it can be used for gd.
cc: Albert ARIBAUD albert.u.boot@aribaud.net Signed-off-by: Jeroen Hofstee jeroen@myspectrum.nl --- arch/arm/lib/relocate.S | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/arch/arm/lib/relocate.S b/arch/arm/lib/relocate.S index ab90430..a62a556 100644 --- a/arch/arm/lib/relocate.S +++ b/arch/arm/lib/relocate.S @@ -22,7 +22,7 @@
ENTRY(relocate_code) ldr r1, =__image_copy_start /* r1 <- SRC &__image_copy_start */ - subs r9, r0, r1 /* r9 <- relocation offset */ + subs r4, r0, r1 /* r4 <- relocation offset */ beq relocate_done /* skip relocation */ ldr r2, =__image_copy_end /* r2 <- SRC &__image_copy_end */
@@ -44,9 +44,9 @@ fixloop: bne fixnext
/* relative fix: increase location by offset */ - add r0, r0, r9 + add r0, r0, r4 ldr r1, [r0] - add r1, r1, r9 + add r1, r1, r4 str r1, [r0] fixnext: cmp r2, r3
To be more EABI compliant and as a preparation for building with clang, use the platform-specific r9 register for gd instead of r8.
note: The FIQ is not updated since it is not used in u-boot, and under discussion for the time being.
The following checkpatch warning is ignored: WARNING: Use of volatile is usually wrong: see Documentation/volatile-considered-harmful.txt
cc: Albert ARIBAUD albert.u.boot@aribaud.net Signed-off-by: Jeroen Hofstee jeroen@myspectrum.nl --- arch/arm/config.mk | 2 +- arch/arm/include/asm/global_data.h | 2 +- arch/arm/lib/crt0.S | 16 ++++++++-------- 3 files changed, 10 insertions(+), 10 deletions(-)
diff --git a/arch/arm/config.mk b/arch/arm/config.mk index 5e382ab..5f6e032 100644 --- a/arch/arm/config.mk +++ b/arch/arm/config.mk @@ -99,4 +99,4 @@ ifneq ($(CONFIG_SPL_BUILD),y) ALL-y += checkarmreloc endif
-OPTION_FIXED_GD=$(call cc-option, -ffixed-r8) +OPTION_FIXED_GD=$(call cc-option, -ffixed-r9) diff --git a/arch/arm/include/asm/global_data.h b/arch/arm/include/asm/global_data.h index 79a9597..e126436 100644 --- a/arch/arm/include/asm/global_data.h +++ b/arch/arm/include/asm/global_data.h @@ -47,6 +47,6 @@ struct arch_global_data {
#include <asm-generic/global_data.h>
-#define DECLARE_GLOBAL_DATA_PTR register volatile gd_t *gd asm ("r8") +#define DECLARE_GLOBAL_DATA_PTR register volatile gd_t *gd asm ("r9")
#endif /* __ASM_GBL_DATA_H */ diff --git a/arch/arm/lib/crt0.S b/arch/arm/lib/crt0.S index 960d12e..ac54b93 100644 --- a/arch/arm/lib/crt0.S +++ b/arch/arm/lib/crt0.S @@ -69,7 +69,7 @@ ENTRY(_main) bic sp, sp, #7 /* 8-byte alignment for ABI compliance */ sub sp, #GD_SIZE /* allocate one GD above SP */ bic sp, sp, #7 /* 8-byte alignment for ABI compliance */ - mov r8, sp /* GD is above SP */ + mov r9, sp /* GD is above SP */ mov r0, #0 bl board_init_f
@@ -81,15 +81,15 @@ ENTRY(_main) * 'here' but relocated. */
- ldr sp, [r8, #GD_START_ADDR_SP] /* sp = gd->start_addr_sp */ + ldr sp, [r9, #GD_START_ADDR_SP] /* sp = gd->start_addr_sp */ bic sp, sp, #7 /* 8-byte alignment for ABI compliance */ - ldr r8, [r8, #GD_BD] /* r8 = gd->bd */ - sub r8, r8, #GD_SIZE /* new GD is below bd */ + ldr r9, [r9, #GD_BD] /* r9 = gd->bd */ + sub r9, r9, #GD_SIZE /* new GD is below bd */
adr lr, here - ldr r0, [r8, #GD_RELOC_OFF] /* r0 = gd->reloc_off */ + ldr r0, [r9, #GD_RELOC_OFF] /* r0 = gd->reloc_off */ add lr, lr, r0 - ldr r0, [r8, #GD_RELOCADDR] /* r0 = gd->relocaddr */ + ldr r0, [r9, #GD_RELOCADDR] /* r0 = gd->relocaddr */ b relocate_code here:
@@ -111,8 +111,8 @@ clbss_l:cmp r0, r1 /* while not at end of BSS */ bl red_led_on
/* call board_init_r(gd_t *id, ulong dest_addr) */ - mov r0, r8 /* gd_t */ - ldr r1, [r8, #GD_RELOCADDR] /* dest_addr */ + mov r0, r9 /* gd_t */ + ldr r1, [r9, #GD_RELOCADDR] /* dest_addr */ /* call board_init_r */ ldr pc, =board_init_r /* this is auto-relocated! */
On 08/14/2013 08:25 PM, Jeroen Hofstee wrote:
To be more EABI compliant and as a preparation for building with clang, use the platform-specific r9 register for gd instead of r8.
note: The FIQ is not updated since it is not used in u-boot, and under discussion for the time being.
The following checkpatch warning is ignored: WARNING: Use of volatile is usually wrong: see Documentation/volatile-considered-harmful.txt
cc: Albert ARIBAUD albert.u.boot@aribaud.net Signed-off-by: Jeroen Hofstee jeroen@myspectrum.nl
arch/arm/config.mk | 2 +- arch/arm/include/asm/global_data.h | 2 +- arch/arm/lib/crt0.S | 16 ++++++++-------- 3 files changed, 10 insertions(+), 10 deletions(-)
This patch assumes only crt0.S sets the register used for gd in asm. I just noticed cpu/armv7/lowlevel_init.S does set gd manually, so all users of the common board.c are likely bricked with the patch as is. Looking into it....
Regards, Jeroen
On 08/17/2013 11:40 AM, Jeroen Hofstee wrote:
On 08/14/2013 08:25 PM, Jeroen Hofstee wrote:
To be more EABI compliant and as a preparation for building with clang, use the platform-specific r9 register for gd instead of r8.
note: The FIQ is not updated since it is not used in u-boot, and under discussion for the time being.
The following checkpatch warning is ignored: WARNING: Use of volatile is usually wrong: see Documentation/volatile-considered-harmful.txt
cc: Albert ARIBAUD albert.u.boot@aribaud.net Signed-off-by: Jeroen Hofstee jeroen@myspectrum.nl
arch/arm/config.mk | 2 +- arch/arm/include/asm/global_data.h | 2 +- arch/arm/lib/crt0.S | 16 ++++++++-------- 3 files changed, 10 insertions(+), 10 deletions(-)
This patch assumes only crt0.S sets the register used for gd in asm. I just noticed cpu/armv7/lowlevel_init.S does set gd manually, so all users of the common board.c are likely bricked with the patch as is. Looking into it....
Looking into this, it seems lowlevel_init.S could simply be removed by placing the call to s_init at a better spot. As Tom pointed out, this is already being discussed at [1]. As that is a topic of its own I intend to simply change lowlevel_init.S to us r9 in this patchset.
Regards, Jeroen
[1] http://u-boot.10912.n7.nabble.com/PATCH-1-3-arm-spl-Fix-SPL-booting-for-OMAP...
Hi Jeroen,
On Sat, Aug 17, 2013 at 3:40 AM, Jeroen Hofstee jeroen@myspectrum.nl wrote:
On 08/14/2013 08:25 PM, Jeroen Hofstee wrote:
To be more EABI compliant and as a preparation for building with clang, use the platform-specific r9 register for gd instead of r8.
note: The FIQ is not updated since it is not used in u-boot, and under discussion for the time being.
The following checkpatch warning is ignored: WARNING: Use of volatile is usually wrong: see Documentation/volatile-considered-harmful.txt
cc: Albert ARIBAUD albert.u.boot@aribaud.net Signed-off-by: Jeroen Hofstee jeroen@myspectrum.nl
arch/arm/config.mk | 2 +- arch/arm/include/asm/global_data.h | 2 +- arch/arm/lib/crt0.S | 16 ++++++++-------- 3 files changed, 10 insertions(+), 10 deletions(-)
This patch assumes only crt0.S sets the register used for gd in asm. I just noticed cpu/armv7/lowlevel_init.S does set gd manually, so all users of the common board.c are likely bricked with the patch as is. Looking into it....
I may misunderstood what you are saying here, but I believe that the code in common/board_f.c which creates a global_data on the stack can be removed for ARM now that Albert has tidied all this up with the crt0.S changes, etc. So in common/board_f.c something like:
void board_init_f(ulong boot_flags) { /* These two archs set up the global_data before board_init_f() */ #if !defined(CONFIG_X86) && !defined(CONFIG_ARM) gd_t data;
gd = &data; #endif
gd->flags = boot_flags;
Regards, Simon
Hello Simon,
On 08/19/2013 05:08 AM, Simon Glass wrote:
On Sat, Aug 17, 2013 at 3:40 AM, Jeroen Hofstee jeroen@myspectrum.nl wrote:
This patch assumes only crt0.S sets the register used for gd in asm. I just noticed cpu/armv7/lowlevel_init.S does set gd manually, so all users of the common board.c are likely bricked with the patch as is. Looking into it....
I may misunderstood what you are saying here,
Likely, this is not about how to set reserve gd, but why gd is setup twice. The answer is because some more cleanup is needed (which deserves its own patch).
but I believe that the code in common/board_f.c which creates a global_data on the stack can be removed for ARM now that Albert has tidied all this up with the crt0.S changes, etc. So in common/board_f.c something like:
void board_init_f(ulong boot_flags) { /* These two archs set up the global_data before board_init_f() */ #if !defined(CONFIG_X86) && !defined(CONFIG_ARM) gd_t data;
gd = &data; #endif
gd->flags = boot_flags;
This won't work in general for ARM as board_init_f returns in the ARM specific board (and will be rather ugly for the clang case).
Regards, Jeroen
Hello Simon,
On 08/19/2013 07:32 PM, Jeroen Hofstee wrote:
On 08/19/2013 05:08 AM, Simon Glass wrote:
On Sat, Aug 17, 2013 at 3:40 AM, Jeroen Hofstee jeroen@myspectrum.nl wrote:
This patch assumes only crt0.S sets the register used for gd in asm. I just noticed cpu/armv7/lowlevel_init.S does set gd manually, so all users of the common board.c are likely bricked with the patch as is. Looking into it....
I may misunderstood what you are saying here,
Likely, this is not about how to set reserve gd, but why gd is setup twice. The answer is because some more cleanup is needed (which deserves its own patch).
Right... Now I understand what you were talking about. Gd is actually setup _three_ times in a row:
1) cpu/armv7/lowlevel_init.S 2) ./arch/arm/lib/crt0.S 3) common/board_f.c
but I believe that the code in common/board_f.c which creates a global_data on the stack can be removed for ARM now that Albert has tidied all this up with the crt0.S changes, etc. So in common/board_f.c something like:
void board_init_f(ulong boot_flags) { /* These two archs set up the global_data before board_init_f() */ #if !defined(CONFIG_X86) && !defined(CONFIG_ARM) gd_t data;
gd = &data; #endif
gd->flags = boot_flags;
Yup understood now, this makes sense (but as part of a cleanup patchset)
Thanks, Regards, Jeroen
Hi Jeroen,
On Wed, Aug 21, 2013 at 10:22 AM, Jeroen Hofstee jeroen@myspectrum.nl wrote:
Hello Simon,
On 08/19/2013 07:32 PM, Jeroen Hofstee wrote:
On 08/19/2013 05:08 AM, Simon Glass wrote:
On Sat, Aug 17, 2013 at 3:40 AM, Jeroen Hofstee jeroen@myspectrum.nl wrote:
This patch assumes only crt0.S sets the register used for gd in asm. I just noticed cpu/armv7/lowlevel_init.S does set gd manually, so all users of the common board.c are likely bricked with the patch as is. Looking into it....
I may misunderstood what you are saying here,
Likely, this is not about how to set reserve gd, but why gd is setup twice. The answer is because some more cleanup is needed (which deserves its own patch).
Right... Now I understand what you were talking about. Gd is actually setup _three_ times in a row:
Yes.
- cpu/armv7/lowlevel_init.S
- ./arch/arm/lib/crt0.S
- common/board_f.c
but I believe that the code in common/board_f.c which creates a global_data on the stack can be removed for ARM now that Albert has tidied all this up with the crt0.S changes, etc. So in common/board_f.c something like:
void board_init_f(ulong boot_flags) { /* These two archs set up the global_data before board_init_f() */ #if !defined(CONFIG_X86) && !defined(CONFIG_ARM) gd_t data;
gd = &data; #endif
gd->flags = boot_flags;
Yup understood now, this makes sense (but as part of a cleanup patchset)
Agreed, no suggestion it should be done here - I just wanted to make sure you are not working hard to preserve unnecessary code.
Regards, Simon
Besides the change of this patchset it also updates the README to reflect that GOT-generated relocations are no longer supported on ARM.
cc: Albert ARIBAUD albert.u.boot@aribaud.net Signed-off-by: Jeroen Hofstee jeroen@myspectrum.nl --- README | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/README b/README index 3918807..d8af074 100644 --- a/README +++ b/README @@ -5534,15 +5534,17 @@ On ARM, the following registers are used:
R0: function argument word/integer result R1-R3: function argument word - R9: GOT pointer - R10: stack limit (used only if stack checking if enabled) + R9: platform specific + R10: stack limit (used only if stack checking is enabled) R11: argument (frame) pointer R12: temporary workspace R13: stack pointer R14: link register R15: program counter
- ==> U-Boot will use R8 to hold a pointer to the global data + ==> U-Boot will use R9 to hold a pointer to the global data + + Note: on ARM, only R_ARM_RELATIVE relocations are supported.
On Nios II, the ABI is documented here: http://www.altera.com/literature/hb/nios2/n2cpu_nii51016.pdf
Changes for v3: Drop the unneeded cc-option for the fixed flag.
Drop "make reserving the gd register a make variable" and use http://patchwork.ozlabs.org/patch/273377/ "ARM: refactor compiler options in config.mk" instead.
Changes for v2: Update the README as requested by Wolfgang Denk
Jeroen Hofstee (3): ARM,relocate: do not use r9 ARM: use r9 for gd README: update ARM register usage
README | 8 +++++--- arch/arm/config.mk | 2 +- arch/arm/cpu/armv7/lowlevel_init.S | 4 ++-- arch/arm/include/asm/global_data.h | 2 +- arch/arm/lib/crt0.S | 16 ++++++++-------- arch/arm/lib/relocate.S | 6 +++--- 6 files changed, 20 insertions(+), 18 deletions(-)
r9 is a platform-specific register in ARM EABI and not per definition a general purpose register. Do not use it while relocating so it can be used for gd.
cc: Albert ARIBAUD albert.u.boot@aribaud.net Signed-off-by: Jeroen Hofstee jeroen@myspectrum.nl --- arch/arm/lib/relocate.S | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/arch/arm/lib/relocate.S b/arch/arm/lib/relocate.S index ab90430..a62a556 100644 --- a/arch/arm/lib/relocate.S +++ b/arch/arm/lib/relocate.S @@ -22,7 +22,7 @@
ENTRY(relocate_code) ldr r1, =__image_copy_start /* r1 <- SRC &__image_copy_start */ - subs r9, r0, r1 /* r9 <- relocation offset */ + subs r4, r0, r1 /* r4 <- relocation offset */ beq relocate_done /* skip relocation */ ldr r2, =__image_copy_end /* r2 <- SRC &__image_copy_end */
@@ -44,9 +44,9 @@ fixloop: bne fixnext
/* relative fix: increase location by offset */ - add r0, r0, r9 + add r0, r0, r4 ldr r1, [r0] - add r1, r1, r9 + add r1, r1, r4 str r1, [r0] fixnext: cmp r2, r3
To be more EABI compliant and as a preparation for building with clang, use the platform-specific r9 register for gd instead of r8.
note: The FIQ is not updated since it is not used in u-boot, and under discussion for the time being.
The following checkpatch warning is ignored: WARNING: Use of volatile is usually wrong: see Documentation/volatile-considered-harmful.txt
cc: Albert ARIBAUD albert.u.boot@aribaud.net
--- This patch depends on http://patchwork.ozlabs.org/patch/273377/
Signed-off-by: Jeroen Hofstee jeroen@myspectrum.nl --- arch/arm/config.mk | 2 +- arch/arm/cpu/armv7/lowlevel_init.S | 4 ++-- arch/arm/include/asm/global_data.h | 2 +- arch/arm/lib/crt0.S | 16 ++++++++-------- 4 files changed, 12 insertions(+), 12 deletions(-)
diff --git a/arch/arm/config.mk b/arch/arm/config.mk index 070dfab..6d1bca9 100644 --- a/arch/arm/config.mk +++ b/arch/arm/config.mk @@ -17,7 +17,7 @@ endif
LDFLAGS_FINAL += --gc-sections PLATFORM_RELFLAGS += -ffunction-sections -fdata-sections \ - -fno-common -ffixed-r8 -msoft-float + -fno-common -ffixed-r9 -msoft-float
# Support generic board on ARM __HAVE_ARCH_GENERIC_BOARD := y diff --git a/arch/arm/cpu/armv7/lowlevel_init.S b/arch/arm/cpu/armv7/lowlevel_init.S index 82b2b86..69e3053 100644 --- a/arch/arm/cpu/armv7/lowlevel_init.S +++ b/arch/arm/cpu/armv7/lowlevel_init.S @@ -22,11 +22,11 @@ ENTRY(lowlevel_init) ldr sp, =CONFIG_SYS_INIT_SP_ADDR bic sp, sp, #7 /* 8-byte alignment for ABI compliance */ #ifdef CONFIG_SPL_BUILD - ldr r8, =gdata + ldr r9, =gdata #else sub sp, #GD_SIZE bic sp, sp, #7 - mov r8, sp + mov r9, sp #endif /* * Save the old lr(passed in ip) and the current lr to stack diff --git a/arch/arm/include/asm/global_data.h b/arch/arm/include/asm/global_data.h index 79a9597..e126436 100644 --- a/arch/arm/include/asm/global_data.h +++ b/arch/arm/include/asm/global_data.h @@ -47,6 +47,6 @@ struct arch_global_data {
#include <asm-generic/global_data.h>
-#define DECLARE_GLOBAL_DATA_PTR register volatile gd_t *gd asm ("r8") +#define DECLARE_GLOBAL_DATA_PTR register volatile gd_t *gd asm ("r9")
#endif /* __ASM_GBL_DATA_H */ diff --git a/arch/arm/lib/crt0.S b/arch/arm/lib/crt0.S index 960d12e..ac54b93 100644 --- a/arch/arm/lib/crt0.S +++ b/arch/arm/lib/crt0.S @@ -69,7 +69,7 @@ ENTRY(_main) bic sp, sp, #7 /* 8-byte alignment for ABI compliance */ sub sp, #GD_SIZE /* allocate one GD above SP */ bic sp, sp, #7 /* 8-byte alignment for ABI compliance */ - mov r8, sp /* GD is above SP */ + mov r9, sp /* GD is above SP */ mov r0, #0 bl board_init_f
@@ -81,15 +81,15 @@ ENTRY(_main) * 'here' but relocated. */
- ldr sp, [r8, #GD_START_ADDR_SP] /* sp = gd->start_addr_sp */ + ldr sp, [r9, #GD_START_ADDR_SP] /* sp = gd->start_addr_sp */ bic sp, sp, #7 /* 8-byte alignment for ABI compliance */ - ldr r8, [r8, #GD_BD] /* r8 = gd->bd */ - sub r8, r8, #GD_SIZE /* new GD is below bd */ + ldr r9, [r9, #GD_BD] /* r9 = gd->bd */ + sub r9, r9, #GD_SIZE /* new GD is below bd */
adr lr, here - ldr r0, [r8, #GD_RELOC_OFF] /* r0 = gd->reloc_off */ + ldr r0, [r9, #GD_RELOC_OFF] /* r0 = gd->reloc_off */ add lr, lr, r0 - ldr r0, [r8, #GD_RELOCADDR] /* r0 = gd->relocaddr */ + ldr r0, [r9, #GD_RELOCADDR] /* r0 = gd->relocaddr */ b relocate_code here:
@@ -111,8 +111,8 @@ clbss_l:cmp r0, r1 /* while not at end of BSS */ bl red_led_on
/* call board_init_r(gd_t *id, ulong dest_addr) */ - mov r0, r8 /* gd_t */ - ldr r1, [r8, #GD_RELOCADDR] /* dest_addr */ + mov r0, r9 /* gd_t */ + ldr r1, [r9, #GD_RELOCADDR] /* dest_addr */ /* call board_init_r */ ldr pc, =board_init_r /* this is auto-relocated! */
Hello Albert,
On 09/21/2013 02:04 PM, Jeroen Hofstee wrote:
cc: Albert ARIBAUD albert.u.boot@aribaud.net
This patch depends on http://patchwork.ozlabs.org/patch/273377/
Signed-off-by: Jeroen Hofstee jeroen@myspectrum.nl
Could you be so kind to remove the first --- and comment if you apply this. As is, it will drop the signed off as well... I can send a new version as well of course if you prefer that.
Regards, Jeroen
Besides the change of this patchset it also updates the README to reflect that GOT-generated relocations are no longer supported on ARM.
cc: Albert ARIBAUD albert.u.boot@aribaud.net Signed-off-by: Jeroen Hofstee jeroen@myspectrum.nl --- README | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/README b/README index ccd47fa..ec3e60b 100644 --- a/README +++ b/README @@ -5588,15 +5588,17 @@ On ARM, the following registers are used:
R0: function argument word/integer result R1-R3: function argument word - R9: GOT pointer - R10: stack limit (used only if stack checking if enabled) + R9: platform specific + R10: stack limit (used only if stack checking is enabled) R11: argument (frame) pointer R12: temporary workspace R13: stack pointer R14: link register R15: program counter
- ==> U-Boot will use R8 to hold a pointer to the global data + ==> U-Boot will use R9 to hold a pointer to the global data + + Note: on ARM, only R_ARM_RELATIVE relocations are supported.
On Nios II, the ABI is documented here: http://www.altera.com/literature/hb/nios2/n2cpu_nii51016.pdf
Hi Jeroen,
On Sat, 21 Sep 2013 14:04:39 +0200, Jeroen Hofstee jeroen@myspectrum.nl wrote:
Changes for v3: Drop the unneeded cc-option for the fixed flag.
Drop "make reserving the gd register a make variable" and use http://patchwork.ozlabs.org/patch/273377/ "ARM: refactor compiler options in config.mk" instead.
Changes for v2: Update the README as requested by Wolfgang Denk
Jeroen Hofstee (3): ARM,relocate: do not use r9 ARM: use r9 for gd README: update ARM register usage
README | 8 +++++--- arch/arm/config.mk | 2 +- arch/arm/cpu/armv7/lowlevel_init.S | 4 ++-- arch/arm/include/asm/global_data.h | 2 +- arch/arm/lib/crt0.S | 16 ++++++++-------- arch/arm/lib/relocate.S | 6 +++--- 6 files changed, 20 insertions(+), 18 deletions(-)
Applied to u-boot-arm/master (with the fix to the 2/3 comment), thanks!
Amicalement,
participants (6)
-
Albert ARIBAUD -
Benoît Thébaudeau -
Jeroen Hofstee -
Jeroen Hofstee -
Simon Glass -
Wolfgang Denk