Re: [XEN][RFC PATCH v3 11/14] xen/arm: Implement device tree node addition functionalities

[Luca Fancellu <Luca.Fancellu@xxxxxxx>]

> 
> +static int dt_overlay_add_node(struct dt_device_node *device_node,
> +                  const char *parent_node_path)

Const should be indented at the same level of struct

> +/*
> + * Adds device tree nodes under target node.
> + * We use dt_host_new to unflatten the updated device_tree_flattened. This is
> + * done to avoid the removal of device_tree generation, iomem regions 
> mapping to
> + * hardware domain done by handle_node().
> + */
> +static long handle_add_overlay_nodes(void *overlay_fdt,
> +                                     uint32_t overlay_fdt_size)
> +{
> +    int rc = 0;
> +    struct dt_device_node *overlay_node;
> +    char **nodes_full_path = NULL;
> +    int **nodes_irq = NULL;
> +    int *node_num_irq = NULL;
> +    void *fdt = NULL;
> +    struct dt_device_node *dt_host_new = NULL;
> +    struct domain *d = hardware_domain;
> +    struct overlay_track *tr = NULL;
> +    unsigned int naddr;
> +    unsigned int num_irq;
> +    unsigned int i, j, k;
> +    unsigned int num_overlay_nodes;

All unsigned int can stay in the same line

> +    u64 addr, size;
> +
> +    fdt = xmalloc_bytes(fdt_totalsize(device_tree_flattened));
> +    if ( fdt == NULL )
> +        return -ENOMEM;
> +
> +    num_overlay_nodes = overlay_node_count(overlay_fdt);
> +    if ( num_overlay_nodes == 0 )
> +    {
> +        xfree(fdt);
> +        return -ENOMEM;
> +    }
> +
> +    spin_lock(&overlay_lock);
> +
> +    memcpy(fdt, device_tree_flattened, fdt_totalsize(device_tree_flattened));
> +
> +    rc = check_overlay_fdt(overlay_fdt, overlay_fdt_size);
> +    if ( rc )
> +    {
> +        xfree(fdt);
> +        return rc;
> +    }
> +
> +    /*
> +     * overlay_get_nodes_info is called to get the node information from 
> dtbo.
> +     * This is done before fdt_overlay_apply() because the overlay apply will
> +     * erase the magic of overlay_fdt.
> +     */
> +    rc = overlay_get_nodes_info(overlay_fdt, &nodes_full_path,
> +                                num_overlay_nodes);
> +    if ( rc )
> +    {
> +        printk(XENLOG_ERR "Getting nodes information failed with error %d\n",
> +               rc);
> +        goto err;
> +    }
> +
> +    nodes_irq = xmalloc_bytes(num_overlay_nodes * sizeof(int *));

You can use xzalloc_bytes and remove the memset below here and...

> +
> +    if ( nodes_irq == NULL )
> +    {
> +        rc = -ENOMEM;
> +        goto err;
> +    }
> +    memset(nodes_irq, 0x0, num_overlay_nodes * sizeof(int *));
> +
> +    node_num_irq = xmalloc_bytes(num_overlay_nodes * sizeof(int));

Here

> +    if ( node_num_irq == NULL )
> +    {
> +        rc = -ENOMEM;
> +        goto err;
> +    }
> +    memset(node_num_irq, 0x0, num_overlay_nodes * sizeof(int));
> +
> +    rc = fdt_overlay_apply(fdt, overlay_fdt);
> +    if ( rc )
> +    {
> +        printk(XENLOG_ERR "Adding overlay node failed with error %d\n", rc);
> +        goto err;
> +    }
> +
> +   […]
> +err:
> +    spin_unlock(&overlay_lock);
> +
> +    xfree(dt_host_new);
> +    xfree(fdt);
> +
> +    if ( nodes_full_path != NULL )
> +    {
> +        for ( i = 0; i < num_overlay_nodes && nodes_full_path[i] != NULL; 
> i++ )
> +        {
> +            xfree(nodes_full_path[i]);
> +        }
> +        xfree(nodes_full_path);
> +    }
> +
> +    if ( nodes_irq != NULL )
> +    {
> +        for ( i = 0; i < num_overlay_nodes && nodes_irq[i] != NULL; i++ )
> +        {
> +            xfree(nodes_irq[i]);
> +        }
> +        xfree(nodes_irq);
> +    }

I see you use this operation quite a bit in this module, perhaps you can create 
a function to
do that