Re: [Xen-devel] [PATCH 1/4] dtb: correct handling of #address-cells and #size-cells.

[Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>]

On Wed, 30 Jan 2013, Ian Campbell wrote:
> If a node does not have #*-cells then the parent's value should be
> used. Currently we were asssuming zero which is useless.
> 
> Signed-off-by: Ian Campbell <ian.campbell@xxxxxxxxxx>
> ---
>  xen/arch/arm/domain_build.c   |    6 ++++--
>  xen/common/device_tree.c      |   12 ++++++++----
>  xen/include/xen/device_tree.h |    3 ++-
>  3 files changed, 14 insertions(+), 7 deletions(-)
> 
> diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
> index 7403f1a..bfbe7c7 100644
> --- a/xen/arch/arm/domain_build.c
> +++ b/xen/arch/arm/domain_build.c
> @@ -198,8 +198,10 @@ static int write_nodes(struct domain *d, struct 
> kernel_info *kinfo,
>          while ( last_depth-- >= depth )
>              fdt_end_node(kinfo->fdt);
>  
> -        address_cells[depth] = device_tree_get_u32(fdt, node, 
> "#address-cells");
> -        size_cells[depth] = device_tree_get_u32(fdt, node, "#size-cells");
> +        address_cells[depth] = device_tree_get_u32(fdt, node, 
> "#address-cells",
> +                                    depth > 0 ? address_cells[depth-1] : 0);
> +        size_cells[depth] = device_tree_get_u32(fdt, node, "#size-cells",
> +                                    depth > 0 ? size_cells[depth-1] : 0);
>  
>          fdt_begin_node(kinfo->fdt, name);

The depth is always increasing by steps of 1 in this loop, right?
Because retrieving address-cells and size-cells should be recursive: if
n-1 doesn't have them, let's look at n-2, etc. Of course if we start from
depth = 0 and go from there without missing any levels the results will
be the same.

I think I convinced myself that this is correct.


> diff --git a/xen/common/device_tree.c b/xen/common/device_tree.c
> index 260c2d4..f10ba1b 100644
> --- a/xen/common/device_tree.c
> +++ b/xen/common/device_tree.c
> @@ -120,13 +120,14 @@ void device_tree_set_reg(u32 **cell, u32 address_cells, 
> u32 size_cells,
>      set_val(cell, size_cells, size);
>  }
>  
> -u32 device_tree_get_u32(const void *fdt, int node, const char *prop_name)
> +u32 device_tree_get_u32(const void *fdt, int node, const char *prop_name,
> +                        u32 dflt)
>  {
>      const struct fdt_property *prop;
>  
>      prop = fdt_get_property(fdt, node, prop_name, NULL);
>      if ( !prop || prop->len < sizeof(u32) )
> -        return 0; /* default to 0 */
> +        return dflt;
>  
>      return fdt32_to_cpu(*(uint32_t*)prop->data);
>  }

where did the vowels go? :)

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