qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2] fdc/i8257: implement verify transfer mode
@ 2019-10-30  8:28 Sven Schnelle
  2019-11-01 10:54 ` Hervé Poussineau
  0 siblings, 1 reply; 3+ messages in thread
From: Sven Schnelle @ 2019-10-30  8:28 UTC (permalink / raw)
  To: John Snow
  Cc: Kevin Wolf, open list:Floppy, Michael S. Tsirkin,
	open list:All patches CC here, Max Reitz, Hervé Poussineau,
	Paolo Bonzini, Sven Schnelle

While working on the Tulip driver i tried to write some Teledisk images to
a floppy image which didn't work. Turned out that Teledisk checks the written
data by issuing a READ command to the FDC but running the DMA controller
in VERIFY mode. As we ignored the DMA request in that case, the DMA transfer
never finished, and Teledisk reported an error.

The i8257 spec says about verify transfers:

3) DMA verify, which does not actually involve the transfer of data. When an
8257 channel is in the DMA verify mode, it will respond the same as described
for transfer operations, except that no memory or I/O read/write control signals
will be generated.

Hervé proposed to remove all the dma_mode_ok stuff from fdc to have a more
clear boundary between DMA and FDC, so this patch also does that.

Suggested-by: Hervé Poussineau <hpoussin@reactos.org>
Signed-off-by: Sven Schnelle <svens@stackframe.org>
---
 hw/block/fdc.c       | 39 +++++++++++++++------------------------
 hw/dma/i8257.c       | 20 +++++++++++++-------
 include/hw/isa/isa.h |  1 -
 3 files changed, 28 insertions(+), 32 deletions(-)

diff --git a/hw/block/fdc.c b/hw/block/fdc.c
index ac5d31e8c1..18fd22bfb7 100644
--- a/hw/block/fdc.c
+++ b/hw/block/fdc.c
@@ -1716,9 +1716,8 @@ static void fdctrl_start_transfer(FDCtrl *fdctrl, int direction)
     if (fdctrl->dor & FD_DOR_DMAEN) {
         IsaDmaTransferMode dma_mode;
         IsaDmaClass *k = ISADMA_GET_CLASS(fdctrl->dma);
-        bool dma_mode_ok;
+
         /* DMA transfer are enabled. Check if DMA channel is well programmed */
-        dma_mode = k->get_transfer_mode(fdctrl->dma, fdctrl->dma_chann);
         FLOPPY_DPRINTF("dma_mode=%d direction=%d (%d - %d)\n",
                        dma_mode, direction,
                        (128 << fdctrl->fifo[5]) *
@@ -1727,40 +1726,32 @@ static void fdctrl_start_transfer(FDCtrl *fdctrl, int direction)
         case FD_DIR_SCANE:
         case FD_DIR_SCANL:
         case FD_DIR_SCANH:
-            dma_mode_ok = (dma_mode == ISADMA_TRANSFER_VERIFY);
             break;
         case FD_DIR_WRITE:
-            dma_mode_ok = (dma_mode == ISADMA_TRANSFER_WRITE);
             break;
         case FD_DIR_READ:
-            dma_mode_ok = (dma_mode == ISADMA_TRANSFER_READ);
             break;
         case FD_DIR_VERIFY:
-            dma_mode_ok = true;
             break;
         default:
-            dma_mode_ok = false;
             break;
         }
-        if (dma_mode_ok) {
-            /* No access is allowed until DMA transfer has completed */
-            fdctrl->msr &= ~FD_MSR_RQM;
-            if (direction != FD_DIR_VERIFY) {
-                /* Now, we just have to wait for the DMA controller to
-                 * recall us...
-                 */
-                k->hold_DREQ(fdctrl->dma, fdctrl->dma_chann);
-                k->schedule(fdctrl->dma);
-            } else {
-                /* Start transfer */
-                fdctrl_transfer_handler(fdctrl, fdctrl->dma_chann, 0,
-                                        fdctrl->data_len);
-            }
-            return;
+
+        /* No access is allowed until DMA transfer has completed */
+        fdctrl->msr &= ~FD_MSR_RQM;
+        if (direction != FD_DIR_VERIFY) {
+            /*
+             * Now, we just have to wait for the DMA controller to
+             * recall us...
+             */
+            k->hold_DREQ(fdctrl->dma, fdctrl->dma_chann);
+            k->schedule(fdctrl->dma);
         } else {
-            FLOPPY_DPRINTF("bad dma_mode=%d direction=%d\n", dma_mode,
-                           direction);
+            /* Start transfer */
+            fdctrl_transfer_handler(fdctrl, fdctrl->dma_chann, 0,
+                    fdctrl->data_len);
         }
+        return;
     }
     FLOPPY_DPRINTF("start non-DMA transfer\n");
     fdctrl->msr |= FD_MSR_NONDMA | FD_MSR_RQM;
diff --git a/hw/dma/i8257.c b/hw/dma/i8257.c
index 792f617eb4..85dad3d391 100644
--- a/hw/dma/i8257.c
+++ b/hw/dma/i8257.c
@@ -292,12 +292,6 @@ static uint64_t i8257_read_cont(void *opaque, hwaddr nport, unsigned size)
     return val;
 }
 
-static IsaDmaTransferMode i8257_dma_get_transfer_mode(IsaDma *obj, int nchan)
-{
-    I8257State *d = I8257(obj);
-    return (d->regs[nchan & 3].mode >> 2) & 3;
-}
-
 static bool i8257_dma_has_autoinitialization(IsaDma *obj, int nchan)
 {
     I8257State *d = I8257(obj);
@@ -400,6 +394,11 @@ static void i8257_dma_register_channel(IsaDma *obj, int nchan,
     r->opaque = opaque;
 }
 
+static bool i8257_is_verify_transfer(I8257Regs *r)
+{
+    return (r->mode & 0x0c) == 0;
+}
+
 static int i8257_dma_read_memory(IsaDma *obj, int nchan, void *buf, int pos,
                                  int len)
 {
@@ -407,6 +406,10 @@ static int i8257_dma_read_memory(IsaDma *obj, int nchan, void *buf, int pos,
     I8257Regs *r = &d->regs[nchan & 3];
     hwaddr addr = ((r->pageh & 0x7f) << 24) | (r->page << 16) | r->now[ADDR];
 
+    if (i8257_is_verify_transfer(r)) {
+        return len;
+    }
+
     if (r->mode & 0x20) {
         int i;
         uint8_t *p = buf;
@@ -431,6 +434,10 @@ static int i8257_dma_write_memory(IsaDma *obj, int nchan, void *buf, int pos,
     I8257Regs *r = &s->regs[nchan & 3];
     hwaddr addr = ((r->pageh & 0x7f) << 24) | (r->page << 16) | r->now[ADDR];
 
+    if (i8257_is_verify_transfer(r)) {
+        return len;
+    }
+
     if (r->mode & 0x20) {
         int i;
         uint8_t *p = buf;
@@ -597,7 +604,6 @@ static void i8257_class_init(ObjectClass *klass, void *data)
     dc->vmsd = &vmstate_i8257;
     dc->props = i8257_properties;
 
-    idc->get_transfer_mode = i8257_dma_get_transfer_mode;
     idc->has_autoinitialization = i8257_dma_has_autoinitialization;
     idc->read_memory = i8257_dma_read_memory;
     idc->write_memory = i8257_dma_write_memory;
diff --git a/include/hw/isa/isa.h b/include/hw/isa/isa.h
index 018ada4f6f..f516e253c5 100644
--- a/include/hw/isa/isa.h
+++ b/include/hw/isa/isa.h
@@ -56,7 +56,6 @@ typedef int (*IsaDmaTransferHandler)(void *opaque, int nchan, int pos,
 typedef struct IsaDmaClass {
     InterfaceClass parent;
 
-    IsaDmaTransferMode (*get_transfer_mode)(IsaDma *obj, int nchan);
     bool (*has_autoinitialization)(IsaDma *obj, int nchan);
     int (*read_memory)(IsaDma *obj, int nchan, void *buf, int pos, int len);
     int (*write_memory)(IsaDma *obj, int nchan, void *buf, int pos, int len);
-- 
2.23.0



^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH v2] fdc/i8257: implement verify transfer mode
  2019-10-30  8:28 [PATCH v2] fdc/i8257: implement verify transfer mode Sven Schnelle
@ 2019-11-01 10:54 ` Hervé Poussineau
  2019-11-01 12:33   ` John Snow
  0 siblings, 1 reply; 3+ messages in thread
From: Hervé Poussineau @ 2019-11-01 10:54 UTC (permalink / raw)
  To: Sven Schnelle, John Snow
  Cc: Kevin Wolf, open list:Floppy, Michael S. Tsirkin,
	open list:All patches CC here, Max Reitz, Paolo Bonzini

Le 30/10/2019 à 09:28, Sven Schnelle a écrit :
> While working on the Tulip driver i tried to write some Teledisk images to
> a floppy image which didn't work. Turned out that Teledisk checks the written
> data by issuing a READ command to the FDC but running the DMA controller
> in VERIFY mode. As we ignored the DMA request in that case, the DMA transfer
> never finished, and Teledisk reported an error.
> 
> The i8257 spec says about verify transfers:
> 
> 3) DMA verify, which does not actually involve the transfer of data. When an
> 8257 channel is in the DMA verify mode, it will respond the same as described
> for transfer operations, except that no memory or I/O read/write control signals
> will be generated.
> 
> Hervé proposed to remove all the dma_mode_ok stuff from fdc to have a more
> clear boundary between DMA and FDC, so this patch also does that.
> 
> Suggested-by: Hervé Poussineau <hpoussin@reactos.org>
> Signed-off-by: Sven Schnelle <svens@stackframe.org>
> ---
>   hw/block/fdc.c       | 39 +++++++++++++++------------------------
>   hw/dma/i8257.c       | 20 +++++++++++++-------
>   include/hw/isa/isa.h |  1 -
>   3 files changed, 28 insertions(+), 32 deletions(-)
> 
> diff --git a/hw/block/fdc.c b/hw/block/fdc.c
> index ac5d31e8c1..18fd22bfb7 100644
> --- a/hw/block/fdc.c
> +++ b/hw/block/fdc.c
> @@ -1716,9 +1716,8 @@ static void fdctrl_start_transfer(FDCtrl *fdctrl, int direction)
>       if (fdctrl->dor & FD_DOR_DMAEN) {
>           IsaDmaTransferMode dma_mode;

You need to remove this dma_mode variable because you don't set it anymore.

>           IsaDmaClass *k = ISADMA_GET_CLASS(fdctrl->dma);
> -        bool dma_mode_ok;
> +
>           /* DMA transfer are enabled. Check if DMA channel is well programmed */

Second part of this comment should be removed.

> -        dma_mode = k->get_transfer_mode(fdctrl->dma, fdctrl->dma_chann);
>           FLOPPY_DPRINTF("dma_mode=%d direction=%d (%d - %d)\n",
>                          dma_mode, direction,
>                          (128 << fdctrl->fifo[5]) *

You need to remove dma_mode variable from printf statement, as you removed the variable.

> @@ -1727,40 +1726,32 @@ static void fdctrl_start_transfer(FDCtrl *fdctrl, int direction)
>           case FD_DIR_SCANE:
>           case FD_DIR_SCANL:
>           case FD_DIR_SCANH:
> -            dma_mode_ok = (dma_mode == ISADMA_TRANSFER_VERIFY);
>               break;
>           case FD_DIR_WRITE:
> -            dma_mode_ok = (dma_mode == ISADMA_TRANSFER_WRITE);
>               break;
>           case FD_DIR_READ:
> -            dma_mode_ok = (dma_mode == ISADMA_TRANSFER_READ);
>               break;
>           case FD_DIR_VERIFY:
> -            dma_mode_ok = true;
>               break;
>           default:
> -            dma_mode_ok = false;
>               break;
>           }

Now that you have removed the dma_mode_ok instructions, you have a switch where all cases do nothing.
Please completly remove the switch statement.

> -        if (dma_mode_ok) {
> -            /* No access is allowed until DMA transfer has completed */
> -            fdctrl->msr &= ~FD_MSR_RQM;
> -            if (direction != FD_DIR_VERIFY) {
> -                /* Now, we just have to wait for the DMA controller to
> -                 * recall us...
> -                 */
> -                k->hold_DREQ(fdctrl->dma, fdctrl->dma_chann);
> -                k->schedule(fdctrl->dma);
> -            } else {
> -                /* Start transfer */
> -                fdctrl_transfer_handler(fdctrl, fdctrl->dma_chann, 0,
> -                                        fdctrl->data_len);
> -            }
> -            return;
> +
> +        /* No access is allowed until DMA transfer has completed */
> +        fdctrl->msr &= ~FD_MSR_RQM;
> +        if (direction != FD_DIR_VERIFY) {
> +            /*
> +             * Now, we just have to wait for the DMA controller to
> +             * recall us...
> +             */
> +            k->hold_DREQ(fdctrl->dma, fdctrl->dma_chann);
> +            k->schedule(fdctrl->dma);
>           } else {
> -            FLOPPY_DPRINTF("bad dma_mode=%d direction=%d\n", dma_mode,
> -                           direction);
> +            /* Start transfer */
> +            fdctrl_transfer_handler(fdctrl, fdctrl->dma_chann, 0,
> +                    fdctrl->data_len);
>           }
> +        return;
>       }
>       FLOPPY_DPRINTF("start non-DMA transfer\n");
>       fdctrl->msr |= FD_MSR_NONDMA | FD_MSR_RQM;
> diff --git a/hw/dma/i8257.c b/hw/dma/i8257.c
> index 792f617eb4..85dad3d391 100644
> --- a/hw/dma/i8257.c
> +++ b/hw/dma/i8257.c
> @@ -292,12 +292,6 @@ static uint64_t i8257_read_cont(void *opaque, hwaddr nport, unsigned size)
>       return val;
>   }
>   
> -static IsaDmaTransferMode i8257_dma_get_transfer_mode(IsaDma *obj, int nchan)
> -{
> -    I8257State *d = I8257(obj);
> -    return (d->regs[nchan & 3].mode >> 2) & 3;
> -}
> -
>   static bool i8257_dma_has_autoinitialization(IsaDma *obj, int nchan)
>   {
>       I8257State *d = I8257(obj);
> @@ -400,6 +394,11 @@ static void i8257_dma_register_channel(IsaDma *obj, int nchan,
>       r->opaque = opaque;
>   }
>   
> +static bool i8257_is_verify_transfer(I8257Regs *r)
> +{
> +    return (r->mode & 0x0c) == 0;
> +}
> +
>   static int i8257_dma_read_memory(IsaDma *obj, int nchan, void *buf, int pos,
>                                    int len)
>   {
> @@ -407,6 +406,10 @@ static int i8257_dma_read_memory(IsaDma *obj, int nchan, void *buf, int pos,
>       I8257Regs *r = &d->regs[nchan & 3];
>       hwaddr addr = ((r->pageh & 0x7f) << 24) | (r->page << 16) | r->now[ADDR];
>   
> +    if (i8257_is_verify_transfer(r)) {
> +        return len;
> +    }
> +
>       if (r->mode & 0x20) {
>           int i;
>           uint8_t *p = buf;
> @@ -431,6 +434,10 @@ static int i8257_dma_write_memory(IsaDma *obj, int nchan, void *buf, int pos,
>       I8257Regs *r = &s->regs[nchan & 3];
>       hwaddr addr = ((r->pageh & 0x7f) << 24) | (r->page << 16) | r->now[ADDR];
>   
> +    if (i8257_is_verify_transfer(r)) {
> +        return len;
> +    }
> +
>       if (r->mode & 0x20) {
>           int i;
>           uint8_t *p = buf;
> @@ -597,7 +604,6 @@ static void i8257_class_init(ObjectClass *klass, void *data)
>       dc->vmsd = &vmstate_i8257;
>       dc->props = i8257_properties;
>   
> -    idc->get_transfer_mode = i8257_dma_get_transfer_mode;
>       idc->has_autoinitialization = i8257_dma_has_autoinitialization;
>       idc->read_memory = i8257_dma_read_memory;
>       idc->write_memory = i8257_dma_write_memory;
> diff --git a/include/hw/isa/isa.h b/include/hw/isa/isa.h
> index 018ada4f6f..f516e253c5 100644
> --- a/include/hw/isa/isa.h
> +++ b/include/hw/isa/isa.h
> @@ -56,7 +56,6 @@ typedef int (*IsaDmaTransferHandler)(void *opaque, int nchan, int pos,
>   typedef struct IsaDmaClass {
>       InterfaceClass parent;
>   
> -    IsaDmaTransferMode (*get_transfer_mode)(IsaDma *obj, int nchan);
>       bool (*has_autoinitialization)(IsaDma *obj, int nchan);
>       int (*read_memory)(IsaDma *obj, int nchan, void *buf, int pos, int len);
>       int (*write_memory)(IsaDma *obj, int nchan, void *buf, int pos, int len);
> 

Otherwise, the i8257.c parts look good. This might fix some other devices (except fdc) which might use VERIFY mode.

Hervé


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH v2] fdc/i8257: implement verify transfer mode
  2019-11-01 10:54 ` Hervé Poussineau
@ 2019-11-01 12:33   ` John Snow
  0 siblings, 0 replies; 3+ messages in thread
From: John Snow @ 2019-11-01 12:33 UTC (permalink / raw)
  To: Hervé Poussineau, Sven Schnelle
  Cc: Kevin Wolf, open list:Floppy, Michael S. Tsirkin,
	open list:All patches CC here, Max Reitz, Paolo Bonzini



On 11/1/19 6:54 AM, Hervé Poussineau wrote:
> Le 30/10/2019 à 09:28, Sven Schnelle a écrit :
>> While working on the Tulip driver i tried to write some Teledisk
>> images to
>> a floppy image which didn't work. Turned out that Teledisk checks the
>> written
>> data by issuing a READ command to the FDC but running the DMA controller
>> in VERIFY mode. As we ignored the DMA request in that case, the DMA
>> transfer
>> never finished, and Teledisk reported an error.
>>
>> The i8257 spec says about verify transfers:
>>
>> 3) DMA verify, which does not actually involve the transfer of data.
>> When an
>> 8257 channel is in the DMA verify mode, it will respond the same as
>> described
>> for transfer operations, except that no memory or I/O read/write
>> control signals
>> will be generated.
>>
>> Hervé proposed to remove all the dma_mode_ok stuff from fdc to have a
>> more
>> clear boundary between DMA and FDC, so this patch also does that.
>>
>> Suggested-by: Hervé Poussineau <hpoussin@reactos.org>
>> Signed-off-by: Sven Schnelle <svens@stackframe.org>
>> ---
>>   hw/block/fdc.c       | 39 +++++++++++++++------------------------
>>   hw/dma/i8257.c       | 20 +++++++++++++-------
>>   include/hw/isa/isa.h |  1 -
>>   3 files changed, 28 insertions(+), 32 deletions(-)
>>
>> diff --git a/hw/block/fdc.c b/hw/block/fdc.c
>> index ac5d31e8c1..18fd22bfb7 100644
>> --- a/hw/block/fdc.c
>> +++ b/hw/block/fdc.c
>> @@ -1716,9 +1716,8 @@ static void fdctrl_start_transfer(FDCtrl
>> *fdctrl, int direction)
>>       if (fdctrl->dor & FD_DOR_DMAEN) {
>>           IsaDmaTransferMode dma_mode;
> 
> You need to remove this dma_mode variable because you don't set it anymore.
> 
>>           IsaDmaClass *k = ISADMA_GET_CLASS(fdctrl->dma);
>> -        bool dma_mode_ok;
>> +
>>           /* DMA transfer are enabled. Check if DMA channel is well
>> programmed */
> 
> Second part of this comment should be removed.
> 
>> -        dma_mode = k->get_transfer_mode(fdctrl->dma, fdctrl->dma_chann);
>>           FLOPPY_DPRINTF("dma_mode=%d direction=%d (%d - %d)\n",
>>                          dma_mode, direction,
>>                          (128 << fdctrl->fifo[5]) *
> 
> You need to remove dma_mode variable from printf statement, as you
> removed the variable.
> 
>> @@ -1727,40 +1726,32 @@ static void fdctrl_start_transfer(FDCtrl
>> *fdctrl, int direction)
>>           case FD_DIR_SCANE:
>>           case FD_DIR_SCANL:
>>           case FD_DIR_SCANH:
>> -            dma_mode_ok = (dma_mode == ISADMA_TRANSFER_VERIFY);
>>               break;
>>           case FD_DIR_WRITE:
>> -            dma_mode_ok = (dma_mode == ISADMA_TRANSFER_WRITE);
>>               break;
>>           case FD_DIR_READ:
>> -            dma_mode_ok = (dma_mode == ISADMA_TRANSFER_READ);
>>               break;
>>           case FD_DIR_VERIFY:
>> -            dma_mode_ok = true;
>>               break;
>>           default:
>> -            dma_mode_ok = false;
>>               break;
>>           }
> 
> Now that you have removed the dma_mode_ok instructions, you have a
> switch where all cases do nothing.
> Please completly remove the switch statement.
> 
>> -        if (dma_mode_ok) {
>> -            /* No access is allowed until DMA transfer has completed */
>> -            fdctrl->msr &= ~FD_MSR_RQM;
>> -            if (direction != FD_DIR_VERIFY) {
>> -                /* Now, we just have to wait for the DMA controller to
>> -                 * recall us...
>> -                 */
>> -                k->hold_DREQ(fdctrl->dma, fdctrl->dma_chann);
>> -                k->schedule(fdctrl->dma);
>> -            } else {
>> -                /* Start transfer */
>> -                fdctrl_transfer_handler(fdctrl, fdctrl->dma_chann, 0,
>> -                                        fdctrl->data_len);
>> -            }
>> -            return;
>> +
>> +        /* No access is allowed until DMA transfer has completed */
>> +        fdctrl->msr &= ~FD_MSR_RQM;
>> +        if (direction != FD_DIR_VERIFY) {
>> +            /*
>> +             * Now, we just have to wait for the DMA controller to
>> +             * recall us...
>> +             */
>> +            k->hold_DREQ(fdctrl->dma, fdctrl->dma_chann);
>> +            k->schedule(fdctrl->dma);
>>           } else {
>> -            FLOPPY_DPRINTF("bad dma_mode=%d direction=%d\n", dma_mode,
>> -                           direction);
>> +            /* Start transfer */
>> +            fdctrl_transfer_handler(fdctrl, fdctrl->dma_chann, 0,
>> +                    fdctrl->data_len);
>>           }
>> +        return;
>>       }
>>       FLOPPY_DPRINTF("start non-DMA transfer\n");
>>       fdctrl->msr |= FD_MSR_NONDMA | FD_MSR_RQM;
>> diff --git a/hw/dma/i8257.c b/hw/dma/i8257.c
>> index 792f617eb4..85dad3d391 100644
>> --- a/hw/dma/i8257.c
>> +++ b/hw/dma/i8257.c
>> @@ -292,12 +292,6 @@ static uint64_t i8257_read_cont(void *opaque,
>> hwaddr nport, unsigned size)
>>       return val;
>>   }
>>   -static IsaDmaTransferMode i8257_dma_get_transfer_mode(IsaDma *obj,
>> int nchan)
>> -{
>> -    I8257State *d = I8257(obj);
>> -    return (d->regs[nchan & 3].mode >> 2) & 3;
>> -}
>> -
>>   static bool i8257_dma_has_autoinitialization(IsaDma *obj, int nchan)
>>   {
>>       I8257State *d = I8257(obj);
>> @@ -400,6 +394,11 @@ static void i8257_dma_register_channel(IsaDma
>> *obj, int nchan,
>>       r->opaque = opaque;
>>   }
>>   +static bool i8257_is_verify_transfer(I8257Regs *r)
>> +{
>> +    return (r->mode & 0x0c) == 0;
>> +}
>> +
>>   static int i8257_dma_read_memory(IsaDma *obj, int nchan, void *buf,
>> int pos,
>>                                    int len)
>>   {
>> @@ -407,6 +406,10 @@ static int i8257_dma_read_memory(IsaDma *obj, int
>> nchan, void *buf, int pos,
>>       I8257Regs *r = &d->regs[nchan & 3];
>>       hwaddr addr = ((r->pageh & 0x7f) << 24) | (r->page << 16) |
>> r->now[ADDR];
>>   +    if (i8257_is_verify_transfer(r)) {
>> +        return len;
>> +    }
>> +
>>       if (r->mode & 0x20) {
>>           int i;
>>           uint8_t *p = buf;
>> @@ -431,6 +434,10 @@ static int i8257_dma_write_memory(IsaDma *obj,
>> int nchan, void *buf, int pos,
>>       I8257Regs *r = &s->regs[nchan & 3];
>>       hwaddr addr = ((r->pageh & 0x7f) << 24) | (r->page << 16) |
>> r->now[ADDR];
>>   +    if (i8257_is_verify_transfer(r)) {
>> +        return len;
>> +    }
>> +
>>       if (r->mode & 0x20) {
>>           int i;
>>           uint8_t *p = buf;
>> @@ -597,7 +604,6 @@ static void i8257_class_init(ObjectClass *klass,
>> void *data)
>>       dc->vmsd = &vmstate_i8257;
>>       dc->props = i8257_properties;
>>   -    idc->get_transfer_mode = i8257_dma_get_transfer_mode;
>>       idc->has_autoinitialization = i8257_dma_has_autoinitialization;
>>       idc->read_memory = i8257_dma_read_memory;
>>       idc->write_memory = i8257_dma_write_memory;
>> diff --git a/include/hw/isa/isa.h b/include/hw/isa/isa.h
>> index 018ada4f6f..f516e253c5 100644
>> --- a/include/hw/isa/isa.h
>> +++ b/include/hw/isa/isa.h
>> @@ -56,7 +56,6 @@ typedef int (*IsaDmaTransferHandler)(void *opaque,
>> int nchan, int pos,
>>   typedef struct IsaDmaClass {
>>       InterfaceClass parent;
>>   -    IsaDmaTransferMode (*get_transfer_mode)(IsaDma *obj, int nchan);
>>       bool (*has_autoinitialization)(IsaDma *obj, int nchan);
>>       int (*read_memory)(IsaDma *obj, int nchan, void *buf, int pos,
>> int len);
>>       int (*write_memory)(IsaDma *obj, int nchan, void *buf, int pos,
>> int len);
>>
> 
> Otherwise, the i8257.c parts look good. This might fix some other
> devices (except fdc) which might use VERIFY mode.
> 
> Hervé

Thanks for the look, Hervé!

(Hey, do you want the Floppy device maintainership?)

--js



^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2019-11-01 12:35 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-10-30  8:28 [PATCH v2] fdc/i8257: implement verify transfer mode Sven Schnelle
2019-11-01 10:54 ` Hervé Poussineau
2019-11-01 12:33   ` John Snow

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).