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

[Vikram Garhwal <vikram.garhwal@xxxxxxx>]

Hi,

On 6/5/23 12:12 PM, Julien Grall wrote:
> Hi,
>
> On 02/06/2023 01:48, Vikram Garhwal wrote:
> > Add device_tree_find_node_by_path() to find a matching node with path 
> > for a
> AFAICT, the only difference in name between the new function and the 
> existing one is "device_tree" vs "dt". The latter is just a shorthand 
> of "device tree", so it feels to me the name are a bit too similar.
> From my understanding, the main difference between the two functions 
> are that the current one is starting from root whereas the current one 
> is starting from a given node. So how about 
> "dt_find_node_by_path_from()"?
Thank you for the suggestion. This name was added in v3 as Luca Fancellu 
suggested to rename this function to "device_tree_find_node_by_path". I 
am okay with renaming it to dt_find_node_by_path_from().
Luca, Michal and Henry: Does the dt_find_node_by_path_from() name works 
for you?
Regards,
Vikram
> > dt_device_node.
> >
> > Reason behind this function:
> >      Each time overlay nodes are added using .dtbo, a new fdt(memcpy of
> >      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 v6:
> >      Rename "dt_node" to "from"
> > ---
> >   xen/common/device_tree.c      |  6 ++++--
> >   xen/include/xen/device_tree.h | 18 ++++++++++++++++--
> >   2 files changed, 20 insertions(+), 4 deletions(-)
> >
> > diff --git a/xen/common/device_tree.c b/xen/common/device_tree.c
> > index 16b4b4e946..c5250a1644 100644
> > --- a/xen/common/device_tree.c
> > +++ b/xen/common/device_tree.c
> > @@ -358,11 +358,13 @@ 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 *
> > +                    device_tree_find_node_by_path(struct 
> > dt_device_node *from,
> > +                                                  const char *path)
> >   {
> >       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 2c35c0d391..e239f7de26 100644
> > --- a/xen/include/xen/device_tree.h
> > +++ b/xen/include/xen/device_tree.h
> > @@ -561,13 +561,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
> > + * device_tree_find_node_by_path - 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 *
> > +                    device_tree_find_node_by_path(struct 
> > dt_device_node *from,
> > +                                                  const char *path);
> The indentation looks slightly odd. In general, if the return type is 
> on its own line, then the function name is not indented on the new line.
Will fix the indention.
> >   +/**
> > + * 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 device_tree_find_node_by_path(dt_host, path);
> > +}
> >     /**
> >    * dt_find_node_by_gpath - Same as dt_find_node_by_path but 
> > retrieve the
> Cheers,