RFA & Update: Using libfdt in u-boot for fdt command

RFA & Update: Using libfdt in u-boot for fdt command

[Jerry Van Baren]

Hi all,

This is a Request for Advice.

First off, for those on both the u-boot and linuxppc-dev lists, sorry 
for cross posting.  :-)

Git repo pointers...
libfdt hacks:
<http://www.cideas.us/cgi-bin/gitweb.cgi?p=linux/libfdt.git;a=summary>
u-boot hacks:
<http://www.cideas.us/cgi-bin/gitweb.cgi?p=u-boot/u-boot-mpc83xx;a=summary>

I have a proof-of-concept (actually, somewhat more than PoC) fdt command 
built using libfdt.  Now I'm looking for advice on the best way to turn 
the PoC into Real Code[tm].

Current commands:
-----------------
fdt address        - set the base address of the blob
fdt get <property> - get a property
fdt print <node>   - print the subtree starting at <node>[1]

Planned commands:
-----------------
fdt set <property> <value> - set a property value (do we want an
                        "=" as in <property> = <value>?)
fdt ?synthesize?   - create the "chosen" branch and optionally the
                        u-boot variables branch.  The OF code/calls
                        currently in bootm would be moved to this
                        command.  I'm open to suggestions for a good
                        subcommand name.

Other commands:
---------------
fdt node <name>    - create a node that can then be populated with 
properties (have not thought about this in any detail yet).

[1] If <node> is actually a <property> (which is a usage error, but I 
expect will happen all the time), "fdt print" actually does a "fdt get 
<property>"  This makes "fdt get" redundant and likely will make it go away.

Philosophy question primarily for David Gibson and Wolfgang Denk:  What 
is the best way to integrate libfdt with u-boot?  Currently it is in its 
own git repository.  Options?
1) Do we want to capture the source in the u-boot git repository?  If 
so, it becomes a snapshot and will require cross pushing/pulling between 
the libfdt repo and the u-boot repo or they will drift apart.  However, 
it makes problems #1 and #2 (below) simpler to solve but causes drift.
2) Not capturing libfdt in the u-boot repo makes it more difficult for 
integrating it with and maintaining it in u-boot, I'm not sure how to 
actually do it in a useful/usable manner.

There are three problem areas with libfdt:
1) The official Makefile is stand-alone which doesn't work well with 
u-boot.  I took the expedient route for the PoC of simply replacing it 
with a u-boot style Makefile from a different lib* subdirectory.  There 
should be a better way.

2) The official libfdt uses two header files that are not in u-boot.  I 
"fixed" this by substituting u-boot headers with equivalent functionality.
* I need to address this and see what the best compromise for header 
files is...
   a) If the u-boot headers are acceptable for the stand-alone version
        of libfdt, that would be the simplest.
   b) It may be more effective to add the necessary linux headers to
        u-boot.
   c) We could use #ifdefs to conditionally include the right files. (but
        does u-boot have a distinctive configuration def?  Probably...)

3) I added a "fdt_next_tag()" function to fdt_ro.c to allow me to step 
through the blob's tags:
   uint32_t fdt_next_tag(const void *fdt, int offset,
                         int *nextoffset, char **namep);

This is similar to "_fdt_next_tag()" (a "private" function, note the 
leading underscore) in fdt.c, but with a related but different purpose - 
  the "_fdt_next_tag()" steps through _node_ tags (skipping property 
tags) where I need to step through all tags sequentially.

Usability trivia for David: libfdt distinguishes between nodes and 
properties, which makes sense since that is how the fdt is structured. 
 From a usability standpoint, however, it is annoying to have to 
separate the property name from the node, find the node, then find the 
property.  I will probably create Yet Another Function:
   int fdt_split(char *path, char **property);
Call it with a path string and the function will separate it into the 
node portion and the property name.  If the path is invalid, it will 
return an error.  If the path is a node, it will set **property to NULL 
and return the node's offset.  If the path is a property, it will return 
the owning node's offset and set the **property pointer to point to the 
start of the property portion of the path (i.e. the next character after 
the last '/').

If you got this far, thank you for your attention.  :-)

Best regards,
gvb

Re: RFA & Update: Using libfdt in u-boot for fdt command

[Mark A. Greer]

On Thu, Mar 01, 2007 at 09:01:24AM -0500, Jerry Van Baren wrote:
> Hi all,
> 
> This is a Request for Advice.

Hi Jerry.

One minor thing.  I'd just want to remind you that we shouldn't stray
too far from the OF interface.  The bootwrapper code sits on top of
either the FDT access interface or the true OF DT access interface.  
The closer we keep the two, the cleaner & easier the we can keep
the bootwrapper code.

Mark

Re: [U-Boot-Users] RFA & Update: Using libfdt in u-boot for fdt command

[Jerry Van Baren]

Mark A. Greer wrote:
> On Thu, Mar 01, 2007 at 09:01:24AM -0500, Jerry Van Baren wrote:
>> Hi all,
>>
>> This is a Request for Advice.
> 
> Hi Jerry.
> 
> One minor thing.  I'd just want to remind you that we shouldn't stray
> too far from the OF interface.  The bootwrapper code sits on top of
> either the FDT access interface or the true OF DT access interface.  
> The closer we keep the two, the cleaner & easier the we can keep
> the bootwrapper code.
> 
> Mark

Hi Mark,

Understood, but that isn't really my battle.  David Gibson created the 
libfdt interface so any linux/bootwrapper changes are really his battle. 
  I'm working in u-boot land so linux/bootwrapper aren't in my problem 
domain - using libfdt in u-boot doesn't affect the bootwrapper code. 
Having said that, I'm hoping for shared code synergy, which would 
require the linux kernel to adopt libfdt as a replacement for 
flatdevtree.[ch].  David has also talked about using libfdt in the dtc 
suite, so there is potentially a three way synergy.

The flattened device tree which is passed from u-boot to linux is 
unchanged*, so the interface is the same at that level (to state the 
obvious).

David's libfdt is a potential replacement for flatdevtree.c.  I have 
pursued using libfdt rather than flatdevtree (in u-boot) because the 
interface is much cleaner (IMHO, and, I'm sure, IDHO).  Of course "much 
cleaner" is the fancy way of saying "not compatible." :-/  On the other 
hand, I looked at arch/powerpc/boot/of.c, ops.h, and flatdevtree_misc.c. 
  It looks like it would be relatively easy to redo flatdevtree_misc.c 
to glue to libfdt instead of flatdevtree.c (he says blithely).

* ...other than a backwards compatible version 16 -> 17 upgrade.

Best regards,
gvb

Re: RFA & Update: Using libfdt in u-boot for fdt command

[David Gibson]

On Thu, Mar 01, 2007 at 09:01:24AM -0500, Jerry Van Baren wrote:
> Hi all,
> 
> This is a Request for Advice.
> 
> First off, for those on both the u-boot and linuxppc-dev lists, sorry 
> for cross posting.  :-)
> 
> Git repo pointers...
> libfdt hacks:
> <http://www.cideas.us/cgi-bin/gitweb.cgi?p=linux/libfdt.git;a=summary>
> u-boot hacks:
> <http://www.cideas.us/cgi-bin/gitweb.cgi?p=u-boot/u-boot-mpc83xx;a=summary>
> 
> I have a proof-of-concept (actually, somewhat more than PoC) fdt command 
> built using libfdt.  Now I'm looking for advice on the best way to turn 
> the PoC into Real Code[tm].
> 
> Current commands:
> -----------------
> fdt address        - set the base address of the blob
> fdt get <property> - get a property
> fdt print <node>   - print the subtree starting at <node>[1]
> 
> Planned commands:
> -----------------
> fdt set <property> <value> - set a property value (do we want an
>                         "=" as in <property> = <value>?)
> fdt ?synthesize?   - create the "chosen" branch and optionally the
>                         u-boot variables branch.  The OF code/calls
>                         currently in bootm would be moved to this
>                         command.  I'm open to suggestions for a good
>                         subcommand name.
> 
> Other commands:
> ---------------
> fdt node <name>    - create a node that can then be populated with 
> properties (have not thought about this in any detail yet).
> 
> [1] If <node> is actually a <property> (which is a usage error, but I 
> expect will happen all the time), "fdt print" actually does a "fdt get 
> <property>"  This makes "fdt get" redundant and likely will make it go away.
> 
> Philosophy question primarily for David Gibson and Wolfgang Denk:  What 
> is the best way to integrate libfdt with u-boot?  Currently it is in its 
> own git repository.  Options?
> 1) Do we want to capture the source in the u-boot git repository?  If 
> so, it becomes a snapshot and will require cross pushing/pulling between 
> the libfdt repo and the u-boot repo or they will drift apart.  However, 
> it makes problems #1 and #2 (below) simpler to solve but causes drift.

> 2) Not capturing libfdt in the u-boot repo makes it more difficult for 
> integrating it with and maintaining it in u-boot, I'm not sure how to 
> actually do it in a useful/usable manner.

I think ultimately, you'll have to pull libfdt into the u-boot
sources.  libfdt is supposed to run in many possible environments, and
there's no realistic way it can be compiled independently of thost
environments.

That said, any changes to the guts of libfdt that you need should go
upstream (in the end; right now, there are bureaucratic problems with
that, more below).  That should reduce things to a one way pull, with
some trivial tweaks to integrate with the u-boot environment.

> There are three problem areas with libfdt:
> 1) The official Makefile is stand-alone which doesn't work well with 
> u-boot.  I took the expedient route for the PoC of simply replacing it 
> with a u-boot style Makefile from a different lib* subdirectory.  There 
> should be a better way.

I think it might well be necessary to replace the Makefile for
building libfdt into other packages.  It would be nice to avoid that
if possible, but a sensible method is not obvious to me.

> 2) The official libfdt uses two header files that are not in u-boot.  I 
> "fixed" this by substituting u-boot headers with equivalent functionality.
> * I need to address this and see what the best compromise for header 
> files is...
>    a) If the u-boot headers are acceptable for the stand-alone version
>         of libfdt, that would be the simplest.
>    b) It may be more effective to add the necessary linux headers to
>         u-boot.
>    c) We could use #ifdefs to conditionally include the right files. (but
>         does u-boot have a distinctive configuration def?  Probably...)

Ok, the way this is supposed to work is that the environment into
which you're building should provide a replacement version of
libfdt_env.h.  The supplied version of libfdt_env.h is just for
userland builds.  u-boot's version will obviously use u-boot headers
instead of standard library headers.

I should provide a comprehensive list of what libfdt_env.h needs to
provide, but I haven't gotten around to it.  From memory it's
basically just the fixed-with integer types, a smallish subset of the
str*() and mem*() functions, and the endian conversion functions.

I've really tried to keep libfdt's environment dependencies down, so I
suggest you just start with an empty libfdt_env.h and add stuff based
on the error messages until the compiler stops whinging about
undefined things.  It shouldn't take long.

> 3) I added a "fdt_next_tag()" function to fdt_ro.c to allow me to step 
> through the blob's tags:
>    uint32_t fdt_next_tag(const void *fdt, int offset,
>                          int *nextoffset, char **namep);
> 
> This is similar to "_fdt_next_tag()" (a "private" function, note the 
> leading underscore) in fdt.c, but with a related but different purpose - 
>   the "_fdt_next_tag()" steps through _node_ tags (skipping property 
> tags) where I need to step through all tags sequentially.

Um... no.  _fdt_next_tag() steps through all tags (how else could it
be used internally to find properties...?).  If you really need this,
we can change the function to be exported, which I've considered
before.  However, what are you using this function for?  I had some
node and property traversal functions on the drawing board.

> Usability trivia for David: libfdt distinguishes between nodes and 
> properties, which makes sense since that is how the fdt is structured. 
>  From a usability standpoint, however, it is annoying to have to 
> separate the property name from the node, find the node, then find the 
> property.  I will probably create Yet Another Function:
>    int fdt_split(char *path, char **property);
> Call it with a path string and the function will separate it into the 
> node portion and the property name.  If the path is invalid, it will 
> return an error.  If the path is a node, it will set **property to NULL 
> and return the node's offset.  If the path is a property, it will return 
> the owning node's offset and set the **property pointer to point to the 
> start of the property portion of the path (i.e. the next character after 
> the last '/').

I don't like combining property and node name into a single path,
because technically the property names occupy a different namespace
from subnode names.  Insane though it is, there exist some Apple
device trees where a node has both a property and a subnode of the
same name.

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

Re: RFA & Update: Using libfdt in u-boot for fdt command

[Jerry Van Baren]

Hi David,

David Gibson wrote:
> On Thu, Mar 01, 2007 at 09:01:24AM -0500, Jerry Van Baren wrote:
>> Hi all,
>>
>> This is a Request for Advice.
>>
>> First off, for those on both the u-boot and linuxppc-dev lists, sorry 
>> for cross posting.  :-)
>>
>> Git repo pointers...
>> libfdt hacks:
>> <http://www.cideas.us/cgi-bin/gitweb.cgi?p=linux/libfdt.git;a=summary>
>> u-boot hacks:
>> <http://www.cideas.us/cgi-bin/gitweb.cgi?p=u-boot/u-boot-mpc83xx;a=summary>

[snip]

>> 2) Not capturing libfdt in the u-boot repo makes it more difficult for 
>> integrating it with and maintaining it in u-boot, I'm not sure how to 
>> actually do it in a useful/usable manner.
> 
> I think ultimately, you'll have to pull libfdt into the u-boot
> sources.  libfdt is supposed to run in many possible environments, and
> there's no realistic way it can be compiled independently of thost
> environments.
> 
> That said, any changes to the guts of libfdt that you need should go
> upstream (in the end; right now, there are bureaucratic problems with
> that, more below).  That should reduce things to a one way pull, with
> some trivial tweaks to integrate with the u-boot environment.

OK, I figured as much, but was hoping for a miracle.

>> There are three problem areas with libfdt:
>> 1) The official Makefile is stand-alone which doesn't work well with 
>> u-boot.  I took the expedient route for the PoC of simply replacing it 
>> with a u-boot style Makefile from a different lib* subdirectory.  There 
>> should be a better way.
> 
> I think it might well be necessary to replace the Makefile for
> building libfdt into other packages.  It would be nice to avoid that
> if possible, but a sensible method is not obvious to me.

OK, no surprise.

>> 2) The official libfdt uses two header files that are not in u-boot.  I 
>> "fixed" this by substituting u-boot headers with equivalent functionality.
>> * I need to address this and see what the best compromise for header 
>> files is...
>>    a) If the u-boot headers are acceptable for the stand-alone version
>>         of libfdt, that would be the simplest.
>>    b) It may be more effective to add the necessary linux headers to
>>         u-boot.
>>    c) We could use #ifdefs to conditionally include the right files. (but
>>         does u-boot have a distinctive configuration def?  Probably...)
> 
> Ok, the way this is supposed to work is that the environment into
> which you're building should provide a replacement version of
> libfdt_env.h.  The supplied version of libfdt_env.h is just for
> userland builds.  u-boot's version will obviously use u-boot headers
> instead of standard library headers.
> 
> I should provide a comprehensive list of what libfdt_env.h needs to
> provide, but I haven't gotten around to it.  From memory it's
> basically just the fixed-with integer types, a smallish subset of the
> str*() and mem*() functions, and the endian conversion functions.
> 
> I've really tried to keep libfdt's environment dependencies down, so I
> suggest you just start with an empty libfdt_env.h and add stuff based
> on the error messages until the compiler stops whinging about
> undefined things.  It shouldn't take long.

Yes, all the functionality is in both linux headers and u-boot headers 
that it stole^Winherited from linux.  The problem is, the set is 
disjoint.  If libfdt is willing to change its headers to use 
linux/types.h, asm/byteorder.h, and linux/string.h, the problem would go 
away:

fdt.h:
-#include <stdint.h>
+#include <linux/types.h>

libfdt_env.h:
-#include <stdint.h>
-#include <string.h>
-#include <endian.h>
-#include <byteswap.h>
+#include <linux/types.h>
+#include <asm/byteorder.h>
+#include <linux/string.h>

Note that asm/byteorder.h provides __be32_to_cpu(x) and friends, which 
can be used directly rather than having to synthesize swap/noswap (i.e. 
bswap_32(x)) based on #if __BYTE_ORDER == __BIG_ENDIAN.  They can be 
used directly, or fdt32_to_cpu and friends can be defined in terms 
of__be32_to_cpu(x) and friends:
+#define fdt32_to_cpu(x)                __be32_to_cpu(x)
+#define cpu_to_fdt32(x)                __cpu_to_be32(x)
+#define fdt64_to_cpu(x)                __be64_to_cpu(x)
+#define cpu_to_fdt64(x)                __cpu_to_be64(x)

When I did the above change and tried to compile it natively under 
linux, there were problems with unresolved types.  I did not pursue the 
resolution of the problem since I am currently concentrating on u-boot 
usage.

>> 3) I added a "fdt_next_tag()" function to fdt_ro.c to allow me to step 
>> through the blob's tags:
>>    uint32_t fdt_next_tag(const void *fdt, int offset,
>>                          int *nextoffset, char **namep);
>>
>> This is similar to "_fdt_next_tag()" (a "private" function, note the 
>> leading underscore) in fdt.c, but with a related but different purpose - 
>>   the "_fdt_next_tag()" steps through _node_ tags (skipping property 
>> tags) where I need to step through all tags sequentially.
> 
> Um... no.  _fdt_next_tag() steps through all tags (how else could it
> be used internally to find properties...?).  If you really need this,
> we can change the function to be exported, which I've considered
> before.  However, what are you using this function for?  I had some
> node and property traversal functions on the drawing board.

Yes, I copied and augmented _fdt_next_tag():

uint32_t fdt_next_tag(const void *fdt, int offset, int *nextoffset, char 
**namep)

to give me a pointer to the node name for node tags and property name 
for property tags.  Now that I have it working, it would be trivial to 
change the calls to _fdt_next_tag() to instead call fdt_next_tag() 
passing NULL for the new fourth parameter **namep. ;-)

The reason I need it, I'm printing an unknown tree by stepping through 
the tree discovering the node and property names.  I need to have 
fdt_next_tag() return the *name* of the node/property as well as the tag 
so that I can print and indent for nodes or look up the property value 
and print the name=value combination.

>> Usability trivia for David: libfdt distinguishes between nodes and 
>> properties, which makes sense since that is how the fdt is structured. 
>>  From a usability standpoint, however, it is annoying to have to 
>> separate the property name from the node, find the node, then find the 
>> property.  I will probably create Yet Another Function:
>>    int fdt_split(char *path, char **property);
>> Call it with a path string and the function will separate it into the 
>> node portion and the property name.  If the path is invalid, it will 
>> return an error.  If the path is a node, it will set **property to NULL 
>> and return the node's offset.  If the path is a property, it will return 
>> the owning node's offset and set the **property pointer to point to the 
>> start of the property portion of the path (i.e. the next character after 
>> the last '/').
> 
> I don't like combining property and node name into a single path,
> because technically the property names occupy a different namespace
> from subnode names.  Insane though it is, there exist some Apple
> device trees where a node has both a property and a subnode of the
> same name.

Oh gaak!  What I hear you saying... if you have node a with subnode b 
and property b, subnode b has a property c:
/a     => node
/a/b   => node
/a/b   => property (inside node a)
/a/b/c => property (inside node b)

Where I am right now is I created a new function fdt_fullpath_offset:

int fdt_fullpath_offset(const void *fdt, const char *path, char **prop);

which will return the _node_ /a/b in the gaak illustration above.  It 
looks up nodes until it either runs out of path to look up or there is 
an error.  On a lookup error, it tries again with the last part of the 
path used as a property name.  As a result, if you pass in /a it will 
return the node "a", if you pass in /a/b it will return the _node_ "b". 
  This is unchanged behavior compared to fdt_path_offset().  (Getting 
property "b" is unchanged: you would have to look up node /a with either 
fdt_fullpath_offset(... "/a" ...) or fdt_path_offset(... "/a" ...) and 
then use that offset with fdt_property() to get the property "b".)

I've changed the original fdt_path_offset() to simply call 
fdt_fullpath_offset() passing in NULL for **prop and everything (should) 
work the same as before ("should" because I haven't debugged it yet).

For my u-boot commands, I have "fdt get <prop>" and "fdt print <node>" 
and was going to consolidate them into one, but the gaak illustration 
says I should _not_ consolidate them.  (Trivia: my "fdt print" of "/" 
and "/a" will (should anyway ;-) print the gaak tree properly.)

What are the odds that someone will pull an Apple on hardware supported 
by u-boot?  Hmmm... extra code to handle stupidity.

Best regards,
gvb

Re: RFA & Update: Using libfdt in u-boot for fdt command

[David Gibson]

On Thu, Mar 01, 2007 at 11:08:38PM -0500, Jerry Van Baren wrote:
[snip] 
> >> 2) The official libfdt uses two header files that are not in u-boot.  I 
> >> "fixed" this by substituting u-boot headers with equivalent functionality.
> >> * I need to address this and see what the best compromise for header 
> >> files is...
> >>    a) If the u-boot headers are acceptable for the stand-alone version
> >>         of libfdt, that would be the simplest.
> >>    b) It may be more effective to add the necessary linux headers to
> >>         u-boot.
> >>    c) We could use #ifdefs to conditionally include the right files. (but
> >>         does u-boot have a distinctive configuration def?  Probably...)
> > 
> > Ok, the way this is supposed to work is that the environment into
> > which you're building should provide a replacement version of
> > libfdt_env.h.  The supplied version of libfdt_env.h is just for
> > userland builds.  u-boot's version will obviously use u-boot headers
> > instead of standard library headers.
> > 
> > I should provide a comprehensive list of what libfdt_env.h needs to
> > provide, but I haven't gotten around to it.  From memory it's
> > basically just the fixed-with integer types, a smallish subset of the
> > str*() and mem*() functions, and the endian conversion functions.
> > 
> > I've really tried to keep libfdt's environment dependencies down, so I
> > suggest you just start with an empty libfdt_env.h and add stuff based
> > on the error messages until the compiler stops whinging about
> > undefined things.  It shouldn't take long.
> 
> Yes, all the functionality is in both linux headers and u-boot headers 
> that it stole^Winherited from linux.  The problem is, the set is 
> disjoint.  If libfdt is willing to change its headers to use 
> linux/types.h, asm/byteorder.h, and linux/string.h, the problem would go 
> away:
> 
> fdt.h:
> -#include <stdint.h>
> +#include <linux/types.h>
> 
> libfdt_env.h:
> -#include <stdint.h>
> -#include <string.h>
> -#include <endian.h>
> -#include <byteswap.h>
> +#include <linux/types.h>
> +#include <asm/byteorder.h>
> +#include <linux/string.h>

Well, as I say, libfdt_env.h is supposed to be replaced in any case.
That leaves only stdint.h as a problem (it's used in the actual
declarations of the flat tree structures).  I'm not entirely sure how
to address that one.  Possibly just omit it from fdt.h and require the
user to include something defining the fixed-width integer types.

> Note that asm/byteorder.h provides __be32_to_cpu(x) and friends, which 
> can be used directly rather than having to synthesize swap/noswap (i.e. 
> bswap_32(x)) based on #if __BYTE_ORDER == __BIG_ENDIAN.  They can be 
> used directly, or fdt32_to_cpu and friends can be defined in terms 
> of__be32_to_cpu(x) and friends:
> +#define fdt32_to_cpu(x)                __be32_to_cpu(x)
> +#define cpu_to_fdt32(x)                __cpu_to_be32(x)
> +#define fdt64_to_cpu(x)                __be64_to_cpu(x)
> +#define cpu_to_fdt64(x)                __cpu_to_be64(x)

Yes, that's what I'd expect to be done in the context of the kernel,
or something similar (which I gather u-boot is).

> When I did the above change and tried to compile it natively under 
> linux, there were problems with unresolved types.  I did not pursue the 
> resolution of the problem since I am currently concentrating on u-boot 
> usage.
> 
> >> 3) I added a "fdt_next_tag()" function to fdt_ro.c to allow me to step 
> >> through the blob's tags:
> >>    uint32_t fdt_next_tag(const void *fdt, int offset,
> >>                          int *nextoffset, char **namep);
> >>
> >> This is similar to "_fdt_next_tag()" (a "private" function, note the 
> >> leading underscore) in fdt.c, but with a related but different purpose - 
> >>   the "_fdt_next_tag()" steps through _node_ tags (skipping property 
> >> tags) where I need to step through all tags sequentially.
> > 
> > Um... no.  _fdt_next_tag() steps through all tags (how else could it
> > be used internally to find properties...?).  If you really need this,
> > we can change the function to be exported, which I've considered
> > before.  However, what are you using this function for?  I had some
> > node and property traversal functions on the drawing board.
> 
> Yes, I copied and augmented _fdt_next_tag():
> 
> uint32_t fdt_next_tag(const void *fdt, int offset, int *nextoffset, char 
> **namep)
> 
> to give me a pointer to the node name for node tags and property name 
> for property tags.  Now that I have it working, it would be trivial to 
> change the calls to _fdt_next_tag() to instead call fdt_next_tag() 
> passing NULL for the new fourth parameter **namep. ;-)
> 
> The reason I need it, I'm printing an unknown tree by stepping through 
> the tree discovering the node and property names.  I need to have 
> fdt_next_tag() return the *name* of the node/property as well as the tag 
> so that I can print and indent for nodes or look up the property value 
> and print the name=value combination.

Hrm.  And it returns NULL for tags without a name?

That might be a useful extension for the next_tag function.  The one
thing I'm concerned about is who's responsible for verifying the name
pointer.  I'm trying to keep libfdt robust enough that evern if
presented with a badly corrupt blob it will fail relatively
gracefully.  Ideally, no matter what it's presented with, it will
always return at worst FDT_ERR_BADSTRUCTURE rather than crashing and
will under no circumstances access memory outside the given blob
size.

> >> Usability trivia for David: libfdt distinguishes between nodes and 
> >> properties, which makes sense since that is how the fdt is structured. 
> >>  From a usability standpoint, however, it is annoying to have to 
> >> separate the property name from the node, find the node, then find the 
> >> property.  I will probably create Yet Another Function:
> >>    int fdt_split(char *path, char **property);
> >> Call it with a path string and the function will separate it into the 
> >> node portion and the property name.  If the path is invalid, it will 
> >> return an error.  If the path is a node, it will set **property to NULL 
> >> and return the node's offset.  If the path is a property, it will return 
> >> the owning node's offset and set the **property pointer to point to the 
> >> start of the property portion of the path (i.e. the next character after 
> >> the last '/').
> > 
> > I don't like combining property and node name into a single path,
> > because technically the property names occupy a different namespace
> > from subnode names.  Insane though it is, there exist some Apple
> > device trees where a node has both a property and a subnode of the
> > same name.
> 
> Oh gaak!  What I hear you saying... if you have node a with subnode b 
> and property b, subnode b has a property c:
> /a     => node
> /a/b   => node
> /a/b   => property (inside node a)
> /a/b/c => property (inside node b)

Well, yes.  Except that in OF and derived terminology, properties are
*never* referred to by path in this way.  It's always:
	"property 'fred' of node /foo/bar/baz"

> Where I am right now is I created a new function fdt_fullpath_offset:
> 
> int fdt_fullpath_offset(const void *fdt, const char *path, char **prop);
> 
> which will return the _node_ /a/b in the gaak illustration above.  It 
> looks up nodes until it either runs out of path to look up or there is 
> an error.  On a lookup error, it tries again with the last part of the 
> path used as a property name.  As a result, if you pass in /a it will 
> return the node "a", if you pass in /a/b it will return the _node_ "b". 
>   This is unchanged behavior compared to fdt_path_offset().  (Getting 
> property "b" is unchanged: you would have to look up node /a with either 
> fdt_fullpath_offset(... "/a" ...) or fdt_path_offset(... "/a" ...) and 
> then use that offset with fdt_property() to get the property "b".)

I really don't like this idea much.  I don't think it's sufficiently
useful to justify the increased implementation complexity and semantic
confusion.

> I've changed the original fdt_path_offset() to simply call 
> fdt_fullpath_offset() passing in NULL for **prop and everything (should) 
> work the same as before ("should" because I haven't debugged it yet).
> 
> For my u-boot commands, I have "fdt get <prop>" and "fdt print <node>" 
> and was going to consolidate them into one, but the gaak illustration 
> says I should _not_ consolidate them.  (Trivia: my "fdt print" of "/" 
> and "/a" will (should anyway ;-) print the gaak tree properly.)
> 
> What are the odds that someone will pull an Apple on hardware supported 
> by u-boot?  Hmmm... extra code to handle stupidity.

I don't know.  But I do know the chances are pretty reasonable of
libfdt being used in the kexec-tools on Apple machines, so regardless
of what u-boot does, the libfdt core should not promote this
confusion.

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

Re: RFA & Update: Using libfdt in u-boot for fdt command

[Jerry Van Baren]

David Gibson wrote:
> On Thu, Mar 01, 2007 at 11:08:38PM -0500, Jerry Van Baren wrote:

[snip]

>> to give me a pointer to the node name for node tags and property name 
>> for property tags.  Now that I have it working, it would be trivial to 
>> change the calls to _fdt_next_tag() to instead call fdt_next_tag() 
>> passing NULL for the new fourth parameter **namep. ;-)
>>
>> The reason I need it, I'm printing an unknown tree by stepping through 
>> the tree discovering the node and property names.  I need to have 
>> fdt_next_tag() return the *name* of the node/property as well as the tag 
>> so that I can print and indent for nodes or look up the property value 
>> and print the name=value combination.
> 
> Hrm.  And it returns NULL for tags without a name?

I was unable to generate a tag without a name using dtc (other than the 
root node).  It should/would return null, which would be a problem. :-/

> That might be a useful extension for the next_tag function.  The one
> thing I'm concerned about is who's responsible for verifying the name
> pointer.  I'm trying to keep libfdt robust enough that evern if
> presented with a badly corrupt blob it will fail relatively
> gracefully.  Ideally, no matter what it's presented with, it will
> always return at worst FDT_ERR_BADSTRUCTURE rather than crashing and
> will under no circumstances access memory outside the given blob
> size.

[snip]

>> Oh gaak!  What I hear you saying... if you have node a with subnode b 
>> and property b, subnode b has a property c:
>> /a     => node
>> /a/b   => node
>> /a/b   => property (inside node a)
>> /a/b/c => property (inside node b)
> 
> Well, yes.  Except that in OF and derived terminology, properties are
> *never* referred to by path in this way.  It's always:
> 	"property 'fred' of node /foo/bar/baz"

I'm coming from a human interface syntax point of view and assumed that 
the human interface is paths like linux where the last item is a 
directory or file with the computer guessing what you really meant 
(which _isn't_ ambiguous in file/dir paths).  Is there a better syntax 
for distinguishing between node paths and properties?

>> Where I am right now is I created a new function fdt_fullpath_offset:
>>
>> int fdt_fullpath_offset(const void *fdt, const char *path, char **prop);
>>
>> which will return the _node_ /a/b in the gaak illustration above.  It 
>> looks up nodes until it either runs out of path to look up or there is 
>> an error.  On a lookup error, it tries again with the last part of the 
>> path used as a property name.  As a result, if you pass in /a it will 
>> return the node "a", if you pass in /a/b it will return the _node_ "b". 
>>   This is unchanged behavior compared to fdt_path_offset().  (Getting 
>> property "b" is unchanged: you would have to look up node /a with either 
>> fdt_fullpath_offset(... "/a" ...) or fdt_path_offset(... "/a" ...) and 
>> then use that offset with fdt_property() to get the property "b".)
> 
> I really don't like this idea much.  I don't think it's sufficiently
> useful to justify the increased implementation complexity and semantic
> confusion.

But that is the human notation, or am I making incorrect assumptions? 
I'm new to OF and fdt notation.

[snip]

FWIIW, this is what I have running in u-boot...

----------------------------------------------------------------
gaak.dts
----------------------------------------------------------------
/*
  * Ugly ugly ugly tree for testing.
  */
/ {
         model = "gaak";
         compatible = "notlikely";
         #address-cells = <1>;
         #size-cells = <1>;
         linux,phandle = <100>;

         ugly = "first level property ugly";
         ugly {
                 ugly {
                         ugly = "third level property ugly";
                 };
/**** dtc doesn't allow having a property after a node
                 ugly = "second level property ugly";
****/
         };
/**** dtc doesn't allow an anonymous node other than the root one?
         {
                 ugly = "ugly property in anonymous node";
         };
****/
};
----------------------------------------------------------------
u-boot "fdt" output
----------------------------------------------------------------
=> fdt print /
/ {
         model="gaak"
         compatible="notlikely"
         #address-cells=<00000001>
         #size-cells=<00000001>
         linux,phandle=<00000100>
         ugly="first level property ugly"
         ugly {
                 ugly {
                         ugly="third level property ugly"
                 }
         }
}
=> fdt print /ugly
ugly {
         ugly {
                 ugly="third level property ugly"
         }
}
=> fdt get /ugly
/ugly="first level property ugly"
----------------------------------------------------------------

Oops, I forgot to print the semicolons on the tree dump.  Something to 
fix tomorrow^W later today.

Best regards,
gvb

Re: RFA & Update: Using libfdt in u-boot for fdt command

[David Gibson]

On Fri, Mar 02, 2007 at 12:25:17AM -0500, Jerry Van Baren wrote:
> David Gibson wrote:
> >On Thu, Mar 01, 2007 at 11:08:38PM -0500, Jerry Van Baren wrote:
> 
> [snip]
> 
> >>to give me a pointer to the node name for node tags and property name 
> >>for property tags.  Now that I have it working, it would be trivial to 
> >>change the calls to _fdt_next_tag() to instead call fdt_next_tag() 
> >>passing NULL for the new fourth parameter **namep. ;-)
> >>
> >>The reason I need it, I'm printing an unknown tree by stepping through 
> >>the tree discovering the node and property names.  I need to have 
> >>fdt_next_tag() return the *name* of the node/property as well as the tag 
> >>so that I can print and indent for nodes or look up the property value 
> >>and print the name=value combination.
> >
> >Hrm.  And it returns NULL for tags without a name?
> 
> I was unable to generate a tag without a name using dtc (other than the 
> root node).  It should/would return null, which would be a problem. :-/

I was thinking more of tag types which don't have a name, to wit,
FDT_END_NODE and FDT_NOP.

> >That might be a useful extension for the next_tag function.  The one
> >thing I'm concerned about is who's responsible for verifying the name
> >pointer.  I'm trying to keep libfdt robust enough that evern if
> >presented with a badly corrupt blob it will fail relatively
> >gracefully.  Ideally, no matter what it's presented with, it will
> >always return at worst FDT_ERR_BADSTRUCTURE rather than crashing and
> >will under no circumstances access memory outside the given blob
> >size.
> 
> [snip]
> 
> >>Oh gaak!  What I hear you saying... if you have node a with subnode b 
> >>and property b, subnode b has a property c:
> >>/a     => node
> >>/a/b   => node
> >>/a/b   => property (inside node a)
> >>/a/b/c => property (inside node b)
> >
> >Well, yes.  Except that in OF and derived terminology, properties are
> >*never* referred to by path in this way.  It's always:
> >	"property 'fred' of node /foo/bar/baz"
> 
> I'm coming from a human interface syntax point of view and assumed that 
> the human interface is paths like linux where the last item is a 
> directory or file with the computer guessing what you really meant 
> (which _isn't_ ambiguous in file/dir paths).  Is there a better syntax 
> for distinguishing between node paths and properties?

You assumed incorrectly.  Well, unless you count /proc/device-tree as
a human interface to the device tree, which isn't entirely
unreasonable.  OF certainly doesn't use that approach, it uses state
instead, first "dev /foo/bar/baz" then ".properties" or "setprop ....".

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

Re: RFA & Update: Using libfdt in u-boot for fdt command

[Jerry Van Baren]

David Gibson wrote:
> On Fri, Mar 02, 2007 at 12:25:17AM -0500, Jerry Van Baren wrote:
>> David Gibson wrote:
>>> On Thu, Mar 01, 2007 at 11:08:38PM -0500, Jerry Van Baren wrote:
>> [snip]
>>
>>>> to give me a pointer to the node name for node tags and property name 
>>>> for property tags.  Now that I have it working, it would be trivial to 
>>>> change the calls to _fdt_next_tag() to instead call fdt_next_tag() 
>>>> passing NULL for the new fourth parameter **namep. ;-)
>>>>
>>>> The reason I need it, I'm printing an unknown tree by stepping through 
>>>> the tree discovering the node and property names.  I need to have 
>>>> fdt_next_tag() return the *name* of the node/property as well as the tag 
>>>> so that I can print and indent for nodes or look up the property value 
>>>> and print the name=value combination.
>>> Hrm.  And it returns NULL for tags without a name?
>> I was unable to generate a tag without a name using dtc (other than the 
>> root node).  It should/would return null, which would be a problem. :-/
> 
> I was thinking more of tag types which don't have a name, to wit,
> FDT_END_NODE and FDT_NOP.
> 
>>> That might be a useful extension for the next_tag function.  The one
>>> thing I'm concerned about is who's responsible for verifying the name
>>> pointer.  I'm trying to keep libfdt robust enough that evern if
>>> presented with a badly corrupt blob it will fail relatively
>>> gracefully.  Ideally, no matter what it's presented with, it will
>>> always return at worst FDT_ERR_BADSTRUCTURE rather than crashing and
>>> will under no circumstances access memory outside the given blob
>>> size.
>> [snip]
>>
>>>> Oh gaak!  What I hear you saying... if you have node a with subnode b 
>>>> and property b, subnode b has a property c:
>>>> /a     => node
>>>> /a/b   => node
>>>> /a/b   => property (inside node a)
>>>> /a/b/c => property (inside node b)
>>> Well, yes.  Except that in OF and derived terminology, properties are
>>> *never* referred to by path in this way.  It's always:
>>> 	"property 'fred' of node /foo/bar/baz"
>> I'm coming from a human interface syntax point of view and assumed that 
>> the human interface is paths like linux where the last item is a 
>> directory or file with the computer guessing what you really meant 
>> (which _isn't_ ambiguous in file/dir paths).  Is there a better syntax 
>> for distinguishing between node paths and properties?
> 
> You assumed incorrectly.  Well, unless you count /proc/device-tree as
> a human interface to the device tree, which isn't entirely
> unreasonable.  OF certainly doesn't use that approach, it uses state
> instead, first "dev /foo/bar/baz" then ".properties" or "setprop ....".

OF is a programming language that has some crude elements of interaction 
(wouldn't that be "/foo/bar/baz" dev in Forthspeak? ;-).  Making user 
interface commands have state, where you do "fdt dev /foo/bar/baz" 
(remembering the offset of the node) and then "fdt .properties" 
implicitly working on /foo/bar/baz is ugly ugly ugly.

We need a usable human interface syntax, replacing the last "/" with 
some other character that is not a legal character for a name and thus 
won't cause confusion.  I am not familiar enough with OF to know if 
there is a unique character that can be used for the node path vs. 
property separator.  If someone has a good one, hollar, otherwise I'll 
do some more research when I have time.

Lacking a good property separator character, I will be sticking with the 
convention that the stuff after the last "/" is a node unless that 
assumption is wrong, in which case it is a property.

Best regards,
gvb

Re: [U-Boot-Users] RFA & Update: Using libfdt in u-boot for fdt command

[Jerry Van Baren]

Jerry Van Baren wrote:
> David Gibson wrote:

[lots of snipping]

>>> I'm coming from a human interface syntax point of view and assumed that 
>>> the human interface is paths like linux where the last item is a 
>>> directory or file with the computer guessing what you really meant 
>>> (which _isn't_ ambiguous in file/dir paths).  Is there a better syntax 
>>> for distinguishing between node paths and properties?
>> You assumed incorrectly.  Well, unless you count /proc/device-tree as
>> a human interface to the device tree, which isn't entirely
>> unreasonable.  OF certainly doesn't use that approach, it uses state
>> instead, first "dev /foo/bar/baz" then ".properties" or "setprop ....".
> 
> OF is a programming language that has some crude elements of interaction 
> (wouldn't that be "/foo/bar/baz" dev in Forthspeak? ;-).  Making user 
> interface commands have state, where you do "fdt dev /foo/bar/baz" 
> (remembering the offset of the node) and then "fdt .properties" 
> implicitly working on /foo/bar/baz is ugly ugly ugly.
> 
> We need a usable human interface syntax, replacing the last "/" with 
> some other character that is not a legal character for a name and thus 
> won't cause confusion.  I am not familiar enough with OF to know if 
> there is a unique character that can be used for the node path vs. 
> property separator.  If someone has a good one, hollar, otherwise I'll 
> do some more research when I have time.
> 
> Lacking a good property separator character, I will be sticking with the 
> convention that the stuff after the last "/" is a node unless that 
> assumption is wrong, in which case it is a property.
> 
> Best regards,
> gvb

OK, here is a reference on the OF device tree browsing commands:
<http://www.firmworks.com/QuickRef.html#Device%20Tree%20Browsing>

Where I'm coming from is that I've written the "fdt print" command to 
put out the same text (possibly with data formatting differences) as 
went into the dtc to create the blob.  This is very useful and intuitive 
to me.

The OF device tree browsing is modeled after filesys directory and file 
browsing (sorta).
   ".properties" ~ "ls" but only shows files (properties ~ files)
   "dev" == "cd"
   "ls" == "ls -d *" (only shows subdirectories)
   "pwd" == "pwd"
   "dend" - has no equiv
   "show-devs" - has no equiv, sounds like it may be my "print" command
   "words" - has no equiv, does not apply (dir *.exe in DOS :-)
   "sift-devs ccc" == find . -name "*ccc*"

Looks a lot more complex with no clear benefit for u-boot.

I have not found any character that could clearly and cleanly be used to 
separate the node path from the property name.
* Comma ',' - used to separate a device name from an argument - one
      could argue that the property name is an argument to the path.
      "/foo/bar,baz" is the property baz under the node "/foo/bar".
* Space ' ' - "/foo/bar/baz" is a node path, "/foo/bar baz" is the
     property baz under the node "/foo/bar".  Spaces complicate parsing.

Any strong opinions?  At this point I don't see any reason to change 
from my current technique and proposed command set for u-boot.

Best regards,
gvb

Re: [U-Boot-Users] RFA & Update: Using libfdt in u-boot for fdt command

[Jerry Van Baren]

David Gibson wrote:
> On Fri, Mar 02, 2007 at 12:25:17AM -0500, Jerry Van Baren wrote:
>> David Gibson wrote:
>>> On Thu, Mar 01, 2007 at 11:08:38PM -0500, Jerry Van Baren wrote:
>> [snip]
>>
>>>> to give me a pointer to the node name for node tags and property name 
>>>> for property tags.  Now that I have it working, it would be trivial to 
>>>> change the calls to _fdt_next_tag() to instead call fdt_next_tag() 
>>>> passing NULL for the new fourth parameter **namep. ;-)
>>>>
>>>> The reason I need it, I'm printing an unknown tree by stepping through 
>>>> the tree discovering the node and property names.  I need to have 
>>>> fdt_next_tag() return the *name* of the node/property as well as the tag 
>>>> so that I can print and indent for nodes or look up the property value 
>>>> and print the name=value combination.
>>> Hrm.  And it returns NULL for tags without a name?
>> I was unable to generate a tag without a name using dtc (other than the 
>> root node).  It should/would return null, which would be a problem. :-/
> 
> I was thinking more of tag types which don't have a name, to wit,
> FDT_END_NODE and FDT_NOP.

fdt_next_tag() returns the tag, so it returns FDT_END_NODE or FDT_NOP 
and, presumably, the caller would know there is no name associated with 
those tags.  Setting the name pointer *namep to NULL would be good 
paranoia, however.

Best regards,
gvb

Re: [U-Boot-Users] RFA & Update: Using libfdt in u-boot for fdt command

[Mark A. Greer]

On Thu, Mar 01, 2007 at 08:17:15PM -0500, Jerry Van Baren wrote:
> Mark A. Greer wrote:
> >On Thu, Mar 01, 2007 at 09:01:24AM -0500, Jerry Van Baren wrote:
> >>Hi all,
> >>
> >>This is a Request for Advice.
> >
> >Hi Jerry.
> >
> >One minor thing.  I'd just want to remind you that we shouldn't stray
> >too far from the OF interface.  The bootwrapper code sits on top of
> >either the FDT access interface or the true OF DT access interface.  
> >The closer we keep the two, the cleaner & easier the we can keep
> >the bootwrapper code.
> >
> >Mark
> 
> Hi Mark,
> 
> Understood, but that isn't really my battle.

Hi Jerry,

Yeah, I know.  I didn't word my msg very well.  I was really making a
general stmt/reminder and not directing it to you specifically.  I
should have made that clearer in my msg tho.  Sorry.

Mark

Re: [U-Boot-Users] RFA & Update: Using libfdt in u-boot for fdt command

[David Gibson]

On Fri, Mar 02, 2007 at 01:35:35PM -0500, Jerry Van Baren wrote:
> Jerry Van Baren wrote:
> > David Gibson wrote:
[snip]
> OK, here is a reference on the OF device tree browsing commands:
> <http://www.firmworks.com/QuickRef.html#Device%20Tree%20Browsing>
> 
> Where I'm coming from is that I've written the "fdt print" command to 
> put out the same text (possibly with data formatting differences) as 
> went into the dtc to create the blob.  This is very useful and intuitive 
> to me.
> 
> The OF device tree browsing is modeled after filesys directory and file 
> browsing (sorta).
>    ".properties" ~ "ls" but only shows files (properties ~ files)
>    "dev" == "cd"
>    "ls" == "ls -d *" (only shows subdirectories)
>    "pwd" == "pwd"
>    "dend" - has no equiv
>    "show-devs" - has no equiv, sounds like it may be my "print" command
>    "words" - has no equiv, does not apply (dir *.exe in DOS :-)
>    "sift-devs ccc" == find . -name "*ccc*"
> 
> Looks a lot more complex with no clear benefit for u-boot.

Sorry, I was unclear.  I wasn't trying to suggest you use the OF
client interface model for device tree commands in general.  Just that
you don't treat properties as having paths.

> I have not found any character that could clearly and cleanly be used to 
> separate the node path from the property name.
> * Comma ',' - used to separate a device name from an argument - one
>       could argue that the property name is an argument to the path.
>       "/foo/bar,baz" is the property baz under the node "/foo/bar".
> * Space ' ' - "/foo/bar/baz" is a node path, "/foo/bar baz" is the
>      property baz under the node "/foo/bar".  Spaces complicate parsing.

Space only complicates parsing if you insist of thinking of these
things as paths-to-properties, which is not really a good idea in the
first place.  Just think of the node-path and the property name as
separate parameters to your commands.

So, getprop takes 2 parameters (node, property), setprop takes 3
(node, property, value).  print, or whatever you end up calling it
takes either 1 or 2 (node, plus optional property name)

> Any strong opinions?  At this point I don't see any reason to change 
> from my current technique and proposed command set for u-boot.

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