Re: [Qemu-arm] [RFC PATCH 2/6] device_tree: add qemu_fdt_add_path

[Igor Mammedov]

On Wed,  4 Jul 2018 14:49:19 +0200
Andrew Jones <address@hidden> wrote:

> qemu_fdt_add_path works like qemu_fdt_add_subnode, except it
> also recursively adds any missing parent nodes.

Probably add here why new helper is need?

> 
> Cc: Peter Crosthwaite <address@hidden>
> Cc: Alexander Graf <address@hidden>
> Signed-off-by: Andrew Jones <address@hidden>
> ---
>  device_tree.c                | 24 ++++++++++++++++++++++++
>  include/sysemu/device_tree.h |  1 +
>  2 files changed, 25 insertions(+)
> 
> diff --git a/device_tree.c b/device_tree.c
> index 6d9c9726f66c..ad570a4dbe3a 100644
> --- a/device_tree.c
> +++ b/device_tree.c
> @@ -520,6 +520,30 @@ int qemu_fdt_add_subnode(void *fdt, const char *name)
>      return retval;
>  }
>  
> +int qemu_fdt_add_path(void *fdt, const char *path)
> +{
> +    char *parent;
> +    int offset;
> +
> +    offset = fdt_path_offset(fdt, path);
> +    if (offset < 0 && offset != -FDT_ERR_NOTFOUND) {
> +        error_report("%s Couldn't find node %s: %s", __func__, path,
> +                     fdt_strerror(offset));
> +        exit(1);
> +    }
> +
> +    if (offset != -FDT_ERR_NOTFOUND) {
> +        return offset;
> +    }
> +
> +    parent = g_strdup(path);
> +    strrchr(parent, '/')[0] = '\0';
> +    qemu_fdt_add_path(fdt, parent);
can it be implemented without recursion?

> +    g_free(parent);
> +
> +    return qemu_fdt_add_subnode(fdt, path);
> +}
> +
>  void qemu_fdt_dumpdtb(void *fdt, int size)
>  {
>      const char *dumpdtb = qemu_opt_get(qemu_get_machine_opts(), "dumpdtb");
> diff --git a/include/sysemu/device_tree.h b/include/sysemu/device_tree.h
> index c16fd69bc0b1..d62fc873a3ea 100644
> --- a/include/sysemu/device_tree.h
> +++ b/include/sysemu/device_tree.h
> @@ -101,6 +101,7 @@ uint32_t qemu_fdt_get_phandle(void *fdt, const char 
> *path);
>  uint32_t qemu_fdt_alloc_phandle(void *fdt);
>  int qemu_fdt_nop_node(void *fdt, const char *node_path);
>  int qemu_fdt_add_subnode(void *fdt, const char *name);

/**
 * qemu_fdt_add_path: ....
 ...
*/

It would be nice to have a doc comment here

> +int qemu_fdt_add_path(void *fdt, const char *path);
>  
>  #define qemu_fdt_setprop_cells(fdt, node_path, property, ...)                
>  \
>      do {                                                                     
>  \