Bryan O'Donoghue wrote:
Greetings.
This patch fixes up a compile error that crept with with debug switched on. Introduces CONFIG_OF_CHOSEN_UPDATE - which is useful if you have a /chosen entry in the dts - which doesn't contain a bootargs entry - in which case you'd want u-boot's version of this.
Signed-off-by: Bryan O'Donoghue bodonoghue@codehermit.ie
diff --git a/README b/README index 26f93c2..bc7a6a4 100644 --- a/README +++ b/README @@ -375,6 +375,11 @@ The following options need to be configured: This define fills in the correct boot cpu in the boot param header, the default value is zero if undefined.
CONFIG_OF_CHOSEN_UPDATEThis define adds or updates a bootargs field to the /chosenentry.
Hi Bryan,
I don't think CONFIG_OF_CHOSEN_UPDATE is needed. If it *is* needed, I don't think it *should be* needed and we need to discuss scenarios.
Initially, we did not touch the /chosen node *at all* if it already existed. That was where the "force" flag came from - the criteria wasn't fine grained enough. That behavior was changed in the 1.3.1 time frame to be fine grained: if /chosen exists, but /chosen/<prop> doesn't, that property is created and set to the required value.
If /chosen/<prop> exists, it is overwritten only if the "force" flag is set. What you have added with CONFIG_OF_CHOSEN_UPDATE it a compile time "force" flag setting.
Maybe that is desirable, but I'm going to challenge you to justify it. :-)
The problem / limitation with adding CONFIG_OF_CHOSEN_UPDATE is that it is a compile time option. If we compile "force" to be TRUE, why not just compile it in as a '1' bit (below)? I guess my problem is that I don't see the usefulness of nearly hardcoding it to be '1' or '0' (e.g. via a #define) vs. absolutely hardcoding it to be '1' or '0'.
If we *do* need to support a force/noforce option (and I have not conceded that point yet ;-), I think it should be an env variable, something the user can control at run time.
[snip]
diff --git a/common/cmd_bootm.c b/common/cmd_bootm.c index 9546729..c729f52 100644 --- a/common/cmd_bootm.c +++ b/common/cmd_bootm.c @@ -975,7 +975,11 @@ do_bootm_linux (cmd_tbl_t *cmdtp, int flag, * if the user wants it (the logic is in the subroutines). */ if (of_flat_tree) {
if (fdt_chosen(of_flat_tree, initrd_start, initrd_end, 0) < 0) {+#ifdef CONFIG_OF_CHOSEN_UPDATE
- if (fdt_chosen(of_flat_tree, initrd_start, initrd_end, 1) < 0) {
+#else
- if (fdt_chosen(of_flat_tree, initrd_start, initrd_end, 0) < 0) {
+#endif
Compile time selection is pretty inflexible. Note also that this is a pretty big hammer - *everything* in the fdt_chosen() routine (potentially) gets forced.
Do we really need the "force" flag any more?
[snip]
Best regards, gvb