Re: [PATCH v2 15/15] xen/arm: update GIC dt node with GIC v3 information

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

On Fri, 2014-04-04 at 17:26 +0530, vijay.kilari@gmail.com wrote:
> From: Vijaya Kumar K <Vijaya.Kumar@caviumnetworks.com>
> 
> Update GIC device tree node for DOM0 with GICv3
> information.

Perhaps this should just be a callback in the vigc ops struct?

> 
> Signed-off-by: Vijaya Kumar K <Vijaya.Kumar@caviumnetworks.com>
> ---
>  xen/arch/arm/domain_build.c |   41 ++++++++++++++++++++++++++++++++++-------
>  xen/arch/arm/gic-v2.c       |    8 ++++++++
>  xen/arch/arm/gic-v3.c       |    8 ++++++++
>  xen/arch/arm/gic.c          |    5 +++++
>  xen/include/asm-arm/gic.h   |    5 +++++
>  5 files changed, 60 insertions(+), 7 deletions(-)
> 
> diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
> index 66a98f1..5a75403 100644
> --- a/xen/arch/arm/domain_build.c
> +++ b/xen/arch/arm/domain_build.c
> @@ -532,6 +532,8 @@ static int make_gic_node(const struct domain *d, void *fdt,
>      u32 len;
>      __be32 *new_cells, *tmp;
>      int res = 0;
> +    int hw_type = GIC_VERSION_V2;

an enum, perhaps?

> +    u32 rd_stride = 0;
>  
>      /*
>       * Xen currently supports only a single GIC. Discard any secondary
> @@ -545,6 +547,8 @@ static int make_gic_node(const struct domain *d, void *fdt,
>  
>      DPRINT("Create gic node\n");
>  
> +    hw_type = gic_hw_version();
> +
>      compatible = dt_get_property(gic, "compatible", &len);
>      if ( !compatible )
>      {
> @@ -552,6 +556,12 @@ static int make_gic_node(const struct domain *d, void *fdt,
>          return -FDT_ERR_XEN(ENOENT);
>      }
>  
> +    if ( hw_type == GIC_VERSION_V3 )

switch over a hw_type enum? With an empty v2 case. Likewise elsewhere,
Then the compiler will help us catch missing cases.

> +    {
> +        res = dt_property_read_u32(gic, "redistributor-stride", &rd_stride);
> +        if ( !res )
> +            rd_stride = 0;
> +    }
>      res = fdt_begin_node(fdt, "interrupt-controller");
>      if ( res )
>          return res;
> diff --git a/xen/arch/arm/gic-v3.c b/xen/arch/arm/gic-v3.c
> index 8cefdab..0fe1059 100644
> --- a/xen/arch/arm/gic-v3.c
> +++ b/xen/arch/arm/gic-v3.c
> @@ -49,6 +49,7 @@ struct rdist_region {
>  
>  /* Global state */
>  static struct {
> +    int hw_version;
>      paddr_t dbase;            /* Address of distributor registers */
>      paddr_t dbase_size;
>      void __iomem *map_dbase;  /* Mapped address of distributor registers */
> @@ -766,6 +767,11 @@ int gicv_init(struct domain *d)
>      return 0;
>  }
>  
> +static int gic_hw_type(void)
> +{
> +    return gic.hw_version;

Wouldn't this just be hardcoded as v3?

Ian.