Re: [SeaBIOS] [PATCH 5/7] Move the code that turns on the floppy motor to floppy_turn_on_motor()

[Kevin O'Connor]

On Sat, Feb 03, 2018 at 07:46:09PM +0200, Nikolay Nikolov wrote:
> Signed-off-by: Nikolay Nikolov
>
> 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);

Thanks!  Patches 1-4 look good to me.  Instead of patch 5/6 could the
below be used instead?  Specifically, instead of calling
floppy_turn_off_motor_delayed() on every return path of
floppy_drive_pio(), can we have floppy_drive_pio() reset the delay
itself before returning?

-Kevin

--- 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/7] Move the code that turns on the floppy motor to floppy_turn_on_motor()

[Nikolay Nikolov]

On Sat, 2018-02-03 at 14:12 -0500, Kevin O'Connor wrote:
> On Sat, Feb 03, 2018 at 07:46:09PM +0200, Nikolay Nikolov wrote:
> > Signed-off-by: Nikolay Nikolov
> >
> > 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);
>
> Thanks!  Patches 1-4 look good to me.  Instead of patch 5/6 could the
> below be used instead?  Specifically, instead of calling
> floppy_turn_off_motor_delayed() on every return path of
> floppy_drive_pio(), can we have floppy_drive_pio() reset the delay
> itself before returning?

Yes, I think so. I'll update my patch set this way.

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