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

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

[Wolfgang Grandegger]

Hi Jerry,

before re-coding fdt_find_compatible_node(), some more comments.

After browsing more carefully the FDT related code of "arch/powerpc"
I think we also need, apart from fdt_find_compatible_node() and
fdt_path_offset(), fdt_find_node_by_type() and maybe 
fdt_find_node_by_name(). These functions do a sequential scan of all 
devices starting at the beginning or after a specified node. They 
actually ignore the hierarchy. Do you agree?
BTW: any reason why not using the more compatible name 
fdt_find_node_by_path() for fdt_path_offset()?

Wolfgang.

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

[Jerry Van Baren]

Wolfgang Grandegger wrote:
> Hi Jerry,
> 
> before re-coding fdt_find_compatible_node(), some more comments.
> 
> After browsing more carefully the FDT related code of "arch/powerpc"
> I think we also need, apart from fdt_find_compatible_node() and
> fdt_path_offset(), fdt_find_node_by_type() and maybe 
> fdt_find_node_by_name(). These functions do a sequential scan of all 
> devices starting at the beginning or after a specified node. They 
> actually ignore the hierarchy. Do you agree?
> BTW: any reason why not using the more compatible name 
> fdt_find_node_by_path() for fdt_path_offset()?
> 
> Wolfgang.

Hi WolfganG,

I'm not an expert, I just fake it on email ;-).  With that disclaimer, I 
would agree with you WRT all the "find" functions.  The original libfdt 
code does not support any "find" functions, so we will need to add them.

WRT to fdt_find_node_by_path() vs. fdt_path_offset(), I vaguely recall 
some renames happening in the kernel source, but I cannot find them so 
my memory likely is faulty[1].  I would be strongly in favor of 
following the kernel's lead and renaming that function since we are 
already divergent from the original libfdt.  The kernel's name is a much 
better description.

Best regards,
gvb

[1] My mind is like a steel trap... unfortunately its been soaking it in 
beer for years now.

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

[Wolfgang Grandegger]

Hi Jerry,

Jerry Van Baren wrote:
> Wolfgang Grandegger wrote:
>> Hi Jerry,
>>
>> before re-coding fdt_find_compatible_node(), some more comments.
>>
>> After browsing more carefully the FDT related code of "arch/powerpc"
>> I think we also need, apart from fdt_find_compatible_node() and
>> fdt_path_offset(), fdt_find_node_by_type() and maybe 
>> fdt_find_node_by_name(). These functions do a sequential scan of all 
>> devices starting at the beginning or after a specified node. They 
>> actually ignore the hierarchy. Do you agree?
>> BTW: any reason why not using the more compatible name 
>> fdt_find_node_by_path() for fdt_path_offset()?
>>
>> Wolfgang.
> 
> Hi WolfganG,
> 
> I'm not an expert, I just fake it on email ;-).  With that disclaimer, I 
> would agree with you WRT all the "find" functions.  The original libfdt 
> code does not support any "find" functions, so we will need to add them.
> 
> WRT to fdt_find_node_by_path() vs. fdt_path_offset(), I vaguely recall 
> some renames happening in the kernel source, but I cannot find them so 
> my memory likely is faulty[1].  I would be strongly in favor of 
> following the kernel's lead and renaming that function since we are 
> already divergent from the original libfdt.  The kernel's name is a much 
> better description.

OK, I have attached two new patches replacing fdt_path_offset() with 
fdt_find_node_by_path() and implementing fdt_find_node_by_type() and 
fdt_find_compatible_node(). This version does not use static variables 
any more to scan the nodes without re-scanning, but looks for the end 
tag. I think it's (almost) good enough to be applied.

Wolfgang.

-------------- next part --------------
A non-text attachment was scrubbed...
Name: fdt-find-node-by-path.patch
Type: text/x-patch
Size: 6187 bytes
Desc: not available
Url : http://lists.denx.de/pipermail/u-boot/attachments/20070505/2e942f0b/attachment.bin 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: fdt-find-compatible-node.patch
Type: text/x-patch
Size: 6175 bytes
Desc: not available
Url : http://lists.denx.de/pipermail/u-boot/attachments/20070505/2e942f0b/attachment-0001.bin 

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

[Wolfgang Grandegger]

Hi Jerry,

any intention to merge these patches?

Thanks,

Wolfgang.

Wolfgang Grandegger wrote:
> Hi Jerry,
> 
> Jerry Van Baren wrote:
>> Wolfgang Grandegger wrote:
>>> Hi Jerry,
>>>
>>> before re-coding fdt_find_compatible_node(), some more comments.
>>>
>>> After browsing more carefully the FDT related code of "arch/powerpc"
>>> I think we also need, apart from fdt_find_compatible_node() and
>>> fdt_path_offset(), fdt_find_node_by_type() and maybe 
>>> fdt_find_node_by_name(). These functions do a sequential scan of all 
>>> devices starting at the beginning or after a specified node. They 
>>> actually ignore the hierarchy. Do you agree?
>>> BTW: any reason why not using the more compatible name 
>>> fdt_find_node_by_path() for fdt_path_offset()?
>>>
>>> Wolfgang.
>>
>> Hi WolfganG,
>>
>> I'm not an expert, I just fake it on email ;-).  With that disclaimer, 
>> I would agree with you WRT all the "find" functions.  The original 
>> libfdt code does not support any "find" functions, so we will need to 
>> add them.
>>
>> WRT to fdt_find_node_by_path() vs. fdt_path_offset(), I vaguely recall 
>> some renames happening in the kernel source, but I cannot find them so 
>> my memory likely is faulty[1].  I would be strongly in favor of 
>> following the kernel's lead and renaming that function since we are 
>> already divergent from the original libfdt.  The kernel's name is a 
>> much better description.
> 
> OK, I have attached two new patches replacing fdt_path_offset() with 
> fdt_find_node_by_path() and implementing fdt_find_node_by_type() and 
> fdt_find_compatible_node(). This version does not use static variables 
> any more to scan the nodes without re-scanning, but looks for the end 
> tag. I think it's (almost) good enough to be applied.
> 
> Wolfgang.
> 
> 
> ------------------------------------------------------------------------
> 
> Replace fdt_node_offset() with fdt_find_node_by_path().
> 
> From: Wolfgang Grandegger <wg@grandegger.com>
> 
> The new name matches more closely the kernel's name, which is also
> a much better description.
> ---
> 
>  board/mpc8360emds/mpc8360emds.c |    2 +-
>  board/mpc8360emds/pci.c         |    2 +-
>  common/cmd_fdt.c                |    6 +++---
>  common/fdt_support.c            |   10 +++++-----
>  cpu/mpc83xx/cpu.c               |    2 +-
>  include/libfdt.h                |    2 +-
>  libfdt/fdt_ro.c                 |    2 +-
>  7 files changed, 13 insertions(+), 13 deletions(-)
> 
> diff --git a/board/mpc8360emds/mpc8360emds.c b/board/mpc8360emds/mpc8360emds.c
> index 562eb8b..3f87f09 100644
> --- a/board/mpc8360emds/mpc8360emds.c
> +++ b/board/mpc8360emds/mpc8360emds.c
> @@ -681,7 +681,7 @@ ft_board_setup(void *blob, bd_t *bd)
>  	int nodeoffset;
>  	int tmp[2];
>  
> -	nodeoffset = fdt_path_offset (fdt, "/memory");
> +	nodeoffset = fdt_find_node_by_path (fdt, "/memory");
>  	if (nodeoffset >= 0) {
>  		tmp[0] = cpu_to_be32(bd->bi_memstart);
>  		tmp[1] = cpu_to_be32(bd->bi_memsize);
> diff --git a/board/mpc8360emds/pci.c b/board/mpc8360emds/pci.c
> index 158effe..4c7a82b 100644
> --- a/board/mpc8360emds/pci.c
> +++ b/board/mpc8360emds/pci.c
> @@ -311,7 +311,7 @@ ft_pci_setup(void *blob, bd_t *bd)
>  	int err;
>  	int tmp[2];
>  
> -	nodeoffset = fdt_path_offset (fdt, "/" OF_SOC "/pci at 8500");
> +	nodeoffset = fdt_find_node_by_path (fdt, "/" OF_SOC "/pci at 8500");
>  	if (nodeoffset >= 0) {
>  		tmp[0] = cpu_to_be32(hose[0].first_busno);
>  		tmp[1] = cpu_to_be32(hose[0].last_busno);
> diff --git a/common/cmd_fdt.c b/common/cmd_fdt.c
> index 08fe351..7058197 100644
> --- a/common/cmd_fdt.c
> +++ b/common/cmd_fdt.c
> @@ -177,7 +177,7 @@ int do_fdt (cmd_tbl_t * cmdtp, int flag, int argc, char *argv[])
>  		if (strcmp(pathp, "/") == 0) {
>  			nodeoffset = 0;
>  		} else {
> -			nodeoffset = fdt_path_offset (fdt, pathp);
> +			nodeoffset = fdt_find_node_by_path (fdt, pathp);
>  			if (nodeoffset < 0) {
>  				/*
>  			 	 * Not found or something else bad happened.
> @@ -306,7 +306,7 @@ int do_fdt (cmd_tbl_t * cmdtp, int flag, int argc, char *argv[])
>  			nodeoffset = 0;
>  			printf("/");
>  		} else {
> -			nodeoffset = fdt_path_offset (fdt, pathp);
> +			nodeoffset = fdt_find_node_by_path (fdt, pathp);
>  			if (nodeoffset < 0) {
>  				/*
>  				 * Not found or something else bad happened.
> @@ -405,7 +405,7 @@ int do_fdt (cmd_tbl_t * cmdtp, int flag, int argc, char *argv[])
>  		if (strcmp(argv[2], "/") == 0) {
>  			nodeoffset = 0;
>  		} else {
> -			nodeoffset = fdt_path_offset (fdt, argv[2]);
> +			nodeoffset = fdt_find_node_by_path (fdt, argv[2]);
>  			if (nodeoffset < 0) {
>  				/*
>  				 * Not found or something else bad happened.
> diff --git a/common/fdt_support.c b/common/fdt_support.c
> index 69099c4..05bf939 100644
> --- a/common/fdt_support.c
> +++ b/common/fdt_support.c
> @@ -92,7 +92,7 @@ int fdt_chosen(void *fdt, ulong initrd_start, ulong initrd_end, int force)
>  	/*
>  	 * Find the "chosen" node.
>  	 */
> -	nodeoffset = fdt_path_offset (fdt, "/chosen");
> +	nodeoffset = fdt_find_node_by_path (fdt, "/chosen");
>  
>  	/*
>  	 * If we have a "chosen" node already the "force the writing"
> @@ -140,7 +140,7 @@ int fdt_chosen(void *fdt, ulong initrd_start, ulong initrd_end, int force)
>  		printf("libfdt: %s\n", fdt_strerror(err));
>  #endif
>  
> -	nodeoffset = fdt_path_offset (fdt, "/cpus");
> +	nodeoffset = fdt_find_node_by_path (fdt, "/cpus");
>  	if (nodeoffset >= 0) {
>  		clock = cpu_to_be32(bd->bi_intfreq);
>  		err = fdt_setprop(fdt, nodeoffset, "clock-frequency", &clock, 4);
> @@ -148,7 +148,7 @@ int fdt_chosen(void *fdt, ulong initrd_start, ulong initrd_end, int force)
>  			printf("libfdt: %s\n", fdt_strerror(err));
>  	}
>  #ifdef OF_TBCLK
> -	nodeoffset = fdt_path_offset (fdt, "/cpus/" OF_CPU "/timebase-frequency");
> +	nodeoffset = fdt_find_node_by_path (fdt, "/cpus/" OF_CPU "/timebase-frequency");
>  	if (nodeoffset >= 0) {
>  		clock = cpu_to_be32(OF_TBCLK);
>  		err = fdt_setprop(fdt, nodeoffset, "clock-frequency", &clock, 4);
> @@ -185,7 +185,7 @@ int fdt_env(void *fdt)
>  	 * See if we already have a "u-boot-env" node, delete it if so.
>  	 * Then create a new empty node.
>  	 */
> -	nodeoffset = fdt_path_offset (fdt, "/u-boot-env");
> +	nodeoffset = fdt_find_node_by_path (fdt, "/u-boot-env");
>  	if (nodeoffset >= 0) {
>  		err = fdt_del_node(fdt, nodeoffset);
>  		if (err < 0) {
> @@ -305,7 +305,7 @@ int fdt_bd_t(void *fdt)
>  	 * See if we already have a "bd_t" node, delete it if so.
>  	 * Then create a new empty node.
>  	 */
> -	nodeoffset = fdt_path_offset (fdt, "/bd_t");
> +	nodeoffset = fdt_find_node_by_path (fdt, "/bd_t");
>  	if (nodeoffset >= 0) {
>  		err = fdt_del_node(fdt, nodeoffset);
>  		if (err < 0) {
> diff --git a/cpu/mpc83xx/cpu.c b/cpu/mpc83xx/cpu.c
> index e934ba6..ef44581 100644
> --- a/cpu/mpc83xx/cpu.c
> +++ b/cpu/mpc83xx/cpu.c
> @@ -460,7 +460,7 @@ ft_cpu_setup(void *blob, bd_t *bd)
>  	int  j;
>  
>  	for (j = 0; j < (sizeof(fixup_props) / sizeof(fixup_props[0])); j++) {
> -		nodeoffset = fdt_path_offset(fdt, fixup_props[j].node);
> +		nodeoffset = fdt_find_node_by_path(fdt, fixup_props[j].node);
>  		if (nodeoffset >= 0) {
>  			err = (*fixup_props[j].set_fn)(blob, nodeoffset, fixup_props[j].prop, bd);
>  			if (err < 0)
> diff --git a/include/libfdt.h b/include/libfdt.h
> index f8bac73..e080028 100644
> --- a/include/libfdt.h
> +++ b/include/libfdt.h
> @@ -77,7 +77,7 @@ int fdt_subnode_offset_namelen(const void *fdt, int parentoffset,
>  			       const char *name, int namelen);
>  int fdt_subnode_offset(const void *fdt, int parentoffset, const char *name);
>  
> -int fdt_path_offset(const void *fdt, const char *path);
> +int fdt_find_node_by_path(const void *fdt, const char *path);
>  
>  struct fdt_property *fdt_get_property(const void *fdt, int nodeoffset,
>  				      const char *name, int *lenp);
> diff --git a/libfdt/fdt_ro.c b/libfdt/fdt_ro.c
> index 4e2c325..e5518c6 100644
> --- a/libfdt/fdt_ro.c
> +++ b/libfdt/fdt_ro.c
> @@ -129,7 +129,7 @@ int fdt_subnode_offset(const void *fdt, int parentoffset,
>   * Searches for the node corresponding to the given path and returns the
>   * offset of that node.
>   */
> -int fdt_path_offset(const void *fdt, const char *path)
> +int fdt_find_node_by_path(const void *fdt, const char *path)
>  {
>  	const char *end = path + strlen(path);
>  	const char *p = path;
> 
> 
> ------------------------------------------------------------------------
> 
> Add fdt_find_node_by_type() and fdt_find_compatible_node() to LIBFDT
> 
> From: Wolfgang Grandegger <wg@grandegger.com>
> 
> 
> ---
> 
>  include/libfdt.h |    6 ++
>  libfdt/fdt_ro.c  |  161 ++++++++++++++++++++++++++++++++++++++++++++++++------
>  2 files changed, 149 insertions(+), 18 deletions(-)
> 
> diff --git a/include/libfdt.h b/include/libfdt.h
> index e080028..340e89d 100644
> --- a/include/libfdt.h
> +++ b/include/libfdt.h
> @@ -78,6 +78,12 @@ int fdt_subnode_offset_namelen(const void *fdt, int parentoffset,
>  int fdt_subnode_offset(const void *fdt, int parentoffset, const char *name);
>  
>  int fdt_find_node_by_path(const void *fdt, const char *path);
> +int fdt_find_node_by_type(const void *fdt, int nodeoffset, const char *type);
> +
> +int fdt_node_is_compatible(const void *fdt, int nodeoffset,
> +			   const char *compat);
> +int fdt_find_compatible_node(const void *fdt, int nodeoffset,
> +			     const char *type, const char *compat);
>  
>  struct fdt_property *fdt_get_property(const void *fdt, int nodeoffset,
>  				      const char *name, int *lenp);
> diff --git a/libfdt/fdt_ro.c b/libfdt/fdt_ro.c
> index e5518c6..e379fc2 100644
> --- a/libfdt/fdt_ro.c
> +++ b/libfdt/fdt_ro.c
> @@ -48,6 +48,33 @@ static int offset_streq(const void *fdt, int offset,
>  }
>  
>  /*
> + * Checks if the property name matches.
> + */
> +static int prop_name_eq(const void *fdt, int offset, const char *name,
> +			struct fdt_property **prop, int *lenp)
> +{
> +	int namestroff, len;
> +
> +	*prop = fdt_offset_ptr_typed(fdt, offset, *prop);
> +	if (! *prop)
> +		return -FDT_ERR_BADSTRUCTURE;
> +
> +	namestroff = fdt32_to_cpu((*prop)->nameoff);
> +	if (streq(fdt_string(fdt, namestroff), name)) {
> +		len = fdt32_to_cpu((*prop)->len);
> +		*prop = fdt_offset_ptr(fdt, offset,
> +				       sizeof(**prop) + len);
> +		if (*prop) {
> +			if (lenp)
> +				*lenp = len;
> +			return 1;
> +		} else
> +			return -FDT_ERR_BADSTRUCTURE;
> +	}
> +	return 0;
> +}
> +
> +/*
>   * Return a pointer to the string at the given string offset.
>   */
>  char *fdt_string(const void *fdt, int stroffset)
> @@ -56,6 +83,118 @@ char *fdt_string(const void *fdt, int stroffset)
>  }
>  
>  /*
> + * 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, len;
> +
> +	cp = fdt_getprop(fdt, nodeoffset, "compatible", &cplen);
> +	if (cp == NULL)
> +		return 0;
> +	while (cplen > 0) {
> +		if (strncmp(cp, compat, strlen(compat)) == 0)
> +			return 1;
> +		len = strlen(cp) + 1;
> +		cp += len;
> +		cplen -= len;
> +	}
> +
> +	return 0;
> +}
> +
> +/*
> + * Find a node by its device type property. On success, the offset of that
> + * node is returned or an error code otherwise:
> + *
> + *   nodeoffset - 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.
> + */
> +int fdt_find_node_by_type(const void *fdt, int nodeoffset, const char *type)
> +{
> +	int offset, nextoffset;
> +	struct fdt_property *prop;
> +	uint32_t tag;
> +	int len, ret;
> +
> +	CHECK_HEADER(fdt);
> +
> +	tag = fdt_next_tag(fdt, nodeoffset, &nextoffset, NULL);
> +	if (tag != FDT_BEGIN_NODE)
> +		return -FDT_ERR_BADOFFSET;
> +	if (nodeoffset)
> +		nodeoffset = 0;	/* start searching with next node */
> +
> +	while (1) {
> +		offset = nextoffset;
> +		tag = fdt_next_tag(fdt, offset, &nextoffset, NULL);
> +
> +		switch (tag) {
> +		case FDT_BEGIN_NODE:
> +			nodeoffset = offset;
> +			break;
> +
> +		case FDT_PROP:
> +			if (nodeoffset == 0)
> +				break;
> +			ret = prop_name_eq(fdt, offset, "device_type",
> +					   &prop, &len);
> +			if (ret < 0)
> +				return ret;
> +			else if (ret > 0 &&
> +				 strncmp(prop->data, type, len - 1) == 0)
> +			    return nodeoffset;
> +			break;
> +
> +		case FDT_END_NODE:
> +		case FDT_NOP:
> +			break;
> +
> +		case FDT_END:
> +			return -FDT_ERR_NOTFOUND;
> +
> +		default:
> +			return -FDT_ERR_BADSTRUCTURE;
> +		}
> +	}
> +}
> +
> +/*
> + * 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 otherwise:
> + *
> + *   nodeoffset - 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.
> + */
> +int fdt_find_compatible_node(const void *fdt, int nodeoffset,
> +			     const char *type, const char *compat)
> +{
> +	int offset;
> +
> +	offset = fdt_find_node_by_type(fdt, nodeoffset, type);
> +	if (offset < 0 || fdt_node_is_compatible(fdt, offset, compat))
> +		return offset;
> +
> +	return -FDT_ERR_NOTFOUND;
> +}
> +
> +/*
>   * Return the node offset of the node specified by:
>   *   parentoffset - starting place (0 to start at the root)
>   *   name         - name being searched for
> @@ -184,7 +323,6 @@ struct fdt_property *fdt_get_property(const void *fdt,
>  	int level = 0;
>  	uint32_t tag;
>  	struct fdt_property *prop;
> -	int namestroff;
>  	int offset, nextoffset;
>  	int err;
>  
> @@ -224,24 +362,11 @@ struct fdt_property *fdt_get_property(const void *fdt,
>  			if (level != 0)
>  				continue;
>  
> -			err = -FDT_ERR_BADSTRUCTURE;
> -			prop = fdt_offset_ptr_typed(fdt, offset, prop);
> -			if (! prop)
> -				goto fail;
> -			namestroff = fdt32_to_cpu(prop->nameoff);
> -			if (streq(fdt_string(fdt, namestroff), name)) {
> -				/* Found it! */
> -				int len = fdt32_to_cpu(prop->len);
> -				prop = fdt_offset_ptr(fdt, offset,
> -						      sizeof(*prop)+len);
> -				if (! prop)
> -					goto fail;
> -
> -				if (lenp)
> -					*lenp = len;
> -
> +			err = prop_name_eq(fdt, offset, name, &prop, lenp);
> +			if (err > 0)
>  				return prop;
> -			}
> +			else if (err < 0)
> +				goto fail;
>  			break;
>  
>  		case FDT_NOP:
> 
> 
> ------------------------------------------------------------------------
> 
> -------------------------------------------------------------------------
> This SF.net email is sponsored by DB2 Express
> Download DB2 Express C - the FREE version of DB2 express and take
> control of your XML. No limits. Just data. Click to get it now.
> http://sourceforge.net/powerbar/db2/
> 
> 
> ------------------------------------------------------------------------
> 
> _______________________________________________
> U-Boot-Users mailing list
> U-Boot-Users at lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/u-boot-users

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

[Jerry Van Baren]

Wolfgang Grandegger wrote:
> Hi Jerry,
> 
> any intention to merge these patches?
> 
> Thanks,
> 
> Wolfgang.

Hi Wolfgang,

Yes.  I have to apologize for my lack of responsiveness, the first two 
weeks of every month are busy for me.  Fortunately, we are in Week Three 
now.  ;-)

I have some minor cleanup of my own to do (line length VIOLATIONS ;-) 
and I'll merge in your patch and push it uphill.
   <http://en.wikipedia.org/wiki/Sisyphus>

Thanks,
gvb


> Wolfgang Grandegger wrote:
>> Hi Jerry,
>>
>> Jerry Van Baren wrote:
>>> Wolfgang Grandegger wrote:
>>>> Hi Jerry,
>>>>
>>>> before re-coding fdt_find_compatible_node(), some more comments.
>>>>
>>>> After browsing more carefully the FDT related code of "arch/powerpc"
>>>> I think we also need, apart from fdt_find_compatible_node() and
>>>> fdt_path_offset(), fdt_find_node_by_type() and maybe 
>>>> fdt_find_node_by_name(). These functions do a sequential scan of all 
>>>> devices starting at the beginning or after a specified node. They 
>>>> actually ignore the hierarchy. Do you agree?
>>>> BTW: any reason why not using the more compatible name 
>>>> fdt_find_node_by_path() for fdt_path_offset()?
>>>>
>>>> Wolfgang.
>>>
>>> Hi WolfganG,
>>>
>>> I'm not an expert, I just fake it on email ;-).  With that 
>>> disclaimer, I would agree with you WRT all the "find" functions.  The 
>>> original libfdt code does not support any "find" functions, so we 
>>> will need to add them.
>>>
>>> WRT to fdt_find_node_by_path() vs. fdt_path_offset(), I vaguely 
>>> recall some renames happening in the kernel source, but I cannot find 
>>> them so my memory likely is faulty[1].  I would be strongly in favor 
>>> of following the kernel's lead and renaming that function since we 
>>> are already divergent from the original libfdt.  The kernel's name is 
>>> a much better description.
>>
>> OK, I have attached two new patches replacing fdt_path_offset() with 
>> fdt_find_node_by_path() and implementing fdt_find_node_by_type() and 
>> fdt_find_compatible_node(). This version does not use static variables 
>> any more to scan the nodes without re-scanning, but looks for the end 
>> tag. I think it's (almost) good enough to be applied.
>>
>> Wolfgang.
>>
>>
>> ------------------------------------------------------------------------
>>
>> Replace fdt_node_offset() with fdt_find_node_by_path().
>>
>> From: Wolfgang Grandegger <wg@grandegger.com>
>>
>> The new name matches more closely the kernel's name, which is also
>> a much better description.
>> ---
>>
>>  board/mpc8360emds/mpc8360emds.c |    2 +-
>>  board/mpc8360emds/pci.c         |    2 +-
>>  common/cmd_fdt.c                |    6 +++---
>>  common/fdt_support.c            |   10 +++++-----
>>  cpu/mpc83xx/cpu.c               |    2 +-
>>  include/libfdt.h                |    2 +-
>>  libfdt/fdt_ro.c                 |    2 +-
>>  7 files changed, 13 insertions(+), 13 deletions(-)
>>
>> diff --git a/board/mpc8360emds/mpc8360emds.c 
>> b/board/mpc8360emds/mpc8360emds.c
>> index 562eb8b..3f87f09 100644
>> --- a/board/mpc8360emds/mpc8360emds.c
>> +++ b/board/mpc8360emds/mpc8360emds.c
>> @@ -681,7 +681,7 @@ ft_board_setup(void *blob, bd_t *bd)
>>      int nodeoffset;
>>      int tmp[2];
>>  
>> -    nodeoffset = fdt_path_offset (fdt, "/memory");
>> +    nodeoffset = fdt_find_node_by_path (fdt, "/memory");
>>      if (nodeoffset >= 0) {
>>          tmp[0] = cpu_to_be32(bd->bi_memstart);
>>          tmp[1] = cpu_to_be32(bd->bi_memsize);
>> diff --git a/board/mpc8360emds/pci.c b/board/mpc8360emds/pci.c
>> index 158effe..4c7a82b 100644
>> --- a/board/mpc8360emds/pci.c
>> +++ b/board/mpc8360emds/pci.c
>> @@ -311,7 +311,7 @@ ft_pci_setup(void *blob, bd_t *bd)
>>      int err;
>>      int tmp[2];
>>  
>> -    nodeoffset = fdt_path_offset (fdt, "/" OF_SOC "/pci at 8500");
>> +    nodeoffset = fdt_find_node_by_path (fdt, "/" OF_SOC "/pci at 8500");
>>      if (nodeoffset >= 0) {
>>          tmp[0] = cpu_to_be32(hose[0].first_busno);
>>          tmp[1] = cpu_to_be32(hose[0].last_busno);
>> diff --git a/common/cmd_fdt.c b/common/cmd_fdt.c
>> index 08fe351..7058197 100644
>> --- a/common/cmd_fdt.c
>> +++ b/common/cmd_fdt.c
>> @@ -177,7 +177,7 @@ int do_fdt (cmd_tbl_t * cmdtp, int flag, int argc, 
>> char *argv[])
>>          if (strcmp(pathp, "/") == 0) {
>>              nodeoffset = 0;
>>          } else {
>> -            nodeoffset = fdt_path_offset (fdt, pathp);
>> +            nodeoffset = fdt_find_node_by_path (fdt, pathp);
>>              if (nodeoffset < 0) {
>>                  /*
>>                    * Not found or something else bad happened.
>> @@ -306,7 +306,7 @@ int do_fdt (cmd_tbl_t * cmdtp, int flag, int argc, 
>> char *argv[])
>>              nodeoffset = 0;
>>              printf("/");
>>          } else {
>> -            nodeoffset = fdt_path_offset (fdt, pathp);
>> +            nodeoffset = fdt_find_node_by_path (fdt, pathp);
>>              if (nodeoffset < 0) {
>>                  /*
>>                   * Not found or something else bad happened.
>> @@ -405,7 +405,7 @@ int do_fdt (cmd_tbl_t * cmdtp, int flag, int argc, 
>> char *argv[])
>>          if (strcmp(argv[2], "/") == 0) {
>>              nodeoffset = 0;
>>          } else {
>> -            nodeoffset = fdt_path_offset (fdt, argv[2]);
>> +            nodeoffset = fdt_find_node_by_path (fdt, argv[2]);
>>              if (nodeoffset < 0) {
>>                  /*
>>                   * Not found or something else bad happened.
>> diff --git a/common/fdt_support.c b/common/fdt_support.c
>> index 69099c4..05bf939 100644
>> --- a/common/fdt_support.c
>> +++ b/common/fdt_support.c
>> @@ -92,7 +92,7 @@ int fdt_chosen(void *fdt, ulong initrd_start, ulong 
>> initrd_end, int force)
>>      /*
>>       * Find the "chosen" node.
>>       */
>> -    nodeoffset = fdt_path_offset (fdt, "/chosen");
>> +    nodeoffset = fdt_find_node_by_path (fdt, "/chosen");
>>  
>>      /*
>>       * If we have a "chosen" node already the "force the writing"
>> @@ -140,7 +140,7 @@ int fdt_chosen(void *fdt, ulong initrd_start, 
>> ulong initrd_end, int force)
>>          printf("libfdt: %s\n", fdt_strerror(err));
>>  #endif
>>  
>> -    nodeoffset = fdt_path_offset (fdt, "/cpus");
>> +    nodeoffset = fdt_find_node_by_path (fdt, "/cpus");
>>      if (nodeoffset >= 0) {
>>          clock = cpu_to_be32(bd->bi_intfreq);
>>          err = fdt_setprop(fdt, nodeoffset, "clock-frequency", &clock, 
>> 4);
>> @@ -148,7 +148,7 @@ int fdt_chosen(void *fdt, ulong initrd_start, 
>> ulong initrd_end, int force)
>>              printf("libfdt: %s\n", fdt_strerror(err));
>>      }
>>  #ifdef OF_TBCLK
>> -    nodeoffset = fdt_path_offset (fdt, "/cpus/" OF_CPU 
>> "/timebase-frequency");
>> +    nodeoffset = fdt_find_node_by_path (fdt, "/cpus/" OF_CPU 
>> "/timebase-frequency");
>>      if (nodeoffset >= 0) {
>>          clock = cpu_to_be32(OF_TBCLK);
>>          err = fdt_setprop(fdt, nodeoffset, "clock-frequency", &clock, 
>> 4);
>> @@ -185,7 +185,7 @@ int fdt_env(void *fdt)
>>       * See if we already have a "u-boot-env" node, delete it if so.
>>       * Then create a new empty node.
>>       */
>> -    nodeoffset = fdt_path_offset (fdt, "/u-boot-env");
>> +    nodeoffset = fdt_find_node_by_path (fdt, "/u-boot-env");
>>      if (nodeoffset >= 0) {
>>          err = fdt_del_node(fdt, nodeoffset);
>>          if (err < 0) {
>> @@ -305,7 +305,7 @@ int fdt_bd_t(void *fdt)
>>       * See if we already have a "bd_t" node, delete it if so.
>>       * Then create a new empty node.
>>       */
>> -    nodeoffset = fdt_path_offset (fdt, "/bd_t");
>> +    nodeoffset = fdt_find_node_by_path (fdt, "/bd_t");
>>      if (nodeoffset >= 0) {
>>          err = fdt_del_node(fdt, nodeoffset);
>>          if (err < 0) {
>> diff --git a/cpu/mpc83xx/cpu.c b/cpu/mpc83xx/cpu.c
>> index e934ba6..ef44581 100644
>> --- a/cpu/mpc83xx/cpu.c
>> +++ b/cpu/mpc83xx/cpu.c
>> @@ -460,7 +460,7 @@ ft_cpu_setup(void *blob, bd_t *bd)
>>      int  j;
>>  
>>      for (j = 0; j < (sizeof(fixup_props) / sizeof(fixup_props[0])); 
>> j++) {
>> -        nodeoffset = fdt_path_offset(fdt, fixup_props[j].node);
>> +        nodeoffset = fdt_find_node_by_path(fdt, fixup_props[j].node);
>>          if (nodeoffset >= 0) {
>>              err = (*fixup_props[j].set_fn)(blob, nodeoffset, 
>> fixup_props[j].prop, bd);
>>              if (err < 0)
>> diff --git a/include/libfdt.h b/include/libfdt.h
>> index f8bac73..e080028 100644
>> --- a/include/libfdt.h
>> +++ b/include/libfdt.h
>> @@ -77,7 +77,7 @@ int fdt_subnode_offset_namelen(const void *fdt, int 
>> parentoffset,
>>                     const char *name, int namelen);
>>  int fdt_subnode_offset(const void *fdt, int parentoffset, const char 
>> *name);
>>  
>> -int fdt_path_offset(const void *fdt, const char *path);
>> +int fdt_find_node_by_path(const void *fdt, const char *path);
>>  
>>  struct fdt_property *fdt_get_property(const void *fdt, int nodeoffset,
>>                        const char *name, int *lenp);
>> diff --git a/libfdt/fdt_ro.c b/libfdt/fdt_ro.c
>> index 4e2c325..e5518c6 100644
>> --- a/libfdt/fdt_ro.c
>> +++ b/libfdt/fdt_ro.c
>> @@ -129,7 +129,7 @@ int fdt_subnode_offset(const void *fdt, int 
>> parentoffset,
>>   * Searches for the node corresponding to the given path and returns the
>>   * offset of that node.
>>   */
>> -int fdt_path_offset(const void *fdt, const char *path)
>> +int fdt_find_node_by_path(const void *fdt, const char *path)
>>  {
>>      const char *end = path + strlen(path);
>>      const char *p = path;
>>
>>
>> ------------------------------------------------------------------------
>>
>> Add fdt_find_node_by_type() and fdt_find_compatible_node() to LIBFDT
>>
>> From: Wolfgang Grandegger <wg@grandegger.com>
>>
>>
>> ---
>>
>>  include/libfdt.h |    6 ++
>>  libfdt/fdt_ro.c  |  161 
>> ++++++++++++++++++++++++++++++++++++++++++++++++------
>>  2 files changed, 149 insertions(+), 18 deletions(-)
>>
>> diff --git a/include/libfdt.h b/include/libfdt.h
>> index e080028..340e89d 100644
>> --- a/include/libfdt.h
>> +++ b/include/libfdt.h
>> @@ -78,6 +78,12 @@ int fdt_subnode_offset_namelen(const void *fdt, int 
>> parentoffset,
>>  int fdt_subnode_offset(const void *fdt, int parentoffset, const char 
>> *name);
>>  
>>  int fdt_find_node_by_path(const void *fdt, const char *path);
>> +int fdt_find_node_by_type(const void *fdt, int nodeoffset, const char 
>> *type);
>> +
>> +int fdt_node_is_compatible(const void *fdt, int nodeoffset,
>> +               const char *compat);
>> +int fdt_find_compatible_node(const void *fdt, int nodeoffset,
>> +                 const char *type, const char *compat);
>>  
>>  struct fdt_property *fdt_get_property(const void *fdt, int nodeoffset,
>>                        const char *name, int *lenp);
>> diff --git a/libfdt/fdt_ro.c b/libfdt/fdt_ro.c
>> index e5518c6..e379fc2 100644
>> --- a/libfdt/fdt_ro.c
>> +++ b/libfdt/fdt_ro.c
>> @@ -48,6 +48,33 @@ static int offset_streq(const void *fdt, int offset,
>>  }
>>  
>>  /*
>> + * Checks if the property name matches.
>> + */
>> +static int prop_name_eq(const void *fdt, int offset, const char *name,
>> +            struct fdt_property **prop, int *lenp)
>> +{
>> +    int namestroff, len;
>> +
>> +    *prop = fdt_offset_ptr_typed(fdt, offset, *prop);
>> +    if (! *prop)
>> +        return -FDT_ERR_BADSTRUCTURE;
>> +
>> +    namestroff = fdt32_to_cpu((*prop)->nameoff);
>> +    if (streq(fdt_string(fdt, namestroff), name)) {
>> +        len = fdt32_to_cpu((*prop)->len);
>> +        *prop = fdt_offset_ptr(fdt, offset,
>> +                       sizeof(**prop) + len);
>> +        if (*prop) {
>> +            if (lenp)
>> +                *lenp = len;
>> +            return 1;
>> +        } else
>> +            return -FDT_ERR_BADSTRUCTURE;
>> +    }
>> +    return 0;
>> +}
>> +
>> +/*
>>   * Return a pointer to the string at the given string offset.
>>   */
>>  char *fdt_string(const void *fdt, int stroffset)
>> @@ -56,6 +83,118 @@ char *fdt_string(const void *fdt, int stroffset)
>>  }
>>  
>>  /*
>> + * 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, len;
>> +
>> +    cp = fdt_getprop(fdt, nodeoffset, "compatible", &cplen);
>> +    if (cp == NULL)
>> +        return 0;
>> +    while (cplen > 0) {
>> +        if (strncmp(cp, compat, strlen(compat)) == 0)
>> +            return 1;
>> +        len = strlen(cp) + 1;
>> +        cp += len;
>> +        cplen -= len;
>> +    }
>> +
>> +    return 0;
>> +}
>> +
>> +/*
>> + * Find a node by its device type property. On success, the offset of 
>> that
>> + * node is returned or an error code otherwise:
>> + *
>> + *   nodeoffset - 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.
>> + */
>> +int fdt_find_node_by_type(const void *fdt, int nodeoffset, const char 
>> *type)
>> +{
>> +    int offset, nextoffset;
>> +    struct fdt_property *prop;
>> +    uint32_t tag;
>> +    int len, ret;
>> +
>> +    CHECK_HEADER(fdt);
>> +
>> +    tag = fdt_next_tag(fdt, nodeoffset, &nextoffset, NULL);
>> +    if (tag != FDT_BEGIN_NODE)
>> +        return -FDT_ERR_BADOFFSET;
>> +    if (nodeoffset)
>> +        nodeoffset = 0;    /* start searching with next node */
>> +
>> +    while (1) {
>> +        offset = nextoffset;
>> +        tag = fdt_next_tag(fdt, offset, &nextoffset, NULL);
>> +
>> +        switch (tag) {
>> +        case FDT_BEGIN_NODE:
>> +            nodeoffset = offset;
>> +            break;
>> +
>> +        case FDT_PROP:
>> +            if (nodeoffset == 0)
>> +                break;
>> +            ret = prop_name_eq(fdt, offset, "device_type",
>> +                       &prop, &len);
>> +            if (ret < 0)
>> +                return ret;
>> +            else if (ret > 0 &&
>> +                 strncmp(prop->data, type, len - 1) == 0)
>> +                return nodeoffset;
>> +            break;
>> +
>> +        case FDT_END_NODE:
>> +        case FDT_NOP:
>> +            break;
>> +
>> +        case FDT_END:
>> +            return -FDT_ERR_NOTFOUND;
>> +
>> +        default:
>> +            return -FDT_ERR_BADSTRUCTURE;
>> +        }
>> +    }
>> +}
>> +
>> +/*
>> + * 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 otherwise:
>> + *
>> + *   nodeoffset - 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.
>> + */
>> +int fdt_find_compatible_node(const void *fdt, int nodeoffset,
>> +                 const char *type, const char *compat)
>> +{
>> +    int offset;
>> +
>> +    offset = fdt_find_node_by_type(fdt, nodeoffset, type);
>> +    if (offset < 0 || fdt_node_is_compatible(fdt, offset, compat))
>> +        return offset;
>> +
>> +    return -FDT_ERR_NOTFOUND;
>> +}
>> +
>> +/*
>>   * Return the node offset of the node specified by:
>>   *   parentoffset - starting place (0 to start at the root)
>>   *   name         - name being searched for
>> @@ -184,7 +323,6 @@ struct fdt_property *fdt_get_property(const void 
>> *fdt,
>>      int level = 0;
>>      uint32_t tag;
>>      struct fdt_property *prop;
>> -    int namestroff;
>>      int offset, nextoffset;
>>      int err;
>>  
>> @@ -224,24 +362,11 @@ struct fdt_property *fdt_get_property(const void 
>> *fdt,
>>              if (level != 0)
>>                  continue;
>>  
>> -            err = -FDT_ERR_BADSTRUCTURE;
>> -            prop = fdt_offset_ptr_typed(fdt, offset, prop);
>> -            if (! prop)
>> -                goto fail;
>> -            namestroff = fdt32_to_cpu(prop->nameoff);
>> -            if (streq(fdt_string(fdt, namestroff), name)) {
>> -                /* Found it! */
>> -                int len = fdt32_to_cpu(prop->len);
>> -                prop = fdt_offset_ptr(fdt, offset,
>> -                              sizeof(*prop)+len);
>> -                if (! prop)
>> -                    goto fail;
>> -
>> -                if (lenp)
>> -                    *lenp = len;
>> -
>> +            err = prop_name_eq(fdt, offset, name, &prop, lenp);
>> +            if (err > 0)
>>                  return prop;
>> -            }
>> +            else if (err < 0)
>> +                goto fail;
>>              break;
>>  
>>          case FDT_NOP:
>>
>>
>> ------------------------------------------------------------------------
>>
>> -------------------------------------------------------------------------
>> This SF.net email is sponsored by DB2 Express
>> Download DB2 Express C - the FREE version of DB2 express and take
>> control of your XML. No limits. Just data. Click to get it now.
>> http://sourceforge.net/powerbar/db2/
>>
>>
>> ------------------------------------------------------------------------
>>
>> _______________________________________________
>> U-Boot-Users mailing list
>> U-Boot-Users at lists.sourceforge.net
>> https://lists.sourceforge.net/lists/listinfo/u-boot-users
> 
> 
> ______________________________________________________________________
> CAUTION: This message was sent via the Public Internet and its 
> authenticity cannot be guaranteed.
> ______________________________________________________

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

[Jerry Van Baren]

Wolfgang Grandegger wrote:
> Hi Jerry,
> 
> Jerry Van Baren wrote:
>> Wolfgang Grandegger wrote:
>>> Hi Jerry,
>>>
>>> before re-coding fdt_find_compatible_node(), some more comments.
>>>
>>> After browsing more carefully the FDT related code of "arch/powerpc"
>>> I think we also need, apart from fdt_find_compatible_node() and
>>> fdt_path_offset(), fdt_find_node_by_type() and maybe 
>>> fdt_find_node_by_name(). These functions do a sequential scan of all 
>>> devices starting at the beginning or after a specified node. They 
>>> actually ignore the hierarchy. Do you agree?
>>> BTW: any reason why not using the more compatible name 
>>> fdt_find_node_by_path() for fdt_path_offset()?
>>>
>>> Wolfgang.
>>
>> Hi WolfganG,
>>
>> I'm not an expert, I just fake it on email ;-).  With that disclaimer, 
>> I would agree with you WRT all the "find" functions.  The original 
>> libfdt code does not support any "find" functions, so we will need to 
>> add them.
>>
>> WRT to fdt_find_node_by_path() vs. fdt_path_offset(), I vaguely recall 
>> some renames happening in the kernel source, but I cannot find them so 
>> my memory likely is faulty[1].  I would be strongly in favor of 
>> following the kernel's lead and renaming that function since we are 
>> already divergent from the original libfdt.  The kernel's name is a 
>> much better description.
> 
> OK, I have attached two new patches replacing fdt_path_offset() with 
> fdt_find_node_by_path() and implementing fdt_find_node_by_type() and 
> fdt_find_compatible_node(). This version does not use static variables 
> any more to scan the nodes without re-scanning, but looks for the end 
> tag. I think it's (almost) good enough to be applied.
> 
> Wolfgang.
> 
> 
> ------------------------------------------------------------------------
> 
> Replace fdt_node_offset() with fdt_find_node_by_path().
> 
> From: Wolfgang Grandegger <wg@grandegger.com>
> 
> The new name matches more closely the kernel's name, which is also
> a much better description.
> ---
> 
>  board/mpc8360emds/mpc8360emds.c |    2 +-
>  board/mpc8360emds/pci.c         |    2 +-
>  common/cmd_fdt.c                |    6 +++---
>  common/fdt_support.c            |   10 +++++-----
>  cpu/mpc83xx/cpu.c               |    2 +-
>  include/libfdt.h                |    2 +-
>  libfdt/fdt_ro.c                 |    2 +-
>  7 files changed, 13 insertions(+), 13 deletions(-)

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.

Your patches have a From: line rather than a Signed-off-by: line, I 
presume it is OK (and desirable) for me to change it into a 
Signed-off-by: line (I'll add my own Acked-by: line).

Thanks,
gvb

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

[Wolfgang Grandegger]

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?

> Your patches have a From: line rather than a Signed-off-by: line, I 
> presume it is OK (and desirable) for me to change it into a 
> Signed-off-by: line (I'll add my own Acked-by: line).

That's fine, of course.

Wolfgang.

[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

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

[Wolfgang Grandegger]

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

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.