Re: [Qemu-devel] [PATCH 08/10] fdc: add CCR (Configuration Control Register) write register

Re: [Qemu-devel] [PATCH 08/10] fdc: add CCR (Configuration Control Register) write register

[Hervé Poussineau]

On 01/09/2012 09:09 AM, Paolo Bonzini wrote:

    This should go before patch 7, shouldn't it?

      


Yes, I can exchange patches 7 and 8.

However, in this order, only floppies other than 1.44 Mb may fail 
(depends if DSR or CCR register is used to write required rate).

I would like to have some more review before respinning (if required...)

Hervé

[Qemu-devel] [PATCH 00/10] Misc fixes for floppy emulation

[Hervé Poussineau]

Here are misc fixes done by VirtualBox team.
With these patches, floppy emulation is now good enough to run Xenix.

Hervé Poussineau (10):
  fdc: take head count into account
  fdc: set busy bit when starting a command
  fdc: most control commands do not generate interrupts
  fdc: emulate stepping 0
  fdc: handle read-only floppies (abort early on write commands)
  block: add a transfer rate for floppy types
  fdc: check if media rate is correct before doing any transfer
  fdc: add CCR (Configuration Control Register) write register
  fdc: fix seek command, which shouldn't check tracks
  fdc: DIR (Digital Input Register) should return status of current
    drive...

 block.c  |   74 +++++++++++++++++++++-------------------
 block.h  |   10 +++++-
 hw/fdc.c |  114 +++++++++++++++++++++++++++++++++++++++++++++----------------
 hw/pc.c  |    3 +-
 4 files changed, 134 insertions(+), 67 deletions(-)

-- 
1.7.7.3

[Qemu-devel] [PATCH 08/10] fdc: add CCR (Configuration Control Register) write register

[Hervé Poussineau]

Signed-off-by: Hervé Poussineau <hpoussin@reactos.org>
---
 hw/fdc.c |   21 +++++++++++++++++++++
 1 files changed, 21 insertions(+), 0 deletions(-)

diff --git a/hw/fdc.c b/hw/fdc.c
index ddfa91f..67cd14f 100644
--- a/hw/fdc.c
+++ b/hw/fdc.c
@@ -227,6 +227,7 @@ static void fdctrl_write_rate(FDCtrl *fdctrl, uint32_t value);
 static uint32_t fdctrl_read_data(FDCtrl *fdctrl);
 static void fdctrl_write_data(FDCtrl *fdctrl, uint32_t value);
 static uint32_t fdctrl_read_dir(FDCtrl *fdctrl);
+static void fdctrl_write_ccr(FDCtrl *fdctrl, uint32_t value);
 
 enum {
     FD_DIR_WRITE   = 0,
@@ -251,6 +252,7 @@ enum {
     FD_REG_DSR = 0x04,
     FD_REG_FIFO = 0x05,
     FD_REG_DIR = 0x07,
+    FD_REG_CCR = 0x07,
 };
 
 enum {
@@ -495,6 +497,8 @@ static void fdctrl_write (void *opaque, uint32_t reg, uint32_t value)
     case FD_REG_FIFO:
         fdctrl_write_data(fdctrl, value);
         break;
+    case FD_REG_CCR:
+        fdctrl_write_ccr(fdctrl, value);
     default:
         break;
     }
@@ -885,6 +889,23 @@ static void fdctrl_write_rate(FDCtrl *fdctrl, uint32_t value)
     fdctrl->dsr = value;
 }
 
+/* Configuration control register: 0x07 (write) */
+static void fdctrl_write_ccr(FDCtrl *fdctrl, uint32_t value)
+{
+    /* Reset mode */
+    if (!(fdctrl->dor & FD_DOR_nRESET)) {
+        FLOPPY_DPRINTF("Floppy controller in RESET state !\n");
+        return;
+    }
+    FLOPPY_DPRINTF("configuration control register set to 0x%02x\n", value);
+
+    /* Only the rate selection bits used in AT mode, and we
+     * store those in the DSR.
+     */
+    fdctrl->dsr = (fdctrl->dsr & ~FD_DSR_DRATEMASK) |
+                  (value & FD_DSR_DRATEMASK);
+}
+
 static int fdctrl_media_changed(FDrive *drv)
 {
     int ret;
-- 
1.7.7.3

Re: [Qemu-devel] [PATCH 08/10] fdc: add CCR (Configuration Control Register) write register

[Paolo Bonzini]

On 01/08/2012 09:27 PM, Hervé Poussineau wrote:
>
> Signed-off-by: Hervé Poussineau<hpoussin@reactos.org>
> ---
>   hw/fdc.c |   21 +++++++++++++++++++++
>   1 files changed, 21 insertions(+), 0 deletions(-)
>
> diff --git a/hw/fdc.c b/hw/fdc.c
> index ddfa91f..67cd14f 100644
> --- a/hw/fdc.c
> +++ b/hw/fdc.c
> @@ -227,6 +227,7 @@ static void fdctrl_write_rate(FDCtrl *fdctrl, uint32_t value);
>   static uint32_t fdctrl_read_data(FDCtrl *fdctrl);
>   static void fdctrl_write_data(FDCtrl *fdctrl, uint32_t value);
>   static uint32_t fdctrl_read_dir(FDCtrl *fdctrl);
> +static void fdctrl_write_ccr(FDCtrl *fdctrl, uint32_t value);
>
>   enum {
>       FD_DIR_WRITE   = 0,
> @@ -251,6 +252,7 @@ enum {
>       FD_REG_DSR = 0x04,
>       FD_REG_FIFO = 0x05,
>       FD_REG_DIR = 0x07,
> +    FD_REG_CCR = 0x07,
>   };
>
>   enum {
> @@ -495,6 +497,8 @@ static void fdctrl_write (void *opaque, uint32_t reg, uint32_t value)
>       case FD_REG_FIFO:
>           fdctrl_write_data(fdctrl, value);
>           break;
> +    case FD_REG_CCR:
> +        fdctrl_write_ccr(fdctrl, value);
>       default:
>           break;
>       }
> @@ -885,6 +889,23 @@ static void fdctrl_write_rate(FDCtrl *fdctrl, uint32_t value)
>       fdctrl->dsr = value;
>   }
>
> +/* Configuration control register: 0x07 (write) */
> +static void fdctrl_write_ccr(FDCtrl *fdctrl, uint32_t value)
> +{
> +    /* Reset mode */
> +    if (!(fdctrl->dor&  FD_DOR_nRESET)) {
> +        FLOPPY_DPRINTF("Floppy controller in RESET state !\n");
> +        return;
> +    }
> +    FLOPPY_DPRINTF("configuration control register set to 0x%02x\n", value);
> +
> +    /* Only the rate selection bits used in AT mode, and we
> +     * store those in the DSR.
> +     */
> +    fdctrl->dsr = (fdctrl->dsr&  ~FD_DSR_DRATEMASK) |
> +                  (value&  FD_DSR_DRATEMASK);
> +}
> +
>   static int fdctrl_media_changed(FDrive *drv)
>   {
>       int ret;

This should go before patch 7, shouldn't it?

Paolo

Re: [Qemu-devel] [PATCH 08/10] fdc: add CCR (Configuration Control Register) write register

[Andreas Färber]

Am 08.01.2012 21:27, schrieb Hervé Poussineau:
> 
> Signed-off-by: Hervé Poussineau <hpoussin@reactos.org>
> ---
>  hw/fdc.c |   21 +++++++++++++++++++++
>  1 files changed, 21 insertions(+), 0 deletions(-)
> 
> diff --git a/hw/fdc.c b/hw/fdc.c
> index ddfa91f..67cd14f 100644
> --- a/hw/fdc.c
> +++ b/hw/fdc.c
> @@ -227,6 +227,7 @@ static void fdctrl_write_rate(FDCtrl *fdctrl, uint32_t value);
>  static uint32_t fdctrl_read_data(FDCtrl *fdctrl);
>  static void fdctrl_write_data(FDCtrl *fdctrl, uint32_t value);
>  static uint32_t fdctrl_read_dir(FDCtrl *fdctrl);
> +static void fdctrl_write_ccr(FDCtrl *fdctrl, uint32_t value);
>  
>  enum {
>      FD_DIR_WRITE   = 0,
> @@ -251,6 +252,7 @@ enum {
>      FD_REG_DSR = 0x04,
>      FD_REG_FIFO = 0x05,
>      FD_REG_DIR = 0x07,
> +    FD_REG_CCR = 0x07,

I'm not familiar with FDCs, so this reads weird: Do DIR and CCR share
the same address, with DIR being read-only and CCR write-only? If so, an
explanatory sentence in the commit message would be nice. (dito for most
other patches in the series)

>  };
>  
>  enum {
> @@ -495,6 +497,8 @@ static void fdctrl_write (void *opaque, uint32_t reg, uint32_t value)
>      case FD_REG_FIFO:
>          fdctrl_write_data(fdctrl, value);
>          break;
> +    case FD_REG_CCR:
> +        fdctrl_write_ccr(fdctrl, value);

break; please, to avoid future errors or complaints by static analysis
tools.

Andreas

>      default:
>          break;
>      }

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg