Bryan O'Donoghue wrote:
On Tue, 12 Feb 2008 08:27:14 -0500 Jerry Van Baren gerald.vanbaren@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....
I think you have an older version of fdt_support.c. Check the latest version - the above check is no longer in there. That was a change that went in towards the end of the last patch window.
The patch that changed this: http://www.denx.de/cgi-bin/gitweb.cgi?p=u-boot.git;a=commitdiff;h=b60af3d4c1680487ee37e11aa1b3db6dec04d8f0
It is possible we have some other wires crossed as well. There was a fair amount of LIBFDT support refactoring going on in the last patch window, so we may have missed some deletions related to refactoring that should have been made.
Best regards, gvb