From mboxrd@z Thu Jan  1 00:00:00 1970
From: Julien Grall <julien.grall@linaro.org>
Subject: Re: [PATCH v2 15/15] xen/arm: update GIC dt node with
	GIC v3 information
Date: Fri, 04 Apr 2014 15:22:12 +0100
Message-ID: <533EC014.4030500@linaro.org>
References: <1396612593-443-1-git-send-email-vijay.kilari@gmail.com>
	<1396612593-443-16-git-send-email-vijay.kilari@gmail.com>
Mime-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
Return-path: <xen-devel-bounces@lists.xen.org>
In-Reply-To: <1396612593-443-16-git-send-email-vijay.kilari@gmail.com>
List-Unsubscribe: <http://lists.xen.org/cgi-bin/mailman/options/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=unsubscribe>
List-Post: <mailto:xen-devel@lists.xen.org>
List-Help: <mailto:xen-devel-request@lists.xen.org?subject=help>
List-Subscribe: <http://lists.xen.org/cgi-bin/mailman/listinfo/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=subscribe>
Sender: xen-devel-bounces@lists.xen.org
Errors-To: xen-devel-bounces@lists.xen.org
To: vijay.kilari@gmail.com
Cc: Ian.Campbell@citrix.com, stefano.stabellini@eu.citrix.com, Prasun.Kapoor@caviumnetworks.com, vijaya.kumar@caviumnetworks.com, xen-devel@lists.xen.org, stefano.stabellini@citrix.com
List-Id: xen-devel@lists.xenproject.org

Hello Vijaya,

Thank you for the patch.

The name of this patch is still wrong. Which GIC DT node are you updating?

On 04/04/2014 12:56 PM, vijay.kilari@gmail.com wrote:
> +    if ( hw_type == GIC_VERSION_V3 )
> +    {
> +        res = dt_property_read_u32(gic, "redistributor-stride", &rd_stride);
> +        if ( !res )
> +            rd_stride = 0;
> +    }


You have skipped some of my remarks on V1. I would definitely prefer to
have a callback in your {v,}GIC structure which add the specific GIC
properties following the version supported by DOM0.

[..]

>  static struct dt_irq * gic_maintenance_irq(void)
>  {
>      return &gic.maintenance;
> @@ -786,6 +792,7 @@ static unsigned int gic_read_vmcr_priority(void)
>  }
>  
>  static struct gic_hw_operations gic_ops = {
> +    .gic_type            = gic_hw_type,

This should go in patch #5. As I said on V1, you don't need a callback
for return a fixed number (that you've initialized in gicv3_init). You
can store a number in this structure.

At the same time what is the difference between this number and the one
in GIC state?

>      .nr_lines            = gic_nr_lines,
>      .nr_lrs              = gic_nr_lrs,
>      .get_maintenance_irq = gic_maintenance_irq,
> @@ -893,6 +900,7 @@ static int __init gicv3_init(struct dt_device_node *node, const void *data)
>      gic_cpu_init();
>      gic_hyp_init();
>  
> +    gic.hw_version = GIC_VERSION_V3;
>      /* Register hw ops*/
>      register_gic_ops(&gic_ops);
>      return 0;
> diff --git a/xen/arch/arm/gic.c b/xen/arch/arm/gic.c
> index 87a7ad0..e09d5a5 100644
> --- a/xen/arch/arm/gic.c
> +++ b/xen/arch/arm/gic.c
> @@ -58,6 +58,11 @@ void update_cpu_lr_mask(void)
>  
>  static void gic_clear_one_lr(struct vcpu *v, int i);
>  
> +int gic_hw_version(void)
> +{
> +    return gic_hw_ops->gic_type();
> +}
> +
>  unsigned int gic_number_lines(void)
>  {
>      return gic_hw_ops->nr_lines();
> diff --git a/xen/include/asm-arm/gic.h b/xen/include/asm-arm/gic.h
> index eadbfcc..32212f9 100644
> --- a/xen/include/asm-arm/gic.h
> +++ b/xen/include/asm-arm/gic.h
> @@ -51,6 +51,9 @@
>  #define GICH_HCR_VGRP1EIE (1 << 6)
>  #define GICH_HCR_VGRP1DIE (1 << 7)
>  
> +#define GIC_VERSION_V3 0x3
> +#define GIC_VERSION_V2 0x2
> +

Should not be defined earlier?

-- 
Julien Grall