[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH 2/3 v6] Replace bdrv_* to bdrv_aio_* functions i

From: Hervé Poussineau
Subject: Re: [Qemu-devel] [PATCH 2/3 v6] Replace bdrv_* to bdrv_aio_* functions in DMA mode in fdc.c
Date: Tue, 15 May 2012 22:49:54 +0200
User-agent: Thunderbird (Windows/20090812)


Paolo Bonzini a écrit :
Il 15/05/2012 11:17, Li Zhi Hui ha scritto:
Signed-off-by: Paolo Bonzini <address@hidden>
Signed-off-by: Li Zhi Hui <address@hidden>
 hw/fdc.c |  313 +++++++++++++++++++++++++++++++++++++++++--------------------
 1 files changed, 210 insertions(+), 103 deletions(-)

To reviewers, this is obviously missing migration support. :)

It is also missing a good commit message.  It should say that the
support in the existing code for scan commands has a lot of "weirdness",
for example:

-                if ((ret < 0 && fdctrl->data_dir == FD_DIR_SCANL) ||
-                    (ret > 0 && fdctrl->data_dir == FD_DIR_SCANH)) {
-                    status2 = 0x00;
-                    goto end_transfer;


- end_transfer:
-    len = fdctrl->data_pos - start_pos;
-    FLOPPY_DPRINTF("end transfer %d %d %d\n",
-                   fdctrl->data_pos, len, fdctrl->data_len);
-    if (fdctrl->data_dir == FD_DIR_SCANE ||
-        fdctrl->data_dir == FD_DIR_SCANL ||
-        fdctrl->data_dir == FD_DIR_SCANH)
-        status2 = FD_SR2_SEH;

which blindly overwrites status2.  Hence the new code was not written
based on it.  However, the new code is untested as far as I know.

+            if (fdctrl->data_dir == FD_DIR_SCANE ||
+                fdctrl->data_dir == FD_DIR_SCANL ||
+                fdctrl->data_dir == FD_DIR_SCANH) {
+                fdctrl->dma_status2 = FD_SR2_SEH;
+            }

This should be FD_SR2_SNS (Spec for the FDC is at
http://www.cepece.info/amstrad/docs/i8272/8272sp.htm: "If the conditions
for scan are not met between the the starting sector (as specified by R)
and the last sector on the cylinder (EOT), then the FDC sets the SN
(Scan Not Satisfied) flag of Status Register 2 to a 1 (high), and
terminates the Scan Command").

I'm not sure whether scan commands should be kept at all though.  Hervé,
do you know anything that uses them?

No, I don't know anything which uses it.
I will ack patches which remove the scan command implementations and call the unimplemented command handler instead (like for format_and_write command).



reply via email to

[Prev in Thread] Current Thread [Next in Thread]