Re: [Qemu-devel] [PATCH RFC for-2.3? 5/8] fdb: Move FDCtrlISABus to header

[John Snow <jsnow@redhat.com>]

You probably meant 'fdc' !

On 03/29/2015 01:53 PM, Andreas Färber wrote:
> To be used for embedding the device.
>
> Add gtk-doc private/public markers for parent field.
>
> Signed-off-by: Andreas Färber <afaerber@suse.de>
> ---
>   hw/block/fdc.c         | 87 -------------------------------------------------
>   include/hw/block/fdc.h | 88 ++++++++++++++++++++++++++++++++++++++++++++++++++
>   2 files changed, 88 insertions(+), 87 deletions(-)
>
> diff --git a/hw/block/fdc.c b/hw/block/fdc.c
> index 2bf87c9..da521f1 100644
> --- a/hw/block/fdc.c
> +++ b/hw/block/fdc.c
> @@ -31,7 +31,6 @@
>   #include "hw/block/fdc.h"
>   #include "qemu/error-report.h"
>   #include "qemu/timer.h"
> -#include "hw/isa/isa.h"
>   #include "hw/sysbus.h"
>   #include "sysemu/block-backend.h"
>   #include "sysemu/blockdev.h"
> @@ -167,33 +166,7 @@ static void pick_geometry(BlockBackend *blk, int *nb_heads,
>   #define FD_SECTOR_SC           2   /* Sector size code */
>   #define FD_RESET_SENSEI_COUNT  4   /* Number of sense interrupts on RESET */
>
> -typedef struct FDCtrl FDCtrl;
> -
>   /* Floppy disk drive emulation */
> -typedef enum FDiskFlags {
> -    FDISK_DBL_SIDES  = 0x01,
> -} FDiskFlags;
> -
> -typedef struct FDrive {
> -    FDCtrl *fdctrl;
> -    BlockBackend *blk;
> -    /* Drive status */
> -    FDriveType drive;
> -    uint8_t perpendicular;    /* 2.88 MB access mode    */
> -    /* Position */
> -    uint8_t head;
> -    uint8_t track;
> -    uint8_t sect;
> -    /* Media */
> -    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    */
> -} FDrive;
> -
>   static void fd_init(FDrive *drv)
>   {
>       /* Drive */
> @@ -498,53 +471,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;
> -    /* 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 */
> -    uint8_t num_floppies;
> -    /* Sun4m quirks? */
> -    int sun4m;
> -    FDrive drives[MAX_FD];
> -    int reset_sensei;
> -    uint32_t check_media_rate;
> -    /* Timers state */
> -    uint8_t timer0;
> -    uint8_t timer1;
> -};
> -
>   #define TYPE_SYSBUS_FDC "base-sysbus-fdc"
>   #define SYSBUS_FDC(obj) OBJECT_CHECK(FDCtrlSysBus, (obj), TYPE_SYSBUS_FDC)
>
> @@ -556,19 +482,6 @@ typedef struct FDCtrlSysBus {
>       struct FDCtrl state;
>   } 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 d48b2f8..86d852d 100644
> --- a/include/hw/block/fdc.h
> +++ b/include/hw/block/fdc.h
> @@ -2,6 +2,7 @@
>   #define HW_FDC_H
>
>   #include "qemu-common.h"
> +#include "hw/isa/isa.h"
>
>   /* fdc.c */
>   #define MAX_FD 2
> @@ -13,7 +14,94 @@ typedef enum FDriveType {
>       FDRIVE_DRV_NONE = 0x03,   /* No drive connected     */
>   } FDriveType;
>
> +typedef enum FDiskFlags {
> +    FDISK_DBL_SIDES  = 0x01,
> +} FDiskFlags;
> +
> +typedef struct FDCtrl FDCtrl;
> +
> +typedef struct FDrive {
> +    FDCtrl *fdctrl;
> +    BlockBackend *blk;
> +    /* Drive status */
> +    FDriveType drive;
> +    uint8_t perpendicular;    /* 2.88 MB access mode    */
> +    /* Position */
> +    uint8_t head;
> +    uint8_t track;
> +    uint8_t sect;
> +    /* Media */
> +    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    */
> +} FDrive;
> +
> +struct FDCtrl {
> +    MemoryRegion iomem;
> +    qemu_irq irq;
> +    /* Controller state */
> +    QEMUTimer *result_timer;
> +    int dma_chann;
> +    /* 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 */
> +    uint8_t num_floppies;
> +    /* Sun4m quirks? */
> +    int sun4m;
> +    FDrive drives[MAX_FD];
> +    int reset_sensei;
> +    uint32_t check_media_rate;
> +    /* Timers state */
> +    uint8_t timer0;
> +    uint8_t timer1;
> +};
> +
>   #define TYPE_ISA_FDC "isa-fdc"
> +#define ISA_FDC(obj) OBJECT_CHECK(FDCtrlISABus, (obj), TYPE_ISA_FDC)
> +
> +typedef struct FDCtrlISABus {
> +    /*< private >*/
> +    ISADevice parent_obj;
> +    /*< public >*/
> +
> +    uint32_t iobase;
> +    uint32_t irq;
> +    uint32_t dma;
> +    struct FDCtrl state;
> +    int32_t bootindexA;
> +    int32_t bootindexB;
> +} FDCtrlISABus;
>
>   ISADevice *fdctrl_init_isa(ISABus *bus, DriveInfo **fds);
>   void fdctrl_init_sysbus(qemu_irq irq, int dma_chann,
>

Both this and the IDE patch look programmatically sound to me, but I 
missed the discussion on why it's needed.