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.
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 fromfdt_find_compatible_node()andfdt_path_offset(),fdt_find_node_by_type()and maybefdt_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 namefdt_find_node_by_path()forfdt_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.
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 fromfdt_find_compatible_node()andfdt_path_offset(),fdt_find_node_by_type()and maybefdt_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 namefdt_find_node_by_path()forfdt_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
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 fromfdt_find_compatible_node()andfdt_path_offset(),fdt_find_node_by_type()and maybefdt_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 namefdt_find_node_by_path()forfdt_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()withfdt_find_node_by_path()and implementingfdt_find_node_by_type()andfdt_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()withfdt_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@@ -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@@ -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@@ -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@@ -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@@ -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@@ -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@@ -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()andfdt_find_compatible_node()to LIBFDTFrom: 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@@ -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@@ -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
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 fromfdt_find_compatible_node()andfdt_path_offset(),fdt_find_node_by_type()and maybefdt_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 namefdt_find_node_by_path()forfdt_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()withfdt_find_node_by_path()and implementingfdt_find_node_by_type()andfdt_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()withfdt_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@@ -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@@ -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@@ -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@@ -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@@ -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@@ -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@@ -129,7 +129,7 @@ int fdt_subnode_offset(const void *fdt, intparentoffset, * 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()andfdt_find_compatible_node()to LIBFDTFrom: 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@@ -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@@ -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.
+ */
+intfdt_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.
______________________________________________________
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 fromfdt_find_compatible_node()andfdt_path_offset(),fdt_find_node_by_type()and maybefdt_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 namefdt_find_node_by_path()forfdt_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()withfdt_find_node_by_path()and implementingfdt_find_node_by_type()andfdt_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()withfdt_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
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.cwhich yourfdt_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.
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.cwhich yourfdt_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) {
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
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.cwhich yourfdt_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) {
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()intofdt_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.