Re: [PATCH 1/2] xen/arm: gic: Refactor the code for creating gic node

[Ian Campbell <ian.campbell@citrix.com>]

On Wed, 2015-04-22 at 23:51 -0500, Suravee Suthikulpanit wrote:
> Since fdt_begin_node() is called by all gicXX_make_dt_node() to create
> the interrupt-controller devicetree node, this patch refactors the call
> and moves it inside make_gic_node(). This also matches the fdt_end_node()
> call at the end of make_gic_node().
> 
> This patch also move the call to gic_make_node() wrapper to be after all
> other generic properties are setup. This allows creating sub-node inside
> gic_make_node() in order to support v2m sub-node.
> 
> Signed-off-by: Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com>

Acked-by: Ian Campbell <ian.campbell@citrix.com>

> ---
>  xen/arch/arm/domain_build.c | 18 +++++++++++++++---
>  xen/arch/arm/gic-hip04.c    |  4 ----
>  xen/arch/arm/gic-v2.c       |  4 ----
>  xen/arch/arm/gic-v3.c       |  4 ----
>  4 files changed, 15 insertions(+), 15 deletions(-)
> 
> diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
> index 987ee1e..a2cd471 100644
> --- a/xen/arch/arm/domain_build.c
> +++ b/xen/arch/arm/domain_build.c
> @@ -808,8 +808,8 @@ static int make_gic_node(const struct domain *d, void *fdt,
>  {
>      const struct dt_device_node *gic = dt_interrupt_controller;
>      int res = 0;
> -    const void *addrcells;
> -    u32 addrcells_len;
> +    const void *addrcells, *sizecells;
> +    u32 addrcells_len, sizecells_len;
>  
>      /*
>       * Xen currently supports only a single GIC. Discard any secondary
> @@ -823,7 +823,7 @@ static int make_gic_node(const struct domain *d, void *fdt,
>  
>      DPRINT("Create gic node\n");
>  
> -    res = gic_make_node(d, node, fdt);
> +    res = fdt_begin_node(fdt, "interrupt-controller");
>      if ( res )
>          return res;
>  
> @@ -847,6 +847,14 @@ static int make_gic_node(const struct domain *d, void *fdt,
>              return res;
>      }
>  
> +    sizecells = dt_get_property(gic, "#size-cells", &sizecells_len);
> +    if ( sizecells )
> +    {
> +        res = fdt_property(fdt, "#size-cells", sizecells, sizecells_len);
> +        if ( res )
> +            return res;
> +    }
> +
>      res = fdt_property_cell(fdt, "#interrupt-cells", 3);
>      if ( res )
>          return res;
> @@ -855,6 +863,10 @@ static int make_gic_node(const struct domain *d, void *fdt,
>      if ( res )
>          return res;
>  
> +    res = gic_make_node(d, node, fdt);
> +    if ( res )
> +        return res;
> +
>      res = fdt_end_node(fdt);
>  
>      return res;
> diff --git a/xen/arch/arm/gic-hip04.c b/xen/arch/arm/gic-hip04.c
> index 223c414..6d527f1 100644
> --- a/xen/arch/arm/gic-hip04.c
> +++ b/xen/arch/arm/gic-hip04.c
> @@ -628,10 +628,6 @@ static int hip04gic_make_dt_node(const struct domain *d,
>      compatible = DT_COMPAT_GIC_CORTEX_A15;
>      len = strlen((char*) compatible) + 1;
>  
> -    res = fdt_begin_node(fdt, "interrupt-controller");
> -    if ( res )
> -        return res;
> -
>      res = fdt_property(fdt, "compatible", compatible, len);
>      if ( res )
>          return res;
> diff --git a/xen/arch/arm/gic-v2.c b/xen/arch/arm/gic-v2.c
> index 073fec2..80acc62 100644
> --- a/xen/arch/arm/gic-v2.c
> +++ b/xen/arch/arm/gic-v2.c
> @@ -616,10 +616,6 @@ static int gicv2_make_dt_node(const struct domain *d,
>          return -FDT_ERR_XEN(ENOENT);
>      }
>  
> -    res = fdt_begin_node(fdt, "interrupt-controller");
> -    if ( res )
> -        return res;
> -
>      res = fdt_property(fdt, "compatible", compatible, len);
>      if ( res )
>          return res;
> diff --git a/xen/arch/arm/gic-v3.c b/xen/arch/arm/gic-v3.c
> index e9a8eda..db498ed 100644
> --- a/xen/arch/arm/gic-v3.c
> +++ b/xen/arch/arm/gic-v3.c
> @@ -1098,10 +1098,6 @@ static int gicv3_make_dt_node(const struct domain *d,
>          return -FDT_ERR_XEN(ENOENT);
>      }
>  
> -    res = fdt_begin_node(fdt, "interrupt-controller");
> -    if ( res )
> -        return res;
> -
>      res = fdt_property(fdt, "compatible", compatible, len);
>      if ( res )
>          return res;