[PATCH v2 1/4] common/board_f: remove XTRN_DECLARE_GLOBAL_DATA_PTR dead code

The XTRN_DECLARE_GLOBAL_DATA_PTR declarations in ppc code are permanently commented out, so there are no users for this macro: #if 1 #define DECLARE_GLOBAL_DATA_PTR register volatile gd_t *gd asm ("r2") #else /* We could use plain global data, but the resulting code is bigger */ #define XTRN_DECLARE_GLOBAL_DATA_PTR extern #define DECLARE_GLOBAL_DATA_PTR XTRN_DECLARE_GLOBAL_DATA_PTR \ gd_t *gd #endif
Remove all references to this macro, but add a documentation note regarding the possibility of using plain global data for the GD pointer.
Signed-off-by: Ovidiu Panait ovpanait@gmail.com ---
Changes in v2: - update global data documentation with the info provided by Wolfgang
arch/powerpc/include/asm/global_data.h | 6 ------ common/board_f.c | 11 ----------- doc/develop/global_data.rst | 5 +++++ 3 files changed, 5 insertions(+), 17 deletions(-)
diff --git a/arch/powerpc/include/asm/global_data.h b/arch/powerpc/include/asm/global_data.h index 6709e692e6..6ed21c781f 100644 --- a/arch/powerpc/include/asm/global_data.h +++ b/arch/powerpc/include/asm/global_data.h @@ -92,12 +92,6 @@ struct arch_global_data {
#include <asm-generic/global_data.h>
-#if 1 #define DECLARE_GLOBAL_DATA_PTR register volatile gd_t *gd asm ("r2") -#else /* We could use plain global data, but the resulting code is bigger */ -#define XTRN_DECLARE_GLOBAL_DATA_PTR extern -#define DECLARE_GLOBAL_DATA_PTR XTRN_DECLARE_GLOBAL_DATA_PTR \ - gd_t *gd -#endif
#endif /* __ASM_GBL_DATA_H */ diff --git a/common/board_f.c b/common/board_f.c index 18e2246733..f4238d4c90 100644 --- a/common/board_f.c +++ b/common/board_f.c @@ -57,18 +57,7 @@ #include <dm/root.h> #include <linux/errno.h>
-/* - * Pointer to initial global data area - * - * Here we initialize it if needed. - */ -#ifdef XTRN_DECLARE_GLOBAL_DATA_PTR -#undef XTRN_DECLARE_GLOBAL_DATA_PTR -#define XTRN_DECLARE_GLOBAL_DATA_PTR /* empty = allocate here */ -DECLARE_GLOBAL_DATA_PTR = (gd_t *)(CONFIG_SYS_INIT_GD_ADDR); -#else DECLARE_GLOBAL_DATA_PTR; -#endif
/* * TODO(sjg@chromium.org): IMO this code should be diff --git a/doc/develop/global_data.rst b/doc/develop/global_data.rst index 2ac893de49..d143f27eed 100644 --- a/doc/develop/global_data.rst +++ b/doc/develop/global_data.rst @@ -36,6 +36,11 @@ On most architectures the global data pointer is stored in a register.
The sandbox, x86_64, and Xtensa are notable exceptions.
+Current implementation uses a register for the GD pointer because this results +in smaller code. However, using plain global data for the GD pointer would be +possible too (and simpler, as it does not require the reservation of a specific +register for it), but the resulting code is bigger. + Clang for ARM does not support assigning a global register. When using Clang gd is defined as an inline function using assembly code. This adds a few bytes to the code size.

asm/mach_type.h header and CONFIG_MACH_TYPE macro are arm-specific, so move related bdinfo logic to arch_setup_bdinfo() in arch/arm/lib/bdinfo.c.
Reviewed-by: Simon Glass sjg@chromium.org Signed-off-by: Ovidiu Panait ovpanait@gmail.com ---
Changes in v2: - add "Reviewed-by" tag from Simon
arch/arm/lib/bdinfo.c | 12 ++++++++++++ common/board_f.c | 7 ------- 2 files changed, 12 insertions(+), 7 deletions(-)
diff --git a/arch/arm/lib/bdinfo.c b/arch/arm/lib/bdinfo.c index b22ee07b85..826e09e72c 100644 --- a/arch/arm/lib/bdinfo.c +++ b/arch/arm/lib/bdinfo.c @@ -9,9 +9,21 @@ #include <common.h> #include <init.h> #include <asm/global_data.h> +#include <asm/mach-types.h>
DECLARE_GLOBAL_DATA_PTR;
+int arch_setup_bdinfo(void) +{ +#ifdef CONFIG_MACH_TYPE + struct bd_info *bd = gd->bd; + + bd->bi_arch_number = CONFIG_MACH_TYPE; /* board id for Linux */ +#endif + + return 0; +} + void arch_print_bdinfo(void) { struct bd_info *bd = gd->bd; diff --git a/common/board_f.c b/common/board_f.c index f4238d4c90..3789708a30 100644 --- a/common/board_f.c +++ b/common/board_f.c @@ -45,9 +45,6 @@ #include <video.h> #include <watchdog.h> #include <asm/cache.h> -#ifdef CONFIG_MACH_TYPE -#include <asm/mach-types.h> -#endif #if defined(CONFIG_MP) && defined(CONFIG_PPC) #include <asm/mp.h> #endif @@ -588,10 +585,6 @@ int setup_bdinfo(void) bd->bi_sramsize = CONFIG_SYS_SRAM_SIZE; /* size of SRAM */ }
-#ifdef CONFIG_MACH_TYPE - bd->bi_arch_number = CONFIG_MACH_TYPE; /* board id for Linux */ -#endif - return arch_setup_bdinfo(); }

On Tue, Sep 13, 2022 at 09:31:27PM +0300, Ovidiu Panait wrote:
asm/mach_type.h header and CONFIG_MACH_TYPE macro are arm-specific, so move related bdinfo logic to arch_setup_bdinfo() in arch/arm/lib/bdinfo.c.
Reviewed-by: Simon Glass sjg@chromium.org Signed-off-by: Ovidiu Panait ovpanait@gmail.com
Applied to u-boot/master, thanks!

In order to move ppc-specific code out of setup_dest_addr(), provide an arch-specific variant arch_setup_dest_addr(), that can be used by architecture code to fix up the initial reloc address.
It is called at the end of setup_dest_addr() initcall and the default implementation is a nop stub.
Reviewed-by: Simon Glass sjg@chromium.org Signed-off-by: Ovidiu Panait ovpanait@gmail.com ---
Changes in v2: - add "Reviewed-by" tag from Simon
arch/powerpc/lib/stack.c | 17 +++++++++++++++++ common/board_f.c | 21 +++++++-------------- include/init.h | 13 +++++++++++++ 3 files changed, 37 insertions(+), 14 deletions(-)
diff --git a/arch/powerpc/lib/stack.c b/arch/powerpc/lib/stack.c index f2a4652e08..2e731aa870 100644 --- a/arch/powerpc/lib/stack.c +++ b/arch/powerpc/lib/stack.c @@ -13,6 +13,7 @@ #include <common.h> #include <init.h> #include <asm/global_data.h> +#include <asm/mp.h>
DECLARE_GLOBAL_DATA_PTR;
@@ -30,3 +31,19 @@ int arch_reserve_stacks(void)
return 0; } + +int arch_setup_dest_addr(void) +{ +#if defined(CONFIG_MP) && (defined(CONFIG_MPC86xx) || defined(CONFIG_E500)) + /* + * We need to make sure the location we intend to put secondary core + * boot code is reserved and not used by any part of u-boot + */ + if (gd->relocaddr > determine_mp_bootpg(NULL)) { + gd->relocaddr = determine_mp_bootpg(NULL); + debug("Reserving MP boot page to %08lx\n", gd->relocaddr); + } +#endif + + return 0; +} diff --git a/common/board_f.c b/common/board_f.c index 3789708a30..96458c5151 100644 --- a/common/board_f.c +++ b/common/board_f.c @@ -45,9 +45,6 @@ #include <video.h> #include <watchdog.h> #include <asm/cache.h> -#if defined(CONFIG_MP) && defined(CONFIG_PPC) -#include <asm/mp.h> -#endif #include <asm/global_data.h> #include <asm/io.h> #include <asm/sections.h> @@ -307,6 +304,11 @@ __weak ulong board_get_usable_ram_top(ulong total_size) return gd->ram_top; }
+__weak int arch_setup_dest_addr(void) +{ + return 0; +} + static int setup_dest_addr(void) { debug("Monitor len: %08lX\n", gd->mon_len); @@ -334,17 +336,8 @@ static int setup_dest_addr(void) gd->ram_top = board_get_usable_ram_top(gd->mon_len); gd->relocaddr = gd->ram_top; debug("Ram top: %08lX\n", (ulong)gd->ram_top); -#if defined(CONFIG_MP) && (defined(CONFIG_MPC86xx) || defined(CONFIG_E500)) - /* - * We need to make sure the location we intend to put secondary core - * boot code is reserved and not used by any part of u-boot - */ - if (gd->relocaddr > determine_mp_bootpg(NULL)) { - gd->relocaddr = determine_mp_bootpg(NULL); - debug("Reserving MP boot page to %08lx\n", gd->relocaddr); - } -#endif - return 0; + + return arch_setup_dest_addr(); }
#ifdef CONFIG_PRAM diff --git a/include/init.h b/include/init.h index 7b8f62c121..448da34b46 100644 --- a/include/init.h +++ b/include/init.h @@ -103,6 +103,19 @@ phys_size_t get_effective_memsize(void);
int testdram(void);
+/** + * arch_setup_dest_addr() - Fix up initial reloc address + * + * This is called in generic board init sequence in common/board_f.c at the end + * of the setup_dest_addr() initcall. Each architecture could provide this + * function to make adjustments to the initial reloc address. + * + * If an implementation is not provided, it will just be a nop stub. + * + * Return: 0 if OK + */ +int arch_setup_dest_addr(void); + /** * arch_reserve_stacks() - Reserve all necessary stacks *

On Tue, Sep 13, 2022 at 09:31:28PM +0300, Ovidiu Panait wrote:
In order to move ppc-specific code out of setup_dest_addr(), provide an arch-specific variant arch_setup_dest_addr(), that can be used by architecture code to fix up the initial reloc address.
It is called at the end of setup_dest_addr() initcall and the default implementation is a nop stub.
Reviewed-by: Simon Glass sjg@chromium.org Signed-off-by: Ovidiu Panait ovpanait@gmail.com
Applied to u-boot/master, thanks!

Drop the remaining ifdef around spl.h include.
Reviewed-by: Simon Glass sjg@chromium.org Signed-off-by: Ovidiu Panait ovpanait@gmail.com ---
Changes in v2: - add "Reviewed-by" tag from Simon
common/board_f.c | 2 -- 1 file changed, 2 deletions(-)
diff --git a/common/board_f.c b/common/board_f.c index 96458c5151..4db1626c29 100644 --- a/common/board_f.c +++ b/common/board_f.c @@ -35,9 +35,7 @@ #include <post.h> #include <relocate.h> #include <serial.h> -#ifdef CONFIG_SPL #include <spl.h> -#endif #include <status_led.h> #include <sysreset.h> #include <timer.h>

On Tue, Sep 13, 2022 at 09:31:29PM +0300, Ovidiu Panait wrote:
Drop the remaining ifdef around spl.h include.
Reviewed-by: Simon Glass sjg@chromium.org Signed-off-by: Ovidiu Panait ovpanait@gmail.com
Applied to u-boot/master, thanks!

On Tue, 13 Sept 2022 at 12:31, Ovidiu Panait ovpanait@gmail.com wrote:
The XTRN_DECLARE_GLOBAL_DATA_PTR declarations in ppc code are permanently commented out, so there are no users for this macro: #if 1 #define DECLARE_GLOBAL_DATA_PTR register volatile gd_t *gd asm ("r2") #else /* We could use plain global data, but the resulting code is bigger */ #define XTRN_DECLARE_GLOBAL_DATA_PTR extern #define DECLARE_GLOBAL_DATA_PTR XTRN_DECLARE_GLOBAL_DATA_PTR \ gd_t *gd #endif
Remove all references to this macro, but add a documentation note regarding the possibility of using plain global data for the GD pointer.
Signed-off-by: Ovidiu Panait ovpanait@gmail.com
Changes in v2:
- update global data documentation with the info provided by Wolfgang
arch/powerpc/include/asm/global_data.h | 6 ------ common/board_f.c | 11 ----------- doc/develop/global_data.rst | 5 +++++ 3 files changed, 5 insertions(+), 17 deletions(-)
Reviewed-by: Simon Glass sjg@chromium.org

On Tue, Sep 13, 2022 at 09:31:26PM +0300, Ovidiu Panait wrote:
The XTRN_DECLARE_GLOBAL_DATA_PTR declarations in ppc code are permanently commented out, so there are no users for this macro: #if 1 #define DECLARE_GLOBAL_DATA_PTR register volatile gd_t *gd asm ("r2") #else /* We could use plain global data, but the resulting code is bigger */ #define XTRN_DECLARE_GLOBAL_DATA_PTR extern #define DECLARE_GLOBAL_DATA_PTR XTRN_DECLARE_GLOBAL_DATA_PTR \ gd_t *gd #endif
Remove all references to this macro, but add a documentation note regarding the possibility of using plain global data for the GD pointer.
Signed-off-by: Ovidiu Panait ovpanait@gmail.com
Reviewed-by: Tom Rini trini@konsulko.com

On Tue, Sep 13, 2022 at 09:31:26PM +0300, Ovidiu Panait wrote:
The XTRN_DECLARE_GLOBAL_DATA_PTR declarations in ppc code are permanently commented out, so there are no users for this macro: #if 1 #define DECLARE_GLOBAL_DATA_PTR register volatile gd_t *gd asm ("r2") #else /* We could use plain global data, but the resulting code is bigger */ #define XTRN_DECLARE_GLOBAL_DATA_PTR extern #define DECLARE_GLOBAL_DATA_PTR XTRN_DECLARE_GLOBAL_DATA_PTR \ gd_t *gd #endif
Remove all references to this macro, but add a documentation note regarding the possibility of using plain global data for the GD pointer.
Signed-off-by: Ovidiu Panait ovpanait@gmail.com Reviewed-by: Simon Glass sjg@chromium.org Reviewed-by: Tom Rini trini@konsulko.com
Applied to u-boot/master, thanks!
participants (3)
-
Ovidiu Panait
-
Simon Glass
-
Tom Rini