Hi Vikram,
On 07/12/2022 07:15, Vikram Garhwal wrote:
>
>
> Add _dt_find_by_path() to find a matching node with path for a dt_device_node.
Here and in commit title you say _dt_find_by_path but you introduce
device_tree_find_node_by_path.
Also, it would be beneficial to state why such change is needed.
>
> Signed-off-by: Vikram Garhwal <vikram.garh...@amd.com>
> ---
> xen/common/device_tree.c | 5 +++--
> xen/include/xen/device_tree.h | 16 ++++++++++++++--
> 2 files changed, 17 insertions(+), 4 deletions(-)
>
> diff --git a/xen/common/device_tree.c b/xen/common/device_tree.c
> index 6518eff9b0..acf26a411d 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 *device_tree_find_node_by_path(struct dt_device_node
> *dt,
> + const char *path)
> {
> struct dt_device_node *np;
>
> - dt_for_each_device_node(dt_host, np)
> + dt_for_each_device_node(dt, 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 bde46d7120..51e251b0b4 100644
> --- a/xen/include/xen/device_tree.h
> +++ b/xen/include/xen/device_tree.h
> @@ -537,13 +537,25 @@ 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 - Find a node matching a full DT path
This description and ...
> + * @dt_node: The device tree to search
> * @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
> *dt,
> + const char *path);
>
> +/**
> + * dt_find_node_by_path - Find a node matching a full DT path
... this are identical. I think you should describe the difference.
The function names are also very similar and can be confused but I won't oppose
it.
I will leave the decision to maintainers.
> + * @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
> --
> 2.17.1
>
>
~Michal
