Skip to contentSkip to conversation

fdt_find_compatible_node() and friends

U-Boot-Users Wolfgang Grandegger 3 participants 9 messages May 3, 2007 – May 17, 2007

lore
Share & tools
lore
#0 lore raw

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.

#1 lore raw
Quoted 13 lines · 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()?

Quoted 1 line

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.

#2 lore raw

Hi Jerry,

Quoted 22 lines · Jerry Van Baren
Quoted 13 lines · 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()?

Quoted 1 line

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.

Quoted 6 lines

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

#3 lore raw

Hi Jerry,

any intention to merge these patches?

Thanks,

Wolfgang.

Quoted 449 lines · Wolfgang Grandegger

Hi Jerry,

Quoted 23 lines · Jerry Van Baren
Quoted 13 lines · 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()?

Quoted 1 line

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.

Quoted 6 lines

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 <[email protected]>

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(-)
board/mpc8360emds/mpc8360emds.c +1 -1
index 562eb8b..3f87f09 100644
@@ -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);
board/mpc8360emds/pci.c +1 -1
index 158effe..4c7a82b 100644
@@ -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);
common/cmd_fdt.c +3 -3
index 08fe351..7058197 100644
@@ -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.
common/fdt_support.c +5 -5
index 69099c4..05bf939 100644
@@ -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) {
cpu/mpc83xx/cpu.c +1 -1
index e934ba6..ef44581 100644
@@ -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)
include/libfdt.h +1 -1
index f8bac73..e080028 100644
@@ -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);
libfdt/fdt_ro.c +1 -2
index 4e2c325..e5518c6 100644
@@ -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 <[email protected]>

---

 include/libfdt.h |    6 ++
 libfdt/fdt_ro.c  |  161 ++++++++++++++++++++++++++++++++++++++++++++++++------
 2 files changed, 149 insertions(+), 18 deletions(-)
include/libfdt.h +6 -0
index e080028..340e89d 100644
@@ -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);
libfdt/fdt_ro.c +143 -20
index e5518c6..e379fc2 100644
@@ -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

#4 lore raw
Quoted 7 lines · Wolfgang Grandegger

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

Quoted 487 lines
Quoted 474 lines · Wolfgang Grandegger

Hi Jerry,

Quoted 23 lines · Jerry Van Baren
Quoted 13 lines · 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()?

Quoted 1 line

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.

Quoted 6 lines

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 <[email protected]>

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(-)
board/mpc8360emds/mpc8360emds.c +1 -1
index 562eb8b..3f87f09 100644
@@ -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);
board/mpc8360emds/pci.c +1 -1
index 158effe..4c7a82b 100644
@@ -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);
common/cmd_fdt.c +1 -1
index 08fe351..7058197 100644
@@ -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.

common/cmd_fdt.c +1 -1
@@ -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.

common/cmd_fdt.c +1 -1
@@ -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.

common/fdt_support.c +1 -1
index 69099c4..05bf939 100644
@@ -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"

common/fdt_support.c +1 -1
@@ -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);

common/fdt_support.c +1 -1
@@ -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);

common/fdt_support.c +2 -2
@@ -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) {
cpu/mpc83xx/cpu.c +1 -1
index e934ba6..ef44581 100644
@@ -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)
include/libfdt.h +1 -1
index f8bac73..e080028 100644
@@ -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);
libfdt/fdt_ro.c +1 -1
index 4e2c325..e5518c6 100644
@@ -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 <[email protected]>

---

 include/libfdt.h |    6 ++
 libfdt/fdt_ro.c  |  161 
++++++++++++++++++++++++++++++++++++++++++++++++------
 2 files changed, 149 insertions(+), 18 deletions(-)
include/libfdt.h +6 -0
index e080028..340e89d 100644
@@ -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);
libfdt/fdt_ro.c +56 -0
index e5518c6..e379fc2 100644
@@ -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)

libfdt/fdt_ro.c +51 -0
+{
+    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.
+ */

libfdt/fdt_ro.c +17 -18
+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/

Quoted 6 lines

------------------------------------------------------------------------

_______________________________________________
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.
______________________________________________________

#5 lore raw
In reply to Wolfgang Grandegger
Quoted 52 lines · Wolfgang Grandegger

Hi Jerry,

Quoted 23 lines · Jerry Van Baren
Quoted 13 lines · 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()?

Quoted 1 line

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.

Quoted 6 lines

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 <[email protected]>

The new name matches more closely the kernel's name, which is also a much better description.
---

Quoted 8 lines
 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

#6 lore raw

Hi Jerry,

Jerry Van Baren wrote:
[...]

Quoted 8 lines

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?

Quoted 3 lines

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.

#7 lore raw
Quoted 18 lines · Wolfgang Grandegger

Hi Jerry,

Jerry Van Baren wrote:
[...]

Quoted 8 lines

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]

Quoted 1 line

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

#8 lore raw
Quoted 28 lines · Jerry Van Baren
Quoted 18 lines · Wolfgang Grandegger

Hi Jerry,

Jerry Van Baren wrote:
[...]

Quoted 8 lines

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]

Quoted 1 line

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.

Quoted 4 lines

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 '"/"'
Quoted 10 lines

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.

Quoted 11 lines

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.