[Qemu-devel] [PATCH v2 4/8] fdc: Use phase in fdctrl_write_data()

[Kevin Wolf <kwolf@redhat.com>]

Instead of relying on a flag in the MSR to distinguish controller phases,
use the explicit phase that we store now. Assertions of the right MSR
flags are added.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: John Snow <jsnow@redhat.com>
---
 hw/block/fdc.c | 69 ++++++++++++++++++++++++++++++++++------------------------
 1 file changed, 41 insertions(+), 28 deletions(-)

diff --git a/hw/block/fdc.c b/hw/block/fdc.c
index f5bcf0b..e3515a1 100644
--- a/hw/block/fdc.c
+++ b/hw/block/fdc.c
@@ -2059,8 +2059,12 @@ static void fdctrl_write_data(FDCtrl *fdctrl, uint32_t value)
         return;
     }
     fdctrl->dsr &= ~FD_DSR_PWRDOWN;
-    /* Is it write command time ? */
-    if (fdctrl->msr & FD_MSR_NONDMA) {
+
+    switch (fdctrl->phase) {
+    case FD_PHASE_EXECUTION:
+        /* For DMA requests, RQM should be cleared during execution phase, so
+         * we would have errored out above. */
+        assert(fdctrl->msr & FD_MSR_NONDMA);
         /* FIFO data write */
         pos = fdctrl->data_pos++;
         pos %= FD_SECTOR_LEN;
@@ -2072,12 +2076,12 @@ static void fdctrl_write_data(FDCtrl *fdctrl, uint32_t value)
                 < 0) {
                 FLOPPY_DPRINTF("error writing sector %d\n",
                                fd_sector(cur_drv));
-                return;
+                break;
             }
             if (!fdctrl_seek_to_next_sect(fdctrl, cur_drv)) {
                 FLOPPY_DPRINTF("error seeking to next sector %d\n",
                                fd_sector(cur_drv));
-                return;
+                break;
             }
         }
         /* Switch from transfer mode to status mode
@@ -2085,33 +2089,42 @@ static void fdctrl_write_data(FDCtrl *fdctrl, uint32_t value)
          */
         if (fdctrl->data_pos == fdctrl->data_len)
             fdctrl_stop_transfer(fdctrl, 0x00, 0x00, 0x00);
-        return;
-    }
-    if (fdctrl->data_pos == 0) {
-        /* Command */
-        pos = command_to_handler[value & 0xff];
-        FLOPPY_DPRINTF("%s command\n", handlers[pos].name);
-        fdctrl->data_len = handlers[pos].parameters + 1;
-        fdctrl->msr |= FD_MSR_CMDBUSY;
-    }
+        break;
 
-    FLOPPY_DPRINTF("%s: %02x\n", __func__, value);
-    pos = fdctrl->data_pos++;
-    pos %= FD_SECTOR_LEN;
-    fdctrl->fifo[pos] = value;
-    if (fdctrl->data_pos == fdctrl->data_len) {
-        /* We now have all parameters
-         * and will be able to treat the command
-         */
-        fdctrl->phase = FD_PHASE_EXECUTION;
-        if (fdctrl->data_state & FD_STATE_FORMAT) {
-            fdctrl_format_sector(fdctrl);
-            return;
+    case FD_PHASE_COMMAND:
+        assert(!(fdctrl->msr & FD_MSR_NONDMA));
+
+        if (fdctrl->data_pos == 0) {
+            /* Command */
+            pos = command_to_handler[value & 0xff];
+            FLOPPY_DPRINTF("%s command\n", handlers[pos].name);
+            fdctrl->data_len = handlers[pos].parameters + 1;
+            fdctrl->msr |= FD_MSR_CMDBUSY;
+        }
+
+        FLOPPY_DPRINTF("%s: %02x\n", __func__, value);
+        pos = fdctrl->data_pos++;
+        pos %= FD_SECTOR_LEN;
+        fdctrl->fifo[pos] = value;
+        if (fdctrl->data_pos == fdctrl->data_len) {
+            /* We now have all parameters
+             * and will be able to treat the command
+             */
+            fdctrl->phase = FD_PHASE_EXECUTION;
+            if (fdctrl->data_state & FD_STATE_FORMAT) {
+                fdctrl_format_sector(fdctrl);
+                break;
+            }
+
+            pos = command_to_handler[fdctrl->fifo[0] & 0xff];
+            FLOPPY_DPRINTF("treat %s command\n", handlers[pos].name);
+            (*handlers[pos].handler)(fdctrl, handlers[pos].direction);
         }
+        break;
 
-        pos = command_to_handler[fdctrl->fifo[0] & 0xff];
-        FLOPPY_DPRINTF("treat %s command\n", handlers[pos].name);
-        (*handlers[pos].handler)(fdctrl, handlers[pos].direction);
+    case FD_PHASE_RESULT:
+    default:
+        abort();
     }
 }
 
-- 
1.8.3.1