* [PATCH 2/4] common/board_f: move CONFIG_MACH_TYPE logic to arch/arm/lib/bdinfo.c
2022-09-11 16:10 [PATCH 1/4] common/board_f: remove XTRN_DECLARE_GLOBAL_DATA_PTR dead code Ovidiu Panait
@ 2022-09-11 16:10 ` Ovidiu Panait
2022-09-12 13:34 ` Simon Glass
2022-09-11 16:10 ` [PATCH 3/4] common/board_f: introduce arch_setup_dest_addr() Ovidiu Panait
` (2 subsequent siblings)
3 siblings, 1 reply; 9+ messages in thread
From: Ovidiu Panait @ 2022-09-11 16:10 UTC (permalink / raw)
To: u-boot; +Cc: Ovidiu Panait, Marek Vasut, Michal Simek, Rick Chen, Simon Glass
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();
}
--
2.25.1
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH 3/4] common/board_f: introduce arch_setup_dest_addr()
2022-09-11 16:10 [PATCH 1/4] common/board_f: remove XTRN_DECLARE_GLOBAL_DATA_PTR dead code Ovidiu Panait
2022-09-11 16:10 ` [PATCH 2/4] common/board_f: move CONFIG_MACH_TYPE logic to arch/arm/lib/bdinfo.c Ovidiu Panait
@ 2022-09-11 16:10 ` Ovidiu Panait
2022-09-12 13:34 ` Simon Glass
2022-09-11 16:10 ` [PATCH 4/4] common/board_f: drop ifdefs around header includes Ovidiu Panait
2022-09-12 9:22 ` [PATCH 1/4] common/board_f: remove XTRN_DECLARE_GLOBAL_DATA_PTR dead code Wolfgang Denk
3 siblings, 1 reply; 9+ messages in thread
From: Ovidiu Panait @ 2022-09-11 16:10 UTC (permalink / raw)
To: u-boot
Cc: Ovidiu Panait, Andy Shevchenko, Dzmitry Sankouski,
Heinrich Schuchardt, Marek Vasut, Michal Simek, Rick Chen,
Simon Glass, Wolfgang Denk
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
*
--
2.25.1
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [PATCH 3/4] common/board_f: introduce arch_setup_dest_addr()
2022-09-11 16:10 ` [PATCH 3/4] common/board_f: introduce arch_setup_dest_addr() Ovidiu Panait
@ 2022-09-12 13:34 ` Simon Glass
0 siblings, 0 replies; 9+ messages in thread
From: Simon Glass @ 2022-09-12 13:34 UTC (permalink / raw)
To: Ovidiu Panait
Cc: U-Boot Mailing List, Andy Shevchenko, Dzmitry Sankouski,
Heinrich Schuchardt, Marek Vasut, Michal Simek, Rick Chen,
Wolfgang Denk
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>
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 4/4] common/board_f: drop ifdefs around header includes
2022-09-11 16:10 [PATCH 1/4] common/board_f: remove XTRN_DECLARE_GLOBAL_DATA_PTR dead code Ovidiu Panait
2022-09-11 16:10 ` [PATCH 2/4] common/board_f: move CONFIG_MACH_TYPE logic to arch/arm/lib/bdinfo.c Ovidiu Panait
2022-09-11 16:10 ` [PATCH 3/4] common/board_f: introduce arch_setup_dest_addr() Ovidiu Panait
@ 2022-09-11 16:10 ` Ovidiu Panait
2022-09-12 13:34 ` Simon Glass
2022-09-12 9:22 ` [PATCH 1/4] common/board_f: remove XTRN_DECLARE_GLOBAL_DATA_PTR dead code Wolfgang Denk
3 siblings, 1 reply; 9+ messages in thread
From: Ovidiu Panait @ 2022-09-11 16:10 UTC (permalink / raw)
To: u-boot; +Cc: Ovidiu Panait, Marek Vasut, Michal Simek, Rick Chen, Simon Glass
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>
--
2.25.1
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 1/4] common/board_f: remove XTRN_DECLARE_GLOBAL_DATA_PTR dead code
2022-09-11 16:10 [PATCH 1/4] common/board_f: remove XTRN_DECLARE_GLOBAL_DATA_PTR dead code Ovidiu Panait
` (2 preceding siblings ...)
2022-09-11 16:10 ` [PATCH 4/4] common/board_f: drop ifdefs around header includes Ovidiu Panait
@ 2022-09-12 9:22 ` Wolfgang Denk
2022-09-12 16:30 ` Tom Rini
3 siblings, 1 reply; 9+ messages in thread
From: Wolfgang Denk @ 2022-09-12 9:22 UTC (permalink / raw)
To: Ovidiu Panait
Cc: u-boot, Holger Brunck, Marek Vasut, Michal Simek, Rick Chen,
Simon Glass
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
--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr. 5, 82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd@denx.de
All repairs tend to destroy the structure, to increase the entropy
and disorder of the system. Less and less effort is spent on fixing
original design flaws; more and more is spent on fixing flaws intro-
duced by earlier fixes. - Fred Brooks, "The Mythical Man Month"
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH 1/4] common/board_f: remove XTRN_DECLARE_GLOBAL_DATA_PTR dead code
2022-09-12 9:22 ` [PATCH 1/4] common/board_f: remove XTRN_DECLARE_GLOBAL_DATA_PTR dead code Wolfgang Denk
@ 2022-09-12 16:30 ` Tom Rini
0 siblings, 0 replies; 9+ messages in thread
From: Tom Rini @ 2022-09-12 16:30 UTC (permalink / raw)
To: Wolfgang Denk
Cc: Ovidiu Panait, u-boot, Holger Brunck, Marek Vasut, Michal Simek,
Rick Chen, Simon Glass
[-- Attachment #1: Type: text/plain, Size: 1564 bytes --]
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.
--
Tom
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]
^ permalink raw reply [flat|nested] 9+ messages in thread