Patchwork [v5,3/4] fdc: rewrite seek and DSKCHG bit handling

login
register
mail settings
Submitter Pavel Hrdina
Date June 13, 2012, 1:43 p.m.
Message ID <dad86cadfb2244150a8ac4ef346939386ca0efee.1339593749.git.phrdina@redhat.com>
Download mbox | patch
Permalink /patch/164692/
State New
Headers show

Comments

Pavel Hrdina - June 13, 2012, 1:43 p.m.
This bit is cleared on every successful seek to a different track (cylinder).
The seek is also called on revalidate or on read/write/format commands which
also clear the DSKCHG bit.

Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
---
 hw/fdc.c |   68 +++++++++++++++++++++++++++++++-------------------------------
 1 files changed, 34 insertions(+), 34 deletions(-)
Kevin Wolf - June 14, 2012, 2:05 p.m.
Am 13.06.2012 15:43, schrieb Pavel Hrdina:
> This bit is cleared on every successful seek to a different track (cylinder).
> The seek is also called on revalidate or on read/write/format commands which
> also clear the DSKCHG bit.
> 
> Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
> ---
>  hw/fdc.c |   68 +++++++++++++++++++++++++++++++-------------------------------
>  1 files changed, 34 insertions(+), 34 deletions(-)

Nice cleanup! Looks good generally, but this and patch 4 need test cases
as well before I can apply them.

> @@ -1004,30 +998,39 @@ static int fdctrl_seek_to_next_sect(FDCtrl *fdctrl, FDrive *cur_drv)
>                     fd_sector(cur_drv));
>      /* XXX: cur_drv->sect >= cur_drv->last_sect should be an
>         error in fact */
> -    if (cur_drv->sect >= cur_drv->last_sect ||
> -        cur_drv->sect == fdctrl->eot) {
> -        cur_drv->sect = 1;
> +    uint8_t new_head = cur_drv->head;
> +    uint8_t new_track = cur_drv->track;
> +    uint8_t new_sect = cur_drv->sect;
> +
> +    int ret = 1;
> +
> +    if (new_sect >= cur_drv->last_sect ||
> +        new_sect == fdctrl->eot) {
> +        new_sect = 1;
>          if (FD_MULTI_TRACK(fdctrl->data_state)) {
> -            if (cur_drv->head == 0 &&
> +            if (new_head == 0 &&
>                  (cur_drv->flags & FDISK_DBL_SIDES) != 0) {
> -                cur_drv->head = 1;
> +                new_head = 1;
>              } else {
> -                cur_drv->head = 0;
> -                cur_drv->track++;
> +                new_head = 0;
> +                new_track++;
>                  if ((cur_drv->flags & FDISK_DBL_SIDES) == 0)
> -                    return 0;
> +                    ret = 0;

Please use the chance to add braces here.

Can you also add a small header comment to the function that explains
what the return value means? Initially I read 0 as error, but in fact it
seems only to mean "request completed".

Kevin

Patch

diff --git a/hw/fdc.c b/hw/fdc.c
index be35201..d338c3f 100644
--- a/hw/fdc.c
+++ b/hw/fdc.c
@@ -153,8 +153,12 @@  static int fd_seek(FDrive *drv, uint8_t head, uint8_t track, uint8_t sect,
         }
 #endif
         drv->head = head;
-        if (drv->track != track)
+        if (drv->track != track) {
+            if (drv->bs != NULL && bdrv_is_inserted(drv->bs)) {
+                drv->media_changed = 0;
+            }
             ret = 1;
+        }
         drv->track = track;
         drv->sect = sect;
     }
@@ -170,9 +174,7 @@  static int fd_seek(FDrive *drv, uint8_t head, uint8_t track, uint8_t sect,
 static void fd_recalibrate(FDrive *drv)
 {
     FLOPPY_DPRINTF("recalibrate\n");
-    drv->head = 0;
-    drv->track = 0;
-    drv->sect = 1;
+    fd_seek(drv, 0, 0, 1, 1);
 }
 
 /* Revalidate a disk drive after a disk change */
@@ -711,14 +713,6 @@  static void fdctrl_raise_irq(FDCtrl *fdctrl, uint8_t status0)
         qemu_set_irq(fdctrl->irq, 1);
         fdctrl->sra |= FD_SRA_INTPEND;
     }
-    if (status0 & FD_SR0_SEEK) {
-        FDrive *cur_drv;
-        /* A seek clears the disk change line (if a disk is inserted) */
-        cur_drv = get_cur_drv(fdctrl);
-        if (cur_drv->bs != NULL && bdrv_is_inserted(cur_drv->bs)) {
-            cur_drv->media_changed = 0;
-        }
-    }
 
     fdctrl->reset_sensei = 0;
     fdctrl->status0 = status0;
@@ -1004,30 +998,39 @@  static int fdctrl_seek_to_next_sect(FDCtrl *fdctrl, FDrive *cur_drv)
                    fd_sector(cur_drv));
     /* XXX: cur_drv->sect >= cur_drv->last_sect should be an
        error in fact */
-    if (cur_drv->sect >= cur_drv->last_sect ||
-        cur_drv->sect == fdctrl->eot) {
-        cur_drv->sect = 1;
+    uint8_t new_head = cur_drv->head;
+    uint8_t new_track = cur_drv->track;
+    uint8_t new_sect = cur_drv->sect;
+
+    int ret = 1;
+
+    if (new_sect >= cur_drv->last_sect ||
+        new_sect == fdctrl->eot) {
+        new_sect = 1;
         if (FD_MULTI_TRACK(fdctrl->data_state)) {
-            if (cur_drv->head == 0 &&
+            if (new_head == 0 &&
                 (cur_drv->flags & FDISK_DBL_SIDES) != 0) {
-                cur_drv->head = 1;
+                new_head = 1;
             } else {
-                cur_drv->head = 0;
-                cur_drv->track++;
+                new_head = 0;
+                new_track++;
                 if ((cur_drv->flags & FDISK_DBL_SIDES) == 0)
-                    return 0;
+                    ret = 0;
             }
         } else {
-            cur_drv->track++;
-            return 0;
+            new_track++;
+            ret = 0;
         }
-        FLOPPY_DPRINTF("seek to next track (%d %02x %02x => %d)\n",
+        if (ret == 1) {
+            FLOPPY_DPRINTF("seek to next track (%d %02x %02x => %d)\n",
                        cur_drv->head, cur_drv->track,
                        cur_drv->sect, fd_sector(cur_drv));
+        }
     } else {
-        cur_drv->sect++;
+        new_sect++;
     }
-    return 1;
+    fd_seek(cur_drv, new_head, new_track, new_sect, 1);
+    return ret;
 }
 
 /* Callback for transfer end (stop or abort) */
@@ -1623,11 +1626,7 @@  static void fdctrl_handle_seek(FDCtrl *fdctrl, int direction)
     /* The seek command just sends step pulses to the drive and doesn't care if
      * there is a medium inserted of if it's banging the head against the drive.
      */
-    if (fdctrl->fifo[2] > cur_drv->max_track) {
-        cur_drv->track = cur_drv->max_track;
-    } else {
-        cur_drv->track = fdctrl->fifo[2];
-    }
+    fd_seek(cur_drv, cur_drv->head, fdctrl->fifo[2], cur_drv->sect, 1);
     /* Raise Interrupt */
     fdctrl_raise_irq(fdctrl, FD_SR0_SEEK);
 }
@@ -1692,9 +1691,10 @@  static void fdctrl_handle_relative_seek_out(FDCtrl *fdctrl, int direction)
     SET_CUR_DRV(fdctrl, fdctrl->fifo[1] & FD_DOR_SELMASK);
     cur_drv = get_cur_drv(fdctrl);
     if (fdctrl->fifo[2] + cur_drv->track >= cur_drv->max_track) {
-        cur_drv->track = cur_drv->max_track - 1;
+        fd_seek(cur_drv, cur_drv->head, cur_drv->max_track - 1,
+                cur_drv->sect, 1);
     } else {
-        cur_drv->track += fdctrl->fifo[2];
+        fd_seek(cur_drv, cur_drv->head, fdctrl->fifo[2], cur_drv->sect, 1);
     }
     fdctrl_reset_fifo(fdctrl);
     /* Raise Interrupt */
@@ -1708,9 +1708,9 @@  static void fdctrl_handle_relative_seek_in(FDCtrl *fdctrl, int direction)
     SET_CUR_DRV(fdctrl, fdctrl->fifo[1] & FD_DOR_SELMASK);
     cur_drv = get_cur_drv(fdctrl);
     if (fdctrl->fifo[2] > cur_drv->track) {
-        cur_drv->track = 0;
+        fd_seek(cur_drv, cur_drv->head, 0, cur_drv->sect, 1);
     } else {
-        cur_drv->track -= fdctrl->fifo[2];
+        fd_seek(cur_drv, cur_drv->head, fdctrl->fifo[2], cur_drv->sect, 1);
     }
     fdctrl_reset_fifo(fdctrl);
     /* Raise Interrupt */