Signed-off-by: Nikolay Nikolov nickysn@users.sourceforge.net --- src/hw/floppy.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/src/hw/floppy.c b/src/hw/floppy.c index f2577c5..9c44a58 100644 --- a/src/hw/floppy.c +++ b/src/hw/floppy.c @@ -180,6 +180,12 @@ find_floppy_type(u32 size)
u8 FloppyDOR VARLOW;
+static inline u8 +floppy_dor_read(void) +{ + return GET_LOW(FloppyDOR); +} + static inline void floppy_dor_write(u8 val) { @@ -318,7 +324,7 @@ static int floppy_drive_pio(u8 floppyid, int command, u8 *param) { // Enable controller if it isn't running. - if (!(GET_LOW(FloppyDOR) & 0x04)) { + if (!(floppy_dor_read() & 0x04)) { int ret = floppy_enable_controller(); if (ret) return ret; @@ -668,6 +674,6 @@ floppy_tick(void) SET_BDA(floppy_motor_counter, fcount); if (fcount == 0) // turn motor(s) off - floppy_dor_write(GET_LOW(FloppyDOR) & ~0xf0); + floppy_dor_write(floppy_dor_read() & ~0xf0); } }
Signed-off-by: Nikolay Nikolov nickysn@users.sourceforge.net --- src/hw/floppy.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/src/hw/floppy.c b/src/hw/floppy.c index 9c44a58..f45676e 100644 --- a/src/hw/floppy.c +++ b/src/hw/floppy.c @@ -193,6 +193,12 @@ floppy_dor_write(u8 val) SET_LOW(FloppyDOR, val); }
+static inline void +floppy_dor_mask(u8 off, u8 on) +{ + floppy_dor_write((floppy_dor_read() & ~off) | on); +} + static void floppy_disable_controller(void) { @@ -674,6 +680,6 @@ floppy_tick(void) SET_BDA(floppy_motor_counter, fcount); if (fcount == 0) // turn motor(s) off - floppy_dor_write(floppy_dor_read() & ~0xf0); + floppy_dor_mask(0xf0, 0); } }
Signed-off-by: Nikolay Nikolov nickysn@users.sourceforge.net --- src/hw/floppy.c | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-)
diff --git a/src/hw/floppy.c b/src/hw/floppy.c index f45676e..992983d 100644 --- a/src/hw/floppy.c +++ b/src/hw/floppy.c @@ -35,6 +35,15 @@ #define FLOPPY_FORMAT_GAPLEN 0x6c #define FLOPPY_PIO_TIMEOUT 1000
+#define FLOPPY_DOR_MOTOR_D 0x80 // Set to turn drive 3's motor ON +#define FLOPPY_DOR_MOTOR_C 0x40 // Set to turn drive 2's motor ON +#define FLOPPY_DOR_MOTOR_B 0x20 // Set to turn drive 1's motor ON +#define FLOPPY_DOR_MOTOR_A 0x10 // Set to turn drive 0's motor ON +#define FLOPPY_DOR_MOTOR_MASK 0xf0 +#define FLOPPY_DOR_IRQ 0x08 // Set to enable IRQs and DMA +#define FLOPPY_DOR_RESET 0x04 // Clear = enter reset mode, Set = normal operation +#define FLOPPY_DOR_DSEL_MASK 0x03 // "Select" drive number for next access + // New diskette parameter table adding 3 parameters from IBM // Since no provisions are made for multiple drive types, most // values in this table are ignored. I set parameters for 1.44M @@ -316,7 +325,7 @@ floppy_enable_controller(void) dprintf(2, "Floppy_enable_controller\n"); SET_BDA(floppy_motor_counter, FLOPPY_MOTOR_TICKS); floppy_dor_write(0x00); - floppy_dor_write(0x0c); + floppy_dor_write(FLOPPY_DOR_IRQ | FLOPPY_DOR_RESET); int ret = floppy_wait_irq(); if (ret) return ret; @@ -330,7 +339,7 @@ static int floppy_drive_pio(u8 floppyid, int command, u8 *param) { // Enable controller if it isn't running. - if (!(floppy_dor_read() & 0x04)) { + if (!(floppy_dor_read() & FLOPPY_DOR_RESET)) { int ret = floppy_enable_controller(); if (ret) return ret; @@ -340,7 +349,7 @@ floppy_drive_pio(u8 floppyid, int command, u8 *param) SET_BDA(floppy_motor_counter, FLOPPY_MOTOR_TICKS);
// Turn on motor of selected drive, DMA & int enabled, normal operation - floppy_dor_write((floppyid ? 0x20 : 0x10) | 0x0c | floppyid); + floppy_dor_write((floppyid ? FLOPPY_DOR_MOTOR_B : FLOPPY_DOR_MOTOR_A) | FLOPPY_DOR_IRQ | FLOPPY_DOR_RESET | floppyid);
// Send command. int ret = floppy_pio(command, param); @@ -680,6 +689,6 @@ floppy_tick(void) SET_BDA(floppy_motor_counter, fcount); if (fcount == 0) // turn motor(s) off - floppy_dor_mask(0xf0, 0); + floppy_dor_mask(FLOPPY_DOR_MOTOR_MASK, 0); } }
In case of read or write errors, the floppy system is usually reset and the operation is retried. In that case, the floppy motor state must be preserved in order to avoid creating jitter and keep the floppy motor spinning smoothly at a constant speed. Additionally, the drive select bits should probably also be preserved, because some systems might need a small delay after selecting a new drive. In that case, the operation would be retried, without changing the currently selected drive.
In floppy_enable_controller(), the IRQ bit is now enabled first, before the reset bit is set. I'm not completely sure whether this is necessary. It is done just in case some hardware introduces a delay between setting this bit and actually enabling the IRQ, which would cause us to miss the IRQ, sent by the controller immediately after reset.
Signed-off-by: Nikolay Nikolov nickysn@users.sourceforge.net --- src/hw/floppy.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/src/hw/floppy.c b/src/hw/floppy.c index 992983d..573c45f 100644 --- a/src/hw/floppy.c +++ b/src/hw/floppy.c @@ -212,7 +212,8 @@ static void floppy_disable_controller(void) { dprintf(2, "Floppy_disable_controller\n"); - floppy_dor_write(0x00); + // Clear the reset bit (enter reset state) and clear 'enable IRQ and DMA' + floppy_dor_mask(FLOPPY_DOR_IRQ | FLOPPY_DOR_RESET, 0); }
static int @@ -324,8 +325,10 @@ floppy_enable_controller(void) { dprintf(2, "Floppy_enable_controller\n"); SET_BDA(floppy_motor_counter, FLOPPY_MOTOR_TICKS); - floppy_dor_write(0x00); - floppy_dor_write(FLOPPY_DOR_IRQ | FLOPPY_DOR_RESET); + // Clear the reset bit (enter reset state), but set 'enable IRQ and DMA' + floppy_dor_mask(FLOPPY_DOR_RESET, FLOPPY_DOR_IRQ); + // Set the reset bit (normal operation) and keep 'enable IRQ and DMA' on + floppy_dor_mask(0, FLOPPY_DOR_IRQ | FLOPPY_DOR_RESET); int ret = floppy_wait_irq(); if (ret) return ret;
Signed-off-by: Nikolay Nikolov nickysn@users.sourceforge.net --- src/hw/floppy.c | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-)
diff --git a/src/hw/floppy.c b/src/hw/floppy.c index 573c45f..b5bc114 100644 --- a/src/hw/floppy.c +++ b/src/hw/floppy.c @@ -337,6 +337,16 @@ floppy_enable_controller(void) return floppy_pio(FC_CHECKIRQ, param); }
+static void +floppy_turn_on_motor(u8 floppyid) +{ + // reset the disk motor timeout value of INT 08 + SET_BDA(floppy_motor_counter, FLOPPY_MOTOR_TICKS); + + // Turn on motor of selected drive, DMA & int enabled, normal operation + floppy_dor_write((floppyid ? FLOPPY_DOR_MOTOR_B : FLOPPY_DOR_MOTOR_A) | FLOPPY_DOR_IRQ | FLOPPY_DOR_RESET | floppyid); +} + // Activate a drive and send a command to it. static int floppy_drive_pio(u8 floppyid, int command, u8 *param) @@ -348,11 +358,8 @@ floppy_drive_pio(u8 floppyid, int command, u8 *param) return ret; }
- // reset the disk motor timeout value of INT 08 - SET_BDA(floppy_motor_counter, FLOPPY_MOTOR_TICKS); - - // Turn on motor of selected drive, DMA & int enabled, normal operation - floppy_dor_write((floppyid ? FLOPPY_DOR_MOTOR_B : FLOPPY_DOR_MOTOR_A) | FLOPPY_DOR_IRQ | FLOPPY_DOR_RESET | floppyid); + // Turn on motor of selected drive + floppy_turn_on_motor(floppyid);
// Send command. int ret = floppy_pio(command, param);
The new function is called after each floppy operation (except controller reset) and resets the floppy motor counter in BDA to FLOPPY_MOTOR_TICKS (about 2 seconds).
Signed-off-by: Nikolay Nikolov nickysn@users.sourceforge.net --- src/hw/floppy.c | 35 ++++++++++++++++++++++++++++------- 1 file changed, 28 insertions(+), 7 deletions(-)
diff --git a/src/hw/floppy.c b/src/hw/floppy.c index b5bc114..acaff3f 100644 --- a/src/hw/floppy.c +++ b/src/hw/floppy.c @@ -347,6 +347,13 @@ floppy_turn_on_motor(u8 floppyid) floppy_dor_write((floppyid ? FLOPPY_DOR_MOTOR_B : FLOPPY_DOR_MOTOR_A) | FLOPPY_DOR_IRQ | FLOPPY_DOR_RESET | floppyid); }
+static inline void +floppy_turn_off_motor_delayed(void) +{ + // reset the disk motor timeout value of INT 08 + SET_BDA(floppy_motor_counter, FLOPPY_MOTOR_TICKS); +} + // Activate a drive and send a command to it. static int floppy_drive_pio(u8 floppyid, int command, u8 *param) @@ -570,8 +577,10 @@ floppy_read(struct disk_op_s *op) { struct chs_s chs = lba2chs(op); int ret = floppy_prep(op->drive_fl, chs.cylinder); - if (ret) + if (ret) { + floppy_turn_off_motor_delayed(); return ret; + }
// send read-normal-data command to controller u8 floppyid = GET_GLOBALFLAT(op->drive_fl->cntl_id); @@ -584,7 +593,9 @@ floppy_read(struct disk_op_s *op) param[5] = chs.sector + op->count - 1; // last sector to read on track param[6] = FLOPPY_GAPLEN; param[7] = FLOPPY_DATALEN; - return floppy_dma_cmd(op, op->count * DISK_SECTOR_SIZE, FC_READ, param); + ret = floppy_dma_cmd(op, op->count * DISK_SECTOR_SIZE, FC_READ, param); + floppy_turn_off_motor_delayed(); + return ret; }
// Write Diskette Sectors @@ -593,8 +604,10 @@ floppy_write(struct disk_op_s *op) { struct chs_s chs = lba2chs(op); int ret = floppy_prep(op->drive_fl, chs.cylinder); - if (ret) + if (ret) { + floppy_turn_off_motor_delayed(); return ret; + }
// send write-normal-data command to controller u8 floppyid = GET_GLOBALFLAT(op->drive_fl->cntl_id); @@ -607,7 +620,9 @@ floppy_write(struct disk_op_s *op) param[5] = chs.sector + op->count - 1; // last sector to write on track param[6] = FLOPPY_GAPLEN; param[7] = FLOPPY_DATALEN; - return floppy_dma_cmd(op, op->count * DISK_SECTOR_SIZE, FC_WRITE, param); + ret = floppy_dma_cmd(op, op->count * DISK_SECTOR_SIZE, FC_WRITE, param); + floppy_turn_off_motor_delayed(); + return ret; }
// Verify Diskette Sectors @@ -616,8 +631,10 @@ floppy_verify(struct disk_op_s *op) { struct chs_s chs = lba2chs(op); int ret = floppy_prep(op->drive_fl, chs.cylinder); - if (ret) + if (ret) { + floppy_turn_off_motor_delayed(); return ret; + }
// This command isn't implemented - just return success. return DISK_RET_SUCCESS; @@ -629,8 +646,10 @@ floppy_format(struct disk_op_s *op) { struct chs_s chs = lba2chs(op); int ret = floppy_prep(op->drive_fl, chs.cylinder); - if (ret) + if (ret) { + floppy_turn_off_motor_delayed(); return ret; + }
// send format-track command to controller u8 floppyid = GET_GLOBALFLAT(op->drive_fl->cntl_id); @@ -640,7 +659,9 @@ floppy_format(struct disk_op_s *op) param[2] = op->count; // number of sectors per track param[3] = FLOPPY_FORMAT_GAPLEN; param[4] = FLOPPY_FILLBYTE; - return floppy_dma_cmd(op, op->count * 4, FC_FORMAT, param); + ret = floppy_dma_cmd(op, op->count * 4, FC_FORMAT, param); + floppy_turn_off_motor_delayed(); + return ret; }
int
The problem with using floppy_motor_counter was that, after it reaches 0, it immediately stops the floppy motors, which is not what is supposed to happen on real hardware. Instead, after a timeout (like in the end of every floppy operation, regardless of the result - success, timeout or error), the floppy motors must be kept spinning for additional 2 seconds (the FLOPPY_MOTOR_TICKS). So, now the floppy_motor_counter is initialized to 255 (the max value) in the beginning of the floppy operation (in floppy_turn_on_motor()). For IRQ timeouts, a different timeout is used, specified by the new FLOPPY_IRQ_TIMEOUT constant (currently set to 5 seconds - a fairly conservative value, but should work reliably on most floppies).
After the floppy operation, floppy_turn_off_motor_delayed() resets the floppy_motor_counter to 2 seconds (FLOPPY_MOTOR_TICKS).
This is also consistent with what other PC BIOSes do.
Signed-off-by: Nikolay Nikolov nickysn@users.sourceforge.net --- src/hw/floppy.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/src/hw/floppy.c b/src/hw/floppy.c index acaff3f..6714402 100644 --- a/src/hw/floppy.c +++ b/src/hw/floppy.c @@ -34,6 +34,7 @@ #define FLOPPY_GAPLEN 0x1B #define FLOPPY_FORMAT_GAPLEN 0x6c #define FLOPPY_PIO_TIMEOUT 1000 +#define FLOPPY_IRQ_TIMEOUT 5000
#define FLOPPY_DOR_MOTOR_D 0x80 // Set to turn drive 3's motor ON #define FLOPPY_DOR_MOTOR_C 0x40 // Set to turn drive 2's motor ON @@ -221,8 +222,9 @@ floppy_wait_irq(void) { u8 frs = GET_BDA(floppy_recalibration_status); SET_BDA(floppy_recalibration_status, frs & ~FRS_IRQ); + u32 end = timer_calc(FLOPPY_IRQ_TIMEOUT); for (;;) { - if (!GET_BDA(floppy_motor_counter)) { + if (timer_check(end)) { warn_timeout(); floppy_disable_controller(); return DISK_RET_ETIMEOUT; @@ -340,8 +342,8 @@ floppy_enable_controller(void) static void floppy_turn_on_motor(u8 floppyid) { - // reset the disk motor timeout value of INT 08 - SET_BDA(floppy_motor_counter, FLOPPY_MOTOR_TICKS); + // set the disk motor timeout value of INT 08 to the highest value + SET_BDA(floppy_motor_counter, 255);
// Turn on motor of selected drive, DMA & int enabled, normal operation floppy_dor_write((floppyid ? FLOPPY_DOR_MOTOR_B : FLOPPY_DOR_MOTOR_A) | FLOPPY_DOR_IRQ | FLOPPY_DOR_RESET | floppyid);
On Sat, Feb 03, 2018 at 07:46:11PM +0200, Nikolay Nikolov wrote:
The problem with using floppy_motor_counter was that, after it reaches 0, it immediately stops the floppy motors, which is not what is supposed to happen on real hardware. Instead, after a timeout (like in the end of every floppy operation, regardless of the result - success, timeout or error), the floppy motors must be kept spinning for additional 2 seconds (the FLOPPY_MOTOR_TICKS). So, now the floppy_motor_counter is initialized to 255 (the max value) in the beginning of the floppy operation (in floppy_turn_on_motor()). For IRQ timeouts, a different timeout is used, specified by the new FLOPPY_IRQ_TIMEOUT constant (currently set to 5 seconds - a fairly conservative value, but should work reliably on most floppies).
After the floppy operation, floppy_turn_off_motor_delayed() resets the floppy_motor_counter to 2 seconds (FLOPPY_MOTOR_TICKS).
This is also consistent with what other PC BIOSes do.
Signed-off-by: Nikolay Nikolov nickysn@users.sourceforge.net
src/hw/floppy.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/src/hw/floppy.c b/src/hw/floppy.c index acaff3f..6714402 100644 --- a/src/hw/floppy.c +++ b/src/hw/floppy.c @@ -34,6 +34,7 @@ #define FLOPPY_GAPLEN 0x1B #define FLOPPY_FORMAT_GAPLEN 0x6c #define FLOPPY_PIO_TIMEOUT 1000 +#define FLOPPY_IRQ_TIMEOUT 5000
#define FLOPPY_DOR_MOTOR_D 0x80 // Set to turn drive 3's motor ON #define FLOPPY_DOR_MOTOR_C 0x40 // Set to turn drive 2's motor ON @@ -221,8 +222,9 @@ floppy_wait_irq(void) { u8 frs = GET_BDA(floppy_recalibration_status); SET_BDA(floppy_recalibration_status, frs & ~FRS_IRQ);
- u32 end = timer_calc(FLOPPY_IRQ_TIMEOUT); for (;;) {
if (!GET_BDA(floppy_motor_counter)) {
if (timer_check(end)) { warn_timeout(); floppy_disable_controller(); return DISK_RET_ETIMEOUT;
Thanks. This patch looks good to me.
One question - if floppy_wait_irq() no longer accesses floppy_motor_counter, can we also change floppy_enable_controller() to not alter floppy_motor_counter as well? That is, can we treat floppy_motor_counter as part of the logic to send commands to drives (as opposed to sending commands to the controller) and thus limit floppy_motor_counter logic to the floppy_drive_pio() code?
-Kevin
On Sat, 2018-02-03 at 15:17 -0500, Kevin O'Connor wrote:
On Sat, Feb 03, 2018 at 07:46:11PM +0200, Nikolay Nikolov wrote:
The problem with using floppy_motor_counter was that, after it reaches 0, it immediately stops the floppy motors, which is not what is supposed to happen on real hardware. Instead, after a timeout (like in the end of every floppy operation, regardless of the result - success, timeout or error), the floppy motors must be kept spinning for additional 2 seconds (the FLOPPY_MOTOR_TICKS). So, now the floppy_motor_counter is initialized to 255 (the max value) in the beginning of the floppy operation (in floppy_turn_on_motor()). For IRQ timeouts, a different timeout is used, specified by the new FLOPPY_IRQ_TIMEOUT constant (currently set to 5 seconds - a fairly conservative value, but should work reliably on most floppies).
After the floppy operation, floppy_turn_off_motor_delayed() resets the floppy_motor_counter to 2 seconds (FLOPPY_MOTOR_TICKS).
This is also consistent with what other PC BIOSes do.
Signed-off-by: Nikolay Nikolov nickysn@users.sourceforge.net
src/hw/floppy.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/src/hw/floppy.c b/src/hw/floppy.c index acaff3f..6714402 100644 --- a/src/hw/floppy.c +++ b/src/hw/floppy.c @@ -34,6 +34,7 @@ #define FLOPPY_GAPLEN 0x1B #define FLOPPY_FORMAT_GAPLEN 0x6c #define FLOPPY_PIO_TIMEOUT 1000 +#define FLOPPY_IRQ_TIMEOUT 5000
#define FLOPPY_DOR_MOTOR_D 0x80 // Set to turn drive 3's motor ON #define FLOPPY_DOR_MOTOR_C 0x40 // Set to turn drive 2's motor ON @@ -221,8 +222,9 @@ floppy_wait_irq(void) { u8 frs = GET_BDA(floppy_recalibration_status); SET_BDA(floppy_recalibration_status, frs & ~FRS_IRQ);
- u32 end = timer_calc(FLOPPY_IRQ_TIMEOUT); for (;;) {
if (!GET_BDA(floppy_motor_counter)) {
if (timer_check(end)) { warn_timeout(); floppy_disable_controller(); return DISK_RET_ETIMEOUT;Thanks. This patch looks good to me.
One question - if floppy_wait_irq() no longer accesses floppy_motor_counter, can we also change floppy_enable_controller() to not alter floppy_motor_counter as well? That is, can we treat floppy_motor_counter as part of the logic to send commands to drives (as opposed to sending commands to the controller) and thus limit floppy_motor_counter logic to the floppy_drive_pio() code?
Yes, I think so. Should I amend it to this patch?
-Kevin
-
Kevin O'Connor -
Nikolay Nikolov -
Nikolay Nikolov