[v6,1/6] qdev: get_child_bus(): Use QOM lookup if available

qbus_realize() adds busses as a QOM child of the device in addition to
adding it to the qdev bus list. Change get_child_bus() to use the QOM
child if it is available. This takes priority over the bus-list, but
the child object is checked for type correctness.

This prepares support for aliasing of buses. The use case is SoCs,
where a SoC container needs to present buses to the board level, but
the buses are implemented by controller IP we already model as self
contained qbus-containing devices.

Signed-off-by: Peter Crosthwaite <crosthwaite.peter@gmail.com>
---
Currently qbus_realize() ignores errors from object_property_add_child,
so it is hard to guarantee that the QOM linkage is reliable. If it were
the case that that object_property_add_child was supposed to be error
asserting, we could remove the old bus-list strcmp iterator altogether.

 hw/core/qdev.c | 6 ++++++
 1 file changed, 6 insertions(+)

On Sat, Dec 19, 2015 at 9:43 PM, Peter Crosthwaite
<crosthwaitepeter@gmail.com> wrote:
> qbus_realize() adds busses as a QOM child of the device in addition to
> adding it to the qdev bus list. Change get_child_bus() to use the QOM
> child if it is available. This takes priority over the bus-list, but
> the child object is checked for type correctness.

This doesn't cause problems for anything else? Is there anything else
with the same name or something where this causes conflicts?

Thanks,

Alistair

>
> This prepares support for aliasing of buses. The use case is SoCs,
> where a SoC container needs to present buses to the board level, but
> the buses are implemented by controller IP we already model as self
> contained qbus-containing devices.
>
> Signed-off-by: Peter Crosthwaite <crosthwaite.peter@gmail.com>
> ---
> Currently qbus_realize() ignores errors from object_property_add_child,
> so it is hard to guarantee that the QOM linkage is reliable. If it were
> the case that that object_property_add_child was supposed to be error
> asserting, we could remove the old bus-list strcmp iterator altogether.
>
>  hw/core/qdev.c | 6 ++++++
>  1 file changed, 6 insertions(+)
>
> diff --git a/hw/core/qdev.c b/hw/core/qdev.c
> index b3ad467..c96c464 100644
> --- a/hw/core/qdev.c
> +++ b/hw/core/qdev.c
> @@ -581,6 +581,12 @@ void qdev_pass_gpios(DeviceState *dev, DeviceState *container,
>  BusState *qdev_get_child_bus(DeviceState *dev, const char *name)
>  {
>      BusState *bus;
> +    Object *child = object_resolve_path_component(OBJECT(dev), name);
> +
> +    bus = (BusState *)object_dynamic_cast(child, TYPE_BUS);
> +    if (bus) {
> +        return bus;
> +    }
>
>      QLIST_FOREACH(bus, &dev->child_bus, sibling) {
>          if (strcmp(name, bus->name) == 0) {
> --
> 1.9.1
>
>

On Wed, Jan 6, 2016 at 4:18 PM, Alistair Francis
<alistair.francis@xilinx.com> wrote:
> On Sat, Dec 19, 2015 at 9:43 PM, Peter Crosthwaite
> <crosthwaitepeter@gmail.com> wrote:
>> qbus_realize() adds busses as a QOM child of the device in addition to
>> adding it to the qdev bus list. Change get_child_bus() to use the QOM
>> child if it is available. This takes priority over the bus-list, but
>> the child object is checked for type correctness.
>
> This doesn't cause problems for anything else? Is there anything else
> with the same name or something where this causes conflicts?
>

Not that I know of, I'm just being defensive as it is core code. I
suspect that that ignored failure path on the child setter is
incorrect but I can't prove it beyond all doubt.

Regards,
Peter

> Thanks,
>
> Alistair
>
>>
>> This prepares support for aliasing of buses. The use case is SoCs,
>> where a SoC container needs to present buses to the board level, but
>> the buses are implemented by controller IP we already model as self
>> contained qbus-containing devices.
>>
>> Signed-off-by: Peter Crosthwaite <crosthwaite.peter@gmail.com>
>> ---
>> Currently qbus_realize() ignores errors from object_property_add_child,
>> so it is hard to guarantee that the QOM linkage is reliable. If it were
>> the case that that object_property_add_child was supposed to be error
>> asserting, we could remove the old bus-list strcmp iterator altogether.
>>
>>  hw/core/qdev.c | 6 ++++++
>>  1 file changed, 6 insertions(+)
>>
>> diff --git a/hw/core/qdev.c b/hw/core/qdev.c
>> index b3ad467..c96c464 100644
>> --- a/hw/core/qdev.c
>> +++ b/hw/core/qdev.c
>> @@ -581,6 +581,12 @@ void qdev_pass_gpios(DeviceState *dev, DeviceState *container,
>>  BusState *qdev_get_child_bus(DeviceState *dev, const char *name)
>>  {
>>      BusState *bus;
>> +    Object *child = object_resolve_path_component(OBJECT(dev), name);
>> +
>> +    bus = (BusState *)object_dynamic_cast(child, TYPE_BUS);
>> +    if (bus) {
>> +        return bus;
>> +    }
>>
>>      QLIST_FOREACH(bus, &dev->child_bus, sibling) {
>>          if (strcmp(name, bus->name) == 0) {
>> --
>> 1.9.1
>>
>>

On 20 December 2015 at 05:43, Peter Crosthwaite
<crosthwaitepeter@gmail.com> wrote:
> qbus_realize() adds busses as a QOM child of the device in addition to
> adding it to the qdev bus list. Change get_child_bus() to use the QOM
> child if it is available. This takes priority over the bus-list, but
> the child object is checked for type correctness.
>
> This prepares support for aliasing of buses. The use case is SoCs,
> where a SoC container needs to present buses to the board level, but
> the buses are implemented by controller IP we already model as self
> contained qbus-containing devices.
>
> Signed-off-by: Peter Crosthwaite <crosthwaite.peter@gmail.com>
> ---
> Currently qbus_realize() ignores errors from object_property_add_child,
> so it is hard to guarantee that the QOM linkage is reliable. If it were
> the case that that object_property_add_child was supposed to be error
> asserting, we could remove the old bus-list strcmp iterator altogether.
>
>  hw/core/qdev.c | 6 ++++++
>  1 file changed, 6 insertions(+)
>
> diff --git a/hw/core/qdev.c b/hw/core/qdev.c
> index b3ad467..c96c464 100644
> --- a/hw/core/qdev.c
> +++ b/hw/core/qdev.c
> @@ -581,6 +581,12 @@ void qdev_pass_gpios(DeviceState *dev, DeviceState *container,
>  BusState *qdev_get_child_bus(DeviceState *dev, const char *name)
>  {
>      BusState *bus;
> +    Object *child = object_resolve_path_component(OBJECT(dev), name);
> +
> +    bus = (BusState *)object_dynamic_cast(child, TYPE_BUS);
> +    if (bus) {
> +        return bus;
> +    }
>
>      QLIST_FOREACH(bus, &dev->child_bus, sibling) {
>          if (strcmp(name, bus->name) == 0) {

This looks OK to me (and I like the effective API you get for
devices using things), but I don't know enough QOM/qdev internals
to be completely confident giving it a reviewed-by.

thanks
-- PMM

On Wed, Jan 6, 2016 at 4:25 PM, Peter Crosthwaite
<crosthwaitepeter@gmail.com> wrote:
> On Wed, Jan 6, 2016 at 4:18 PM, Alistair Francis
> <alistair.francis@xilinx.com> wrote:
>> On Sat, Dec 19, 2015 at 9:43 PM, Peter Crosthwaite
>> <crosthwaitepeter@gmail.com> wrote:
>>> qbus_realize() adds busses as a QOM child of the device in addition to
>>> adding it to the qdev bus list. Change get_child_bus() to use the QOM
>>> child if it is available. This takes priority over the bus-list, but
>>> the child object is checked for type correctness.
>>
>> This doesn't cause problems for anything else? Is there anything else
>> with the same name or something where this causes conflicts?
>>
>
> Not that I know of, I'm just being defensive as it is core code. I
> suspect that that ignored failure path on the child setter is
> incorrect but I can't prove it beyond all doubt.

I just re-read what I said and it doesn't really make sense. My only
concern was busses with the same name somehow having conflicts that
weren't previously there. I can't think of any case where this would
happen though.

Acked-by: Alistair Francis <alistair.francis@xilinx.com>

Thanks,

Alistair

>
> Regards,
> Peter
>
>> Thanks,
>>
>> Alistair
>>
>>>
>>> This prepares support for aliasing of buses. The use case is SoCs,
>>> where a SoC container needs to present buses to the board level, but
>>> the buses are implemented by controller IP we already model as self
>>> contained qbus-containing devices.
>>>
>>> Signed-off-by: Peter Crosthwaite <crosthwaite.peter@gmail.com>
>>> ---
>>> Currently qbus_realize() ignores errors from object_property_add_child,
>>> so it is hard to guarantee that the QOM linkage is reliable. If it were
>>> the case that that object_property_add_child was supposed to be error
>>> asserting, we could remove the old bus-list strcmp iterator altogether.
>>>
>>>  hw/core/qdev.c | 6 ++++++
>>>  1 file changed, 6 insertions(+)
>>>
>>> diff --git a/hw/core/qdev.c b/hw/core/qdev.c
>>> index b3ad467..c96c464 100644
>>> --- a/hw/core/qdev.c
>>> +++ b/hw/core/qdev.c
>>> @@ -581,6 +581,12 @@ void qdev_pass_gpios(DeviceState *dev, DeviceState *container,
>>>  BusState *qdev_get_child_bus(DeviceState *dev, const char *name)
>>>  {
>>>      BusState *bus;
>>> +    Object *child = object_resolve_path_component(OBJECT(dev), name);
>>> +
>>> +    bus = (BusState *)object_dynamic_cast(child, TYPE_BUS);
>>> +    if (bus) {
>>> +        return bus;
>>> +    }
>>>
>>>      QLIST_FOREACH(bus, &dev->child_bus, sibling) {
>>>          if (strcmp(name, bus->name) == 0) {
>>> --
>>> 1.9.1
>>>
>>>
>