powerpc, mpc5xxx: fix missing bootflag init

PATCH New Heiko Schocher 4 participants 7 messages Nov 25, 2015 – Jan 8, 2016 PowerPC

lore

Share & tools

lore

Heiko Schocher

author

2015-11-25 08:46 UTC #0 lore raw

PATCH 1/1 powerpc, mpc5xxx: fix missing bootflag init

since:
commit: f05ad9ba "Add a way to skip relocation"

tqm5200s board fails to boot. Reason is that board_init_f has a function parameter bootflag, which is not setup in
in arch/powerpc/cpu/mpc5xxx/start.S _start

So board_init_f gets a undefined bootflag, currently the gd pointer address. Unfortunately this address sets the GD_FLG_SKIP_RELOC bit, so u-boot code gets not relocated and u-boot does not boot ...

Init bootflag with 0, and tqm5200 boots fine again.

Signed-off-by Heiko Schocher <[email protected]>

---
I added a nightly build to:
http://xeidos.ddns.net/buildbot/waterfall
http://xeidos.ddns.net/buildbot/builders/tqm5200s_ml_ub

What does this nightly build:
- checkout current u-boot mainline code
- compile u-boot for tqm5200s board
- install u-boot image on the board with bdi
- run the image, and look if u-boot boots ToDo:
- add more tbot [1] tests for this board

Currently failing, as this patch is not in mainline, but it should go to green, if this patch is applied, cross your fingers ;-)

[1] https://github.com/hsdenx/tbot

 arch/powerpc/cpu/mpc5xxx/start.S | 2 ++
 1 file changed, 2 insertions(+)

arch/powerpc/cpu/mpc5xxx/start.S +2 -0

index 94eb0d3..54793f0 100644
@@ -91,6 +91,7 @@ _start:
 	li	r5,GD_SIZE	/* parameter 3:	 count			*/
 	bl	memset
 
+	li	r3, 0		/* parameter 1:	 bootflag		*/
 	bl	board_init_f	/* run 1st part of board init code (in Flash)*/
 	/* NOTREACHED - board_init_f() does not return */
 #else

@@ -169,6 +170,7 @@ lowboot_reentry:
 	/* r3: IMMR */
 	bl	cpu_init_f	/* run low-level CPU init code (in Flash)*/
 
+	li	r3, 0		/* parameter 1:	 bootflag		*/
 	bl	board_init_f	/* run 1st part of board init code (in Flash)*/
 
 	/* NOTREACHED - board_init_f() does not return */

Signature · 1 line

2.1.0

Simon Glass

2015-11-26 17:49 UTC #1 lore raw

PATCH 1/1 powerpc, mpc5xxx: fix missing bootflag init

Quoted 60 lines · Heiko Schocher

since:
commit: f05ad9ba "Add a way to skip relocation"
tqm5200s board fails to boot. Reason is that board_init_f has a function parameter bootflag, which is not setup in
in arch/powerpc/cpu/mpc5xxx/start.S _start
So board_init_f gets a undefined bootflag, currently the gd pointer address. Unfortunately this address sets the GD_FLG_SKIP_RELOC bit, so u-boot code gets not relocated and u-boot does not boot ...
Init bootflag with 0, and tqm5200 boots fine again.
Signed-off-by Heiko Schocher <[email protected]>
---
I added a nightly build to:
http://xeidos.ddns.net/buildbot/waterfall
http://xeidos.ddns.net/buildbot/builders/tqm5200s_ml_ub
What does this nightly build:
- checkout current u-boot mainline code
- compile u-boot for tqm5200s board
- install u-boot image on the board with bdi
- run the image, and look if u-boot boots ToDo:
- add more tbot [1] tests for this board
Currently failing, as this patch is not in mainline, but it should go to green, if this patch is applied, cross your fingers ;-)
[1] https://github.com/hsdenx/tbot
 arch/powerpc/cpu/mpc5xxx/start.S | 2 ++
 1 file changed, 2 insertions(+)
arch/powerpc/cpu/mpc5xxx/start.S +2 -1
index 94eb0d3..54793f0 100644
@@ -91,6 +91,7 @@ _start:
        li      r5,GD_SIZE      /* parameter 3:  count                  */
        bl      memset
 
+       li      r3, 0           /* parameter 1:  bootflag               */
        bl      board_init_f    /* run 1st part of board init code (in Flash)*/
        /* NOTREACHED - board_init_f() does not return */
 #else
@@ -169,6 +170,7 @@ lowboot_reentry:
        /* r3: IMMR */
        bl      cpu_init_f      /* run low-level CPU init code (in Flash)*/
 
+       li      r3, 0           /* parameter 1:  bootflag               */
        bl      board_init_f    /* run 1st part of board init code (in Flash)*/
 
        /* NOTREACHED - board_init_f() does not return */
--

Quoted 2 lines

2.1.0

Funny, these latent bugs!

Reviewed-by Simon Glass <[email protected]>

Heiko Schocher

author

2015-11-27 05:23 UTC #2 lore raw

PATCH 1/1 powerpc, mpc5xxx: fix missing bootflag init

Hello Simon,

Am 26.11.2015 um 18:49 schrieb Simon Glass:

Quoted 59 lines · Heiko Schocher

Quoted 58 lines
since:
commit: f05ad9ba "Add a way to skip relocation"
tqm5200s board fails to boot. Reason is that board_init_f has a function parameter bootflag, which is not setup in
in arch/powerpc/cpu/mpc5xxx/start.S _start
So board_init_f gets a undefined bootflag, currently the gd pointer address. Unfortunately this address sets the GD_FLG_SKIP_RELOC bit, so u-boot code gets not relocated and u-boot does not boot ...
Init bootflag with 0, and tqm5200 boots fine again.
Signed-off-by Heiko Schocher <[email protected]>
---
I added a nightly build to:
http://xeidos.ddns.net/buildbot/waterfall
http://xeidos.ddns.net/buildbot/builders/tqm5200s_ml_ub
What does this nightly build:
- checkout current u-boot mainline code
- compile u-boot for tqm5200s board
- install u-boot image on the board with bdi
- run the image, and look if u-boot boots ToDo:
- add more tbot [1] tests for this board
Currently failing, as this patch is not in mainline, but it should go to green, if this patch is applied, cross your fingers ;-)
[1] https://github.com/hsdenx/tbot
  arch/powerpc/cpu/mpc5xxx/start.S | 2 ++
  1 file changed, 2 insertions(+)
arch/powerpc/cpu/mpc5xxx/start.S +2 -0
index 94eb0d3..54793f0 100644
@@ -91,6 +91,7 @@ _start:
         li      r5,GD_SIZE      /* parameter 3:  count                  */
         bl      memset
 
+       li      r3, 0           /* parameter 1:  bootflag               */
         bl      board_init_f    /* run 1st part of board init code (in Flash)*/
         /* NOTREACHED - board_init_f() does not return */
  #else
@@ -169,6 +170,7 @@ lowboot_reentry:
         /* r3: IMMR */
         bl      cpu_init_f      /* run low-level CPU init code (in Flash)*/
 
+       li      r3, 0           /* parameter 1:  bootflag               */
         bl      board_init_f    /* run 1st part of board init code (in Flash)*/
 

Quoted 6 lines

Quoted 4 lines
arch/powerpc/cpu/mpc5xxx/start.S +0 -1
         /* NOTREACHED - board_init_f() does not return */
--
2.1.0
Funny, these latent bugs!

That;s the salt in our programming life ;-)

BTW: I wonder if this bootflag is really used?

Quoted 1 line

Reviewed-by Simon Glass <[email protected]>

bye,
Heiko

Signature · 2 lines

DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany

Simon Glass

2015-11-27 18:28 UTC #3 lore raw

PATCH 1/1 powerpc, mpc5xxx: fix missing bootflag init

Hi Heiko,

Quoted 80 lines · Heiko Schocher

Hello Simon,
Am 26.11.2015 um 18:49 schrieb Simon Glass:
Quoted 68 lines · Heiko Schocher
Quoted 66 lines
since:
commit: f05ad9ba "Add a way to skip relocation"
tqm5200s board fails to boot. Reason is that board_init_f has a function parameter bootflag, which is not setup in
in arch/powerpc/cpu/mpc5xxx/start.S _start
So board_init_f gets a undefined bootflag, currently the gd pointer address. Unfortunately this address sets the GD_FLG_SKIP_RELOC bit, so u-boot code gets not relocated and u-boot does not boot ...
Init bootflag with 0, and tqm5200 boots fine again.
Signed-off-by Heiko Schocher <[email protected]>
---
I added a nightly build to:
http://xeidos.ddns.net/buildbot/waterfall
http://xeidos.ddns.net/buildbot/builders/tqm5200s_ml_ub
What does this nightly build:
- checkout current u-boot mainline code
- compile u-boot for tqm5200s board
- install u-boot image on the board with bdi
- run the image, and look if u-boot boots ToDo:
- add more tbot [1] tests for this board
Currently failing, as this patch is not in mainline, but it should go to green, if this patch is applied, cross your fingers ;-)
[1] https://github.com/hsdenx/tbot
  arch/powerpc/cpu/mpc5xxx/start.S | 2 ++
  1 file changed, 2 insertions(+)
arch/powerpc/cpu/mpc5xxx/start.S +1 -0
index 94eb0d3..54793f0 100644
@@ -91,6 +91,7 @@ _start:
         li      r5,GD_SIZE      /* parameter 3:  count
 */
         bl      memset
 
+       li      r3, 0           /* parameter 1:  bootflag
 */
         bl      board_init_f    /* run 1st part of board init code (in
Flash)*/
         /* NOTREACHED - board_init_f() does not return */
#else
arch/powerpc/cpu/mpc5xxx/start.S +0 -0
@@ -169,6 +170,7 @@ lowboot_reentry:
         /* r3: IMMR */
         bl      cpu_init_f      /* run low-level CPU init code (in
Flash)*/
+ li r3, 0 /* parameter 1: bootflag
*/
bl board_init_f /* run 1st part of board init code (in
Flash)*/
Quoted 6 lines
Quoted 4 lines
         /* NOTREACHED - board_init_f() does not return */
--
2.1.0
Funny, these latent bugs!

Quoted 3 lines

That;s the salt in our programming life ;-)
BTW: I wonder if this bootflag is really used?

Not that I have noticed. If PowerPC could get a crt0.S like ARM (i.e. rationalise all the copies of similar assembly code) then it would be easier to drop it, or change it.

Regards,
Simon

york sun

2016-01-07 16:58 UTC #4 lore raw

PATCH 1/1 powerpc, mpc5xxx: fix missing bootflag init

In reply to Heiko Schocher

Quoted 14 lines · Heiko Schocher

since:
commit: f05ad9ba "Add a way to skip relocation"
tqm5200s board fails to boot. Reason is that board_init_f has a function parameter bootflag, which is not setup in
in arch/powerpc/cpu/mpc5xxx/start.S _start
So board_init_f gets a undefined bootflag, currently the gd pointer address. Unfortunately this address sets the GD_FLG_SKIP_RELOC bit, so u-boot code gets not relocated and u-boot does not boot ...

Quoted 4 lines

Init bootflag with 0, and tqm5200 boots fine again.
Signed-off-by Heiko Schocher <[email protected]>
---

Applied to u-boot-mpc85x. Merged upstream.

York

Wolfgang Denk

2016-01-07 19:30 UTC #5 lore raw

PATCH 1/1 powerpc, mpc5xxx: fix missing bootflag init

Dear York,

Quoted 8 lines · In message <AM4PR0401MB1732D7EBA1C64E62F3A403449AF50@AM4PR0401MB1732.eurprd04.p…

Quoted 8 lines
So board_init_f gets a undefined bootflag, currently the gd pointer address. Unfortunately this address sets the GD_FLG_SKIP_RELOC bit, so u-boot code gets not relocated and u-boot does not boot ...
Init bootflag with 0, and tqm5200 boots fine again.

...

Quoted 1 line

Applied to u-boot-mpc85x. Merged upstream.

Thanmks - but I think this needs a follow-up patch - Heiko is already looking into that. On closer inspection it appears as if the whole passing around of "bootflag" variables is just dead code - there is no place anywhere in U-Boot that makes real use of this data. We should remove all of the related code, which will make the code cleaner and smaller - which might be especially welcome in a number of SPL configurations.

Best regards,

Wolfgang Denk

Signature · 6 lines

DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Quotation, n. The act of repeating erroneously the words of  another.
The  words  erroneously  repeated.
                            - Ambrose Bierce _The Devil's Dictionary_

Heiko Schocher

author

2016-01-08 08:07 UTC #6 lore raw

PATCH 1/1 powerpc, mpc5xxx: fix missing bootflag init

Hello Wolfgang, York,

Am 07.01.2016 um 20:30 schrieb Wolfgang Denk:

Quoted 14 lines

Dear York,
Quoted 8 lines · In message <AM4PR0401MB1732D7EBA1C64E62F3A403449AF50@AM4PR0401MB1732.eurprd04.p…
Quoted 8 lines
So board_init_f gets a undefined bootflag, currently the gd pointer address. Unfortunately this address sets the GD_FLG_SKIP_RELOC bit, so u-boot code gets not relocated and u-boot does not boot ...
Init bootflag with 0, and tqm5200 boots fine again.
...
Quoted 1 line
Applied to u-boot-mpc85x. Merged upstream.

Quoted 7 lines

Thanmks - but I think this needs a follow-up patch - Heiko is already looking into that. On closer inspection it appears as if the whole passing around of "bootflag" variables is just dead code - there is no place anywhere in U-Boot that makes real use of this data. We should remove all of the related code, which will make the code cleaner and smaller - which might be especially welcome in a number of SPL configurations.

Yes, I am trying to look into it ...

BTW: Now with this patch applied, automated tbot test [1] is now green -> tqm5200 board works again with current mainline :-D

bye,
Heiko
[1] http://xeidos.ddns.net/buildbot/builders/tqm5200s_ml_ub
log:
http://xeidos.ddns.net/buildbot/builders/tqm5200s_ml_ub/builds/20/steps/shell/logs/tbotlog

Signature · 2 lines

DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany