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

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

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

Hi Wolfgang,

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

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

Thanks,
gvb


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