[PATCH 5/7] hw/ide: Move IDE DMA related definitions to a separate header ide-dma.h

Thomas Huth posted 7 patches 8 months, 4 weeks ago
Maintainers: Paolo Bonzini <pbonzini@redhat.com>, Peter Maydell <peter.maydell@linaro.org>, Richard Henderson <richard.henderson@linaro.org>, Eduardo Habkost <eduardo@habkost.net>, "Michael S. Tsirkin" <mst@redhat.com>, Marcel Apfelbaum <marcel.apfelbaum@gmail.com>, John Snow <jsnow@redhat.com>, BALATON Zoltan <balaton@eik.bme.hu>
There is a newer version of this series
[PATCH 5/7] hw/ide: Move IDE DMA related definitions to a separate header ide-dma.h
Posted by Thomas Huth 8 months, 4 weeks ago
These definitions are required outside of the hw/ide/ code, too,
so lets's move them from internal.h to a new header called ide-dma.h.

Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 include/hw/ide/ide-dma.h  | 30 ++++++++++++++++++++++++++++++
 include/hw/ide/internal.h | 27 +--------------------------
 2 files changed, 31 insertions(+), 26 deletions(-)
 create mode 100644 include/hw/ide/ide-dma.h

diff --git a/include/hw/ide/ide-dma.h b/include/hw/ide/ide-dma.h
new file mode 100644
index 0000000000..fb82966bdd
--- /dev/null
+++ b/include/hw/ide/ide-dma.h
@@ -0,0 +1,30 @@
+#ifndef HW_IDE_DMA_H
+#define HW_IDE_DMA_H
+
+typedef void DMAStartFunc(const IDEDMA *, IDEState *, BlockCompletionFunc *);
+typedef void DMAVoidFunc(const IDEDMA *);
+typedef int DMAIntFunc(const IDEDMA *, bool);
+typedef int32_t DMAInt32Func(const IDEDMA *, int32_t len);
+typedef void DMAu32Func(const IDEDMA *, uint32_t);
+typedef void DMAStopFunc(const IDEDMA *, bool);
+
+struct IDEDMAOps {
+    DMAStartFunc *start_dma;
+    DMAVoidFunc *pio_transfer;
+    DMAInt32Func *prepare_buf;
+    DMAu32Func *commit_buf;
+    DMAIntFunc *rw_buf;
+    DMAVoidFunc *restart;
+    DMAVoidFunc *restart_dma;
+    DMAStopFunc *set_inactive;
+    DMAVoidFunc *cmd_done;
+    DMAVoidFunc *reset;
+};
+
+struct IDEDMA {
+    const struct IDEDMAOps *ops;
+    QEMUIOVector qiov;
+    BlockAIOCB *aiocb;
+};
+
+#endif
diff --git a/include/hw/ide/internal.h b/include/hw/ide/internal.h
index 642bd1a979..d1d3fcd23a 100644
--- a/include/hw/ide/internal.h
+++ b/include/hw/ide/internal.h
@@ -9,6 +9,7 @@
 
 #include "hw/ide.h"
 #include "hw/ide/ide-bus.h"
+#include "hw/ide/ide-dma.h"
 
 /* debug IDE devices */
 #define USE_DMA_CDROM
@@ -316,13 +317,6 @@
 #define SMART_DISABLE         0xd9
 #define SMART_STATUS          0xda
 
-typedef void DMAStartFunc(const IDEDMA *, IDEState *, BlockCompletionFunc *);
-typedef void DMAVoidFunc(const IDEDMA *);
-typedef int DMAIntFunc(const IDEDMA *, bool);
-typedef int32_t DMAInt32Func(const IDEDMA *, int32_t len);
-typedef void DMAu32Func(const IDEDMA *, uint32_t);
-typedef void DMAStopFunc(const IDEDMA *, bool);
-
 extern const char *IDE_DMA_CMD_lookup[IDE_DMA__COUNT];
 
 extern const MemoryRegionPortio ide_portio_list[];
@@ -340,25 +334,6 @@ typedef struct IDEBufferedRequest {
     bool orphaned;
 } IDEBufferedRequest;
 
-struct IDEDMAOps {
-    DMAStartFunc *start_dma;
-    DMAVoidFunc *pio_transfer;
-    DMAInt32Func *prepare_buf;
-    DMAu32Func *commit_buf;
-    DMAIntFunc *rw_buf;
-    DMAVoidFunc *restart;
-    DMAVoidFunc *restart_dma;
-    DMAStopFunc *set_inactive;
-    DMAVoidFunc *cmd_done;
-    DMAVoidFunc *reset;
-};
-
-struct IDEDMA {
-    const struct IDEDMAOps *ops;
-    QEMUIOVector qiov;
-    BlockAIOCB *aiocb;
-};
-
 /* These are used for the error_status field of IDEBus */
 #define IDE_RETRY_MASK 0xf8
 #define IDE_RETRY_DMA  0x08
-- 
2.43.2
Re: [PATCH 5/7] hw/ide: Move IDE DMA related definitions to a separate header ide-dma.h
Posted by BALATON Zoltan 8 months, 4 weeks ago
On Mon, 19 Feb 2024, Thomas Huth wrote:
> These definitions are required outside of the hw/ide/ code, too,
> so lets's move them from internal.h to a new header called ide-dma.h.
>
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
> include/hw/ide/ide-dma.h  | 30 ++++++++++++++++++++++++++++++
> include/hw/ide/internal.h | 27 +--------------------------
> 2 files changed, 31 insertions(+), 26 deletions(-)
> create mode 100644 include/hw/ide/ide-dma.h
>
> diff --git a/include/hw/ide/ide-dma.h b/include/hw/ide/ide-dma.h
> new file mode 100644
> index 0000000000..fb82966bdd
> --- /dev/null
> +++ b/include/hw/ide/ide-dma.h
> @@ -0,0 +1,30 @@
> +#ifndef HW_IDE_DMA_H
> +#define HW_IDE_DMA_H
> +
> +typedef void DMAStartFunc(const IDEDMA *, IDEState *, BlockCompletionFunc *);
> +typedef void DMAVoidFunc(const IDEDMA *);
> +typedef int DMAIntFunc(const IDEDMA *, bool);
> +typedef int32_t DMAInt32Func(const IDEDMA *, int32_t len);
> +typedef void DMAu32Func(const IDEDMA *, uint32_t);
> +typedef void DMAStopFunc(const IDEDMA *, bool);
> +
> +struct IDEDMAOps {
> +    DMAStartFunc *start_dma;
> +    DMAVoidFunc *pio_transfer;
> +    DMAInt32Func *prepare_buf;
> +    DMAu32Func *commit_buf;
> +    DMAIntFunc *rw_buf;
> +    DMAVoidFunc *restart;
> +    DMAVoidFunc *restart_dma;
> +    DMAStopFunc *set_inactive;
> +    DMAVoidFunc *cmd_done;
> +    DMAVoidFunc *reset;
> +};
> +
> +struct IDEDMA {
> +    const struct IDEDMAOps *ops;
> +    QEMUIOVector qiov;
> +    BlockAIOCB *aiocb;

Doesn't this need to #include something to define QEMUIOVector and 
BlockAIOCB and some of the DMA and IDE types not defined above?

Regards,
BALATON Zoltan

> +};
> +
> +#endif
> diff --git a/include/hw/ide/internal.h b/include/hw/ide/internal.h
> index 642bd1a979..d1d3fcd23a 100644
> --- a/include/hw/ide/internal.h
> +++ b/include/hw/ide/internal.h
> @@ -9,6 +9,7 @@
>
> #include "hw/ide.h"
> #include "hw/ide/ide-bus.h"
> +#include "hw/ide/ide-dma.h"
>
> /* debug IDE devices */
> #define USE_DMA_CDROM
> @@ -316,13 +317,6 @@
> #define SMART_DISABLE         0xd9
> #define SMART_STATUS          0xda
>
> -typedef void DMAStartFunc(const IDEDMA *, IDEState *, BlockCompletionFunc *);
> -typedef void DMAVoidFunc(const IDEDMA *);
> -typedef int DMAIntFunc(const IDEDMA *, bool);
> -typedef int32_t DMAInt32Func(const IDEDMA *, int32_t len);
> -typedef void DMAu32Func(const IDEDMA *, uint32_t);
> -typedef void DMAStopFunc(const IDEDMA *, bool);
> -
> extern const char *IDE_DMA_CMD_lookup[IDE_DMA__COUNT];
>
> extern const MemoryRegionPortio ide_portio_list[];
> @@ -340,25 +334,6 @@ typedef struct IDEBufferedRequest {
>     bool orphaned;
> } IDEBufferedRequest;
>
> -struct IDEDMAOps {
> -    DMAStartFunc *start_dma;
> -    DMAVoidFunc *pio_transfer;
> -    DMAInt32Func *prepare_buf;
> -    DMAu32Func *commit_buf;
> -    DMAIntFunc *rw_buf;
> -    DMAVoidFunc *restart;
> -    DMAVoidFunc *restart_dma;
> -    DMAStopFunc *set_inactive;
> -    DMAVoidFunc *cmd_done;
> -    DMAVoidFunc *reset;
> -};
> -
> -struct IDEDMA {
> -    const struct IDEDMAOps *ops;
> -    QEMUIOVector qiov;
> -    BlockAIOCB *aiocb;
> -};
> -
> /* These are used for the error_status field of IDEBus */
> #define IDE_RETRY_MASK 0xf8
> #define IDE_RETRY_DMA  0x08
>
Re: [PATCH 5/7] hw/ide: Move IDE DMA related definitions to a separate header ide-dma.h
Posted by Thomas Huth 8 months, 4 weeks ago
On 19/02/2024 12.53, BALATON Zoltan wrote:
> On Mon, 19 Feb 2024, Thomas Huth wrote:
>> These definitions are required outside of the hw/ide/ code, too,
>> so lets's move them from internal.h to a new header called ide-dma.h.
>>
>> Signed-off-by: Thomas Huth <thuth@redhat.com>
>> ---
>> include/hw/ide/ide-dma.h  | 30 ++++++++++++++++++++++++++++++
>> include/hw/ide/internal.h | 27 +--------------------------
>> 2 files changed, 31 insertions(+), 26 deletions(-)
>> create mode 100644 include/hw/ide/ide-dma.h
>>
>> diff --git a/include/hw/ide/ide-dma.h b/include/hw/ide/ide-dma.h
>> new file mode 100644
>> index 0000000000..fb82966bdd
>> --- /dev/null
>> +++ b/include/hw/ide/ide-dma.h
>> @@ -0,0 +1,30 @@
>> +#ifndef HW_IDE_DMA_H
>> +#define HW_IDE_DMA_H
>> +
>> +typedef void DMAStartFunc(const IDEDMA *, IDEState *, BlockCompletionFunc 
>> *);
>> +typedef void DMAVoidFunc(const IDEDMA *);
>> +typedef int DMAIntFunc(const IDEDMA *, bool);
>> +typedef int32_t DMAInt32Func(const IDEDMA *, int32_t len);
>> +typedef void DMAu32Func(const IDEDMA *, uint32_t);
>> +typedef void DMAStopFunc(const IDEDMA *, bool);
>> +
>> +struct IDEDMAOps {
>> +    DMAStartFunc *start_dma;
>> +    DMAVoidFunc *pio_transfer;
>> +    DMAInt32Func *prepare_buf;
>> +    DMAu32Func *commit_buf;
>> +    DMAIntFunc *rw_buf;
>> +    DMAVoidFunc *restart;
>> +    DMAVoidFunc *restart_dma;
>> +    DMAStopFunc *set_inactive;
>> +    DMAVoidFunc *cmd_done;
>> +    DMAVoidFunc *reset;
>> +};
>> +
>> +struct IDEDMA {
>> +    const struct IDEDMAOps *ops;
>> +    QEMUIOVector qiov;
>> +    BlockAIOCB *aiocb;
> 
> Doesn't this need to #include something to define QEMUIOVector and 
> BlockAIOCB and some of the DMA and IDE types not defined above?

Yes, it currently works by accident since the header is only included in 
spots where those things are already defined, but I'll better add some 
#include statements here so that this header can also be used stand-alone in 
other spots.

  Thomas