Re: [Qemu-block] [Qemu-devel] [PATCH v3 01/16] fdc: move object structures to header file

2018-01-04 Thread Hervé Poussineau 

Le 04/01/2018 à 14:11, Marcel Apfelbaum a écrit :

On 29/12/2017 16:29, Hervé Poussineau wrote:

We are now able to embed floppy controllers in another object.



Hi Hervé,

Are you sure we need to move all the struct definitions to the header file?

I looked at patch 11/16 and it seems only FDCtrlISABus definition is needed.
And also only the typedef is needed and not the fields.

It may worth a look also patches 2/16 and 3/16.


Structure FDCtrlISABus contains a structure FDCtrl (state) which contains a 
structure FloppyBus and FDrive.
So I think I need to move all these struct definitions to the header file, and 
that a typedef is not enough.

On patch 11, I include a FDCtrlISABus structure in the PIIX4State structure.
So, FDCtrlISABus structure must be shared between fdc.c and piix4.c
That's what I do when I move all these structures to fdc.h

Do you see another possibility?

Regards,

Hervé



Thanks,
Marcel


Signed-off-by: Hervé Poussineau 
---
  hw/block/fdc.c | 102 
  include/hw/block/fdc.h | 103 +
  2 files changed, 103 insertions(+), 102 deletions(-)

diff --git a/hw/block/fdc.c b/hw/block/fdc.c
index 7b7dd41296..c81e0313c8 100644
--- a/hw/block/fdc.c
+++ b/hw/block/fdc.c
@@ -60,15 +60,8 @@
  #define TYPE_FLOPPY_BUS "floppy-bus"
  #define FLOPPY_BUS(obj) OBJECT_CHECK(FloppyBus, (obj), TYPE_FLOPPY_BUS)
-typedef struct FDCtrl FDCtrl;
-typedef struct FDrive FDrive;
  static FDrive *get_drv(FDCtrl *fdctrl, int unit);
-typedef struct FloppyBus {
-    BusState bus;
-    FDCtrl *fdc;
-} FloppyBus;
-
  static const TypeInfo floppy_bus_info = {
  .name = TYPE_FLOPPY_BUS,
  .parent = TYPE_BUS,
@@ -178,36 +171,6 @@ static FDriveSize drive_size(FloppyDriveType drive)
  #define FD_SECTOR_SC   2   /* Sector size code */
  #define FD_RESET_SENSEI_COUNT  4   /* Number of sense interrupts on RESET */
-/* Floppy disk drive emulation */
-typedef enum FDiskFlags {
-    FDISK_DBL_SIDES  = 0x01,
-} FDiskFlags;
-
-struct FDrive {
-    FDCtrl *fdctrl;
-    BlockBackend *blk;
-    BlockConf *conf;
-    /* Drive status */
-    FloppyDriveType drive;    /* CMOS drive type    */
-    uint8_t perpendicular;    /* 2.88 MB access mode    */
-    /* Position */
-    uint8_t head;
-    uint8_t track;
-    uint8_t sect;
-    /* Media */
-    FloppyDriveType disk; /* Current disk type  */
-    FDiskFlags flags;
-    uint8_t last_sect;    /* Nb sector per track    */
-    uint8_t max_track;    /* Nb of tracks   */
-    uint16_t bps; /* Bytes per sector   */
-    uint8_t ro;   /* Is read-only   */
-    uint8_t media_changed;    /* Is media changed   */
-    uint8_t media_rate;   /* Data rate of medium    */
-
-    bool media_validated; /* Have we validated the media? */
-};
-
-
  static FloppyDriveType get_fallback_drive_type(FDrive *drv);
  /* Hack: FD_SEEK is expected to work on empty drives. However, QEMU
@@ -819,60 +782,6 @@ enum {
  #define FD_MULTI_TRACK(state) ((state) & FD_STATE_MULTI)
  #define FD_FORMAT_CMD(state) ((state) & FD_STATE_FORMAT)
-struct FDCtrl {
-    MemoryRegion iomem;
-    qemu_irq irq;
-    /* Controller state */
-    QEMUTimer *result_timer;
-    int dma_chann;
-    uint8_t phase;
-    IsaDma *dma;
-    /* Controller's identification */
-    uint8_t version;
-    /* HW */
-    uint8_t sra;
-    uint8_t srb;
-    uint8_t dor;
-    uint8_t dor_vmstate; /* only used as temp during vmstate */
-    uint8_t tdr;
-    uint8_t dsr;
-    uint8_t msr;
-    uint8_t cur_drv;
-    uint8_t status0;
-    uint8_t status1;
-    uint8_t status2;
-    /* Command FIFO */
-    uint8_t *fifo;
-    int32_t fifo_size;
-    uint32_t data_pos;
-    uint32_t data_len;
-    uint8_t data_state;
-    uint8_t data_dir;
-    uint8_t eot; /* last wanted sector */
-    /* States kept only to be returned back */
-    /* precompensation */
-    uint8_t precomp_trk;
-    uint8_t config;
-    uint8_t lock;
-    /* Power down config (also with status regB access mode */
-    uint8_t pwrd;
-    /* Floppy drives */
-    FloppyBus bus;
-    uint8_t num_floppies;
-    FDrive drives[MAX_FD];
-    struct {
-    BlockBackend *blk;
-    FloppyDriveType type;
-    } qdev_for_drives[MAX_FD];
-    int reset_sensei;
-    uint32_t check_media_rate;
-    FloppyDriveType fallback; /* type=auto failure fallback */
-    /* Timers state */
-    uint8_t timer0;
-    uint8_t timer1;
-    PortioList portio_list;
-};
-
  static FloppyDriveType get_fallback_drive_type(FDrive *drv)
  {
  return drv->fdctrl->fallback;
@@ -891,17 +800,6 @@ typedef struct FDCtrlSysBus {
  #define ISA_FDC(obj) OBJECT_CHECK(FDCtrlISABus, (obj), TYPE_ISA_FDC)
-typedef struct FDCtrlISABus {
-    ISADevice parent_obj;
-
-    uint32_t iobase;
-    uint32_t irq;
-    uint32_t dma;
-    struct FDCtrl state;
-    int32_t bootindexA;
-    int32_t bootindexB;
-}

Re: [Qemu-block] [Qemu-devel] [PATCH v3 01/16] fdc: move object structures to header file

2018-01-04 Thread Marcel Apfelbaum 

On 29/12/2017 16:29, Hervé Poussineau wrote:

We are now able to embed floppy controllers in another object.



Hi Hervé,

Are you sure we need to move all the struct definitions to the header file?

I looked at patch 11/16 and it seems only FDCtrlISABus definition is needed.
And also only the typedef is needed and not the fields.

It may worth a look also patches 2/16 and 3/16.

Thanks,
Marcel


Signed-off-by: Hervé Poussineau 
---
  hw/block/fdc.c | 102 
  include/hw/block/fdc.h | 103 +
  2 files changed, 103 insertions(+), 102 deletions(-)

diff --git a/hw/block/fdc.c b/hw/block/fdc.c
index 7b7dd41296..c81e0313c8 100644
--- a/hw/block/fdc.c
+++ b/hw/block/fdc.c
@@ -60,15 +60,8 @@
  #define TYPE_FLOPPY_BUS "floppy-bus"
  #define FLOPPY_BUS(obj) OBJECT_CHECK(FloppyBus, (obj), TYPE_FLOPPY_BUS)
  
-typedef struct FDCtrl FDCtrl;

-typedef struct FDrive FDrive;
  static FDrive *get_drv(FDCtrl *fdctrl, int unit);
  
-typedef struct FloppyBus {

-BusState bus;
-FDCtrl *fdc;
-} FloppyBus;
-
  static const TypeInfo floppy_bus_info = {
  .name = TYPE_FLOPPY_BUS,
  .parent = TYPE_BUS,
@@ -178,36 +171,6 @@ static FDriveSize drive_size(FloppyDriveType drive)
  #define FD_SECTOR_SC   2   /* Sector size code */
  #define FD_RESET_SENSEI_COUNT  4   /* Number of sense interrupts on RESET */
  
-/* Floppy disk drive emulation */

-typedef enum FDiskFlags {
-FDISK_DBL_SIDES  = 0x01,
-} FDiskFlags;
-
-struct FDrive {
-FDCtrl *fdctrl;
-BlockBackend *blk;
-BlockConf *conf;
-/* Drive status */
-FloppyDriveType drive;/* CMOS drive type*/
-uint8_t perpendicular;/* 2.88 MB access mode*/
-/* Position */
-uint8_t head;
-uint8_t track;
-uint8_t sect;
-/* Media */
-FloppyDriveType disk; /* Current disk type  */
-FDiskFlags flags;
-uint8_t last_sect;/* Nb sector per track*/
-uint8_t max_track;/* Nb of tracks   */
-uint16_t bps; /* Bytes per sector   */
-uint8_t ro;   /* Is read-only   */
-uint8_t media_changed;/* Is media changed   */
-uint8_t media_rate;   /* Data rate of medium*/
-
-bool media_validated; /* Have we validated the media? */
-};
-
-
  static FloppyDriveType get_fallback_drive_type(FDrive *drv);
  
  /* Hack: FD_SEEK is expected to work on empty drives. However, QEMU

@@ -819,60 +782,6 @@ enum {
  #define FD_MULTI_TRACK(state) ((state) & FD_STATE_MULTI)
  #define FD_FORMAT_CMD(state) ((state) & FD_STATE_FORMAT)
  
-struct FDCtrl {

-MemoryRegion iomem;
-qemu_irq irq;
-/* Controller state */
-QEMUTimer *result_timer;
-int dma_chann;
-uint8_t phase;
-IsaDma *dma;
-/* Controller's identification */
-uint8_t version;
-/* HW */
-uint8_t sra;
-uint8_t srb;
-uint8_t dor;
-uint8_t dor_vmstate; /* only used as temp during vmstate */
-uint8_t tdr;
-uint8_t dsr;
-uint8_t msr;
-uint8_t cur_drv;
-uint8_t status0;
-uint8_t status1;
-uint8_t status2;
-/* Command FIFO */
-uint8_t *fifo;
-int32_t fifo_size;
-uint32_t data_pos;
-uint32_t data_len;
-uint8_t data_state;
-uint8_t data_dir;
-uint8_t eot; /* last wanted sector */
-/* States kept only to be returned back */
-/* precompensation */
-uint8_t precomp_trk;
-uint8_t config;
-uint8_t lock;
-/* Power down config (also with status regB access mode */
-uint8_t pwrd;
-/* Floppy drives */
-FloppyBus bus;
-uint8_t num_floppies;
-FDrive drives[MAX_FD];
-struct {
-BlockBackend *blk;
-FloppyDriveType type;
-} qdev_for_drives[MAX_FD];
-int reset_sensei;
-uint32_t check_media_rate;
-FloppyDriveType fallback; /* type=auto failure fallback */
-/* Timers state */
-uint8_t timer0;
-uint8_t timer1;
-PortioList portio_list;
-};
-
  static FloppyDriveType get_fallback_drive_type(FDrive *drv)
  {
  return drv->fdctrl->fallback;
@@ -891,17 +800,6 @@ typedef struct FDCtrlSysBus {
  
  #define ISA_FDC(obj) OBJECT_CHECK(FDCtrlISABus, (obj), TYPE_ISA_FDC)
  
-typedef struct FDCtrlISABus {

-ISADevice parent_obj;
-
-uint32_t iobase;
-uint32_t irq;
-uint32_t dma;
-struct FDCtrl state;
-int32_t bootindexA;
-int32_t bootindexB;
-} FDCtrlISABus;
-
  static uint32_t fdctrl_read (void *opaque, uint32_t reg)
  {
  FDCtrl *fdctrl = opaque;
diff --git a/include/hw/block/fdc.h b/include/hw/block/fdc.h
index 1749dabf25..d076b2fc1a 100644
--- a/include/hw/block/fdc.h
+++ b/include/hw/block/fdc.h
@@ -2,12 +2,115 @@
  #define HW_FDC_H
  
  #include "qemu-common.h"

+#include "hw/block/block.h"
+#include "hw/isa/isa.h"
  
  /* fdc.c */

  #define MAX_FD 2
  
+typedef struct FDCtrl FDCtrl;

+
+/* Floppy disk drive emulation */
+typedef