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

[David Gibson <david@gibson.dropbear.id.au>]

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:
> > On Tue, Aug 21, 2007 at 11:09:58AM -0500, Scott Wood wrote:
> > > 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.
> 
> > > >  (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