Re: [PATCH 05/20] bootwrapper: flatdevtree fixes

On Wed, Aug 22, 2007 at 12:24:56PM -0500, Scott Wood wrote:

On Wed, Aug 22, 2007 at 11:09:07AM +1000, David Gibson wrote:

quoted

On Tue, Aug 21, 2007 at 11:09:58AM -0500, Scott Wood wrote:

quoted

The point of #2 was as part of the fix to #1 -- otherwise, the same 
check for NULL would have to be moved into ft_create_node to 
conditionally call ft_find_device or ft_find_device_rel.

Um... oh, ok, I hadn't spotted that (1) made ft_create() use
find_device_rel().  That sounds doubly wrong: you have the internal
offset pointer, you should be able to create a phandle using the
phandle allocation stuff, rather than having to refind the node you've
just created from the parent.

Yeah, that'd make more sense.

quoted

quoted

quoted

 (3) I really dislike;  I just don't see the point.

It's needed by dt_get_path().

No, it isn't.  dt_get_path() needs *some* way of getting the name of a
node, but it could be a separate function, which I think would be
preferable rather than folding it into getprop - you don't need to
search for the name, so a getname() function would have quite a
different structure to getprop().

I'd rather not add a new entry in ops just for that; it's more of an
attribute of the dtb format that name is handled specially.  IIUC, on
real OF you'd use the same code for both.

Actually, no - sorry, that's the other problem with this, which I
forgot to mention.  On real OF, the "name" property contains the
node's name *without the unit address*; that is, only the portion
before the '@'.  So your getprop change does not match real OF
behaviour - and real OF behaviour will not do what you want for
dt_get_path().

Actually, in any case, I don't think we want to implement get_path()
this way for real OF.  Better to have get_path() itself as a callback:
on real OF I believe we can directly ask for the full path to a given
phandle, the get name based implementation can then be made specific
to the flat device tree.

Or actually, I think we might be able to come up with a get_path()
implementation for flat tree that's less hideous than repeatedly
calling get_parent() which is an ugly, ugly operation on the flat tree
(and will get worse with libfdt).

Plus, something might come along that needs to dynamically look for
either name or something else.  It's more flexible this way.

Hrm... "something might come along" just seems contrived to me.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson