| Message ID | 20230411191636.26926-7-vikram.garhwal@amd.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | dynamic node programming using overlay dtbo | expand |
Hi Vikram, On 11/04/2023 21:16, 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 > 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. You do not mention making dt_find_node_by_path() static inline. > > Signed-off-by: Vikram Garhwal <vikram.garhwal@amd.com> > --- > xen/common/device_tree.c | 5 +++-- > xen/include/xen/device_tree.h | 17 +++++++++++++++-- > 2 files changed, 18 insertions(+), 4 deletions(-) > > diff --git a/xen/common/device_tree.c b/xen/common/device_tree.c > index bf847b2584..507b4ac5b6 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 58ac12abe3..998f972ebc 100644 > --- a/xen/include/xen/device_tree.h > +++ b/xen/include/xen/device_tree.h > @@ -534,13 +534,26 @@ 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 > + * @dt_node: The device tree to search This should be @dt to match the parameter. Also, shouldn't the description say: "the node to start searching from" or "device tree root node" FWICS, you expect to pass a root node as dt node. However, in device_tree_find_node_by_path() you do not check if a provided node is a root node or not (e.g. no parent). Is this intended? ~Michal
Hi Michal, On 4/13/23 6:40 AM, Michal Orzel wrote: > Hi Vikram, > > On 11/04/2023 21:16, 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 >> 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. > You do not mention making dt_find_node_by_path() static inline. Will add a comment about it in v6. > >> Signed-off-by: Vikram Garhwal <vikram.garhwal@amd.com> >> --- >> xen/common/device_tree.c | 5 +++-- >> xen/include/xen/device_tree.h | 17 +++++++++++++++-- >> 2 files changed, 18 insertions(+), 4 deletions(-) >> >> diff --git a/xen/common/device_tree.c b/xen/common/device_tree.c >> index bf847b2584..507b4ac5b6 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 58ac12abe3..998f972ebc 100644 >> --- a/xen/include/xen/device_tree.h >> +++ b/xen/include/xen/device_tree.h >> @@ -534,13 +534,26 @@ 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 >> + * @dt_node: The device tree to search > This should be @dt to match the parameter. Also, shouldn't the description say: > "the node to start searching from" > or > "device tree root node" > > FWICS, you expect to pass a root node as dt node. However, in device_tree_find_node_by_path() > you do not check if a provided node is a root node or not (e.g. no parent). Is this intended? Yeah, intent was to write a generic function where we can search from middle of a device_tree as long as we have the start node from where to search. But yeah so far for dynamic programming it's been called for a root nodes only. > > ~Michal
diff --git a/xen/common/device_tree.c b/xen/common/device_tree.c index bf847b2584..507b4ac5b6 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 58ac12abe3..998f972ebc 100644 --- a/xen/include/xen/device_tree.h +++ b/xen/include/xen/device_tree.h @@ -534,13 +534,26 @@ 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 + * @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 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
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 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. Signed-off-by: Vikram Garhwal <vikram.garhwal@amd.com> --- xen/common/device_tree.c | 5 +++-- xen/include/xen/device_tree.h | 17 +++++++++++++++-- 2 files changed, 18 insertions(+), 4 deletions(-)