From mboxrd@z Thu Jan  1 00:00:00 1970
From: Julien Grall <julien.grall@citrix.com>
Subject: Re: [PATCH v7 12/28] xen/arm: ITS: Plumbing
 hw_irq_controller for LPIs
Date: Mon, 21 Sep 2015 15:44:40 +0100
Message-ID: <560017D8.8090602@citrix.com>
References: <1442581755-2668-1-git-send-email-vijay.kilari@gmail.com>
	<1442581755-2668-13-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: <1442581755-2668-13-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, Ian.Campbell@citrix.com, stefano.stabellini@eu.citrix.com, stefano.stabellini@citrix.com, tim@xen.org, xen-devel@lists.xen.org
Cc: Prasun.Kapoor@caviumnetworks.com, Vijaya Kumar K <Vijaya.Kumar@caviumnetworks.com>, Zoltan Kiss <zoltan.kiss@huawei.com>, manish.jaggi@caviumnetworks.com
List-Id: xen-devel@lists.xenproject.org

Hi Vijay,

Title: I would replace "Plumbing" by "Plumb"

On 18/09/15 14:08, vijay.kilari@gmail.com wrote:
> From: Vijaya Kumar K <Vijaya.Kumar@caviumnetworks.com>
> 
> Change callbacks gic_host_irq_type and gic_guest_irq_type
> to gic_get_host_irq_type and gic_get_guest_irq_type
> in gic_hw_operations, which returns hw_irq_controller based
> on irq type (SPI or LPI).
> 
> Signed-off-by: Vijaya Kumar K <Vijaya.Kumar@caviumnetworks.com>

With the title and 2 remarks below fixed:

Reviewed-by: Julien Grall <julien.grall@citrix.com>

[...]

> +static inline hw_irq_controller *get_host_hw_irq_controller(unsigned int irq)
> +{
> +    return gic_hw_ops->gic_get_host_irq_type(irq);
> +}
> +
> +static inline hw_irq_controller *get_guest_hw_irq_controller(unsigned int irq)
> +{
> +    return gic_hw_ops->gic_get_guest_irq_type(irq);
> +}
> +

Each of these helpers are used in a single call-site and doesn't bring
much improvement. I would prefer to see those two helpers dropped in
favor of open-coding the call to the callback.

>  /*
>   * needs to be called with a valid cpu_mask, ie each cpu in the mask has
>   * already called gic_cpu_init
> @@ -128,7 +138,7 @@ void gic_route_irq_to_xen(struct irq_desc *desc, const cpumask_t *cpu_mask,
>      ASSERT(test_bit(_IRQ_DISABLED, &desc->status));
>      ASSERT(spin_is_locked(&desc->lock));
>  
> -    desc->handler = gic_hw_ops->gic_host_irq_type;
> +    desc->handler = get_host_hw_irq_controller(desc->irq);
>      gic_set_irq_properties(desc, cpu_mask, priority);
>  }
> @@ -159,7 +169,7 @@ int gic_route_irq_to_guest(struct domain *d, unsigned int virq,
>           test_bit(GIC_IRQ_GUEST_ENABLED, &p->status) )
>          goto out;
>  
> -    desc->handler = gic_hw_ops->gic_guest_irq_type;
> +    desc->handler = get_guest_hw_irq_controller(desc->irq);
>      set_bit(_IRQ_GUEST, &desc->status);
>  
>      gic_set_irq_properties(desc, cpumask_of(v_target->processor), priority);
> diff --git a/xen/include/asm-arm/gic-its.h b/xen/include/asm-arm/gic-its.h
> index f5fba49..4455178 100644
> --- a/xen/include/asm-arm/gic-its.h
> +++ b/xen/include/asm-arm/gic-its.h
> @@ -275,6 +275,8 @@ struct its_device {
>      struct rb_node          node;
>  };
>  
> +extern const hw_irq_controller its_host_lpi_type;
> +extern const hw_irq_controller its_guest_lpi_type;

Newline here.

>  void irqdesc_set_lpi_event(struct irq_desc *desc, unsigned id);
>  unsigned int irqdesc_get_lpi_event(struct irq_desc *desc);
>  struct its_device *irqdesc_get_its_device(struct irq_desc *desc);

Regards,

-- 
Julien Grall