Re: [PATCH v3 23/33] dp8393x: replace PROP_PTR with PROP_LINK

2019-10-24 Thread Philippe Mathieu-Daudé 

On 10/24/19 1:12 PM, Marc-André Lureau wrote:

On Thu, Oct 24, 2019 at 1:02 AM Philippe Mathieu-Daudé
 wrote:


On 10/23/19 7:31 PM, Marc-André Lureau wrote:

Link property is the correct way to pass a MemoryRegion to a device
for DMA purposes.

Sidenote: as a sysbus device, this remains non-usercreatable
even though we can drop the specific flag here.

Signed-off-by: Marc-André Lureau 
Reviewed-by: Peter Maydell 
---
   hw/mips/mips_jazz.c | 3 ++-
   hw/net/dp8393x.c| 7 +++
   2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/hw/mips/mips_jazz.c b/hw/mips/mips_jazz.c
index 8d010a0b6e..878925a963 100644
--- a/hw/mips/mips_jazz.c
+++ b/hw/mips/mips_jazz.c
@@ -284,7 +284,8 @@ static void mips_jazz_init(MachineState *machine,
   dev = qdev_create(NULL, "dp8393x");
   qdev_set_nic_properties(dev, nd);
   qdev_prop_set_uint8(dev, "it_shift", 2);
-qdev_prop_set_ptr(dev, "dma_mr", rc4030_dma_mr);
+object_property_set_link(OBJECT(dev), OBJECT(rc4030_dma_mr),
+ "dma_mr", &error_abort);
   qdev_init_nofail(dev);
   sysbus = SYS_BUS_DEVICE(dev);
   sysbus_mmio_map(sysbus, 0, 0x80001000);
diff --git a/hw/net/dp8393x.c b/hw/net/dp8393x.c
index a5678e11fa..946c7a8f64 100644
--- a/hw/net/dp8393x.c
+++ b/hw/net/dp8393x.c
@@ -173,7 +173,7 @@ typedef struct dp8393xState {
   int loopback_packet;

   /* Memory access */
-void *dma_mr;
+MemoryRegion *dma_mr;
   AddressSpace as;
   } dp8393xState;

@@ -922,7 +922,8 @@ static const VMStateDescription vmstate_dp8393x = {

   static Property dp8393x_properties[] = {
   DEFINE_NIC_PROPERTIES(dp8393xState, conf),
-DEFINE_PROP_PTR("dma_mr", dp8393xState, dma_mr),
+DEFINE_PROP_LINK("dma_mr", dp8393xState, dma_mr,
+ TYPE_MEMORY_REGION, MemoryRegion *),
   DEFINE_PROP_UINT8("it_shift", dp8393xState, it_shift, 0),
   DEFINE_PROP_END_OF_LIST(),
   };
@@ -936,8 +937,6 @@ static void dp8393x_class_init(ObjectClass *klass, void 
*data)
   dc->reset = dp8393x_reset;
   dc->vmsd = &vmstate_dp8393x;
   dc->props = dp8393x_properties;
-/* Reason: dma_mr property can't be set */
-dc->user_creatable = false;


Patch is OK except this user_creatable change I don't understand.


It's a sysbus device, so it's not user-creatable anyway. I'll add a
commit comment.


With comment:
Reviewed-by: Philippe Mathieu-Daudé 






   }

   static const TypeInfo dp8393x_info = {

Re: [PATCH v3 23/33] dp8393x: replace PROP_PTR with PROP_LINK

2019-10-24 Thread Marc-André Lureau 
On Thu, Oct 24, 2019 at 1:02 AM Philippe Mathieu-Daudé
 wrote:
>
> On 10/23/19 7:31 PM, Marc-André Lureau wrote:
> > Link property is the correct way to pass a MemoryRegion to a device
> > for DMA purposes.
> >
> > Sidenote: as a sysbus device, this remains non-usercreatable
> > even though we can drop the specific flag here.
> >
> > Signed-off-by: Marc-André Lureau 
> > Reviewed-by: Peter Maydell 
> > ---
> >   hw/mips/mips_jazz.c | 3 ++-
> >   hw/net/dp8393x.c| 7 +++
> >   2 files changed, 5 insertions(+), 5 deletions(-)
> >
> > diff --git a/hw/mips/mips_jazz.c b/hw/mips/mips_jazz.c
> > index 8d010a0b6e..878925a963 100644
> > --- a/hw/mips/mips_jazz.c
> > +++ b/hw/mips/mips_jazz.c
> > @@ -284,7 +284,8 @@ static void mips_jazz_init(MachineState *machine,
> >   dev = qdev_create(NULL, "dp8393x");
> >   qdev_set_nic_properties(dev, nd);
> >   qdev_prop_set_uint8(dev, "it_shift", 2);
> > -qdev_prop_set_ptr(dev, "dma_mr", rc4030_dma_mr);
> > +object_property_set_link(OBJECT(dev), OBJECT(rc4030_dma_mr),
> > + "dma_mr", &error_abort);
> >   qdev_init_nofail(dev);
> >   sysbus = SYS_BUS_DEVICE(dev);
> >   sysbus_mmio_map(sysbus, 0, 0x80001000);
> > diff --git a/hw/net/dp8393x.c b/hw/net/dp8393x.c
> > index a5678e11fa..946c7a8f64 100644
> > --- a/hw/net/dp8393x.c
> > +++ b/hw/net/dp8393x.c
> > @@ -173,7 +173,7 @@ typedef struct dp8393xState {
> >   int loopback_packet;
> >
> >   /* Memory access */
> > -void *dma_mr;
> > +MemoryRegion *dma_mr;
> >   AddressSpace as;
> >   } dp8393xState;
> >
> > @@ -922,7 +922,8 @@ static const VMStateDescription vmstate_dp8393x = {
> >
> >   static Property dp8393x_properties[] = {
> >   DEFINE_NIC_PROPERTIES(dp8393xState, conf),
> > -DEFINE_PROP_PTR("dma_mr", dp8393xState, dma_mr),
> > +DEFINE_PROP_LINK("dma_mr", dp8393xState, dma_mr,
> > + TYPE_MEMORY_REGION, MemoryRegion *),
> >   DEFINE_PROP_UINT8("it_shift", dp8393xState, it_shift, 0),
> >   DEFINE_PROP_END_OF_LIST(),
> >   };
> > @@ -936,8 +937,6 @@ static void dp8393x_class_init(ObjectClass *klass, void 
> > *data)
> >   dc->reset = dp8393x_reset;
> >   dc->vmsd = &vmstate_dp8393x;
> >   dc->props = dp8393x_properties;
> > -/* Reason: dma_mr property can't be set */
> > -dc->user_creatable = false;
>
> Patch is OK except this user_creatable change I don't understand.

It's a sysbus device, so it's not user-creatable anyway. I'll add a
commit comment.

>
> >   }
> >
> >   static const TypeInfo dp8393x_info = {
> >

Re: [PATCH v3 23/33] dp8393x: replace PROP_PTR with PROP_LINK

2019-10-23 Thread Philippe Mathieu-Daudé 

On 10/23/19 7:31 PM, Marc-André Lureau wrote:

Link property is the correct way to pass a MemoryRegion to a device
for DMA purposes.

Sidenote: as a sysbus device, this remains non-usercreatable
even though we can drop the specific flag here.

Signed-off-by: Marc-André Lureau 
Reviewed-by: Peter Maydell 
---
  hw/mips/mips_jazz.c | 3 ++-
  hw/net/dp8393x.c| 7 +++
  2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/hw/mips/mips_jazz.c b/hw/mips/mips_jazz.c
index 8d010a0b6e..878925a963 100644
--- a/hw/mips/mips_jazz.c
+++ b/hw/mips/mips_jazz.c
@@ -284,7 +284,8 @@ static void mips_jazz_init(MachineState *machine,
  dev = qdev_create(NULL, "dp8393x");
  qdev_set_nic_properties(dev, nd);
  qdev_prop_set_uint8(dev, "it_shift", 2);
-qdev_prop_set_ptr(dev, "dma_mr", rc4030_dma_mr);
+object_property_set_link(OBJECT(dev), OBJECT(rc4030_dma_mr),
+ "dma_mr", &error_abort);
  qdev_init_nofail(dev);
  sysbus = SYS_BUS_DEVICE(dev);
  sysbus_mmio_map(sysbus, 0, 0x80001000);
diff --git a/hw/net/dp8393x.c b/hw/net/dp8393x.c
index a5678e11fa..946c7a8f64 100644
--- a/hw/net/dp8393x.c
+++ b/hw/net/dp8393x.c
@@ -173,7 +173,7 @@ typedef struct dp8393xState {
  int loopback_packet;
  
  /* Memory access */

-void *dma_mr;
+MemoryRegion *dma_mr;
  AddressSpace as;
  } dp8393xState;
  
@@ -922,7 +922,8 @@ static const VMStateDescription vmstate_dp8393x = {
  
  static Property dp8393x_properties[] = {

  DEFINE_NIC_PROPERTIES(dp8393xState, conf),
-DEFINE_PROP_PTR("dma_mr", dp8393xState, dma_mr),
+DEFINE_PROP_LINK("dma_mr", dp8393xState, dma_mr,
+ TYPE_MEMORY_REGION, MemoryRegion *),
  DEFINE_PROP_UINT8("it_shift", dp8393xState, it_shift, 0),
  DEFINE_PROP_END_OF_LIST(),
  };
@@ -936,8 +937,6 @@ static void dp8393x_class_init(ObjectClass *klass, void 
*data)
  dc->reset = dp8393x_reset;
  dc->vmsd = &vmstate_dp8393x;
  dc->props = dp8393x_properties;
-/* Reason: dma_mr property can't be set */
-dc->user_creatable = false;


Patch is OK except this user_creatable change I don't understand.


  }
  
  static const TypeInfo dp8393x_info = {