[Qemu-devel] [PATCH 10/16] hw/dma/pl080: Allow use as embedded-struct device

Peter Maydell posted 16 patches 7 years, 2 months ago
[Qemu-devel] [PATCH 10/16] hw/dma/pl080: Allow use as embedded-struct device
Posted by Peter Maydell 7 years, 2 months ago
Create a new include file for the pl081's device struct,
type macros, etc, so that it can be instantiated using
the "embedded struct" coding style.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 include/hw/dma/pl080.h | 62 ++++++++++++++++++++++++++++++++++++++++++
 hw/dma/pl080.c         | 34 ++---------------------
 MAINTAINERS            |  1 +
 3 files changed, 65 insertions(+), 32 deletions(-)
 create mode 100644 include/hw/dma/pl080.h

diff --git a/include/hw/dma/pl080.h b/include/hw/dma/pl080.h
new file mode 100644
index 00000000000..7deb46c8578
--- /dev/null
+++ b/include/hw/dma/pl080.h
@@ -0,0 +1,62 @@
+/*
+ * ARM PrimeCell PL080/PL081 DMA controller
+ *
+ * Copyright (c) 2006 CodeSourcery.
+ * Copyright (c) 2018 Linaro Limited
+ * Written by Paul Brook, Peter Maydell
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 or
+ * (at your option) any later version.
+ */
+
+/* This is a model of the Arm PrimeCell PL080/PL081 DMA controller:
+ * The PL080 TRM is:
+ * http://infocenter.arm.com/help/topic/com.arm.doc.ddi0196g/DDI0196.pdf
+ * and the PL081 TRM is:
+ * http://infocenter.arm.com/help/topic/com.arm.doc.ddi0218e/DDI0218.pdf
+ *
+ * QEMU interface:
+ * + sysbus IRQ: DMACINTR combined interrupt line
+ * + sysbus MMIO region 0: MemoryRegion for the device's registers
+ */
+
+#ifndef HW_DMA_PL080_H
+#define HW_DMA_PL080_H
+
+#include "hw/sysbus.h"
+
+#define PL080_MAX_CHANNELS 8
+
+typedef struct {
+    uint32_t src;
+    uint32_t dest;
+    uint32_t lli;
+    uint32_t ctrl;
+    uint32_t conf;
+} pl080_channel;
+
+#define TYPE_PL080 "pl080"
+#define TYPE_PL081 "pl081"
+#define PL080(obj) OBJECT_CHECK(PL080State, (obj), TYPE_PL080)
+
+typedef struct PL080State {
+    SysBusDevice parent_obj;
+
+    MemoryRegion iomem;
+    uint8_t tc_int;
+    uint8_t tc_mask;
+    uint8_t err_int;
+    uint8_t err_mask;
+    uint32_t conf;
+    uint32_t sync;
+    uint32_t req_single;
+    uint32_t req_burst;
+    pl080_channel chan[PL080_MAX_CHANNELS];
+    int nchannels;
+    /* Flag to avoid recursive DMA invocations.  */
+    int running;
+    qemu_irq irq;
+} PL080State;
+
+#endif
diff --git a/hw/dma/pl080.c b/hw/dma/pl080.c
index 7724c93b8f2..0f79c2d8a6c 100644
--- a/hw/dma/pl080.c
+++ b/hw/dma/pl080.c
@@ -11,8 +11,8 @@
 #include "hw/sysbus.h"
 #include "exec/address-spaces.h"
 #include "qemu/log.h"
+#include "hw/dma/pl080.h"
 
-#define PL080_MAX_CHANNELS 8
 #define PL080_CONF_E    0x1
 #define PL080_CONF_M1   0x2
 #define PL080_CONF_M2   0x4
@@ -30,36 +30,6 @@
 #define PL080_CCTRL_D   0x02000000
 #define PL080_CCTRL_S   0x01000000
 
-typedef struct {
-    uint32_t src;
-    uint32_t dest;
-    uint32_t lli;
-    uint32_t ctrl;
-    uint32_t conf;
-} pl080_channel;
-
-#define TYPE_PL080 "pl080"
-#define PL080(obj) OBJECT_CHECK(PL080State, (obj), TYPE_PL080)
-
-typedef struct PL080State {
-    SysBusDevice parent_obj;
-
-    MemoryRegion iomem;
-    uint8_t tc_int;
-    uint8_t tc_mask;
-    uint8_t err_int;
-    uint8_t err_mask;
-    uint32_t conf;
-    uint32_t sync;
-    uint32_t req_single;
-    uint32_t req_burst;
-    pl080_channel chan[PL080_MAX_CHANNELS];
-    int nchannels;
-    /* Flag to avoid recursive DMA invocations.  */
-    int running;
-    qemu_irq irq;
-} PL080State;
-
 static const VMStateDescription vmstate_pl080_channel = {
     .name = "pl080_channel",
     .version_id = 1,
@@ -408,7 +378,7 @@ static const TypeInfo pl080_info = {
 };
 
 static const TypeInfo pl081_info = {
-    .name          = "pl081",
+    .name          = TYPE_PL081,
     .parent        = TYPE_PL080,
     .instance_init = pl081_init,
 };
diff --git a/MAINTAINERS b/MAINTAINERS
index 5d1a3645dd4..92ccca716c6 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -444,6 +444,7 @@ F: hw/char/pl011.c
 F: include/hw/char/pl011.h
 F: hw/display/pl110*
 F: hw/dma/pl080.c
+F: include/hw/dma/pl080.h
 F: hw/dma/pl330.c
 F: hw/gpio/pl061.c
 F: hw/input/pl050.c
-- 
2.17.1


Re: [Qemu-devel] [Qemu-arm] [PATCH 10/16] hw/dma/pl080: Allow use as embedded-struct device
Posted by Philippe Mathieu-Daudé 7 years, 2 months ago
On 08/09/2018 10:01 AM, Peter Maydell wrote:
> Create a new include file for the pl081's device struct,
> type macros, etc, so that it can be instantiated using
> the "embedded struct" coding style.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  include/hw/dma/pl080.h | 62 ++++++++++++++++++++++++++++++++++++++++++
>  hw/dma/pl080.c         | 34 ++---------------------
>  MAINTAINERS            |  1 +
>  3 files changed, 65 insertions(+), 32 deletions(-)
>  create mode 100644 include/hw/dma/pl080.h
> 
> diff --git a/include/hw/dma/pl080.h b/include/hw/dma/pl080.h
> new file mode 100644
> index 00000000000..7deb46c8578
> --- /dev/null
> +++ b/include/hw/dma/pl080.h
> @@ -0,0 +1,62 @@
> +/*
> + * ARM PrimeCell PL080/PL081 DMA controller
> + *
> + * Copyright (c) 2006 CodeSourcery.
> + * Copyright (c) 2018 Linaro Limited
> + * Written by Paul Brook, Peter Maydell
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 or
> + * (at your option) any later version.
> + */
> +
> +/* This is a model of the Arm PrimeCell PL080/PL081 DMA controller:
> + * The PL080 TRM is:
> + * http://infocenter.arm.com/help/topic/com.arm.doc.ddi0196g/DDI0196.pdf
> + * and the PL081 TRM is:
> + * http://infocenter.arm.com/help/topic/com.arm.doc.ddi0218e/DDI0218.pdf
> + *
> + * QEMU interface:
> + * + sysbus IRQ: DMACINTR combined interrupt line
> + * + sysbus MMIO region 0: MemoryRegion for the device's registers
> + */
> +
> +#ifndef HW_DMA_PL080_H
> +#define HW_DMA_PL080_H
> +
> +#include "hw/sysbus.h"
> +
> +#define PL080_MAX_CHANNELS 8
> +
> +typedef struct {
> +    uint32_t src;
> +    uint32_t dest;
> +    uint32_t lli;
> +    uint32_t ctrl;
> +    uint32_t conf;
> +} pl080_channel;
> +
> +#define TYPE_PL080 "pl080"
> +#define TYPE_PL081 "pl081"
> +#define PL080(obj) OBJECT_CHECK(PL080State, (obj), TYPE_PL080)

The PL080() macro can stay in the source.

Regardless:
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

> +
> +typedef struct PL080State {
> +    SysBusDevice parent_obj;
> +
> +    MemoryRegion iomem;
> +    uint8_t tc_int;
> +    uint8_t tc_mask;
> +    uint8_t err_int;
> +    uint8_t err_mask;
> +    uint32_t conf;
> +    uint32_t sync;
> +    uint32_t req_single;
> +    uint32_t req_burst;
> +    pl080_channel chan[PL080_MAX_CHANNELS];
> +    int nchannels;
> +    /* Flag to avoid recursive DMA invocations.  */
> +    int running;
> +    qemu_irq irq;
> +} PL080State;
> +
> +#endif
> diff --git a/hw/dma/pl080.c b/hw/dma/pl080.c
> index 7724c93b8f2..0f79c2d8a6c 100644
> --- a/hw/dma/pl080.c
> +++ b/hw/dma/pl080.c
> @@ -11,8 +11,8 @@
>  #include "hw/sysbus.h"
>  #include "exec/address-spaces.h"
>  #include "qemu/log.h"
> +#include "hw/dma/pl080.h"
>  
> -#define PL080_MAX_CHANNELS 8
>  #define PL080_CONF_E    0x1
>  #define PL080_CONF_M1   0x2
>  #define PL080_CONF_M2   0x4
> @@ -30,36 +30,6 @@
>  #define PL080_CCTRL_D   0x02000000
>  #define PL080_CCTRL_S   0x01000000
>  
> -typedef struct {
> -    uint32_t src;
> -    uint32_t dest;
> -    uint32_t lli;
> -    uint32_t ctrl;
> -    uint32_t conf;
> -} pl080_channel;
> -
> -#define TYPE_PL080 "pl080"
> -#define PL080(obj) OBJECT_CHECK(PL080State, (obj), TYPE_PL080)
> -
> -typedef struct PL080State {
> -    SysBusDevice parent_obj;
> -
> -    MemoryRegion iomem;
> -    uint8_t tc_int;
> -    uint8_t tc_mask;
> -    uint8_t err_int;
> -    uint8_t err_mask;
> -    uint32_t conf;
> -    uint32_t sync;
> -    uint32_t req_single;
> -    uint32_t req_burst;
> -    pl080_channel chan[PL080_MAX_CHANNELS];
> -    int nchannels;
> -    /* Flag to avoid recursive DMA invocations.  */
> -    int running;
> -    qemu_irq irq;
> -} PL080State;
> -
>  static const VMStateDescription vmstate_pl080_channel = {
>      .name = "pl080_channel",
>      .version_id = 1,
> @@ -408,7 +378,7 @@ static const TypeInfo pl080_info = {
>  };
>  
>  static const TypeInfo pl081_info = {
> -    .name          = "pl081",
> +    .name          = TYPE_PL081,
>      .parent        = TYPE_PL080,
>      .instance_init = pl081_init,
>  };
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 5d1a3645dd4..92ccca716c6 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -444,6 +444,7 @@ F: hw/char/pl011.c
>  F: include/hw/char/pl011.h
>  F: hw/display/pl110*
>  F: hw/dma/pl080.c
> +F: include/hw/dma/pl080.h
>  F: hw/dma/pl330.c
>  F: hw/gpio/pl061.c
>  F: hw/input/pl050.c
> 

Re: [Qemu-devel] [Qemu-arm] [PATCH 10/16] hw/dma/pl080: Allow use as embedded-struct device
Posted by Philippe Mathieu-Daudé 7 years, 2 months ago
On 08/10/2018 02:18 AM, Philippe Mathieu-Daudé wrote:
> On 08/09/2018 10:01 AM, Peter Maydell wrote:
>> Create a new include file for the pl081's device struct,
>> type macros, etc, so that it can be instantiated using
>> the "embedded struct" coding style.
[...]
>> +#ifndef HW_DMA_PL080_H
>> +#define HW_DMA_PL080_H
>> +
>> +#include "hw/sysbus.h"
>> +
>> +#define PL080_MAX_CHANNELS 8
>> +
>> +typedef struct {
>> +    uint32_t src;
>> +    uint32_t dest;
>> +    uint32_t lli;
>> +    uint32_t ctrl;
>> +    uint32_t conf;
>> +} pl080_channel;
>> +
>> +#define TYPE_PL080 "pl080"
>> +#define TYPE_PL081 "pl081"
>> +#define PL080(obj) OBJECT_CHECK(PL080State, (obj), TYPE_PL080)
> 
> The PL080() macro can stay in the source.

Oh this respect the "coding style" indeed.
Can you add the PL081() companion? Thanks.

> 
> Regardless:
> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> 
>> +
>> +typedef struct PL080State {
>> +    SysBusDevice parent_obj;
>> +
>> +    MemoryRegion iomem;
>> +    uint8_t tc_int;
>> +    uint8_t tc_mask;
>> +    uint8_t err_int;
>> +    uint8_t err_mask;
>> +    uint32_t conf;
>> +    uint32_t sync;
>> +    uint32_t req_single;
>> +    uint32_t req_burst;
>> +    pl080_channel chan[PL080_MAX_CHANNELS];
>> +    int nchannels;
>> +    /* Flag to avoid recursive DMA invocations.  */
>> +    int running;
>> +    qemu_irq irq;
>> +} PL080State;
>> +
>> +#endif
>> diff --git a/hw/dma/pl080.c b/hw/dma/pl080.c
>> index 7724c93b8f2..0f79c2d8a6c 100644
>> --- a/hw/dma/pl080.c
>> +++ b/hw/dma/pl080.c
>> @@ -11,8 +11,8 @@
>>  #include "hw/sysbus.h"
>>  #include "exec/address-spaces.h"
>>  #include "qemu/log.h"
>> +#include "hw/dma/pl080.h"
>>  
>> -#define PL080_MAX_CHANNELS 8
>>  #define PL080_CONF_E    0x1
>>  #define PL080_CONF_M1   0x2
>>  #define PL080_CONF_M2   0x4
>> @@ -30,36 +30,6 @@
>>  #define PL080_CCTRL_D   0x02000000
>>  #define PL080_CCTRL_S   0x01000000
>>  
>> -typedef struct {
>> -    uint32_t src;
>> -    uint32_t dest;
>> -    uint32_t lli;
>> -    uint32_t ctrl;
>> -    uint32_t conf;
>> -} pl080_channel;
>> -
>> -#define TYPE_PL080 "pl080"
>> -#define PL080(obj) OBJECT_CHECK(PL080State, (obj), TYPE_PL080)
>> -
>> -typedef struct PL080State {
>> -    SysBusDevice parent_obj;
>> -
>> -    MemoryRegion iomem;
>> -    uint8_t tc_int;
>> -    uint8_t tc_mask;
>> -    uint8_t err_int;
>> -    uint8_t err_mask;
>> -    uint32_t conf;
>> -    uint32_t sync;
>> -    uint32_t req_single;
>> -    uint32_t req_burst;
>> -    pl080_channel chan[PL080_MAX_CHANNELS];
>> -    int nchannels;
>> -    /* Flag to avoid recursive DMA invocations.  */
>> -    int running;
>> -    qemu_irq irq;
>> -} PL080State;
>> -
>>  static const VMStateDescription vmstate_pl080_channel = {
>>      .name = "pl080_channel",
>>      .version_id = 1,
>> @@ -408,7 +378,7 @@ static const TypeInfo pl080_info = {
>>  };
>>  
>>  static const TypeInfo pl081_info = {
>> -    .name          = "pl081",
>> +    .name          = TYPE_PL081,
>>      .parent        = TYPE_PL080,
>>      .instance_init = pl081_init,
>>  };
>> diff --git a/MAINTAINERS b/MAINTAINERS
>> index 5d1a3645dd4..92ccca716c6 100644
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
>> @@ -444,6 +444,7 @@ F: hw/char/pl011.c
>>  F: include/hw/char/pl011.h
>>  F: hw/display/pl110*
>>  F: hw/dma/pl080.c
>> +F: include/hw/dma/pl080.h
>>  F: hw/dma/pl330.c
>>  F: hw/gpio/pl061.c
>>  F: hw/input/pl050.c
>>

Re: [Qemu-devel] [Qemu-arm] [PATCH 10/16] hw/dma/pl080: Allow use as embedded-struct device
Posted by Peter Maydell 7 years, 2 months ago
On 10 August 2018 at 06:27, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
> On 08/10/2018 02:18 AM, Philippe Mathieu-Daudé wrote:
>> On 08/09/2018 10:01 AM, Peter Maydell wrote:
>>> Create a new include file for the pl081's device struct,
>>> type macros, etc, so that it can be instantiated using
>>> the "embedded struct" coding style.
> [...]
>>> +#ifndef HW_DMA_PL080_H
>>> +#define HW_DMA_PL080_H
>>> +
>>> +#include "hw/sysbus.h"
>>> +
>>> +#define PL080_MAX_CHANNELS 8
>>> +
>>> +typedef struct {
>>> +    uint32_t src;
>>> +    uint32_t dest;
>>> +    uint32_t lli;
>>> +    uint32_t ctrl;
>>> +    uint32_t conf;
>>> +} pl080_channel;
>>> +
>>> +#define TYPE_PL080 "pl080"
>>> +#define TYPE_PL081 "pl081"
>>> +#define PL080(obj) OBJECT_CHECK(PL080State, (obj), TYPE_PL080)
>>
>> The PL080() macro can stay in the source.
>
> Oh this respect the "coding style" indeed.

Yes.

> Can you add the PL081() companion? Thanks.

No, because the PL081 has no separate data structure
and there is no PL081State* for a PL081() macro to
be casting to.

thanks
-- PMM