Patchwork [U-Boot,09/10] arm: Move bootstage record for board_init_f() to after arch_cpu_init()

login
register
mail settings
Submitter Simon Glass
Date Nov. 1, 2012, 11:42 p.m.
Message ID <1351813330-23741-9-git-send-email-sjg@chromium.org>
Download mbox | patch
Permalink /patch/196422/
State Superseded, archived
Delegated to: Albert ARIBAUD
Headers show

Comments

Simon Glass - Nov. 1, 2012, 11:42 p.m.
The timer may be inited in arch_cpu_init() so it is not safe to make a
bootstage mark before this is called. Arrange the code to fix this.

We now get a correct time for board_init_f:

Timer summary in microseconds:
       Mark    Elapsed  Stage
          0          0  reset
    100,000    100,000  spl_start
    848,530    748,530  board_init_f
    907,301     58,771  board_init_r
    910,478      3,177  board_init

Signed-off-by: Simon Glass <sjg@chromium.org>
---
 arch/arm/lib/board.c |   11 +++++++++--
 1 files changed, 9 insertions(+), 2 deletions(-)
Wolfgang Denk - Nov. 3, 2012, 2:55 p.m.
Dear Simon Glass,

In message <1351813330-23741-9-git-send-email-sjg@chromium.org> you wrote:
> The timer may be inited in arch_cpu_init() so it is not safe to make a
> bootstage mark before this is called. Arrange the code to fix this.
> 
> We now get a correct time for board_init_f:
> 
> Timer summary in microseconds:
>        Mark    Elapsed  Stage
>           0          0  reset
>     100,000    100,000  spl_start
>     848,530    748,530  board_init_f
>     907,301     58,771  board_init_r
>     910,478      3,177  board_init
> 
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
>  arch/arm/lib/board.c |   11 +++++++++--
>  1 files changed, 9 insertions(+), 2 deletions(-)

NAK as is.  Please make sure to keep all arhcitectures in sync.  The
long term goal iss till to merge the lib/board.c files into a single,
common one.

Best regards,

Wolfgang Denk
Simon Glass - Nov. 7, 2012, 12:51 a.m.
Hi Wolfgang,

On Sat, Nov 3, 2012 at 7:55 AM, Wolfgang Denk <wd@denx.de> wrote:
> Dear Simon Glass,
>
> In message <1351813330-23741-9-git-send-email-sjg@chromium.org> you wrote:
>> The timer may be inited in arch_cpu_init() so it is not safe to make a
>> bootstage mark before this is called. Arrange the code to fix this.
>>
>> We now get a correct time for board_init_f:
>>
>> Timer summary in microseconds:
>>        Mark    Elapsed  Stage
>>           0          0  reset
>>     100,000    100,000  spl_start
>>     848,530    748,530  board_init_f
>>     907,301     58,771  board_init_r
>>     910,478      3,177  board_init
>>
>> Signed-off-by: Simon Glass <sjg@chromium.org>
>> ---
>>  arch/arm/lib/board.c |   11 +++++++++--
>>  1 files changed, 9 insertions(+), 2 deletions(-)
>
> NAK as is.  Please make sure to keep all arhcitectures in sync.  The
> long term goal iss till to merge the lib/board.c files into a single,
> common one.

See my notes on the other patch, most of which apply here.

http://patchwork.ozlabs.org/patch/196419/

I don't believe this affects other architectures, but it is a problem
on at least exynos 5.

Regards,
Simon

>
> Best regards,
>
> Wolfgang Denk
>
> --
> DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
> HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
> Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd@denx.de
> The universe contains any amount of horrible ways  to  be  woken  up,
> such as the noise of the mob breaking down the front door, the scream
> of fire engines, or the realization that today is the Monday which on
> Friday night was a comfortably long way off.
>                                  - Terry Pratchett, _Moving Pictures_
Wolfgang Denk - Nov. 7, 2012, 1:18 p.m.
Dear Simon,

In message <CAPnjgZ0D4g4bK70VooK3SYkMzOwLSyPXdjE2yCn3gvJ1jAv6EQ@mail.gmail.com> you wrote:
> 
> > NAK as is.  Please make sure to keep all arhcitectures in sync.  The
> > long term goal iss till to merge the lib/board.c files into a single,
> > common one.
> 
> See my notes on the other patch, most of which apply here.
> 
> http://patchwork.ozlabs.org/patch/196419/
> 
> I don't believe this affects other architectures, but it is a problem
> on at least exynos 5.

Well, but if you just compare arch/arm/lib/board.c and
arch/powerpc/lib/board.c, we are heading into pretty much deviation
directions.  I'd like to stop that.

Best regards,

Wolfgang Denk
Simon Glass - Nov. 22, 2012, 2:57 p.m.
Hi Wolfgang,

On Wed, Nov 7, 2012 at 5:18 AM, Wolfgang Denk <wd@denx.de> wrote:
> Dear Simon,
>
> In message <CAPnjgZ0D4g4bK70VooK3SYkMzOwLSyPXdjE2yCn3gvJ1jAv6EQ@mail.gmail.com> you wrote:
>>
>> > NAK as is.  Please make sure to keep all arhcitectures in sync.  The
>> > long term goal iss till to merge the lib/board.c files into a single,
>> > common one.
>>
>> See my notes on the other patch, most of which apply here.
>>
>> http://patchwork.ozlabs.org/patch/196419/
>>
>> I don't believe this affects other architectures, but it is a problem
>> on at least exynos 5.
>
> Well, but if you just compare arch/arm/lib/board.c and
> arch/powerpc/lib/board.c, we are heading into pretty much deviation
> directions.  I'd like to stop that.

Well powerpc doesn't have specific bootstage markers at present
(although it does use boot progress). I hope that the generic board
series will solve this problem in general, but in the meantime this is
a real problem, and only in ARM.

Regards,
Simon

>
> Best regards,
>
> Wolfgang Denk
>
> --
> DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
> HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
> Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd@denx.de
> It is impractical for  the  standard  to  attempt  to  constrain  the
> behavior  of code that does not obey the constraints of the standard.
>                                                           - Doug Gwyn

Patch

diff --git a/arch/arm/lib/board.c b/arch/arm/lib/board.c
index d420307..ec3739f 100644
--- a/arch/arm/lib/board.c
+++ b/arch/arm/lib/board.c
@@ -233,8 +233,17 @@  int __arch_cpu_init(void)
 int arch_cpu_init(void)
 	__attribute__((weak, alias("__arch_cpu_init")));
 
+/* Record the board_init_f() bootstage (after arch_cpu_init()) */
+static int mark_bootstage(void)
+{
+	bootstage_mark_name(BOOTSTAGE_ID_START_UBOOT_F, "board_init_f");
+
+	return 0;
+}
+
 init_fnc_t *init_sequence[] = {
 	arch_cpu_init,		/* basic arch cpu dependent setup */
+	mark_bootstage,
 #ifdef CONFIG_OF_CONTROL
 	fdtdec_check_fdt,
 #endif
@@ -278,8 +287,6 @@  void board_init_f(ulong bootflag)
 	void *new_fdt = NULL;
 	size_t fdt_size = 0;
 
-	bootstage_mark_name(BOOTSTAGE_ID_START_UBOOT_F, "board_init_f");
-
 	/* Pointer is writable since we allocated a register for it */
 	gd = (gd_t *) ((CONFIG_SYS_INIT_SP_ADDR) & ~0x07);
 	/* compiler optimization barrier needed for GCC >= 3.4 */