From mboxrd@z Thu Jan  1 00:00:00 1970
From: Ian Campbell <ian.campbell@citrix.com>
Subject: Re: [PATCH v4 06/17] xen/arm: ITS: Add virtual ITS
	driver
Date: Fri, 10 Jul 2015 14:54:13 +0100
Message-ID: <1436536453.10074.33.camel@citrix.com>
References: <1436514172-3263-1-git-send-email-vijay.kilari@gmail.com>
	<1436514172-3263-7-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: <1436514172-3263-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
Cc: stefano.stabellini@eu.citrix.com, Prasun.Kapoor@caviumnetworks.com, vijaya.kumar@caviumnetworks.com, tim@xen.org, xen-devel@lists.xen.org, julien.grall@citrix.com, stefano.stabellini@citrix.com, manish.jaggi@caviumnetworks.com
List-Id: xen-devel@lists.xenproject.org

On Fri, 2015-07-10 at 13:12 +0530, vijay.kilari@gmail.com wrote:
> +/* RB-tree helpers for vits_device attached to a domain */

In the rest of the series I found this used in three places:
      * On assignment, to insert the device into the tree
      * On deassignment, to remove it again
      * In vgic_vcpu_inject_lpi, where the device is looked up and then
        never used.

I don't see any other use and therefore I don't think this RB tree
serves any purpose, which is consistent with the design which doesn't
require this lookup anywhere. Please remove it.

If there is some use of it in some future series (e.g. perhaps the PCI
one) then please still remove it and add a patch to that series to
introduce it.



> +struct vits_device *vits_find_device(struct rb_root *root, uint32_t devid)
> +{
> +    struct rb_node *node = root->rb_node;
> +
> +    while ( node )
> +    {
> +        struct vits_device *dev;
> +
> +        dev = container_of(node, struct vits_device, node);
> +
> +        if ( devid < dev->vdevid )
> +            node = node->rb_left;
> +        else if ( devid > dev->vdevid )
> +            node = node->rb_right;
> +        else
> +            return dev;
> +    }
> +
> +    return NULL;
> +}
> +
> +int vits_insert_device(struct rb_root *root, struct vits_device *dev)
> +{
> +    struct rb_node **new, *parent;
> +
> +    new = &root->rb_node;
> +    parent = NULL;
> +    while ( *new )
> +    {
> +        struct vits_device *this;
> +
> +        this  = container_of(*new, struct vits_device, node);
> +
> +        parent = *new;
> +        if ( dev->vdevid < this->vdevid )
> +            new = &((*new)->rb_left);
> +        else if ( dev->vdevid > this->vdevid )
> +            new = &((*new)->rb_right);
> +        else
> +            return -EEXIST;
> +    }
> +
> +    rb_link_node(&dev->node, parent, new);
> +    rb_insert_color(&dev->node, root);
> +
> +    return 0;
> +}
> +
> +void vits_remove_device(struct rb_root *root, struct vits_device *dev)
> +{
> +    if ( dev )
> +        rb_erase(&dev->node, root);
> +}
> +
> +/*
> + * Local variables:
> + * mode: C
> + * c-file-style: "BSD"
> + * c-basic-offset: 4
> + * indent-tabs-mode: nil
> + * End:
> + */