Re: [Qemu-devel] [PATCH v3 1/8] device_tree: Add qemu_devtree_setprop_sized_cells() utility functions

[Peter Crosthwaite]

Hi Peter,

On Tue, Jul 16, 2013 at 10:25 PM, Peter Maydell
<address@hidden> wrote:
> We already have a qemu_devtree_setprop_cells() which sets a dtb
> property to an array of cells whose values are specified by varargs.
> However for the fairly common case of setting a property to a list
> of addresses or of address,size pairs the number of cells used by
> each element in the list depends on the parent's #address-cells
> and #size-cells properties. To make this easier we provide an analogous
> qemu_devtree_setprop_sized_cells() macro which allows the number
> of cells used by each element to be specified. This is implemented
> using an underlying qemu_devtree_setprop_sized_cells_from_array()
> function which takes the values and sizes as an array; this may
> also be directly useful for cases where the cell contents are
> constructed programmatically.
>
> Signed-off-by: Peter Maydell <address@hidden>

Reviewed-by: Peter Crosthwaite <address@hidden>

> ---
>  device_tree.c                |   33 +++++++++++++++++++++++
>  include/sysemu/device_tree.h |   59 
> ++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 92 insertions(+)
>
> diff --git a/device_tree.c b/device_tree.c
> index 10cf3d0..ffec99a 100644
> --- a/device_tree.c
> +++ b/device_tree.c
> @@ -308,3 +308,36 @@ void qemu_devtree_dumpdtb(void *fdt, int size)
>          exit(g_file_set_contents(dumpdtb, fdt, size, NULL) ? 0 : 1);
>      }
>  }
> +
> +int qemu_devtree_setprop_sized_cells_from_array(void *fdt,
> +                                                const char *node_path,
> +                                                const char *property,
> +                                                int numvalues,
> +                                                uint64_t *values)
> +{
> +    uint32_t *propcells;
> +    uint64_t value;
> +    int cellnum, vnum, ncells;
> +    uint32_t hival;

ncells, hival and value do not persist across iterations of the for loop ...

> +
> +    propcells = g_new0(uint32_t, numvalues * 2);
> +
> +    cellnum = 0;
> +    for (vnum = 0; vnum < numvalues; vnum++) {

... so you could define them here instead for localisation.

Regards,
Peter

> +        ncells = values[vnum * 2];
> +        if (ncells != 1 && ncells != 2) {
> +            return -1;
> +        }
> +        value = values[vnum * 2 + 1];
> +        hival = cpu_to_be32(value >> 32);
> +        if (ncells > 1) {
> +            propcells[cellnum++] = hival;
> +        } else if (hival != 0) {
> +            return -1;
> +        }
> +        propcells[cellnum++] = cpu_to_be32(value);
> +    }
> +
> +    return qemu_devtree_setprop(fdt, node_path, property, propcells,
> +                                cellnum * sizeof(uint32_t));
> +}
> diff --git a/include/sysemu/device_tree.h b/include/sysemu/device_tree.h
> index f0b3f35..2b58baf 100644
> --- a/include/sysemu/device_tree.h
> +++ b/include/sysemu/device_tree.h
> @@ -51,4 +51,63 @@ int qemu_devtree_add_subnode(void *fdt, const char *name);
>
>  void qemu_devtree_dumpdtb(void *fdt, int size);
>
> +/**
> + * qemu_devtree_setprop_sized_cells_from_array:
> + * @fdt: device tree blob
> + * @node_path: node to set property on
> + * @property: property to set
> + * @numvalues: number of values
> + * @values: array of number-of-cells, value pairs
> + *
> + * Set the specified property on the specified node in the device tree
> + * to be an array of cells. The values of the cells are specified via
> + * the values list, which alternates between "number of cells used by
> + * this value" and "value".
> + * number-of-cells must be either 1 or 2 (other values will result in
> + * an error being returned). If a value is too large to fit in the
> + * number of cells specified for it, an error is returned.
> + *
> + * This function is useful because device tree nodes often have cell arrays
> + * which are either lists of addresses or lists of address,size tuples, but
> + * the number of cells used for each element vary depending on the
> + * #address-cells and #size-cells properties of their parent node.
> + * If you know all your cell elements are one cell wide you can use the
> + * simpler qemu_devtree_setprop_cells(). If you're not setting up the
> + * array programmatically, qemu_devtree_setprop_sized_cells may be more
> + * convenient.
> + *
> + * Return value: 0 on success, <0 on error.
> + */
> +int qemu_devtree_setprop_sized_cells_from_array(void *fdt,
> +                                                const char *node_path,
> +                                                const char *property,
> +                                                int numvalues,
> +                                                uint64_t *values);
> +
> +/**
> + * qemu_devtree_setprop_sized_cells:
> + * @fdt: device tree blob
> + * @node_path: node to set property on
> + * @property: property to set
> + * @...: list of number-of-cells, value pairs
> + *
> + * Set the specified property on the specified node in the device tree
> + * to be an array of cells. The values of the cells are specified via
> + * the variable arguments, which alternates between "number of cells
> + * used by this value" and "value".
> + *
> + * This is a convenience wrapper for the function
> + * qemu_devtree_setprop_sized_cells_from_array().
> + *
> + * Return value: 0 on success, <0 on error.
> + */
> +#define qemu_devtree_setprop_sized_cells(fdt, node_path, property, ...)      
>  \
> +    ({                                                                       
>  \
> +        uint64_t qdt_tmp[] = { __VA_ARGS__ };                                
>  \
> +        qemu_devtree_setprop_sized_cells_from_array(fdt, node_path,          
>  \
> +                                                    property,                
>  \
> +                                                    ARRAY_SIZE(qdt_tmp) / 2, 
>  \
> +                                                    qdt_tmp);                
>  \
> +    })
> +
>  #endif /* __DEVICE_TREE_H__ */
> --
> 1.7.9.5
>
>