Hi Julien,
On Tue, Aug 22, 2023 at 08:21:17PM +0100, Julien Grall wrote:
> 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.garh...@amd.com>
>
> > ---
> > 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?
Doing this will also need changes in dt_find_node_by_path() and
dt_for_each_device_node(). This function calls both of these.
>
> > {
> > 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.
Will do!
>
> > +/**
> > + * 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