[U-Boot-Users] [PATCH 2/3] fdt: Fixup compile error and add a new OF manipulation option

[Bryan O'Donoghue]

On Tue, 12 Feb 2008 08:27:14 -0500
Jerry Van Baren <gerald.vanbaren at ge.com> wrote:

> Bryan O'Donoghue wrote:

> 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.

Hey Jerry.

I'd have to say I fundamentally agree with the principal you're putting
forward here - i.e. that using a compile time define is a bit inflexible and
doubly wrong in that it's manipulating a parameter which ideally shouldn't
exist anyway.

The behavior I'm looking for is simply where /chosen/bootargs - doesn't exist
in a dts - that U-boot will fill in it's environment bootargs entry in the
tree - i.e. it creates /chosen/<prop> if <prop> doesn't exist.

> 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.

That creation won't happen though due to the code fragment below.
 
> 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.  :-)

Nope it's admitedly a blunt instrument to accomplish stuffing /chosen/<prop> if
<prop> doesn't exist.

What the define works around is this in fdt_support.c

/*
 * If we have a "chosen" node already the "force the writing"
 * is not set, our job is done.
 */
if ((nodeoffset >= 0) && !force)
	return 0;

The default sans CONFIG_OF_CHOSEN_UPDATE in do_bootm_linux is to pass force = 0
- which means if /chosen is present in the dts - then no manipulation of
the /chosen entry will be performed - as I read it anyway.

I was thinking that some sort of define would be the least invasive way of
changing that.... with a view to the least amount of change being the most
likely to be acceptable to other developers - not necessarily the best way to
do the thing....