From mboxrd@z Thu Jan 1 00:00:00 1970 From: Wolfgang Grandegger Date: Thu, 17 May 2007 14:31:15 +0200 Subject: [U-Boot-Users] fdt_find_compatible_node() and friends In-Reply-To: <464C44D6.7060002@smiths-aerospace.com> References: <4639F61F.2050506@grandegger.com> <4639FC57.8080403@smiths-aerospace.com> <463C80BB.5080305@grandegger.com> <464C30A8.3000002@gmail.com> <464C3D3A.5020409@grandegger.com> <464C44D6.7060002@smiths-aerospace.com> Message-ID: <464C4B13.4000100@grandegger.com> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de Jerry Van Baren wrote: > 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()). And it is frequently used in the kernel as the following command reveals: $ find . -name '*.c' | xargs grep find_node_by_path | grep '"/"' > > 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) { > > I actually wanted to get the string for the property "model" and yes, fdt_getprop(fdt, 0, "model", NULL) does work. > > 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... and no hurry. Wolfgang.