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

Marc-André Lureau posted 33 patches 6 years, 3 months ago
Maintainers: "Daniel P. Berrangé" <berrange@redhat.com>, Artyom Tarasenko <atar4qemu@gmail.com>, Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>, Paolo Bonzini <pbonzini@redhat.com>, Jason Wang <jasowang@redhat.com>, Aleksandar Rikalo <aleksandar.rikalo@rt-rk.com>, Paul Burton <pburton@wavecomp.com>, Aleksandar Markovic <amarkovic@wavecomp.com>, KONRAD Frederic <frederic.konrad@adacore.com>, Peter Maydell <peter.maydell@linaro.org>, Corey Minyard <cminyard@mvista.com>, Richard Henderson <rth@twiddle.net>, Marcel Apfelbaum <marcel.apfelbaum@gmail.com>, Aurelien Jarno <aurelien@aurel32.net>, Magnus Damm <magnus.damm@gmail.com>, BALATON Zoltan <balaton@eik.bme.hu>, Fabien Chouteau <chouteau@adacore.com>, "Marc-André Lureau" <marcandre.lureau@redhat.com>, "Michael S. Tsirkin" <mst@redhat.com>, "Hervé Poussineau" <hpoussin@reactos.org>, Eduardo Habkost <ehabkost@redhat.com>, "Edgar E. Iglesias" <edgar.iglesias@gmail.com>
There is a newer version of this series
[PATCH v3 21/33] lance: replace PROP_PTR with PROP_LINK
Posted by Marc-André Lureau 6 years, 3 months ago
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;
 }
 
 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;
 };
-- 
2.23.0.606.g08da6496b6


Re: [PATCH v3 21/33] lance: replace PROP_PTR with PROP_LINK
Posted by Philippe Mathieu-Daudé 6 years, 3 months ago
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?

>   }
>   
>   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;
>   };
> 

Re: [PATCH v3 21/33] lance: replace PROP_PTR with PROP_LINK
Posted by Marc-André Lureau 6 years, 3 months ago
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.

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

>
> >   }
> >
> >   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;
> >   };
> >

Re: [PATCH v3 21/33] lance: replace PROP_PTR with PROP_LINK
Posted by Philippe Mathieu-Daudé 6 years, 3 months ago
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;
>>>    };
>>>

Re: [PATCH v3 21/33] lance: replace PROP_PTR with PROP_LINK
Posted by Peter Maydell 6 years, 3 months ago
On Thu, 24 Oct 2019 at 12:48, Philippe Mathieu-Daudé <philmd@redhat.com> wrote:
> 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?

I don't think this is a good plan -- users shouldn't have to know
about the memory map of their boards. Plus it doesn't deal with
the complications of multiple address spaces, DMA, wiring up
irq lines to an interrupt controller, SoC reset handling,
clocks, power-managment...  Command line -device was designed
for pluggable devices, where in the world of real hardware
the device can be physically plugged and unplugged and there's
a clear interface that can be modelled. You can't add an
extra UART to an embedded board in real hardware either.

The only plausible argument I've seen for command-line
plugging of embedded devices is as a sort of side-effect
of having a configuration language syntax for them for
the purpose of being able to write board models as
data-driven config files rather than in C code. But
that would be a lot of design and engineering work, and
if we want that I think we should approach it forwards,
not arrive at it backwards by adding gradual tweaks like
'address' properties to devices.

thanks
-- PMM

Re: [PATCH v3 21/33] lance: replace PROP_PTR with PROP_LINK
Posted by Eduardo Habkost 6 years, 3 months ago
On Thu, Oct 24, 2019 at 12:52:28PM +0100, Peter Maydell wrote:
> On Thu, 24 Oct 2019 at 12:48, Philippe Mathieu-Daudé <philmd@redhat.com> wrote:
> > 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?
> 
> I don't think this is a good plan -- users shouldn't have to know
> about the memory map of their boards. Plus it doesn't deal with
> the complications of multiple address spaces, DMA, wiring up
> irq lines to an interrupt controller, SoC reset handling,
> clocks, power-managment...  Command line -device was designed
> for pluggable devices, where in the world of real hardware
> the device can be physically plugged and unplugged and there's
> a clear interface that can be modelled. You can't add an
> extra UART to an embedded board in real hardware either.
> 
> The only plausible argument I've seen for command-line
> plugging of embedded devices is as a sort of side-effect
> of having a configuration language syntax for them for
> the purpose of being able to write board models as
> data-driven config files rather than in C code. But
> that would be a lot of design and engineering work, and
> if we want that I think we should approach it forwards,
> not arrive at it backwards by adding gradual tweaks like
> 'address' properties to devices.

The QEMU community spent years designing QOM and QMP with that
goal.  Which other pieces to you consider to be missing, to
make you reject making gradual changes towards it?

I agree we shouldn't be introducing new external interfaces
without careful thought.  But I welcome gradual internal API
changes that are helpful for our long term goals.

-- 
Eduardo


Re: [PATCH v3 21/33] lance: replace PROP_PTR with PROP_LINK
Posted by Peter Maydell 6 years, 3 months ago
On Thu, 24 Oct 2019 at 19:07, Eduardo Habkost <ehabkost@redhat.com> wrote:
> On Thu, Oct 24, 2019 at 12:52:28PM +0100, Peter Maydell wrote:
> > I don't think this is a good plan -- users shouldn't have to know
> > about the memory map of their boards. Plus it doesn't deal with
> > the complications of multiple address spaces, DMA, wiring up
> > irq lines to an interrupt controller, SoC reset handling,
> > clocks, power-managment...  Command line -device was designed
> > for pluggable devices, where in the world of real hardware
> > the device can be physically plugged and unplugged and there's
> > a clear interface that can be modelled. You can't add an
> > extra UART to an embedded board in real hardware either.
> >
> > The only plausible argument I've seen for command-line
> > plugging of embedded devices is as a sort of side-effect
> > of having a configuration language syntax for them for
> > the purpose of being able to write board models as
> > data-driven config files rather than in C code. But
> > that would be a lot of design and engineering work, and
> > if we want that I think we should approach it forwards,
> > not arrive at it backwards by adding gradual tweaks like
> > 'address' properties to devices.
>
> The QEMU community spent years designing QOM and QMP with that
> goal.  Which other pieces to you consider to be missing, to
> make you reject making gradual changes towards it?

QOM is an *internal* object model. It's fine for building
machine models *in C*. We have no mechanism for doing
this on the command line or via QMP, because there are
lots of parts of machine models (listed in the first
paragraph above) which aren't possible to do with purely
generic links and properties.

> I agree we shouldn't be introducing new external interfaces
> without careful thought.  But I welcome gradual internal API
> changes that are helpful for our long term goals.

Yeah, I have no objection to useful internal changes that
move generally in directions we'd like to go. But if
"machines via config files" really is a goal I'd like to
see at least a sketch of a design, rationale, summary of
what would need to change and proposals for what would be
done. And I don't think we should be exposing "MMIO addresses"
to users without at least some idea of why that would
fit in with other things we would be doing to move
towards where we're going.

(TBH: I also don't really see 'machines via config
files' as a serious project goal -- we haven't really moved
towards doing that in a decade, as far as I can see.)

thanks
-- PMM

Re: [PATCH v3 21/33] lance: replace PROP_PTR with PROP_LINK
Posted by Marc-André Lureau 6 years, 3 months ago
On Thu, Oct 24, 2019 at 2:17 PM Philippe Mathieu-Daudé
<philmd@redhat.com> wrote:
>
> 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?

I can't tell much, as I am not very familiar with the various sysbus
devices. I think you'll have troubles to specify the various io/mmio &
irq to map to. In theory though, we could probably go in that
direction, even perhaps make "machine" from command line or
description file... I am not sure it's worth though.

>
> >>
> >>>    }
> >>>
> >>>    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;
> >>>    };
> >>>
>


-- 
Marc-André Lureau