Re: [U-Boot-Users] LIBFDT: first version of fdt_find_compatible_node

Wolfgang Grandegger wrote:

Hello,

attached you can find a patch implementing fdt_find_compatible_node():

Yeeee-ha! :-)

/*

• Find a node based on its device type and one of the tokens in
• its its "compatible" property. On success, the offset of that
• node is returned or an error code:
• startoffset - the node to start searching from or 0, the node

•              you pass will not be searched, only the next one
  

•              will; typically, you pass 0 to start the search
  

•              and then what the previous call returned.
  

• type - the device type string to match against
• compat - the string to match to one of the tokens

•              in the "compatible" list.
  

*/

It should be used as shown below:

offset = 0;
do {
    offset = fdt_find_compatible_node(fdt, offset, "type", "comp");
} while (offset >= 0);

This first hack also implements a cached version as alternative, because tag re-scanning might take quite long. In principle, the cache could also be used for other functions, like fdt_path_offset(), and could be invalidated in case the FDT gets updated.

What do you think?

Thanks.

Wolfgang.

Looks good. In the real patch, I would like to see the cache addition split from the fdt_node_is_compatible() fdt_find_compatible_node() and addition.

---

diff --git a/include/libfdt.h b/include/libfdt.h index f8bac73..3fd7c9f 100644 --- a/include/libfdt.h +++ b/include/libfdt.h @@ -89,6 +89,12 @@ uint32_t fdt_next_tag(const void *fdt, int offset, int fdt_num_reservemap(void *fdt, int *used, int *total); int fdt_get_reservemap(void *fdt, int n, struct fdt_reserve_entry *re);

+int fdt_node_is_compatible(const void *fdt, int nodeoffset,

• 	   const char *compat);
  

+int fdt_find_compatible_node(const void *fdt, int startoffset,

• 	     const char *type, const char *compat);
  

/* Write-in-place functions */ int fdt_setprop_inplace(void *fdt, int nodeoffset, const char *name, const void *val, int len); diff --git a/libfdt/fdt_ro.c b/libfdt/fdt_ro.c index 4e2c325..65ede88 100644 --- a/libfdt/fdt_ro.c +++ b/libfdt/fdt_ro.c @@ -55,6 +55,236 @@ char *fdt_string(const void *fdt, int stroffset) return (char *)fdt + fdt_off_dt_strings(fdt) + stroffset; }

+#define CONFIG_OF_LIBFDT_TABLE 64 +#if CONFIG_OF_LIBFDT_TABLE > 0

[snip]

+#endif /* CONFIG_OF_LIBFDT_TABLE > 0 */

+/*

• • Check if the specified node is compatible by comparing the
• • tokens in its "compatible" property with the specified string:
• • nodeoffset - starting place of the node
• • compat - the string to match to one of the tokens

• •            in the "compatible" list.
    

• */

+int fdt_node_is_compatible(const void *fdt, int nodeoffset,

• 	   const char *compat)
  

+{

• const char* cp;
• int cplen, l;
• cp = fdt_getprop(fdt, nodeoffset, "compatible", &cplen);
• if (cp == NULL)

• return 0;
  

• while (cplen > 0) {

• if (strncmp(cp, compat, strlen(compat)) == 0)
  

• 	return 1;
  

• l = strlen(cp) + 1;
  

• cp += l;
  

• cplen -= l;
  

• }
• return 0;

+}

I see this came directly from arch/powerpc/kernel/prom.c, but using "l" for a variable is evil. For a minute, I was wondering how the compiler was compiling "1" (one) as a lvalue. I would prefer it to be "len" or something more descriptive.

+/*

• • Find a node based on its device type and one of the tokens in
• • its its "compatible" property. On success, the offset of that
• • node is return or an error code:
• • startoffset - the node to start searching from or 0, the node

• •              you pass will not be searched, only the next one
    

• •              will; typically, you pass 0 to start the search
    

• •              and then what the previous call returned.
    

• • type - the device type string to match against
• • compat - the string to match to one of the tokens

• •              in the "compatible" list.
    

• */

+#if CONFIG_OF_LIBFDT_TABLE > 0

[snip]

+#else /* CONFIG_OF_LIBFDT_TABLE <= 0 */ +int fdt_find_compatible_node(const void *fdt, int startoffset,

• 	     const char *type, const char *compat)
  

+{

• static int level, typefound;
• static int nodeoffset, nextoffset;
• int offset, namestroff;
• struct fdt_property *prop;
• uint32_t tag;
• CHECK_HEADER(fdt);
• if (startoffset == 0) {

• level = 0;
  

• tag = fdt_next_tag(fdt, 0, &nextoffset, NULL);
  

• if (tag != FDT_BEGIN_NODE)
  

• 	return -FDT_ERR_BADOFFSET;
  

• } else if (startoffset != nodeoffset)

• return -FDT_ERR_BADOFFSET;
  

• do {

• offset = nextoffset;
  

• tag = fdt_next_tag(fdt, offset, &nextoffset, NULL);
  

• switch (tag) {
  

• case FDT_END:
  

• 	return -FDT_ERR_TRUNCATED;
  

• case FDT_BEGIN_NODE:
  

• 	level++;
  

• 	nodeoffset = offset;
  

• 	typefound = 0;
  

• 	break;
  

• case FDT_END_NODE:
  

• 	level--;
  

• 	break;
  

• case FDT_PROP:
  

• 	if (typefound)
  

• 		continue;
  

• 	prop = fdt_offset_ptr_typed(fdt, offset, prop);
  

• 	if (! prop)
  

• 		continue;
  

• 	namestroff = fdt32_to_cpu(prop->nameoff);
  

• 	if (streq(fdt_string(fdt, namestroff), "device_type")) {
  

• 		int len = fdt32_to_cpu(prop->len);
  

• 		typefound = 0;
  

• 		prop = fdt_offset_ptr(fdt, offset,
  

• 				      sizeof(*prop)+len);
  

• 		if (! prop)
  

• 			continue;
  

• 		if (strncasecmp(prop->data, type, len - 1) == 0 &&
  

• 		    fdt_node_is_compatible(fdt, nodeoffset, compat))
  

• 			return nodeoffset;
  

• 	}
  

• 	break;
  

• case FDT_NOP:
  

• 	break;
  

• default:
  

• 	return -FDT_ERR_BADSTRUCTURE;
  

• }
  

• } while (level >= 0);
• return -FDT_ERR_NOTFOUND;

+} +#endif /* CONFIG_OF_LIBFDT_TABLE > 0 */

/*

• Return the node offset of the node specified by:
• parentoffset - starting place (0 to start at the root)

For the above version of fdt_find_compatible_node(), as well as the one that fills the cache table (snipped), I'm thinking it would be better to use the function I added:

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

I added this to step through the nodes and properties for dumping the tree. Rather than having Yet Another (slightly modified) Copy of the loop & switch, you should be able to use fdt_next_tag() to step through the nodes and properties, doing the if (streq(fdt_string(fdt, namestroff), "device_type")) on the **namep parameter on every call to find the device_type properties. Does this make sense?

Thanks, gvb