[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCHv4 03/13] sparc32_dma: move type declarations fro
From: |
Philippe Mathieu-Daudé |
Subject: |
Re: [Qemu-devel] [PATCHv4 03/13] sparc32_dma: move type declarations from sparc32_dma.c to sparc32_dma.h |
Date: |
Fri, 27 Oct 2017 13:04:19 -0300 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.4.0 |
Hi Mark,
On 10/25/2017 12:59 PM, Mark Cave-Ayland wrote:
> Signed-off-by: Mark Cave-Ayland <address@hidden>
> Reviewed-by: Artyom Tarasenko <address@hidden>
> ---
> hw/dma/sparc32_dma.c | 34 ----------------------------------
> include/hw/sparc/sparc32_dma.h | 37 +++++++++++++++++++++++++++++++++++++
> 2 files changed, 37 insertions(+), 34 deletions(-)
>
> diff --git a/hw/dma/sparc32_dma.c b/hw/dma/sparc32_dma.c
> index e4ff4a8..ae8fa06 100644
> --- a/hw/dma/sparc32_dma.c
> +++ b/hw/dma/sparc32_dma.c
> @@ -40,7 +40,6 @@
> * http://www.ibiblio.org/pub/historic-linux/early-ports/Sparc/NCR/DMA2.txt
> */
>
> -#define DMA_REGS 4
> #define DMA_SIZE (4 * sizeof(uint32_t))
Since you move this out, can you replace with:
#define DMA_SIZE (DMA_REGS * sizeof(uint32_t))
> /* We need the mask, because one instance of the device is not page
> aligned (ledma, start address 0x0010) */
> @@ -61,39 +60,6 @@
> /* XXX SCSI and ethernet should have different read-only bit masks */
> #define DMA_CSR_RO_MASK 0xfe000007
>
> -#define TYPE_SPARC32_DMA_DEVICE "sparc32-dma-device"
> -#define SPARC32_DMA_DEVICE(obj) OBJECT_CHECK(DMADeviceState, (obj), \
> - TYPE_SPARC32_DMA_DEVICE)
> -
> -typedef struct DMADeviceState DMADeviceState;
> -
> -struct DMADeviceState {
> - SysBusDevice parent_obj;
> -
> - MemoryRegion iomem;
> - uint32_t dmaregs[DMA_REGS];
> - qemu_irq irq;
> - void *iommu;
> - qemu_irq gpio[2];
OK to move until here to sparc32_dma.h
However I don't like having now network/scsi fields mixed in
sparc32_dma.h which is supposed to be device agnostic IMHO.
So I'd rather keep this part here
(from here ...
> - uint32_t is_ledma;
> -};
> -
> -#define TYPE_SPARC32_ESPDMA_DEVICE "sparc32-espdma"
> -#define SPARC32_ESPDMA_DEVICE(obj) OBJECT_CHECK(ESPDMADeviceState, (obj), \
> - TYPE_SPARC32_ESPDMA_DEVICE)
> -
> -typedef struct ESPDMADeviceState {
> - DMADeviceState parent_obj;
> -} ESPDMADeviceState;
> -
> -#define TYPE_SPARC32_LEDMA_DEVICE "sparc32-ledma"
> -#define SPARC32_LEDMA_DEVICE(obj) OBJECT_CHECK(LEDMADeviceState, (obj), \
> - TYPE_SPARC32_LEDMA_DEVICE)
> -
> -typedef struct LEDMADeviceState {
> - DMADeviceState parent_obj;
> -} LEDMADeviceState;
... until here)
Or move it to hw/sparc/sun4m.c (or maybe clever in "hw/sparc/sun4m_dma.h").
> -
> enum {
> GPIO_RESET = 0,
> GPIO_DMA,
> diff --git a/include/hw/sparc/sparc32_dma.h b/include/hw/sparc/sparc32_dma.h
> index 9497b13..df7491d 100644
> --- a/include/hw/sparc/sparc32_dma.h
> +++ b/include/hw/sparc/sparc32_dma.h
> @@ -1,6 +1,43 @@
> #ifndef SPARC32_DMA_H
> #define SPARC32_DMA_H
>
> +#include "hw/sysbus.h"
> +
> +#define DMA_REGS 4
> +
> +#define TYPE_SPARC32_DMA_DEVICE "sparc32-dma-device"
> +#define SPARC32_DMA_DEVICE(obj) OBJECT_CHECK(DMADeviceState, (obj), \
> + TYPE_SPARC32_DMA_DEVICE)
> +
> +typedef struct DMADeviceState DMADeviceState;
> +
> +struct DMADeviceState {
> + SysBusDevice parent_obj;
> +
> + MemoryRegion iomem;
> + uint32_t dmaregs[DMA_REGS];
> + qemu_irq irq;
> + void *iommu;
> + qemu_irq gpio[2];
};
[no ...
> + uint32_t is_ledma;
> +};
> +
> +#define TYPE_SPARC32_ESPDMA_DEVICE "sparc32-espdma"
> +#define SPARC32_ESPDMA_DEVICE(obj) OBJECT_CHECK(ESPDMADeviceState, (obj), \
> + TYPE_SPARC32_ESPDMA_DEVICE)
> +
> +typedef struct ESPDMADeviceState {
> + DMADeviceState parent_obj;
> +} ESPDMADeviceState;
> +
> +#define TYPE_SPARC32_LEDMA_DEVICE "sparc32-ledma"
> +#define SPARC32_LEDMA_DEVICE(obj) OBJECT_CHECK(LEDMADeviceState, (obj), \
> + TYPE_SPARC32_LEDMA_DEVICE)
> +
> +typedef struct LEDMADeviceState {
> + DMADeviceState parent_obj;
> +} LEDMADeviceState;
> +
... no]
> /* sparc32_dma.c */
> void ledma_memory_read(void *opaque, hwaddr addr,
> uint8_t *buf, int len, int do_bswap);
>
What do you think?
Regards,
Phil.
- [Qemu-devel] [PATCHv4 09/13] lance: move TYPE_LANCE and SysBusPCNetState from lance.c to lance.h, (continued)
- [Qemu-devel] [PATCHv4 09/13] lance: move TYPE_LANCE and SysBusPCNetState from lance.c to lance.h, Mark Cave-Ayland, 2017/10/25
- Re: [Qemu-devel] [PATCHv4 09/13] lance: move TYPE_LANCE and SysBusPCNetState from lance.c to lance.h, Peter Maydell, 2017/10/25
- Re: [Qemu-devel] [PATCHv4 09/13] lance: move TYPE_LANCE and SysBusPCNetState from lance.c to lance.h, Mark Cave-Ayland, 2017/10/26
- Re: [Qemu-devel] [PATCHv4 09/13] lance: move TYPE_LANCE and SysBusPCNetState from lance.c to lance.h, Mark Cave-Ayland, 2017/10/30
- Re: [Qemu-devel] [PATCHv4 09/13] lance: move TYPE_LANCE and SysBusPCNetState from lance.c to lance.h, Peter Maydell, 2017/10/30
- Re: [Qemu-devel] [PATCHv4 09/13] lance: move TYPE_LANCE and SysBusPCNetState from lance.c to lance.h, Mark Cave-Ayland, 2017/10/30
- Re: [Qemu-devel] [PATCHv4 09/13] lance: move TYPE_LANCE and SysBusPCNetState from lance.c to lance.h, Philippe Mathieu-Daudé, 2017/10/30
Re: [Qemu-devel] [PATCHv4 09/13] lance: move TYPE_LANCE and SysBusPCNetState from lance.c to lance.h, Philippe Mathieu-Daudé, 2017/10/27
[Qemu-devel] [PATCHv4 06/13] sparc32_dma: use object link instead of qdev property to pass IOMMU reference, Mark Cave-Ayland, 2017/10/25
[Qemu-devel] [PATCHv4 03/13] sparc32_dma: move type declarations from sparc32_dma.c to sparc32_dma.h, Mark Cave-Ayland, 2017/10/25
[Qemu-devel] [PATCHv4 08/13] sparc32_dma: make esp device child of espdma device, Mark Cave-Ayland, 2017/10/25
[Qemu-devel] [PATCHv4 12/13] sparc32_dma: remove is_ledma hack and replace with memory region alias, Mark Cave-Ayland, 2017/10/25
[Qemu-devel] [PATCHv4 10/13] sparc32_dma: make lance device child of ledma device, Mark Cave-Ayland, 2017/10/25
[Qemu-devel] [PATCHv4 11/13] sparc32_dma: introduce new SPARC32_DMA type container object, Mark Cave-Ayland, 2017/10/25