[PATCH 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 #define XTRN_DECLARE_GLOBAL_DATA_PTR extern #define DECLARE_GLOBAL_DATA_PTR XTRN_DECLARE_GLOBAL_DATA_PTR \ gd_t *gd #endif
Remove all references.
Signed-off-by: Ovidiu Panait ovpanait@gmail.com ---
arch/powerpc/include/asm/global_data.h | 6 ------ common/board_f.c | 11 ----------- 2 files changed, 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

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.
Signed-off-by: Ovidiu Panait ovpanait@gmail.com ---
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 Sun, 11 Sept 2022 at 10:11, Ovidiu Panait ovpanait@gmail.com 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.
Signed-off-by: Ovidiu Panait ovpanait@gmail.com
arch/arm/lib/bdinfo.c | 12 ++++++++++++ common/board_f.c | 7 ------- 2 files changed, 12 insertions(+), 7 deletions(-)
Reviewed-by: Simon Glass sjg@chromium.org

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.
Signed-off-by: Ovidiu Panait ovpanait@gmail.com ---
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 Sun, 11 Sept 2022 at 10:11, Ovidiu Panait ovpanait@gmail.com 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.
Signed-off-by: Ovidiu Panait ovpanait@gmail.com
arch/powerpc/lib/stack.c | 17 +++++++++++++++++ common/board_f.c | 21 +++++++-------------- include/init.h | 13 +++++++++++++ 3 files changed, 37 insertions(+), 14 deletions(-)
Reviewed-by: Simon Glass sjg@chromium.org

Drop the remaining ifdef around spl.h include.
Signed-off-by: Ovidiu Panait ovpanait@gmail.com ---
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 Sun, 11 Sept 2022 at 10:11, Ovidiu Panait ovpanait@gmail.com wrote:
Drop the remaining ifdef around spl.h include.
Signed-off-by: Ovidiu Panait ovpanait@gmail.com
common/board_f.c | 2 -- 1 file changed, 2 deletions(-)
Reviewed-by: Simon Glass sjg@chromium.org

Dear Ovidiu,
In message 20220911161052.2986264-1-ovpanait@gmail.com you 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 #define XTRN_DECLARE_GLOBAL_DATA_PTR extern #define DECLARE_GLOBAL_DATA_PTR XTRN_DECLARE_GLOBAL_DATA_PTR \ gd_t *gd #endif
Remove all references.
Actually the commented out code contained some information, and I feel it would be a pity if that got lost:
-#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
Maybe we can keep the information that using 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 that the implementation uses a register nevertheless because this results in smaller code?
Maybe add such a comment instead ?
Thanks!
Best regards,
Wolfgang Denk

On Mon, Sep 12, 2022 at 11:22:53AM +0200, Wolfgang Denk wrote:
Dear Ovidiu,
In message 20220911161052.2986264-1-ovpanait@gmail.com you 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 #define XTRN_DECLARE_GLOBAL_DATA_PTR extern #define DECLARE_GLOBAL_DATA_PTR XTRN_DECLARE_GLOBAL_DATA_PTR \ gd_t *gd #endif
Remove all references.
Actually the commented out code contained some information, and I feel it would be a pity if that got lost:
-#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
Maybe we can keep the information that using 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 that the implementation uses a register nevertheless because this results in smaller code?
Maybe add such a comment instead ?
This is slightly mentioned in doc/develop/global_data.rst currently, but a follow-up to expand on this would be good.
participants (4)
-
Ovidiu Panait
-
Simon Glass
-
Tom Rini
-
Wolfgang Denk