diff mbox

Re: [PATCH] fdc: fix MAX_FD probelm

Message ID 200909161437.AA00108@YOUR-BD18D6DD63.m1.interq.or.jp
State Superseded
Headers show

Commit Message

武田 =?ISO-2022-JP?B?IBskQj1TTGkbKEI=?= Sept. 16, 2009, 2:37 p.m. UTC 
Dear Juan and members,

>t-takeda@m1.interq.or.jp (武田 俊也) wrote:
>> Dear members,
>
>Hi
>>  static const VMStateDescription vmstate_fdc = {
>>      .name = "fdc",
>> -    .version_id = 2,
>> -    .minimum_version_id = 2,
>> -    .minimum_version_id_old = 2,
>> +    .version_id = 3,
>> +    .minimum_version_id = 3,
>> +    .minimum_version_id_old = 3,
>
>This is wrong.  You can't move to version 3 without a good reason.  And
>if you move, you have to retain backward compatibility).


>>      .pre_save = fdc_pre_save,
>>      .post_load = fdc_post_load,
>>      .fields      = (VMStateField []) {
>> @@ -692,7 +694,7 @@ static const VMStateDescription vmstate_fdc = {
>>          VMSTATE_UINT8(lock, fdctrl_t),
>>          VMSTATE_UINT8(pwrd, fdctrl_t),
>>          VMSTATE_UINT8_EQUAL(num_floppies, fdctrl_t),
>> -        VMSTATE_STRUCT_ARRAY(drives, fdctrl_t, MAX_FD, 1,
>> +        VMSTATE_STRUCT_ARRAY(drives, fdctrl_t, MAX_LOGICAL_FD, 1,
>
>Will send a patch with support for:
>
>VMSTATE_STRUCT_ARRAY(drives, fdctrl_t, num_floppies, 1, wmstate_fdrive,
>fdrive_t)

Well, it seems that we cannot specify the variable to WMSTATE_STRUCT_ARRAY.

And I found the problem that,
if I defined fdctrl_t->drives as fdrive_t drives[MAX_LOGICAL_FD];
and specify MAX_FD as VMSTATE_STRUCT_ARRAY(drives, fdctrl_t, MAX_FD, ...)
it cause the build error on MinGW.
It seems that we need to specify the same constant value.

So I added null_drives[] and fdctrl_get_drive(), though I feel it circumlocutory.


>to do what you need here.
>> diff --git a/qemu/hw/fdc.h b/qemu/hw/fdc.h
>> index 1b81ec1..9614e20 100644
>> --- a/qemu/hw/fdc.h
>> +++ b/qemu/hw/fdc.h
>> @@ -1,5 +1,5 @@
>>  /* fdc.c */
>> -#define MAX_FD 2
>> +#define MAX_PHYSICAL_FD 2
>
>This name change makes you change lots of places.  I think that just
>adding a comment that MAX_FD means MAX_PHYSICAL_FD will do the trick.
>
>Later, Juan.

This is new patch.

Comments

Juan Quintela Sept. 16, 2009, 3:08 p.m. UTC | #1 
"TAKEDA, toshiya" <t-takeda@m1.interq.or.jp> wrote:
> Dear Juan and members,
>
>>t-takeda@m1.interq.or.jp (武田 俊也) wrote:
>>> Dear members,
>>
>>Hi
>>>  static const VMStateDescription vmstate_fdc = {
>>>      .name = "fdc",
>>> -    .version_id = 2,
>>> -    .minimum_version_id = 2,
>>> -    .minimum_version_id_old = 2,
>>> +    .version_id = 3,
>>> +    .minimum_version_id = 3,
>>> +    .minimum_version_id_old = 3,
>>
>>This is wrong.  You can't move to version 3 without a good reason.  And
>>if you move, you have to retain backward compatibility).
>
>
>>>      .pre_save = fdc_pre_save,
>>>      .post_load = fdc_post_load,
>>>      .fields      = (VMStateField []) {
>>> @@ -692,7 +694,7 @@ static const VMStateDescription vmstate_fdc = {
>>>          VMSTATE_UINT8(lock, fdctrl_t),
>>>          VMSTATE_UINT8(pwrd, fdctrl_t),
>>>          VMSTATE_UINT8_EQUAL(num_floppies, fdctrl_t),
>>> -        VMSTATE_STRUCT_ARRAY(drives, fdctrl_t, MAX_FD, 1,
>>> +        VMSTATE_STRUCT_ARRAY(drives, fdctrl_t, MAX_LOGICAL_FD, 1,
>>
>>Will send a patch with support for:
>>
>>VMSTATE_STRUCT_ARRAY(drives, fdctrl_t, num_floppies, 1, wmstate_fdrive,
>>fdrive_t)
>
> Well, it seems that we cannot specify the variable to WMSTATE_STRUCT_ARRAY.

> And I found the problem that,
> if I defined fdctrl_t->drives as fdrive_t drives[MAX_LOGICAL_FD];
> and specify MAX_FD as VMSTATE_STRUCT_ARRAY(drives, fdctrl_t, MAX_FD, ...)
> it cause the build error on MinGW.
> It seems that we need to specify the same constant value.

I sent patch to support it.  Current code is to send whole arrays (and
it complains if array size and what you send is not the same).

This are the hearders of the mail that gets its support (not tested)

From: Juan Quintela <quintela@redhat.com>
Subject: [PATCH 4/4] vmstate: Add support for sending partial arrays
To: qemu-devel@nongnu.org
Date: Mon, 14 Sep 2009 22:15:21 +0200


Later, Juan.
武田 =?ISO-2022-JP?B?IBskQj1TTGkbKEI=?= Sept. 16, 2009, 3:59 p.m. UTC | #2 
Dear Juan,

>"TAKEDA, toshiya" <t-takeda@m1.interq.or.jp> wrote:
>> Dear Juan and members,
>>
>>>t-takeda@m1.interq.or.jp (武田 俊也) wrote:
	[snip]
>>>>      .pre_save = fdc_pre_save,
>>>>      .post_load = fdc_post_load,
>>>>      .fields      = (VMStateField []) {
>>>> @@ -692,7 +694,7 @@ static const VMStateDescription vmstate_fdc = {
>>>>          VMSTATE_UINT8(lock, fdctrl_t),
>>>>          VMSTATE_UINT8(pwrd, fdctrl_t),
>>>>          VMSTATE_UINT8_EQUAL(num_floppies, fdctrl_t),
>>>> -        VMSTATE_STRUCT_ARRAY(drives, fdctrl_t, MAX_FD, 1,
>>>> +        VMSTATE_STRUCT_ARRAY(drives, fdctrl_t, MAX_LOGICAL_FD, 1,
>>>
>>>Will send a patch with support for:
>>>
>>>VMSTATE_STRUCT_ARRAY(drives, fdctrl_t, num_floppies, 1, wmstate_fdrive,
>>>fdrive_t)
>>
>> Well, it seems that we cannot specify the variable to WMSTATE_STRUCT_ARRAY.
>
>> And I found the problem that,
>> if I defined fdctrl_t->drives as fdrive_t drives[MAX_LOGICAL_FD];
>> and specify MAX_FD as VMSTATE_STRUCT_ARRAY(drives, fdctrl_t, MAX_FD, ...)
>> it cause the build error on MinGW.
>> It seems that we need to specify the same constant value.
>
>I sent patch to support it.  Current code is to send whole arrays (and
>it complains if array size and what you send is not the same).
>
>This are the hearders of the mail that gets its support (not tested)
>
>From: Juan Quintela <quintela@redhat.com>
>Subject: [PATCH 4/4] vmstate: Add support for sending partial arrays
>To: qemu-devel@nongnu.org
>Date: Mon, 14 Sep 2009 22:15:21 +0200
>
>
>Later, Juan.

Oh, thank you very much !
I will wait that your patch is committed in git repository.
diff mbox

Patch

diff --git a/qemu/hw/fdc.c b/qemu/hw/fdc.c
index 389d9e6..6450fb5 100644
--- a/qemu/hw/fdc.c
+++ b/qemu/hw/fdc.c
@@ -83,6 +83,7 @@  typedef enum fdisk_flags_t {
 typedef struct fdrive_t {
     BlockDriverState *bs;
     /* Drive status */
+    uint8_t connected;
     fdrive_type_t drive;
     uint8_t perpendicular;    /* 2.88 MB access mode    */
     /* Position */
@@ -424,11 +425,7 @@  enum {
 };
 
 enum {
-#if MAX_FD == 4
     FD_DOR_SELMASK  = 0x03,
-#else
-    FD_DOR_SELMASK  = 0x01,
-#endif
     FD_DOR_nRESET   = 0x04,
     FD_DOR_DMAEN    = 0x08,
     FD_DOR_MOTEN0   = 0x10,
@@ -438,11 +435,7 @@  enum {
 };
 
 enum {
-#if MAX_FD == 4
     FD_TDR_BOOTSEL  = 0x0c,
-#else
-    FD_TDR_BOOTSEL  = 0x04,
-#endif
 };
 
 enum {
@@ -470,6 +463,14 @@  enum {
 #define FD_DID_SEEK(state) ((state) & FD_STATE_SEEK)
 #define FD_FORMAT_CMD(state) ((state) & FD_STATE_FORMAT)
 
+/* MAX_LOGICAL_FD determines the max drive number that Intel 82078 can control,
+   and it should be 4.
+   fdctrl->num_floppies determines the number of physical drives that may be
+   connected and it is usually initialized to MAX_FD, but it may be initialized
+   to other value if any system requires */
+
+#define MAX_LOGICAL_FD 4
+
 struct fdctrl_t {
     /* Controller's identification */
     uint8_t version;
@@ -508,10 +509,13 @@  struct fdctrl_t {
     /* Power down config (also with status regB access mode */
     uint8_t pwrd;
     /* Sun4m quirks? */
-    int sun4m;
+    uint8_t sun4m;
     /* Floppy drives */
     uint8_t num_floppies;
     fdrive_t drives[MAX_FD];
+#if MAX_FD != MAX_LOGICAL_FD
+    fdrive_t null_drives[MAX_LOGICAL_FD - MAX_FD];
+#endif
     int reset_sensei;
 };
 
@@ -525,6 +529,19 @@  typedef struct fdctrl_isabus_t {
     struct fdctrl_t state;
 } fdctrl_isabus_t;
 
+static fdrive_t *fdctrl_get_drive (fdctrl_t *fdctrl, int drive_num)
+{
+    if (drive_num < MAX_FD) {
+        return &fdctrl->drives[drive_num];
+#if MAX_FD != MAX_LOGICAL_FD
+    } else if (drive_num < MAX_LOGICAL_FD) {
+        return &fdctrl->null_drives[drive_num - MAX_FD];
+#endif
+    } else {
+        return NULL;
+    }
+}
+
 static uint32_t fdctrl_read (void *opaque, uint32_t reg)
 {
     fdctrl_t *fdctrl = opaque;
@@ -718,7 +735,7 @@  static void fdctrl_handle_tc(void *opaque, int irq, int level)
 /* XXX: may change if moved to bdrv */
 int fdctrl_get_drive_type(fdctrl_t *fdctrl, int drive_num)
 {
-    return fdctrl->drives[drive_num].drive;
+    return fdctrl_get_drive(fdctrl, drive_num)->drive;
 }
 
 /* Change IRQ state */
@@ -760,7 +777,7 @@  static void fdctrl_reset (fdctrl_t *fdctrl, int do_irq)
     /* Initialise controller */
     fdctrl->sra = 0;
     fdctrl->srb = 0xc0;
-    if (!fdctrl->drives[1].bs)
+    if (!fdctrl_get_drive(fdctrl, 1)->bs)
         fdctrl->sra |= FD_SRA_nDRV2;
     fdctrl->cur_drv = 0;
     fdctrl->dor = FD_DOR_nRESET;
@@ -771,8 +788,8 @@  static void fdctrl_reset (fdctrl_t *fdctrl, int do_irq)
     fdctrl->data_len = 0;
     fdctrl->data_state = 0;
     fdctrl->data_dir = FD_DIR_WRITE;
-    for (i = 0; i < MAX_FD; i++)
-        fd_recalibrate(&fdctrl->drives[i]);
+    for (i = 0; i < MAX_LOGICAL_FD; i++)
+        fd_recalibrate(fdctrl_get_drive(fdctrl, i));
     fdctrl_reset_fifo(fdctrl);
     if (do_irq) {
         fdctrl_raise_irq(fdctrl, FD_SR0_RDYCHG);
@@ -782,44 +799,40 @@  static void fdctrl_reset (fdctrl_t *fdctrl, int do_irq)
 
 static inline fdrive_t *drv0 (fdctrl_t *fdctrl)
 {
-    return &fdctrl->drives[(fdctrl->tdr & FD_TDR_BOOTSEL) >> 2];
+    return fdctrl_get_drive(fdctrl, (fdctrl->tdr & FD_TDR_BOOTSEL) >> 2);
 }
 
 static inline fdrive_t *drv1 (fdctrl_t *fdctrl)
 {
     if ((fdctrl->tdr & FD_TDR_BOOTSEL) < (1 << 2))
-        return &fdctrl->drives[1];
+        return fdctrl_get_drive(fdctrl, 1);
     else
-        return &fdctrl->drives[0];
+        return fdctrl_get_drive(fdctrl, 0);
 }
 
-#if MAX_FD == 4
 static inline fdrive_t *drv2 (fdctrl_t *fdctrl)
 {
     if ((fdctrl->tdr & FD_TDR_BOOTSEL) < (2 << 2))
-        return &fdctrl->drives[2];
+        return fdctrl_get_drive(fdctrl, 2);
     else
-        return &fdctrl->drives[1];
+        return fdctrl_get_drive(fdctrl, 1);
 }
 
 static inline fdrive_t *drv3 (fdctrl_t *fdctrl)
 {
     if ((fdctrl->tdr & FD_TDR_BOOTSEL) < (3 << 2))
-        return &fdctrl->drives[3];
+        return fdctrl_get_drive(fdctrl, 3);
     else
-        return &fdctrl->drives[2];
+        return fdctrl_get_drive(fdctrl, 2);
 }
-#endif
 
 static fdrive_t *get_cur_drv (fdctrl_t *fdctrl)
 {
     switch (fdctrl->cur_drv) {
         case 0: return drv0(fdctrl);
         case 1: return drv1(fdctrl);
-#if MAX_FD == 4
         case 2: return drv2(fdctrl);
         case 3: return drv3(fdctrl);
-#endif
         default: return NULL;
     }
 }
@@ -969,12 +982,10 @@  static uint32_t fdctrl_read_dir (fdctrl_t *fdctrl)
 {
     uint32_t retval = 0;
 
-    if (fdctrl_media_changed(drv0(fdctrl))
-     || fdctrl_media_changed(drv1(fdctrl))
-#if MAX_FD == 4
-     || fdctrl_media_changed(drv2(fdctrl))
-     || fdctrl_media_changed(drv3(fdctrl))
-#endif
+    if ((drv0(fdctrl)->connected && fdctrl_media_changed(drv0(fdctrl)))
+     || (drv1(fdctrl)->connected && fdctrl_media_changed(drv1(fdctrl)))
+     || (drv2(fdctrl)->connected && fdctrl_media_changed(drv2(fdctrl)))
+     || (drv3(fdctrl)->connected && fdctrl_media_changed(drv3(fdctrl)))
         )
         retval |= FD_DIR_DSKCHG;
     if (retval != 0)
@@ -1416,15 +1427,10 @@  static void fdctrl_handle_dumpreg (fdctrl_t *fdctrl, int direction)
     fdrive_t *cur_drv = get_cur_drv(fdctrl);
 
     /* Drives position */
-    fdctrl->fifo[0] = drv0(fdctrl)->track;
-    fdctrl->fifo[1] = drv1(fdctrl)->track;
-#if MAX_FD == 4
-    fdctrl->fifo[2] = drv2(fdctrl)->track;
-    fdctrl->fifo[3] = drv3(fdctrl)->track;
-#else
-    fdctrl->fifo[2] = 0;
-    fdctrl->fifo[3] = 0;
-#endif
+    fdctrl->fifo[0] = drv0(fdctrl)->connected ? drv0(fdctrl)->track : 0;
+    fdctrl->fifo[1] = drv1(fdctrl)->connected ? drv1(fdctrl)->track : 0;
+    fdctrl->fifo[2] = drv2(fdctrl)->connected ? drv2(fdctrl)->track : 0;
+    fdctrl->fifo[3] = drv3(fdctrl)->connected ? drv3(fdctrl)->track : 0;
     /* timers */
     fdctrl->fifo[4] = fdctrl->timer0;
     fdctrl->fifo[5] = (fdctrl->timer1 << 1) | (fdctrl->dor & FD_DOR_DMAEN ? 1 : 0);
@@ -1454,12 +1460,18 @@  static void fdctrl_handle_restore (fdctrl_t *fdctrl, int direction)
     fdrive_t *cur_drv = get_cur_drv(fdctrl);
 
     /* Drives position */
-    drv0(fdctrl)->track = fdctrl->fifo[3];
-    drv1(fdctrl)->track = fdctrl->fifo[4];
-#if MAX_FD == 4
-    drv2(fdctrl)->track = fdctrl->fifo[5];
-    drv3(fdctrl)->track = fdctrl->fifo[6];
-#endif
+    if(drv0(fdctrl)->connected) {
+        drv0(fdctrl)->track = fdctrl->fifo[3];
+    }
+    if(drv1(fdctrl)->connected) {
+        drv1(fdctrl)->track = fdctrl->fifo[4];
+    }
+    if(drv2(fdctrl)->connected) {
+        drv2(fdctrl)->track = fdctrl->fifo[5];
+    }
+    if(drv3(fdctrl)->connected) {
+        drv3(fdctrl)->track = fdctrl->fifo[6];
+    }
     /* timers */
     fdctrl->timer0 = fdctrl->fifo[7];
     fdctrl->timer1 = fdctrl->fifo[8];
@@ -1479,15 +1491,10 @@  static void fdctrl_handle_save (fdctrl_t *fdctrl, int direction)
     fdctrl->fifo[0] = 0;
     fdctrl->fifo[1] = 0;
     /* Drives position */
-    fdctrl->fifo[2] = drv0(fdctrl)->track;
-    fdctrl->fifo[3] = drv1(fdctrl)->track;
-#if MAX_FD == 4
-    fdctrl->fifo[4] = drv2(fdctrl)->track;
-    fdctrl->fifo[5] = drv3(fdctrl)->track;
-#else
-    fdctrl->fifo[4] = 0;
-    fdctrl->fifo[5] = 0;
-#endif
+    fdctrl->fifo[2] = drv0(fdctrl)->connected ? drv0(fdctrl)->track : 0;
+    fdctrl->fifo[3] = drv1(fdctrl)->connected ? drv1(fdctrl)->track : 0;
+    fdctrl->fifo[4] = drv2(fdctrl)->connected ? drv2(fdctrl)->track : 0;
+    fdctrl->fifo[5] = drv3(fdctrl)->connected ? drv3(fdctrl)->track : 0;
     /* timers */
     fdctrl->fifo[6] = fdctrl->timer0;
     fdctrl->fifo[7] = fdctrl->timer1;
@@ -1833,9 +1840,16 @@  static void fdctrl_connect_drives(fdctrl_t *fdctrl, BlockDriverState **fds)
 {
     unsigned int i;
 
-    for (i = 0; i < MAX_FD; i++) {
-        fd_init(&fdctrl->drives[i], fds[i]);
-        fd_revalidate(&fdctrl->drives[i]);
+    for (i = 0; i < MAX_LOGICAL_FD; i++) {
+        fdrive_t *drive = fdctrl_get_drive(fdctrl, i);
+        if (i < fdctrl->num_floppies) {
+            drive->connected = 1;
+            fd_init(drive, fds[i]);
+        } else {
+            drive->connected = 0;
+            fd_init(drive, NULL);
+        }
+        fd_revalidate(drive);
     }
 }
 
@@ -1851,6 +1865,7 @@  fdctrl_t *fdctrl_init_isa(BlockDriverState **fds)
     fdctrl->dma_chann = dma_chann;
     DMA_register_channel(dma_chann, &fdctrl_transfer_handler, fdctrl);
 
+    fdctrl->num_floppies = MAX_FD;
     fdctrl_connect_drives(fdctrl, fds);
 
     return fdctrl;
@@ -1873,6 +1888,8 @@  fdctrl_t *fdctrl_init_sysbus(qemu_irq irq, int dma_chann,
 
     fdctrl->dma_chann = dma_chann;
     DMA_register_channel(dma_chann, &fdctrl_transfer_handler, fdctrl);
+
+    fdctrl->num_floppies = MAX_FD;
     fdctrl_connect_drives(fdctrl, fds);
 
     return fdctrl;
@@ -1895,6 +1912,7 @@  fdctrl_t *sun4m_fdctrl_init (qemu_irq irq, target_phys_addr_t io_base,
 
     fdctrl->dma_chann = -1;
 
+    fdctrl->num_floppies = MAX_FD;
     fdctrl_connect_drives(fdctrl, fds);
 
     return fdctrl;