[U-Boot-Users] Can U-boot Autodetect arch/ppc versus arch/powerpc from info in the uImage?
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
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
-----Original Message----- From: u-boot-users-bounces@lists.sourceforge.net [mailto:u-boot-users-bounces@lists.sourceforge.net] On Behalf Of Wolfgang Denk Sent: den 3 december 2007 22:36 To: Darcy Watkins Cc: u-boot-users@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
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
-----Original Message----- From: wd@denx.de [mailto:wd@denx.de] Sent: den 4 december 2007 01:24 To: Joakim Tjernlund Cc: 'Darcy Watkins'; u-boot-users@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@denx.de Behind every great man, there is a woman -- urging him on. -- Harry Mudd, "I, Mudd", stardate 4513.3
-----Original Message----- From: u-boot-users-bounces@lists.sourceforge.net [mailto:u-boot-users-bounces@lists.sourceforge.net] On Behalf Of Joakim Tjernlund Sent: den 4 december 2007 02:06 To: wd@denx.de Cc: u-boot-users@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@denx.de [mailto:wd@denx.de] Sent: den 4 december 2007 01:24 To: Joakim Tjernlund Cc: 'Darcy Watkins'; u-boot-users@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
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
-----Original Message----- From: wd@denx.de [mailto:wd@denx.de] Sent: den 7 december 2007 01:58 To: Joakim Tjernlund Cc: u-boot-users@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
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
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
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?
From 2f20cc2bb5eddd1ef06671ebf2080aadd8694e58 Mon Sep 17 00:00:00 2001
From: Joakim Tjernlund Joakim.Tjernlund@transmode.se Date: Sun, 9 Dec 2007 14:06:32 +0100 Subject: [PATCH] Add CFG_OF_TREE knob.
Make it possible to construct a OF tree from board code. See README for details.
Signed-off-by: Joakim Tjernlund Joakim.Tjernlund@transmode.se --- README | 8 ++++++++ common/cmd_bootm.c | 4 ++++ 2 files changed, 12 insertions(+), 0 deletions(-)
diff --git a/README b/README index 3dad5fc..8bd70bf 100644 --- a/README +++ b/README @@ -340,6 +340,14 @@ The following options need to be configured: * Adds the "fdt" command * The bootm command automatically updates the fdt
+ CFG_OF_TREE + * Define this to a function in board code to construct + your own OF tree. The function should return a char * + pointing to your OF tree. + Useful if you have multiple OF trees or want to build your + tree dynamically. You can still overide this tree by + passing a OF tree to bootm. + CONFIG_OF_FLAT_TREE * Deprecated, see CONFIG_OF_LIBFDT * Original ft_build.c-based support diff --git a/common/cmd_bootm.c b/common/cmd_bootm.c index d816349..68dfb59 100644 --- a/common/cmd_bootm.c +++ b/common/cmd_bootm.c @@ -527,7 +527,11 @@ 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) +#ifdef CFG_OF_TREE + char *of_flat_tree = CFG_OF_TREE; +#else char *of_flat_tree = NULL; +#endif ulong of_data = 0; #endif
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
-----Original Message----- From: wd@denx.de [mailto:wd@denx.de] Sent: den 9 december 2007 16:01 To: joakim.tjernlund@transmode.se Cc: Jerry Van Baren; u-boot-users@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:
- 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.
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.
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
In message 010601c83a89$8b22bfa0$02ac10ac@Jocke you wrote:
I hereby NAK this patch for 3 reasons:
- 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?
- 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
-----Original Message----- From: wd@denx.de [mailto:wd@denx.de] Sent: den 9 december 2007 20:28 To: Joakim Tjernlund Cc: 'Jerry Van Baren'; u-boot-users@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:
- 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.
- 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-2006The 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
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
-----Original Message----- From: wd@denx.de [mailto:wd@denx.de] Sent: den 9 december 2007 22:36 To: Joakim Tjernlund Cc: 'Jerry Van Baren'; u-boot-users@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
[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
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:
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
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.
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
-----Original Message----- From: Jerry Van Baren [mailto:gerald.vanbaren@ge.com] Sent: den 13 december 2007 19:49 To: Wolfgang Denk Cc: joakim.tjernlund@transmode.se; u-boot-users@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:
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
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.
- 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
On Fri, 2007-12-14 at 00:03 +0100, Joakim Tjernlund wrote:
-----Original Message----- From: Jerry Van Baren [mailto:gerald.vanbaren@ge.com] Sent: den 13 december 2007 19:49 To: Wolfgang Denk Cc: joakim.tjernlund@transmode.se; u-boot-users@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:
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
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.
- 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
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
-----Original Message----- From: wd@denx.de [mailto:wd@denx.de] Sent: den 16 december 2007 21:15 To: joakim.tjernlund@transmode.se Cc: 'Jerry Van Baren'; u-boot-users@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
On Sun, 2007-12-16 at 22:27 +0100, Joakim Tjernlund wrote:
-----Original Message----- From: wd@denx.de [mailto:wd@denx.de] Sent: den 16 december 2007 21:15 To: joakim.tjernlund@transmode.se Cc: 'Jerry Van Baren'; u-boot-users@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
participants (4)
-
Darcy Watkins -
Jerry Van Baren -
Joakim Tjernlund -
Wolfgang Denk