Re: [PATCH v3 21/33] lance: replace PROP_PTR with PROP_LINK

["Philippe Mathieu-Daudé" <philmd@redhat.com>]

On 10/24/19 1:09 PM, Marc-André Lureau wrote:
> Hi
> 
> 
> On Thu, Oct 24, 2019 at 1:01 AM Philippe Mathieu-Daudé
> <philmd@redhat.com> wrote:
>>
>> On 10/23/19 7:31 PM, Marc-André Lureau wrote:
>>> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
>>> Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
>>> ---
>>>    hw/dma/sparc32_dma.c | 2 +-
>>>    hw/net/lance.c       | 5 ++---
>>>    hw/net/pcnet-pci.c   | 2 +-
>>>    hw/net/pcnet.h       | 2 +-
>>>    4 files changed, 5 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/hw/dma/sparc32_dma.c b/hw/dma/sparc32_dma.c
>>> index 0e5bbcdc7f..3e4da0c47f 100644
>>> --- a/hw/dma/sparc32_dma.c
>>> +++ b/hw/dma/sparc32_dma.c
>>> @@ -346,7 +346,7 @@ static void sparc32_ledma_device_realize(DeviceState *dev, Error **errp)
>>>        d = qdev_create(NULL, TYPE_LANCE);
>>>        object_property_add_child(OBJECT(dev), "lance", OBJECT(d), errp);
>>>        qdev_set_nic_properties(d, nd);
>>> -    qdev_prop_set_ptr(d, "dma", dev);
>>> +    object_property_set_link(OBJECT(d), OBJECT(dev), "dma", errp);
>>>        qdev_init_nofail(d);
>>>    }
>>>
>>> diff --git a/hw/net/lance.c b/hw/net/lance.c
>>> index 6631e2a4e0..4d96299041 100644
>>> --- a/hw/net/lance.c
>>> +++ b/hw/net/lance.c
>>> @@ -138,7 +138,8 @@ static void lance_instance_init(Object *obj)
>>>    }
>>>
>>>    static Property lance_properties[] = {
>>> -    DEFINE_PROP_PTR("dma", SysBusPCNetState, state.dma_opaque),
>>> +    DEFINE_PROP_LINK("dma", SysBusPCNetState, state.dma_opaque,
>>> +                     TYPE_DEVICE, DeviceState *),
>>>        DEFINE_NIC_PROPERTIES(SysBusPCNetState, state.conf),
>>>        DEFINE_PROP_END_OF_LIST(),
>>>    };
>>> @@ -153,8 +154,6 @@ static void lance_class_init(ObjectClass *klass, void *data)
>>>        dc->reset = lance_reset;
>>>        dc->vmsd = &vmstate_lance;
>>>        dc->props = lance_properties;
>>> -    /* Reason: pointer property "dma" */
>>> -    dc->user_creatable = false;
>>
>> But we still can not start it with the -device option and set the dma,
>> can we?
> 
> This is a sysbus device, so you can't. I'll add a commit comment.

Ah OK, understood now.

With comment:
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

> 
> In theory, link property allows you to pass a QOM path to reference a
> QOM instance from -device.

Just wondering, if we had a "bus_address" property to the abstract 
SysBus class (and eventually "bus_name" for later) we could create/map
sysbus devices from command line?

>>
>>>    }
>>>
>>>    static const TypeInfo lance_info = {
>>> diff --git a/hw/net/pcnet-pci.c b/hw/net/pcnet-pci.c
>>> index 4723c30c79..d067d21e2c 100644
>>> --- a/hw/net/pcnet-pci.c
>>> +++ b/hw/net/pcnet-pci.c
>>> @@ -231,7 +231,7 @@ static void pci_pcnet_realize(PCIDevice *pci_dev, Error **errp)
>>>        s->irq = pci_allocate_irq(pci_dev);
>>>        s->phys_mem_read = pci_physical_memory_read;
>>>        s->phys_mem_write = pci_physical_memory_write;
>>> -    s->dma_opaque = pci_dev;
>>> +    s->dma_opaque = DEVICE(pci_dev);
>>>
>>>        pcnet_common_init(DEVICE(pci_dev), s, &net_pci_pcnet_info);
>>>    }
>>> diff --git a/hw/net/pcnet.h b/hw/net/pcnet.h
>>> index 28d19a5c6f..f49b213c57 100644
>>> --- a/hw/net/pcnet.h
>>> +++ b/hw/net/pcnet.h
>>> @@ -50,7 +50,7 @@ struct PCNetState_st {
>>>                             uint8_t *buf, int len, int do_bswap);
>>>        void (*phys_mem_write)(void *dma_opaque, hwaddr addr,
>>>                              uint8_t *buf, int len, int do_bswap);
>>> -    void *dma_opaque;
>>> +    DeviceState *dma_opaque;
>>>        int tx_busy;
>>>        int looptest;
>>>    };
>>>