Patchwork [v3,1/9] fdc: take side count into account

login
register
mail settings
Submitter Hervé Poussineau
Date Jan. 23, 2012, 8:50 a.m.
Message ID <1327308641-14736-2-git-send-email-hpoussin@reactos.org>
Download mbox | patch
Permalink /patch/137298/
State New
Headers show

Comments

Hervé Poussineau - Jan. 23, 2012, 8:50 a.m.
Floppies can be simple or double-sided. However, current code
was only taking the common case into account (ie 2 sides).

Signed-off-by: Hervé Poussineau <hpoussin@reactos.org>
---
 hw/fdc.c |   17 +++++++++++------
 1 files changed, 11 insertions(+), 6 deletions(-)
Markus Armbruster - Jan. 27, 2012, 8:38 a.m.
Hervé Poussineau <hpoussin@reactos.org> writes:

> Floppies can be simple or double-sided. However, current code
> was only taking the common case into account (ie 2 sides).

Impact?  Are single sided floppies broken before the patch?  How?
Hervé Poussineau - Jan. 27, 2012, 8:11 p.m.
Markus Armbruster a écrit :
> Hervé Poussineau <hpoussin@reactos.org> writes:
> 
>> Floppies can be simple or double-sided. However, current code
>> was only taking the common case into account (ie 2 sides).
> 
> Impact?

This repairs single-sided floppies

> Are single sided floppies broken before the patch? How?

Yes. For head > 0, wrong sector number is calculated, so data is 
read/written at wrong place on underlying block device.

Fortunately, mostly nobody is still using single-sided floppies (only 
some 360 kB floppies are single-sided), so bug has been neglected since 
a long time.

Hervé
Markus Armbruster - Jan. 31, 2012, 9:28 a.m.
Hervé Poussineau <hpoussin@reactos.org> writes:

> Markus Armbruster a écrit :
>> Hervé Poussineau <hpoussin@reactos.org> writes:
>>
>>> Floppies can be simple or double-sided. However, current code
>>> was only taking the common case into account (ie 2 sides).
>>
>> Impact?
>
> This repairs single-sided floppies
>
>> Are single sided floppies broken before the patch? How?
>
> Yes. For head > 0, wrong sector number is calculated, so data is
> read/written at wrong place on underlying block device.
>
> Fortunately, mostly nobody is still using single-sided floppies (only
> some 360 kB floppies are single-sided), so bug has been neglected
> since a long time.

Nice info, thanks.  Best put it in the commit message.
Hervé Poussineau - Jan. 31, 2012, 10:24 p.m.
Hi,

Markus Armbruster a écrit :
>>> Are single sided floppies broken before the patch? How?
>> Yes. For head > 0, wrong sector number is calculated, so data is
>> read/written at wrong place on underlying block device.
>>
>> Fortunately, mostly nobody is still using single-sided floppies (only
>> some 360 kB floppies are single-sided), so bug has been neglected
>> since a long time.
> 
> Nice info, thanks.  Best put it in the commit message.

Will do when respinning the serie

Hervé

Patch

diff --git a/hw/fdc.c b/hw/fdc.c
index 70aa5c7..c1898a6 100644
--- a/hw/fdc.c
+++ b/hw/fdc.c
@@ -95,16 +95,19 @@  static void fd_init(FDrive *drv)
     drv->max_track = 0;
 }
 
+#define NUM_SIDES(drv) ((drv)->flags & FDISK_DBL_SIDES ? 2 : 1)
+
 static int fd_sector_calc(uint8_t head, uint8_t track, uint8_t sect,
-                          uint8_t last_sect)
+                          uint8_t last_sect, uint8_t num_sides)
 {
-    return (((track * 2) + head) * last_sect) + sect - 1;
+    return (((track * num_sides) + head) * last_sect) + sect - 1;
 }
 
 /* Returns current position, in sectors, for given drive */
 static int fd_sector(FDrive *drv)
 {
-    return fd_sector_calc(drv->head, drv->track, drv->sect, drv->last_sect);
+    return fd_sector_calc(drv->head, drv->track, drv->sect, drv->last_sect,
+                          NUM_SIDES(drv));
 }
 
 /* Seek to a new position:
@@ -135,7 +138,7 @@  static int fd_seek(FDrive *drv, uint8_t head, uint8_t track, uint8_t sect,
                        drv->max_track, drv->last_sect);
         return 3;
     }
-    sector = fd_sector_calc(head, track, sect, drv->last_sect);
+    sector = fd_sector_calc(head, track, sect, drv->last_sect, NUM_SIDES(drv));
     ret = 0;
     if (sector != fd_sector(drv)) {
 #if 0
@@ -1019,7 +1022,8 @@  static void fdctrl_start_transfer(FDCtrl *fdctrl, int direction)
     ks = fdctrl->fifo[4];
     FLOPPY_DPRINTF("Start transfer at %d %d %02x %02x (%d)\n",
                    GET_CUR_DRV(fdctrl), kh, kt, ks,
-                   fd_sector_calc(kh, kt, ks, cur_drv->last_sect));
+                   fd_sector_calc(kh, kt, ks, cur_drv->last_sect,
+                                  NUM_SIDES(cur_drv)));
     switch (fd_seek(cur_drv, kh, kt, ks, fdctrl->config & FD_CONFIG_EIS)) {
     case 2:
         /* sect too big */
@@ -1289,7 +1293,8 @@  static void fdctrl_format_sector(FDCtrl *fdctrl)
     ks = fdctrl->fifo[8];
     FLOPPY_DPRINTF("format sector at %d %d %02x %02x (%d)\n",
                    GET_CUR_DRV(fdctrl), kh, kt, ks,
-                   fd_sector_calc(kh, kt, ks, cur_drv->last_sect));
+                   fd_sector_calc(kh, kt, ks, cur_drv->last_sect,
+                                  NUM_SIDES(cur_drv)));
     switch (fd_seek(cur_drv, kh, kt, ks, fdctrl->config & FD_CONFIG_EIS)) {
     case 2:
         /* sect too big */