Re: [PATCH v6 3/5] xen/arm: Make gic-v2 code handle hip04-d01 platform

[Julien Grall <julien.grall@linaro.org>]

Hi Frediano,

On 26/02/15 12:40, Frediano Ziglio wrote:
> diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
> index c2dcb49..0834053 100644
> --- a/xen/arch/arm/domain_build.c
> +++ b/xen/arch/arm/domain_build.c
> @@ -1038,6 +1038,7 @@ static int handle_node(struct domain *d, struct kernel_info *kinfo,
>      static const struct dt_device_match gic_matches[] __initconst =
>      {
>          DT_MATCH_GIC_V2,
> +        DT_MATCH_GIC_HIP04,
>          DT_MATCH_GIC_V3,
>          { /* sentinel */ },
>      };

I think this is the perfect time to introduce a callback to check if the
node is a GIC.

This would avoid to grow up this structure.

[..]

> -static const char * const gicv2_dt_compat[] __initconst =
> +static const char * const hip04gic_dt_compat[] __initconst =
>  {
> -    DT_COMPAT_GIC_CORTEX_A15,
> -    DT_COMPAT_GIC_CORTEX_A7,
> -    DT_COMPAT_GIC_400,
> +    DT_COMPAT_GIC_HIP04,
>      NULL
>  };
>  
> -DT_DEVICE_START(gicv2, "GICv2:", DEVICE_GIC)
> -        .compatible = gicv2_dt_compat,
> -        .init = gicv2_init,
> +DT_DEVICE_START(hip04gic, "GIC-HIP04:", DEVICE_GIC)

The ":" was a mistake in the GICv2. Please don't reproduce it here.

> +        .compatible = hip04gic_dt_compat,
> +        .init = hip04gic_init,
>  DT_DEVICE_END
>  
>  /*
> diff --git a/xen/arch/arm/gic.c b/xen/arch/arm/gic.c
> index 390c8b0..e4512a8 100644
> --- a/xen/arch/arm/gic.c
> +++ b/xen/arch/arm/gic.c
> @@ -565,12 +565,13 @@ static void do_sgi(struct cpu_user_regs *regs, enum gic_sgi sgi)
>  void gic_interrupt(struct cpu_user_regs *regs, int is_fiq)
>  {
>      unsigned int irq;
> +    unsigned int max_irq = gic_hw_ops->info->nr_lines;
>  
>      do  {
>          /* Reading IRQ will ACK it */
>          irq = gic_hw_ops->read_irq();
>  
> -        if ( likely(irq >= 16 && irq < 1021) )
> +        if ( likely(irq >= 16 && irq < max_irq) )
>          {
>              local_irq_enable();
>              do_IRQ(regs, irq, is_fiq);

This change should belong to a separate patch.

> diff --git a/xen/include/asm-arm/gic.h b/xen/include/asm-arm/gic.h
> index 187dc46..a36f486 100644
> --- a/xen/include/asm-arm/gic.h
> +++ b/xen/include/asm-arm/gic.h
> @@ -160,6 +160,10 @@
>                          DT_MATCH_COMPATIBLE(DT_COMPAT_GIC_CORTEX_A7), \
>                          DT_MATCH_COMPATIBLE(DT_COMPAT_GIC_400)
>  
> +#define DT_COMPAT_GIC_HIP04          "hisilicon,hip04-intc"
> +
> +#define DT_MATCH_GIC_HIP04 DT_MATCH_COMPATIBLE(DT_COMPAT_GIC_HIP04)
> +
>  #define DT_COMPAT_GIC_V3             "arm,gic-v3"
>  
>  #define DT_MATCH_GIC_V3 DT_MATCH_COMPATIBLE(DT_COMPAT_GIC_V3)

I would prefer if we avoid to add more compatibles like that in gic.h.

I have a patch to drop a part of this mess. I would advise your to use
cherry-pick the commit [1] in your branch.

[1]
http://xenbits.xen.org/gitweb/?p=people/julieng/xen-unstable.git;a=commit;h=7ba87910e557b06c589c3c0fbc6757fa664d029e

Regards,

-- 
Julien Grall