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 <[EMAIL PROTECTED]>
> ---
>
> 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_UPDATE
> +
> + This define adds or updates a bootargs field to the /chosen
> + entry.
> +
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
-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/
_______________________________________________
U-Boot-Users mailing list
U-Boot-Users@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/u-boot-users
