| Message ID | 20230502233650.20121-9-vikram.garhwal@amd.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | dynamic node programming using overlay dtbo | expand |
Hi Vikram, > -----Original Message----- > Subject: [XEN][PATCH v6 08/19] xen/device-tree: Add > device_tree_find_node_by_path() to find nodes in device tree > > 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. > > Also, make 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(-) > [...] > /** > - * 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 I noticed that you missed Michal's comment here about renaming the "dt_node" here to "dt" to match below function prototype... > * @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, ...here. I personally agree with Michal so I think please fix the comment to keep consistency. The rest of the patch looks good to me, so as long as you fixed this, you can have my: Reviewed-by: Henry Wang <Henry.Wang@arm.com> Kind regards, Henry
Henry Wang <Henry.Wang@arm.com>于2023年5月4日 周四12:23写道： > Hi Vikram, > > > -----Original Message----- > > Subject: [XEN][PATCH v6 08/19] xen/device-tree: Add > > device_tree_find_node_by_path() to find nodes in device tree > > > > 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. > > > > Also, make 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(-) > > > > [...] > > > /** > > - * 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 > > I noticed that you missed Michal's comment here about renaming the > "dt_node" here to "dt" to match below function prototype... > > > * @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, > > ...here. I personally agree with Michal so I think please fix the comment > to keep consistency. > > The rest of the patch looks good to me, so as long as you fixed this, you > can have my: > > Reviewed-by: Henry Wang <Henry.Wang@arm.com> > > Kind regards, > Henry > > >
On 04/05/2023 06:23, Henry Wang wrote: > > > Hi Vikram, > >> -----Original Message----- >> Subject: [XEN][PATCH v6 08/19] xen/device-tree: Add >> device_tree_find_node_by_path() to find nodes in device tree >> >> 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. >> >> Also, make 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(-) >> > > [...] > >> /** >> - * 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 > > I noticed that you missed Michal's comment here about renaming the > "dt_node" here to "dt" to match below function prototype... This is one thing. The other is that in v5 you said this is to be a generic function where you can search from a middle of a device tree. This means that the parameter should be named "node" or "from" and the description needs to say "The node to start searching from" + seeing the lack of ->allnext you can mention that this is inclusive (i.e. the passed node will also be searched). ~Michal
Hi Henry & Michal, On 5/9/23 4:29 AM, Michal Orzel wrote: > > On 04/05/2023 06:23, Henry Wang wrote: >> >> Hi Vikram, >> >>> -----Original Message----- >>> Subject: [XEN][PATCH v6 08/19] xen/device-tree: Add >>> device_tree_find_node_by_path() to find nodes in device tree >>> >>> 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. >>> >>> Also, make 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(-) >>> >> [...] >> >>> /** >>> - * 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 >> I noticed that you missed Michal's comment here about renaming the >> "dt_node" here to "dt" to match below function prototype... > This is one thing. The other is that in v5 you said this is to be a generic function > where you can search from a middle of a device tree. This means that the parameter should be > named "node" or "from" and the description needs to say "The node to start searching from" + > seeing the lack of ->allnext you can mention that this is inclusive (i.e. the passed node will also be searched). Changed this for v7. Will send it out soon. @Henry, i didn't add reviewed-by as the patch is bit changed with renaming. Can you please review v7 and give your feedback. > > ~Michal
Hi Vikram, > -----Original Message----- > Hi Henry & Michal, > Changed this for v7. Will send it out soon. > > @Henry, i didn't add reviewed-by as the patch is bit changed with > renaming. Can you please review v7 and give your feedback. Thanks for the reminder, yes I would be more than happy to review the v7 series once you send it. Kind regards, Henry > >
diff --git a/xen/common/device_tree.c b/xen/common/device_tree.c index 47ab2f7940..426a809f42 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 eef0335b79..d6366d3dac 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. Also, make 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(-)