From mboxrd@z Thu Jan  1 00:00:00 1970
From: Julien Grall <julien.grall@citrix.com>
Subject: Re: [RFC PATCH v3 06/18] xen/arm: ITS: Add helper
 functions to manage its_devices
Date: Tue, 23 Jun 2015 11:21:55 +0100
Message-ID: <55893343.4060903@citrix.com>
References: <1434974517-12136-1-git-send-email-vijay.kilari@gmail.com>
	<1434974517-12136-7-git-send-email-vijay.kilari@gmail.com>
Mime-Version: 1.0
Content-Type: text/plain; charset="us-ascii"; Format="flowed"
Content-Transfer-Encoding: 7bit
Return-path: <xen-devel-bounces@lists.xen.org>
In-Reply-To: <1434974517-12136-7-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, julien.grall@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>, manish.jaggi@caviumnetworks.com
List-Id: xen-devel@lists.xenproject.org

Hi Vijay,

On 22/06/2015 13:01, vijay.kilari@gmail.com wrote:
> From: Vijaya Kumar K <Vijaya.Kumar@caviumnetworks.com>
>
> Helper functions to mange its devices using RB-tree

s/mange/manage/

> are introduced in physical ITS driver.
>
> This is global list of all the devices.
>
> Signed-off-by: Vijaya Kumar K <Vijaya.Kumar@caviumnetworks.com>
> ---
>   xen/arch/arm/gic-v3-its.c     |   49 +++++++++++++++++++++++++++++++++++++++++
>   xen/include/asm-arm/gic-its.h |    3 +++
>   2 files changed, 52 insertions(+)
>
> diff --git a/xen/arch/arm/gic-v3-its.c b/xen/arch/arm/gic-v3-its.c
> index b1a97c1..349d0bb 100644
> --- a/xen/arch/arm/gic-v3-its.c
> +++ b/xen/arch/arm/gic-v3-its.c
> @@ -92,6 +92,7 @@ struct its_node {
>   static LIST_HEAD(its_nodes);
>   static DEFINE_SPINLOCK(its_lock);
>   static struct rdist_prop  *gic_rdists;
> +static struct rb_root rb_its_dev;
>
>   #define gic_data_rdist()            (per_cpu(rdist, smp_processor_id()))
>   #define gic_data_rdist_rd_base()    (per_cpu(rdist, smp_processor_id()).rbase)
> @@ -145,6 +146,53 @@ void dump_cmd(its_cmd_block *cmd)
>   }
>   #endif
>
> +/* RB-tree helpers for its_device */
> +struct its_device * find_its_device(struct rb_root *root, u32 devid)

coding style: struct its_device *find


I would rename to its_find_device to keep consistent with the function 
name within this file.

Also, for any exported function you have to declare the prototype in the 
header within the same patch.

> +{
> +    struct rb_node *node = root->rb_node;
> +
> +    while ( node )
> +    {
> +        struct its_device *dev;
> +
> +        dev = container_of(node, struct its_device, node);
> +        if ( devid < dev->device_id )
> +            node = node->rb_left;
> +        else if ( devid > dev->device_id )
> +            node = node->rb_right;
> +        else
> +            return dev;
> +    }
> +
> +    return NULL;
> +}
> +
> +int insert_its_device(struct rb_root *root, struct its_device *dev)

Why do you need the root in parameter? You already it within the file.

Also, I would rename to its_add_device.

Regards,

-- 
Julien Grall