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

[Alexander Graf <agraf@suse.de>]

On 06/26/2013 02:38 PM, Peter Crosthwaite wrote:
> Hi,
>
> On Wed, Jun 26, 2013 at 8:50 PM, Peter Maydell<peter.maydell@linaro.org>  wrote:
>> On 26 June 2013 11:31, Alexander Graf<agraf@suse.de>  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?

I've had a lengthy discussion about that with Anthony a while ago. His 
take was that this is perfectly reasonable, as long as the device tree 
generation code stays within the machine model. The machine would just 
traverse the QOM hierachy and generate device tree nodes for everything 
it knows.


Alex