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

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.

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.

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.

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@8500");
- nodeoffset = fdt_find_node_by_path (fdt, "/" OF_SOC "/pci@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);
if (nodeoffset >= 0) { err = (*fixup_props[j].set_fn)(blob, nodeoffset, fixup_props[j].prop, bd); if (err < 0)nodeoffset = fdt_find_node_by_path(fdt, fixup_props[j].node);
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@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/u-boot-users

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@8500");
- nodeoffset = fdt_find_node_by_path (fdt, "/" OF_SOC "/pci@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@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. ______________________________________________________

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

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.

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

Jerry Van Baren wrote:
Wolfgang Grandegger wrote:
Hi Jerry,
Jerry Van Baren wrote: [...]
Hi Wolfgang G,
I've applied your patches to my local (working) repository and will push the changes tonight (my tonight, your tomorrow ;-). I created a subroutine out of three snippets of code in cmd_fdt.c which your fdt_find_node_by_path() change fixed up so I had to fix one line in the new subroutine by hand. Not bad at all considering the changes I made in that file.
Thanks. In the meantime I observed, that
fdt_find_node_by_path(fdt, 0, "/");
returns an error. Is that by purpose?
[snip]
Wolfgang.
Hmm, interesting observation. The character '/' is a figment of our imagination, offset 0 in the tree is the root node. The character '/' is the path separator and doesn't actually exist anywhere in the fdt - when parsing paths, the stuff between the slashes is searched for and the slashes themselves are skipped over.
What you asked in the above call is a node with no name under the root node, i.e. "//" in human-speak. That wasn't found, of course. On the other hand, it is a pretty obvious "mistake" (I was guilty of making the same mistake when I first tried to use fdt_path_offset()).
And it is frequently used in the kernel as the following command reveals:
$ find . -name '*.c' | xargs grep find_node_by_path | grep '"/"'
It seems like I was forever doing a conditional:
59 if (strcmp(pathp, "/") == 0) { 60 nodeoffset = 0; 61 } else { 62 nodeoffset = fdt_path_offset (fdt, pathp); 63 if (nodeoffset < 0) {
I actually wanted to get the string for the property "model" and yes, fdt_getprop(fdt, 0, "model", NULL) does work.
Trivia: Your patch we are discussing changed exactly this fdt_path_offset() into fdt_find_node_by_path() - this is the one place your patch didn't apply, because I refactored the three conditionals into a single wrapper subroutine.
At the risk of being accused of codling our users, I would propose we add the equivalent "/" detection code (above) to fdt_find_node_by_path(), (I will do that tonight unless you beat me to it). It seems silly to have the caller replicate or wrap the conditional since it is going to be such a common idiom/mistake.
Thanks... and no hurry.
Wolfgang.
participants (3)
-
Jerry Van Baren
-
Jerry Van Baren
-
Wolfgang Grandegger