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

Re: [Xen-devel] [PATCH 3/8] device tree: add device_tree_for_each_node()



On Tue, 2012-02-28 at 16:54 +0000, David Vrabel wrote:
> From: David Vrabel <david.vrabel@xxxxxxxxxx>
> 
> Add device_tree_for_each_node() to iterate over all nodes in a flat
> device tree.  Use this in device_tree_early_init().
> 
> Signed-off-by: David Vrabel <david.vrabel@xxxxxxxxxx>
> ---
>  xen/common/device_tree.c      |   71 
> ++++++++++++++++++++++++++---------------
>  xen/include/xen/device_tree.h |    8 +++++
>  2 files changed, 53 insertions(+), 26 deletions(-)
> 
> diff --git a/xen/common/device_tree.c b/xen/common/device_tree.c
> index e5df748..e95ff3c 100644
> --- a/xen/common/device_tree.c
> +++ b/xen/common/device_tree.c
> @@ -66,7 +66,42 @@ static u32 __init prop_by_name_u32(const void *fdt, int 
> node, const char *prop_n
>      return fdt32_to_cpu(*(uint32_t*)prop->data);
>  }
>  
> -static void __init process_memory_node(const void *fdt, int node,
> +#define MAX_DEPTH 16
> +
> +/**
> + * device_tree_for_each_node - iterate over all device tree nodes
> + * @fdt: flat device tree.
> + * @func: function to call for each node.
> + * @data: data to pass to @func.
> + */
> +int device_tree_for_each_node(const void *fdt,
> +                              device_tree_node_func func, void *data)
> +{
> +    int node;
> +    int depth;
> +    u32 address_cells[MAX_DEPTH];
> +    u32 size_cells[MAX_DEPTH];
> +    int ret;
> +
> +    for (node = 0, depth = 0;
> +         node >=0 && depth >= 0;
> +         node = fdt_next_node(fdt, node, &depth))

Xen coding style has spaces both sides of the outermost "(" and ")" of a
for loop (similarly for if / while etc)

> +    {
> +        if (depth >= MAX_DEPTH)

Some sort of warning or error message would be useful here?

> +            continue;
> +
> +        address_cells[depth] = prop_by_name_u32(fdt, node, "#address-cells");
> +        size_cells[depth] = prop_by_name_u32(fdt, node, "#size-cells");
> +
> +        ret = func(fdt, node, fdt_get_name(fdt, node, NULL), depth,
> +                   address_cells[depth-1], size_cells[depth-1], data);

I suppose this function could have been written recursively and avoided
the arbitrary MAX_DEPTH, but actually in the hypervisor an iterative
version with explicit maximum stack usage seems like a reasonable idea.

> +        if (ret < 0)
> +            return ret;
> +    }
> +    return 0;
> +}
> +
> +static void __init process_memory_node(const void *fdt, int node, const char 
> *name,
>                                         u32 address_cells, u32 size_cells)
>  {
>      const struct fdt_property *prop;
> @@ -77,15 +112,13 @@ static void __init process_memory_node(const void *fdt, 
> int node,
>      paddr_t start, size;
>  
>      if (address_cells < 1 || size_cells < 1) {

I should have spotted this before, coding style needs a space inside the
() and { should be on the next line. There's a bunch of this in both the
context and the new code added by this patch. Obviously the new code
should be fixed, I don't mind if you deal with the existing bits by a
cleanup sweep or by picking it up as you go along.

> -        early_printk("fdt: node `%s': invalid #address-cells or #size-cells",
> -                     fdt_get_name(fdt, node, NULL));
> +        early_printk("fdt: node `%s': invalid #address-cells or 
> #size-cells", name);
>          return;
>      }
>  
>      prop = fdt_get_property(fdt, node, "reg", NULL);
>      if (!prop) {
> -        early_printk("fdt: node `%s': missing `reg' property\n",
> -                     fdt_get_name(fdt, node, NULL));
> +        early_printk("fdt: node `%s': missing `reg' property\n", name);
>          return;
>      }
>  
> @@ -101,28 +134,14 @@ static void __init process_memory_node(const void *fdt, 
> int node,
>      }
>  }
>  
> -#define MAX_DEPTH 16
> -
> -static void __init early_scan(const void *fdt)
> +static int __init early_scan_node(const void *fdt,
> +                                  int node, const char *name, int depth,
> +                                  u32 address_cells, u32 size_cells, void 
> *data)
>  {
> -    int node;
> -    int depth;
> -    u32 address_cells[MAX_DEPTH];
> -    u32 size_cells[MAX_DEPTH];
> -
> -    for (node = 0; depth >= 0; node = fdt_next_node(fdt, node, &depth)) {
> -        if (depth >= MAX_DEPTH) {
> -            early_printk("fdt: node '%s': nested too deep\n",
> -                         fdt_get_name(fdt, node, NULL));
> -            continue;
> -        }
> +    if (node_matches(fdt, node, "memory"))
> +        process_memory_node(fdt, node, name, address_cells, size_cells);
>  
> -        address_cells[depth] = prop_by_name_u32(fdt, node, "#address-cells");
> -        size_cells[depth] = prop_by_name_u32(fdt, node, "#size-cells");
> -
> -        if (node_matches(fdt, node, "memory"))
> -            process_memory_node(fdt, node, address_cells[depth-1], 
> size_cells[depth-1]);
> -    }
> +    return 0;
>  }
>  
>  static void __init early_print_info(void)
> @@ -149,7 +168,7 @@ size_t __init device_tree_early_init(const void *fdt)
>      if (ret < 0)
>          early_panic("No valid device tree\n");
>  
> -    early_scan(fdt);
> +    device_tree_for_each_node((void *)fdt, early_scan_node, NULL);
>      early_print_info();
>  
>      return fdt_totalsize(fdt);
> diff --git a/xen/include/xen/device_tree.h b/xen/include/xen/device_tree.h
> index 28a3dee..505f3e3 100644
> --- a/xen/include/xen/device_tree.h
> +++ b/xen/include/xen/device_tree.h
> @@ -28,10 +28,18 @@ struct dt_early_info {
>      struct dt_mem_info mem;
>  };
>  
> +typedef int (*device_tree_node_func)(const void *fdt,
> +                                     int node, const char *name, int depth,
> +                                     u32 address_cells, u32 size_cells,
> +                                     void *data);
> +
>  extern struct dt_early_info early_info;
>  extern void *device_tree_flattened;
>  
>  size_t device_tree_early_init(const void *fdt);
>  paddr_t device_tree_get_xen_paddr(void);
>  
> +int device_tree_for_each_node(const void *fdt,
> +                              device_tree_node_func func, void *data);
> +
>  #endif



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel


 


Rackspace

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