[U-Boot-Users] Can U-boot Autodetect arch/ppc versus arch/powerpc from info in the uImage?

[U-Boot-Users] Can U-boot Autodetect arch/ppc versus arch/powerpc from info in the uImage?

[Darcy Watkins]

Hello,

 

I have looked at some of the u-boot code for booting up a linux kernel.
It appears that the determination of whether or not to make use of
flattened device tree (fdt) in the boot is a compile time selection.
For the ppc4xx family of processors, linux kernels are currently built
as arch/ppc but there is work-in-progress to move it to arch/powerpc.

 

Can we determine during runtime from info in the uImage whether the
linux kernel was built as arch/ppc or as arch/powerpc and then select
the fdt support accordingly?

 

It will soon be possible to build a U-Boot for one or the other (for
ppc4xx family).  I think I will need to implement a U-Boot that will
support both since we currently build as arch/ppc but will need to
support upgrades to arch/powerpc base kernels down the road.  For this,
I think we need the U-Boot to be able to support both but detect which
one the uImage was built under.

 

In the image.h file, there appears to only be one selection for powerpc
so there will likely be need for some other "magic" to make this work.
Has anyone checked into this before, or come up with a workable
solution?

 

 

Regards,

 

Darcy

 

-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.denx.de/pipermail/u-boot/attachments/20071203/a918f185/attachment.htm 

[U-Boot-Users] Can U-boot Autodetect arch/ppc versus arch/powerpc from info in the uImage?

[Wolfgang Denk]

In message <D61182AC8012EA4EBC531B3AF23BE109480232@tranzeo-mail2.12stewart.tranzeo.com> you wrote:
> 
> I have looked at some of the u-boot code for booting up a linux kernel.
> It appears that the determination of whether or not to make use of
> flattened device tree (fdt) in the boot is a compile time selection.

No, this is not the case. You can use a FDT-enabled U-Boot to boot
kernel images which use a FDT, and of course such that don't, too.

> Can we determine during runtime from info in the uImage whether the
> linux kernel was built as arch/ppc or as arch/powerpc and then select
> the fdt support accordingly?

No. There is no such information available in the current image
format.

> It will soon be possible to build a U-Boot for one or the other (for
> ppc4xx family).  I think I will need to implement a U-Boot that will

This is not correct. You can build U-Boot  without  FDT-support,  and
this will only work with old (arch/ppc) kernels, or with arch/powerpc
kernels  using  the  bootwrapper  (cuImage).  Or you can build U-Boot
*with* FDT-support, which can do exactly the  same,  *plus*  directly
boot FDT enabled Linux kernels.

> I think we need the U-Boot to be able to support both but detect which
> one the uImage was built under.

The first part of the sentence is true, and describes the current
state of things, and the second part is IMO not needed.

> In the image.h file, there appears to only be one selection for powerpc
> so there will likely be need for some other "magic" to make this work.
> Has anyone checked into this before, or come up with a workable
> solution?

I see no need for any such changes.

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
Disc space - the final frontier!

[U-Boot-Users] Can U-boot Autodetect arch/ppc versusarch/powerpc from info in the uImage?

[Joakim Tjernlund]

> -----Original Message-----
> From: u-boot-users-bounces at lists.sourceforge.net 
> [mailto:u-boot-users-bounces at lists.sourceforge.net] On Behalf 
> Of Wolfgang Denk
> Sent: den 3 december 2007 22:36
> To: Darcy Watkins
> Cc: u-boot-users at lists.sourceforge.net
> Subject: Re: [U-Boot-Users] Can U-boot Autodetect arch/ppc 
> versusarch/powerpc from info in the uImage?
> 
> In message 
> <D61182AC8012EA4EBC531B3AF23BE109480232@tranzeo-mail2.12stewar
> t.tranzeo.com> you wrote:
> > 
> > I have looked at some of the u-boot code for booting up a 
> linux kernel.
> > It appears that the determination of whether or not to make use of
> > flattened device tree (fdt) in the boot is a compile time selection.
> 
> No, this is not the case. You can use a FDT-enabled U-Boot to boot
> kernel images which use a FDT, and of course such that don't, too.

Wolfgang, have you read the "Passing the OF tree to bootm from board code?"
thread? Would you mind explain why it was removed and why you don't think
it should be in u-boot?

  Regards
            Jocke

[U-Boot-Users] Can U-boot Autodetect arch/ppc versusarch/powerpc from info in the uImage?

[Wolfgang Denk]

Dear Jocke,

in message <057d01c83604$e0bb9ce0$5267a8c0@Jocke> you wrote:
>
> Wolfgang, have you read the "Passing the OF tree to bootm from board code?"
> thread? Would you mind explain why it was removed and why you don't think
> it should be in u-boot?

Is there a thread to it? I can just find a single posting from you,
without any replies...

But as for your question:

I cannot remember that, or when, or why exactly this feature was
removed; I cannot even remember that it ever exited.

But in genral I think it is a very bad idea  to  compile  the  device
tree  into the U-Boot image, because that means you can never replace
/ change / update it. Also I fail to understand why you would want to
do that. To me it seems much more sensible to  have  separate  U-Boot
and  device  tree  images,  which can be replaced independent of each
other. If you want, you can still store these in  adjacent  locations
in flash. You can even attach these (you probably want to insert some
padding  to  align on flash sector boundaries) images to for a single
file which can be downloaded and flashed in a single operation.

As far as I understand your request, this offers all the same options
you can get with a compiled in device tree blob, but with  the  added
benefit of allowing many more things at the same time, too.

Maybe I'm missing something?

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
Behind every great man, there is a woman -- urging him on.
	-- Harry Mudd, "I, Mudd", stardate 4513.3

[U-Boot-Users] Can U-boot Autodetect arch/ppc versusarch/powerpc from info in the uImage?

[Joakim Tjernlund]

> -----Original Message-----
> From: wd at denx.de [mailto:wd at denx.de] 
> Sent: den 4 december 2007 01:24
> To: Joakim Tjernlund
> Cc: 'Darcy Watkins'; u-boot-users at lists.sourceforge.net
> Subject: Re: [U-Boot-Users] Can U-boot Autodetect arch/ppc 
> versusarch/powerpc from info in the uImage?
> 
> Dear Jocke,
> 
> in message <057d01c83604$e0bb9ce0$5267a8c0@Jocke> you wrote:
> >
> > Wolfgang, have you read the "Passing the OF tree to bootm 
> from board code?"
> > thread? Would you mind explain why it was removed and why 
> you don't think
> > it should be in u-boot?
> 
> Is there a thread to it? I can just find a single posting from you,
> without any replies...

Yes, there were a few replies.

> 
> But as for your question:
> 
> I cannot remember that, or when, or why exactly this feature was
> removed; I cannot even remember that it ever exited.

Was a long time ago, I too can't recall details.

> 
> But in genral I think it is a very bad idea  to  compile  the  device
> tree  into the U-Boot image, because that means you can never replace
> / change / update it. Also I fail to understand why you would want to
> do that. To me it seems much more sensible to  have  separate  U-Boot
> and  device  tree  images,  which can be replaced independent of each
> other. If you want, you can still store these in  adjacent  locations
> in flash. You can even attach these (you probably want to insert some
> padding  to  align on flash sector boundaries) images to for a single
> file which can be downloaded and flashed in a single operation.

I create a template tree that is compiled into u-boot. Then runtime
code adjusts/amends the tree as needed for that particular board.
We do have many diffrent boards that more or less look the same to
u-boot and kernel. For all these boards I do only one u-boot/kernel that
runs on all boards. Reason for that is too ease SW updates and save
space. This might not be the best methond of doing things but thats
how it has been the last 7 years so it is not something that is easy to
change now.

Even if the above method isn't something you want in u-boot I still
think there should be a way to pass the dev. tree to the bootm command
without using the environment/command arguments. For instance if
one has several trees, board code should be able decide which tree to use.
Command args should still overide this builtin tree.

 Jocke
> 
> As far as I understand your request, this offers all the same options
> you can get with a compiled in device tree blob, but with  the  added
> benefit of allowing many more things at the same time, too.
> 
> Maybe I'm missing something?
> 
> 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
> Behind every great man, there is a woman -- urging him on.
> 	-- Harry Mudd, "I, Mudd", stardate 4513.3
> 
> 

[U-Boot-Users] Can U-boot Autodetect arch/ppcversusarch/powerpc from info in the uImage?

[Joakim Tjernlund]

> -----Original Message-----
> From: u-boot-users-bounces at lists.sourceforge.net 
> [mailto:u-boot-users-bounces at lists.sourceforge.net] On Behalf 
> Of Joakim Tjernlund
> Sent: den 4 december 2007 02:06
> To: wd at denx.de
> Cc: u-boot-users at lists.sourceforge.net; 'Darcy Watkins'
> Subject: Re: [U-Boot-Users] Can U-boot Autodetect 
> arch/ppcversusarch/powerpc from info in the uImage?
> 
> > -----Original Message-----
> > From: wd at denx.de [mailto:wd at denx.de] 
> > Sent: den 4 december 2007 01:24
> > To: Joakim Tjernlund
> > Cc: 'Darcy Watkins'; u-boot-users at lists.sourceforge.net
> > Subject: Re: [U-Boot-Users] Can U-boot Autodetect arch/ppc 
> > versusarch/powerpc from info in the uImage?
> > 
> > Dear Jocke,
> > 
> > in message <057d01c83604$e0bb9ce0$5267a8c0@Jocke> you wrote:
> > >
> > > Wolfgang, have you read the "Passing the OF tree to bootm 
> > from board code?"
> > > thread? Would you mind explain why it was removed and why 
> > you don't think
> > > it should be in u-boot?
> > 
> > Is there a thread to it? I can just find a single posting from you,
> > without any replies...
> 
> Yes, there were a few replies.
> 
> > 
> > But as for your question:
> > 
> > I cannot remember that, or when, or why exactly this feature was
> > removed; I cannot even remember that it ever exited.
> 
> Was a long time ago, I too can't recall details.
> 
> > 
> > But in genral I think it is a very bad idea  to  compile  
> the  device
> > tree  into the U-Boot image, because that means you can 
> never replace
> > / change / update it. Also I fail to understand why you 
> would want to
> > do that. To me it seems much more sensible to  have  
> separate  U-Boot
> > and  device  tree  images,  which can be replaced 
> independent of each
> > other. If you want, you can still store these in  adjacent  
> locations
> > in flash. You can even attach these (you probably want to 
> insert some
> > padding  to  align on flash sector boundaries) images to 
> for a single
> > file which can be downloaded and flashed in a single operation.
> 
> I create a template tree that is compiled into u-boot. Then runtime
> code adjusts/amends the tree as needed for that particular board.
> We do have many diffrent boards that more or less look the same to
> u-boot and kernel. For all these boards I do only one 
> u-boot/kernel that
> runs on all boards. Reason for that is too ease SW updates and save
> space. This might not be the best methond of doing things but thats
> how it has been the last 7 years so it is not something that 
> is easy to
> change now.
> 
> Even if the above method isn't something you want in u-boot I still
> think there should be a way to pass the dev. tree to the bootm command
> without using the environment/command arguments. For instance if
> one has several trees, board code should be able decide which 
> tree to use.
> Command args should still overide this builtin tree.
> 
>  Jocke
> > 
> > As far as I understand your request, this offers all the 
> same options
> > you can get with a compiled in device tree blob, but with  
> the  added
> > benefit of allowing many more things at the same time, too.
> > 
> > Maybe I'm missing something?

Yes, here are some things to consider.

The extension I made can just as easily be a function that returns a dev. tree.

- One can have a dev. tree update procedure similar to
  rendundant env. The funktion then selects which dev. tree is valid and passes
  that back.

- The function can construct a dev. tree from builtin rules/code.

- One can even make it tftp a tree at boot and pass that back. Great
  in a development env. especially if you are debugging the dev. tree.

and the kicker is that you can still override this tree at runtime by passing
a dev. tree argument to bootm.

All this will be possible by the simple patch I posted earlier, included
below.

[PATCH] Make it possible to use a builtin OF tree.
Signed-off-by: Joakim Tjernlund <Joakim.Tjernlund@transmode.se>
---
 common/cmd_bootm.c |    5 ++++-
 1 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/common/cmd_bootm.c b/common/cmd_bootm.c
index d816349..8405de7 100644
--- a/common/cmd_bootm.c
+++ b/common/cmd_bootm.c
@@ -507,6 +507,9 @@ fixup_silent_linux ()
 #endif /* CONFIG_SILENT_CONSOLE */
 
 #ifdef CONFIG_PPC
+#ifndef DEFAULT_OF_TREE
+ #define DEFAULT_OF_TREE NULL
+#endif
 static void  __attribute__((noinline))
 do_bootm_linux (cmd_tbl_t *cmdtp, int flag,
 		int	argc, char *argv[],
@@ -527,7 +530,7 @@ do_bootm_linux (cmd_tbl_t *cmdtp, int flag,
 	void	(*kernel)(bd_t *, ulong, ulong, ulong, ulong);
 	image_header_t *hdr = &header;
 #if defined(CONFIG_OF_FLAT_TREE) || defined(CONFIG_OF_LIBFDT)
-	char	*of_flat_tree = NULL;
+	char	*of_flat_tree = DEFAULT_OF_TREE;
 	ulong	of_data = 0;
 #endif
 
-- 
1.5.3.6

> > 
> > Best regards,
> > 
> > Wolfgang Denk

[U-Boot-Users] Can U-boot Autodetect arch/ppcversusarch/powerpc from info in the uImage?

[Wolfgang Denk]

In message <00aa01c83859$77863cb0$02ac10ac@Jocke> you wrote:
>
> The extension I made can just as easily be a function that returns a dev. tree.
> 
> - One can have a dev. tree update procedure similar to
>   rendundant env. The funktion then selects which dev. tree is valid and passes
>   that back.
> 
> - The function can construct a dev. tree from builtin rules/code.
> 
> - One can even make it tftp a tree at boot and pass that back. Great
>   in a development env. especially if you are debugging the dev. tree.
> 
> and the kicker is that you can still override this tree at runtime by passing
> a dev. tree argument to bootm.
> 
> All this will be possible by the simple patch I posted earlier, included
> below.

Um... and which of these cannot be done without your patch?

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
If a train station is a place where a train stops,
                                           then what's a workstation?

[U-Boot-Users] Can U-boot Autodetect arch/ppcversusarch/powerpc from info in the uImage?

[Joakim Tjernlund]

> -----Original Message-----
> From: wd at denx.de [mailto:wd at denx.de] 
> Sent: den 7 december 2007 01:58
> To: Joakim Tjernlund
> Cc: u-boot-users at lists.sourceforge.net; 'Darcy Watkins'
> Subject: Re: [U-Boot-Users] Can U-boot Autodetect 
> arch/ppcversusarch/powerpc from info in the uImage?
> 
> In message <00aa01c83859$77863cb0$02ac10ac@Jocke> you wrote:
> >
> > The extension I made can just as easily be a function that 
> returns a dev. tree.
> > 
> > - One can have a dev. tree update procedure similar to
> >   rendundant env. The funktion then selects which dev. tree 
> is valid and passes
> >   that back.
> > 
> > - The function can construct a dev. tree from builtin rules/code.
> > 
> > - One can even make it tftp a tree at boot and pass that back. Great
> >   in a development env. especially if you are debugging the 
> dev. tree.
> > 
> > and the kicker is that you can still override this tree at 
> runtime by passing
> > a dev. tree argument to bootm.
> > 
> > All this will be possible by the simple patch I posted 
> earlier, included
> > below.
> 
> Um... and which of these cannot be done without your patch?

The kicker part, override the tree at runtime. It is
also a bit easier do the above my way than messing with
a env variable.

 Jocke

[U-Boot-Users] Can U-boot Autodetect arch/ppcversusarch/powerpc from info in the uImage?

[Wolfgang Denk]

In message <00b401c8389e$37636c30$02ac10ac@Jocke> you wrote:
>
> > Um... and which of these cannot be done without your patch?
> 
> The kicker part, override the tree at runtime. It is
> also a bit easier do the above my way than messing with
> a env variable.

I'm afraid I fail to understand what you mean. Where is the problem
of passing different addresses to the bootm command?

And why do you think that using an environment variable is in any way
messy? That's what they are made for...

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
Death. Destruction. Disease. Horror. That's what war  is  all  about.
That's what makes it a thing to be avoided.
	-- Kirk, "A Taste of Armageddon", stardate 3193.0

[U-Boot-Users] Can U-boot Autodetect arch/ppcversusarch/powerpc from info in the uImage?

[Jerry Van Baren]

Joakim Tjernlund wrote:

[snip]

>>  Jocke
>>> As far as I understand your request, this offers all the 
>> same options
>>> you can get with a compiled in device tree blob, but with  
>> the  added
>>> benefit of allowing many more things at the same time, too.
>>>
>>> Maybe I'm missing something?
> 
> Yes, here are some things to consider.
> 
> The extension I made can just as easily be a function that returns a dev. tree.
> 
> - One can have a dev. tree update procedure similar to
>   rendundant env. The funktion then selects which dev. tree is valid and passes
>   that back.
> 
> - The function can construct a dev. tree from builtin rules/code.
> 
> - One can even make it tftp a tree at boot and pass that back. Great
>   in a development env. especially if you are debugging the dev. tree.
> 
> and the kicker is that you can still override this tree at runtime by passing
> a dev. tree argument to bootm.
> 
> All this will be possible by the simple patch I posted earlier, included
> below.
> 
> [PATCH] Make it possible to use a builtin OF tree.
> Signed-off-by: Joakim Tjernlund <Joakim.Tjernlund@transmode.se>
> ---
>  common/cmd_bootm.c |    5 ++++-
>  1 files changed, 4 insertions(+), 1 deletions(-)
> 
> diff --git a/common/cmd_bootm.c b/common/cmd_bootm.c
> index d816349..8405de7 100644
> --- a/common/cmd_bootm.c
> +++ b/common/cmd_bootm.c
> @@ -507,6 +507,9 @@ fixup_silent_linux ()
>  #endif /* CONFIG_SILENT_CONSOLE */
>  
>  #ifdef CONFIG_PPC
> +#ifndef DEFAULT_OF_TREE
> + #define DEFAULT_OF_TREE NULL
> +#endif
>  static void  __attribute__((noinline))
>  do_bootm_linux (cmd_tbl_t *cmdtp, int flag,
>  		int	argc, char *argv[],
> @@ -527,7 +530,7 @@ do_bootm_linux (cmd_tbl_t *cmdtp, int flag,
>  	void	(*kernel)(bd_t *, ulong, ulong, ulong, ulong);
>  	image_header_t *hdr = &header;
>  #if defined(CONFIG_OF_FLAT_TREE) || defined(CONFIG_OF_LIBFDT)
> -	char	*of_flat_tree = NULL;
> +	char	*of_flat_tree = DEFAULT_OF_TREE;
>  	ulong	of_data = 0;
>  #endif

FWIIW, while I appreciate and agree with Wolfgang's points, I don't see 
why we cannot add Joakim's configuration tweak.  It is a pretty minor 
issue and, if he finds it useful, perhaps it would be useful to others.

Having said that, the patch could be made better by IMHO using the 
#ifdef logic embedded rather than defining DEFAULT_OF_TREE to NULL if it 
isn't already defined:

#if defined(CONFIG_OF_FLAT_TREE) || defined(CONFIG_OF_LIBFDT)
#ifdef DEFAULT_OF_TREE
	char	*of_flat_tree = DEFAULT_OF_TREE;
#else
	char	*of_flat_tree = NULL;
#endif
	ulong	of_data = 0;
#endif

Also, we should have an update to the README to explain that 
DEFAULT_OF_TREE can be defined in the board-specific config file to be 
the address of a FDT embedded in u-boot or a function that 
builds/modifies a FDT and returns the address.

This should probably go somewhere in the section on LIBFDT line 332 ff. 
(I would put it at line 342, your line numbers may vary).
<http://www.denx.de/cgi-bin/gitweb.cgi?p=u-boot/u-boot-fdt.git;a=blob;f=README;h=09eb76fe4528dfedca027b119ac760fce2570e35;hb=HEAD#l341>

Best regards,
gvb

[U-Boot-Users] Can U-boot Autodetect arch/ppcversusarch/powerpc from info in the uImage?

[Joakim Tjernlund]

On Fri, 2007-12-07 at 08:14 -0500, Jerry Van Baren wrote:
> Joakim Tjernlund wrote:
> 
> [snip]
> 
> >>  Jocke
> >>> As far as I understand your request, this offers all the 
> >> same options
> >>> you can get with a compiled in device tree blob, but with  
> >> the  added
> >>> benefit of allowing many more things at the same time, too.
> >>>
> >>> Maybe I'm missing something?
> > 
> > Yes, here are some things to consider.
> > 
> > The extension I made can just as easily be a function that returns a dev. tree.
> > 
> > - One can have a dev. tree update procedure similar to
> >   rendundant env. The funktion then selects which dev. tree is valid and passes
> >   that back.
> > 
> > - The function can construct a dev. tree from builtin rules/code.
> > 
> > - One can even make it tftp a tree at boot and pass that back. Great
> >   in a development env. especially if you are debugging the dev. tree.
> > 
> > and the kicker is that you can still override this tree at runtime by passing
> > a dev. tree argument to bootm.
> > 
> > All this will be possible by the simple patch I posted earlier, included
> > below.
[SNIP]
> 
> FWIIW, while I appreciate and agree with Wolfgang's points, I don't see 
> why we cannot add Joakim's configuration tweak.  It is a pretty minor 
> issue and, if he finds it useful, perhaps it would be useful to others.
> 
> Having said that, the patch could be made better by IMHO using the 
> #ifdef logic embedded rather than defining DEFAULT_OF_TREE to NULL if it 
> isn't already defined:
> 
> #if defined(CONFIG_OF_FLAT_TREE) || defined(CONFIG_OF_LIBFDT)
> #ifdef DEFAULT_OF_TREE
> 	char	*of_flat_tree = DEFAULT_OF_TREE;
> #else
> 	char	*of_flat_tree = NULL;
> #endif
> 	ulong	of_data = 0;
> #endif
> 
> Also, we should have an update to the README to explain that 
> DEFAULT_OF_TREE can be defined in the board-specific config file to be 
> the address of a FDT embedded in u-boot or a function that 
> builds/modifies a FDT and returns the address.
> 
> This should probably go somewhere in the section on LIBFDT line 332 ff. 
> (I would put it at line 342, your line numbers may vary).
> <http://www.denx.de/cgi-bin/gitweb.cgi?p=u-boot/u-boot-fdt.git;a=blob;f=README;h=09eb76fe4528dfedca027b119ac760fce2570e35;hb=HEAD#l341>
> 
> Best regards,
> gvb

Made a new patch with Jerrys comments addressed. Also renamed
DEFAULT_OF_TREE to CFG_OF_TREE. OK?

[U-Boot-Users] Can U-boot Autodetect arch/ppcversusarch/powerpc from info in the uImage?

[Wolfgang Denk]

In message <1197205927.937.51.camel@gentoo-jocke.transmode.se> you wrote:
> 
> Made a new patch with Jerrys comments addressed. Also renamed
> DEFAULT_OF_TREE to CFG_OF_TREE. OK?

I still object against this modification.

> +#ifdef CFG_OF_TREE
> +	char	*of_flat_tree = CFG_OF_TREE;
> +#else
>  	char	*of_flat_tree = NULL;
> +#endif
>  	ulong	of_data = 0;
>  #endif

I hereby NAK this patch for 3 reasons:

1) The patch does not solve a problem. Instead of harwiring the
   address, you can just pass it as argument to the bootm command
   which seems more straightforward to me

2) The patch causes confusion. Documented behaviour is that "bootm"
   with one or two arguments (kernel address, or kernel plus ramdisk
   addresses) will boot a non-OF enabled kernel. With this patch,
   "bootm" will behave different on systems where the CFG_OF_TREE has
   been silected - which is usually not known to and cannot be
   checked by the end user, thus causing confusion.

3) With the patch applied and CFG_OF_TREE defined, there is no way to
   boot a non-OF kernel, thus breaking backward compatibility.

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
If you're not part of the solution, you're part of the problem.

[U-Boot-Users] Can U-boot Autodetect arch/ppcversusarch/powerpc from info in the uImage?

[Joakim Tjernlund]

> -----Original Message-----
> From: wd at denx.de [mailto:wd at denx.de] 
> Sent: den 9 december 2007 16:01
> To: joakim.tjernlund at transmode.se
> Cc: Jerry Van Baren; u-boot-users at lists.sourceforge.net; 
> 'Darcy Watkins'
> Subject: Re: [U-Boot-Users] Can U-boot Autodetect 
> arch/ppcversusarch/powerpc from info in the uImage?
> 
> In message 
> <1197205927.937.51.camel@gentoo-jocke.transmode.se> you wrote:
> > 
> > Made a new patch with Jerrys comments addressed. Also renamed
> > DEFAULT_OF_TREE to CFG_OF_TREE. OK?
> 
> I still object against this modification.
> 
> > +#ifdef CFG_OF_TREE
> > +	char	*of_flat_tree = CFG_OF_TREE;
> > +#else
> >  	char	*of_flat_tree = NULL;
> > +#endif
> >  	ulong	of_data = 0;
> >  #endif
> 
> I hereby NAK this patch for 3 reasons:
> 
> 1) The patch does not solve a problem. Instead of harwiring the
>    address, you can just pass it as argument to the bootm command
>    which seems more straightforward to me

It is not a hardwired address, it is a function call which returns an address to
the OF tree of your choise. You can define it to an address too if you like.

> 
> 2) The patch causes confusion. Documented behaviour is that "bootm"
>    with one or two arguments (kernel address, or kernel plus ramdisk
>    addresses) will boot a non-OF enabled kernel. With this patch,
>    "bootm" will behave different on systems where the CFG_OF_TREE has
>    been silected - which is usually not known to and cannot be
>    checked by the end user, thus causing confusion.
> 
> 3) With the patch applied and CFG_OF_TREE defined, there is no way to
>    boot a non-OF kernel, thus breaking backward compatibility.

hmm, backwards compatibility. Current u-boot breaks backwards compatibility
with older u-boots that didn't require an OF tree to be passed as an argument to
bootm. Dunno when that happened but 1.1.4 has
  extern const unsigned char oftree_dtb[];
  extern const unsigned int oftree_dtb_len;
That needs to be defined in board code and holds the OF tree.
The CFG_OF_TREE(or some variation there of) is the only way I can
think of to support uppgrade of the older boards in the field in a
easy and non error prone way.

 Jocke

[U-Boot-Users] Can U-boot Autodetect arch/ppcversusarch/powerpc from info in the uImage?

[Wolfgang Denk]

In message <010601c83a89$8b22bfa0$02ac10ac@Jocke> you wrote:
>
> > I hereby NAK this patch for 3 reasons:
> > 
> > 1) The patch does not solve a problem. Instead of harwiring the
> >    address, you can just pass it as argument to the bootm command
> >    which seems more straightforward to me
> 
> It is not a hardwired address, it is a function call which returns an address to
> the OF tree of your choise. You can define it to an address too if you like.

Thanks for the correction. But actually it makes little difference;
it's a static thingy as we can't pass (varying) arguments to it. Or am
I missing something?

> > 3) With the patch applied and CFG_OF_TREE defined, there is no way to
> >    boot a non-OF kernel, thus breaking backward compatibility.
> 
> hmm, backwards compatibility. Current u-boot breaks backwards compatibility
> with older u-boots that didn't require an OF tree to be passed as an argument to
> bootm. Dunno when that happened but 1.1.4 has

In which way? We use OF-enabled versions of U-Boot on several boards
that are in use both with old (2.4.25) and new (2.6.23.X) non-OF
kernels, and with OF-enabled kernels as well. There are problems,
indeed, in certain constellations using multi-file images, but as long
as you use separate kernel, ramdisk and/or dtb images it works fine.

>   extern const unsigned char oftree_dtb[];
>   extern const unsigned int oftree_dtb_len;
> That needs to be defined in board code and holds the OF tree.

These lines were removed by this commit:

Author: Matthew McClintock <msm@freescale.com>  2006-06-28 17:41:37
Committer: Jon Loeliger <jdl@freescale.com>  2006-08-09 20:50:11
Parent: d87080b721e4f8dca977af7571c5338ae7bb8db7 (GCC-4.x fixes: clean up global data pointer initialization for all boards.)
Child:  5498d90312aad9f6bdbf047986027c35b03cd163 (* Patch to modify ft_build.c to update flat device trees in place Patch by Matthew McClintock 26-June-2006)
Follows: NIOS2-5_0_0
Precedes: U-Boot-1_1_6

    * Modify bootm command to support booting with flat device trees
      Patch by Matthew McClintock 26-June-2006

> The CFG_OF_TREE(or some variation there of) is the only way I can
> think of to support uppgrade of the older boards in the field in a
> easy and non error prone way.

Sorry, but I don't get you.

You started this discussion by mentioning that you want to embed  the
DTB into the U-Boot image (and I commented you could just append it).
If  you  upgrade  an  old board by installing a new U-Boot image, you
will also overwrite the old embedded DTB and install a new  one  with
the  new  image, and this new address is well known to you so you can
easily pass it on the bootm command line.

Assuming the DTB image was NOT part of the U-Boot image it's  address
would be known, too, so the same method could be used.

Finally, you could even compute the address by some code (although  I
have  to  admit  that I cannot image how that could be needed, but my
imagination is definitely limited) - nothing prevents you to add some
board specific code that is run automatically (for example,  as  part
of  your  misc_init_r()  or  last_stage_init()  code) or manually (by
defining a board-specific command). This code could define or  modify
an  environment  variable  which  then  in  turn  gets  used as third
argument to the bootm call. [Yes,  this  sounds  complicated,  and  I
agree  that  it's  probably  never needed, but it would implement the
same what you want to do in  a  board-specific  way,  i.  e.  without
impact on other boards.]



I feel we are pretty much stuck here. You insist you need this, and I
fail to see any such need.

Could please somebody else (probaly with a  deeper  understanding  of
DTB  magic than me) try to moderate this discussion so we can come to
a decision that is based on mutual understanding?

Volunteers welcome...

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
There are bugs and then there are bugs.  And then there are bugs.
                                                    - Karl Lehenbauer

[U-Boot-Users] Can U-boot Autodetect arch/ppcversusarch/powerpc from info in the uImage?

[Joakim Tjernlund]

> -----Original Message-----
> From: wd at denx.de [mailto:wd at denx.de] 
> Sent: den 9 december 2007 20:28
> To: Joakim Tjernlund
> Cc: 'Jerry Van Baren'; u-boot-users at lists.sourceforge.net; 
> 'Darcy Watkins'
> Subject: Re: [U-Boot-Users] Can U-boot Autodetect 
> arch/ppcversusarch/powerpc from info in the uImage?
> 
> In message <010601c83a89$8b22bfa0$02ac10ac@Jocke> you wrote:
> >
> > > I hereby NAK this patch for 3 reasons:
> > > 
> > > 1) The patch does not solve a problem. Instead of harwiring the
> > >    address, you can just pass it as argument to the bootm command
> > >    which seems more straightforward to me
> > 
> > It is not a hardwired address, it is a function call which 
> returns an address to
> > the OF tree of your choise. You can define it to an address 
> too if you like.
> 
> Thanks for the correction. But actually it makes little difference;
> it's a static thingy as we can't pass (varying) arguments to it. Or am
> I missing something?

No args can be passed but this function can have other criteria for
selecting/constructing an OF tree, a time stamp, an env variable or
perhaps just checking a CRC among two OF images and select one
that has a vaild CRC. The selection of OF tree also takes place
during bootm which makes it possible to have the CFG_OF_TREE function
tftp an OF tree and pass that to the bootm command.
  
So there are a few interesting options with CFG_OF_TREE which
is easier to do within a function than playing with env variables.

> 
> > > 3) With the patch applied and CFG_OF_TREE defined, there 
> is no way to
> > >    boot a non-OF kernel, thus breaking backward compatibility.
> > 
> > hmm, backwards compatibility. Current u-boot breaks 
> backwards compatibility
> > with older u-boots that didn't require an OF tree to be 
> passed as an argument to
> > bootm. Dunno when that happened but 1.1.4 has
> 
> In which way? We use OF-enabled versions of U-Boot on several boards
> that are in use both with old (2.4.25) and new (2.6.23.X) non-OF
> kernels, and with OF-enabled kernels as well. There are problems,
> indeed, in certain constellations using multi-file images, but as long
> as you use separate kernel, ramdisk and/or dtb images it works fine.

You need to pass an extra arg to bootm. Our boards don't do that.

> 
> >   extern const unsigned char oftree_dtb[];
> >   extern const unsigned int oftree_dtb_len;
> > That needs to be defined in board code and holds the OF tree.
> 
> These lines were removed by this commit:
> 
> Author: Matthew McClintock <msm@freescale.com>  2006-06-28 17:41:37
> Committer: Jon Loeliger <jdl@freescale.com>  2006-08-09 20:50:11
> Parent: d87080b721e4f8dca977af7571c5338ae7bb8db7 (GCC-4.x 
> fixes: clean up global data pointer initialization for all boards.)
> Child:  5498d90312aad9f6bdbf047986027c35b03cd163 (* Patch to 
> modify ft_build.c to update flat device trees in place Patch 
> by Matthew McClintock 26-June-2006)
> Follows: NIOS2-5_0_0
> Precedes: U-Boot-1_1_6
> 
>     * Modify bootm command to support booting with flat device trees
>       Patch by Matthew McClintock 26-June-2006
> 
> > The CFG_OF_TREE(or some variation there of) is the only way I can
> > think of to support uppgrade of the older boards in the field in a
> > easy and non error prone way.
> 
> Sorry, but I don't get you.
> 
> You started this discussion by mentioning that you want to embed  the
> DTB into the U-Boot image (and I commented you could just append it).
> If  you  upgrade  an  old board by installing a new U-Boot image, you
> will also overwrite the old embedded DTB and install a new  one  with
> the  new  image, and this new address is well known to you so you can
> easily pass it on the bootm command line.

That would involve updating the bootcmd env. variable as part of the
upgrade and I was hoping to avoid that. Futhermore if you downgrade
I must reverse that change to bootcmd and the old code don't know anything
about that so I need to invent some method to ensure this happens.

> 
> Assuming the DTB image was NOT part of the U-Boot image it's  address
> would be known, too, so the same method could be used.
> 
> Finally, you could even compute the address by some code (although  I
> have  to  admit  that I cannot image how that could be needed, but my
> imagination is definitely limited) - nothing prevents you to add some
> board specific code that is run automatically (for example,  as  part
> of  your  misc_init_r()  or  last_stage_init()  code) or manually (by
> defining a board-specific command). This code could define or  modify
> an  environment  variable  which  then  in  turn  gets  used as third
> argument to the bootm call. [Yes,  this  sounds  complicated,  and  I
> agree  that  it's  probably  never needed, but it would implement the
> same what you want to do in  a  board-specific  way,  i.  e.  without
> impact on other boards.]

Computing the address isn't the problem, the problem is the new
syntax of bootm. The CFG_OF_TREE was the best I could come up with.
Maybe other boards will face the same problem in the future when
they upgrade boards? I really have no idea.

> 
> 
> 
> I feel we are pretty much stuck here. You insist you need this, and I
> fail to see any such need.
> 
> Could please somebody else (probaly with a  deeper  understanding  of
> DTB  magic than me) try to moderate this discussion so we can come to
> a decision that is based on mutual understanding?

Wolgang, thanks for bearing with me for so long. If you still think
CFG_OF_TREE(or some variation there of) is inappropriate for u-boot
I will accept that and move on.

> 
> Volunteers welcome...
> 
> Best regards,
> 
> Wolfgang Denk
> 

[U-Boot-Users] Can U-boot Autodetect arch/ppcversusarch/powerpc from info in the uImage?

[Wolfgang Denk]

In message <011301c83aa8$84a60a50$02ac10ac@Jocke> you wrote:
>
> > Thanks for the correction. But actually it makes little difference;
> > it's a static thingy as we can't pass (varying) arguments to it. Or am
> > I missing something?
> 
> No args can be passed but this function can have other criteria for
> selecting/constructing an OF tree, a time stamp, an env variable or
> perhaps just checking a CRC among two OF images and select one
> that has a vaild CRC. The selection of OF tree also takes place
> during bootm which makes it possible to have the CFG_OF_TREE function
> tftp an OF tree and pass that to the bootm command.

All that could be done using U-Boot commands / scripts, too.

> So there are a few interesting options with CFG_OF_TREE which
> is easier to do within a function than playing with env variables.

I still diagree...

> > > hmm, backwards compatibility. Current u-boot breaks backwards compatibility
> > > with older u-boots that didn't require an OF tree to be passed as an argument to
> > > bootm. Dunno when that happened but 1.1.4 has
> > 
> > In which way? We use OF-enabled versions of U-Boot on several boards
> > that are in use both with old (2.4.25) and new (2.6.23.X) non-OF
> > kernels, and with OF-enabled kernels as well. There are problems,
> > indeed, in certain constellations using multi-file images, but as long
> > as you use separate kernel, ramdisk and/or dtb images it works fine.
> 
> You need to pass an extra arg to bootm. Our boards don't do that.

True. I consider this a bug that needed to be fixed.

> > You started this discussion by mentioning that you want to embed  the
> > DTB into the U-Boot image (and I commented you could just append it).
> > If  you  upgrade  an  old board by installing a new U-Boot image, you
> > will also overwrite the old embedded DTB and install a new  one  with
> > the  new  image, and this new address is well known to you so you can
> > easily pass it on the bootm command line.
> 
> That would involve updating the bootcmd env. variable as part of the
> upgrade and I was hoping to avoid that. Futhermore if you downgrade
> I must reverse that change to bootcmd and the old code don't know anything
> about that so I need to invent some method to ensure this happens.

Well, I think you're seeing more complications than necessary.

Assume you change your bootcmd to something like this:

=> sete bootcmd "... ; ... ; bootm ${kernel} ${ramdisk} ${dtb}"

The all it takes to  make  this  bootcmd  compatible  with  your  old
version of U-Boot is deleting the "dtb" environment variable.

> Computing the address isn't the problem, the problem is the new
> syntax of bootm. The CFG_OF_TREE was the best I could come up with.

Ah, I see. But that is exactly the reason why I  reject  your  patch.
The  (documented) behaviour of the "bootm" command as is shall not be
changed - a DTB shall be passed to the Linux kernel if and only if  a
third  argument  is present (again, not considering the addex compli-
cations of multifile images). The state of the code you are referring
to was an intermediate state, which was recognized to have  problems,
that were addressed and fixed later. I'm sorry if you relied too much
on  this  behaviour.  On  the other hand, migration to the new scheme
(and even back, if really necessary) seems not too difficult to me.

> Wolgang, thanks for bearing with me for so long. If you still think
> CFG_OF_TREE(or some variation there of) is inappropriate for u-boot
> I will accept that and move on.

Thanks. I still would appreciate some independent comments - maybe  I
just fail to see the full scope of this.

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
It usually takes more than three weeks to prepare  a  good  impromptu
speech.                                                  - Mark Twain

[U-Boot-Users] Can U-boot Autodetect arch/ppcversusarch/powerpc from info in the uImage?

[Joakim Tjernlund]

> -----Original Message-----
> From: wd at denx.de [mailto:wd at denx.de] 
> Sent: den 9 december 2007 22:36
> To: Joakim Tjernlund
> Cc: 'Jerry Van Baren'; u-boot-users at lists.sourceforge.net; 
> 'Darcy Watkins'
> Subject: Re: [U-Boot-Users] Can U-boot Autodetect 
> arch/ppcversusarch/powerpc from info in the uImage?
> 
> In message <011301c83aa8$84a60a50$02ac10ac@Jocke> you wrote:
> >
> > > Thanks for the correction. But actually it makes little 
> difference;
> > > it's a static thingy as we can't pass (varying) arguments 
> to it. Or am
> > > I missing something?
> > 
> > No args can be passed but this function can have other criteria for
> > selecting/constructing an OF tree, a time stamp, an env variable or
> > perhaps just checking a CRC among two OF images and select one
> > that has a vaild CRC. The selection of OF tree also takes place
> > during bootm which makes it possible to have the 
> CFG_OF_TREE function
> > tftp an OF tree and pass that to the bootm command.
> 
> All that could be done using U-Boot commands / scripts, too.
> 
> > So there are a few interesting options with CFG_OF_TREE which
> > is easier to do within a function than playing with env variables.
> 
> I still diagree...

I thought you disagreed with how I modified the bootm command, I can't
image than all the above is easier to do by using env. variables/scripts?
Especially the tftp idea.

> 
> > > > hmm, backwards compatibility. Current u-boot breaks 
> backwards compatibility
> > > > with older u-boots that didn't require an OF tree to be 
> passed as an argument to
> > > > bootm. Dunno when that happened but 1.1.4 has
> > > 
> > > In which way? We use OF-enabled versions of U-Boot on 
> several boards
> > > that are in use both with old (2.4.25) and new (2.6.23.X) non-OF
> > > kernels, and with OF-enabled kernels as well. There are problems,
> > > indeed, in certain constellations using multi-file 
> images, but as long
> > > as you use separate kernel, ramdisk and/or dtb images it 
> works fine.
> > 
> > You need to pass an extra arg to bootm. Our boards don't do that.
> 
> True. I consider this a bug that needed to be fixed.

My bug or an u-boot bug?

> 
> > > You started this discussion by mentioning that you want 
> to embed  the
> > > DTB into the U-Boot image (and I commented you could just 
> append it).
> > > If  you  upgrade  an  old board by installing a new 
> U-Boot image, you
> > > will also overwrite the old embedded DTB and install a 
> new  one  with
> > > the  new  image, and this new address is well known to 
> you so you can
> > > easily pass it on the bootm command line.
> > 
> > That would involve updating the bootcmd env. variable as part of the
> > upgrade and I was hoping to avoid that. Futhermore if you downgrade
> > I must reverse that change to bootcmd and the old code 
> don't know anything
> > about that so I need to invent some method to ensure this happens.
> 
> Well, I think you're seeing more complications than necessary.
> 
> Assume you change your bootcmd to something like this:
> 
> => sete bootcmd "... ; ... ; bootm ${kernel} ${ramdisk} ${dtb}"
> 
> The all it takes to  make  this  bootcmd  compatible  with  your  old
> version of U-Boot is deleting the "dtb" environment variable.

Yes, but I would have do make sure it is deleted BEFORE I downgrade.

> 
> > Computing the address isn't the problem, the problem is the new
> > syntax of bootm. The CFG_OF_TREE was the best I could come up with.
> 
> Ah, I see. But that is exactly the reason why I  reject  your  patch.
> The  (documented) behaviour of the "bootm" command as is shall not be
> changed - a DTB shall be passed to the Linux kernel if and only if  a
> third  argument  is present (again, not considering the addex compli-
> cations of multifile images). The state of the code you are referring
> to was an intermediate state, which was recognized to have  problems,
> that were addressed and fixed later. I'm sorry if you relied too much
> on  this  behaviour.  On  the other hand, migration to the new scheme
> (and even back, if really necessary) seems not too difficult to me.

Yes, I realize that there was a reason for the new bootm syntax. I was 
hoping for an easy migration path for those who got caught in the middle.

> 
> > Wolgang, thanks for bearing with me for so long. If you still think
> > CFG_OF_TREE(or some variation there of) is inappropriate for u-boot
> > I will accept that and move on.
> 
> Thanks. I still would appreciate some independent comments - maybe  I
> just fail to see the full scope of this.

Me too, lets see if any of the OF tree experts has anything to add.

 Regards
         Jocke

> 
> Best regards,
> 
> Wolfgang Denk

[U-Boot-Users] Can U-boot Autodetect arch/ppcversusarch/powerpcfrom info in the uImage?

[Joakim Tjernlund]

[SNIP]

> > 
> > > Wolgang, thanks for bearing with me for so long. If you 
> still think
> > > CFG_OF_TREE(or some variation there of) is inappropriate 
> for u-boot
> > > I will accept that and move on.
> > 
> > Thanks. I still would appreciate some independent comments 
> - maybe  I
> > just fail to see the full scope of this.
> 
> Me too, lets see if any of the OF tree experts has anything to add.
> 
>  Regards
>          Jocke

Just wanted to add something. I think most OF tree users see the
DTB as something static, install once and you are done. I don't
see it that way.
The syntax of the OF tree is still evolving and I see updates
to existing devices go into the kernel. Backwards compatibility
is usually kept but you can't update to the new syntax if you
want the option to downgrade.
The type of CPUs I work with(mpc8xx and mpc83xx) has a lot
of devices in them and I just use a few but as time goes I add
one or two devices since I need them. Trying to add in all conceivable
devices from the beginning won't work either as some
of them might not have a syntax yet or is evolving.
When downgrading to an older version I don't really
want to the old boot/kernel use the new OF tree as there is
no way to know if something will go wrong without testing that
combination first. This adds a lot of extra work for the
verification team and some extra work for myself.

Basically the DTB follows the with the release and if
I downgrade I want the DTB to follow suit too.
This is why I want to keep the DTB in the u-boot image so
I don't have to mange yet another image that buys us nothing.

Comments?

 Jocke 
  

[U-Boot-Users] Can U-boot Autodetect arch/ppcversusarch/powerpc from info in the uImage?

[Jerry Van Baren]

Wolfgang Denk wrote:
> In message <1197205927.937.51.camel@gentoo-jocke.transmode.se> you wrote:
>> Made a new patch with Jerrys comments addressed. Also renamed
>> DEFAULT_OF_TREE to CFG_OF_TREE. OK?
> 
> I still object against this modification.
> 
>> +#ifdef CFG_OF_TREE
>> +	char	*of_flat_tree = CFG_OF_TREE;
>> +#else
>>  	char	*of_flat_tree = NULL;
>> +#endif
>>  	ulong	of_data = 0;
>>  #endif
> 
> I hereby NAK this patch for 3 reasons:
> 
> 1) The patch does not solve a problem. Instead of hardwiring the
>    address, you can just pass it as argument to the bootm command
>    which seems more straightforward to me
> 
> 2) The patch causes confusion. Documented behaviour is that "bootm"
>    with one or two arguments (kernel address, or kernel plus ramdisk
>    addresses) will boot a non-OF enabled kernel. With this patch,
>    "bootm" will behave different on systems where the CFG_OF_TREE has
>    been selected - which is usually not known to and cannot be
>    checked by the end user, thus causing confusion.
> 
> 3) With the patch applied and CFG_OF_TREE defined, there is no way to
>    boot a non-OF kernel, thus breaking backward compatibility.
> 
> Best regards,
> 
> Wolfgang Denk

FWIIW, #2 and #3 are serious problems that I had not considered when I 
supported Jocke's proposed patch.  Sorry, Jocke, but I have to side with 
Wolfgang in light of those arguments.

Best regards,
gvb

[U-Boot-Users] Can U-boot Autodetect arch/ppcversusarch/powerpc from info in the uImage?

[Joakim Tjernlund]

> -----Original Message-----
> From: Jerry Van Baren [mailto:gerald.vanbaren at ge.com] 
> Sent: den 13 december 2007 19:49
> To: Wolfgang Denk
> Cc: joakim.tjernlund at transmode.se; u-boot-users at lists.sourceforge.net
> Subject: Re: [U-Boot-Users] Can U-boot Autodetect 
> arch/ppcversusarch/powerpc from info in the uImage?
> 
> Wolfgang Denk wrote:
> > In message 
> <1197205927.937.51.camel@gentoo-jocke.transmode.se> you wrote:
> >> Made a new patch with Jerrys comments addressed. Also renamed
> >> DEFAULT_OF_TREE to CFG_OF_TREE. OK?
> > 
> > I still object against this modification.
> > 
> >> +#ifdef CFG_OF_TREE
> >> +	char	*of_flat_tree = CFG_OF_TREE;
> >> +#else
> >>  	char	*of_flat_tree = NULL;
> >> +#endif
> >>  	ulong	of_data = 0;
> >>  #endif
> > 
> > I hereby NAK this patch for 3 reasons:
> > 
> > 1) The patch does not solve a problem. Instead of hardwiring the
> >    address, you can just pass it as argument to the bootm command
> >    which seems more straightforward to me
> > 
> > 2) The patch causes confusion. Documented behaviour is that "bootm"
> >    with one or two arguments (kernel address, or kernel plus ramdisk
> >    addresses) will boot a non-OF enabled kernel. With this patch,
> >    "bootm" will behave different on systems where the 
> CFG_OF_TREE has
> >    been selected - which is usually not known to and cannot be
> >    checked by the end user, thus causing confusion.
> > 
> > 3) With the patch applied and CFG_OF_TREE defined, there is 
> no way to
> >    boot a non-OF kernel, thus breaking backward compatibility.
> > 
> > Best regards,
> > 
> > Wolfgang Denk
> 
> FWIIW, #2 and #3 are serious problems that I had not 
> considered when I 
> supported Jocke's proposed patch.  Sorry, Jocke, but I have 
> to side with 
> Wolfgang in light of those arguments.

Right and I havn't been able to come up with a solution to that either.
So I am looking at passing a $dtb to bootm.

I noticed that I could define dtb i HUSH only by
dtb=0x12345678
Is it possible to do that from within board code too?

If so I don't have to worry about deleting $dtb when downgrading,
because it will never be saved in the env.

 Jocke

[U-Boot-Users] Can U-boot Autodetect arch/ppcversusarch/powerpc from info in the uImage?

[Joakim Tjernlund]

On Fri, 2007-12-14 at 00:03 +0100, Joakim Tjernlund wrote:
> > -----Original Message-----
> > From: Jerry Van Baren [mailto:gerald.vanbaren at ge.com] 
> > Sent: den 13 december 2007 19:49
> > To: Wolfgang Denk
> > Cc: joakim.tjernlund at transmode.se; u-boot-users at lists.sourceforge.net
> > Subject: Re: [U-Boot-Users] Can U-boot Autodetect 
> > arch/ppcversusarch/powerpc from info in the uImage?
> > 
> > Wolfgang Denk wrote:
> > > In message 
> > <1197205927.937.51.camel@gentoo-jocke.transmode.se> you wrote:
> > >> Made a new patch with Jerrys comments addressed. Also renamed
> > >> DEFAULT_OF_TREE to CFG_OF_TREE. OK?
> > > 
> > > I still object against this modification.
> > > 
> > >> +#ifdef CFG_OF_TREE
> > >> +	char	*of_flat_tree = CFG_OF_TREE;
> > >> +#else
> > >>  	char	*of_flat_tree = NULL;
> > >> +#endif
> > >>  	ulong	of_data = 0;
> > >>  #endif
> > > 
> > > I hereby NAK this patch for 3 reasons:
> > > 
> > > 1) The patch does not solve a problem. Instead of hardwiring the
> > >    address, you can just pass it as argument to the bootm command
> > >    which seems more straightforward to me
> > > 
> > > 2) The patch causes confusion. Documented behaviour is that "bootm"
> > >    with one or two arguments (kernel address, or kernel plus ramdisk
> > >    addresses) will boot a non-OF enabled kernel. With this patch,
> > >    "bootm" will behave different on systems where the 
> > CFG_OF_TREE has
> > >    been selected - which is usually not known to and cannot be
> > >    checked by the end user, thus causing confusion.
> > > 
> > > 3) With the patch applied and CFG_OF_TREE defined, there is 
> > no way to
> > >    boot a non-OF kernel, thus breaking backward compatibility.
> > > 
> > > Best regards,
> > > 
> > > Wolfgang Denk
> > 
> > FWIIW, #2 and #3 are serious problems that I had not 
> > considered when I 
> > supported Jocke's proposed patch.  Sorry, Jocke, but I have 
> > to side with 
> > Wolfgang in light of those arguments.
> 
> Right and I havn't been able to come up with a solution to that either.
> So I am looking at passing a $dtb to bootm.
> 
> I noticed that I could define dtb i HUSH only by
> dtb=0x12345678
> Is it possible to do that from within board code too?
> 
> If so I don't have to worry about deleting $dtb when downgrading,
> because it will never be saved in the env.
> 
>  Jocke

This is what I came up with to make $dtb a hush variable that don't
end up in the environment:

#define FLAG_PARSE_SEMICOLON (1 << 1)
#define FLAG_EXIT_FROM_LOOP 1
int misc_init_r (void)
{
	char *bootstr, dtb_str[30], workstr[256];

	u_boot_hush_start();
	sprintf(dtb_str, "dtb=0x%lx", (ulong)dt_blob_start);
	/* Set $dtb in local HUSH env.to my OF tree */
	if (parse_string_outer(dtb_str,
			       FLAG_PARSE_SEMICOLON | FLAG_EXIT_FROM_LOOP) != 0) {
		/* Add " - $dtb" to $bootcmd if it is missing */
		bootstr = getenv("bootcmd");
		if (bootstr && !strstr(bootstr, "- $dtb")) {
			strcpy(workstr, bootstr);
			strcat(workstr, " - $dtb");
			setenv("bootcmd", workstr);
		}
	}
	return 0;
}

It is fairly ugly, but it works. Comments?

BTW, whats syntax is preferred in HUSH for variables:
$var or ${var}?

 Jocke

[U-Boot-Users] Can U-boot Autodetect arch/ppcversusarch/powerpc from info in the uImage?

[Wolfgang Denk]

In message <1197647601.21876.12.camel@gentoo-jocke.transmode.se> you wrote:
>
> This is what I came up with to make $dtb a hush variable that don't
> end up in the environment:

> #define FLAG_PARSE_SEMICOLON (1 << 1)
> #define FLAG_EXIT_FROM_LOOP 1
> int misc_init_r (void)
> {
> 	char *bootstr, dtb_str[30], workstr[256];
> 
> 	u_boot_hush_start();

Be careful. You are invoking undefined behaviour heare. The  idea  is
that  u_boot_hush_start() is supposed to be called exactly once only.
[Yes, I know from looking at the code that at the moment  this  seems
to be OK, but ther eis no guarantee for it.]

> 	sprintf(dtb_str, "dtb=0x%lx", (ulong)dt_blob_start);
> 	/* Set $dtb in local HUSH env.to my OF tree */
> 	if (parse_string_outer(dtb_str,
> 			       FLAG_PARSE_SEMICOLON | FLAG_EXIT_FROM_LOOP) != 0) {
> 		/* Add " - $dtb" to $bootcmd if it is missing */
> 		bootstr = getenv("bootcmd");
> 		if (bootstr && !strstr(bootstr, "- $dtb")) {
> 			strcpy(workstr, bootstr);
> 			strcat(workstr, " - $dtb");
> 			setenv("bootcmd", workstr);
> 		}
> 	}
> 	return 0;
> }
> 
> It is fairly ugly, but it works. Comments?

I don't understand why you make it so complicated when a preboot
command would do exactly the same?

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
The only time the world beats a path to your door is when you are  in
the bathroom.

[U-Boot-Users] Can U-boot Autodetect arch/ppcversusarch/powerpc from info in the uImage?

[Joakim Tjernlund]

> -----Original Message-----
> From: wd at denx.de [mailto:wd at denx.de] 
> Sent: den 16 december 2007 21:15
> To: joakim.tjernlund at transmode.se
> Cc: 'Jerry Van Baren'; u-boot-users at lists.sourceforge.net
> Subject: Re: [U-Boot-Users] Can U-boot Autodetect 
> arch/ppcversusarch/powerpc from info in the uImage?
> 
> In message 
> <1197647601.21876.12.camel@gentoo-jocke.transmode.se> you wrote:
> >
> > This is what I came up with to make $dtb a hush variable that don't
> > end up in the environment:
> 
> > #define FLAG_PARSE_SEMICOLON (1 << 1)
> > #define FLAG_EXIT_FROM_LOOP 1
> > int misc_init_r (void)
> > {
> > 	char *bootstr, dtb_str[30], workstr[256];
> > 
> > 	u_boot_hush_start();
> 
> Be careful. You are invoking undefined behaviour heare. The  idea  is
> that  u_boot_hush_start() is supposed to be called exactly once only.
> [Yes, I know from looking at the code that at the moment  this  seems
> to be OK, but ther eis no guarantee for it.]

Yes, I suspected this. Maybe it would be a good idea to move the call
to u_boot_hush_start() earlier in common code so that there is no need
to do it here?

> 
> > 	sprintf(dtb_str, "dtb=0x%lx", (ulong)dt_blob_start);
> > 	/* Set $dtb in local HUSH env.to my OF tree */
> > 	if (parse_string_outer(dtb_str,
> > 			       FLAG_PARSE_SEMICOLON | 
> FLAG_EXIT_FROM_LOOP) != 0) {
> > 		/* Add " - $dtb" to $bootcmd if it is missing */
> > 		bootstr = getenv("bootcmd");
> > 		if (bootstr && !strstr(bootstr, "- $dtb")) {
> > 			strcpy(workstr, bootstr);
> > 			strcat(workstr, " - $dtb");
> > 			setenv("bootcmd", workstr);
> > 		}
> > 	}
> > 	return 0;
> > }
> > 
> > It is fairly ugly, but it works. Comments?
> 
> I don't understand why you make it so complicated when a preboot
> command would do exactly the same?

hmm, didn't think of that. Will look into that tmw, thanks

> 
> Best regards,
> 
> Wolfgang Denk

[U-Boot-Users] Can U-boot Autodetect arch/ppcversusarch/powerpc from info in the uImage?

[Joakim Tjernlund]

On Sun, 2007-12-16 at 22:27 +0100, Joakim Tjernlund wrote:
> > -----Original Message-----
> > From: wd at denx.de [mailto:wd at denx.de] 
> > Sent: den 16 december 2007 21:15
> > To: joakim.tjernlund at transmode.se
> > Cc: 'Jerry Van Baren'; u-boot-users at lists.sourceforge.net
> > Subject: Re: [U-Boot-Users] Can U-boot Autodetect 
> > arch/ppcversusarch/powerpc from info in the uImage?
> > 
> > In message 
> > <1197647601.21876.12.camel@gentoo-jocke.transmode.se> you wrote:
> > >
> > > This is what I came up with to make $dtb a hush variable that don't
> > > end up in the environment:
> > 
> > > #define FLAG_PARSE_SEMICOLON (1 << 1)
> > > #define FLAG_EXIT_FROM_LOOP 1
> > > int misc_init_r (void)
> > > {
> > > 	char *bootstr, dtb_str[30], workstr[256];
> > > 
> > > 	u_boot_hush_start();
> > 
> > Be careful. You are invoking undefined behaviour heare. The  idea  is
> > that  u_boot_hush_start() is supposed to be called exactly once only.
> > [Yes, I know from looking at the code that at the moment  this  seems
> > to be OK, but ther eis no guarantee for it.]
> 
> Yes, I suspected this. Maybe it would be a good idea to move the call
> to u_boot_hush_start() earlier in common code so that there is no need
> to do it here?
> 
> > 
> > > 	sprintf(dtb_str, "dtb=0x%lx", (ulong)dt_blob_start);
> > > 	/* Set $dtb in local HUSH env.to my OF tree */
> > > 	if (parse_string_outer(dtb_str,
> > > 			       FLAG_PARSE_SEMICOLON | 
> > FLAG_EXIT_FROM_LOOP) != 0) {
> > > 		/* Add " - $dtb" to $bootcmd if it is missing */
> > > 		bootstr = getenv("bootcmd");
> > > 		if (bootstr && !strstr(bootstr, "- $dtb")) {
> > > 			strcpy(workstr, bootstr);
> > > 			strcat(workstr, " - $dtb");
> > > 			setenv("bootcmd", workstr);
> > > 		}
> > > 	}
> > > 	return 0;
> > > }
> > > 
> > > It is fairly ugly, but it works. Comments?
> > 
> > I don't understand why you make it so complicated when a preboot
> > command would do exactly the same?
> 
> hmm, didn't think of that. Will look into that tmw, thanks

There is a chance that the preboot variable will end up in the
environment and that could break the old boot when downgrading.
Anyhow, I got an solution now that works and after a few releases, I
can remove it.

When looking through the code I noticed that there is a lot of  #ifdef
CFG_HUSH_PARSER, especially code like this:
#ifndef CFG_HUSH_PARSER
		if (run_command (arg, flag) == -1)
			return 1;
#else
		if (parse_string_outer(arg,
		    FLAG_PARSE_SEMICOLON | FLAG_EXIT_FROM_LOOP) != 0)
			return 1;
#endif
It seems to me that arse_string_outer() could be moved inside the
run_command() instead. That would make the code cleaner and make the
image smaller. I guess there is a reason why it is the way it is?

 Jocke