Re: [XEN][PATCH v9 08/19] xen/device-tree: Add device_tree_find_node_by_path() to find nodes in device tree

[Julien Grall <julien@xxxxxxx>]

Hi Vikram,

On 19/08/2023 01:28, Vikram Garhwal wrote:
> Add device_tree_find_node_by_path() to find a matching node with path for a
> dt_device_node.
>
> Reason behind this function:
>      Each time overlay nodes are added using .dtbo, a new fdt(memcpy of
Typo: missing space before (.

>      device_tree_flattened) is created and updated with overlay nodes. This
>      updated fdt is further unflattened to a dt_host_new. Next, we need to find
>      the overlay nodes in dt_host_new, find the overlay node's parent in dt_host
>      and add the nodes as child under their parent in the dt_host. Thus we need
>      this function to search for node in different unflattened device trees.
>
> Also, make dt_find_node_by_path() static inline.
>
> Signed-off-by: Vikram Garhwal <vikram.garhwal@xxxxxxx>
> ---
> Changes from v7:
>      Rename device_tree_find_node_by_path() to dt_find_node_by_path_from().
>      Fix indentation.
> ---
> ---
>   xen/common/device_tree.c      |  5 +++--
>   xen/include/xen/device_tree.h | 18 ++++++++++++++++--
>   2 files changed, 19 insertions(+), 4 deletions(-)
>
> diff --git a/xen/common/device_tree.c b/xen/common/device_tree.c
> index 67e9b6de3e..0f10037745 100644
> --- a/xen/common/device_tree.c
> +++ b/xen/common/device_tree.c
> @@ -358,11 +358,12 @@ struct dt_device_node *dt_find_node_by_type(struct 
> dt_device_node *from,
>       return np;
>   }
>   
> -struct dt_device_node *dt_find_node_by_path(const char *path)
> +struct dt_device_node *dt_find_node_by_path_from(struct dt_device_node *from,
> +                                                 const char *path)
Any change this both 'from' and the return can be const?

>   {
>       struct dt_device_node *np;
>   
> -    dt_for_each_device_node(dt_host, np)
> +    dt_for_each_device_node(from, np)
>           if ( np->full_name && (dt_node_cmp(np->full_name, path) == 0) )
>               break;
>   
> diff --git a/xen/include/xen/device_tree.h b/xen/include/xen/device_tree.h
> index 5941599eff..e507658b23 100644
> --- a/xen/include/xen/device_tree.h
> +++ b/xen/include/xen/device_tree.h
> @@ -568,13 +568,27 @@ struct dt_device_node *dt_find_node_by_type(struct 
> dt_device_node *from,
>   struct dt_device_node *dt_find_node_by_alias(const char *alias);
>   
>   /**
> - * dt_find_node_by_path - Find a node matching a full DT path
> + * dt_find_node_by_path_from - Generic function to find a node matching the
> + * full DT path for any given unflatten device tree
> + * @from: The device tree node to start searching from
>    * @path: The full path to match
>    *
>    * Returns a node pointer.
>    */
> -struct dt_device_node *dt_find_node_by_path(const char *path);
> +struct dt_device_node *
> +                    dt_find_node_by_path_from(struct dt_device_node *from,
> +                                                  const char *path);
Coding style: The indentation look rather odd. I am not sure it will 
render properly here. But one style that is closer to the rest of the 
file and Xen is:
struct dt_device_node *dt_find_node_by_path_from(struct dt_device_node 
*from,
                                                 const char *path);

Where the return type, function name and first parameter is one line and 
the second parameter is on the second line with the type aligned with 
the type of the first parameter.
>   
> +/**
> + * dt_find_node_by_path - Find a node matching a full DT path in dt_host
> + * @path: The full path to match
> + *
> + * Returns a node pointer.
> + */
> +static inline struct dt_device_node *dt_find_node_by_path(const char *path)
> +{
> +    return dt_find_node_by_path_from(dt_host, path);
> +}
>   
>   /**
>    * dt_find_node_by_gpath - Same as dt_find_node_by_path but retrieve the
Cheers,

--
Julien Grall