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

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

On Tue, Aug 21, 2007 at 11:09:58AM -0500, Scott Wood wrote:
> David Gibson wrote:
> > On Mon, Aug 20, 2007 at 12:39:49PM -0500, Scott Wood wrote:
> > 
> >>1. ft_create_node was returning the internal pointer rather than a phandle.
> >>2. ft_find_device_rel was treating a "top" phandle of NULL as an error,
> >>rather than as the root of the tree.
> >>3. Return the node's name when getprop() is called with the "name"
> >>property.
> > 
> > 
> > Hrm.  I'm not convinced.  (1) certainly needs fixing.  (2) is kind of
> > unclear - there is an ft_find_device() after all for doing root-based
> > searches.
> 
> 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.

> The non-relative function should probably be removed, though.

Well, yes, I wouldn't have much problem with just having a relative
version.

Come to that, I don't actually care all that much what happens to
flatdevtree.c, seeing as I intend to replace it with libfdt, just as
soon as I can get enough other things off my plate.

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

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