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

Re: [Xen-devel] [PATCH v6 2/8] xen/arm: make process_memory_node a device_tree_node_func


On 16/08/2019 00:36, Stefano Stabellini wrote:
Change the signature of process_memory_node to match
device_tree_node_func. Thanks to this change, the next patch will be
able to use device_tree_for_each_node to call process_memory_node on all
the children of a provided node.

Return error if there is no reg property or if nr_banks is reached. Let
the caller deal with the error.

This sentence does not match the change below. Only 2 of the new error paths are described here.

Add a printk when device tree parsing fails.

Signed-off-by: Stefano Stabellini <stefanos@xxxxxxxxxx>
Changes in v6:
- fix out of space check
- bring back printk when address_cells or size_cells are not properly set
- return -EINVAL in that case (different from reg missing)
- add printk when parsing fails
- return -ENOENT when memory size is 0

Changes in v5:
- return -ENOENT if address_cells or size_cells are not properly set

Changes in v4:
- return error if there is no reg propery, remove printk
- return error if nr_banks is reached

Changes in v3:
- improve commit message
- check return value of process_memory_node

Changes in v2:
- new
  xen/arch/arm/bootfdt.c | 29 ++++++++++++++++++-----------
  1 file changed, 18 insertions(+), 11 deletions(-)

diff --git a/xen/arch/arm/bootfdt.c b/xen/arch/arm/bootfdt.c
index f1614ef7fc..9dc2c1352d 100644
--- a/xen/arch/arm/bootfdt.c
+++ b/xen/arch/arm/bootfdt.c
@@ -130,9 +130,10 @@ int __init device_tree_for_each_node(const void *fdt, int 
      return 0;
-static void __init process_memory_node(const void *fdt, int node,
-                                       const char *name,
-                                       u32 address_cells, u32 size_cells)
+static int __init process_memory_node(const void *fdt, int node,
+                                      const char *name, int depth,
+                                      u32 address_cells, u32 size_cells,
+                                      void *data)
      const struct fdt_property *prop;
      int i;
@@ -145,15 +146,12 @@ static void __init process_memory_node(const void *fdt, 
int node,
          printk("fdt: node `%s': invalid #address-cells or #size-cells",
-        return;
+        return -EINVAL;
prop = fdt_get_property(fdt, node, "reg", NULL);
      if ( !prop )
-    {
-        printk("fdt: node `%s': missing `reg' property\n", name);
-        return;
-    }
+        return -ENOENT;
cell = (const __be32 *)prop->data;
      banks = fdt32_to_cpu(prop->len) / (reg_cells * sizeof (u32));
@@ -162,11 +160,15 @@ static void __init process_memory_node(const void *fdt, 
int node,
          device_tree_get_reg(&cell, address_cells, size_cells, &start, &size);
          if ( !size )
-            continue;
+            return -ENOENT;

I don't think we can treat the same way the lack of "regs" properties and a size of 0.

The former is expected as binding allow you to do it for reserved-memory. The latter is the user not writing the property correctly. So ignoring the latter will result to Xen potentially missing some reserved-regions (not great!).

So, similar to #address-cells/#size-cells discussion, we should return an error we are able to distinguish. Probably -EINVAL.


Julien Grall

Xen-devel mailing list



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