From mboxrd@z Thu Jan  1 00:00:00 1970
From: Ian Campbell <ian.campbell@citrix.com>
Subject: Re: [PATCH v3 07/24] xen/arm: Introduce xen,
	passthrough property
Date: Fri, 20 Feb 2015 15:42:54 +0000
Message-ID: <1424446974.30924.315.camel@citrix.com>
References: <1421159133-31526-1-git-send-email-julien.grall@linaro.org>
	<1421159133-31526-8-git-send-email-julien.grall@linaro.org>
Mime-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
Return-path: <xen-devel-bounces@lists.xen.org>
Received: from mail6.bemta14.messagelabs.com ([193.109.254.103])
	by lists.xen.org with esmtp (Exim 4.72)
	(envelope-from <Ian.Campbell@citrix.com>) id 1YOqQ4-0005UR-Kg
	for xen-devel@lists.xenproject.org; Fri, 20 Feb 2015 16:27:12 +0000
In-Reply-To: <1421159133-31526-8-git-send-email-julien.grall@linaro.org>
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: Julien Grall <julien.grall@linaro.org>
Cc: xen-devel@lists.xenproject.org, tim@xen.org, stefano.stabellini@citrix.com
List-Id: xen-devel@lists.xenproject.org

On Tue, 2015-01-13 at 14:25 +0000, Julien Grall wrote:
> @@ -919,8 +943,14 @@ static int make_timer_node(const struct domain *d, void *fdt,
>      return res;
>  }
>  
> -/* Map the device in the domain */
> -static int map_device(struct domain *d, struct dt_device_node *dev)
> +/* For a given device node:

Strictly speaking should be:
 /*
  * For a given...

(I don't care all that much, but since I'm commenting elsewhere)

> @@ -947,7 +979,7 @@ static int map_device(struct domain *d, struct dt_device_node *dev)
>          }
>      }
>  
> -    /* Map IRQs */
> +    /* Give permission and  map IRQs */

Another Nit: "  " -> " ".

> +        if ( need_mapping )
> +        {
> +            /*
> +             * Checking the return of vgic_reserve_virq is not
> +             * necessary. It should not fail except when we try to map
> +             * twice the IRQ. This can happen if the IRQ is shared

"when we try to map the IRQ twice"

Other than those nits the code itself looks good, will ack once we've
agreed on the bindings wording.

Ian.