[U-Boot-Users] [PATCH 1/3] ppc: Report back the location we put the device tree if we dont boot

[U-Boot-Users] [PATCH 1/3] ppc: Report back the location we put the device tree if we dont boot

[Kumar Gala]

Its useful to know where the device tree is if we have set 'autostart'
to 'no.  We come back to the prompt after a boot command and we can
than post process the device tree but we need to know where it was put
report this back via the env variable 'bootm_fdtaddr'.

Signed-off-by: Kumar Gala <galak@kernel.crashing.org>
---
 lib_ppc/bootm.c |   11 ++++++++++-
 1 files changed, 10 insertions(+), 1 deletions(-)

diff --git a/lib_ppc/bootm.c b/lib_ppc/bootm.c
index 81803dd..a872d31 100644
--- a/lib_ppc/bootm.c
+++ b/lib_ppc/bootm.c
@@ -277,8 +277,17 @@ do_bootm_linux(cmd_tbl_t *cmdtp, int flag, int argc, char *argv[],
 #if defined(CFG_INIT_RAM_LOCK) && !defined(CONFIG_E500)
 	unlock_ram_in_cache();
 #endif
-	if (!images->autostart)
+	if (!images->autostart) {
+#if defined(CONFIG_OF_LIBFDT)
+		if (of_flat_tree) {
+			char buf[32];
+
+			sprintf (buf, "%llx", (u64)(u32)of_flat_tree);
+			setenv("bootm_fdtaddr", buf);
+		}
+#endif
 		return ;
+	}
 
 #if defined(CONFIG_OF_LIBFDT)
 	if (of_flat_tree) {	/* device tree; boot new style */
-- 
1.5.5.1

[U-Boot-Users] [PATCH 2/3] Add ePAPR boot command

[Kumar Gala]

Add a boot command that supports the ePAPR client interface on powerpc.

Signed-off-by: Kumar Gala <galak@kernel.crashing.org>
---
 lib_ppc/bootm.c |   63 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 63 insertions(+), 0 deletions(-)

diff --git a/lib_ppc/bootm.c b/lib_ppc/bootm.c
index a872d31..1182c50 100644
--- a/lib_ppc/bootm.c
+++ b/lib_ppc/bootm.c
@@ -821,3 +821,66 @@ error:
 	return 1;
 }
 #endif
+
+/*******************************************************************/
+/* boote - boot ePAPR */
+/*******************************************************************/
+#if defined(CONFIG_CMD_BOOT_EPAPR)
+int do_boot_epapr (cmd_tbl_t *cmdtp, int flag, int argc, char *argv[])
+{
+	void  (*kernel)(ulong r3, ulong r4, ulong r5, ulong r6,
+			ulong r7, ulong r8, ulong r9);
+	ulong kern, of_flat_tree, epapr_magic;
+
+	if (argc != 3) {
+                printf ("Usage:\n%s\n", cmdtp->usage);
+                return 1;
+        }
+
+	kern = simple_strtoul (argv[1], NULL, 16);
+	of_flat_tree = simple_strtoul (argv[2], NULL, 16);
+
+	if (kern & 0x3) {
+                printf ("kernel address 0x%lx is not 4-byte aligned\n", kern);
+                return 1;
+	}
+
+	if (of_flat_tree & 0x7) {
+                printf ("Flat device tree address 0x%lx is not 8-byte aligned\n", of_flat_tree);
+                return 1;
+	}
+
+	kernel = (void (*)(ulong, ulong, ulong, ulong,
+			   ulong, ulong, ulong))kern;
+
+	disable_interrupts();
+
+	/*
+	 * Linux Kernel Parameters (passing device tree):
+	 *   r3: pointer to the fdt
+	 *   r4: 0
+	 *   r5: 0
+	 *   r6: epapr magic
+	 *   r7: size of IMA in bytes
+	 *   r8: 0
+	 *   r9: 0
+	 */
+#if defined(CONFIG_85xx) || defined(CONFIG_440)
+	epapr_magic = 0x45504150;
+#else
+	epapr_magic = 0x65504150;
+#endif
+
+	debug ("   Booting using OF flat tree...\n");
+	(*kernel) (of_flat_tree, 0, 0, epapr_magic, CFG_BOOTMAPSZ, 0, 0);
+	/* does not return */
+
+	return 0;
+}
+
+U_BOOT_CMD(
+	bootepapr,	3,	1,	do_boot_epapr,
+	"bootepapr - boot according to ePAPR client interface\n",
+	"<entry point> <device tree address>\n"
+);
+#endif
-- 
1.5.5.1

[U-Boot-Users] [PATCH 3/3] add ability to disable ft_board_setup as part of bootm

[Kumar Gala]

if the environment variable 'disable_fdt_boardsetup' is set we skip
doing the ft_board_setup().

Signed-off-by: Kumar Gala <galak@kernel.crashing.org>
---
 lib_ppc/bootm.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/lib_ppc/bootm.c b/lib_ppc/bootm.c
index 1182c50..a5b3a45 100644
--- a/lib_ppc/bootm.c
+++ b/lib_ppc/bootm.c
@@ -192,7 +192,8 @@ do_bootm_linux(cmd_tbl_t *cmdtp, int flag, int argc, char *argv[],
 		}
 #ifdef CONFIG_OF_BOARD_SETUP
 		/* Call the board-specific fixup routine */
-		ft_board_setup(of_flat_tree, gd->bd);
+		if (!getenv("disable_fdt_boardsetup"))
+			ft_board_setup(of_flat_tree, gd->bd);
 #endif
 	}
 
-- 
1.5.5.1

[U-Boot-Users] [PATCH 1/3] ppc: Report back the location we put the device tree if we dont boot

[Wolfgang Denk]

In message <1218004332-20311-1-git-send-email-galak@kernel.crashing.org> you wrote:
> Its useful to know where the device tree is if we have set 'autostart'
> to 'no.  We come back to the prompt after a boot command and we can
> than post process the device tree but we need to know where it was put
> report this back via the env variable 'bootm_fdtaddr'.

NAK.

The whole code sequence in bootm.c seems broken to me:

272         debug ("## Transferring control to Linux (at address %08lx) ...\n",
273                 (ulong)kernel);
274
275         show_boot_progress (15);
276
277 #if defined(CFG_INIT_RAM_LOCK) && !defined(CONFIG_E500)
278         unlock_ram_in_cache();
279 #endif
280         if (!images->autostart)
281                 return ;
282
283 #if defined(CONFIG_OF_LIBFDT)


The debug() [272f] should come immediately before booting the kernel
(i. e. move below line 282) because it is supposed to show when we
branch to Linux. No other code should be inbetween.

And the (!images->autostart) test makes absolutely no sense here.
Documentation says:

	autostart: if set to "yes", an image loaded using the rarpb,
	bootp, dhcp, tftp, disk, or docb commands will be
	automatically started (by internally calling the bootm
	command).

The "autostart" field introduced with the new image stuff behaves very
different, and actually makes no sense to me at all.

Bartek, could you please comment what the intended behaviour was, and
how it relates to the documentated behaviour?

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 at denx.de
Bus error -- driver executed.

[U-Boot-Users] [PATCH 2/3] Add ePAPR boot command

[Wolfgang Denk]

In message <1218004332-20311-2-git-send-email-galak@kernel.crashing.org> you wrote:
> Add a boot command that supports the ePAPR client interface on powerpc.

What is the intended use of such a command?

How does it intergrate with with image formats supported  by  U-Boot?
To  me  it seems that it's mostly intended to be called by other code
and not interactively, since I cannot see any  interfacing  with  the
existing image types?


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 at denx.de
Punishment becomes ineffective after a certain point. Men become  in-
sensitive.
	-- Eneg, "Patterns of Force", stardate 2534.7

[U-Boot-Users] [PATCH 1/3] ppc: Report back the location we put the device tree if we dont boot

[Bartlomiej Sieka]

Wolfgang Denk wrote:
> In message <1218004332-20311-1-git-send-email-galak@kernel.crashing.org> you wrote:
>> Its useful to know where the device tree is if we have set 'autostart'
>> to 'no.  We come back to the prompt after a boot command and we can
>> than post process the device tree but we need to know where it was put
>> report this back via the env variable 'bootm_fdtaddr'.
> 
> NAK.
> 
> The whole code sequence in bootm.c seems broken to me:
> 
> 272         debug ("## Transferring control to Linux (at address %08lx) ...\n",
> 273                 (ulong)kernel);
> 274
> 275         show_boot_progress (15);
> 276
> 277 #if defined(CFG_INIT_RAM_LOCK) && !defined(CONFIG_E500)
> 278         unlock_ram_in_cache();
> 279 #endif
> 280         if (!images->autostart)
> 281                 return ;
> 282
> 283 #if defined(CONFIG_OF_LIBFDT)
> 
> 
> The debug() [272f] should come immediately before booting the kernel
> (i. e. move below line 282) because it is supposed to show when we
> branch to Linux. No other code should be inbetween.
> 
> And the (!images->autostart) test makes absolutely no sense here.
> Documentation says:
> 
> 	autostart: if set to "yes", an image loaded using the rarpb,
> 	bootp, dhcp, tftp, disk, or docb commands will be
> 	automatically started (by internally calling the bootm
> 	command).
> 

Hi Wolfgang,

The test you're referring to was introduced by commit
75fa002c47171b73fb4c1f2c2fe4d6391c136276 "[new uImage] Respect autostart
setting in linux bootm" by Kumar -- he should be better able to explain
the details.


> The "autostart" field introduced with the new image stuff behaves very
> different, and actually makes no sense to me at all.
> 
> Bartek, could you please comment what the intended behaviour was, and
> how it relates to the documentated behaviour?

It looks like that the "autostart" field has been added to the
bootm_headers structure so that the arch-specific code can make
decisions about booting without the need to call getenv("autostart").
Instead, the "autostart" field is set based on the env. variable once,
and passed to boot-related functions via a parameter (e.g., "images" in
do_bootm_linux()).

Again, this field has beed introduced by Kumar
(f5614e7926863bf0225ec860d9b319741a9c4004, "[new uImage] Add autostart
flag to bootm_headers structure"), who should be able to comment more.

Regards,
Bartlomiej

[U-Boot-Users] [PATCH 1/3] ppc: Report back the location we put the device tree if we dont boot

[Wolfgang Denk]

Dear Bartek,

in message <48995F0F.8070807@semihalf.com> you wrote:
>
> The test you're referring to was introduced by commit
> 75fa002c47171b73fb4c1f2c2fe4d6391c136276 "[new uImage] Respect autostart
> setting in linux bootm" by Kumar -- he should be better able to explain
> the details.

Thanks - and sorry for blaming you, I should have checked this myself
first.

> It looks like that the "autostart" field has been added to the
> bootm_headers structure so that the arch-specific code can make
> decisions about booting without the need to call getenv("autostart").
> Instead, the "autostart" field is set based on the env. variable once,
> and passed to boot-related functions via a parameter (e.g., "images" in
> do_bootm_linux()).
> 
> Again, this field has beed introduced by Kumar
> (f5614e7926863bf0225ec860d9b319741a9c4004, "[new uImage] Add autostart
> flag to bootm_headers structure"), who should be able to comment more.

Indeed - but as mentioned before, this all makes no sense to me,
because with the intended and documented use of the "autostart"
variable the bootm command will not be called at all.


Kumar, if I were to back out commit f5614e79 - what exactly would it
break in your opinion?

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 at denx.de
Children are natural mimics who act like their parents despite  every
effort to teach them good manners.

[U-Boot-Users] [PATCH 2/3] Add ePAPR boot command

[Kumar Gala]

On Aug 6, 2008, at 3:21 AM, Wolfgang Denk wrote:

> In message <1218004332-20311-2-git-send-email-galak@kernel.crashing.org 
> > you wrote:
>> Add a boot command that supports the ePAPR client interface on  
>> powerpc.
>
> What is the intended use of such a command?
>
> How does it intergrate with with image formats supported  by  U-Boot?
> To  me  it seems that it's mostly intended to be called by other code
> and not interactively, since I cannot see any  interfacing  with  the
> existing image types?

I've been using as follows:

setenv autostart no
bootm
<fdt fixups>
bootepapr 0 $bootm_fdtaddr

Additionally these seems like the low level functionality needed if we  
want to move to the "scriptable" bootm were we decouple image loading  
from the actual boot mechanism.

- k

[U-Boot-Users] [PATCH 1/3] ppc: Report back the location we put the device tree if we dont boot

[Kumar Gala]

On Aug 6, 2008, at 3:33 AM, Wolfgang Denk wrote:

> Dear Bartek,
>
> in message <48995F0F.8070807@semihalf.com> you wrote:
>>
>> The test you're referring to was introduced by commit
>> 75fa002c47171b73fb4c1f2c2fe4d6391c136276 "[new uImage] Respect  
>> autostart
>> setting in linux bootm" by Kumar -- he should be better able to  
>> explain
>> the details.
>
> Thanks - and sorry for blaming you, I should have checked this myself
> first.
>
>> It looks like that the "autostart" field has been added to the
>> bootm_headers structure so that the arch-specific code can make
>> decisions about booting without the need to call getenv("autostart").
>> Instead, the "autostart" field is set based on the env. variable  
>> once,
>> and passed to boot-related functions via a parameter (e.g.,  
>> "images" in
>> do_bootm_linux()).
>>
>> Again, this field has beed introduced by Kumar
>> (f5614e7926863bf0225ec860d9b319741a9c4004, "[new uImage] Add  
>> autostart
>> flag to bootm_headers structure"), who should be able to comment  
>> more.
>
> Indeed - but as mentioned before, this all makes no sense to me,
> because with the intended and documented use of the "autostart"
> variable the bootm command will not be called at all.
>
>
> Kumar, if I were to back out commit f5614e79 - what exactly would it
> break in your opinion?

There is intent and what the old code did.  My feeling is that  
'autostart = no' means to load the images but not actually jump to the  
new image.

The old code would load the "kernel" image and set 'filesize' and  
return (only if the image was of type IH_TYPE_STANDALONE).

I think when sent 'f5614e7926863bf0225ec860d9b319741a9c4004' I didn't  
notice the IH_TYPE_STANDALONE aspect.

We can revert the commit but it puts be back to square one w/o a  
solution to my problem.

- k

[U-Boot-Users] [PATCH 2/3] Add ePAPR boot command

[Wolfgang Denk]

In message <2C702584-3214-4E3C-8F59-7C46C2F4E8AF@kernel.crashing.org> you wrote:
> 
> I've been using as follows:
> 
> setenv autostart no
> bootm
> <fdt fixups>
> bootepapr 0 $bootm_fdtaddr

Now I see where your intentions with the autostart are coming from.

Sorry, this is not the way it is supposed to work, and not the way it
is documented.

> Additionally these seems like the low level functionality needed if we  
> want to move to the "scriptable" bootm were we decouple image loading  
> from the actual boot mechanism.

In terms of splitting bootm into smaller units you are riught. In
terms of implementation it's wrong.

I think I will back out the autostart patch because it introduces
incorrect and undocumented behaviour.

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 at denx.de
Commitment, n.:      Commitment can be illustrated by a breakfast
of ham and eggs. The chicken was involved, the pig was committed.

[U-Boot-Users] [PATCH 1/3] ppc: Report back the location we put the device tree if we dont boot

[Wolfgang Denk]

In message <A71F1F8F-5136-40FB-8F4F-2603D284E451@kernel.crashing.org> you wrote:
> 
> There is intent and what the old code did.  My feeling is that  
> 'autostart = no' means to load the images but not actually jump to the  
> new image.

Correct. To be a bit more specific, "load" here  means  to  load  the
kernel  image  to  RAM  (over  Ethernet,  USB,  from  disk etc.). The
intention of "autostart" is (as  documented)  to  avoid  an  explicit
bootm.

It has never been an intention to split bootm  into  separate  phases
and make it terminate early. This cannot work in general.

> We can revert the commit but it puts be back to square one w/o a  
> solution to my problem.

I understand this, and I'm sorry for that. But the  real  fix  is  to
split up bootm as discussed yesterday.

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 at denx.de
"A verbal contract isn't worth the paper it's printed on."
- Samuel Goldwyn

[U-Boot-Users] [PATCH 3/3] add ability to disable ft_board_setup as part of bootm

[Wolfgang Denk]

Dear Kumar,

In message <1218004332-20311-3-git-send-email-galak@kernel.crashing.org> you wrote:
> if the environment variable 'disable_fdt_boardsetup' is set we skip
> doing the ft_board_setup().

Such a long variable name is a PITA to type. Please chose a shorter
name, and please add documentation for it.

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 at denx.de
To understand a program you must become  both  the  machine  and  the
program.