Skip to contentSkip to conversation

fdc: fix FD_SR0_SEEK flag + implement VERIFY

PATCH In review Hervé Poussineau 2 participants 10 messages Sep 6, 2012 – Sep 7, 2012

lore
Share & tools
lore

Hervé Poussineau

author
#0 lore raw

cover fdc: fix FD_SR0_SEEK flag + implement VERIFY

Attached patches have 3 goals:
- patches 1-4 fix the NT 3.1 floppy driver problem.
- patches 5-6 implement the VERIFY command
- patches 7-8 are some simple cleanup

Hervé Poussineau (6):
fdc: use status0 field instead of a local variable
fdc: fix false FD_SR0_SEEK
fdc: implement VERIFY command
fdc-tests: add tests for VERIFY command
fdc: remove double affectation of FD_MSR_CMDBUSY flag
fdc: fix typo in zero constant

Kevin Wolf (2):
fdc: Remove status0 parameter from fdctrl_set_fifo()
fdc-test: Check READ ID

 hw/fdc.c         |   99 ++++++++++++++++++++++++++++++++----------------------
 tests/fdc-test.c |   84 ++++++++++++++++++++++++++++++++++++++++++---
 2 files changed, 138 insertions(+), 45 deletions(-)
Signature · 1 line
1.7.10.4
author
#1 lore raw

PATCH 1/8 fdc: Remove status0 parameter from fdctrl_set_fifo()

From: Kevin Wolf <[email protected]>

It decided whether an interrupt is triggered. Only one caller made use of this functionality, so move the code there.

In this one caller, the interrupt must actually be triggered unconditionally, like it was before commit 2fee0088. For example, a successful read without an implied seek can result in st0 = 0, but still triggers the interrupt.

Signed-off-by Kevin Wolf <[email protected]>
Signed-off-by Hervé Poussineau <[email protected]>
---
 hw/fdc.c |   33 ++++++++++++++++-----------------
 1 file changed, 16 insertions(+), 17 deletions(-)
hw/fdc.c +16 -17
index 08830c1..78ae064 100644
@@ -1079,15 +1079,12 @@ static void fdctrl_reset_fifo(FDCtrl *fdctrl)
 }
 
 /* Set FIFO status for the host to read */
-static void fdctrl_set_fifo(FDCtrl *fdctrl, int fifo_len, uint8_t status0)
+static void fdctrl_set_fifo(FDCtrl *fdctrl, int fifo_len)
 {
     fdctrl->data_dir = FD_DIR_READ;
     fdctrl->data_len = fifo_len;
     fdctrl->data_pos = 0;
     fdctrl->msr |= FD_MSR_CMDBUSY | FD_MSR_RQM | FD_MSR_DIO;
-    if (status0) {
-        fdctrl_raise_irq(fdctrl, status0);
-    }
 }
 
 /* Set an error: unimplemented/unknown command */

@@ -1096,7 +1093,7 @@ static void fdctrl_unimplemented(FDCtrl *fdctrl, int direction)
     qemu_log_mask(LOG_UNIMP, "fdc: unimplemented command 0x%02x\n",
                   fdctrl->fifo[0]);
     fdctrl->fifo[0] = FD_SR0_INVCMD;
-    fdctrl_set_fifo(fdctrl, 1, 0);
+    fdctrl_set_fifo(fdctrl, 1);
 }
 
 /* Seek to next sector

@@ -1170,7 +1167,9 @@ static void fdctrl_stop_transfer(FDCtrl *fdctrl, uint8_t status0,
     }
     fdctrl->msr |= FD_MSR_RQM | FD_MSR_DIO;
     fdctrl->msr &= ~FD_MSR_NONDMA;
-    fdctrl_set_fifo(fdctrl, 7, fdctrl->status0);
+
+    fdctrl_set_fifo(fdctrl, 7);
+    fdctrl_raise_irq(fdctrl, fdctrl->status0);
 }
 
 /* Prepare a data transfer (either DMA or FIFO) */

@@ -1538,7 +1537,7 @@ static void fdctrl_handle_lock(FDCtrl *fdctrl, int direction)
 {
     fdctrl->lock = (fdctrl->fifo[0] & 0x80) ? 1 : 0;
     fdctrl->fifo[0] = fdctrl->lock << 4;
-    fdctrl_set_fifo(fdctrl, 1, 0);
+    fdctrl_set_fifo(fdctrl, 1);
 }
 
 static void fdctrl_handle_dumpreg(FDCtrl *fdctrl, int direction)

@@ -1563,20 +1562,20 @@ static void fdctrl_handle_dumpreg(FDCtrl *fdctrl, int direction)
         (cur_drv->perpendicular << 2);
     fdctrl->fifo[8] = fdctrl->config;
     fdctrl->fifo[9] = fdctrl->precomp_trk;
-    fdctrl_set_fifo(fdctrl, 10, 0);
+    fdctrl_set_fifo(fdctrl, 10);
 }
 
 static void fdctrl_handle_version(FDCtrl *fdctrl, int direction)
 {
     /* Controller's version */
     fdctrl->fifo[0] = fdctrl->version;
-    fdctrl_set_fifo(fdctrl, 1, 0);
+    fdctrl_set_fifo(fdctrl, 1);
 }
 
 static void fdctrl_handle_partid(FDCtrl *fdctrl, int direction)
 {
     fdctrl->fifo[0] = 0x41; /* Stepping 1 */
-    fdctrl_set_fifo(fdctrl, 1, 0);
+    fdctrl_set_fifo(fdctrl, 1);
 }
 
 static void fdctrl_handle_restore(FDCtrl *fdctrl, int direction)

@@ -1629,7 +1628,7 @@ static void fdctrl_handle_save(FDCtrl *fdctrl, int direction)
     fdctrl->fifo[12] = fdctrl->pwrd;
     fdctrl->fifo[13] = 0;
     fdctrl->fifo[14] = 0;
-    fdctrl_set_fifo(fdctrl, 15, 0);
+    fdctrl_set_fifo(fdctrl, 15);
 }
 
 static void fdctrl_handle_readid(FDCtrl *fdctrl, int direction)

@@ -1695,7 +1694,7 @@ static void fdctrl_handle_sense_drive_status(FDCtrl *fdctrl, int direction)
         (cur_drv->head << 2) |
         GET_CUR_DRV(fdctrl) |
         0x28;
-    fdctrl_set_fifo(fdctrl, 1, 0);
+    fdctrl_set_fifo(fdctrl, 1);
 }
 
 static void fdctrl_handle_recalibrate(FDCtrl *fdctrl, int direction)

@@ -1720,7 +1719,7 @@ static void fdctrl_handle_sense_interrupt_status(FDCtrl *fdctrl, int direction)
         fdctrl->reset_sensei--;
     } else if (!(fdctrl->sra & FD_SRA_INTPEND)) {
         fdctrl->fifo[0] = FD_SR0_INVCMD;
-        fdctrl_set_fifo(fdctrl, 1, 0);
+        fdctrl_set_fifo(fdctrl, 1);
         return;
     } else {
         fdctrl->fifo[0] =

@@ -1729,7 +1728,7 @@ static void fdctrl_handle_sense_interrupt_status(FDCtrl *fdctrl, int direction)
     }
 
     fdctrl->fifo[1] = cur_drv->track;
-    fdctrl_set_fifo(fdctrl, 2, 0);
+    fdctrl_set_fifo(fdctrl, 2);
     fdctrl_reset_irq(fdctrl);
     fdctrl->status0 = FD_SR0_RDYCHG;
 }

@@ -1771,7 +1770,7 @@ static void fdctrl_handle_powerdown_mode(FDCtrl *fdctrl, int direction)
 {
     fdctrl->pwrd = fdctrl->fifo[1];
     fdctrl->fifo[0] = fdctrl->fifo[1];
-    fdctrl_set_fifo(fdctrl, 1, 0);
+    fdctrl_set_fifo(fdctrl, 1);
 }
 
 static void fdctrl_handle_option(FDCtrl *fdctrl, int direction)

@@ -1790,7 +1789,7 @@ static void fdctrl_handle_drive_specification_command(FDCtrl *fdctrl, int direct
             fdctrl->fifo[0] = fdctrl->fifo[1];
             fdctrl->fifo[2] = 0;
             fdctrl->fifo[3] = 0;
-            fdctrl_set_fifo(fdctrl, 4, 0);
+            fdctrl_set_fifo(fdctrl, 4);
         } else {
             fdctrl_reset_fifo(fdctrl);
         }

@@ -1798,7 +1797,7 @@ static void fdctrl_handle_drive_specification_command(FDCtrl *fdctrl, int direct
         /* ERROR */
         fdctrl->fifo[0] = 0x80 |
             (cur_drv->head << 2) | GET_CUR_DRV(fdctrl);
-        fdctrl_set_fifo(fdctrl, 1, 0);
+        fdctrl_set_fifo(fdctrl, 1);
     }
 }
Signature · 1 line
1.7.10.4
author
#2 lore raw

PATCH 2/8 fdc: use status0 field instead of a local variable

In reply to Hervé Poussineau
Signed-off-by Hervé Poussineau <[email protected]>
---
 hw/fdc.c |   29 ++++++++++++++++-------------
 1 file changed, 16 insertions(+), 13 deletions(-)
hw/fdc.c +16 -13
index 78ae064..d8bffb6 100644
@@ -327,7 +327,7 @@ static void fdctrl_reset(FDCtrl *fdctrl, int do_irq);
 static void fdctrl_reset_fifo(FDCtrl *fdctrl);
 static int fdctrl_transfer_handler (void *opaque, int nchan,
                                     int dma_pos, int dma_len);
-static void fdctrl_raise_irq(FDCtrl *fdctrl, uint8_t status0);
+static void fdctrl_raise_irq(FDCtrl *fdctrl);
 static FDrive *get_cur_drv(FDCtrl *fdctrl);
 
 static uint32_t fdctrl_read_statusA(FDCtrl *fdctrl);

@@ -799,6 +799,7 @@ static void fdctrl_handle_tc(void *opaque, int irq, int level)
 /* Change IRQ state */
 static void fdctrl_reset_irq(FDCtrl *fdctrl)
 {
+    fdctrl->status0 = 0;
     if (!(fdctrl->sra & FD_SRA_INTPEND))
         return;
     FLOPPY_DPRINTF("Reset interrupt\n");

@@ -806,14 +807,13 @@ static void fdctrl_reset_irq(FDCtrl *fdctrl)
     fdctrl->sra &= ~FD_SRA_INTPEND;
 }
 
-static void fdctrl_raise_irq(FDCtrl *fdctrl, uint8_t status0)
+static void fdctrl_raise_irq(FDCtrl *fdctrl)
 {
     /* Sparc mutation */
     if (fdctrl->sun4m && (fdctrl->msr & FD_MSR_CMDBUSY)) {
         /* XXX: not sure */
         fdctrl->msr &= ~FD_MSR_CMDBUSY;
         fdctrl->msr |= FD_MSR_RQM | FD_MSR_DIO;
-        fdctrl->status0 = status0;
         return;
     }
     if (!(fdctrl->sra & FD_SRA_INTPEND)) {

@@ -822,7 +822,6 @@ static void fdctrl_raise_irq(FDCtrl *fdctrl, uint8_t status0)
     }
 
     fdctrl->reset_sensei = 0;
-    fdctrl->status0 = status0;
     FLOPPY_DPRINTF("Set interrupt status to 0x%02x\n", fdctrl->status0);
 }
 

@@ -851,7 +850,8 @@ static void fdctrl_reset(FDCtrl *fdctrl, int do_irq)
         fd_recalibrate(&fdctrl->drives[i]);
     fdctrl_reset_fifo(fdctrl);
     if (do_irq) {
-        fdctrl_raise_irq(fdctrl, FD_SR0_RDYCHG);
+        fdctrl->status0 |= FD_SR0_RDYCHG;
+        fdctrl_raise_irq(fdctrl);
         fdctrl->reset_sensei = FD_RESET_SENSEI_COUNT;
     }
 }

@@ -1169,7 +1169,7 @@ static void fdctrl_stop_transfer(FDCtrl *fdctrl, uint8_t status0,
     fdctrl->msr &= ~FD_MSR_NONDMA;
 
     fdctrl_set_fifo(fdctrl, 7);
-    fdctrl_raise_irq(fdctrl, fdctrl->status0);
+    fdctrl_raise_irq(fdctrl);
 }
 
 /* Prepare a data transfer (either DMA or FIFO) */

@@ -1284,9 +1284,8 @@ static void fdctrl_start_transfer(FDCtrl *fdctrl, int direction)
     if (direction != FD_DIR_WRITE)
         fdctrl->msr |= FD_MSR_DIO;
     /* IO based transfer: calculate len */
-    fdctrl_raise_irq(fdctrl, FD_SR0_SEEK);
-
-    return;
+    fdctrl->status0 |= FD_SR0_SEEK;
+    fdctrl_raise_irq(fdctrl);
 }
 
 /* Prepare a transfer of deleted data */

@@ -1706,7 +1705,8 @@ static void fdctrl_handle_recalibrate(FDCtrl *fdctrl, int direction)
     fd_recalibrate(cur_drv);
     fdctrl_reset_fifo(fdctrl);
     /* Raise Interrupt */
-    fdctrl_raise_irq(fdctrl, FD_SR0_SEEK);
+    fdctrl->status0 |= FD_SR0_SEEK;
+    fdctrl_raise_irq(fdctrl);
 }
 
 static void fdctrl_handle_sense_interrupt_status(FDCtrl *fdctrl, int direction)

@@ -1745,7 +1745,8 @@ static void fdctrl_handle_seek(FDCtrl *fdctrl, int direction)
      */
     fd_seek(cur_drv, cur_drv->head, fdctrl->fifo[2], cur_drv->sect, 1);
     /* Raise Interrupt */
-    fdctrl_raise_irq(fdctrl, FD_SR0_SEEK);
+    fdctrl->status0 |= FD_SR0_SEEK;
+    fdctrl_raise_irq(fdctrl);
 }
 
 static void fdctrl_handle_perpendicular_mode(FDCtrl *fdctrl, int direction)

@@ -1816,7 +1817,8 @@ static void fdctrl_handle_relative_seek_in(FDCtrl *fdctrl, int direction)
     }
     fdctrl_reset_fifo(fdctrl);
     /* Raise Interrupt */
-    fdctrl_raise_irq(fdctrl, FD_SR0_SEEK);
+    fdctrl->status0 |= FD_SR0_SEEK;
+    fdctrl_raise_irq(fdctrl);
 }
 
 static void fdctrl_handle_relative_seek_out(FDCtrl *fdctrl, int direction)

@@ -1833,7 +1835,8 @@ static void fdctrl_handle_relative_seek_out(FDCtrl *fdctrl, int direction)
     }
     fdctrl_reset_fifo(fdctrl);
     /* Raise Interrupt */
-    fdctrl_raise_irq(fdctrl, FD_SR0_SEEK);
+    fdctrl->status0 |= FD_SR0_SEEK;
+    fdctrl_raise_irq(fdctrl);
 }
 
 static const struct {
Signature · 1 line
1.7.10.4
author
#3 lore raw

PATCH 3/8 fdc-test: Check READ ID

In reply to Hervé Poussineau

From: Kevin Wolf <[email protected]>

ST0 shouldn't include 0x20 (FD_SR0_SEEK) after READ ID.

Signed-off-by Kevin Wolf <[email protected]>
Signed-off-by Hervé Poussineau <[email protected]>
---
 tests/fdc-test.c |   66 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 66 insertions(+)
tests/fdc-test.c +66 -0
index fa74411..18051ef 100644
@@ -48,6 +48,7 @@ enum {
 
 enum {
     CMD_SENSE_INT           = 0x08,
+    CMD_READ_ID             = 0x0a,
     CMD_SEEK                = 0x0f,
     CMD_READ                = 0xe6,
     CMD_RELATIVE_SEEK_OUT   = 0x8f,

@@ -320,6 +321,70 @@ static void test_relative_seek(void)
     g_assert(pcn == 0);
 }
 
+static void test_read_id(void)
+{
+    uint8_t drive = 0;
+    uint8_t head = 0;
+    uint8_t cyl;
+    uint8_t st0;
+
+    /* Seek to track 0 and check with READ ID */
+    send_seek(0);
+
+    floppy_send(CMD_READ_ID);
+    g_assert(!get_irq(FLOPPY_IRQ));
+    floppy_send(head << 2 | drive);
+
+    while (!get_irq(FLOPPY_IRQ)) {
+        /* qemu involves a timer with READ ID... */
+        clock_step(1000000000LL / 50);
+    }
+
+    st0 = floppy_recv();
+    floppy_recv();
+    floppy_recv();
+    cyl = floppy_recv();
+    head = floppy_recv();
+    floppy_recv();
+    floppy_recv();
+
+    g_assert_cmpint(cyl, ==, 0);
+    g_assert_cmpint(head, ==, 0);
+    g_assert_cmpint(st0, ==, head << 2);
+
+    /* Seek to track 8 on head 1 and check with READ ID */
+    head = 1;
+    cyl = 8;
+
+    floppy_send(CMD_SEEK);
+    floppy_send(head << 2 | drive);
+    g_assert(!get_irq(FLOPPY_IRQ));
+    floppy_send(cyl);
+    g_assert(get_irq(FLOPPY_IRQ));
+    ack_irq(NULL);
+
+    floppy_send(CMD_READ_ID);
+    g_assert(!get_irq(FLOPPY_IRQ));
+    floppy_send(head << 2 | drive);
+
+    while (!get_irq(FLOPPY_IRQ)) {
+        /* qemu involves a timer with READ ID... */
+        clock_step(1000000000LL / 50);
+    }
+
+    st0 = floppy_recv();
+    floppy_recv();
+    floppy_recv();
+    cyl = floppy_recv();
+    head = floppy_recv();
+    floppy_recv();
+    floppy_recv();
+
+    g_assert_cmpint(cyl, ==, 8);
+    g_assert_cmpint(head, ==, 1);
+    g_assert_cmpint(st0, ==, head << 2);
+}
+
 /* success if no crash or abort */
 static void fuzz_registers(void)
 {

@@ -369,6 +434,7 @@ int main(int argc, char **argv)
     qtest_add_func("/fdc/media_change", test_media_change);
     qtest_add_func("/fdc/sense_interrupt", test_sense_interrupt);
     qtest_add_func("/fdc/relative_seek", test_relative_seek);
+    qtest_add_func("/fdc/read_id", test_read_id);
     qtest_add_func("/fdc/fuzz-registers", fuzz_registers);
 
     ret = g_test_run();
Signature · 1 line
1.7.10.4
author
#4 lore raw

PATCH 4/8 fdc: fix false FD_SR0_SEEK

In reply to Hervé Poussineau

fdctrl_start/stop_transfer() used to set FD_SR0_SEEK no matter if there actually was a seek or not. This is obviously wrong.

fdctrl_seek_to_next_sect() has this information because it performs the seek itself.

Cc Kevin Wolf <[email protected]>
Signed-off-by Hervé Poussineau <[email protected]>
---
 hw/fdc.c         |   13 +++++++++----
 tests/fdc-test.c |    2 +-
 2 files changed, 10 insertions(+), 5 deletions(-)
hw/fdc.c +9 -4
index d8bffb6..3c343b1 100644
@@ -1123,11 +1123,13 @@ static int fdctrl_seek_to_next_sect(FDCtrl *fdctrl, FDrive *cur_drv)
             } else {
                 new_head = 0;
                 new_track++;
+                fdctrl->status0 |= FD_SR0_SEEK;
                 if ((cur_drv->flags & FDISK_DBL_SIDES) == 0) {
                     ret = 0;
                 }
             }
         } else {
+            fdctrl->status0 |= FD_SR0_SEEK;
             new_track++;
             ret = 0;
         }

@@ -1147,10 +1149,14 @@ static void fdctrl_stop_transfer(FDCtrl *fdctrl, uint8_t status0,
                                  uint8_t status1, uint8_t status2)
 {
     FDrive *cur_drv;
-
     cur_drv = get_cur_drv(fdctrl);
-    fdctrl->status0 = status0 | FD_SR0_SEEK | (cur_drv->head << 2) |
-                      GET_CUR_DRV(fdctrl);
+
+    fdctrl->status0 &= ~(FD_SR0_DS0 | FD_SR0_DS1 | FD_SR0_HEAD);
+    fdctrl->status0 |= GET_CUR_DRV(fdctrl);
+    if (cur_drv->head) {
+        fdctrl->status0 |= FD_SR0_HEAD;
+    }
+    fdctrl->status0 |= status0;
 
     FLOPPY_DPRINTF("transfer status: %02x %02x %02x (%02x)\n",
                    status0, status1, status2, fdctrl->status0);

@@ -1284,7 +1290,6 @@ static void fdctrl_start_transfer(FDCtrl *fdctrl, int direction)
     if (direction != FD_DIR_WRITE)
         fdctrl->msr |= FD_MSR_DIO;
     /* IO based transfer: calculate len */
-    fdctrl->status0 |= FD_SR0_SEEK;
     fdctrl_raise_irq(fdctrl);
 }
tests/fdc-test.c +1 -1
index 18051ef..b825959 100644
@@ -153,7 +153,7 @@ static uint8_t send_read_command(void)
     }
 
     st0 = floppy_recv();
-    if (st0 != 0x60) {
+    if (st0 != 0x40) {
         ret = 1;
     }
Signature · 1 line
1.7.10.4
author
#5 lore raw

PATCH 5/8 fdc: implement VERIFY command

In reply to Hervé Poussineau

VERIFY command is like a READ command, except that read data is not transfered by DMA.
As DMA engine is not used, so we have to start data transfer ourselves.

Signed-off-by Hervé Poussineau <[email protected]>
---
 hw/fdc.c |   25 ++++++++++++++++++-------
 1 file changed, 18 insertions(+), 7 deletions(-)
hw/fdc.c +18 -7
index 3c343b1..551160e 100644
@@ -349,6 +349,7 @@ enum {
     FD_DIR_SCANE   = 2,
     FD_DIR_SCANL   = 3,
     FD_DIR_SCANH   = 4,
+    FD_DIR_VERIFY  = 5,
 };
 
 enum {

@@ -1271,14 +1272,21 @@ static void fdctrl_start_transfer(FDCtrl *fdctrl, int direction)
         if (((direction == FD_DIR_SCANE || direction == FD_DIR_SCANL ||
               direction == FD_DIR_SCANH) && dma_mode == 0) ||
             (direction == FD_DIR_WRITE && dma_mode == 2) ||
-            (direction == FD_DIR_READ && dma_mode == 1)) {
+            (direction == FD_DIR_READ && dma_mode == 1) ||
+            (direction == FD_DIR_VERIFY)) {
             /* No access is allowed until DMA transfer has completed */
             fdctrl->msr &= ~FD_MSR_RQM;
-            /* Now, we just have to wait for the DMA controller to
-             * recall us...
-             */
-            DMA_hold_DREQ(fdctrl->dma_chann);
-            DMA_schedule(fdctrl->dma_chann);
+            if (direction != FD_DIR_VERIFY) {
+                /* Now, we just have to wait for the DMA controller to
+                 * recall us...
+                 */
+                DMA_hold_DREQ(fdctrl->dma_chann);
+                DMA_schedule(fdctrl->dma_chann);
+            } else {
+                /* Start transfer */
+                fdctrl_transfer_handler(fdctrl, fdctrl->dma_chann, 0,
+                                        fdctrl->data_len);
+            }
             return;
         } else {
             FLOPPY_DPRINTF("bad dma_mode=%d direction=%d\n", dma_mode,

@@ -1381,6 +1389,9 @@ static int fdctrl_transfer_handler (void *opaque, int nchan,
                 goto transfer_error;
             }
             break;
+        case FD_DIR_VERIFY:
+            /* VERIFY commands */
+            break;
         default:
             /* SCAN commands */
             {

@@ -1863,7 +1874,7 @@ static const struct {
     { FD_CMD_SAVE, 0xff, "SAVE", 0, fdctrl_handle_save }, /* part of READ DELETED DATA */
     { FD_CMD_READ_DELETED, 0x1f, "READ DELETED DATA", 8, fdctrl_start_transfer_del, FD_DIR_READ },
     { FD_CMD_SCAN_EQUAL, 0x1f, "SCAN EQUAL", 8, fdctrl_start_transfer, FD_DIR_SCANE },
-    { FD_CMD_VERIFY, 0x1f, "VERIFY", 8, fdctrl_unimplemented },
+    { FD_CMD_VERIFY, 0x1f, "VERIFY", 8, fdctrl_start_transfer, FD_DIR_VERIFY },
     { FD_CMD_SCAN_LOW_OR_EQUAL, 0x1f, "SCAN LOW OR EQUAL", 8, fdctrl_start_transfer, FD_DIR_SCANL },
     { FD_CMD_SCAN_HIGH_OR_EQUAL, 0x1f, "SCAN HIGH OR EQUAL", 8, fdctrl_start_transfer, FD_DIR_SCANH },
     { FD_CMD_WRITE_DELETED, 0x3f, "WRITE DELETED DATA", 8, fdctrl_start_transfer_del, FD_DIR_WRITE },
Signature · 1 line
1.7.10.4
author
#6 lore raw

PATCH 6/8 fdc-tests: add tests for VERIFY command

In reply to Hervé Poussineau
Signed-off-by Hervé Poussineau <[email protected]>
---
 tests/fdc-test.c |   16 +++++++++++++---
 1 file changed, 13 insertions(+), 3 deletions(-)
tests/fdc-test.c +13 -3
index b825959..4783b08 100644
@@ -50,6 +50,7 @@ enum {
     CMD_SENSE_INT           = 0x08,
     CMD_READ_ID             = 0x0a,
     CMD_SEEK                = 0x0f,
+    CMD_VERIFY              = 0x16,
     CMD_READ                = 0xe6,
     CMD_RELATIVE_SEEK_OUT   = 0x8f,
     CMD_RELATIVE_SEEK_IN    = 0xcf,

@@ -111,7 +112,7 @@ static void ack_irq(uint8_t *pcn)
     g_assert(!get_irq(FLOPPY_IRQ));
 }
 
-static uint8_t send_read_command(void)
+static uint8_t send_read_command(uint8_t cmd)
 {
     uint8_t drive = 0;
     uint8_t head = 0;

@@ -127,7 +128,7 @@ static uint8_t send_read_command(void)
 
     uint8_t ret = 0;
 
-    floppy_send(CMD_READ);
+    floppy_send(cmd);
     floppy_send(head << 2 | drive);
     g_assert(!get_irq(FLOPPY_IRQ));
     floppy_send(cyl);

@@ -214,7 +215,7 @@ static void test_read_without_media(void)
 {
     uint8_t ret;
 
-    ret = send_read_command();
+    ret = send_read_command(CMD_READ);
     g_assert(ret == 0);
 }
 

@@ -385,6 +386,14 @@ static void test_read_id(void)
     g_assert_cmpint(st0, ==, head << 2);
 }
 
+static void test_verify(void)
+{
+    uint8_t ret;
+
+    ret = send_read_command(CMD_VERIFY);
+    g_assert(ret == 0);
+}
+
 /* success if no crash or abort */
 static void fuzz_registers(void)
 {

@@ -435,6 +444,7 @@ int main(int argc, char **argv)
     qtest_add_func("/fdc/sense_interrupt", test_sense_interrupt);
     qtest_add_func("/fdc/relative_seek", test_relative_seek);
     qtest_add_func("/fdc/read_id", test_read_id);
+    qtest_add_func("/fdc/verify", test_verify);
     qtest_add_func("/fdc/fuzz-registers", fuzz_registers);
 
     ret = g_test_run();
Signature · 1 line
1.7.10.4
author
#7 lore raw

PATCH 7/8 fdc: remove double affectation of FD_MSR_CMDBUSY flag

In reply to Hervé Poussineau

FD_MSR_CMDBUSY flag is already set in fdctrl_write_data(), just before calling the command handler (fdctrl_start_transfer() here).

Signed-off-by Hervé Poussineau <[email protected]>
---
 hw/fdc.c |    1 -
 1 file changed, 1 deletion(-)
hw/fdc.c +0 -1
index 551160e..9c2bad2 100644
@@ -1240,7 +1240,6 @@ static void fdctrl_start_transfer(FDCtrl *fdctrl, int direction)
     /* Set the FIFO state */
     fdctrl->data_dir = direction;
     fdctrl->data_pos = 0;
-    fdctrl->msr |= FD_MSR_CMDBUSY;
     if (fdctrl->fifo[0] & 0x80)
         fdctrl->data_state |= FD_STATE_MULTI;
     else
Signature · 1 line
1.7.10.4
author
#8 lore raw

PATCH 8/8 fdc: fix typo in zero constant

In reply to Hervé Poussineau
Signed-off-by Hervé Poussineau <[email protected]>
---
 hw/fdc.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
hw/fdc.c +1 -1
index 9c2bad2..e087694 100644
@@ -1248,7 +1248,7 @@ static void fdctrl_start_transfer(FDCtrl *fdctrl, int direction)
         fdctrl->data_state |= FD_STATE_SEEK;
     else
         fdctrl->data_state &= ~FD_STATE_SEEK;
-    if (fdctrl->fifo[5] == 00) {
+    if (fdctrl->fifo[5] == 0) {
         fdctrl->data_len = fdctrl->fifo[8];
     } else {
         int tmp;
Signature · 1 line
1.7.10.4
#9 lore raw
In reply to Hervé Poussineau

Am 06.09.2012 21:17, schrieb Hervé Poussineau:

Quoted 4 lines

Attached patches have 3 goals:
- patches 1-4 fix the NT 3.1 floppy driver problem.
- patches 5-6 implement the VERIFY command
- patches 7-8 are some simple cleanup

Any specific reason for resending and reordering my patches? They are already reviewed and applied to block-next, so just posting something on top of it would be easier.

Even if my patches were wrong, I think it would be useful to have the fix on top because then you could add a qtest case that shows that my patches were wrong and your changes are correct.

Kevin