Re: [Qemu-devel] [PATCH v3 2/6] hw/arm/dyn_sysbus_devtree: helpers for sysbus device dynamic dt node generation

[Eric Auger <eric.auger@linaro.org>]

On 09/09/2014 06:03 PM, Paolo Bonzini wrote:
> Il 09/09/2014 17:56, Eric Auger ha scritto:
>>>>>> +    if (!sbdev) {
>>>>>> +        /* Container, traverse it for children */
>>>>>> +        return object_child_foreach(obj,
>>>>>> +                                    arm_sysbus_device_create_devtree, data);
>>>>>> +    }
>> When we add support for a dynamically instantiable device we add
>> something like
>>
>>     if (object_dynamic_cast(obj, TYPE_ETSEC_COMMON)) {
>>         create_devtree_etsec(ETSEC_COMMON(dev), data);
>>         matched = true;
>>     }
>>>>>> +
>>>>>> +    if (!matched) {
>>>>
>>>> Who is going to set "matched", since it doesn't escape?
>>
>>
> 
> That's not part of this patch though, right?

right
  So this code for now is
> dead.  Please remove the dead code if it is not used in this series.
> 
> We really should make that an interface, so that the code can do just
> 
>     if (object_dynamic_cast(obj, TYPE_FDT_BUILDER)) {
>         fdt_builder_create_fdt(FDT_BUILDER(dev), data);
>     } else {
>         ...
>     }
> 
> (and so can the generic virt.c code) but that can come later.

OK I will correct
> 
>>> Why does arm_sysbus_device_create_devtree need intc and irq_start?
>>
>> irq_start: needed because when the "interrupts" property is set for the
>> leaf component the irq number is irq_start +
>> object_property_get_int(obj, "irq[i]", NULL)
>> irq[i] being in [0, params->platform_bus_num_irqs]
> 
> Ah, it's passed to the not-yet-existing create_* functions.
yep
> 
>> intc: this was in case the leaf component would use "interrupt-parent"
>> prop. I miss experience on device trees and I don't know if it make
>> sense the leaf component uses a different interrupt controller than the
>> parent platform bus or if such property is mandatory in some cases.
>> Maybe not needed indeed.
> 
> No idea, sorry.
- Eric
> 
> Paolo
>