Re: [Qemu-devel] [PATCH 2/3] hw/dma/pl330: Factor out pl330_init() from hw/arm/xilinx_zynq.c

2018-10-30 Thread Philippe Mathieu-Daudé 

On 30/10/18 13:42, Peter Maydell wrote:

On 30 October 2018 at 11:28, Philippe Mathieu-Daudé  wrote:

On 30/10/18 10:36, Peter Maydell wrote:


On 29 October 2018 at 23:20, Philippe Mathieu-Daudé 
wrote:


Signed-off-by: Philippe Mathieu-Daudé 
---
   MAINTAINERS|  1 +
   hw/arm/xilinx_zynq.c   | 18 ++
   hw/dma/pl330.c |  2 +-
   include/hw/dma/pl330.h | 41 +
   4 files changed, 45 insertions(+), 17 deletions(-)
   create mode 100644 include/hw/dma/pl330.h




+static inline void pl330_init(uint32_t base, qemu_irq irq, int nreq)
+{
+SysBusDevice *busdev;
+DeviceState *dev;
+
+dev = qdev_create(NULL, TYPE_PL330);
+qdev_prop_set_uint8(dev, "num_chnls", 8);
+qdev_prop_set_uint8(dev, "num_periph_req", nreq);
+qdev_prop_set_uint8(dev, "num_events", 16);
+qdev_prop_set_uint8(dev, "data_width", 64);
+qdev_prop_set_uint8(dev, "wr_cap", 8);
+qdev_prop_set_uint8(dev, "wr_q_dep", 16);
+qdev_prop_set_uint8(dev, "rd_cap", 8);
+qdev_prop_set_uint8(dev, "rd_q_dep", 16);
+qdev_prop_set_uint16(dev, "data_buffer_dep", 256);
+qdev_init_nofail(dev);



These are the settings the Xilinx board uses, but are
they really the settings every SoC that has a PL330 will use ?



Except "num_periph_req", all are pl330_properties defaults.


If they're all the device's defaults there's not much point
in setting them by hand. But my point is that the reason they're
properties is that in the real hardware these are configurable
values in the RTL. So any given SoC model needs to be able to
set them appropriately. Having a helper function that doesn't
let you set them makes it too easy for people modelling SoCs
not to think about the question, I think...


Yes, I understood and agree.

I now respined v2 without this.

Thanks for your review!

Phil.



thanks
-- PMM

Re: [Qemu-devel] [PATCH 2/3] hw/dma/pl330: Factor out pl330_init() from hw/arm/xilinx_zynq.c

2018-10-30 Thread Peter Maydell 
On 30 October 2018 at 11:28, Philippe Mathieu-Daudé  wrote:
> On 30/10/18 10:36, Peter Maydell wrote:
>>
>> On 29 October 2018 at 23:20, Philippe Mathieu-Daudé 
>> wrote:
>>>
>>> Signed-off-by: Philippe Mathieu-Daudé 
>>> ---
>>>   MAINTAINERS|  1 +
>>>   hw/arm/xilinx_zynq.c   | 18 ++
>>>   hw/dma/pl330.c |  2 +-
>>>   include/hw/dma/pl330.h | 41 +
>>>   4 files changed, 45 insertions(+), 17 deletions(-)
>>>   create mode 100644 include/hw/dma/pl330.h
>>
>>
>>> +static inline void pl330_init(uint32_t base, qemu_irq irq, int nreq)
>>> +{
>>> +SysBusDevice *busdev;
>>> +DeviceState *dev;
>>> +
>>> +dev = qdev_create(NULL, TYPE_PL330);
>>> +qdev_prop_set_uint8(dev, "num_chnls", 8);
>>> +qdev_prop_set_uint8(dev, "num_periph_req", nreq);
>>> +qdev_prop_set_uint8(dev, "num_events", 16);
>>> +qdev_prop_set_uint8(dev, "data_width", 64);
>>> +qdev_prop_set_uint8(dev, "wr_cap", 8);
>>> +qdev_prop_set_uint8(dev, "wr_q_dep", 16);
>>> +qdev_prop_set_uint8(dev, "rd_cap", 8);
>>> +qdev_prop_set_uint8(dev, "rd_q_dep", 16);
>>> +qdev_prop_set_uint16(dev, "data_buffer_dep", 256);
>>> +qdev_init_nofail(dev);
>>
>>
>> These are the settings the Xilinx board uses, but are
>> they really the settings every SoC that has a PL330 will use ?
>
>
> Except "num_periph_req", all are pl330_properties defaults.

If they're all the device's defaults there's not much point
in setting them by hand. But my point is that the reason they're
properties is that in the real hardware these are configurable
values in the RTL. So any given SoC model needs to be able to
set them appropriately. Having a helper function that doesn't
let you set them makes it too easy for people modelling SoCs
not to think about the question, I think...

thanks
-- PMM

Re: [Qemu-devel] [PATCH 2/3] hw/dma/pl330: Factor out pl330_init() from hw/arm/xilinx_zynq.c

2018-10-30 Thread Philippe Mathieu-Daudé 

On 30/10/18 0:20, Philippe Mathieu-Daudé wrote:

Signed-off-by: Philippe Mathieu-Daudé 
---
  MAINTAINERS|  1 +
  hw/arm/xilinx_zynq.c   | 18 ++
  hw/dma/pl330.c |  2 +-
  include/hw/dma/pl330.h | 41 +
  4 files changed, 45 insertions(+), 17 deletions(-)
  create mode 100644 include/hw/dma/pl330.h

diff --git a/MAINTAINERS b/MAINTAINERS
index d794bd7a66..647e2aa0d5 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -452,6 +452,7 @@ F: hw/display/pl110*
  F: hw/dma/pl080.c
  F: include/hw/dma/pl080.h
  F: hw/dma/pl330.c
+F: include/hw/dma/pl330.h
  F: hw/gpio/pl061.c
  F: hw/input/pl050.c
  F: hw/intc/pl190.c
diff --git a/hw/arm/xilinx_zynq.c b/hw/arm/xilinx_zynq.c
index 57497b0c4d..a4c4d44f00 100644
--- a/hw/arm/xilinx_zynq.c
+++ b/hw/arm/xilinx_zynq.c
@@ -34,6 +34,7 @@
  #include "hw/char/cadence_uart.h"
  #include "hw/net/cadence_gem.h"
  #include "hw/cpu/a9mpcore.h"
+#include "hw/dma/pl330.h"
  
  #define NUM_SPI_FLASHES 4

  #define NUM_QSPI_FLASHES 2
@@ -278,22 +279,7 @@ static void zynq_init(MachineState *machine)
  sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, 0xF8007100);
  sysbus_connect_irq(SYS_BUS_DEVICE(dev), 0, pic[39-IRQ_OFFSET]);
  
-dev = qdev_create(NULL, "pl330");

-qdev_prop_set_uint8(dev, "num_chnls",  8);
-qdev_prop_set_uint8(dev, "num_periph_req",  4);
-qdev_prop_set_uint8(dev, "num_events",  16);
-
-qdev_prop_set_uint8(dev, "data_width",  64);
-qdev_prop_set_uint8(dev, "wr_cap",  8);
-qdev_prop_set_uint8(dev, "wr_q_dep",  16);
-qdev_prop_set_uint8(dev, "rd_cap",  8);
-qdev_prop_set_uint8(dev, "rd_q_dep",  16);
-qdev_prop_set_uint16(dev, "data_buffer_dep",  256);
-
-qdev_init_nofail(dev);
-busdev = SYS_BUS_DEVICE(dev);
-sysbus_mmio_map(busdev, 0, 0xF8003000);
-sysbus_connect_irq(busdev, 0, pic[45-IRQ_OFFSET]); /* abort irq line */
+pl330_init(0xf8003000, pic[45 - IRQ_OFFSET], 4); /* abort irq line */
  for (n = 0; n < ARRAY_SIZE(dma_irqs); ++n) { /* event irqs */
  sysbus_connect_irq(busdev, n + 1, pic[dma_irqs[n] - IRQ_OFFSET]);


Sorry, this is a buggy patch   ^ busdev is now invalid.


  }
diff --git a/hw/dma/pl330.c b/hw/dma/pl330.c
index d071049233..711cf9a605 100644
--- a/hw/dma/pl330.c
+++ b/hw/dma/pl330.c
@@ -20,6 +20,7 @@
  #include "qemu/timer.h"
  #include "sysemu/dma.h"
  #include "qemu/log.h"
+#include "hw/dma/pl330.h"
  
  #ifndef PL330_ERR_DEBUG

  #define PL330_ERR_DEBUG 0
@@ -271,7 +272,6 @@ struct PL330State {
  
  };
  
-#define TYPE_PL330 "pl330"

  #define PL330(obj) OBJECT_CHECK(PL330State, (obj), TYPE_PL330)
  
  static const VMStateDescription vmstate_pl330 = {

diff --git a/include/hw/dma/pl330.h b/include/hw/dma/pl330.h
new file mode 100644
index 00..9a586c0df9
--- /dev/null
+++ b/include/hw/dma/pl330.h
@@ -0,0 +1,41 @@
+/*
+ * ARM PrimeCell PL330 DMA Controller
+ *
+ * Copyright (c) 2009 Samsung Electronics.
+ * Contributed by Kirill Batuzov 
+ * Copyright (c) 2012 Peter A.G. Crosthwaite (peter.crosthwa...@petalogix.com)
+ * Copyright (c) 2012 PetaLogix Pty Ltd.
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+
+#ifndef HW_DMA_PL330_H
+#define HW_DMA_PL330_H
+
+#include "hw/sysbus.h"
+
+#define TYPE_PL330 "pl330"
+
+static inline void pl330_init(uint32_t base, qemu_irq irq, int nreq)
+{
+SysBusDevice *busdev;
+DeviceState *dev;
+
+dev = qdev_create(NULL, TYPE_PL330);
+qdev_prop_set_uint8(dev, "num_chnls", 8);
+qdev_prop_set_uint8(dev, "num_periph_req", nreq);
+qdev_prop_set_uint8(dev, "num_events", 16);
+qdev_prop_set_uint8(dev, "data_width", 64);
+qdev_prop_set_uint8(dev, "wr_cap", 8);
+qdev_prop_set_uint8(dev, "wr_q_dep", 16);
+qdev_prop_set_uint8(dev, "rd_cap", 8);
+qdev_prop_set_uint8(dev, "rd_q_dep", 16);
+qdev_prop_set_uint16(dev, "data_buffer_dep", 256);
+qdev_init_nofail(dev);
+
+busdev = SYS_BUS_DEVICE(dev);
+sysbus_mmio_map(busdev, 0, base);
+sysbus_connect_irq(busdev, 0, irq);
+}
+
+#endif /* HW_DMA_PL330_H */

Re: [Qemu-devel] [PATCH 2/3] hw/dma/pl330: Factor out pl330_init() from hw/arm/xilinx_zynq.c

2018-10-30 Thread Philippe Mathieu-Daudé 

On 30/10/18 10:36, Peter Maydell wrote:

On 29 October 2018 at 23:20, Philippe Mathieu-Daudé  wrote:

Signed-off-by: Philippe Mathieu-Daudé 
---
  MAINTAINERS|  1 +
  hw/arm/xilinx_zynq.c   | 18 ++
  hw/dma/pl330.c |  2 +-
  include/hw/dma/pl330.h | 41 +
  4 files changed, 45 insertions(+), 17 deletions(-)
  create mode 100644 include/hw/dma/pl330.h



+static inline void pl330_init(uint32_t base, qemu_irq irq, int nreq)
+{
+SysBusDevice *busdev;
+DeviceState *dev;
+
+dev = qdev_create(NULL, TYPE_PL330);
+qdev_prop_set_uint8(dev, "num_chnls", 8);
+qdev_prop_set_uint8(dev, "num_periph_req", nreq);
+qdev_prop_set_uint8(dev, "num_events", 16);
+qdev_prop_set_uint8(dev, "data_width", 64);
+qdev_prop_set_uint8(dev, "wr_cap", 8);
+qdev_prop_set_uint8(dev, "wr_q_dep", 16);
+qdev_prop_set_uint8(dev, "rd_cap", 8);
+qdev_prop_set_uint8(dev, "rd_q_dep", 16);
+qdev_prop_set_uint16(dev, "data_buffer_dep", 256);
+qdev_init_nofail(dev);


These are the settings the Xilinx board uses, but are
they really the settings every SoC that has a PL330 will use ?


Except "num_periph_req", all are pl330_properties defaults.



thanks
-- PMM

Re: [Qemu-devel] [PATCH 2/3] hw/dma/pl330: Factor out pl330_init() from hw/arm/xilinx_zynq.c

2018-10-30 Thread Peter Maydell 
On 29 October 2018 at 23:20, Philippe Mathieu-Daudé  wrote:
> Signed-off-by: Philippe Mathieu-Daudé 
> ---
>  MAINTAINERS|  1 +
>  hw/arm/xilinx_zynq.c   | 18 ++
>  hw/dma/pl330.c |  2 +-
>  include/hw/dma/pl330.h | 41 +
>  4 files changed, 45 insertions(+), 17 deletions(-)
>  create mode 100644 include/hw/dma/pl330.h

> +static inline void pl330_init(uint32_t base, qemu_irq irq, int nreq)
> +{
> +SysBusDevice *busdev;
> +DeviceState *dev;
> +
> +dev = qdev_create(NULL, TYPE_PL330);
> +qdev_prop_set_uint8(dev, "num_chnls", 8);
> +qdev_prop_set_uint8(dev, "num_periph_req", nreq);
> +qdev_prop_set_uint8(dev, "num_events", 16);
> +qdev_prop_set_uint8(dev, "data_width", 64);
> +qdev_prop_set_uint8(dev, "wr_cap", 8);
> +qdev_prop_set_uint8(dev, "wr_q_dep", 16);
> +qdev_prop_set_uint8(dev, "rd_cap", 8);
> +qdev_prop_set_uint8(dev, "rd_q_dep", 16);
> +qdev_prop_set_uint16(dev, "data_buffer_dep", 256);
> +qdev_init_nofail(dev);

These are the settings the Xilinx board uses, but are
they really the settings every SoC that has a PL330 will use ?

thanks
-- PMM

Re: [Qemu-devel] [PATCH 2/3] hw/dma/pl330: Factor out pl330_init() from hw/arm/xilinx_zynq.c

2018-10-30 Thread Philippe Mathieu-Daudé 

On 30/10/18 9:18, Richard Henderson wrote:

On 10/29/18 11:20 PM, Philippe Mathieu-Daudé wrote:

+static inline void pl330_init(uint32_t base, qemu_irq irq, int nreq)
+{
+SysBusDevice *busdev;
+DeviceState *dev;
+
+dev = qdev_create(NULL, TYPE_PL330);
+qdev_prop_set_uint8(dev, "num_chnls", 8);
+qdev_prop_set_uint8(dev, "num_periph_req", nreq);
+qdev_prop_set_uint8(dev, "num_events", 16);
+qdev_prop_set_uint8(dev, "data_width", 64);
+qdev_prop_set_uint8(dev, "wr_cap", 8);
+qdev_prop_set_uint8(dev, "wr_q_dep", 16);
+qdev_prop_set_uint8(dev, "rd_cap", 8);
+qdev_prop_set_uint8(dev, "rd_q_dep", 16);
+qdev_prop_set_uint16(dev, "data_buffer_dep", 256);
+qdev_init_nofail(dev);
+
+busdev = SYS_BUS_DEVICE(dev);
+sysbus_mmio_map(busdev, 0, base);
+sysbus_connect_irq(busdev, 0, irq);
+}


Why is this inline instead of in hw/dma/pl300.c?
There should be nothing performance sensative here...


Yeah I didn't like it much neither and wondered the same :)
I was looking a examples in hw/char:

[phil@x1w qemu]$ git grep -hA 2 'static inline' include/hw/char/
52:static inline DeviceState *cadence_uart_create(hwaddr addr,
53-qemu_irq irq,
54-Chardev *chr)
--
52:static inline DeviceState *cmsdk_apb_uart_create(hwaddr addr,
53- qemu_irq txint,
54- qemu_irq rxint,
--
18:static inline DeviceState *pl011_create(hwaddr addr,
19-qemu_irq irq,
20-Chardev *chr)
--
35:static inline DeviceState *pl011_luminary_create(hwaddr addr,
36- qemu_irq irq,
37- Chardev *chr)
--
18:static inline DeviceState *xilinx_uartlite_create(hwaddr addr,
19-qemu_irq irq,
20-Chardev *chr)

I'll clean it and add a docstring.

Thanks for the review,

Phil.

Re: [Qemu-devel] [PATCH 2/3] hw/dma/pl330: Factor out pl330_init() from hw/arm/xilinx_zynq.c

2018-10-30 Thread Richard Henderson 
On 10/29/18 11:20 PM, Philippe Mathieu-Daudé wrote:
> +static inline void pl330_init(uint32_t base, qemu_irq irq, int nreq)
> +{
> +SysBusDevice *busdev;
> +DeviceState *dev;
> +
> +dev = qdev_create(NULL, TYPE_PL330);
> +qdev_prop_set_uint8(dev, "num_chnls", 8);
> +qdev_prop_set_uint8(dev, "num_periph_req", nreq);
> +qdev_prop_set_uint8(dev, "num_events", 16);
> +qdev_prop_set_uint8(dev, "data_width", 64);
> +qdev_prop_set_uint8(dev, "wr_cap", 8);
> +qdev_prop_set_uint8(dev, "wr_q_dep", 16);
> +qdev_prop_set_uint8(dev, "rd_cap", 8);
> +qdev_prop_set_uint8(dev, "rd_q_dep", 16);
> +qdev_prop_set_uint16(dev, "data_buffer_dep", 256);
> +qdev_init_nofail(dev);
> +
> +busdev = SYS_BUS_DEVICE(dev);
> +sysbus_mmio_map(busdev, 0, base);
> +sysbus_connect_irq(busdev, 0, irq);
> +}

Why is this inline instead of in hw/dma/pl300.c?
There should be nothing performance sensative here...


r~

Re: [Qemu-devel] [PATCH 2/3] hw/dma/pl330: Factor out pl330_init() from hw/arm/xilinx_zynq.c

2018-10-29 Thread Alistair Francis 
On Mon, Oct 29, 2018 at 4:24 PM Philippe Mathieu-Daudé
 wrote:
>
> Signed-off-by: Philippe Mathieu-Daudé 

Reviewed-by: Alistair Francis 

Alistair

> ---
>  MAINTAINERS|  1 +
>  hw/arm/xilinx_zynq.c   | 18 ++
>  hw/dma/pl330.c |  2 +-
>  include/hw/dma/pl330.h | 41 +
>  4 files changed, 45 insertions(+), 17 deletions(-)
>  create mode 100644 include/hw/dma/pl330.h
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index d794bd7a66..647e2aa0d5 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -452,6 +452,7 @@ F: hw/display/pl110*
>  F: hw/dma/pl080.c
>  F: include/hw/dma/pl080.h
>  F: hw/dma/pl330.c
> +F: include/hw/dma/pl330.h
>  F: hw/gpio/pl061.c
>  F: hw/input/pl050.c
>  F: hw/intc/pl190.c
> diff --git a/hw/arm/xilinx_zynq.c b/hw/arm/xilinx_zynq.c
> index 57497b0c4d..a4c4d44f00 100644
> --- a/hw/arm/xilinx_zynq.c
> +++ b/hw/arm/xilinx_zynq.c
> @@ -34,6 +34,7 @@
>  #include "hw/char/cadence_uart.h"
>  #include "hw/net/cadence_gem.h"
>  #include "hw/cpu/a9mpcore.h"
> +#include "hw/dma/pl330.h"
>
>  #define NUM_SPI_FLASHES 4
>  #define NUM_QSPI_FLASHES 2
> @@ -278,22 +279,7 @@ static void zynq_init(MachineState *machine)
>  sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, 0xF8007100);
>  sysbus_connect_irq(SYS_BUS_DEVICE(dev), 0, pic[39-IRQ_OFFSET]);
>
> -dev = qdev_create(NULL, "pl330");
> -qdev_prop_set_uint8(dev, "num_chnls",  8);
> -qdev_prop_set_uint8(dev, "num_periph_req",  4);
> -qdev_prop_set_uint8(dev, "num_events",  16);
> -
> -qdev_prop_set_uint8(dev, "data_width",  64);
> -qdev_prop_set_uint8(dev, "wr_cap",  8);
> -qdev_prop_set_uint8(dev, "wr_q_dep",  16);
> -qdev_prop_set_uint8(dev, "rd_cap",  8);
> -qdev_prop_set_uint8(dev, "rd_q_dep",  16);
> -qdev_prop_set_uint16(dev, "data_buffer_dep",  256);
> -
> -qdev_init_nofail(dev);
> -busdev = SYS_BUS_DEVICE(dev);
> -sysbus_mmio_map(busdev, 0, 0xF8003000);
> -sysbus_connect_irq(busdev, 0, pic[45-IRQ_OFFSET]); /* abort irq line */
> +pl330_init(0xf8003000, pic[45 - IRQ_OFFSET], 4); /* abort irq line */
>  for (n = 0; n < ARRAY_SIZE(dma_irqs); ++n) { /* event irqs */
>  sysbus_connect_irq(busdev, n + 1, pic[dma_irqs[n] - IRQ_OFFSET]);
>  }
> diff --git a/hw/dma/pl330.c b/hw/dma/pl330.c
> index d071049233..711cf9a605 100644
> --- a/hw/dma/pl330.c
> +++ b/hw/dma/pl330.c
> @@ -20,6 +20,7 @@
>  #include "qemu/timer.h"
>  #include "sysemu/dma.h"
>  #include "qemu/log.h"
> +#include "hw/dma/pl330.h"
>
>  #ifndef PL330_ERR_DEBUG
>  #define PL330_ERR_DEBUG 0
> @@ -271,7 +272,6 @@ struct PL330State {
>
>  };
>
> -#define TYPE_PL330 "pl330"
>  #define PL330(obj) OBJECT_CHECK(PL330State, (obj), TYPE_PL330)
>
>  static const VMStateDescription vmstate_pl330 = {
> diff --git a/include/hw/dma/pl330.h b/include/hw/dma/pl330.h
> new file mode 100644
> index 00..9a586c0df9
> --- /dev/null
> +++ b/include/hw/dma/pl330.h
> @@ -0,0 +1,41 @@
> +/*
> + * ARM PrimeCell PL330 DMA Controller
> + *
> + * Copyright (c) 2009 Samsung Electronics.
> + * Contributed by Kirill Batuzov 
> + * Copyright (c) 2012 Peter A.G. Crosthwaite 
> (peter.crosthwa...@petalogix.com)
> + * Copyright (c) 2012 PetaLogix Pty Ltd.
> + *
> + * SPDX-License-Identifier: GPL-2.0-or-later
> + */
> +
> +#ifndef HW_DMA_PL330_H
> +#define HW_DMA_PL330_H
> +
> +#include "hw/sysbus.h"
> +
> +#define TYPE_PL330 "pl330"
> +
> +static inline void pl330_init(uint32_t base, qemu_irq irq, int nreq)
> +{
> +SysBusDevice *busdev;
> +DeviceState *dev;
> +
> +dev = qdev_create(NULL, TYPE_PL330);
> +qdev_prop_set_uint8(dev, "num_chnls", 8);
> +qdev_prop_set_uint8(dev, "num_periph_req", nreq);
> +qdev_prop_set_uint8(dev, "num_events", 16);
> +qdev_prop_set_uint8(dev, "data_width", 64);
> +qdev_prop_set_uint8(dev, "wr_cap", 8);
> +qdev_prop_set_uint8(dev, "wr_q_dep", 16);
> +qdev_prop_set_uint8(dev, "rd_cap", 8);
> +qdev_prop_set_uint8(dev, "rd_q_dep", 16);
> +qdev_prop_set_uint16(dev, "data_buffer_dep", 256);
> +qdev_init_nofail(dev);
> +
> +busdev = SYS_BUS_DEVICE(dev);
> +sysbus_mmio_map(busdev, 0, base);
> +sysbus_connect_irq(busdev, 0, irq);
> +}
> +
> +#endif /* HW_DMA_PL330_H */
> --
> 2.17.2
>
>

[Qemu-devel] [PATCH 2/3] hw/dma/pl330: Factor out pl330_init() from hw/arm/xilinx_zynq.c

2018-10-29 Thread Philippe Mathieu-Daudé 
Signed-off-by: Philippe Mathieu-Daudé 
---
 MAINTAINERS|  1 +
 hw/arm/xilinx_zynq.c   | 18 ++
 hw/dma/pl330.c |  2 +-
 include/hw/dma/pl330.h | 41 +
 4 files changed, 45 insertions(+), 17 deletions(-)
 create mode 100644 include/hw/dma/pl330.h

diff --git a/MAINTAINERS b/MAINTAINERS
index d794bd7a66..647e2aa0d5 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -452,6 +452,7 @@ F: hw/display/pl110*
 F: hw/dma/pl080.c
 F: include/hw/dma/pl080.h
 F: hw/dma/pl330.c
+F: include/hw/dma/pl330.h
 F: hw/gpio/pl061.c
 F: hw/input/pl050.c
 F: hw/intc/pl190.c
diff --git a/hw/arm/xilinx_zynq.c b/hw/arm/xilinx_zynq.c
index 57497b0c4d..a4c4d44f00 100644
--- a/hw/arm/xilinx_zynq.c
+++ b/hw/arm/xilinx_zynq.c
@@ -34,6 +34,7 @@
 #include "hw/char/cadence_uart.h"
 #include "hw/net/cadence_gem.h"
 #include "hw/cpu/a9mpcore.h"
+#include "hw/dma/pl330.h"
 
 #define NUM_SPI_FLASHES 4
 #define NUM_QSPI_FLASHES 2
@@ -278,22 +279,7 @@ static void zynq_init(MachineState *machine)
 sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, 0xF8007100);
 sysbus_connect_irq(SYS_BUS_DEVICE(dev), 0, pic[39-IRQ_OFFSET]);
 
-dev = qdev_create(NULL, "pl330");
-qdev_prop_set_uint8(dev, "num_chnls",  8);
-qdev_prop_set_uint8(dev, "num_periph_req",  4);
-qdev_prop_set_uint8(dev, "num_events",  16);
-
-qdev_prop_set_uint8(dev, "data_width",  64);
-qdev_prop_set_uint8(dev, "wr_cap",  8);
-qdev_prop_set_uint8(dev, "wr_q_dep",  16);
-qdev_prop_set_uint8(dev, "rd_cap",  8);
-qdev_prop_set_uint8(dev, "rd_q_dep",  16);
-qdev_prop_set_uint16(dev, "data_buffer_dep",  256);
-
-qdev_init_nofail(dev);
-busdev = SYS_BUS_DEVICE(dev);
-sysbus_mmio_map(busdev, 0, 0xF8003000);
-sysbus_connect_irq(busdev, 0, pic[45-IRQ_OFFSET]); /* abort irq line */
+pl330_init(0xf8003000, pic[45 - IRQ_OFFSET], 4); /* abort irq line */
 for (n = 0; n < ARRAY_SIZE(dma_irqs); ++n) { /* event irqs */
 sysbus_connect_irq(busdev, n + 1, pic[dma_irqs[n] - IRQ_OFFSET]);
 }
diff --git a/hw/dma/pl330.c b/hw/dma/pl330.c
index d071049233..711cf9a605 100644
--- a/hw/dma/pl330.c
+++ b/hw/dma/pl330.c
@@ -20,6 +20,7 @@
 #include "qemu/timer.h"
 #include "sysemu/dma.h"
 #include "qemu/log.h"
+#include "hw/dma/pl330.h"
 
 #ifndef PL330_ERR_DEBUG
 #define PL330_ERR_DEBUG 0
@@ -271,7 +272,6 @@ struct PL330State {
 
 };
 
-#define TYPE_PL330 "pl330"
 #define PL330(obj) OBJECT_CHECK(PL330State, (obj), TYPE_PL330)
 
 static const VMStateDescription vmstate_pl330 = {
diff --git a/include/hw/dma/pl330.h b/include/hw/dma/pl330.h
new file mode 100644
index 00..9a586c0df9
--- /dev/null
+++ b/include/hw/dma/pl330.h
@@ -0,0 +1,41 @@
+/*
+ * ARM PrimeCell PL330 DMA Controller
+ *
+ * Copyright (c) 2009 Samsung Electronics.
+ * Contributed by Kirill Batuzov 
+ * Copyright (c) 2012 Peter A.G. Crosthwaite (peter.crosthwa...@petalogix.com)
+ * Copyright (c) 2012 PetaLogix Pty Ltd.
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+
+#ifndef HW_DMA_PL330_H
+#define HW_DMA_PL330_H
+
+#include "hw/sysbus.h"
+
+#define TYPE_PL330 "pl330"
+
+static inline void pl330_init(uint32_t base, qemu_irq irq, int nreq)
+{
+SysBusDevice *busdev;
+DeviceState *dev;
+
+dev = qdev_create(NULL, TYPE_PL330);
+qdev_prop_set_uint8(dev, "num_chnls", 8);
+qdev_prop_set_uint8(dev, "num_periph_req", nreq);
+qdev_prop_set_uint8(dev, "num_events", 16);
+qdev_prop_set_uint8(dev, "data_width", 64);
+qdev_prop_set_uint8(dev, "wr_cap", 8);
+qdev_prop_set_uint8(dev, "wr_q_dep", 16);
+qdev_prop_set_uint8(dev, "rd_cap", 8);
+qdev_prop_set_uint8(dev, "rd_q_dep", 16);
+qdev_prop_set_uint16(dev, "data_buffer_dep", 256);
+qdev_init_nofail(dev);
+
+busdev = SYS_BUS_DEVICE(dev);
+sysbus_mmio_map(busdev, 0, base);
+sysbus_connect_irq(busdev, 0, irq);
+}
+
+#endif /* HW_DMA_PL330_H */
-- 
2.17.2