[U-Boot-Users] fdt_find_compatible_node() and friends

[Jerry Van Baren]

Wolfgang Grandegger wrote:
> Hi Jerry,
> 
> Jerry Van Baren wrote:
> [...]
>> Hi Wolfgang G,
>>
>> I've applied your patches to my local (working) repository and will push 
>> the changes tonight (my tonight, your tomorrow ;-).  I created a 
>> subroutine out of three snippets of code in cmd_fdt.c which your 
>> fdt_find_node_by_path() change  fixed up so I had to fix one line in the 
>> new subroutine by hand.  Not bad at all considering the changes I made 
>> in that file.
> 
> Thanks. In the meantime I observed, that
> 
>    fdt_find_node_by_path(fdt, 0, "/");
> 
> returns an error. Is that by purpose?
[snip]
> Wolfgang.

Hmm, interesting observation.  The character '/' is a figment of our 
imagination, offset 0 in the tree is the root node.  The character '/' 
is the path separator and doesn't actually exist anywhere in the fdt - 
when parsing paths, the stuff between the slashes is searched for and 
the slashes themselves are skipped over.

What you asked in the above call is a node with no name under the root 
node, i.e. "//" in human-speak.  That wasn't found, of course.  On the 
other hand, it is a pretty obvious "mistake" (I was guilty of making the 
same mistake when I first tried to use fdt_path_offset()).

It seems like I was forever doing a conditional:

59     if (strcmp(pathp, "/") == 0) {
60             nodeoffset = 0;
61     } else {
62             nodeoffset = fdt_path_offset (fdt, pathp);
63             if (nodeoffset < 0) {

<http://www.denx.de/cgi-bin/gitweb.cgi?p=u-boot/u-boot-fdt.git;a=blob;f=common/cmd_fdt.c;h=bf0aef70cedea6ec4a2a446af54c9e1467617a96;hb=HEAD#l55>

Trivia: Your patch we are discussing changed exactly this 
fdt_path_offset() into fdt_find_node_by_path() - this is the one place 
your patch didn't apply, because I refactored the three conditionals 
into a single wrapper subroutine.

At the risk of being accused of codling our users, I would propose we 
add the equivalent "/" detection code (above) to 
fdt_find_node_by_path(), (I will do that tonight unless you beat me to 
it).  It seems silly to have the caller replicate or wrap the 
conditional since it is going to be such a common idiom/mistake.

Thanks,
gvb