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

[Jerry Van Baren <gerald.vanbaren@smiths-aerospace.com>]

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