[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [XEN][PATCH v7 04/19] common/device_tree: change __unflatten_device_tree() type



Hi,

On 17/08/2023 00:49, Vikram Garhwal wrote:
On Tue, Jun 06, 2023 at 12:09:35PM -0700, Vikram Garhwal wrote:
Hi Julien,
Will update the commit message regarding why we need to export this for dtbo
programming.

On 6/5/23 12:04 PM, Julien Grall wrote:
Hi,

Title:

'type' is a bit confusing here. How about "Export
__unflatten_device_tre()"?

On 02/06/2023 01:48, Vikram Garhwal wrote:
Following changes are done to __unflatten_device_tree():
      1. __unflatten_device_tree() is renamed to unflatten_device_tree().
      2. Remove __init and static function type.

As there is no external caller yet, please explain why you want to
export the function.
Update the commit message in v8.

Cheers,


Signed-off-by: Vikram Garhwal <vikram.garhwal@xxxxxxx>
Reviewed-by: Henry Wang <Henry.Wang@xxxxxxx>
---
   xen/common/device_tree.c      | 9 ++++-----
   xen/include/xen/device_tree.h | 5 +++++
   2 files changed, 9 insertions(+), 5 deletions(-)

diff --git a/xen/common/device_tree.c b/xen/common/device_tree.c
index bbdab07596..16b4b4e946 100644
--- a/xen/common/device_tree.c
+++ b/xen/common/device_tree.c
@@ -2083,7 +2083,7 @@ static unsigned long unflatten_dt_node(const
void *fdt,
   }
     /**
- * __unflatten_device_tree - create tree of device_nodes from flat blob
+ * unflatten_device_tree - create tree of device_nodes from flat blob
    *
    * unflattens a device-tree, creating the
    * tree of struct device_node. It also fills the "name" and "type"
@@ -2092,8 +2092,7 @@ static unsigned long unflatten_dt_node(const
void *fdt,
    * @fdt: The fdt to expand
    * @mynodes: The device_node tree created by the call
    */
-static int __init __unflatten_device_tree(const void *fdt,
-                                          struct dt_device_node
**mynodes)
+int unflatten_device_tree(const void *fdt, struct dt_device_node
**mynodes)
   {
       unsigned long start, mem, size;
       struct dt_device_node **allnextp = mynodes;
@@ -2230,10 +2229,10 @@ dt_find_interrupt_controller(const struct
dt_device_match *matches)
     void __init dt_unflatten_host_device_tree(void)
   {
-    int error = __unflatten_device_tree(device_tree_flattened,
&dt_host);
+    int error = unflatten_device_tree(device_tree_flattened, &dt_host);
         if ( error )
-        panic("__unflatten_device_tree failed with error %d\n", error);
+        panic("unflatten_device_tree failed with error %d\n", error);
         dt_alias_scan();

This function doesn't seem to be called in the case of the overlay
device-tree. Does this mean that it will never contain any alias?

I haven't seen any overlay example for FPGA use cases where alias are added.
I have added a TODO in patch 16/19 where we are calling unflatten_device_tree().
   }
diff --git a/xen/include/xen/device_tree.h
b/xen/include/xen/device_tree.h
index c2eada7489..2c35c0d391 100644
--- a/xen/include/xen/device_tree.h
+++ b/xen/include/xen/device_tree.h
@@ -178,6 +178,11 @@ int device_tree_for_each_node(const void *fdt,
int node,
    */
   void dt_unflatten_host_device_tree(void);
   +/**
+ * unflatten any device tree.

Most of the exported function in device_tre.h have documentation. Can
you do the same here?
Done!

+ */
+int unflatten_device_tree(const void *fdt, struct dt_device_node
**mynodes);

NIT: From an external interface perspective, do we actually need to pass
an extra pointer? Can't we instead, return the pointer?
We will also need the error from the function. So, that's why i kept it as it 
is.

This can be achieved by using the ERR_PTR() infrastructure which I would rather prefer over passing an extra pointer here.

Cheers,

--
Julien Grall



 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.