Re: [Qemu-devel] [PATCH 1/2] device_tree: Add qemu_devtree_setprop_sized_cells() utility function

[Peter Crosthwaite]

Hi,

On Wed, Jun 26, 2013 at 8:50 PM, Peter Maydell <address@hidden> wrote:
> On 26 June 2013 11:31, Alexander Graf <address@hidden> wrote:
>> I think it makes sense to make this API special-purpose for "reg".
>> We currently have a generic "put any number of 32bit values into the
>> property" function (qemu_devtree_setprop_cells).
>
> Yes, but that doesn't work for things that aren't simple arrays
> of 32 bit values, so I think that a generic way to deal
> with those too would be useful. If you wanted to write a
> "ranges" property you'd need this too, so it doesn't just
> apply to "reg".
>

+1. And wouldn't an implementation of such a reg-specific function
back onto Peter's new function quite nicely anyway?

> I think we could avoid the "varargs doesn't promote" problem
> by using a varargs-macro wrapper:
>
> #define qemu_devtree_setprop_sized_cells(fdt, node, prop, ...) \
>     do {   \
>         uint64_t args[] = { __VA_ARGS__ }; \
>         do_qemu_devtree_setprop_sized_cells(fdt, node, prop, \
>             args, sizeof(args));
>     } while (0)
>

Are statement expressions sanctioned? Or do we need to give up the
nice caller accessible return codes?

And can we factor out common functionality (mainly the error checking)
with existing set_prop_cells to make the interfaces consistent? (we
need to get rid of those aborts sooner or later)

> which will promote everything (including the size arguments,
> harmlessly) to uint64_t, and avoids having a varargs function.
>
>> Can't we also just add a qemu_devtree_setprop_reg() that walks
>> the tree downwards in search for #address-cells and #size-cells
>> and assembles the correct reg property from a list of 64bit
>> arguments?
>

I have a patch in my tree that generalises qemu_devtree_getprop* to
allow you walk parents for properties (as per the #foo-cells
semantic). I use it for interrupt cells however, which kinda suggests
that this wish for parent traversal is more generic than just
populating reg. I think that Peters patch, along with a parent search
friendly property search will be enough to be able to do whatever you
want in only a handful of lines.

> Do we have an actual use for this? It seems pretty complicated.
> I would expect that in practice there are two major use cases:
>  (a) create your own fdt from scratch (in which case you can
>      just make everything 64 bits and in any case will know
>      when creating nodes what the #address-cells etc are)
>  (b) modify an existing fdt, in which case you definitely don't
>      want to go poking around too deeply in the tree; anything
>      more than just "put an extra node in the root" is starting
>      to get pretty chancy.
>

Looking to the future, what about -device adding a periph then having
qemu add it to the dtb before boot?

Regards,
Peter

> thanks
> -- PMM