qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: "J. Mayer" <l_indien@magic.fr>
To: qemu-devel@nongnu.org
Subject: Re: [Qemu-devel] [PATCH] FDC: rework status0, status1, status2 handling
Date: Thu, 26 Jun 2008 09:53:01 +0200	[thread overview]
Message-ID: <1214466781.29120.14.camel@localhost> (raw)
In-Reply-To: <4862B7CB.8040407@reactos.org>

On Wed, 2008-06-25 at 23:25 +0200, Hervé Poussineau wrote:
> Hi,
> 
> Attached patch fixes status0, status1 and status2 handling, which were 
> broken in read/write commands and in some subtle ways. Status values are 
> now calculated during command execution and not at the very end.
> This allows removing of one hack in  fdctrl_handle_sense_interrupt_status().

Hi,

this way of proceeding seem very strange, as actual hardware do compute
the status bytes in the FIFO when entering the status phase. This is
quite logical as those bytes are supposed to reflect the state of the
hardware at the end of the command completion and cannot be computed
before, apart from a few bits, like the DID_SEEK bit.
Furthermore, doing this way, you have to introduce weird hack in the
main access routine :
@@ -1811,6 +1781,15 @@
>              return;
>          }
>  
> +        if (fdctrl->fifo[0] != FD_CMD_SENSE_INTERRUPT_STATUS)
> +        {
> +            /* Reset status0, except for SENSE_INTERRUPT_STATUS
> +             * which returns it */
> +            fdctrl->status0 = 0;
> +        }
> +        fdctrl->status1 = 0;
> +        fdctrl->status2 = 0;
> +

what is the exact bug and why isn't it possible to try to going on doing
things like actual hardware do ?
Another remark: moving code, like you do in this patch make very
difficult to figure what the real changes are and to track the
regressions if any. Could you please avoid introducing such artificial
diffs ?

-- 
J. Mayer <l_indien@magic.fr>
Never organized

      reply	other threads:[~2008-06-26  7:53 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-06-25 21:25 [Qemu-devel] [PATCH] FDC: rework status0, status1, status2 handling Hervé Poussineau
2008-06-26  7:53 ` J. Mayer [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1214466781.29120.14.camel@localhost \
    --to=l_indien@magic.fr \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).