diff mbox series

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

Message ID 20180809130115.28951-11-peter.maydell@linaro.org
State Superseded
Headers show
Series arm: Implement MPS2 watchdogs and DMA | expand

Commit Message

Peter Maydell Aug. 9, 2018, 1:01 p.m. UTC 
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

-- 
2.17.1

Comments

Philippe Mathieu-Daudé Aug. 10, 2018, 5:18 a.m. UTC | #1 
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

>
Philippe Mathieu-Daudé Aug. 10, 2018, 5:27 a.m. UTC | #2 
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

>>
Peter Maydell Aug. 10, 2018, 9:03 a.m. UTC | #3 
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
diff mbox series

Patch

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