[PATCH 1/6] Introduce the floppy_dor_read() function

[Nikolay Nikolov]

Signed-off-by: Nikolay Nikolov 
---
 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);
     }
 }

[PATCH 2/6] Introduce floppy_dor_mask()

[Nikolay Nikolov]

Signed-off-by: Nikolay Nikolov 
---
 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);
     }
 }

[PATCH 3/6] Introduce FLOPPY_DOR_XXX constants

[Nikolay Nikolov]

Signed-off-by: Nikolay Nikolov 
---
 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);
     }
 }

[PATCH 4/6] Preserve the motor and drive sel bits when resetting the floppy controller

[Nikolay Nikolov]

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 
---
 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;

[PATCH 5/6] Reset the floppy motor count, so the motor keeps spinning 2 seconds after a floppy operation, performed by floppy_drive_pio()

[Nikolay Nikolov]

Signed-off-by: Nikolay Nikolov 
---
 src/hw/floppy.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/src/hw/floppy.c b/src/hw/floppy.c
index 573c45f..77dbade 100644
--- a/src/hw/floppy.c
+++ b/src/hw/floppy.c
@@ -356,6 +356,7 @@ floppy_drive_pio(u8 floppyid, int command, u8 *param)

// Send command.
     int ret = floppy_pio(command, param);
+    SET_BDA(floppy_motor_counter, FLOPPY_MOTOR_TICKS); // reset motor timeout
     if (ret)
         return ret;

[PATCH 5/6] Reset the floppy motor count, so the motor keeps spinning 2 seconds after a floppy operation, performed by floppy_drive_pio()

[Nikolay Nikolov]

This replaces patches 5 and 6 from the previous patch series as per
your suggestion.

Nikolay

On Sun, 2018-02-04 at 17:27 +0200, Nikolay Nikolov wrote:
> Signed-off-by: Nikolay Nikolov
>
> src/hw/floppy.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/src/hw/floppy.c b/src/hw/floppy.c
> index 573c45f..77dbade 100644
> --- a/src/hw/floppy.c
> +++ b/src/hw/floppy.c
> @@ -356,6 +356,7 @@ floppy_drive_pio(u8 floppyid, int command, u8
> *param)
>  // Send command.
>  int ret = floppy_pio(command, param);
> SET_BDA(floppy_motor_counter, FLOPPY_MOTOR_TICKS); // reset
>
> motor timeout
>      if (ret)
>          return ret;

[PATCH 6/6] Use timer_check() instead of using floppy_motor_counter in BDA for the timeout check in floppy_wait_irq()

[Nikolay Nikolov]

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 
---
 src/hw/floppy.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/src/hw/floppy.c b/src/hw/floppy.c
index 77dbade..3012b3a 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;
@@ -324,7 +326,6 @@ static int
 floppy_enable_controller(void)
 {
     dprintf(2, "Floppy_enable_controller\n");
-    SET_BDA(floppy_motor_counter, FLOPPY_MOTOR_TICKS);
     // 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
@@ -348,8 +349,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);
+    // 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);

[PATCH 6/6] Use timer_check() instead of using floppy_motor_counter in BDA for the timeout check in floppy_wait_irq()

[Nikolay Nikolov]

This is the same as patch 7 from the previous series, except it also
removes the line "SET_BDA(floppy_motor_counter, FLOPPY_MOTOR_TICKS);"
from floppy_enable_controller().

Nikolay

On Sun, 2018-02-04 at 17:27 +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
>
> src/hw/floppy.c | 9 +++++----
>  1 file changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/src/hw/floppy.c b/src/hw/floppy.c
> index 77dbade..3012b3a 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;
>
> @@ -324,7 +326,6 @@ static int
>  floppy_enable_controller(void)
>  {
>      dprintf(2, "Floppy_enable_controller\n");
> SET_BDA(floppy_motor_counter, FLOPPY_MOTOR_TICKS);
> // 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
> @@ -348,8 +349,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);
> // 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);

[Kevin O'Connor]

On Sun, Feb 04, 2018 at 05:26:56PM +0200, Nikolay Nikolov wrote:
> Signed-off-by: Nikolay Nikolov

Thanks.  The series looks good to me.  I'll give it a few days to see
if anyone else comments - otherwise I will commit it.

-Kevin

[Kevin O'Connor]

On Sun, Feb 04, 2018 at 05:26:56PM +0200, Nikolay Nikolov wrote:
> Signed-off-by: Nikolay Nikolov

Thanks.  I made some minor changes to the commit messages and
committed this series.

-Kevin

[Nikolay Nikolov]

On 02/08/2018 04:23 AM, Kevin O'Connor wrote:
> On Sun, Feb 04, 2018 at 05:26:56PM +0200, Nikolay Nikolov wrote:
> > Signed-off-by: Nikolay Nikolov
>
> Thanks.  I made some minor changes to the commit messages and
> committed this series.

Thanks. I'll prepare the rest of the patches, needed to get SeaBIOS 
working with a real floppy.

Nikolay